Re: [PULL v2 31/41] virtio-pci: implement queue_enabled method

[Laurent Vivier <lvivier@redhat.com>]

On 04/07/2020 20:30, Michael S. Tsirkin wrote:
> From: Jason Wang <jasowang@redhat.com>
> 
> With version 1, we can detect whether a queue is enabled via
> queue_enabled.
> 
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> Signed-off-by: Cindy Lu <lulu@redhat.com>
> Message-Id: <20200701145538.22333-5-lulu@redhat.com>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Acked-by: Jason Wang <jasowang@redhat.com>
> ---
>  hw/virtio/virtio-pci.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index 7bc8c1c056..8554cf2a03 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -1107,6 +1107,18 @@ static AddressSpace *virtio_pci_get_dma_as(DeviceState *d)
>      return pci_get_address_space(dev);
>  }
>  
> +static bool virtio_pci_queue_enabled(DeviceState *d, int n)
> +{
> +    VirtIOPCIProxy *proxy = VIRTIO_PCI(d);
> +    VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
> +
> +    if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
> +        return proxy->vqs[vdev->queue_sel].enabled;
> +    }
> +
> +    return virtio_queue_enabled(vdev, n);
> +}

With "disable-legacy=off,disable-modern=true",
this changes introduces an infinite loop: virtio_queue_enabled() calls
again virtio_pci_queue_enabled() that calls
againvirtio_pci_queue_enabled()...

I think this should be changed like this:

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index ada1101d07..0a85c17e91 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1116,7 +1116,7 @@ static bool virtio_pci_queue_enabled(DeviceState
*d, int n)
         return proxy->vqs[vdev->queue_sel].enabled;
     }

-    return virtio_queue_enabled(vdev, n);
+    return virtio_queue_get_desc_addr(vdev, n) != 0;
 }


> +
>  static int virtio_pci_add_mem_cap(VirtIOPCIProxy *proxy,
>                                     struct virtio_pci_cap *cap)
>  {
> @@ -2064,6 +2076,7 @@ static void virtio_pci_bus_class_init(ObjectClass *klass, void *data)
>      k->ioeventfd_enabled = virtio_pci_ioeventfd_enabled;
>      k->ioeventfd_assign = virtio_pci_ioeventfd_assign;
>      k->get_dma_as = virtio_pci_get_dma_as;
> +    k->queue_enabled = virtio_pci_queue_enabled;
>  }
>  
>  static const TypeInfo virtio_pci_bus_info = {
> 

Thanks,
Laurent