Re: [PATCH] OvmfPkg/VirtioScsiDxe: Allocate all required vrings at VirtioScsiInit

[Maxime Coquelin <maxime.coquelin@redhat.com>]

On 01/11/2018 02:23 PM, Maxime Coquelin wrote:
> Hi Xiang,
> 
> On 12/14/2017 02:25 PM, zhengxiang (A) wrote:
>>
>>
>> On 2017/12/14 17:06, Paolo Bonzini wrote:
>>> On 14/12/2017 07:55, zhengxiang (A) wrote:
>>>> Hello Laszlo and Paolo,
>>>>
>>>> Thanks for your review!
>>>>
>>>> On 2017/12/13 19:16, Laszlo Ersek wrote:
>>>>> On 12/13/17 10:29, Paolo Bonzini wrote:
>>>>>> On 13/12/2017 09:35, Laszlo Ersek wrote:
>>>>>>> Perhaps you can update vhost-scsi similarly to the last patch of
>>>>>>> Maxime's v4 series, even without "VIRTIO_SCSI_F_MQ" -- in the
>>>>>>> SET_FEATURES request handler, just destroy the unused virtqueues 
>>>>>>> that
>>>>>>> have not been configured by the guest driver until that time?
>>>>>> Yes, this is the right solution.  We can assume that if the 
>>>>>> descriptor
>>>>>> address is equal to zero, the queue is not in use.  This is not in 
>>>>>> the
>>>>>> spec as far as I can see, but it is QEMU's assumption.  I will send a
>>>>>> patch to the virtio specification.
>>>>
>>>> I would try this solution! However, is there any possibility that 
>>>> the allocated
>>>> descriptor address is exactly equal to zero and the queue is in use?
>>>
>>> That would break QEMU's virtio implementation, so it's pretty unlikely.
>>>
>>> Paolo
>>>
>>
>> So could I judge the not-in-use queues by adding the below sentence in 
>> order
>> to skip calling vhost_virtqueue_start?
>>
>> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
>> index e4290ce..05c3322 100644
>> --- a/hw/virtio/vhost.c
>> +++ b/hw/virtio/vhost.c
>> @@ -1532,6 +1532,7 @@ int vhost_dev_start(struct vhost_dev *hdev, 
>> VirtIODevice *vdev)
>>           goto fail_mem;
>>       }
>>       for (i = 0; i < hdev->nvqs; ++i) {
>> +        if (virtio_queue_get_desc_addr(vdev, i) == 0) continue;

BTW, the queue index is wrong here, it should be:

if (virtio_queue_get_desc_addr(vdev, hdev->vq_index + i) == 0)
     continue;

>>           r = vhost_virtqueue_start(hdev,
>>                                     vdev,
>>                                     hdev->vqs + i,
>>
> 

Maxime