On Thu, Jul 7, 2022 at 2:40 AM Eugenio Pérez <epere...@redhat.com> wrote:
>
> The used idx used to match with this, but it will not match from the
> moment we introduce svq_inject.
It might be better to explain what "svq_inject" means here.
> Rewind all the descriptors not used by
> vdpa device and get the vq state properly.
>
> Signed-off-by: Eugenio Pérez <epere...@redhat.com>
> ---
> include/hw/virtio/virtio.h | 1 +
> hw/virtio/vhost-vdpa.c | 7 +++----
> hw/virtio/virtio.c | 5 +++++
> 3 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index db1c0ddf6b..4b51ab9d06 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -302,6 +302,7 @@ hwaddr virtio_queue_get_desc_size(VirtIODevice *vdev, int
> n);
> hwaddr virtio_queue_get_avail_size(VirtIODevice *vdev, int n);
> hwaddr virtio_queue_get_used_size(VirtIODevice *vdev, int n);
> unsigned int virtio_queue_get_last_avail_idx(VirtIODevice *vdev, int n);
> +unsigned int virtio_queue_get_in_use(const VirtQueue *vq);
> void virtio_queue_set_last_avail_idx(VirtIODevice *vdev, int n,
> unsigned int idx);
> void virtio_queue_restore_last_avail_idx(VirtIODevice *vdev, int n);
> diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> index 2ee8009594..de76128030 100644
> --- a/hw/virtio/vhost-vdpa.c
> +++ b/hw/virtio/vhost-vdpa.c
> @@ -1189,12 +1189,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);
> -
> + const VirtQueue *vq = virtio_get_queue(dev->vdev, ring->index);
> /*
> * 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
> @@ -1203,7 +1201,8 @@ static int vhost_vdpa_get_vring_base(struct vhost_dev
> *dev,
> * 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) -
> + virtio_queue_get_in_use(vq);
I think we need to change the above comment as well otherwise readers
might get confused.
I wonder why we need to bother at this time. Is this an issue for
networking devices? And for block device, it's not sufficient since
there's no guarantee that the descriptor is handled in order?
Thanks
> return 0;
> }
>
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 5d607aeaa0..e02656f7a2 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -3420,6 +3420,11 @@ unsigned int
> virtio_queue_get_last_avail_idx(VirtIODevice *vdev, int n)
> }
> }
>
> +unsigned int virtio_queue_get_in_use(const VirtQueue *vq)
> +{
> + return vq->inuse;
> +}
> +
> static void virtio_queue_packed_set_last_avail_idx(VirtIODevice *vdev,
> int n, unsigned int idx)
> {
> --
> 2.31.1
>
