Re: [PATCH v2 12/19] vhost: add vhost_svq_poll

[Jason Wang]

On Fri, Jul 15, 2022 at 12:32 AM Eugenio Pérez <eperezma@redhat.com> wrote:
>
> It allows the Shadow Control VirtQueue to wait for the device to use the
> available buffers.
>
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> ---
>  hw/virtio/vhost-shadow-virtqueue.h |  1 +
>  hw/virtio/vhost-shadow-virtqueue.c | 22 ++++++++++++++++++++++
>  2 files changed, 23 insertions(+)
>
> diff --git a/hw/virtio/vhost-shadow-virtqueue.h 
> b/hw/virtio/vhost-shadow-virtqueue.h
> index 1692541cbb..b5c6e3b3b4 100644
> --- a/hw/virtio/vhost-shadow-virtqueue.h
> +++ b/hw/virtio/vhost-shadow-virtqueue.h
> @@ -89,6 +89,7 @@ void vhost_svq_push_elem(VhostShadowVirtqueue *svq, const 
> SVQElement *elem,
>  int vhost_svq_add(VhostShadowVirtqueue *svq, const struct iovec *out_sg,
>                    size_t out_num, const struct iovec *in_sg, size_t in_num,
>                    SVQElement *elem);
> +size_t vhost_svq_poll(VhostShadowVirtqueue *svq);
>
>  void vhost_svq_set_svq_kick_fd(VhostShadowVirtqueue *svq, int svq_kick_fd);
>  void vhost_svq_set_svq_call_fd(VhostShadowVirtqueue *svq, int call_fd);
> diff --git a/hw/virtio/vhost-shadow-virtqueue.c 
> b/hw/virtio/vhost-shadow-virtqueue.c
> index 5244896358..31a267f721 100644
> --- a/hw/virtio/vhost-shadow-virtqueue.c
> +++ b/hw/virtio/vhost-shadow-virtqueue.c
> @@ -486,6 +486,28 @@ static void vhost_svq_flush(VhostShadowVirtqueue *svq,
>      } while (!vhost_svq_enable_notification(svq));
>  }
>
> +/**
> + * Poll the SVQ for one device used buffer.
> + *
> + * This function race with main event loop SVQ polling, so extra
> + * synchronization is needed.
> + *
> + * Return the length written by the device.
> + */
> +size_t vhost_svq_poll(VhostShadowVirtqueue *svq)
> +{
> +    do {
> +        uint32_t len;
> +        SVQElement *elem = vhost_svq_get_buf(svq, &len);
> +        if (elem) {
> +            return len;
> +        }
> +
> +        /* Make sure we read new used_idx */
> +        smp_rmb();

There's already one smp_rmb(0 in vhost_svq_get_buf(). So this seems useless?

Thanks

> +    } while (true);
> +}
> +
>  /**
>   * Forward used buffers.
>   *
> --
> 2.31.1
>