Re: [PATCH v10 03/17] PCI/AER: Report CXL or PCIe bus error type in trace logging

["Bowman, Terry" <terry.bowman@amd.com>]

On 6/26/2025 6:25 PM, Sathyanarayanan Kuppuswamy wrote:
> On 6/26/25 3:42 PM, Terry Bowman wrote:
>> The AER service driver and aer_event tracing currently log 'PCIe Bus Type'
>> for all errors. Update the driver and aer_event tracing to log 'CXL Bus
>> Type' for CXL device errors.
>>
>> This requires the AER can identify and distinguish between PCIe errors and
>> CXL errors.
>>
>> Introduce boolean 'is_cxl' to 'struct aer_err_info'. Add assignment in
>> aer_get_device_error_info() and pci_print_aer().
>>
>> Update the aer_event trace routine to accept a bus type string parameter.
>>
>> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
>> Reviewed-by: Ira Weiny <ira.weiny@intel.com>
>> ---
> Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

Thank you. -Terry

>>   drivers/pci/pci.h       |  6 ++++++
>>   drivers/pci/pcie/aer.c  | 21 +++++++++++++++------
>>   include/ras/ras_event.h |  9 ++++++---
>>   3 files changed, 27 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
>> index 12215ee72afb..a0d1e59b5666 100644
>> --- a/drivers/pci/pci.h
>> +++ b/drivers/pci/pci.h
>> @@ -608,6 +608,7 @@ struct aer_err_info {
>>   	int ratelimit_print[AER_MAX_MULTI_ERR_DEVICES];
>>   	int error_dev_num;
>>   	const char *level;		/* printk level */
>> +	bool is_cxl;
>>   
>>   	unsigned int id:16;
>>   
>> @@ -628,6 +629,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 70ac66188367..a2df9456595a 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))
>> @@ -939,6 +944,9 @@ void pci_print_aer(struct pci_dev *dev, int aer_severity,
>>   	if (tlp_header_valid)
>>   		pcie_print_tlp_log(dev, &aer->header_log, info.level,
>>   				   dev_fmt("  "));
>> +
>> +	trace_aer_event(dev_name(&dev->dev), bus_type, (status & ~mask),
>> +			aer_severity, tlp_header_valid, &aer->header_log);
>>   }
>>   EXPORT_SYMBOL_NS_GPL(pci_print_aer, "CXL");
>>   
>> @@ -1371,6 +1379,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);
>>   
>>   	/* The device might not support AER */
>>   	if (!aer)
>> diff --git a/include/ras/ras_event.h b/include/ras/ras_event.h
>> index 14c9f943d53f..080829d59c36 100644
>> --- a/include/ras/ras_event.h
>> +++ b/include/ras/ras_event.h
>> @@ -297,15 +297,17 @@ TRACE_EVENT(non_standard_event,
>>   
>>   TRACE_EVENT(aer_event,
>>   	TP_PROTO(const char *dev_name,
>> +		 const char *bus_type,
>>   		 const u32 status,
>>   		 const u8 severity,
>>   		 const u8 tlp_header_valid,
>>   		 struct pcie_tlp_log *tlp),
>>   
>> -	TP_ARGS(dev_name, status, severity, tlp_header_valid, tlp),
>> +	TP_ARGS(dev_name, bus_type, status, severity, tlp_header_valid, tlp),
>>   
>>   	TP_STRUCT__entry(
>>   		__string(	dev_name,	dev_name	)
>> +		__string(	bus_type,	bus_type	)
>>   		__field(	u32,		status		)
>>   		__field(	u8,		severity	)
>>   		__field(	u8, 		tlp_header_valid)
>> @@ -314,6 +316,7 @@ TRACE_EVENT(aer_event,
>>   
>>   	TP_fast_assign(
>>   		__assign_str(dev_name);
>> +		__assign_str(bus_type);
>>   		__entry->status		= status;
>>   		__entry->severity	= severity;
>>   		__entry->tlp_header_valid = tlp_header_valid;
>> @@ -325,8 +328,8 @@ TRACE_EVENT(aer_event,
>>   		}
>>   	),
>>   
>> -	TP_printk("%s PCIe Bus Error: severity=%s, %s, TLP Header=%s\n",
>> -		__get_str(dev_name),
>> +	TP_printk("%s %s Bus Error: severity=%s, %s, TLP Header=%s\n",
>> +		__get_str(dev_name), __get_str(bus_type),
>>   		__entry->severity == AER_CORRECTABLE ? "Corrected" :
>>   			__entry->severity == AER_FATAL ?
>>   			"Fatal" : "Uncorrected, non-fatal",