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

[Shuai Xue]

在 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