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

[Andreas Färber]

Am 10.03.2015 um 21:50 schrieb John Snow:
> 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.

CC'ing the PCI/virtio maintainer might help. ;)

Cheers,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)