diff mbox series

[RFC,v2,08/13] virtio: Add vhost_shadow_vq_get_vring_addr

Message ID 20210315194842.277740-9-eperezma@redhat.com
State New
Headers show
Series vDPA software assisted live migration | expand

Commit Message

Eugenio Perez Martin March 15, 2021, 7:48 p.m. UTC 
It reports the shadow virtqueue address from qemu virtual address space

Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
---
 hw/virtio/vhost-shadow-virtqueue.h |  2 ++
 hw/virtio/vhost-shadow-virtqueue.c | 24 +++++++++++++++++++++++-
 2 files changed, 25 insertions(+), 1 deletion(-)

Comments

Jason Wang March 16, 2021, 7:50 a.m. UTC | #1 
在 2021/3/16 上午3:48, Eugenio Pérez 写道:
> It reports the shadow virtqueue address from qemu virtual address space


Note that to be used by vDPA, we can't use qemu VA directly here.


>
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> ---
>   hw/virtio/vhost-shadow-virtqueue.h |  2 ++
>   hw/virtio/vhost-shadow-virtqueue.c | 24 +++++++++++++++++++++++-
>   2 files changed, 25 insertions(+), 1 deletion(-)
>
> diff --git a/hw/virtio/vhost-shadow-virtqueue.h b/hw/virtio/vhost-shadow-virtqueue.h
> index 2ca4b92b12..d82c35bccf 100644
> --- a/hw/virtio/vhost-shadow-virtqueue.h
> +++ b/hw/virtio/vhost-shadow-virtqueue.h
> @@ -19,6 +19,8 @@ typedef struct VhostShadowVirtqueue VhostShadowVirtqueue;
>   
>   void vhost_shadow_vq_mask(VhostShadowVirtqueue *svq, EventNotifier *masked);
>   void vhost_shadow_vq_unmask(VhostShadowVirtqueue *svq);
> +void vhost_shadow_vq_get_vring_addr(const VhostShadowVirtqueue *svq,
> +                                    struct vhost_vring_addr *addr);
>   
>   bool vhost_shadow_vq_start(struct vhost_dev *dev,
>                              unsigned idx,
> diff --git a/hw/virtio/vhost-shadow-virtqueue.c b/hw/virtio/vhost-shadow-virtqueue.c
> index b6bab438d6..1460d1d5d1 100644
> --- a/hw/virtio/vhost-shadow-virtqueue.c
> +++ b/hw/virtio/vhost-shadow-virtqueue.c
> @@ -17,6 +17,9 @@
>   
>   /* Shadow virtqueue to relay notifications */
>   typedef struct VhostShadowVirtqueue {
> +    /* Shadow vring */
> +    struct vring vring;
> +
>       /* Shadow kick notifier, sent to vhost */
>       EventNotifier kick_notifier;
>       /* Shadow call notifier, sent to vhost */
> @@ -51,6 +54,9 @@ typedef struct VhostShadowVirtqueue {
>   
>       /* Virtio device */
>       VirtIODevice *vdev;
> +
> +    /* Descriptors copied from guest */
> +    vring_desc_t descs[];
>   } VhostShadowVirtqueue;
>   
>   /* Forward guest notifications */
> @@ -132,6 +138,19 @@ void vhost_shadow_vq_unmask(VhostShadowVirtqueue *svq)
>       qemu_event_wait(&svq->masked_notifier.is_free);
>   }
>   
> +/*
> + * Get the shadow vq vring address.
> + * @svq Shadow virtqueue
> + * @addr Destination to store address
> + */
> +void vhost_shadow_vq_get_vring_addr(const VhostShadowVirtqueue *svq,
> +                                    struct vhost_vring_addr *addr)
> +{
> +    addr->desc_user_addr = (uint64_t)svq->vring.desc;
> +    addr->avail_user_addr = (uint64_t)svq->vring.avail;
> +    addr->used_user_addr = (uint64_t)svq->vring.used;
> +}
> +
>   /*
>    * Restore the vhost guest to host notifier, i.e., disables svq effect.
>    */
> @@ -262,7 +281,9 @@ void vhost_shadow_vq_stop(struct vhost_dev *dev,
>   VhostShadowVirtqueue *vhost_shadow_vq_new(struct vhost_dev *dev, int idx)
>   {
>       int vq_idx = dev->vq_index + idx;
> -    g_autofree VhostShadowVirtqueue *svq = g_new0(VhostShadowVirtqueue, 1);
> +    unsigned num = virtio_queue_get_num(dev->vdev, vq_idx);
> +    size_t ring_size = vring_size(num, VRING_DESC_ALIGN_SIZE);
> +    g_autofree VhostShadowVirtqueue *svq = g_malloc0(sizeof(*svq) + ring_size);
>       int r;
>   
>       r = event_notifier_init(&svq->kick_notifier, 0);
> @@ -279,6 +300,7 @@ VhostShadowVirtqueue *vhost_shadow_vq_new(struct vhost_dev *dev, int idx)
>           goto err_init_call_notifier;
>       }
>   
> +    vring_init(&svq->vring, num, svq->descs, VRING_DESC_ALIGN_SIZE);


We had some dicussion in the past. Exporting vring_init() is wrong but 
too late to fix (assumes a legacy split layout). Let's not depend on 
this buggy uAPI.

Thanks


>       svq->vq = virtio_get_queue(dev->vdev, vq_idx);
>       svq->vdev = dev->vdev;
>       event_notifier_set_handler(&svq->call_notifier,
Eugenio Perez Martin March 16, 2021, 3:20 p.m. UTC | #2 
On Tue, Mar 16, 2021 at 8:50 AM Jason Wang <jasowang@redhat.com> wrote:
>
>
> 在 2021/3/16 上午3:48, Eugenio Pérez 写道:
> > It reports the shadow virtqueue address from qemu virtual address space
>
>
> Note that to be used by vDPA, we can't use qemu VA directly here.
>

Right, I'm planning to use a different virtual address space if the
device has such limitations.

>
> >
> > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> > ---
> >   hw/virtio/vhost-shadow-virtqueue.h |  2 ++
> >   hw/virtio/vhost-shadow-virtqueue.c | 24 +++++++++++++++++++++++-
> >   2 files changed, 25 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/virtio/vhost-shadow-virtqueue.h b/hw/virtio/vhost-shadow-virtqueue.h
> > index 2ca4b92b12..d82c35bccf 100644
> > --- a/hw/virtio/vhost-shadow-virtqueue.h
> > +++ b/hw/virtio/vhost-shadow-virtqueue.h
> > @@ -19,6 +19,8 @@ typedef struct VhostShadowVirtqueue VhostShadowVirtqueue;
> >
> >   void vhost_shadow_vq_mask(VhostShadowVirtqueue *svq, EventNotifier *masked);
> >   void vhost_shadow_vq_unmask(VhostShadowVirtqueue *svq);
> > +void vhost_shadow_vq_get_vring_addr(const VhostShadowVirtqueue *svq,
> > +                                    struct vhost_vring_addr *addr);
> >
> >   bool vhost_shadow_vq_start(struct vhost_dev *dev,
> >                              unsigned idx,
> > diff --git a/hw/virtio/vhost-shadow-virtqueue.c b/hw/virtio/vhost-shadow-virtqueue.c
> > index b6bab438d6..1460d1d5d1 100644
> > --- a/hw/virtio/vhost-shadow-virtqueue.c
> > +++ b/hw/virtio/vhost-shadow-virtqueue.c
> > @@ -17,6 +17,9 @@
> >
> >   /* Shadow virtqueue to relay notifications */
> >   typedef struct VhostShadowVirtqueue {
> > +    /* Shadow vring */
> > +    struct vring vring;
> > +
> >       /* Shadow kick notifier, sent to vhost */
> >       EventNotifier kick_notifier;
> >       /* Shadow call notifier, sent to vhost */
> > @@ -51,6 +54,9 @@ typedef struct VhostShadowVirtqueue {
> >
> >       /* Virtio device */
> >       VirtIODevice *vdev;
> > +
> > +    /* Descriptors copied from guest */
> > +    vring_desc_t descs[];
> >   } VhostShadowVirtqueue;
> >
> >   /* Forward guest notifications */
> > @@ -132,6 +138,19 @@ void vhost_shadow_vq_unmask(VhostShadowVirtqueue *svq)
> >       qemu_event_wait(&svq->masked_notifier.is_free);
> >   }
> >
> > +/*
> > + * Get the shadow vq vring address.
> > + * @svq Shadow virtqueue
> > + * @addr Destination to store address
> > + */
> > +void vhost_shadow_vq_get_vring_addr(const VhostShadowVirtqueue *svq,
> > +                                    struct vhost_vring_addr *addr)
> > +{
> > +    addr->desc_user_addr = (uint64_t)svq->vring.desc;
> > +    addr->avail_user_addr = (uint64_t)svq->vring.avail;
> > +    addr->used_user_addr = (uint64_t)svq->vring.used;
> > +}
> > +
> >   /*
> >    * Restore the vhost guest to host notifier, i.e., disables svq effect.
> >    */
> > @@ -262,7 +281,9 @@ void vhost_shadow_vq_stop(struct vhost_dev *dev,
> >   VhostShadowVirtqueue *vhost_shadow_vq_new(struct vhost_dev *dev, int idx)
> >   {
> >       int vq_idx = dev->vq_index + idx;
> > -    g_autofree VhostShadowVirtqueue *svq = g_new0(VhostShadowVirtqueue, 1);
> > +    unsigned num = virtio_queue_get_num(dev->vdev, vq_idx);
> > +    size_t ring_size = vring_size(num, VRING_DESC_ALIGN_SIZE);
> > +    g_autofree VhostShadowVirtqueue *svq = g_malloc0(sizeof(*svq) + ring_size);
> >       int r;
> >
> >       r = event_notifier_init(&svq->kick_notifier, 0);
> > @@ -279,6 +300,7 @@ VhostShadowVirtqueue *vhost_shadow_vq_new(struct vhost_dev *dev, int idx)
> >           goto err_init_call_notifier;
> >       }
> >
> > +    vring_init(&svq->vring, num, svq->descs, VRING_DESC_ALIGN_SIZE);
>
>
> We had some dicussion in the past. Exporting vring_init() is wrong but
> too late to fix (assumes a legacy split layout). Let's not depend on
> this buggy uAPI.
>

Ok, I will change the way to allocate and initialize it.

> Thanks
>
>
> >       svq->vq = virtio_get_queue(dev->vdev, vq_idx);
> >       svq->vdev = dev->vdev;
> >       event_notifier_set_handler(&svq->call_notifier,
>
Eugenio Perez Martin May 17, 2021, 5:39 p.m. UTC | #3 
On Tue, Mar 16, 2021 at 4:20 PM Eugenio Perez Martin
<eperezma@redhat.com> wrote:
>
> On Tue, Mar 16, 2021 at 8:50 AM Jason Wang <jasowang@redhat.com> wrote:
> >
> >
> > 在 2021/3/16 上午3:48, Eugenio Pérez 写道:
> > > It reports the shadow virtqueue address from qemu virtual address space
> >
> >
> > Note that to be used by vDPA, we can't use qemu VA directly here.
> >
>
> Right, I'm planning to use a different virtual address space if the
> device has such limitations.
>
> >
> > >
> > > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> > > ---
> > >   hw/virtio/vhost-shadow-virtqueue.h |  2 ++
> > >   hw/virtio/vhost-shadow-virtqueue.c | 24 +++++++++++++++++++++++-
> > >   2 files changed, 25 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/hw/virtio/vhost-shadow-virtqueue.h b/hw/virtio/vhost-shadow-virtqueue.h
> > > index 2ca4b92b12..d82c35bccf 100644
> > > --- a/hw/virtio/vhost-shadow-virtqueue.h
> > > +++ b/hw/virtio/vhost-shadow-virtqueue.h
> > > @@ -19,6 +19,8 @@ typedef struct VhostShadowVirtqueue VhostShadowVirtqueue;
> > >
> > >   void vhost_shadow_vq_mask(VhostShadowVirtqueue *svq, EventNotifier *masked);
> > >   void vhost_shadow_vq_unmask(VhostShadowVirtqueue *svq);
> > > +void vhost_shadow_vq_get_vring_addr(const VhostShadowVirtqueue *svq,
> > > +                                    struct vhost_vring_addr *addr);
> > >
> > >   bool vhost_shadow_vq_start(struct vhost_dev *dev,
> > >                              unsigned idx,
> > > diff --git a/hw/virtio/vhost-shadow-virtqueue.c b/hw/virtio/vhost-shadow-virtqueue.c
> > > index b6bab438d6..1460d1d5d1 100644
> > > --- a/hw/virtio/vhost-shadow-virtqueue.c
> > > +++ b/hw/virtio/vhost-shadow-virtqueue.c
> > > @@ -17,6 +17,9 @@
> > >
> > >   /* Shadow virtqueue to relay notifications */
> > >   typedef struct VhostShadowVirtqueue {
> > > +    /* Shadow vring */
> > > +    struct vring vring;
> > > +
> > >       /* Shadow kick notifier, sent to vhost */
> > >       EventNotifier kick_notifier;
> > >       /* Shadow call notifier, sent to vhost */
> > > @@ -51,6 +54,9 @@ typedef struct VhostShadowVirtqueue {
> > >
> > >       /* Virtio device */
> > >       VirtIODevice *vdev;
> > > +
> > > +    /* Descriptors copied from guest */
> > > +    vring_desc_t descs[];
> > >   } VhostShadowVirtqueue;
> > >
> > >   /* Forward guest notifications */
> > > @@ -132,6 +138,19 @@ void vhost_shadow_vq_unmask(VhostShadowVirtqueue *svq)
> > >       qemu_event_wait(&svq->masked_notifier.is_free);
> > >   }
> > >
> > > +/*
> > > + * Get the shadow vq vring address.
> > > + * @svq Shadow virtqueue
> > > + * @addr Destination to store address
> > > + */
> > > +void vhost_shadow_vq_get_vring_addr(const VhostShadowVirtqueue *svq,
> > > +                                    struct vhost_vring_addr *addr)
> > > +{
> > > +    addr->desc_user_addr = (uint64_t)svq->vring.desc;
> > > +    addr->avail_user_addr = (uint64_t)svq->vring.avail;
> > > +    addr->used_user_addr = (uint64_t)svq->vring.used;
> > > +}
> > > +
> > >   /*
> > >    * Restore the vhost guest to host notifier, i.e., disables svq effect.
> > >    */
> > > @@ -262,7 +281,9 @@ void vhost_shadow_vq_stop(struct vhost_dev *dev,
> > >   VhostShadowVirtqueue *vhost_shadow_vq_new(struct vhost_dev *dev, int idx)
> > >   {
> > >       int vq_idx = dev->vq_index + idx;
> > > -    g_autofree VhostShadowVirtqueue *svq = g_new0(VhostShadowVirtqueue, 1);
> > > +    unsigned num = virtio_queue_get_num(dev->vdev, vq_idx);
> > > +    size_t ring_size = vring_size(num, VRING_DESC_ALIGN_SIZE);
> > > +    g_autofree VhostShadowVirtqueue *svq = g_malloc0(sizeof(*svq) + ring_size);
> > >       int r;
> > >
> > >       r = event_notifier_init(&svq->kick_notifier, 0);
> > > @@ -279,6 +300,7 @@ VhostShadowVirtqueue *vhost_shadow_vq_new(struct vhost_dev *dev, int idx)
> > >           goto err_init_call_notifier;
> > >       }
> > >
> > > +    vring_init(&svq->vring, num, svq->descs, VRING_DESC_ALIGN_SIZE);
> >
> >
> > We had some dicussion in the past. Exporting vring_init() is wrong but
> > too late to fix (assumes a legacy split layout). Let's not depend on
> > this buggy uAPI.
> >
>
> Ok, I will change the way to allocate and initialize it.
>

Could we define VIRTIO_RING_NO_LEGACY macro in qemu/osdep.h or similar
to avoid repeating this mistake in the future?

Thanks!

> > Thanks
> >
> >
> > >       svq->vq = virtio_get_queue(dev->vdev, vq_idx);
> > >       svq->vdev = dev->vdev;
> > >       event_notifier_set_handler(&svq->call_notifier,
> >
diff mbox series

Patch

diff --git a/hw/virtio/vhost-shadow-virtqueue.h b/hw/virtio/vhost-shadow-virtqueue.h
index 2ca4b92b12..d82c35bccf 100644
--- a/hw/virtio/vhost-shadow-virtqueue.h
+++ b/hw/virtio/vhost-shadow-virtqueue.h
@@ -19,6 +19,8 @@  typedef struct VhostShadowVirtqueue VhostShadowVirtqueue;
 
 void vhost_shadow_vq_mask(VhostShadowVirtqueue *svq, EventNotifier *masked);
 void vhost_shadow_vq_unmask(VhostShadowVirtqueue *svq);
+void vhost_shadow_vq_get_vring_addr(const VhostShadowVirtqueue *svq,
+                                    struct vhost_vring_addr *addr);
 
 bool vhost_shadow_vq_start(struct vhost_dev *dev,
                            unsigned idx,
diff --git a/hw/virtio/vhost-shadow-virtqueue.c b/hw/virtio/vhost-shadow-virtqueue.c
index b6bab438d6..1460d1d5d1 100644
--- a/hw/virtio/vhost-shadow-virtqueue.c
+++ b/hw/virtio/vhost-shadow-virtqueue.c
@@ -17,6 +17,9 @@ 
 
 /* Shadow virtqueue to relay notifications */
 typedef struct VhostShadowVirtqueue {
+    /* Shadow vring */
+    struct vring vring;
+
     /* Shadow kick notifier, sent to vhost */
     EventNotifier kick_notifier;
     /* Shadow call notifier, sent to vhost */
@@ -51,6 +54,9 @@  typedef struct VhostShadowVirtqueue {
 
     /* Virtio device */
     VirtIODevice *vdev;
+
+    /* Descriptors copied from guest */
+    vring_desc_t descs[];
 } VhostShadowVirtqueue;
 
 /* Forward guest notifications */
@@ -132,6 +138,19 @@  void vhost_shadow_vq_unmask(VhostShadowVirtqueue *svq)
     qemu_event_wait(&svq->masked_notifier.is_free);
 }
 
+/*
+ * Get the shadow vq vring address.
+ * @svq Shadow virtqueue
+ * @addr Destination to store address
+ */
+void vhost_shadow_vq_get_vring_addr(const VhostShadowVirtqueue *svq,
+                                    struct vhost_vring_addr *addr)
+{
+    addr->desc_user_addr = (uint64_t)svq->vring.desc;
+    addr->avail_user_addr = (uint64_t)svq->vring.avail;
+    addr->used_user_addr = (uint64_t)svq->vring.used;
+}
+
 /*
  * Restore the vhost guest to host notifier, i.e., disables svq effect.
  */
@@ -262,7 +281,9 @@  void vhost_shadow_vq_stop(struct vhost_dev *dev,
 VhostShadowVirtqueue *vhost_shadow_vq_new(struct vhost_dev *dev, int idx)
 {
     int vq_idx = dev->vq_index + idx;
-    g_autofree VhostShadowVirtqueue *svq = g_new0(VhostShadowVirtqueue, 1);
+    unsigned num = virtio_queue_get_num(dev->vdev, vq_idx);
+    size_t ring_size = vring_size(num, VRING_DESC_ALIGN_SIZE);
+    g_autofree VhostShadowVirtqueue *svq = g_malloc0(sizeof(*svq) + ring_size);
     int r;
 
     r = event_notifier_init(&svq->kick_notifier, 0);
@@ -279,6 +300,7 @@  VhostShadowVirtqueue *vhost_shadow_vq_new(struct vhost_dev *dev, int idx)
         goto err_init_call_notifier;
     }
 
+    vring_init(&svq->vring, num, svq->descs, VRING_DESC_ALIGN_SIZE);
     svq->vq = virtio_get_queue(dev->vdev, vq_idx);
     svq->vdev = dev->vdev;
     event_notifier_set_handler(&svq->call_notifier,