On 6/18/2024 4:04 AM, Terry Bowman wrote: > PCIe port devices are bound to portdrv, the PCIe port bus driver. portdrv > does not implement an AER correctable handler (CE) but does implement the > AER uncorrectable error (UCE). The UCE handler is fairly straightforward > in that it only checks for frozen error state and returns the next step > for recovery accordingly. > > As a result, port devices relying on AER correctable internal errors (CIE) > and AER uncorrectable internal errors (UIE) will not be handled. Note, > the PCIe spec indicates AER CIE/UIE can be used to report implementation > specific errors.[1] > > CXL root ports, CXL downstream switch ports, and CXL upstream switch ports > are examples of devices using the AER CIE/UIE for implementation specific > purposes. These CXL ports use the AER interrupt and AER CIE/UIE status to > report CXL RAS errors.[2] > > Add an atomic notifier to portdrv's CE/UCE handlers. Use the atomic > notifier to report CIE/UIE errors to the registered functions. This will > require adding a CE handler and updating the existing UCE handler. > > For the UCE handler, the CXL spec states UIE errors should return need > reset: "The only method of recovering from an Uncorrectable Internal Error > is reset or hardware replacement."[1] > > [1] PCI6.0 - 6.2.10 Internal Errors > [2] CXL3.1 - 12.2.2 CXL Root Ports, Downstream Switch Ports, and > Upstream Switch Ports > > Signed-off-by: Terry Bowman <terry.bowman@xxxxxxx> > Cc: Bjorn Helgaas <bhelgaas@xxxxxxxxxx> > Cc: linux-pci@xxxxxxxxxxxxxxx > --- > drivers/pci/pcie/portdrv.c | 32 ++++++++++++++++++++++++++++++++ > drivers/pci/pcie/portdrv.h | 2 ++ > 2 files changed, 34 insertions(+) > > diff --git a/drivers/pci/pcie/portdrv.c b/drivers/pci/pcie/portdrv.c > index 14a4b89a3b83..86d80e0e9606 100644 > --- a/drivers/pci/pcie/portdrv.c > +++ b/drivers/pci/pcie/portdrv.c > @@ -37,6 +37,9 @@ struct portdrv_service_data { > u32 service; > }; > > +ATOMIC_NOTIFIER_HEAD(portdrv_aer_internal_err_chain); > +EXPORT_SYMBOL_GPL(portdrv_aer_internal_err_chain); > + > /** > * release_pcie_device - free PCI Express port service device structure > * @dev: Port service device to release > @@ -745,11 +748,39 @@ static void pcie_portdrv_shutdown(struct pci_dev *dev) > static pci_ers_result_t pcie_portdrv_error_detected(struct pci_dev *dev, > pci_channel_state_t error) > { > + if (dev->aer_cap) { > + u32 status; > + > + pci_read_config_dword(dev, dev->aer_cap + PCI_ERR_UNCOR_STATUS, > + &status); > + > + if (status & PCI_ERR_UNC_INTN) { > + atomic_notifier_call_chain(&portdrv_aer_internal_err_chain, > + AER_FATAL, (void *)dev); > + return PCI_ERS_RESULT_NEED_RESET; > + } > + } > + > if (error == pci_channel_io_frozen) > return PCI_ERS_RESULT_NEED_RESET; > return PCI_ERS_RESULT_CAN_RECOVER; > } > > +static void pcie_portdrv_cor_error_detected(struct pci_dev *dev) > +{ > + u32 status; > + > + if (!dev->aer_cap) > + return; Seems like that dev->aer_cap checking is not needed for cor_error_detected, aer_get_device_error_info() already checked it and won't call handle_error_source() if device has not AER capability. But I am curious why pci_aer_handle_error() checks dev->aer_cap again after aer_get_device_error_info(). > + > + pci_read_config_dword(dev, dev->aer_cap + PCI_ERR_COR_STATUS, > + &status); > + > + if (status & PCI_ERR_COR_INTERNAL) > + atomic_notifier_call_chain(&portdrv_aer_internal_err_chain, > + AER_CORRECTABLE, (void *)dev); > +} > + > static pci_ers_result_t pcie_portdrv_slot_reset(struct pci_dev *dev) > { > size_t off = offsetof(struct pcie_port_service_driver, slot_reset); > @@ -780,6 +811,7 @@ static const struct pci_device_id port_pci_ids[] = { > > static const struct pci_error_handlers pcie_portdrv_err_handler = { > .error_detected = pcie_portdrv_error_detected, > + .cor_error_detected = pcie_portdrv_cor_error_detected, > .slot_reset = pcie_portdrv_slot_reset, > .mmio_enabled = pcie_portdrv_mmio_enabled, > }; > diff --git a/drivers/pci/pcie/portdrv.h b/drivers/pci/pcie/portdrv.h > index 12c89ea0313b..8a39197f0203 100644 > --- a/drivers/pci/pcie/portdrv.h > +++ b/drivers/pci/pcie/portdrv.h > @@ -121,4 +121,6 @@ static inline void pcie_pme_interrupt_enable(struct pci_dev *dev, bool en) {} > #endif /* !CONFIG_PCIE_PME */ > > struct device *pcie_port_find_device(struct pci_dev *dev, u32 service); > + > +extern struct atomic_notifier_head portdrv_aer_internal_err_chain; > #endif /* _PORTDRV_H_ */