Re: [Qemu-devel] [PATCH v2 13/34] test/qgraph: virtio_start_device function

[Emanuele <e.emanuelegiuseppe@gmail.com>]

On 08/09/2018 03:39 PM, Laurent Vivier wrote:
> On 06/08/2018 16:33, Emanuele Giuseppe Esposito wrote:
>> This function is intended to group all the qvirtio_* functions that
>> start the qvirtio devices.
>> Applied in all tests using this combination of functions.
>>
>> Signed-off-by: Emanuele Giuseppe Esposito <e.emanuelegiuseppe@gmail.com>
>> ---
>>   tests/libqos/virtio.c    |  8 ++++++++
>>   tests/libqos/virtio.h    |  1 +
>>   tests/vhost-user-test.c  |  5 +----
>>   tests/virtio-9p-test.c   |  6 +-----
>>   tests/virtio-blk-test.c  | 18 ++----------------
>>   tests/virtio-net-test.c  |  6 +-----
>>   tests/virtio-scsi-test.c |  4 +---
>>   7 files changed, 15 insertions(+), 33 deletions(-)
>>
>> diff --git a/tests/libqos/virtio.c b/tests/libqos/virtio.c
>> index 0dad5c19ac..56007ef11b 100644
>> --- a/tests/libqos/virtio.c
>> +++ b/tests/libqos/virtio.c
>> @@ -365,3 +365,11 @@ const char *qvirtio_get_dev_type(void)
>>           return "pci";
>>       }
>>   }
>> +
>> +void qvirtio_start_device(QVirtioDevice *vdev)
>> +{
>> +    qvirtio_reset(vdev);
>> +    qvirtio_set_acknowledge(vdev);
>> +    qvirtio_set_driver(vdev);
>> +    qvirtio_set_driver_ok(vdev);
> I'm not sure you can put qvirtio_set_driver_ok() here.
>
> qvirtio_set_driver_ok() must be called when the driver is OK, it means
> that the virtio device must be fully initialized before (features,
> virtqueue, MSI, ...).
>
> http://docs.oasis-open.org/virtio/virtio/v1.0/virtio-v1.0.pdf
>
> 3.1.1 Driver Requirements: Device Initialization
>
> The driver MUST follow this sequence to initialize a device:
>
> 1. Reset the device.
>
> 2. Set the ACKNOWLEDGE status bit: the guest OS has notice the device.
>
> 3. Set the DRIVER status bit: the guest OS knows how to drive the device.
>
> 4. Read device feature bits, and write the subset of feature bits
> understood by the OS and driver to the
> device. During this step the driver MAY read (but MUST NOT write) the
> device-specific configuration
> fields to check that it can support the device before accepting it.
>
> 5. Set the FEATURES_OK status bit. The driver MUST NOT accept new
> feature bits after this step.
>
> 6. Re-read device status to ensure the FEATURES_OK bit is still set:
> otherwise, the device does not
> support our subset of features and the device is unusable.
>
> 7. Perform device-specific setup, including discovery of virtqueues for
> the device, optional per-bus setup,
> reading and possibly writing the device’s virtio configuration space,
> and population of virtqueues.
>
> 8. Set the DRIVER_OK status bit. At this point the device is “live”.
>
> Thanks,
> Laurent
You're right, I removed qvirtio_set_driver_ok from qvirtio_start_device 
and put it back where it was before.
I noticed that while virtqueues need to have features set before they 
are setup, the device still works even though features are set after the ok.

Thank you,
Emanuele