On 27/07/2020 16:00, Michael S. Tsirkin wrote: > 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!
Ironically, the bug has been introduced because of my comment on the series: https://patchew.org/QEMU/[email protected]/[email protected]/ Sorry for that. > 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. Yes. I'm going to do that. Thanks, Laurent
