On Mon, Jul 27, 2020 at 03:51:41PM +0200, Laurent Vivier wrote:
> On 04/07/2020 20:30, Michael S. Tsirkin wrote:
> > From: Jason Wang <[email protected]>
> >
> > With version 1, we can detect whether a queue is enabled via
> > queue_enabled.
> >
> > Signed-off-by: Jason Wang <[email protected]>
> > Signed-off-by: Cindy Lu <[email protected]>
> > Message-Id: <[email protected]>
> > Reviewed-by: Michael S. Tsirkin <[email protected]>
> > Signed-off-by: Michael S. Tsirkin <[email protected]>
> > Acked-by: Jason Wang <[email protected]>
> > ---
> > 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;
> }
>
Thanks for the report and debugging the issue!
Maybe move
return virtio_queue_get_desc_addr(vdev, n) != 0;
to a new API
virtio_pci_queue_enabled_legacy()
to avoid code duplication.
Could you cook up a patch pls? don't forget the Fixes: tag
so people remember to backport it.
> > +
> > 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
