[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <16e5e21e-e4dc-4e8e-85a9-e2b236f1251c@amd.com>
Date: Mon, 6 Oct 2025 14:59:14 -0500
From: "Bowman, Terry" <terry.bowman@....com>
To: "Cheatham, Benjamin" <benjamin.cheatham@....com>
Cc: linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org,
dave@...olabs.net, jonathan.cameron@...wei.com, dave.jiang@...el.com,
alison.schofield@...el.com, dan.j.williams@...el.com, bhelgaas@...gle.com,
shiju.jose@...wei.com, ming.li@...omail.com,
Smita.KoralahalliChannabasappa@....com, rrichter@....com,
dan.carpenter@...aro.org, PradeepVineshReddy.Kodamati@....com,
lukas@...ner.de, sathyanarayanan.kuppuswamy@...ux.intel.com,
linux-cxl@...r.kernel.org, alucerop@....com, ira.weiny@...el.com
Subject: Re: [PATCH v12 09/25] PCI/AER: Report CXL or PCIe bus error type in
trace logging
On 10/3/2025 3:11 PM, Cheatham, Benjamin wrote:
> [snip]
>
>> +/**
>> + * struct aer_err_info - AER Error Information
>> + * @dev: Devices reporting error
>> + * @ratelimit_print: Flag to log or not log the devices' error. 0=NotLog/1=Log
>> + * @error_devnum: Number of devices reporting an error
>> + * @level: printk level to use in logging
>> + * @id: Value from register PCI_ERR_ROOT_ERR_SRC
>> + * @severity: AER severity, 0-UNCOR Non-fatal, 1-UNCOR fatal, 2-COR
>> + * @root_ratelimit_print: Flag to log or not log the root's error. 0=NotLog/1=Log
>> + * @multi_error_valid: If multiple errors are reported
>> + * @first_error: First reported error
>> + * @is_cxl: Bus type error: 0-PCI Bus error, 1-CXL Bus error
>> + * @tlp_header_valid: Indicates if TLP field contains error information
>> + * @status: COR/UNCOR error status
>> + * @mask: COR/UNCOR mask
>> + * @tlp: Transaction packet information
>> + */
>> struct aer_err_info {
>> struct pci_dev *dev[AER_MAX_MULTI_ERR_DEVICES];
>> int ratelimit_print[AER_MAX_MULTI_ERR_DEVICES];
>> @@ -621,7 +638,8 @@ struct aer_err_info {
>> unsigned int multi_error_valid:1;
>>
>> unsigned int first_error:5;
>> - unsigned int __pad2:2;
>> + unsigned int __pad2:1;
>> + bool is_cxl:1; /* CXL or PCI bus error? */
>> unsigned int tlp_header_valid:1;
>>
>> unsigned int status; /* COR/UNCOR Error Status */
> I'd get rid of the comments after the members since it's the exact same thing as the kernel
> doc above the struct.
Good idea.
>> @@ -632,6 +650,11 @@ struct aer_err_info {
>> int aer_get_device_error_info(struct aer_err_info *info, int i);
>> void aer_print_error(struct aer_err_info *info, int i);
>>
>> +static inline const char *aer_err_bus(struct aer_err_info *info)
>> +{
>> + return info->is_cxl ? "CXL" : "PCIe";
>> +}
>> +
>> int pcie_read_tlp_log(struct pci_dev *dev, int where, int where2,
>> unsigned int tlp_len, bool flit,
>> struct pcie_tlp_log *log);
>> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
>> index 6e5c9efe2920..befa73ace9bb 100644
>> --- a/drivers/pci/pcie/aer.c
>> +++ b/drivers/pci/pcie/aer.c
>> @@ -837,6 +837,7 @@ void aer_print_error(struct aer_err_info *info, int i)
>> struct pci_dev *dev;
>> int layer, agent, id;
>> const char *level = info->level;
>> + const char *bus_type = aer_err_bus(info);
>>
>> if (WARN_ON_ONCE(i >= AER_MAX_MULTI_ERR_DEVICES))
>> return;
>> @@ -845,23 +846,23 @@ void aer_print_error(struct aer_err_info *info, int i)
>> id = pci_dev_id(dev);
>>
>> pci_dev_aer_stats_incr(dev, info);
>> - trace_aer_event(pci_name(dev), (info->status & ~info->mask),
>> + trace_aer_event(pci_name(dev), bus_type, (info->status & ~info->mask),
>> info->severity, info->tlp_header_valid, &info->tlp);
>>
>> if (!info->ratelimit_print[i])
>> return;
>>
>> if (!info->status) {
>> - pci_err(dev, "PCIe Bus Error: severity=%s, type=Inaccessible, (Unregistered Agent ID)\n",
>> - aer_error_severity_string[info->severity]);
>> + pci_err(dev, "%s Bus Error: severity=%s, type=Inaccessible, (Unregistered Agent ID)\n",
>> + bus_type, aer_error_severity_string[info->severity]);
>> goto out;
>> }
>>
>> layer = AER_GET_LAYER_ERROR(info->severity, info->status);
>> agent = AER_GET_AGENT(info->severity, info->status);
>>
>> - aer_printk(level, dev, "PCIe Bus Error: severity=%s, type=%s, (%s)\n",
>> - aer_error_severity_string[info->severity],
>> + aer_printk(level, dev, "%s Bus Error: severity=%s, type=%s, (%s)\n",
>> + bus_type, aer_error_severity_string[info->severity],
>> aer_error_layer[layer], aer_agent_string[agent]);
>>
>> aer_printk(level, dev, " device [%04x:%04x] error status/mask=%08x/%08x\n",
>> @@ -895,6 +896,7 @@ EXPORT_SYMBOL_GPL(cper_severity_to_aer);
>> void pci_print_aer(struct pci_dev *dev, int aer_severity,
>> struct aer_capability_regs *aer)
>> {
>> + const char *bus_type;
>> int layer, agent, tlp_header_valid = 0;
>> u32 status, mask;
>> struct aer_err_info info = {
>> @@ -915,9 +917,12 @@ void pci_print_aer(struct pci_dev *dev, int aer_severity,
>>
>> info.status = status;
>> info.mask = mask;
>> + info.is_cxl = pcie_is_cxl(dev);
>> +
>> + bus_type = aer_err_bus(&info);
>>
>> pci_dev_aer_stats_incr(dev, &info);
>> - trace_aer_event(pci_name(dev), (status & ~mask),
>> + trace_aer_event(pci_name(dev), bus_type, (status & ~mask),
>> aer_severity, tlp_header_valid, &aer->header_log);
>>
>> if (!aer_ratelimit(dev, info.severity))
>> @@ -1278,6 +1283,7 @@ int aer_get_device_error_info(struct aer_err_info *info, int i)
>> /* Must reset in this function */
>> info->status = 0;
>> info->tlp_header_valid = 0;
>> + info->is_cxl = pcie_is_cxl(dev);
>>
> So am I right in assuming every AER error that occurs while the link is trained
> as a CXL link will be reported as a CXL error? Sorry if this is a stupid question,
> but is it possible for a PCIe error to occur or does CXL.io just replace the PCIe
> protocol once the link is trained as CXL?
Correct. Any PCI bus protocol errors reported while CXL trained will be reported as
CXL errors.
In your example a "PCIe error" will be detected as a CXL.io error and the AER driver
will log the extended AER register status. The device's CXL RAS will also be logged
if it is a CXL bus error.
> If so, do we not care if the error is a PCIe-level error and just report it as
> a CXL error anyway?
We can't access CXL RAS if its not a CXL error and not a device.
> Sorry if you've already hashed all of this out, but I figured I'd ask just to make sure.
Terry
Powered by blists - more mailing lists