[SLOF] [PATCH v3 18/22] virtio: add and enable 1.0 capability parsing
Thomas Huth
thuth at redhat.com
Wed Jan 27 20:05:45 AEDT 2016
On 22.01.2016 11:55, Nikunj A Dadhania wrote:
> Introduce parsing routines for virtio capabilities. This would also
> determine whether we need to function in legacy mode or virtio 1.0.
>
> Drivers need to negotiate the 1.0 feature capability before starting to
> use 1.0.
>
> Disable all the drivers until 1.0 is enabled.
>
> Signed-off-by: Nikunj A Dadhania <nikunj at linux.vnet.ibm.com>
> ---
> board-qemu/slof/virtio.fs | 1 +
> lib/libvirtio/virtio-9p.c | 3 ++
> lib/libvirtio/virtio-blk.c | 3 ++
> lib/libvirtio/virtio-net.c | 3 ++
> lib/libvirtio/virtio-scsi.c | 3 ++
> lib/libvirtio/virtio.c | 117 ++++++++++++++++++++++++++++++++++++++++++++
> lib/libvirtio/virtio.code | 6 +++
> lib/libvirtio/virtio.h | 1 +
> lib/libvirtio/virtio.in | 2 +
> 9 files changed, 139 insertions(+)
...
> diff --git a/lib/libvirtio/virtio.c b/lib/libvirtio/virtio.c
> index 04d3125..0264baa 100644
> --- a/lib/libvirtio/virtio.c
> +++ b/lib/libvirtio/virtio.c
> @@ -14,6 +14,7 @@
> #include <stdbool.h>
> #include <stdlib.h>
> #include <string.h>
> +#include <stddef.h>
> #include <cpu.h>
> #include <cache.h>
> #include <byteorder.h>
> @@ -31,6 +32,36 @@
> #define VIRTIOHDR_ISR_STATUS 19
> #define VIRTIOHDR_DEVICE_CONFIG 20
>
> +/* PCI defines */
> +#define PCI_BASE_ADDR_SPACE_IO 0x01
> +#define PCI_BASE_ADDR_SPACE_MEM 0x00
That PCI_BASE_ADDR_SPACE_MEM looks a little bit useless and is also
completely unused, as far as I can see... maybe simply remove it?
> +#define PCI_BASE_ADDR_MEM_MASK (~0x0fUL)
> +#define PCI_BASE_ADDR_IO_MASK (~0x03UL)
> +
> +#define PCI_BASE_ADDR_REG_0 0x10
> +#define PCI_CONFIG_CAP_REG 0x34
> +
> +#define PCI_CAP_ID_VNDR 0x9
> +
> +/* Common configuration */
> +#define VIRTIO_PCI_CAP_COMMON_CFG 1
> +/* Notifications */
> +#define VIRTIO_PCI_CAP_NOTIFY_CFG 2
> +/* ISR access */
> +#define VIRTIO_PCI_CAP_ISR_CFG 3
> +/* Device specific configuration */
> +#define VIRTIO_PCI_CAP_DEVICE_CFG 4
> +/* PCI configuration access */
> +#define VIRTIO_PCI_CAP_PCI_CFG 5
> +
> +#define VIRTIO_PCI_CAP_VNDR 0 /* Generic PCI field: PCI_CAP_ID_VNDR */
> +#define VIRTIO_PCI_CAP_NEXT 1 /* Generic PCI field: next ptr. */
> +#define VIRTIO_PCI_CAP_LEN 2 /* Generic PCI field: capability length */
> +#define VIRTIO_PCI_CAP_CFG_TYPE 3 /* Identifies the structure. */
> +#define VIRTIO_PCI_CAP_BAR 4 /* Where to find it. */
> +#define VIRTIO_PCI_CAP_OFFSET 8 /* Offset within bar. */
> +#define VIRTIO_PCI_CAP_LENGTH 12 /* Length of the structure, in bytes. */
> +
> struct virtio_dev_common {
> le32 dev_features_sel;
> le32 dev_features;
> @@ -76,6 +107,92 @@ static uint64_t virtio_pci_read64(void *addr)
> return (hi << 32) | lo;
> }
>
> +static void virtio_process_cap(struct virtio_device *dev, uint8_t cap_ptr)
> +{
> + struct virtio_cap *cap;
> + uint8_t cfg_type, bar;
> + uint32_t offset;
> + uint64_t addr = 0;
> +
> + cfg_type = SLOF_pci_config_read8(cap_ptr + VIRTIO_PCI_CAP_CFG_TYPE);
> + bar = SLOF_pci_config_read8(cap_ptr + VIRTIO_PCI_CAP_BAR);
> + offset = SLOF_pci_config_read32(cap_ptr + VIRTIO_PCI_CAP_OFFSET);
> +
> + switch(cfg_type) {
> + case VIRTIO_PCI_CAP_COMMON_CFG:
> + cap = &dev->common;
> + break;
> + case VIRTIO_PCI_CAP_NOTIFY_CFG:
> + cap = &dev->notify;
> + dev->notify_off_mul = SLOF_pci_config_read32(cap_ptr + sizeof(struct virtio_cap));
> + break;
> + case VIRTIO_PCI_CAP_ISR_CFG:
> + cap = &dev->isr;
> + break;
> + case VIRTIO_PCI_CAP_DEVICE_CFG:
> + cap = &dev->device;
> + break;
> + default:
> + return;
> + }
> +
> + cap->bar = bar;
> + addr = SLOF_pci_config_read32(PCI_BASE_ADDR_REG_0 + 4 * cap->bar);
> + if (addr & PCI_BASE_ADDR_SPACE_IO) {
> + cap->is_io = 1;
> + addr = addr & PCI_BASE_ADDR_IO_MASK;
> + } else if (addr & 4) {
Maybe add a #define for "4", too, just as you did with PCI_BASE_ADDR_SPACE_IO?
> + addr |= SLOF_pci_config_read32(PCI_BASE_ADDR_REG_0 + 4 * (cap->bar + 1)) << 32;
> + addr = addr & PCI_BASE_ADDR_MEM_MASK;
> + addr = (uint64_t)SLOF_translate_my_address((void *)addr);
No "cap->is_io = 0;" in this case here?
> + } else {
> + cap->is_io = 0;
> + addr = addr & PCI_BASE_ADDR_MEM_MASK;
> + addr = (uint64_t)SLOF_translate_my_address((void *)addr);
> + }
Maybe you could simplify the above code as follows:
if (addr & PCI_BASE_ADDR_SPACE_IO) {
addr = addr & PCI_BASE_ADDR_IO_MASK;
cap->is_io = 1;
} else {
if (addr & PCI_BASE_ADDR_SPACE_64BIT)
addr |= SLOF_pci_config_read32(PCI_BASE_ADDR_REG_0 + 4 * (cap->bar + 1)) << 32;
addr = addr & PCI_BASE_ADDR_MEM_MASK;
addr = (uint64_t)SLOF_translate_my_address((void *)addr);
cap->is_io = 0;
}
> + cap->addr = (void *)addr + offset;
> + cap->cap_id = cfg_type;
> + return;
Superfluous return statement.
> +}
> +
> +/**
> + * Reads the virtio device capabilities, gets called from SLOF routines The
> + * function determines legacy or modern device and sets up driver registers
> + */
> +void virtio_parse_capabilities(struct virtio_device *dev)
> +{
> + uint8_t cap_ptr, cap_vndr;
> + uint64_t addr = 0;
> +
> + cap_ptr = SLOF_pci_config_read8(PCI_CONFIG_CAP_REG);
> + while (cap_ptr != 0) {
> + cap_vndr = SLOF_pci_config_read8(cap_ptr + VIRTIO_PCI_CAP_VNDR);
> + if (cap_vndr == PCI_CAP_ID_VNDR)
> + virtio_process_cap(dev, cap_ptr);
> + cap_ptr = SLOF_pci_config_read8(cap_ptr+VIRTIO_PCI_CAP_NEXT);
> + }
> +
> + if (dev->common.cap_id && dev->notify.cap_id &&
> + dev->isr.cap_id && dev->device.cap_id) {
> + dev->is_modern = 1;
> + } else {
> + dev->is_modern = 0;
> + dev->legacy.cap_id = 0;
> + dev->legacy.bar = 0;
> + addr = SLOF_pci_config_read32(PCI_BASE_ADDR_REG_0);
> + if (addr & PCI_BASE_ADDR_SPACE_IO) {
> + dev->legacy.is_io = 1;
> + addr = addr & PCI_BASE_ADDR_IO_MASK;
> + } else {
> + dev->legacy.is_io = 0;
> + addr = addr & PCI_BASE_ADDR_MEM_MASK;
> + addr = (uint64_t)SLOF_translate_my_address((void *)addr);
No support for 64 bit bars in this case?
> + }
> + dev->legacy.addr = (void *)addr;
> + }
> + return;
Superfluous return statement.
> +}
> +
Thomas
More information about the SLOF
mailing list