All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] hw/usb: hcd-xhci-pci: Raise MSI/MSI-X interrupts only when told to
@ 2021-05-21  2:42 Bin Meng
  2021-05-21  2:42 ` [PATCH 2/2] hw/usb: hcd-xhci-pci: Fix spec violation of IP flag for MSI/MSI-X Bin Meng
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Bin Meng @ 2021-05-21  2:42 UTC (permalink / raw)
  To: Gerd Hoffmann, qemu-devel; +Cc: Bin Meng, Ruimei Yan

From: Ruimei Yan <ruimei.yan@windriver.com>

At present MSI / MSI-X interrupts are triggered regardless of the
irq level. We should have checked the level to determine whether
the interrupt needs to be delivered.

The level check logic was present in early versions of the xhci
model, but got dropped later by a rework of interrupt handling
under commit 4c4abe7cc903 ("xhci: rework interrupt handling").

Fixes: 4c4abe7cc903 ("xhci: rework interrupt handling")
Signed-off-by: Ruimei Yan <ruimei.yan@windriver.com>
Signed-off-by: Bin Meng <bin.meng@windriver.com>
---

 hw/usb/hcd-xhci-pci.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/usb/hcd-xhci-pci.c b/hw/usb/hcd-xhci-pci.c
index 9421734d0f..b6acd1790c 100644
--- a/hw/usb/hcd-xhci-pci.c
+++ b/hw/usb/hcd-xhci-pci.c
@@ -67,12 +67,13 @@ static void xhci_pci_intr_raise(XHCIState *xhci, int n, bool level)
          msi_enabled(pci_dev))) {
         pci_set_irq(pci_dev, level);
     }
-    if (msix_enabled(pci_dev)) {
+
+    if (msix_enabled(pci_dev) && level) {
         msix_notify(pci_dev, n);
         return;
     }
 
-    if (msi_enabled(pci_dev)) {
+    if (msi_enabled(pci_dev) && level) {
         msi_notify(pci_dev, n);
         return;
     }
-- 
2.25.1



^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 2/2] hw/usb: hcd-xhci-pci: Fix spec violation of IP flag for MSI/MSI-X
  2021-05-21  2:42 [PATCH 1/2] hw/usb: hcd-xhci-pci: Raise MSI/MSI-X interrupts only when told to Bin Meng
@ 2021-05-21  2:42 ` Bin Meng
  2021-05-21 12:46   ` Philippe Mathieu-Daudé
  2021-05-21 12:48 ` [PATCH 1/2] hw/usb: hcd-xhci-pci: Raise MSI/MSI-X interrupts only when told to Philippe Mathieu-Daudé
  2021-05-27  6:06 ` Bin Meng
  2 siblings, 1 reply; 7+ messages in thread
From: Bin Meng @ 2021-05-21  2:42 UTC (permalink / raw)
  To: Gerd Hoffmann, qemu-devel; +Cc: Bin Meng, Ruimei Yan

From: Ruimei Yan <ruimei.yan@windriver.com>

Per xHCI spec v1.2 chapter 4.17.5 page 296:

  If MSI or MSI-X interrupts are enabled, Interrupt Pending (IP)
  shall be cleared automatically when the PCI dword write generated
  by the interrupt assertion is complete.

Currently QEMU does not clear the IP flag in the MSI / MSI-X mode.
This causes subsequent spurious interrupt to be delivered to guests.
To solve this, we change the xhci intr_raise() hook routine to have
a bool return value that is passed to its caller (the xhci core),
with true indicating that IP should be self-cleared.

Fixes: 62c6ae04cf43 ("xhci: Initial xHCI implementation")
Fixes: 4c47f800631a ("xhci: add msix support")
Signed-off-by: Ruimei Yan <ruimei.yan@windriver.com>
[bmeng: move IP clear codes from xhci pci to xhci core]
Signed-off-by: Bin Meng <bin.meng@windriver.com>
---

 hw/usb/hcd-xhci.h        | 2 +-
 hw/usb/hcd-xhci-pci.c    | 8 +++++---
 hw/usb/hcd-xhci-sysbus.c | 4 +++-
 hw/usb/hcd-xhci.c        | 8 ++++++--
 4 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/hw/usb/hcd-xhci.h b/hw/usb/hcd-xhci.h
index 7bba361f3b..98f598382a 100644
--- a/hw/usb/hcd-xhci.h
+++ b/hw/usb/hcd-xhci.h
@@ -194,7 +194,7 @@ typedef struct XHCIState {
     uint32_t flags;
     uint32_t max_pstreams_mask;
     void (*intr_update)(XHCIState *s, int n, bool enable);
-    void (*intr_raise)(XHCIState *s, int n, bool level);
+    bool (*intr_raise)(XHCIState *s, int n, bool level);
     DeviceState *hostOpaque;
 
     /* Operational Registers */
diff --git a/hw/usb/hcd-xhci-pci.c b/hw/usb/hcd-xhci-pci.c
index b6acd1790c..e934b1a5b1 100644
--- a/hw/usb/hcd-xhci-pci.c
+++ b/hw/usb/hcd-xhci-pci.c
@@ -57,7 +57,7 @@ static void xhci_pci_intr_update(XHCIState *xhci, int n, bool enable)
     }
 }
 
-static void xhci_pci_intr_raise(XHCIState *xhci, int n, bool level)
+static bool xhci_pci_intr_raise(XHCIState *xhci, int n, bool level)
 {
     XHCIPciState *s = container_of(xhci, XHCIPciState, xhci);
     PCIDevice *pci_dev = PCI_DEVICE(s);
@@ -70,13 +70,15 @@ static void xhci_pci_intr_raise(XHCIState *xhci, int n, bool level)
 
     if (msix_enabled(pci_dev) && level) {
         msix_notify(pci_dev, n);
-        return;
+        return true;
     }
 
     if (msi_enabled(pci_dev) && level) {
         msi_notify(pci_dev, n);
-        return;
+        return true;
     }
+
+    return false;
 }
 
 static void xhci_pci_reset(DeviceState *dev)
diff --git a/hw/usb/hcd-xhci-sysbus.c b/hw/usb/hcd-xhci-sysbus.c
index 42e2574c82..a14e438196 100644
--- a/hw/usb/hcd-xhci-sysbus.c
+++ b/hw/usb/hcd-xhci-sysbus.c
@@ -16,11 +16,13 @@
 #include "hw/acpi/aml-build.h"
 #include "hw/irq.h"
 
-static void xhci_sysbus_intr_raise(XHCIState *xhci, int n, bool level)
+static bool xhci_sysbus_intr_raise(XHCIState *xhci, int n, bool level)
 {
     XHCISysbusState *s = container_of(xhci, XHCISysbusState, xhci);
 
     qemu_set_irq(s->irq[n], level);
+
+    return false;
 }
 
 void xhci_sysbus_reset(DeviceState *dev)
diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 46212b1e69..e01700039b 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -551,7 +551,9 @@ static void xhci_intr_update(XHCIState *xhci, int v)
             level = 1;
         }
         if (xhci->intr_raise) {
-            xhci->intr_raise(xhci, 0, level);
+            if (xhci->intr_raise(xhci, 0, level)) {
+                xhci->intr[0].iman &= ~IMAN_IP;
+            }
         }
     }
     if (xhci->intr_update) {
@@ -579,7 +581,9 @@ static void xhci_intr_raise(XHCIState *xhci, int v)
         return;
     }
     if (xhci->intr_raise) {
-        xhci->intr_raise(xhci, v, true);
+        if (xhci->intr_raise(xhci, v, true)) {
+            xhci->intr[v].iman &= ~IMAN_IP;
+        }
     }
 }
 
-- 
2.25.1



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] hw/usb: hcd-xhci-pci: Fix spec violation of IP flag for MSI/MSI-X
  2021-05-21  2:42 ` [PATCH 2/2] hw/usb: hcd-xhci-pci: Fix spec violation of IP flag for MSI/MSI-X Bin Meng
@ 2021-05-21 12:46   ` Philippe Mathieu-Daudé
  2021-05-21 13:25     ` Bin Meng
  0 siblings, 1 reply; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-05-21 12:46 UTC (permalink / raw)
  To: Bin Meng, Gerd Hoffmann, qemu-devel; +Cc: Bin Meng, Ruimei Yan

On 5/21/21 4:42 AM, Bin Meng wrote:
> From: Ruimei Yan <ruimei.yan@windriver.com>
> 
> Per xHCI spec v1.2 chapter 4.17.5 page 296:
> 
>   If MSI or MSI-X interrupts are enabled, Interrupt Pending (IP)
>   shall be cleared automatically when the PCI dword write generated
>   by the interrupt assertion is complete.
> 
> Currently QEMU does not clear the IP flag in the MSI / MSI-X mode.
> This causes subsequent spurious interrupt to be delivered to guests.
> To solve this, we change the xhci intr_raise() hook routine to have
> a bool return value that is passed to its caller (the xhci core),
> with true indicating that IP should be self-cleared.
> 
> Fixes: 62c6ae04cf43 ("xhci: Initial xHCI implementation")
> Fixes: 4c47f800631a ("xhci: add msix support")
> Signed-off-by: Ruimei Yan <ruimei.yan@windriver.com>
> [bmeng: move IP clear codes from xhci pci to xhci core]
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> ---
> 
>  hw/usb/hcd-xhci.h        | 2 +-
>  hw/usb/hcd-xhci-pci.c    | 8 +++++---
>  hw/usb/hcd-xhci-sysbus.c | 4 +++-
>  hw/usb/hcd-xhci.c        | 8 ++++++--
>  4 files changed, 15 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/usb/hcd-xhci.h b/hw/usb/hcd-xhci.h
> index 7bba361f3b..98f598382a 100644
> --- a/hw/usb/hcd-xhci.h
> +++ b/hw/usb/hcd-xhci.h
> @@ -194,7 +194,7 @@ typedef struct XHCIState {
>      uint32_t flags;
>      uint32_t max_pstreams_mask;
>      void (*intr_update)(XHCIState *s, int n, bool enable);
> -    void (*intr_raise)(XHCIState *s, int n, bool level);
> +    bool (*intr_raise)(XHCIState *s, int n, bool level);
>      DeviceState *hostOpaque;
>  
>      /* Operational Registers */
> diff --git a/hw/usb/hcd-xhci-pci.c b/hw/usb/hcd-xhci-pci.c
> index b6acd1790c..e934b1a5b1 100644
> --- a/hw/usb/hcd-xhci-pci.c
> +++ b/hw/usb/hcd-xhci-pci.c
> @@ -57,7 +57,7 @@ static void xhci_pci_intr_update(XHCIState *xhci, int n, bool enable)
>      }
>  }
>  
> -static void xhci_pci_intr_raise(XHCIState *xhci, int n, bool level)
> +static bool xhci_pci_intr_raise(XHCIState *xhci, int n, bool level)
>  {
>      XHCIPciState *s = container_of(xhci, XHCIPciState, xhci);
>      PCIDevice *pci_dev = PCI_DEVICE(s);
> @@ -70,13 +70,15 @@ static void xhci_pci_intr_raise(XHCIState *xhci, int n, bool level)
>  
>      if (msix_enabled(pci_dev) && level) {
>          msix_notify(pci_dev, n);
> -        return;
> +        return true;
>      }
>  
>      if (msi_enabled(pci_dev) && level) {
>          msi_notify(pci_dev, n);
> -        return;
> +        return true;
>      }
> +
> +    return false;
>  }

Could be simplified as:

  if (!level) {
    return false;
  }
  if (msix_enabled(pci_dev)) {
    msix_notify(pci_dev, n);
  }
  if (msi_enabled(pci_dev)) {
    msi_notify(pci_dev, n);
  }
  return true;

Otherwise,
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] hw/usb: hcd-xhci-pci: Raise MSI/MSI-X interrupts only when told to
  2021-05-21  2:42 [PATCH 1/2] hw/usb: hcd-xhci-pci: Raise MSI/MSI-X interrupts only when told to Bin Meng
  2021-05-21  2:42 ` [PATCH 2/2] hw/usb: hcd-xhci-pci: Fix spec violation of IP flag for MSI/MSI-X Bin Meng
@ 2021-05-21 12:48 ` Philippe Mathieu-Daudé
  2021-05-27  6:06 ` Bin Meng
  2 siblings, 0 replies; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-05-21 12:48 UTC (permalink / raw)
  To: Bin Meng, Gerd Hoffmann, qemu-devel; +Cc: Bin Meng, Ruimei Yan

On 5/21/21 4:42 AM, Bin Meng wrote:
> From: Ruimei Yan <ruimei.yan@windriver.com>
> 
> At present MSI / MSI-X interrupts are triggered regardless of the
> irq level. We should have checked the level to determine whether
> the interrupt needs to be delivered.
> 
> The level check logic was present in early versions of the xhci
> model, but got dropped later by a rework of interrupt handling
> under commit 4c4abe7cc903 ("xhci: rework interrupt handling").
> 
> Fixes: 4c4abe7cc903 ("xhci: rework interrupt handling")
> Signed-off-by: Ruimei Yan <ruimei.yan@windriver.com>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> ---
> 
>  hw/usb/hcd-xhci-pci.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/usb/hcd-xhci-pci.c b/hw/usb/hcd-xhci-pci.c
> index 9421734d0f..b6acd1790c 100644
> --- a/hw/usb/hcd-xhci-pci.c
> +++ b/hw/usb/hcd-xhci-pci.c
> @@ -67,12 +67,13 @@ static void xhci_pci_intr_raise(XHCIState *xhci, int n, bool level)
>           msi_enabled(pci_dev))) {
>          pci_set_irq(pci_dev, level);
>      }

I read the next patch before this one :)

So please consider:

if (!level) {
  return;
}

Otherwise:
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> -    if (msix_enabled(pci_dev)) {
> +
> +    if (msix_enabled(pci_dev) && level) {
>          msix_notify(pci_dev, n);
>          return;
>      }
>  
> -    if (msi_enabled(pci_dev)) {
> +    if (msi_enabled(pci_dev) && level) {
>          msi_notify(pci_dev, n);
>          return;
>      }
> 



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] hw/usb: hcd-xhci-pci: Fix spec violation of IP flag for MSI/MSI-X
  2021-05-21 12:46   ` Philippe Mathieu-Daudé
@ 2021-05-21 13:25     ` Bin Meng
  2021-05-21 15:44       ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 7+ messages in thread
From: Bin Meng @ 2021-05-21 13:25 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Bin Meng, Ruimei Yan, Gerd Hoffmann, qemu-devel@nongnu.org Developers

On Fri, May 21, 2021 at 8:46 PM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> On 5/21/21 4:42 AM, Bin Meng wrote:
> > From: Ruimei Yan <ruimei.yan@windriver.com>
> >
> > Per xHCI spec v1.2 chapter 4.17.5 page 296:
> >
> >   If MSI or MSI-X interrupts are enabled, Interrupt Pending (IP)
> >   shall be cleared automatically when the PCI dword write generated
> >   by the interrupt assertion is complete.
> >
> > Currently QEMU does not clear the IP flag in the MSI / MSI-X mode.
> > This causes subsequent spurious interrupt to be delivered to guests.
> > To solve this, we change the xhci intr_raise() hook routine to have
> > a bool return value that is passed to its caller (the xhci core),
> > with true indicating that IP should be self-cleared.
> >
> > Fixes: 62c6ae04cf43 ("xhci: Initial xHCI implementation")
> > Fixes: 4c47f800631a ("xhci: add msix support")
> > Signed-off-by: Ruimei Yan <ruimei.yan@windriver.com>
> > [bmeng: move IP clear codes from xhci pci to xhci core]
> > Signed-off-by: Bin Meng <bin.meng@windriver.com>
> > ---
> >
> >  hw/usb/hcd-xhci.h        | 2 +-
> >  hw/usb/hcd-xhci-pci.c    | 8 +++++---
> >  hw/usb/hcd-xhci-sysbus.c | 4 +++-
> >  hw/usb/hcd-xhci.c        | 8 ++++++--
> >  4 files changed, 15 insertions(+), 7 deletions(-)
> >
> > diff --git a/hw/usb/hcd-xhci.h b/hw/usb/hcd-xhci.h
> > index 7bba361f3b..98f598382a 100644
> > --- a/hw/usb/hcd-xhci.h
> > +++ b/hw/usb/hcd-xhci.h
> > @@ -194,7 +194,7 @@ typedef struct XHCIState {
> >      uint32_t flags;
> >      uint32_t max_pstreams_mask;
> >      void (*intr_update)(XHCIState *s, int n, bool enable);
> > -    void (*intr_raise)(XHCIState *s, int n, bool level);
> > +    bool (*intr_raise)(XHCIState *s, int n, bool level);
> >      DeviceState *hostOpaque;
> >
> >      /* Operational Registers */
> > diff --git a/hw/usb/hcd-xhci-pci.c b/hw/usb/hcd-xhci-pci.c
> > index b6acd1790c..e934b1a5b1 100644
> > --- a/hw/usb/hcd-xhci-pci.c
> > +++ b/hw/usb/hcd-xhci-pci.c
> > @@ -57,7 +57,7 @@ static void xhci_pci_intr_update(XHCIState *xhci, int n, bool enable)
> >      }
> >  }
> >
> > -static void xhci_pci_intr_raise(XHCIState *xhci, int n, bool level)
> > +static bool xhci_pci_intr_raise(XHCIState *xhci, int n, bool level)
> >  {
> >      XHCIPciState *s = container_of(xhci, XHCIPciState, xhci);
> >      PCIDevice *pci_dev = PCI_DEVICE(s);
> > @@ -70,13 +70,15 @@ static void xhci_pci_intr_raise(XHCIState *xhci, int n, bool level)
> >
> >      if (msix_enabled(pci_dev) && level) {
> >          msix_notify(pci_dev, n);
> > -        return;
> > +        return true;
> >      }
> >
> >      if (msi_enabled(pci_dev) && level) {
> >          msi_notify(pci_dev, n);
> > -        return;
> > +        return true;
> >      }
> > +
> > +    return false;
> >  }
>
> Could be simplified as:
>
>   if (!level) {
>     return false;
>   }
>   if (msix_enabled(pci_dev)) {
>     msix_notify(pci_dev, n);
>   }
>   if (msi_enabled(pci_dev)) {
>     msi_notify(pci_dev, n);
>   }
>   return true;

The simplified logic will deliver both interrupts if both msix and msi
are enabled. The previous logic prevents such from happening.

>
> Otherwise,
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>

Regards,
Bin


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] hw/usb: hcd-xhci-pci: Fix spec violation of IP flag for MSI/MSI-X
  2021-05-21 13:25     ` Bin Meng
@ 2021-05-21 15:44       ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-05-21 15:44 UTC (permalink / raw)
  To: Bin Meng
  Cc: Bin Meng, Ruimei Yan, Gerd Hoffmann, qemu-devel@nongnu.org Developers

On 5/21/21 3:25 PM, Bin Meng wrote:
> On Fri, May 21, 2021 at 8:46 PM Philippe Mathieu-Daudé
> <philmd@redhat.com> wrote:
>>
>> On 5/21/21 4:42 AM, Bin Meng wrote:
>>> From: Ruimei Yan <ruimei.yan@windriver.com>
>>>
>>> Per xHCI spec v1.2 chapter 4.17.5 page 296:
>>>
>>>   If MSI or MSI-X interrupts are enabled, Interrupt Pending (IP)
>>>   shall be cleared automatically when the PCI dword write generated
>>>   by the interrupt assertion is complete.
>>>
>>> Currently QEMU does not clear the IP flag in the MSI / MSI-X mode.
>>> This causes subsequent spurious interrupt to be delivered to guests.
>>> To solve this, we change the xhci intr_raise() hook routine to have
>>> a bool return value that is passed to its caller (the xhci core),
>>> with true indicating that IP should be self-cleared.
>>>
>>> Fixes: 62c6ae04cf43 ("xhci: Initial xHCI implementation")
>>> Fixes: 4c47f800631a ("xhci: add msix support")
>>> Signed-off-by: Ruimei Yan <ruimei.yan@windriver.com>
>>> [bmeng: move IP clear codes from xhci pci to xhci core]
>>> Signed-off-by: Bin Meng <bin.meng@windriver.com>
>>> ---
>>>
>>>  hw/usb/hcd-xhci.h        | 2 +-
>>>  hw/usb/hcd-xhci-pci.c    | 8 +++++---
>>>  hw/usb/hcd-xhci-sysbus.c | 4 +++-
>>>  hw/usb/hcd-xhci.c        | 8 ++++++--
>>>  4 files changed, 15 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/hw/usb/hcd-xhci.h b/hw/usb/hcd-xhci.h
>>> index 7bba361f3b..98f598382a 100644
>>> --- a/hw/usb/hcd-xhci.h
>>> +++ b/hw/usb/hcd-xhci.h
>>> @@ -194,7 +194,7 @@ typedef struct XHCIState {
>>>      uint32_t flags;
>>>      uint32_t max_pstreams_mask;
>>>      void (*intr_update)(XHCIState *s, int n, bool enable);
>>> -    void (*intr_raise)(XHCIState *s, int n, bool level);
>>> +    bool (*intr_raise)(XHCIState *s, int n, bool level);
>>>      DeviceState *hostOpaque;
>>>
>>>      /* Operational Registers */
>>> diff --git a/hw/usb/hcd-xhci-pci.c b/hw/usb/hcd-xhci-pci.c
>>> index b6acd1790c..e934b1a5b1 100644
>>> --- a/hw/usb/hcd-xhci-pci.c
>>> +++ b/hw/usb/hcd-xhci-pci.c
>>> @@ -57,7 +57,7 @@ static void xhci_pci_intr_update(XHCIState *xhci, int n, bool enable)
>>>      }
>>>  }
>>>
>>> -static void xhci_pci_intr_raise(XHCIState *xhci, int n, bool level)
>>> +static bool xhci_pci_intr_raise(XHCIState *xhci, int n, bool level)
>>>  {
>>>      XHCIPciState *s = container_of(xhci, XHCIPciState, xhci);
>>>      PCIDevice *pci_dev = PCI_DEVICE(s);
>>> @@ -70,13 +70,15 @@ static void xhci_pci_intr_raise(XHCIState *xhci, int n, bool level)
>>>
>>>      if (msix_enabled(pci_dev) && level) {
>>>          msix_notify(pci_dev, n);
>>> -        return;
>>> +        return true;
>>>      }
>>>
>>>      if (msi_enabled(pci_dev) && level) {
>>>          msi_notify(pci_dev, n);
>>> -        return;
>>> +        return true;
>>>      }
>>> +
>>> +    return false;
>>>  }
>>
>> Could be simplified as:
>>
>>   if (!level) {
>>     return false;
>>   }
>>   if (msix_enabled(pci_dev)) {
>>     msix_notify(pci_dev, n);
>>   }
>>   if (msi_enabled(pci_dev)) {
>>     msi_notify(pci_dev, n);
>>   }
>>   return true;
> 
> The simplified logic will deliver both interrupts if both msix and msi
> are enabled. The previous logic prevents such from happening.

Oops you are right :)

>> Otherwise,
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

This stands.



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] hw/usb: hcd-xhci-pci: Raise MSI/MSI-X interrupts only when told to
  2021-05-21  2:42 [PATCH 1/2] hw/usb: hcd-xhci-pci: Raise MSI/MSI-X interrupts only when told to Bin Meng
  2021-05-21  2:42 ` [PATCH 2/2] hw/usb: hcd-xhci-pci: Fix spec violation of IP flag for MSI/MSI-X Bin Meng
  2021-05-21 12:48 ` [PATCH 1/2] hw/usb: hcd-xhci-pci: Raise MSI/MSI-X interrupts only when told to Philippe Mathieu-Daudé
@ 2021-05-27  6:06 ` Bin Meng
  2 siblings, 0 replies; 7+ messages in thread
From: Bin Meng @ 2021-05-27  6:06 UTC (permalink / raw)
  To: Gerd Hoffmann, qemu-devel@nongnu.org Developers; +Cc: Bin Meng, Ruimei Yan

On Fri, May 21, 2021 at 10:42 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> From: Ruimei Yan <ruimei.yan@windriver.com>
>
> At present MSI / MSI-X interrupts are triggered regardless of the
> irq level. We should have checked the level to determine whether
> the interrupt needs to be delivered.
>
> The level check logic was present in early versions of the xhci
> model, but got dropped later by a rework of interrupt handling
> under commit 4c4abe7cc903 ("xhci: rework interrupt handling").
>
> Fixes: 4c4abe7cc903 ("xhci: rework interrupt handling")
> Signed-off-by: Ruimei Yan <ruimei.yan@windriver.com>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> ---
>
>  hw/usb/hcd-xhci-pci.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>

Ping?


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-05-27  6:08 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-21  2:42 [PATCH 1/2] hw/usb: hcd-xhci-pci: Raise MSI/MSI-X interrupts only when told to Bin Meng
2021-05-21  2:42 ` [PATCH 2/2] hw/usb: hcd-xhci-pci: Fix spec violation of IP flag for MSI/MSI-X Bin Meng
2021-05-21 12:46   ` Philippe Mathieu-Daudé
2021-05-21 13:25     ` Bin Meng
2021-05-21 15:44       ` Philippe Mathieu-Daudé
2021-05-21 12:48 ` [PATCH 1/2] hw/usb: hcd-xhci-pci: Raise MSI/MSI-X interrupts only when told to Philippe Mathieu-Daudé
2021-05-27  6:06 ` Bin Meng

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.