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

[Eugenio Perez Martin]

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,
>