Re: [PATCH v2 1/6] PCI/DPC: Avoid access to non-existent AER capability

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

Hi,

On 11/3/2026 11:47 AM, Yazen Ghannam wrote:
> On Tue, Oct 20, 2026 at 11:19:18AM -0700, Kuppuswamy Sathyanarayanan wrote:
>> From: Lukas Wunner <lukas@wunner.de>
>>
>> Downstream Port Containment does not mandate presence of an Advanced Error
>> Reporting capability, so a Downstream Port may support DPC, but not AER
>> (PCIe r7.1 sec 6.2.11.2).
>>
>> In February 2019, commit 9f08a5d896ce ("PCI/DPC: Fix print AER status in
>> DPC event handling") amended the DPC driver to access the AER capability
>> without checking for its presence.
>>
>> In May 2020, commit 708b20003624 ("PCI/AER: Remove HEST/FIRMWARE_FIRST
>> parsing for AER ownership") fixed it by inserting a call to
>> pcie_aer_is_native() in dpc_probe(), which implicitly checks for presence
>> of an AER capability.
>>
>> However already in October 2019, commit 35a0b2378c19 ("PCI/DPC: Add
>> "pcie_ports=dpc-native" to allow DPC without AER control") made it
>> possible to override the check:  The DPC driver may access a non-existent
>> AER capability if "pcie_ports=dpc-native" is passed on the command line.
>>
>> Fix it by making the DPC driver cope with AER-unsupporting Downstream
>> Ports.
>>
>> There are two places where the AER capability is accessed:
>>
>> - dpc_get_aer_uncorrect_severity() uses it to discern whether a Fatal or
>>   Non-Fatal Error triggered DPC.  Access the Device Status Register
>>   instead, in accordance with PCIe r7.1 sec 6.2.5.
>>
>> - dpc_is_surprise_removal() uses it to detect whether a Surprise Down
>>   Error triggered DPC.  Return false on AER-unsupporting devices.  The
>>   function works around an AMD-specific quirk and it seems reasonable to
>>   assume that all affected products are AER-supporting.  In any case the
>>   detection is not possible without AER capability.
>>
>> Insert a temporary check for an AER capability after the call to
>> aer_get_device_error_info() because the function currently returns false
>> for AER-unsupporting devices.  The check will become obsolete and will be
>> removed with the imminent baseline capability error reporting.
>>
>> Fixes: 35a0b2378c19 ("PCI/DPC: Add "pcie_ports=dpc-native" to allow DPC without AER control")
>> Signed-off-by: Lukas Wunner <lukas@wunner.de>
>> Cc: stable@vger.kernel.org # v5.5+
>> ---
>>  drivers/pci/pcie/dpc.c | 23 ++++++++++-------------
>>  1 file changed, 10 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c
>> index 2b779bd1d861..793a799053f1 100644
>> --- a/drivers/pci/pcie/dpc.c
>> +++ b/drivers/pci/pcie/dpc.c
>> @@ -236,21 +236,15 @@ static void dpc_process_rp_pio_error(struct pci_dev *pdev)
>>  static int dpc_get_aer_uncorrect_severity(struct pci_dev *dev,
>>  					  struct aer_err_info *info)
>>  {
>> -	int pos = dev->aer_cap;
>> -	u32 status, mask, sev;
>> +	u16 devsta;
>>  
>> -	pci_read_config_dword(dev, pos + PCI_ERR_UNCOR_STATUS, &status);
>> -	pci_read_config_dword(dev, pos + PCI_ERR_UNCOR_MASK, &mask);
>> -	status &= ~mask;
>> -	if (!status)
>> -		return 0;
>> -
>> -	pci_read_config_dword(dev, pos + PCI_ERR_UNCOR_SEVER, &sev);
>> -	status &= sev;
>> -	if (status)
>> +	pcie_capability_read_word(dev, PCI_EXP_DEVSTA, &devsta);
>> +	if (devsta & PCI_EXP_DEVSTA_FED)
>>  		info->severity = AER_FATAL;
>> -	else
>> +	else if (devsta & PCI_EXP_DEVSTA_NFED)
>>  		info->severity = AER_NONFATAL;
>> +	else
>> +		return 0;
>>  
>>  	info->level = KERN_ERR;
> 
> Did you settle your question from here?
> 
> https://lore.kernel.org/all/74fe0d79-dc90-4c05-ba9c-9a0eec1ada48@linux.intel.com/

No, not yet.  Lukas hasn't replied, and v2 carries his patch unchanged.

The concern still stands.  For a port with an AER capability, the
Fatal/Non-Fatal Error Detected bits in Device Status are set
regardless of the Uncorrectable Error Mask (PCIe r7.1 sec 7.5.3.5).
So a masked Fatal error next to the unmasked Non-Fatal error that
triggered DPC is now reported as Fatal.

If Lukas agrees, I can fix it in v3 by keeping the AER-based logic
when dev->aer_cap is present and using Device Status only for ports
without AER.

> 
> Thanks,
> Yazen

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer