[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC v2 12/13] vdpa: preemptive kick at enable
From: |
Eugenio Perez Martin |
Subject: |
Re: [RFC v2 12/13] vdpa: preemptive kick at enable |
Date: |
Thu, 2 Feb 2023 17:55:38 +0100 |
On Mon, Jan 16, 2023 at 8:02 AM Jason Wang <jasowang@redhat.com> wrote:
>
>
> 在 2023/1/13 17:06, Eugenio Perez Martin 写道:
> > On Fri, Jan 13, 2023 at 4:39 AM Jason Wang <jasowang@redhat.com> wrote:
> >> On Fri, Jan 13, 2023 at 11:25 AM Zhu, Lingshan <lingshan.zhu@intel.com>
> >> wrote:
> >>>
> >>>
> >>> On 1/13/2023 10:31 AM, Jason Wang wrote:
> >>>> On Fri, Jan 13, 2023 at 1:27 AM Eugenio Pérez <eperezma@redhat.com>
> >>>> wrote:
> >>>>> Spuriously kick the destination device's queue so it knows in case there
> >>>>> are new descriptors.
> >>>>>
> >>>>> RFC: This is somehow a gray area. The guest may have placed descriptors
> >>>>> in a virtqueue but not kicked it, so it might be surprised if the device
> >>>>> starts processing it.
> >>>> So I think this is kind of the work of the vDPA parent. For the parent
> >>>> that needs this trick, we should do it in the parent driver.
> >>> Agree, it looks easier implementing this in parent driver,
> >>> I can implement it in ifcvf set_vq_ready right now
> >> Great, but please check whether or not it is really needed.
> >>
> >> Some device implementation could check the available descriptions
> >> after DRIVER_OK without waiting for a kick.
> >>
> > So IIUC we can entirely drop this from the series (and I hope we can).
> > But then, what with the devices that does *not* check for them?
>
>
> It needs mediation in the vDPA parent driver.
>
>
> >
> > If we drop it it seems to me we must mandate devices to check for
> > descriptors at queue_enable. The queue could stall if not, isn't it?
>
>
> I'm not sure, did you see real issue with this? (Note that we don't do
> this for vhost-user-(vDPA))
>
Still unchecked, sorry. But not needing it for vhost-user-vDPA is a
very good signal indeed, thanks for pointing that.
> Btw, the code can result of kick before DRIVER_OK, which seems racy.
>
Good catch :). I'll fix it in the next revision if we see we need it.
I really hope to be able to drop it though.
Thanks!
> Thanks
>
>
> >
> > Thanks!
> >
> >> Thanks
> >>
> >>> Thanks
> >>> Zhu Lingshan
> >>>> Thanks
> >>>>
> >>>>> However, that information is not in the migration stream and it should
> >>>>> be an edge case anyhow, being resilient to parallel notifications from
> >>>>> the guest.
> >>>>>
> >>>>> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> >>>>> ---
> >>>>> hw/virtio/vhost-vdpa.c | 5 +++++
> >>>>> 1 file changed, 5 insertions(+)
> >>>>>
> >>>>> diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> >>>>> index 40b7e8706a..dff94355dd 100644
> >>>>> --- a/hw/virtio/vhost-vdpa.c
> >>>>> +++ b/hw/virtio/vhost-vdpa.c
> >>>>> @@ -732,11 +732,16 @@ static int vhost_vdpa_set_vring_ready(struct
> >>>>> vhost_dev *dev, int ready)
> >>>>> }
> >>>>> trace_vhost_vdpa_set_vring_ready(dev);
> >>>>> for (i = 0; i < dev->nvqs; ++i) {
> >>>>> + VirtQueue *vq;
> >>>>> struct vhost_vring_state state = {
> >>>>> .index = dev->vq_index + i,
> >>>>> .num = 1,
> >>>>> };
> >>>>> vhost_vdpa_call(dev, VHOST_VDPA_SET_VRING_ENABLE, &state);
> >>>>> +
> >>>>> + /* Preemptive kick */
> >>>>> + vq = virtio_get_queue(dev->vdev, dev->vq_index + i);
> >>>>> + event_notifier_set(virtio_queue_get_host_notifier(vq));
> >>>>> }
> >>>>> return 0;
> >>>>> }
> >>>>> --
> >>>>> 2.31.1
> >>>>>
>
- Re: [RFC v2 12/13] vdpa: preemptive kick at enable,
Eugenio Perez Martin <=