[PATCH 0/3] Fix irq allocation of PCI host bridge on powernv

[PATCH 0/3] Fix irq allocation of PCI host bridge on powernv

[Frederic Barrat]

This series removes a bogus allocation of a LSI interrupt for the PCI
Host Bridge found in the powernv model (phb4). The real hardware
doesn't declare any LSI, so the model should match. It was causing
some inconsistencies in the interrupt controller data.

However, removing that LSI shows that the PCI AER code assumes an
interrupt is defined (LSI or MSI or MSI-X), which is not the case with
the root bridge device on powernv. So the last patch adds a check to
make sure a LSI is defined before entering pci_set_irq() as it asserts
if it's called with no LSI defined.


Frederic Barrat (3):
  ppc/pnv: Tune the POWER9 PCIe Host bridge model
  pci: Export the pci_intx() function
  pcie_aer: Don't trigger a LSI if none are defined

 hw/pci-host/pnv_phb4.c | 5 ++++-
 hw/pci/pci.c           | 5 -----
 hw/pci/pcie_aer.c      | 4 +++-
 include/hw/pci/pci.h   | 5 +++++
 4 files changed, 12 insertions(+), 7 deletions(-)

-- 
2.33.1

[PATCH 1/3] ppc/pnv: Tune the POWER9 PCIe Host bridge model

[Frederic Barrat]

The PHB v4 found on POWER9 doesn't request any LSI, so let's clear the
Interrupt Pin register in the config space so that the model matches
the hardware.

If we don't, then we inherit from the default pcie root bridge, which
requests a LSI. And because we don't map it correctly in the device
tree, all PHBs allocate the same bogus hw interrupt. We end up with
inconsistent interrupt controller (xive) data. The problem goes away
if we don't allocate the LSI in the first place.

Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
---
 hw/pci-host/pnv_phb4.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
index 5c375a9f28..1659d55b4f 100644
--- a/hw/pci-host/pnv_phb4.c
+++ b/hw/pci-host/pnv_phb4.c
@@ -1234,10 +1234,13 @@ static void pnv_phb4_reset(DeviceState *dev)
     PCIDevice *root_dev = PCI_DEVICE(&phb->root);
 
     /*
-     * Configure PCI device id at reset using a property.
+     * Configure the PCI device at reset:
+     *   - set the Vendor and Device ID to for the root bridge
+     *   - no LSI
      */
     pci_config_set_vendor_id(root_dev->config, PCI_VENDOR_ID_IBM);
     pci_config_set_device_id(root_dev->config, phb->device_id);
+    pci_config_set_interrupt_pin(root_dev->config, 0);
 }
 
 static const char *pnv_phb4_root_bus_path(PCIHostState *host_bridge,
-- 
2.33.1

Re: [PATCH 1/3] ppc/pnv: Tune the POWER9 PCIe Host bridge model

[Cédric Le Goater]

On 11/16/21 18:01, Frederic Barrat wrote:
> The PHB v4 found on POWER9 doesn't request any LSI, so let's clear the
> Interrupt Pin register in the config space so that the model matches
> the hardware.
> 
> If we don't, then we inherit from the default pcie root bridge, which
> requests a LSI. And because we don't map it correctly in the device
> tree, all PHBs allocate the same bogus hw interrupt. We end up with
> inconsistent interrupt controller (xive) data. The problem goes away
> if we don't allocate the LSI in the first place.
> 
> Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>

Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.


> ---
>   hw/pci-host/pnv_phb4.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
> index 5c375a9f28..1659d55b4f 100644
> --- a/hw/pci-host/pnv_phb4.c
> +++ b/hw/pci-host/pnv_phb4.c
> @@ -1234,10 +1234,13 @@ static void pnv_phb4_reset(DeviceState *dev)
>       PCIDevice *root_dev = PCI_DEVICE(&phb->root);
>   
>       /*
> -     * Configure PCI device id at reset using a property.
> +     * Configure the PCI device at reset:
> +     *   - set the Vendor and Device ID to for the root bridge
> +     *   - no LSI
>        */
>       pci_config_set_vendor_id(root_dev->config, PCI_VENDOR_ID_IBM);
>       pci_config_set_device_id(root_dev->config, phb->device_id);
> +    pci_config_set_interrupt_pin(root_dev->config, 0);
>   }
>   
>   static const char *pnv_phb4_root_bus_path(PCIHostState *host_bridge,
> 

Re: [PATCH 1/3] ppc/pnv: Tune the POWER9 PCIe Host bridge model

[Cédric Le Goater]

On 11/16/21 18:01, Frederic Barrat wrote:
> The PHB v4 found on POWER9 doesn't request any LSI, so let's clear the
> Interrupt Pin register in the config space so that the model matches
> the hardware.
> 
> If we don't, then we inherit from the default pcie root bridge, which
> requests a LSI. And because we don't map it correctly in the device
> tree, all PHBs allocate the same bogus hw interrupt. We end up with
> inconsistent interrupt controller (xive) data. The problem goes away
> if we don't allocate the LSI in the first place.
> 
> Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
> ---
>   hw/pci-host/pnv_phb4.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
> index 5c375a9f28..1659d55b4f 100644
> --- a/hw/pci-host/pnv_phb4.c
> +++ b/hw/pci-host/pnv_phb4.c
> @@ -1234,10 +1234,13 @@ static void pnv_phb4_reset(DeviceState *dev)
>       PCIDevice *root_dev = PCI_DEVICE(&phb->root);
>   
>       /*
> -     * Configure PCI device id at reset using a property.
> +     * Configure the PCI device at reset:
> +     *   - set the Vendor and Device ID to for the root bridge
> +     *   - no LSI
>        */
>       pci_config_set_vendor_id(root_dev->config, PCI_VENDOR_ID_IBM);
>       pci_config_set_device_id(root_dev->config, phb->device_id);
> +    pci_config_set_interrupt_pin(root_dev->config, 0);
>   }
>   
>   static const char *pnv_phb4_root_bus_path(PCIHostState *host_bridge,
> 

FYI, I am seeing an issue with FreeBSD when booting from iso :

   https://download.freebsd.org/ftp/snapshots/powerpc/powerpc64/ISO-IMAGES/14.0/FreeBSD-14.0-CURRENT-powerpc-powerpc64-20211028-4827bf76bce-250301-disc1.iso.xz

Thanks,

C.

SIGTERM received, booting...
KDB: debugger backends: ddb
KDB: current backend: ddb
---<<BOOT>>---
Copyright (c) 1992-2021 The FreeBSD Project.
Copyright (c) 1979, 1980, 1983, 1986, 1988, 1989, 1991, 1992, 1993, 1994
	The Regents of the University of California. All rights reserved.
FreeBSD is a registered trademark of The FreeBSD Foundation.
FreeBSD 14.0-CURRENT #0 main-n250301-4827bf76bce: Thu Oct 28 06:53:58 UTC 2021
     root@releng1.nyi.freebsd.org:/usr/obj/usr/src/powerpc.powerpc64/sys/GENERIC64 powerpc
FreeBSD clang version 12.0.1 (git@github.com:llvm/llvm-project.git llvmorg-12.0.1-0-gfed41342a82f)
WARNING: WITNESS option enabled, expect reduced performance.
VT: init without driver.
ofw_initrd: initrd loaded at 0x28000000-0x28c7928c
cpu0: IBM POWER9 revision 2.0, 1000.00 MHz
cpu0: Features dc007182<PPC32,PPC64,ALTIVEC,FPU,MMU,SMT,ISNOOP,ARCH205,ARCH206,VSX,TRUELE>
cpu0: Features2 bee00000<ARCH207,DSCR,ISEL,TAR,VCRYPTO,ARCH300,IEEE128,DARN>
real memory  = 1014484992 (967 MB)
avail memory = 117903360 (112 MB)
random: registering fast source PowerISA DARN random number generator
random: fast provider: "PowerISA DARN random number generator"
arc4random: WARNING: initial seeding bypassed the cryptographic random device because it was not yet seeded and the knob 'bypass_before_seeding' was enabled.
random: entropy device external interface
kbd0 at kbdmux0
ofwbus0: <Open Firmware Device Tree> on nexus0
opal0: <OPAL Abstraction Firmware> irq 1048560,1048561,1048562,1048563,1048564,1048565,1048566,1048567,1048568,1048569,1048570,1048571,1048572,1048573 on ofwbus0
opal0: registered as a time-of-day clock, resolution 0.002000s
simplebus0: <Flattened device tree simple bus> mem 0x6030000000000-0x60300ffffffff on ofwbus0
pcib0: <OPAL Host-PCI bridge> mem 0x600c3c0000000-0x600c3c0000fff,0x600c300000000-0x600c30fffffff on ofwbus0
pci0: <OFW PCI bus> numa-domain 0 on pcib0
qemu-system-ppc64: ../hw/pci/pci.c:1487: pci_irq_handler: Assertion `0 <= irq_num && irq_num < PCI_NUM_PINS' failed.

Re: [PATCH 1/3] ppc/pnv: Tune the POWER9 PCIe Host bridge model

[Cédric Le Goater]

[ Adding Alfredo the thread ]

On 11/26/21 10:09, Cédric Le Goater wrote:
> On 11/16/21 18:01, Frederic Barrat wrote:
>> The PHB v4 found on POWER9 doesn't request any LSI, so let's clear the
>> Interrupt Pin register in the config space so that the model matches
>> the hardware.
>>
>> If we don't, then we inherit from the default pcie root bridge, which
>> requests a LSI. And because we don't map it correctly in the device
>> tree, all PHBs allocate the same bogus hw interrupt. We end up with
>> inconsistent interrupt controller (xive) data. The problem goes away
>> if we don't allocate the LSI in the first place.
>>
>> Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
>> ---
>>   hw/pci-host/pnv_phb4.c | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
>> index 5c375a9f28..1659d55b4f 100644
>> --- a/hw/pci-host/pnv_phb4.c
>> +++ b/hw/pci-host/pnv_phb4.c
>> @@ -1234,10 +1234,13 @@ static void pnv_phb4_reset(DeviceState *dev)
>>       PCIDevice *root_dev = PCI_DEVICE(&phb->root);
>>       /*
>> -     * Configure PCI device id at reset using a property.
>> +     * Configure the PCI device at reset:
>> +     *   - set the Vendor and Device ID to for the root bridge
>> +     *   - no LSI
>>        */
>>       pci_config_set_vendor_id(root_dev->config, PCI_VENDOR_ID_IBM);
>>       pci_config_set_device_id(root_dev->config, phb->device_id);
>> +    pci_config_set_interrupt_pin(root_dev->config, 0);
>>   }
>>   static const char *pnv_phb4_root_bus_path(PCIHostState *host_bridge,
>>
> 
> FYI, I am seeing an issue with FreeBSD when booting from iso :
> 
>    https://download.freebsd.org/ftp/snapshots/powerpc/powerpc64/ISO-IMAGES/14.0/FreeBSD-14.0-CURRENT-powerpc-powerpc64-20211028-4827bf76bce-250301-disc1.iso.xz
> 
> Thanks,
> 
> C.
> 
> SIGTERM received, booting...
> KDB: debugger backends: ddb
> KDB: current backend: ddb
> ---<<BOOT>>---
> Copyright (c) 1992-2021 The FreeBSD Project.
> Copyright (c) 1979, 1980, 1983, 1986, 1988, 1989, 1991, 1992, 1993, 1994
>      The Regents of the University of California. All rights reserved.
> FreeBSD is a registered trademark of The FreeBSD Foundation.
> FreeBSD 14.0-CURRENT #0 main-n250301-4827bf76bce: Thu Oct 28 06:53:58 UTC 2021
>      root@releng1.nyi.freebsd.org:/usr/obj/usr/src/powerpc.powerpc64/sys/GENERIC64 powerpc
> FreeBSD clang version 12.0.1 (git@github.com:llvm/llvm-project.git llvmorg-12.0.1-0-gfed41342a82f)
> WARNING: WITNESS option enabled, expect reduced performance.
> VT: init without driver.
> ofw_initrd: initrd loaded at 0x28000000-0x28c7928c
> cpu0: IBM POWER9 revision 2.0, 1000.00 MHz
> cpu0: Features dc007182<PPC32,PPC64,ALTIVEC,FPU,MMU,SMT,ISNOOP,ARCH205,ARCH206,VSX,TRUELE>
> cpu0: Features2 bee00000<ARCH207,DSCR,ISEL,TAR,VCRYPTO,ARCH300,IEEE128,DARN>
> real memory  = 1014484992 (967 MB)
> avail memory = 117903360 (112 MB)
> random: registering fast source PowerISA DARN random number generator
> random: fast provider: "PowerISA DARN random number generator"
> arc4random: WARNING: initial seeding bypassed the cryptographic random device because it was not yet seeded and the knob 'bypass_before_seeding' was enabled.
> random: entropy device external interface
> kbd0 at kbdmux0
> ofwbus0: <Open Firmware Device Tree> on nexus0
> opal0: <OPAL Abstraction Firmware> irq 1048560,1048561,1048562,1048563,1048564,1048565,1048566,1048567,1048568,1048569,1048570,1048571,1048572,1048573 on ofwbus0
> opal0: registered as a time-of-day clock, resolution 0.002000s
> simplebus0: <Flattened device tree simple bus> mem 0x6030000000000-0x60300ffffffff on ofwbus0
> pcib0: <OPAL Host-PCI bridge> mem 0x600c3c0000000-0x600c3c0000fff,0x600c300000000-0x600c30fffffff on ofwbus0
> pci0: <OFW PCI bus> numa-domain 0 on pcib0
> qemu-system-ppc64: ../hw/pci/pci.c:1487: pci_irq_handler: Assertion `0 <= irq_num && irq_num < PCI_NUM_PINS' failed.
> 
> 

Re: [PATCH 1/3] ppc/pnv: Tune the POWER9 PCIe Host bridge model

[Michael S. Tsirkin]

On Fri, Nov 26, 2021 at 06:08:30PM +0100, Cédric Le Goater wrote:
> [ Adding Alfredo the thread ]
> 
> On 11/26/21 10:09, Cédric Le Goater wrote:
> > On 11/16/21 18:01, Frederic Barrat wrote:
> > > The PHB v4 found on POWER9 doesn't request any LSI, so let's clear the
> > > Interrupt Pin register in the config space so that the model matches
> > > the hardware.
> > > 
> > > If we don't, then we inherit from the default pcie root bridge, which
> > > requests a LSI. And because we don't map it correctly in the device
> > > tree, all PHBs allocate the same bogus hw interrupt. We end up with
> > > inconsistent interrupt controller (xive) data. The problem goes away
> > > if we don't allocate the LSI in the first place.
> > > 
> > > Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
> > > ---
> > >   hw/pci-host/pnv_phb4.c | 5 ++++-
> > >   1 file changed, 4 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
> > > index 5c375a9f28..1659d55b4f 100644
> > > --- a/hw/pci-host/pnv_phb4.c
> > > +++ b/hw/pci-host/pnv_phb4.c
> > > @@ -1234,10 +1234,13 @@ static void pnv_phb4_reset(DeviceState *dev)
> > >       PCIDevice *root_dev = PCI_DEVICE(&phb->root);
> > >       /*
> > > -     * Configure PCI device id at reset using a property.
> > > +     * Configure the PCI device at reset:
> > > +     *   - set the Vendor and Device ID to for the root bridge
> > > +     *   - no LSI
> > >        */
> > >       pci_config_set_vendor_id(root_dev->config, PCI_VENDOR_ID_IBM);
> > >       pci_config_set_device_id(root_dev->config, phb->device_id);
> > > +    pci_config_set_interrupt_pin(root_dev->config, 0);
> > >   }
> > >   static const char *pnv_phb4_root_bus_path(PCIHostState *host_bridge,
> > > 
> > 
> > FYI, I am seeing an issue with FreeBSD when booting from iso :
> > 
> >    https://download.freebsd.org/ftp/snapshots/powerpc/powerpc64/ISO-IMAGES/14.0/FreeBSD-14.0-CURRENT-powerpc-powerpc64-20211028-4827bf76bce-250301-disc1.iso.xz
> > 
> > Thanks,
> > 
> > C.
> > 
> > SIGTERM received, booting...
> > KDB: debugger backends: ddb
> > KDB: current backend: ddb
> > ---<<BOOT>>---
> > Copyright (c) 1992-2021 The FreeBSD Project.
> > Copyright (c) 1979, 1980, 1983, 1986, 1988, 1989, 1991, 1992, 1993, 1994
> >      The Regents of the University of California. All rights reserved.
> > FreeBSD is a registered trademark of The FreeBSD Foundation.
> > FreeBSD 14.0-CURRENT #0 main-n250301-4827bf76bce: Thu Oct 28 06:53:58 UTC 2021
> >      root@releng1.nyi.freebsd.org:/usr/obj/usr/src/powerpc.powerpc64/sys/GENERIC64 powerpc
> > FreeBSD clang version 12.0.1 (git@github.com:llvm/llvm-project.git llvmorg-12.0.1-0-gfed41342a82f)
> > WARNING: WITNESS option enabled, expect reduced performance.
> > VT: init without driver.
> > ofw_initrd: initrd loaded at 0x28000000-0x28c7928c
> > cpu0: IBM POWER9 revision 2.0, 1000.00 MHz
> > cpu0: Features dc007182<PPC32,PPC64,ALTIVEC,FPU,MMU,SMT,ISNOOP,ARCH205,ARCH206,VSX,TRUELE>
> > cpu0: Features2 bee00000<ARCH207,DSCR,ISEL,TAR,VCRYPTO,ARCH300,IEEE128,DARN>
> > real memory  = 1014484992 (967 MB)
> > avail memory = 117903360 (112 MB)
> > random: registering fast source PowerISA DARN random number generator
> > random: fast provider: "PowerISA DARN random number generator"
> > arc4random: WARNING: initial seeding bypassed the cryptographic random device because it was not yet seeded and the knob 'bypass_before_seeding' was enabled.
> > random: entropy device external interface
> > kbd0 at kbdmux0
> > ofwbus0: <Open Firmware Device Tree> on nexus0
> > opal0: <OPAL Abstraction Firmware> irq 1048560,1048561,1048562,1048563,1048564,1048565,1048566,1048567,1048568,1048569,1048570,1048571,1048572,1048573 on ofwbus0
> > opal0: registered as a time-of-day clock, resolution 0.002000s
> > simplebus0: <Flattened device tree simple bus> mem 0x6030000000000-0x60300ffffffff on ofwbus0
> > pcib0: <OPAL Host-PCI bridge> mem 0x600c3c0000000-0x600c3c0000fff,0x600c300000000-0x600c30fffffff on ofwbus0
> > pci0: <OFW PCI bus> numa-domain 0 on pcib0
> > qemu-system-ppc64: ../hw/pci/pci.c:1487: pci_irq_handler: Assertion `0 <= irq_num && irq_num < PCI_NUM_PINS' failed.
> > 


Frederic?

-- 
MST

Re: [PATCH 1/3] ppc/pnv: Tune the POWER9 PCIe Host bridge model

[Frederic Barrat]

On 26/11/2021 10:09, Cédric Le Goater wrote:
> On 11/16/21 18:01, Frederic Barrat wrote:
>> The PHB v4 found on POWER9 doesn't request any LSI, so let's clear the
>> Interrupt Pin register in the config space so that the model matches
>> the hardware.
>>
>> If we don't, then we inherit from the default pcie root bridge, which
>> requests a LSI. And because we don't map it correctly in the device
>> tree, all PHBs allocate the same bogus hw interrupt. We end up with
>> inconsistent interrupt controller (xive) data. The problem goes away
>> if we don't allocate the LSI in the first place.
>>
>> Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
>> ---
>>   hw/pci-host/pnv_phb4.c | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
>> index 5c375a9f28..1659d55b4f 100644
>> --- a/hw/pci-host/pnv_phb4.c
>> +++ b/hw/pci-host/pnv_phb4.c
>> @@ -1234,10 +1234,13 @@ static void pnv_phb4_reset(DeviceState *dev)
>>       PCIDevice *root_dev = PCI_DEVICE(&phb->root);
>>       /*
>> -     * Configure PCI device id at reset using a property.
>> +     * Configure the PCI device at reset:
>> +     *   - set the Vendor and Device ID to for the root bridge
>> +     *   - no LSI
>>        */
>>       pci_config_set_vendor_id(root_dev->config, PCI_VENDOR_ID_IBM);
>>       pci_config_set_device_id(root_dev->config, phb->device_id);
>> +    pci_config_set_interrupt_pin(root_dev->config, 0);
>>   }
>>   static const char *pnv_phb4_root_bus_path(PCIHostState *host_bridge,
>>
> 
> FYI, I am seeing an issue with FreeBSD when booting from iso :
> 
>    
> https://download.freebsd.org/ftp/snapshots/powerpc/powerpc64/ISO-IMAGES/14.0/FreeBSD-14.0-CURRENT-powerpc-powerpc64-20211028-4827bf76bce-250301-disc1.iso.xz 
> 
> 


I see what's going on... Since the phb4 model borrows most of its code 
from the pcie_root bridge, there are several instances of code such as:

     if (msix_enabled(dev)) {
         do something;
     } else if (msi_enabled(dev)) {
         do something else;
     } else {
         yet something else which assumes a LSI;
     }

With this series, I removed the LSI from the phb4 root port to match the 
hardware and fixed one such code pattern in patch 3. But there are 
others, and we hit one of those when installing from the free bsd iso.

So this is going to need more work.

   Fred



> Thanks,
> 
> C.
> 
> SIGTERM received, booting...
> KDB: debugger backends: ddb
> KDB: current backend: ddb
> ---<<BOOT>>---
> Copyright (c) 1992-2021 The FreeBSD Project.
> Copyright (c) 1979, 1980, 1983, 1986, 1988, 1989, 1991, 1992, 1993, 1994
>      The Regents of the University of California. All rights reserved.
> FreeBSD is a registered trademark of The FreeBSD Foundation.
> FreeBSD 14.0-CURRENT #0 main-n250301-4827bf76bce: Thu Oct 28 06:53:58 
> UTC 2021
>      
> root@releng1.nyi.freebsd.org:/usr/obj/usr/src/powerpc.powerpc64/sys/GENERIC64 
> powerpc
> FreeBSD clang version 12.0.1 (git@github.com:llvm/llvm-project.git 
> llvmorg-12.0.1-0-gfed41342a82f)
> WARNING: WITNESS option enabled, expect reduced performance.
> VT: init without driver.
> ofw_initrd: initrd loaded at 0x28000000-0x28c7928c
> cpu0: IBM POWER9 revision 2.0, 1000.00 MHz
> cpu0: Features 
> dc007182<PPC32,PPC64,ALTIVEC,FPU,MMU,SMT,ISNOOP,ARCH205,ARCH206,VSX,TRUELE>
> cpu0: Features2 
> bee00000<ARCH207,DSCR,ISEL,TAR,VCRYPTO,ARCH300,IEEE128,DARN>
> real memory  = 1014484992 (967 MB)
> avail memory = 117903360 (112 MB)
> random: registering fast source PowerISA DARN random number generator
> random: fast provider: "PowerISA DARN random number generator"
> arc4random: WARNING: initial seeding bypassed the cryptographic random 
> device because it was not yet seeded and the knob 
> 'bypass_before_seeding' was enabled.
> random: entropy device external interface
> kbd0 at kbdmux0
> ofwbus0: <Open Firmware Device Tree> on nexus0
> opal0: <OPAL Abstraction Firmware> irq 
> 1048560,1048561,1048562,1048563,1048564,1048565,1048566,1048567,1048568,1048569,1048570,1048571,1048572,1048573 
> on ofwbus0
> opal0: registered as a time-of-day clock, resolution 0.002000s
> simplebus0: <Flattened device tree simple bus> mem 
> 0x6030000000000-0x60300ffffffff on ofwbus0
> pcib0: <OPAL Host-PCI bridge> mem 
> 0x600c3c0000000-0x600c3c0000fff,0x600c300000000-0x600c30fffffff on ofwbus0
> pci0: <OFW PCI bus> numa-domain 0 on pcib0
> qemu-system-ppc64: ../hw/pci/pci.c:1487: pci_irq_handler: Assertion `0 
> <= irq_num && irq_num < PCI_NUM_PINS' failed.
> 
> 
> 

[PATCH 2/3] pci: Export the pci_intx() function

[Frederic Barrat]

Move the pci_intx() definition to the PCI header file, so that it can
be called from other PCI files. It is used by the next patch.

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

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index e5993c1ef5..249d7e4cf6 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -1497,11 +1497,6 @@ static void pci_irq_handler(void *opaque, int irq_num, int level)
     pci_change_irq_level(pci_dev, irq_num, change);
 }
 
-static inline int pci_intx(PCIDevice *pci_dev)
-{
-    return pci_get_byte(pci_dev->config + PCI_INTERRUPT_PIN) - 1;
-}
-
 qemu_irq pci_allocate_irq(PCIDevice *pci_dev)
 {
     int intx = pci_intx(pci_dev);
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index e7cdf2d5ec..35f8eb67bd 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -735,6 +735,11 @@ void lsi53c8xx_handle_legacy_cmdline(DeviceState *lsi_dev);
 qemu_irq pci_allocate_irq(PCIDevice *pci_dev);
 void pci_set_irq(PCIDevice *pci_dev, int level);
 
+static inline int pci_intx(PCIDevice *pci_dev)
+{
+    return pci_get_byte(pci_dev->config + PCI_INTERRUPT_PIN) - 1;
+}
+
 static inline void pci_irq_assert(PCIDevice *pci_dev)
 {
     pci_set_irq(pci_dev, 1);
-- 
2.33.1

Re: [PATCH 2/3] pci: Export the pci_intx() function

[Cédric Le Goater]

On 11/16/21 18:01, Frederic Barrat wrote:
> Move the pci_intx() definition to the PCI header file, so that it can
> be called from other PCI files. It is used by the next patch.
> 
> Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>


Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.


> ---
>   hw/pci/pci.c         | 5 -----
>   include/hw/pci/pci.h | 5 +++++
>   2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index e5993c1ef5..249d7e4cf6 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -1497,11 +1497,6 @@ static void pci_irq_handler(void *opaque, int irq_num, int level)
>       pci_change_irq_level(pci_dev, irq_num, change);
>   }
>   
> -static inline int pci_intx(PCIDevice *pci_dev)
> -{
> -    return pci_get_byte(pci_dev->config + PCI_INTERRUPT_PIN) - 1;
> -}
> -
>   qemu_irq pci_allocate_irq(PCIDevice *pci_dev)
>   {
>       int intx = pci_intx(pci_dev);
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index e7cdf2d5ec..35f8eb67bd 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -735,6 +735,11 @@ void lsi53c8xx_handle_legacy_cmdline(DeviceState *lsi_dev);
>   qemu_irq pci_allocate_irq(PCIDevice *pci_dev);
>   void pci_set_irq(PCIDevice *pci_dev, int level);
>   
> +static inline int pci_intx(PCIDevice *pci_dev)
> +{
> +    return pci_get_byte(pci_dev->config + PCI_INTERRUPT_PIN) - 1;
> +}
> +
>   static inline void pci_irq_assert(PCIDevice *pci_dev)
>   {
>       pci_set_irq(pci_dev, 1);
> 

[PATCH 3/3] pcie_aer: Don't trigger a LSI if none are defined

[Frederic Barrat]

Skip triggering an LSI when the AER root error status is updated if no
LSI is defined for the device. We can have a root bridge with no LSI,
MSI and MSI-X defined, for example on POWER systems.

Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
---
 hw/pci/pcie_aer.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c
index 27f9cc56af..e1a8a88c8c 100644
--- a/hw/pci/pcie_aer.c
+++ b/hw/pci/pcie_aer.c
@@ -774,7 +774,9 @@ void pcie_aer_root_write_config(PCIDevice *dev,
     uint32_t root_cmd = pci_get_long(aer_cap + PCI_ERR_ROOT_COMMAND);
     /* 6.2.4.1.2 Interrupt Generation */
     if (!msix_enabled(dev) && !msi_enabled(dev)) {
-        pci_set_irq(dev, !!(root_cmd & enabled_cmd));
+        if (pci_intx(dev) != -1) {
+            pci_set_irq(dev, !!(root_cmd & enabled_cmd));
+        }
         return;
     }
 
-- 
2.33.1

Re: [PATCH 3/3] pcie_aer: Don't trigger a LSI if none are defined

[Cédric Le Goater]

On 11/16/21 18:01, Frederic Barrat wrote:
> Skip triggering an LSI when the AER root error status is updated if no
> LSI is defined for the device. We can have a root bridge with no LSI,
> MSI and MSI-X defined, for example on POWER systems.
> 
> Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
> ---


Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.



>   hw/pci/pcie_aer.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c
> index 27f9cc56af..e1a8a88c8c 100644
> --- a/hw/pci/pcie_aer.c
> +++ b/hw/pci/pcie_aer.c
> @@ -774,7 +774,9 @@ void pcie_aer_root_write_config(PCIDevice *dev,
>       uint32_t root_cmd = pci_get_long(aer_cap + PCI_ERR_ROOT_COMMAND);
>       /* 6.2.4.1.2 Interrupt Generation */
>       if (!msix_enabled(dev) && !msi_enabled(dev)) {
> -        pci_set_irq(dev, !!(root_cmd & enabled_cmd));
> +        if (pci_intx(dev) != -1) {
> +            pci_set_irq(dev, !!(root_cmd & enabled_cmd));
> +        }
>           return;
>       }
>   
>