Re: [PATCH v2] vhost: Get vring base from vq, not svq

[Jason Wang]

On Mon, Jul 25, 2022 at 3:07 PM Jason Wang <jasowang@redhat.com> wrote:
>
> On Mon, Jul 18, 2022 at 8:05 PM Eugenio Pérez <eperezma@redhat.com> wrote:
> >
> > The SVQ vring used idx usually match with the guest visible one, as long
> > as all the guest buffers (GPA) maps to exactly one buffer within qemu's
> > VA. However, as we can see in virtqueue_map_desc, a single guest buffer
> > could map to many buffers in SVQ vring.
> >
> > Also, its also a mistake to rewind them at the source of migration.
> > Since VirtQueue is able to migrate the inflight descriptors, its
> > responsability of the destination to perform the rewind just in case it
> > cannot report the inflight descriptors to the device.
> >
> > This makes easier to migrate between backends or to recover them in
> > vhost devices that support set in flight descriptors.
> >
> > Fixes: 6d0b22266633 ("vdpa: Adapt vhost_vdpa_get_vring_base to SVQ")
> > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
>
> Acked-by: Jason Wang <jasowang@redhat.com>

I've queued this for hard-freeze.

Thanks

>
> >
> > --
> > v2: Squash both fixes in one.
> > ---
> >  hw/virtio/vhost-vdpa.c | 24 ++++++++++++------------
> >  1 file changed, 12 insertions(+), 12 deletions(-)
> >
> > diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> > index 795ed5a049..4458c8d23e 100644
> > --- a/hw/virtio/vhost-vdpa.c
> > +++ b/hw/virtio/vhost-vdpa.c
> > @@ -1178,7 +1178,18 @@ static int vhost_vdpa_set_vring_base(struct 
> > vhost_dev *dev,
> >                                         struct vhost_vring_state *ring)
> >  {
> >      struct vhost_vdpa *v = dev->opaque;
> > +    VirtQueue *vq = virtio_get_queue(dev->vdev, ring->index);
> >
> > +    /*
> > +     * vhost-vdpa devices does not support in-flight requests. Set all of 
> > them
> > +     * as available.
> > +     *
> > +     * TODO: This is ok for networking, but other kinds of devices might
> > +     * have problems with these retransmissions.
> > +     */
> > +    while (virtqueue_rewind(vq, 1)) {
> > +        continue;
> > +    }
> >      if (v->shadow_vqs_enabled) {
> >          /*
> >           * Device vring base was set at device start. SVQ base is handled 
> > by
> > @@ -1194,21 +1205,10 @@ static int vhost_vdpa_get_vring_base(struct 
> > vhost_dev *dev,
> >                                         struct vhost_vring_state *ring)
> >  {
> >      struct vhost_vdpa *v = dev->opaque;
> > -    int vdpa_idx = ring->index - dev->vq_index;
> >      int ret;
> >
> >      if (v->shadow_vqs_enabled) {
> > -        VhostShadowVirtqueue *svq = g_ptr_array_index(v->shadow_vqs, 
> > vdpa_idx);
> > -
> > -        /*
> > -         * Setting base as last used idx, so destination will see as 
> > available
> > -         * all the entries that the device did not use, including the 
> > in-flight
> > -         * processing ones.
> > -         *
> > -         * TODO: This is ok for networking, but other kinds of devices 
> > might
> > -         * have problems with these retransmissions.
> > -         */
> > -        ring->num = svq->last_used_idx;
> > +        ring->num = virtio_queue_get_last_avail_idx(dev->vdev, 
> > ring->index);
> >          return 0;
> >      }
> >
> > --
> > 2.31.1
> >