Re: [RFC PATCH 1/9] PCI/AER: Update AER driver to call root port and downstream port UCE handlers

Dan Williams
2024-06-21T19:17:50Z

Terry Bowman wrote:
> The AER service driver does not currently call a handler for AER
> uncorrectable errors (UCE) detected in root ports or downstream
> ports. This is not needed in most cases because common PCIe port
> functionality is handled by portdrv service drivers.
>
> CXL root ports include CXL specific RAS registers that need logging
> before starting do_recovery() in the UCE case.
>
> Update the AER service driver to call the UCE handler for root ports
> and downstream ports. These PCIe port devices are bound to the portdrv
> driver that includes a CE and UCE handler to be called.
>
> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: linux-pci@vger.kernel.org
> ---
> drivers/pci/pcie/err.c | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/drivers/pci/pcie/err.c b/drivers/pci/pcie/err.c
> index 705893b5f7b0..a4db474b2be5 100644
> --- a/drivers/pci/pcie/err.c
> +++ b/drivers/pci/pcie/err.c
> @@ -203,6 +203,26 @@ pci_ers_result_t pcie_do_recovery(struct pci_dev *dev,
> pci_ers_result_t status = PCI_ERS_RESULT_CAN_RECOVER;
> struct pci_host_bridge *host = pci_find_host_bridge(dev->bus);
>
> + /*
> + * PCIe ports may include functionality beyond the standard
> + * extended port capabilities. This may present a need to log and
> + * handle errors not addressed in this driver. Examples are CXL
> + * root ports and CXL downstream switch ports using AER UIE to
> + * indicate CXL UCE RAS protocol errors.
> + */
> + if (type == PCI_EXP_TYPE_ROOT_PORT ||
> + type == PCI_EXP_TYPE_DOWNSTREAM) {
> + struct pci_driver *pdrv = dev->driver;
> +
> + if (pdrv && pdrv->err_handler &&
> + pdrv->err_handler->error_detected) {
> + const struct pci_error_handlers *err_handler;
> +
> + err_handler = pdrv->err_handler;
> + status = err_handler->error_detected(dev, state);
> + }
> + }
> +

Would not a more appropriate place for this be pci_walk_bridge() where
the ->subordinate == NULL and these type-check cases are unified?