[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6b1bf5ab-0005-4c88-99ec-92edca828f44@intel.com>
Date: Wed, 26 Jun 2024 10:54:55 +0800
From: "Li, Ming4" <ming4.li@...el.com>
To: Terry Bowman <terry.bowman@....com>, "Williams, Dan J"
<dan.j.williams@...el.com>, "Weiny, Ira" <ira.weiny@...el.com>,
"dave@...olabs.net" <dave@...olabs.net>, "Jiang, Dave"
<dave.jiang@...el.com>, "Schofield, Alison" <Alison.Schofield@...el.com>,
"Verma, Vishal L" <vishal.l.verma@...el.com>, "jim.harris@...sung.com"
<jim.harris@...sung.com>, "ilpo.jarvinen@...ux.intel.com"
<ilpo.jarvinen@...ux.intel.com>, "ardb@...nel.org" <ardb@...nel.org>,
"sathyanarayanan.kuppuswamy@...ux.intel.com"
<sathyanarayanan.kuppuswamy@...ux.intel.com>, "linux-cxl@...r.kernel.org"
<linux-cxl@...r.kernel.org>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>, "Yazen.Ghannam@....com"
<Yazen.Ghannam@....com>, "Robert.Richter@....com" <Robert.Richter@....com>
CC: Bjorn Helgaas <bhelgaas@...gle.com>, "linux-pci@...r.kernel.org"
<linux-pci@...r.kernel.org>
Subject: Re: [RFC PATCH 3/9] PCI/portdrv: Update portdrv with an atomic
notifier for reporting AER internal errors
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@....com>
> Cc: Bjorn Helgaas <bhelgaas@...gle.com>
> Cc: linux-pci@...r.kernel.org
> ---
> 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_ */
Powered by blists - more mailing lists