On 2/3/26 4:06 PM, Lukas Wunner wrote:
On Sat, Jan 24, 2026 at 03:45:56PM +0800, Shuai Xue wrote:
The DPC driver clears AER fatal status for the port that reported the
error, but not for the downstream device that deteced the error. The
current recovery code only clears non-fatal AER status, leaving fatal
status bits set in the error device.
That's not quite accurate:
The error device has undergone a Hot Reset as a result of the Link Down
event. To be able to use it again, pci_restore_state() is invoked by
the driver's ->slot_reset() callback. And pci_restore_state() does
clear fatal status bits.
pci_restore_state()
pci_aer_clear_status()
pci_aer_raw_clear_status()
Use pci_aer_raw_clear_status() to clear both fatal and non-fatal error
status in the error device, ensuring all AER status bits are properly
cleared after recovery.
Well, pci_restore_state() already clears all AER status bits so why
is this patch necessary?
You're right that many drivers call pci_restore_state() in their
->slot_reset() callback, which clears all AER status bits. However,
since ->slot_reset() is driver-defined and not all drivers invoke
pci_restore_state(), there could be cases where fatal AER status bits
remain set after the frozen recovery completes.
+++ b/drivers/pci/pcie/err.c
@@ -285,7 +285,7 @@ pci_ers_result_t pcie_do_recovery(struct pci_dev *dev,
*/
if (host->native_aer || pcie_ports_native) {
pcie_clear_device_status(dev);
- pci_aer_clear_nonfatal_status(dev);
+ pci_aer_raw_clear_status(dev);
}
This code path is for the case when pcie_do_recovery() is called
with state=pci_channel_io_normal, i.e. in the nonfatal case.
That's why only the nonfatal bits need to be cleared here.
In the fatal case clearing of the error bits is done by
pci_restore_state().
I understand that this is subtle and should probably be changed
to improve clarity, but this patch doesn't look like a step
in that direction.
I notice that pcie_clear_device_status()
(called just before pci_aer_clear_nonfatal_status()) already clears
*all* error bits in the Device Status register (CED, NFED, FED, URD),
including the Fatal Error Detected (FED) bit, regardless of whether
we're in a fatal or nonfatal recovery path.
So there's an inconsistency in the current design:
- pcie_clear_device_status() clears all error bits (including FED)
- pci_aer_clear_nonfatal_status() only clears nonfatal AER status
This means that even in the nonfatal case, the FED bit in Device
Status is cleared, even though we preserve the fatal bits in the AER
Uncorrectable Error Status register.
Would you prefer that I:
1. Make both pcie_clear_device_status() and the AER clearing
conditional on the 'state' parameter, so that nonfatal recovery
truly preserves fatal bits in both registers, or
2. Drop this patch and instead move the AER-specific clearing logic
out of pcie_do_recovery() entirely (as you suggested earlier), so
that each caller can handle the status clearing explicitly for the
appropriate error sources and severity?
Thanks,
Shuai
