[SLOF] [PATCH v4 20/23] virtio: add and enable 1.0 capability parsing

[Thomas Huth]

On 28.01.2016 11:24, 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.

s/Disable all/Disable it in all/

> 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      | 109 ++++++++++++++++++++++++++++++++++++++++++++
>  lib/libvirtio/virtio.code   |   6 +++
>  lib/libvirtio/virtio.h      |   1 +
>  lib/libvirtio/virtio.in     |   2 +
>  9 files changed, 131 insertions(+)
...
> +/**
> + * 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;
> +
> +	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;
> +		virtio_cap_set_base_addr(&dev->legacy, 0);
> +	}
> +}

Do you ever use dev->legacy in the code later? struct virtio_device
already contains the "addr" field, which is the pointer to the legacy
BAR address, right? So you might want to remove dev->addr in the end and
use dev->legacy.bar instead, I guess? Or remove the dev->legacy
structure and set the dev->addr field here instead? Either way, you
could finally get rid of the Forth code that reads the legacy BAR in the
virtio-setup-vd function.

Anyway, you could also do this as a separate clean-up patch later
(this patch here basically also looks fine to me as it is right now).

 Thomas