Hi Lukas, On 9/30/2026 12:08 AM, Lukas Wunner wrote: > On Tue, Sep 29, 2026 at 02:01:42PM -0700, Kuppuswamy Sathyanarayanan wrote: >> On 9/27/2026 11:02 AM, Lukas Wunner wrote: >>> @@ -431,6 +432,9 @@ void pci_aer_init(struct pci_dev *dev) >>> PCI_ERR_COR_ADV_NFAT, 0); >>> >>> pci_aer_clear_status(dev); >>> +enable: >>> + if (pcie_aer_is_native(dev)) >>> + pcie_clear_device_status(dev); >>> >>> if (pci_aer_available()) >>> pci_enable_pcie_error_reporting(dev); >> >> This also enables error reporting below AER-incapable Root Ports, where >> no AER service handles the ERR_* Messages. The Root Control System >> Error enable bits are only cleared by aer_enable_rootport(), which >> doesn't run on such ports. If firmware left them set, the newly >> enabled Messages could result in System Errors. >> >> Should reporting be enabled only if an AER service (or DPC) is above >> the device? > > Excellent observation. This is a pre-existing issue but I think you're > right. However it's non-trivial to fix because just checking for DPC > capability in the ancestry or AER capability at the Root Port isn't > sufficient: > > For RCiEPs, we'd need to check whether an RCEC exists which has > AER capability. There's an "rcec" pointer in struct pci_dev which > allows discovering the RCEC responsible for an RCiEP. But the pointer > is only set when portdrv binds to the RCEC (pcie_link_rcec()). > That's much later than when the RCiEP and its capabilities are enumerated. > > When enumerating an RCiEP, we'd need to walk the entire set of PCI devices, > check if it's an RCEC, check if it's responsible for this RCiEP and assign > the rcec pointer. We could try to avoid that by running pcie_link_rcec() > already on enumeration of the RCEC (and not on probing of portdrv), > but the RCEC may be enumerated after the RCiEP. User space could also > force an unset rcec pointer by issuing remove/rescan of the RCiEP. > > Also, right now when firmware does keep System Error Enable bits in the > Root Control register set, there's a window between endpoints being > enumerated (which enables sending of ERR_* messages) and Root Ports > being bound to portdrv (which clears System Error Enable bits). >
Agreed, the RCiEP case makes this hard, and the window already exists. Could you add a sentence to the commit message noting that reporting is now also enabled on AER-incapable devices below AER-incapable Root Ports? That way, if someone bisects a new System Error to this commit, the reason is obvious. > Any errors that occur during that window will cause a System Error > right now. > >> Or alternatively, clear the Root Control System Error >> enable bits on AER-incapable Root Ports? > > I'm worried that users may deliberately enable System Error bits in > BIOS on such systems precisely because there's no other way to catch > them. > Fair point, let's leave them alone. > Thanks for the thoughtful review, much appreciated! > > Lukas -- Sathyanarayanan Kuppuswamy Linux Kernel Developer
