diff mbox series

[v2,1/2] pcie: Don't try triggering a LSI when not defined

Message ID 20220408131303.147840-2-fbarrat@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series Remove PCIE root bridge LSI on powernv | expand

Commit Message

Frederic Barrat April 8, 2022, 1:13 p.m. UTC
This patch skips [de]asserting a LSI interrupt if the device doesn't
have any LSI defined. Doing so would trigger an assert in
pci_irq_handler().

The PCIE root port implementation in qemu requests a LSI (INTA), but a
subclass may want to change that behavior since it's a valid
configuration. For example on the POWER8/POWER9/POWER10 systems, the
root bridge doesn't request any LSI.

Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
---
 hw/pci/pcie.c     | 5 +++--
 hw/pci/pcie_aer.c | 2 +-
 2 files changed, 4 insertions(+), 3 deletions(-)

Comments

Daniel Henrique Barboza April 8, 2022, 9:12 p.m. UTC | #1
On 4/8/22 10:13, Frederic Barrat wrote:
> This patch skips [de]asserting a LSI interrupt if the device doesn't
> have any LSI defined. Doing so would trigger an assert in
> pci_irq_handler().
> 
> The PCIE root port implementation in qemu requests a LSI (INTA), but a
> subclass may want to change that behavior since it's a valid
> configuration. For example on the POWER8/POWER9/POWER10 systems, the
> root bridge doesn't request any LSI.
> 
> Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
> ---

Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>

>   hw/pci/pcie.c     | 5 +++--
>   hw/pci/pcie_aer.c | 2 +-
>   2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> index 67a5d67372..68a62da0b5 100644
> --- a/hw/pci/pcie.c
> +++ b/hw/pci/pcie.c
> @@ -353,7 +353,7 @@ static void hotplug_event_notify(PCIDevice *dev)
>           msix_notify(dev, pcie_cap_flags_get_vector(dev));
>       } else if (msi_enabled(dev)) {
>           msi_notify(dev, pcie_cap_flags_get_vector(dev));
> -    } else {
> +    } else if (pci_intx(dev) != -1) {
>           pci_set_irq(dev, dev->exp.hpev_notified);
>       }
>   }
> @@ -361,7 +361,8 @@ static void hotplug_event_notify(PCIDevice *dev)
>   static void hotplug_event_clear(PCIDevice *dev)
>   {
>       hotplug_event_update_event_status(dev);
> -    if (!msix_enabled(dev) && !msi_enabled(dev) && !dev->exp.hpev_notified) {
> +    if (!msix_enabled(dev) && !msi_enabled(dev) && pci_intx(dev) != -1 &&
> +        !dev->exp.hpev_notified) {
>           pci_irq_deassert(dev);
>       }
>   }
> diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c
> index e1a8a88c8c..92bd0530dd 100644
> --- a/hw/pci/pcie_aer.c
> +++ b/hw/pci/pcie_aer.c
> @@ -290,7 +290,7 @@ static void pcie_aer_root_notify(PCIDevice *dev)
>           msix_notify(dev, pcie_aer_root_get_vector(dev));
>       } else if (msi_enabled(dev)) {
>           msi_notify(dev, pcie_aer_root_get_vector(dev));
> -    } else {
> +    } else if (pci_intx(dev) != -1) {
>           pci_irq_assert(dev);
>       }
>   }
Daniel Henrique Barboza April 20, 2022, 5:09 p.m. UTC | #2
Michael,

Let me know if you want me to grab this patch from the ppc tree. I'm
going to send a PR in the next few days.


Thanks,


Daniel

On 4/8/22 10:13, Frederic Barrat wrote:
> This patch skips [de]asserting a LSI interrupt if the device doesn't
> have any LSI defined. Doing so would trigger an assert in
> pci_irq_handler().
> 
> The PCIE root port implementation in qemu requests a LSI (INTA), but a
> subclass may want to change that behavior since it's a valid
> configuration. For example on the POWER8/POWER9/POWER10 systems, the
> root bridge doesn't request any LSI.
> 
> Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
> ---
>   hw/pci/pcie.c     | 5 +++--
>   hw/pci/pcie_aer.c | 2 +-
>   2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> index 67a5d67372..68a62da0b5 100644
> --- a/hw/pci/pcie.c
> +++ b/hw/pci/pcie.c
> @@ -353,7 +353,7 @@ static void hotplug_event_notify(PCIDevice *dev)
>           msix_notify(dev, pcie_cap_flags_get_vector(dev));
>       } else if (msi_enabled(dev)) {
>           msi_notify(dev, pcie_cap_flags_get_vector(dev));
> -    } else {
> +    } else if (pci_intx(dev) != -1) {
>           pci_set_irq(dev, dev->exp.hpev_notified);
>       }
>   }
> @@ -361,7 +361,8 @@ static void hotplug_event_notify(PCIDevice *dev)
>   static void hotplug_event_clear(PCIDevice *dev)
>   {
>       hotplug_event_update_event_status(dev);
> -    if (!msix_enabled(dev) && !msi_enabled(dev) && !dev->exp.hpev_notified) {
> +    if (!msix_enabled(dev) && !msi_enabled(dev) && pci_intx(dev) != -1 &&
> +        !dev->exp.hpev_notified) {
>           pci_irq_deassert(dev);
>       }
>   }
> diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c
> index e1a8a88c8c..92bd0530dd 100644
> --- a/hw/pci/pcie_aer.c
> +++ b/hw/pci/pcie_aer.c
> @@ -290,7 +290,7 @@ static void pcie_aer_root_notify(PCIDevice *dev)
>           msix_notify(dev, pcie_aer_root_get_vector(dev));
>       } else if (msi_enabled(dev)) {
>           msi_notify(dev, pcie_aer_root_get_vector(dev));
> -    } else {
> +    } else if (pci_intx(dev) != -1) {
>           pci_irq_assert(dev);
>       }
>   }
Michael S. Tsirkin April 20, 2022, 5:18 p.m. UTC | #3
On Fri, Apr 08, 2022 at 03:13:02PM +0200, Frederic Barrat wrote:
> This patch skips [de]asserting a LSI interrupt if the device doesn't
> have any LSI defined. Doing so would trigger an assert in
> pci_irq_handler().
> 
> The PCIE root port implementation in qemu requests a LSI (INTA), but a
> subclass may want to change that behavior since it's a valid
> configuration. For example on the POWER8/POWER9/POWER10 systems, the
> root bridge doesn't request any LSI.
> 
> Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>


Reviewed-by: Michael S. Tsirkin <mst@redhat.com>


Feel free to merge with the second patch. Thanks!

> ---
>  hw/pci/pcie.c     | 5 +++--
>  hw/pci/pcie_aer.c | 2 +-
>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> index 67a5d67372..68a62da0b5 100644
> --- a/hw/pci/pcie.c
> +++ b/hw/pci/pcie.c
> @@ -353,7 +353,7 @@ static void hotplug_event_notify(PCIDevice *dev)
>          msix_notify(dev, pcie_cap_flags_get_vector(dev));
>      } else if (msi_enabled(dev)) {
>          msi_notify(dev, pcie_cap_flags_get_vector(dev));
> -    } else {
> +    } else if (pci_intx(dev) != -1) {
>          pci_set_irq(dev, dev->exp.hpev_notified);
>      }
>  }
> @@ -361,7 +361,8 @@ static void hotplug_event_notify(PCIDevice *dev)
>  static void hotplug_event_clear(PCIDevice *dev)
>  {
>      hotplug_event_update_event_status(dev);
> -    if (!msix_enabled(dev) && !msi_enabled(dev) && !dev->exp.hpev_notified) {
> +    if (!msix_enabled(dev) && !msi_enabled(dev) && pci_intx(dev) != -1 &&
> +        !dev->exp.hpev_notified) {
>          pci_irq_deassert(dev);
>      }
>  }
> diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c
> index e1a8a88c8c..92bd0530dd 100644
> --- a/hw/pci/pcie_aer.c
> +++ b/hw/pci/pcie_aer.c
> @@ -290,7 +290,7 @@ static void pcie_aer_root_notify(PCIDevice *dev)
>          msix_notify(dev, pcie_aer_root_get_vector(dev));
>      } else if (msi_enabled(dev)) {
>          msi_notify(dev, pcie_aer_root_get_vector(dev));
> -    } else {
> +    } else if (pci_intx(dev) != -1) {
>          pci_irq_assert(dev);
>      }
>  }
> -- 
> 2.35.1
diff mbox series

Patch

diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index 67a5d67372..68a62da0b5 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -353,7 +353,7 @@  static void hotplug_event_notify(PCIDevice *dev)
         msix_notify(dev, pcie_cap_flags_get_vector(dev));
     } else if (msi_enabled(dev)) {
         msi_notify(dev, pcie_cap_flags_get_vector(dev));
-    } else {
+    } else if (pci_intx(dev) != -1) {
         pci_set_irq(dev, dev->exp.hpev_notified);
     }
 }
@@ -361,7 +361,8 @@  static void hotplug_event_notify(PCIDevice *dev)
 static void hotplug_event_clear(PCIDevice *dev)
 {
     hotplug_event_update_event_status(dev);
-    if (!msix_enabled(dev) && !msi_enabled(dev) && !dev->exp.hpev_notified) {
+    if (!msix_enabled(dev) && !msi_enabled(dev) && pci_intx(dev) != -1 &&
+        !dev->exp.hpev_notified) {
         pci_irq_deassert(dev);
     }
 }
diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c
index e1a8a88c8c..92bd0530dd 100644
--- a/hw/pci/pcie_aer.c
+++ b/hw/pci/pcie_aer.c
@@ -290,7 +290,7 @@  static void pcie_aer_root_notify(PCIDevice *dev)
         msix_notify(dev, pcie_aer_root_get_vector(dev));
     } else if (msi_enabled(dev)) {
         msi_notify(dev, pcie_aer_root_get_vector(dev));
-    } else {
+    } else if (pci_intx(dev) != -1) {
         pci_irq_assert(dev);
     }
 }