[PATCH v2 06/14] PCI/AER: Change AER driver to read UCE fatal status for all CXL PCIe port devices

Terry Bowman posted 14 patches 1 month ago
There is a newer version of this series
[PATCH v2 06/14] PCI/AER: Change AER driver to read UCE fatal status for all CXL PCIe port devices
Posted by Terry Bowman 1 month ago
The AER service driver's aer_get_device_error_info() function doesn't read
uncorrectable (UCE) fatal error status from PCIe upstream port devices,
including CXL upstream switch ports. As a result, fatal errors are not
logged or handled as needed for CXL PCIe upstream switch port devices.

Update the aer_get_device_error_info() function to read the UCE fatal
status for all CXL PCIe port devices.

The fatal error status will be used in future patches implementing
CXL PCIe port uncorrectable error handling and logging.

Signed-off-by: Terry Bowman <terry.bowman@amd.com>
---
 drivers/pci/pcie/aer.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index 1d3e5b929661..d772f123c6a2 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -1250,6 +1250,7 @@ int aer_get_device_error_info(struct pci_dev *dev, struct aer_err_info *info)
 	} else if (type == PCI_EXP_TYPE_ROOT_PORT ||
 		   type == PCI_EXP_TYPE_RC_EC ||
 		   type == PCI_EXP_TYPE_DOWNSTREAM ||
+		   type == PCI_EXP_TYPE_UPSTREAM ||
 		   info->severity == AER_NONFATAL) {
 
 		/* Link is still healthy for IO reads */
-- 
2.34.1
Re: [PATCH v2 06/14] PCI/AER: Change AER driver to read UCE fatal status for all CXL PCIe port devices
Posted by Dave Jiang 3 weeks, 3 days ago

On 10/25/24 2:02 PM, Terry Bowman wrote:
> The AER service driver's aer_get_device_error_info() function doesn't read
> uncorrectable (UCE) fatal error status from PCIe upstream port devices,
> including CXL upstream switch ports. As a result, fatal errors are not
> logged or handled as needed for CXL PCIe upstream switch port devices.
> 
> Update the aer_get_device_error_info() function to read the UCE fatal
> status for all CXL PCIe port devices.
> 
> The fatal error status will be used in future patches implementing
> CXL PCIe port uncorrectable error handling and logging.
> 
> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
> ---
>  drivers/pci/pcie/aer.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> index 1d3e5b929661..d772f123c6a2 100644
> --- a/drivers/pci/pcie/aer.c
> +++ b/drivers/pci/pcie/aer.c
> @@ -1250,6 +1250,7 @@ int aer_get_device_error_info(struct pci_dev *dev, struct aer_err_info *info)
>  	} else if (type == PCI_EXP_TYPE_ROOT_PORT ||
>  		   type == PCI_EXP_TYPE_RC_EC ||
>  		   type == PCI_EXP_TYPE_DOWNSTREAM ||
> +		   type == PCI_EXP_TYPE_UPSTREAM ||

At minimal we probably should do something like
(pcie_is_cxl(dev) && type == PCI_EXP_TYPE_UPSTREAM)
instead so we don't regress the original PCI behavior?
    
>  		   info->severity == AER_NONFATAL) {
>  
>  		/* Link is still healthy for IO reads */
Re: [PATCH v2 06/14] PCI/AER: Change AER driver to read UCE fatal status for all CXL PCIe port devices
Posted by Bowman, Terry 3 weeks, 3 days ago
Hi Dave,

On 10/31/2024 11:58 AM, Dave Jiang wrote:
>
> On 10/25/24 2:02 PM, Terry Bowman wrote:
>> The AER service driver's aer_get_device_error_info() function doesn't read
>> uncorrectable (UCE) fatal error status from PCIe upstream port devices,
>> including CXL upstream switch ports. As a result, fatal errors are not
>> logged or handled as needed for CXL PCIe upstream switch port devices.
>>
>> Update the aer_get_device_error_info() function to read the UCE fatal
>> status for all CXL PCIe port devices.
>>
>> The fatal error status will be used in future patches implementing
>> CXL PCIe port uncorrectable error handling and logging.
>>
>> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
>> ---
>>  drivers/pci/pcie/aer.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
>> index 1d3e5b929661..d772f123c6a2 100644
>> --- a/drivers/pci/pcie/aer.c
>> +++ b/drivers/pci/pcie/aer.c
>> @@ -1250,6 +1250,7 @@ int aer_get_device_error_info(struct pci_dev *dev, struct aer_err_info *info)
>>  	} else if (type == PCI_EXP_TYPE_ROOT_PORT ||
>>  		   type == PCI_EXP_TYPE_RC_EC ||
>>  		   type == PCI_EXP_TYPE_DOWNSTREAM ||
>> +		   type == PCI_EXP_TYPE_UPSTREAM ||
> At minimal we probably should do something like
> (pcie_is_cxl(dev) && type == PCI_EXP_TYPE_UPSTREAM)
> instead so we don't regress the original PCI behavior?

Good Idea. I'll change the condition to what you recommend.

Regards,
Terry
Re: [PATCH v2 06/14] PCI/AER: Change AER driver to read UCE fatal status for all CXL PCIe port devices
Posted by Jonathan Cameron 3 weeks, 4 days ago
On Fri, 25 Oct 2024 16:02:57 -0500
Terry Bowman <terry.bowman@amd.com> wrote:

> The AER service driver's aer_get_device_error_info() function doesn't read
> uncorrectable (UCE) fatal error status from PCIe upstream port devices,
> including CXL upstream switch ports. As a result, fatal errors are not
> logged or handled as needed for CXL PCIe upstream switch port devices.
> 
> Update the aer_get_device_error_info() function to read the UCE fatal
> status for all CXL PCIe port devices.
> 
> The fatal error status will be used in future patches implementing
> CXL PCIe port uncorrectable error handling and logging.
> 
> Signed-off-by: Terry Bowman <terry.bowman@amd.com>

I assume this was previously not done because the upstream port
requires a healthy link and maybe the error indicates we don't have one.

So I'd imagine this change may have a bad effect on PCIe devices
even if we know it's fine CXL ones in the case of certain protocol errors.

Also, does the error log stuff that follows make much sense for
an upstream port?

> ---
>  drivers/pci/pcie/aer.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> index 1d3e5b929661..d772f123c6a2 100644
> --- a/drivers/pci/pcie/aer.c
> +++ b/drivers/pci/pcie/aer.c
> @@ -1250,6 +1250,7 @@ int aer_get_device_error_info(struct pci_dev *dev, struct aer_err_info *info)
>  	} else if (type == PCI_EXP_TYPE_ROOT_PORT ||
>  		   type == PCI_EXP_TYPE_RC_EC ||
>  		   type == PCI_EXP_TYPE_DOWNSTREAM ||
> +		   type == PCI_EXP_TYPE_UPSTREAM ||
>  		   info->severity == AER_NONFATAL) {
>  
>  		/* Link is still healthy for IO reads */