[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <89bf0d37-5b13-4527-b9de-a2773e0259f4@amd.com>
Date: Wed, 26 Jun 2024 08:39:59 -0500
From: Terry Bowman <Terry.Bowman@....com>
To: "Li, Ming4" <ming4.li@...el.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/25/24 21:54, Li, Ming4 wrote:
> 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().
>
Hi Ming,
I agree this check should be removed.
Regards,
Terry
Powered by blists - more mailing lists