Re: [PATCH 7/7] PCI/AER: Enable baseline capability error reporting

[Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>]

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