Re: [PATCH 3/3 v4] ACPI: extlog: Trace CPER CXL Protocol Error Section

["Fabio M. De Francesco" <fabio.m.de.francesco@linux.intel.com>]

On Tuesday, July 1, 2025 3:05:03 PM Central European Summer Time Jonathan Cameron wrote:
> On Mon, 23 Jun 2025 16:54:20 +0200
> "Fabio M. De Francesco" <fabio.m.de.francesco@linux.intel.com> wrote:
> 
> > When Firmware First is enabled, BIOS handles errors first and then it makes
> > them available to the kernel via the Common Platform Error Record (CPER)
> > sections (UEFI 2.10 Appendix N). Linux parses the CPER sections via one of
> > two similar paths, either ELOG or GHES. The errors managed by ELOG are
> > signaled to the BIOS by the I/O Machine Check Architecture (I/O MCA).
> > 
> > Currently, ELOG and GHES show some inconsistencies in how they report to
> > userspace via trace events.
> > 
> > Therefore, make the two mentioned paths act similarly by tracing the CPER
> > CXL Protocol Error Section (UEFI v2.10, Appendix N.2.13).
> > 
> > Cc: Dan Williams <dan.j.williams@intel.com>
> > Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> > Signed-off-by: Fabio M. De Francesco <fabio.m.de.francesco@linux.intel.com>
> > ---
> >  drivers/acpi/acpi_extlog.c | 62 ++++++++++++++++++++++++++++++++++++++
> >  drivers/cxl/core/ras.c     |  6 ++++
> >  include/cxl/event.h        |  2 ++
> >  3 files changed, 70 insertions(+)
> > 
> > diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c
> > index cefe8d2d8affc..9a37b08aacfea 100644
> > --- a/drivers/acpi/acpi_extlog.c
> > +++ b/drivers/acpi/acpi_extlog.c
> > @@ -12,6 +12,7 @@
> >  #include <linux/ratelimit.h>
> >  #include <linux/edac.h>
> >  #include <linux/ras.h>
> > +#include <cxl/event.h>
> >  #include <acpi/ghes.h>
> >  #include <asm/cpu.h>
> >  #include <asm/mce.h>
> > @@ -160,6 +161,60 @@ static void extlog_print_pcie(struct cper_sec_pcie *pcie_err,
> >  	pci_dev_put(pdev);
> >  }
> >  
> > +static void
> > +extlog_cxl_cper_handle_prot_err(struct cxl_cper_sec_prot_err *prot_err,
> > +				int severity)
> > +{
> > +	struct cxl_cper_prot_err_work_data wd;
> > +	u8 *dvsec_start, *cap_start;
> 
> 
> A bunch of this is identical to cxl_cper_post_prot_err()
> Can we factor that stuff out for common use?
> 
> > +
> > +	if (!(prot_err->valid_bits & PROT_ERR_VALID_AGENT_ADDRESS)) {
> > +		pr_warn_ratelimited("CXL CPER invalid agent type\n");
> > +		return;
> > +	}
> > +
> > +	if (!(prot_err->valid_bits & PROT_ERR_VALID_ERROR_LOG)) {
> > +		pr_warn_ratelimited("CXL CPER invalid protocol error log\n");
> > +		return;
> > +	}
> > +
> > +	if (prot_err->err_len != sizeof(struct cxl_ras_capability_regs)) {
> > +		pr_warn_ratelimited("CXL CPER invalid RAS Cap size (%u)\n",
> > +				    prot_err->err_len);
> > +		return;
> > +	}
> > +
> > +	if ((prot_err->agent_type == RCD || prot_err->agent_type == DEVICE ||
> > +	     prot_err->agent_type == LD || prot_err->agent_type == FMLD) &&
> > +	    !(prot_err->valid_bits & PROT_ERR_VALID_SERIAL_NUMBER))
> > +		pr_warn_ratelimited(FW_WARN
> > +				    "CXL CPER no device serial number\n");
> 
> Whilst some of this check isn't present in cxl_cper_post_prot_err(), it should
> be harmless.
>
Maybe all these checks should go to a static helper in cxl/core/ras.c which
cxl_cper_handle_prot_err can call? But I'm not entirely sure yet it would 
really be worth. Anyway, I'll look into it.

Thanks,

Fabio