Re: [Qemu-devel] [PATCH V3] vhost_net: don't set backend for the uninitialized virtqueue

[Michael S. Tsirkin]

On Tue, Apr 09, 2019 at 01:58:50PM +0800, Jason Wang wrote:
> We used to set backend unconditionally, this won't work for some
> guests (e.g windows driver) who may not initialize all virtqueues. For
> kernel backend, this will fail since it may try to validate the rings
> during setting backend.
> 
> Fixing this by simply skipping the backend set when we find desc is
> not ready.
> 
> Signed-off-by: Jason Wang <address@hidden>

Reviewed-by: Michael S. Tsirkin <address@hidden>

> ---
> Changes from V1:
> - introduce a generic helper to check if a virtqueue is enabled
> ---
>  hw/net/vhost_net.c         | 10 ++++++++++
>  hw/virtio/virtio.c         |  5 +++++
>  include/hw/virtio/virtio.h |  1 +
>  3 files changed, 16 insertions(+)
> 
> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> index be3cc88370..a6b719035c 100644
> --- a/hw/net/vhost_net.c
> +++ b/hw/net/vhost_net.c
> @@ -244,6 +244,11 @@ static int vhost_net_start_one(struct vhost_net *net,
>          qemu_set_fd_handler(net->backend, NULL, NULL, NULL);
>          file.fd = net->backend;
>          for (file.index = 0; file.index < net->dev.nvqs; ++file.index) {
> +            if (!virtio_queue_enabled(dev, net->dev.vq_index +
> +                                      file.index)) {
> +                /* Queue might not be ready for start */
> +                continue;
> +            }
>              r = vhost_net_set_backend(&net->dev, &file);
>              if (r < 0) {
>                  r = -errno;
> @@ -256,6 +261,11 @@ fail:
>      file.fd = -1;
>      if (net->nc->info->type == NET_CLIENT_DRIVER_TAP) {
>          while (file.index-- > 0) {
> +            if (!virtio_queue_enabled(dev, net->dev.vq_index +
> +                                      file.index)) {
> +                /* Queue might not be ready for start */
> +                continue;
> +            }
>              int r = vhost_net_set_backend(&net->dev, &file);
>              assert(r >= 0);
>          }
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 2626a895cb..28056a7ef7 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -2318,6 +2318,11 @@ hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, 
> int n)
>      return vdev->vq[n].vring.desc;
>  }
>  
> +bool virtio_queue_enabled(VirtIODevice *vdev, int n)
> +{
> +    return virtio_queue_get_desc_addr(vdev, n) != 0;
> +}
> +
>  hwaddr virtio_queue_get_avail_addr(VirtIODevice *vdev, int n)
>  {
>      return vdev->vq[n].vring.avail;
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index ce9516236a..7140381e3a 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -282,6 +282,7 @@ typedef struct VirtIORNGConf VirtIORNGConf;
>                        VIRTIO_F_IOMMU_PLATFORM, false)
>  
>  hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n);
> +bool virtio_queue_enabled(VirtIODevice *vdev, int n);
>  hwaddr virtio_queue_get_avail_addr(VirtIODevice *vdev, int n);
>  hwaddr virtio_queue_get_used_addr(VirtIODevice *vdev, int n);
>  hwaddr virtio_queue_get_desc_size(VirtIODevice *vdev, int n);
> -- 
> 2.19.1