* [Qemu-devel] [PATCH] libqos: Solve bug in interrupt checking when using MSIX in virtio-pci.c
@ 2015-02-24 16:34 Marc Marí
2015-02-24 18:09 ` John Snow
0 siblings, 1 reply; 5+ messages in thread
From: Marc Marí @ 2015-02-24 16:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Marc Marí, jsnow, afaerber, stefanha
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í <marc.mari.barcelo@gmail.com>
---
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.7.10.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [Qemu-devel] [PATCH] libqos: Solve bug in interrupt checking when using MSIX in virtio-pci.c
2015-02-24 16:34 [Qemu-devel] [PATCH] libqos: Solve bug in interrupt checking when using MSIX in virtio-pci.c Marc Marí
@ 2015-02-24 18:09 ` John Snow
2015-03-10 20:50 ` John Snow
0 siblings, 1 reply; 5+ messages in thread
From: John Snow @ 2015-02-24 18:09 UTC (permalink / raw)
To: Marc Marí, qemu-devel; +Cc: afaerber, stefanha
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í <marc.mari.barcelo@gmail.com>
> ---
> 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 <jsnow@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [Qemu-devel] [PATCH] libqos: Solve bug in interrupt checking when using MSIX in virtio-pci.c
2015-02-24 18:09 ` John Snow
@ 2015-03-10 20:50 ` John Snow
2015-03-10 20:53 ` Marc Marí
2015-03-10 20:53 ` Andreas Färber
0 siblings, 2 replies; 5+ messages in thread
From: John Snow @ 2015-03-10 20:50 UTC (permalink / raw)
To: stefanha; +Cc: Marc Marí, qemu-devel, afaerber
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í <marc.mari.barcelo@gmail.com>
>> ---
>> 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 <jsnow@redhat.com>
> Reviewed-by: John Snow <jsnow@redhat.com>
>
Ping?
Still hitting this failure upstream.
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [Qemu-devel] [PATCH] libqos: Solve bug in interrupt checking when using MSIX in virtio-pci.c
2015-03-10 20:50 ` John Snow
@ 2015-03-10 20:53 ` Marc Marí
2015-03-10 20:53 ` Andreas Färber
1 sibling, 0 replies; 5+ messages in thread
From: Marc Marí @ 2015-03-10 20:53 UTC (permalink / raw)
To: John Snow; +Cc: qemu-devel, stefanha, afaerber
El Tue, 10 Mar 2015 16:50:48 -0400
John Snow <jsnow@redhat.com> 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í <marc.mari.barcelo@gmail.com>
> >> ---
> >> 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 <jsnow@redhat.com>
> > Reviewed-by: John Snow <jsnow@redhat.com>
> >
>
> Ping?
>
> Still hitting this failure upstream.
I think this is in Stefan's pull request, which has issues merging.
Marc
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [Qemu-devel] [PATCH] libqos: Solve bug in interrupt checking when using MSIX in virtio-pci.c
2015-03-10 20:50 ` John Snow
2015-03-10 20:53 ` Marc Marí
@ 2015-03-10 20:53 ` Andreas Färber
1 sibling, 0 replies; 5+ messages in thread
From: Andreas Färber @ 2015-03-10 20:53 UTC (permalink / raw)
To: John Snow; +Cc: Marc Marí, qemu-devel, stefanha, Michael S. Tsirkin
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í <marc.mari.barcelo@gmail.com>
>>> ---
>>> 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 <jsnow@redhat.com>
>> Reviewed-by: John Snow <jsnow@redhat.com>
>>
>
> 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)
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-03-10 20:53 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-02-24 16:34 [Qemu-devel] [PATCH] libqos: Solve bug in interrupt checking when using MSIX in virtio-pci.c Marc Marí
2015-02-24 18:09 ` John Snow
2015-03-10 20:50 ` John Snow
2015-03-10 20:53 ` Marc Marí
2015-03-10 20:53 ` Andreas Färber
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).