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

[Nikunj A Dadhania]

Thomas Huth <thuth at redhat.com> writes:

> 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?

I think let me not maintain a separate addr, instead us the
legacy->addr. Will add a new patch for converting this, as there will be
changes in all the accessor routines.

> 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).

I will add one patch after this to take care of this clean-up

Regards
Nikunj