Re: [Qemu-devel] [PATCH] libqos: Solve bug in interrupt checking when using MSIX in virtio-pci.c

[Marc Marí]

El Tue, 10 Mar 2015 16:50:48 -0400
John Snow <address@hidden> escribió:
> 
> 
> On 02/24/2015 01:09 PM, John Snow wrote:
> >
> >
> > On 02/24/2015 11:34 AM, Marc Marí wrote:
> >> The MSIX interrupt was always acked without checking its value,
> >> which caused a
> >> race condition. If the ISR was raised between the read and the
> >> acking, the ISR
> >> was never detected and it timed out.
> >>
> >> Signed-off-by: Marc Marí <address@hidden>
> >> ---
> >>   tests/libqos/virtio-pci.c |   16 ++++++++++++----
> >>   1 file changed, 12 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c
> >> index 788ebaf..c74a669 100644
> >> --- a/tests/libqos/virtio-pci.c
> >> +++ b/tests/libqos/virtio-pci.c
> >> @@ -140,8 +140,12 @@ static bool
> >> qvirtio_pci_get_queue_isr_status(QVirtioDevice *d, QVirtQueue *vq)
> >>               return qpci_msix_pending(dev->pdev,
> >> vqpci->msix_entry); } else {
> >>               data = readl(vqpci->msix_addr);
> >> -            writel(vqpci->msix_addr, 0);
> >> -            return data == vqpci->msix_data;
> >> +            if (data == vqpci->msix_data) {
> >> +                writel(vqpci->msix_addr, 0);
> >> +                return true;
> >> +            } else {
> >> +                return false;
> >> +            }
> >>           }
> >>       } else {
> >>           return qpci_io_readb(dev->pdev, dev->addr +
> >> QVIRTIO_ISR_STATUS) & 1;
> >> @@ -160,8 +164,12 @@ static bool
> >> qvirtio_pci_get_config_isr_status(QVirtioDevice *d)
> >>               return qpci_msix_pending(dev->pdev,
> >> dev->config_msix_entry);
> >>           } else {
> >>               data = readl(dev->config_msix_addr);
> >> -            writel(dev->config_msix_addr, 0);
> >> -            return data == dev->config_msix_data;
> >> +            if (data == dev->config_msix_data) {
> >> +                writel(dev->config_msix_addr, 0);
> >> +                return true;
> >> +            } else {
> >> +                return false;
> >> +            }
> >>           }
> >>       } else {
> >>           return qpci_io_readb(dev->pdev, dev->addr +
> >> QVIRTIO_ISR_STATUS) & 2;
> >>
> >
> > 1,600+ runs and no hang, thanks :)
> >
> > Tested-by: John Snow <address@hidden>
> > Reviewed-by: John Snow <address@hidden>
> >
> 
> Ping?
> 
> Still hitting this failure upstream.

I think this is in Stefan's pull request, which has issues merging.

Marc