On Mon, Feb 28, 2022 at 4:18 AM Jason Wang <jasow...@redhat.com> wrote:
>
>
> 在 2022/2/27 下午9:41, Eugenio Pérez 写道:
> > This will make qemu aware of the device used buffers, allowing it to
> > write the guest memory with its contents if needed.
> >
> > Signed-off-by: Eugenio Pérez <epere...@redhat.com>
> > ---
> > hw/virtio/vhost-shadow-virtqueue.h | 4 ++++
> > hw/virtio/vhost-shadow-virtqueue.c | 34 ++++++++++++++++++++++++++++++
> > hw/virtio/vhost-vdpa.c | 31 +++++++++++++++++++++++++--
> > 3 files changed, 67 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/virtio/vhost-shadow-virtqueue.h
> > b/hw/virtio/vhost-shadow-virtqueue.h
> > index 1cbc87d5d8..1d4c160d0a 100644
> > --- a/hw/virtio/vhost-shadow-virtqueue.h
> > +++ b/hw/virtio/vhost-shadow-virtqueue.h
> > @@ -28,9 +28,13 @@ typedef struct VhostShadowVirtqueue {
> > * So shadow virtqueue must not clean it, or we would lose VirtQueue
> > one.
> > */
> > EventNotifier svq_kick;
> > +
> > + /* Guest's call notifier, where the SVQ calls guest. */
> > + EventNotifier svq_call;
> > } VhostShadowVirtqueue;
> >
> > void vhost_svq_set_svq_kick_fd(VhostShadowVirtqueue *svq, int
> > svq_kick_fd);
> > +void vhost_svq_set_guest_call_notifier(VhostShadowVirtqueue *svq, int
> > call_fd);
> >
> > void vhost_svq_stop(VhostShadowVirtqueue *svq);
> >
> > diff --git a/hw/virtio/vhost-shadow-virtqueue.c
> > b/hw/virtio/vhost-shadow-virtqueue.c
> > index a5d0659f86..54c701a196 100644
> > --- a/hw/virtio/vhost-shadow-virtqueue.c
> > +++ b/hw/virtio/vhost-shadow-virtqueue.c
> > @@ -23,6 +23,38 @@ static void vhost_handle_guest_kick(EventNotifier *n)
> > event_notifier_set(&svq->hdev_kick);
> > }
> >
> > +/* Forward vhost notifications */
> > +static void vhost_svq_handle_call(EventNotifier *n)
> > +{
> > + VhostShadowVirtqueue *svq = container_of(n, VhostShadowVirtqueue,
> > + hdev_call);
> > + event_notifier_test_and_clear(n);
> > + event_notifier_set(&svq->svq_call);
> > +}
> > +
> > +/**
> > + * Set the call notifier for the SVQ to call the guest
> > + *
> > + * @svq Shadow virtqueue
> > + * @call_fd call notifier
> > + *
> > + * Called on BQL context.
> > + */
> > +void vhost_svq_set_guest_call_notifier(VhostShadowVirtqueue *svq, int
> > call_fd)
>
>
> I think we need to have consistent naming for both kick and call. Note
> that in patch 2 we had
>
> vhost_svq_set_svq_kick_fd
>
> Maybe it's better to use vhost_svq_set_guest_call_fd() here.
>
I think the same, I will replace it for the next version.
>
> > +{
> > + if (call_fd == VHOST_FILE_UNBIND) {
> > + /*
> > + * Fail event_notifier_set if called handling device call.
> > + *
> > + * SVQ still needs device notifications, since it needs to keep
> > + * forwarding used buffers even with the unbind.
> > + */
> > + memset(&svq->svq_call, 0, sizeof(svq->svq_call));
>
>
> I may miss something but shouldn't we stop polling svq_call here like
>
> event_notifier_set_handle(&svq->svq_call, false);
>
SVQ never polls that descriptor: It uses that descriptor to call (as
notify) the guest at vhost_svq_flush when SVQ uses descriptors.
svq_kick, svq_call: Descriptors that the guest send to SVQ
hdev_kick, hdev_call: Descriptors that qemu/SVQ send to the device.
I admit it is confusing when reading the code but I cannot come up
with a better naming. Maybe it helps to add a diagram at the top of
the file like:
+-------+-> svq_kick_fd ->+-----+-> hdev_kick ->+-----+
| Guest | | SVQ | | Dev |
+-------+<- svq_call_fd <-+-----+<- hdev_call <-+-----+
Thanks!
> ?
>
> Thanks
>
>
