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

[Shuai Xue <xueshuai@linux.alibaba.com>]

在 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