public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Maxime Coquelin <maxime.coquelin@redhat.com>
To: "zhengxiang (A)" <zhengxiang9@huawei.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Laszlo Ersek <lersek@redhat.com>
Cc: edk2-devel@lists.01.org,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	Jordan Justen <jordan.l.justen@intel.com>,
	Shannon Zhao <zhaoshenglong@huawei.com>
Subject: Re: [PATCH] OvmfPkg/VirtioScsiDxe: Allocate all required vrings at VirtioScsiInit
Date: Thu, 11 Jan 2018 14:23:28 +0100	[thread overview]
Message-ID: <8940cc0e-64e7-c253-a8e0-5081eab4efbe@redhat.com> (raw)
In-Reply-To: <835f9103-1892-4963-9bbb-1fa9b417af60@huawei.com>

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;
>           r = vhost_virtqueue_start(hdev,
>                                     vdev,
>                                     hdev->vqs + i,
> 

I think it should work, or you could detect it by checking that desc,
used and avail rings have the same address.

We would need this also for virtio-net, as Windows guest only setup as
much queue pairs as vcpus, but vhost_virtqueue_start is called for all
queue pairs declred in QEMU. With DPDK Vhost-user backend, it turns out
that it uses these uninitialized queues, corrupting guest's physical
address 0.

Do you plan to post the fix, or you'd like me to propose it?

Thanks,
Maxime
> Thanks,
> Xiang
> 


  reply	other threads:[~2018-01-11 13:18 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-13  3:16 [PATCH] OvmfPkg/VirtioScsiDxe: Allocate all required vrings at VirtioScsiInit Zheng Xiang
2017-12-13  8:35 ` Laszlo Ersek
2017-12-13  9:29   ` Paolo Bonzini
2017-12-13 11:04     ` Laszlo Ersek
2017-12-13 11:16     ` Laszlo Ersek
2017-12-14  6:55       ` zhengxiang (A)
2017-12-14  9:06         ` Paolo Bonzini
2017-12-14 13:25           ` zhengxiang (A)
2018-01-11 13:23             ` Maxime Coquelin [this message]
2018-01-11 14:46               ` Maxime Coquelin
2018-01-12  1:35                 ` zhengxiang (A)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8940cc0e-64e7-c253-a8e0-5081eab4efbe@redhat.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox