| Message ID | 20220928105946.12469-9-chenzhuo.1@bytedance.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | PCI/AER: Fix and optimize usage of status clearing api | expand |
Hi Zhuo, On Wed, Sep 28, 2022 at 06:59:45PM +0800, Zhuo Chen wrote: > When state is pci_channel_io_frozen in pcie_do_recovery(), the > severity is fatal and fatal error status should be cleared. > So add pci_aer_clear_fatal_status(). > > Signed-off-by: Zhuo Chen <chenzhuo.1@bytedance.com> > --- > drivers/pci/pcie/err.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/pcie/err.c b/drivers/pci/pcie/err.c > index f80b21244ef1..b46f1d36c090 100644 > --- a/drivers/pci/pcie/err.c > +++ b/drivers/pci/pcie/err.c > @@ -241,7 +241,10 @@ pci_ers_result_t pcie_do_recovery(struct pci_dev *dev, > pci_walk_bridge(bridge, report_resume, &status); > > pcie_clear_device_status(dev); > - pci_aer_clear_nonfatal_status(dev); > + if (state == pci_channel_io_frozen) > + pci_aer_clear_fatal_status(dev); > + else > + pci_aer_clear_nonfatal_status(dev); I'm confused. It seems like we certainly need to clear fatal errors after they occur *somewhere*, and if we don't, surely this would be a very obvious issue. But you didn't mention this being a bug fix, so I assume it's more of a cleanup. If it *is* a bug fix, please say that and give a hint about what the bug looks like, e.g., what sort of messages a user might see. If it's not a bug fix, I don't understand how AER fatal errors get cleared today. The PCI_ERR_UNCOR_STATUS bits are sticky, so they're not cleared by a reset. In the current tree, these are the only places I see that clear AER fatal errors: pci_init_capabilities pci_aer_init # once at device enumeration pci_aer_clear_status pci_aer_raw_clear_status pci_write_config_dword(dev, aer + PCI_ERR_UNCOR_STATUS, status) aer_probe aer_enable_rootport # once at Root Port enumeration pci_write_config_dword(pdev, aer + PCI_ERR_UNCOR_STATUS, reg32) dpc_process_error # after DPC triggered pci_aer_clear_fatal_status pci_write_config_dword(dev, aer + PCI_ERR_UNCOR_STATUS, status) edr_handle_event # after EDR event pci_aer_raw_clear_status pci_write_config_dword(dev, aer + PCI_ERR_UNCOR_STATUS, status) pci_restore_state # after reset or PM sleep/resume pci_aer_clear_status pci_aer_raw_clear_status pci_write_config_dword(dev, aer + PCI_ERR_UNCOR_STATUS, status) The only one that could clear errors after an AER error (not DPC or EDR), would be the pci_restore_state() in the reset path. If the current code relies on that, I'd say that's a pretty non-obvious dependency. > pci_info(bridge, "device recovery successful\n"); > return status; > -- > 2.30.1 (Apple Git-130) >
diff --git a/drivers/pci/pcie/err.c b/drivers/pci/pcie/err.c index f80b21244ef1..b46f1d36c090 100644 --- a/drivers/pci/pcie/err.c +++ b/drivers/pci/pcie/err.c @@ -241,7 +241,10 @@ pci_ers_result_t pcie_do_recovery(struct pci_dev *dev, pci_walk_bridge(bridge, report_resume, &status); pcie_clear_device_status(dev); - pci_aer_clear_nonfatal_status(dev); + if (state == pci_channel_io_frozen) + pci_aer_clear_fatal_status(dev); + else + pci_aer_clear_nonfatal_status(dev); pci_info(bridge, "device recovery successful\n"); return status;
When state is pci_channel_io_frozen in pcie_do_recovery(), the severity is fatal and fatal error status should be cleared. So add pci_aer_clear_fatal_status(). Signed-off-by: Zhuo Chen <chenzhuo.1@bytedance.com> --- drivers/pci/pcie/err.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)