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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250327174919.GA1437572@bhelgaas>
Date: Thu, 27 Mar 2025 12:49:19 -0500
From: Bjorn Helgaas <helgaas@...nel.org>
To: "Bowman, Terry" <terry.bowman@....com>
Cc: linux-cxl@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-pci@...r.kernel.org, nifan.cxl@...il.com, dave@...olabs.net,
	jonathan.cameron@...wei.com, dave.jiang@...el.com,
	alison.schofield@...el.com, vishal.l.verma@...el.com,
	dan.j.williams@...el.com, bhelgaas@...gle.com, mahesh@...ux.ibm.com,
	ira.weiny@...el.com, oohall@...il.com, Benjamin.Cheatham@....com,
	rrichter@....com, nathan.fontenot@....com,
	Smita.KoralahalliChannabasappa@....com, lukas@...ner.de,
	ming.li@...omail.com, PradeepVineshReddy.Kodamati@....com
Subject: Re: [PATCH v8 02/16] PCI/AER: Modify AER driver logging to report
 CXL or PCIe bus error type

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