[SLOF] [PATCH v2 16/19] virtio: add and enable 1.0 capability parsing
Thomas Huth
thuth at redhat.com
Thu Jan 21 07:48:54 AEDT 2016
On 20.01.2016 13:10, Nikunj A Dadhania wrote:
> Introcduce 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 | 118 ++++++++++++++++++++++++++++++++++++++++++++
> lib/libvirtio/virtio.code | 6 +++
> lib/libvirtio/virtio.h | 1 +
> lib/libvirtio/virtio.in | 2 +
> 9 files changed, 140 insertions(+)
...
> diff --git a/lib/libvirtio/virtio.c b/lib/libvirtio/virtio.c
> index 0b939fc..12d1b2e 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
> +#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,93 @@ static uint64_t virtio_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:
> + cap = NULL;
Simply return here ...
> + }
> +
> + if (cap && cfg_type) {
... then you can get rid of this if-statement and the code below is less indented.
> + 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) {
> + addr = SLOF_pci_config_read32(PCI_BASE_ADDR_REG_0 + 4 * cap->bar) |
> + (SLOF_pci_config_read32(PCI_BASE_ADDR_REG_0 + 4 * (cap->bar + 1)) << 32);
I think you could simplify the above two lines to:
addr |= SLOF_pci_config_read32(PCI_BASE_ADDR_REG_0 + 4 * (cap->bar + 1)) << 32
since the first 32 bit have already been read.
> + addr = (uint64_t)SLOF_translate_my_address((void *)addr);
> + addr = addr & PCI_BASE_ADDR_MEM_MASK;
I'd maybe swap the above two lines since the mask applies to the PCI
address, not the memory address. (shouldn't make a difference in real
life, but IMHO it's clearer the other way round).
> + } else {
> + cap->is_io = 0;
> + addr = addr & PCI_BASE_ADDR_MEM_MASK;
Isn't a SLOF_translate_my_address() needed for 32-bit addresses, too?
> + }
> + cap->addr = (void *)addr + offset;
> + cap->cap_id = cfg_type;
> + }
> + return;
Superfluous return statement, please remove.
> +}
> +
> +/**
> + * 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;
> + }
> + dev->legacy.addr = (void *)addr;
> + }
> + return;
Superfluous return statement again.
> +}
> +
> /**
> * Calculate ring size according to queue size number
> */
> diff --git a/lib/libvirtio/virtio.code b/lib/libvirtio/virtio.code
> index 258b9bb..c3cb4de 100644
> --- a/lib/libvirtio/virtio.code
> +++ b/lib/libvirtio/virtio.code
> @@ -18,6 +18,12 @@
>
> /******** core virtio ********/
>
> +// : virtio-parse-capabilities ( dev -- )
> +PRIM(virtio_X2d_parse_X2d_capabilities)
> + void *dev = TOS.a; POP;
Wrong indentation in above line?
> + virtio_parse_capabilities(dev);
> +MIRP
> +
Thomas
More information about the SLOF
mailing list