linux-kernel - Re: [PATCH v7 1/2] acpi/ghes, cxl/pci: Process CXL CPER Protocol Errors

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] [day] [month] [year] [list]

Message-ID: <67c7a124e2790_364d129453@iweiny-mobl.notmuch>
Date: Tue, 4 Mar 2025 18:56:04 -0600
From: Ira Weiny <ira.weiny@...el.com>
To: Ira Weiny <ira.weiny@...el.com>, Smita Koralahalli
	<Smita.KoralahalliChannabasappa@....com>, <linux-efi@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <linux-cxl@...r.kernel.org>, "Alejandro
 Lucero Palau" <alucerop@....com>
CC: Ard Biesheuvel <ardb@...nel.org>, Alison Schofield
	<alison.schofield@...el.com>, Vishal Verma <vishal.l.verma@...el.com>, "Ira
 Weiny" <ira.weiny@...el.com>, Dan Williams <dan.j.williams@...el.com>,
	Jonathan Cameron <Jonathan.Cameron@...wei.com>, Yazen Ghannam
	<yazen.ghannam@....com>, Terry Bowman <terry.bowman@....com>, "Smita
 Koralahalli" <Smita.KoralahalliChannabasappa@....com>
Subject: Re: [PATCH v7 1/2] acpi/ghes, cxl/pci: Process CXL CPER Protocol
 Errors

Ira Weiny wrote:
> Smita Koralahalli wrote:
> > When PCIe AER is in FW-First, OS should process CXL Protocol errors from
> > CPER records. Introduce support for handling and logging CXL Protocol
> > errors.
> > 
> > The defined trace events cxl_aer_uncorrectable_error and
> > cxl_aer_correctable_error trace native CXL AER endpoint errors. Reuse them
> > to trace FW-First Protocol errors.
> > 
> > Since the CXL code is required to be called from process context and
> > GHES is in interrupt context, use workqueues for processing.
> > 
> > Similar to CXL CPER event handling, use kfifo to handle errors as it
> > simplifies queue processing by providing lock free fifo operations.
> > 
> > Add the ability for the CXL sub-system to register a workqueue to
> > process CXL CPER protocol errors.
> > 
> > Signed-off-by: Smita Koralahalli <Smita.KoralahalliChannabasappa@....com>
> > ---
> > Comments: There is a potential failure case, and I am seeking feedback.
> > 
> > If a CXL Protocol Error occurs during boot: Both acpi_ghes_init() and
> > cxl_core_init() are subsys_initcall. GHES might detect the error and
> > trigger cxl_cper_post_prot_err() even before CXL device is completely
> > enumerated. (i.e pdev might return NULL OR pdev might succeed and cxlds
> > might be NULL as cxl_pci driver is not loaded.)
> > 
> 
> I don't think this is something we should be overly concerned about.
> If protocol errors are occurring that early then they are very likely to
> be bad hardware which is going to happen once the subsystems are brought
> up and start working with the devices.  So new errors will alert the user.
> 
> > Usage of delayed_workqueue(): Would delaying the handling/logging of
> > errors, particularly uncorrectable errors, be acceptable?
> > Any alternative suggestions for addressing this issue would be greatly
> > appreciated.
> > 
> > Tony questioned choosing value 8 for FIFO_DEPTH in v6. That was just a
> > random value that I picked. I would appreciate any suggestions in
> > considering the appropriate value for number of entries.
> 
> FWIW I think this is fine until someone sees a reason to increase it.
> 
> [snip]
> 
> > +
> > +static void cxl_cper_trace_corr_prot_err(struct pci_dev *pdev,
> > +				  struct cxl_ras_capability_regs ras_cap)
> > +{
> > +	u32 status = ras_cap.cor_status & ~ras_cap.cor_mask;
> > +	struct cxl_dev_state *cxlds;
> > +
> > +	cxlds = pci_get_drvdata(pdev);
> > +	if (!cxlds)
> > +		return;
> > +
> > +	trace_cxl_aer_correctable_error(cxlds->cxlmd, status);
> 
> I dug into this just a bit wondering if passing cxl_memdev is the best way
> for this tracepoint to work given the type 2 work...
> 
> 	+ Alejandro
> 
> For now I think this patch is fine.  So.
> 
> Reviewed-by: Ira Weiny <ira.weiny@...el.com>

Unfortunately I missed the fact that this breaks the test build.  ras.o
needs to be in the test build files as well.

Ira

diff --git a/tools/testing/cxl/Kbuild b/tools/testing/cxl/Kbuild
index ef10a896a384..4efcc0606bd6 100644
--- a/tools/testing/cxl/Kbuild
+++ b/tools/testing/cxl/Kbuild
@@ -62,6 +62,7 @@ cxl_core-y += $(CXL_CORE_SRC)/hdm.o
 cxl_core-y += $(CXL_CORE_SRC)/pmu.o
 cxl_core-y += $(CXL_CORE_SRC)/cdat.o
 cxl_core-y += $(CXL_CORE_SRC)/acpi.o
+cxl_core-y += $(CXL_CORE_SRC)/ras.o
 cxl_core-$(CONFIG_TRACING) += $(CXL_CORE_SRC)/trace.o
 cxl_core-$(CONFIG_CXL_REGION) += $(CXL_CORE_SRC)/region.o
 cxl_core-$(CONFIG_CXL_FEATURES) += $(CXL_CORE_SRC)/features.o