Re: [PATCH v7 4/5] PCI/AER: Clear both AER fatal and non-fatal status

Shuai Xue
2026-02-07T08:35:44Z

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