Re: [PATCH v8 02/16] PCI/AER: Modify AER driver logging to report CXL or PCIe bus error type

[Bjorn Helgaas]

On Thu, Mar 27, 2025 at 12:15:10PM -0500, Bowman, Terry wrote:
> On 3/27/2025 11:48 AM, Bjorn Helgaas wrote:
> > On Wed, Mar 26, 2025 at 08:47:03PM -0500, 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.
> >> +++ b/drivers/pci/pci.h
> >> @@ -533,6 +533,7 @@ static inline bool pci_dev_test_and_set_removed(struct pci_dev *dev)
> >>  struct aer_err_info {
> >>  	struct pci_dev *dev[AER_MAX_MULTI_ERR_DEVICES];
> >>  	int error_dev_num;
> >> +	bool is_cxl;
> >>  
> >>  	unsigned int id:16;
> >>  
> >> @@ -549,6 +550,11 @@ struct aer_err_info {
> >>  	struct pcie_tlp_log tlp;	/* TLP Header */
> >>  };
> >>  
> >> +static inline const char *aer_err_bus(struct aer_err_info *info)
> >> +{
> >> +	return info->is_cxl ? "CXL" : "PCIe";
> >
> > I don't really see the point in adding struct aer_err_info.is_cxl.
> > Every place where we call aer_err_bus() to look at it, we also have
> > the struct pci_dev pointer, so we could just as easily use
> > pcie_is_cxl() here.
> 
> My understanding is Dan wanted the decorated aer_err_info::is_cxl to
> capture the type of error (CXL or PCIe) and cache it because it
> could change. That is, the CXL device's alternate protocol could
> transition down before handling but the CXL driver must keep
> knowledge of the original state for reporting. But, the alternate
> protocol transition is not currently detected and used to update
> pci_dev::is_cxl. pci_dev::is_cxl is currently only assigned during
> device creation at the moment.

Sounds like there's always a race here if the CXL/PCIe type isn't
captured as part of a hardware error log register.

Bjorn