diff mbox series

[RFC,v2,12/13] vhost: Check for device VRING_USED_F_NO_NOTIFY at shadow virtqueue kick

Message ID 20210315194842.277740-13-eperezma@redhat.com (mailing list archive)
State New, archived
Headers show
Series vDPA software assisted live migration | expand

Commit Message

Eugenio Perez Martin March 15, 2021, 7:48 p.m. UTC
Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
---
 hw/virtio/vhost-shadow-virtqueue.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

Comments

Jason Wang March 16, 2021, 8:07 a.m. UTC | #1
在 2021/3/16 上午3:48, Eugenio Pérez 写道:
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> ---
>   hw/virtio/vhost-shadow-virtqueue.c | 11 ++++++++++-
>   1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/hw/virtio/vhost-shadow-virtqueue.c b/hw/virtio/vhost-shadow-virtqueue.c
> index 68ed0f2740..7df98fc43f 100644
> --- a/hw/virtio/vhost-shadow-virtqueue.c
> +++ b/hw/virtio/vhost-shadow-virtqueue.c
> @@ -145,6 +145,15 @@ static void vhost_shadow_vq_add(VhostShadowVirtqueue *svq,
>       svq->ring_id_maps[qemu_head] = elem;
>   }
>   
> +static void vhost_shadow_vq_kick(VhostShadowVirtqueue *svq)
> +{
> +    /* Make sure we are reading updated device flag */
> +    smp_rmb();


smp_mb() actually? Or it's better to explain this following read needs 
to be orderd with what read before.

Thanks


> +    if (!(svq->vring.used->flags & VRING_USED_F_NO_NOTIFY)) {
> +        event_notifier_set(&svq->kick_notifier);
> +    }
> +}
> +
>   /* Handle guest->device notifications */
>   static void vhost_handle_guest_kick(EventNotifier *n)
>   {
> @@ -174,7 +183,7 @@ static void vhost_handle_guest_kick(EventNotifier *n)
>               }
>   
>               vhost_shadow_vq_add(svq, elem);
> -            event_notifier_set(&svq->kick_notifier);
> +            vhost_shadow_vq_kick(svq);
>           }
>   
>           virtio_queue_set_notification(svq->vq, true);
Eugenio Perez Martin May 17, 2021, 5:11 p.m. UTC | #2
On Tue, Mar 16, 2021 at 9:07 AM Jason Wang <jasowang@redhat.com> wrote:
>
>
> 在 2021/3/16 上午3:48, Eugenio Pérez 写道:
> > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> > ---
> >   hw/virtio/vhost-shadow-virtqueue.c | 11 ++++++++++-
> >   1 file changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/virtio/vhost-shadow-virtqueue.c b/hw/virtio/vhost-shadow-virtqueue.c
> > index 68ed0f2740..7df98fc43f 100644
> > --- a/hw/virtio/vhost-shadow-virtqueue.c
> > +++ b/hw/virtio/vhost-shadow-virtqueue.c
> > @@ -145,6 +145,15 @@ static void vhost_shadow_vq_add(VhostShadowVirtqueue *svq,
> >       svq->ring_id_maps[qemu_head] = elem;
> >   }
> >
> > +static void vhost_shadow_vq_kick(VhostShadowVirtqueue *svq)
> > +{
> > +    /* Make sure we are reading updated device flag */
> > +    smp_rmb();
>
>
> smp_mb() actually? Or it's better to explain this following read needs
> to be orderd with what read before.
>
> Thanks
>

Sorry for the late reply, I moved to vhost-vdpa usage of SVQ and I
missed these comments.

My intentions were just to order the reading of used ring flags. In
other words, to avoid reading an old value in the next conditional.

Descriptors themselves should be already written because
vhost_shadow_vq_add_split just calls smp_wmb almost at the end of the
execution. avail_ring->idx is not protected by it though. Is that what
you meant about turning it to a full barrier?

Maybe it's clearer just to call smp_mb() between the calls to
vhost_shadow_vq_add_split and vhost_shadow_vq_kick, merging both
memory barriers?

Thanks!

>
> > +    if (!(svq->vring.used->flags & VRING_USED_F_NO_NOTIFY)) {
> > +        event_notifier_set(&svq->kick_notifier);
> > +    }
> > +}
> > +
> >   /* Handle guest->device notifications */
> >   static void vhost_handle_guest_kick(EventNotifier *n)
> >   {
> > @@ -174,7 +183,7 @@ static void vhost_handle_guest_kick(EventNotifier *n)
> >               }
> >
> >               vhost_shadow_vq_add(svq, elem);
> > -            event_notifier_set(&svq->kick_notifier);
> > +            vhost_shadow_vq_kick(svq);
> >           }
> >
> >           virtio_queue_set_notification(svq->vq, true);
>
diff mbox series

Patch

diff --git a/hw/virtio/vhost-shadow-virtqueue.c b/hw/virtio/vhost-shadow-virtqueue.c
index 68ed0f2740..7df98fc43f 100644
--- a/hw/virtio/vhost-shadow-virtqueue.c
+++ b/hw/virtio/vhost-shadow-virtqueue.c
@@ -145,6 +145,15 @@  static void vhost_shadow_vq_add(VhostShadowVirtqueue *svq,
     svq->ring_id_maps[qemu_head] = elem;
 }
 
+static void vhost_shadow_vq_kick(VhostShadowVirtqueue *svq)
+{
+    /* Make sure we are reading updated device flag */
+    smp_rmb();
+    if (!(svq->vring.used->flags & VRING_USED_F_NO_NOTIFY)) {
+        event_notifier_set(&svq->kick_notifier);
+    }
+}
+
 /* Handle guest->device notifications */
 static void vhost_handle_guest_kick(EventNotifier *n)
 {
@@ -174,7 +183,7 @@  static void vhost_handle_guest_kick(EventNotifier *n)
             }
 
             vhost_shadow_vq_add(svq, elem);
-            event_notifier_set(&svq->kick_notifier);
+            vhost_shadow_vq_kick(svq);
         }
 
         virtio_queue_set_notification(svq->vq, true);