[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1eaf1f94-e26b-4313-b6b7-51ad966fe28e@linux.alibaba.com>
Date: Fri, 24 Oct 2025 11:09:25 +0800
From: Shuai Xue <xueshuai@...ux.alibaba.com>
To: Lukas Wunner <lukas@...ner.de>, Bjorn Helgaas <bhelgaas@...gle.com>
Cc: linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org,
linuxppc-dev@...ts.ozlabs.org, kbusch@...nel.org,
sathyanarayanan.kuppuswamy@...ux.intel.com, mahesh@...ux.ibm.com,
oohall@...il.com, Jonathan.Cameron@...wei.com, terry.bowman@....com,
tianruidong@...ux.alibaba.com
Subject: Re: [PATCH v6 4/5] PCI/ERR: Use pcie_aer_is_native() to check for
native AER control
在 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
Powered by blists - more mailing lists