Re: [PATCH v6 4/5] PCI/ERR: Use pcie_aer_is_native() to check for native AER control

Shuai Xue
2025-10-24T03:09:36Z

在 2025/10/23 18:29, Lukas Wunner 写道:
> On Mon, Oct 20, 2025 at 10:45:31PM +0800, Shuai Xue wrote:
>> if (host->native_aer || pcie_ports_native) {
>> pcie_clear_device_status(bridge);
>> pci_aer_clear_nonfatal_status(bridge);
>> }
>>
>> This code clears both the PCIe Device Status register and AER status
>> registers when in native AER mode.
>>
>> pcie_clear_device_status() is renamed from
>> pci_aer_clear_device_status(). Does it intends to clear only AER error
>> status?
>>
>> - BIT 0: Correctable Error Detected
>> - BIT 1: Non-Fatal Error Detected
>> - BIT 2: Fatal Error Detected
>> - BIT 3: Unsupported Request Detected
>>
>> From PCIe spec, BIT 0-2 are logged for functions supporting Advanced
>> Error Handling.
>>
>> I am not sure if we should clear BIT 3, and also BIT 6 (Emergency Power
>> Reduction Detected) and in case a AER error.
>
> AFAIUI, bits 0 to 3 are what the PCIe r7.0 sec 6.2.1 calls
> "baseline capability" error reporting. They're supported
> even if AER is not supported.
>
> Bit 6 has nothing to do with this AFAICS.

Hi, Lukas,

Per PCIe r7.0 section 7.5.3.5:

**For Functions supporting Advanced Error Handling**, errors are logged
in this register regardless of the settings of the Uncorrectable Error
Mask register. Default value of this bit is 0b.

From this, it's clear that bits 0 to 2 are not logged unless AER is supported.

So, if dev->aer_cap is not true, there’s no need to clear bits 0 to 2.
This validates the dev->aer_cap sanity check in pcie_aer_is_native():

int pcie_aer_is_native(struct pci_dev *dev)
{
struct pci_host_bridge *host = pci_find_host_bridge(dev->bus);

if (!dev->aer_cap)
return 0;

return pcie_ports_native || host->native_aer;
}
EXPORT_SYMBOL_NS_GPL(pcie_aer_is_native, "CXL");

Based on this, the introduction of pcie_aer_is_native() in the patch
seems reasonable and consistent with the PCIe specification.

Further, should we rename pcie_clear_device_status() back to
pci_aer_clear_device_status():

-void pcie_clear_device_status(struct pci_dev *dev)
+void pci_aer_clear_device_status(struct pci_dev *dev)
{
u16 sta;

pcie_capability_read_word(dev, PCI_EXP_DEVSTA, &sta);
- pcie_capability_write_word(dev, PCI_EXP_DEVSTA, sta);
+ /* Bits 0-2 are logged if AER is supported */
+ pcie_capability_write_word(dev, PCI_EXP_DEVSTA, sta & 0x7);
}

I am still uncertain whether bit 3 ("Unsupported Request Detected")
should be cleared in this function. It’s not directly tied to AER
capability.

I’d love to hear your thoughts, as well as @Bjorn’s, on both the renaming
suggestion and whether bit 3 should be cleared alongside bits 0 to 2.

Thanks.
Shuai