linux-kernel - Re: [PATCH 4/4] ras/events: Trace CXL CPER events even without the CXL stack loaded

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: <65e24fda80e44_3651e29440@dwillia2-mobl3.amr.corp.intel.com.notmuch>
Date: Fri, 1 Mar 2024 13:59:54 -0800
From: Dan Williams <dan.j.williams@...el.com>
To: Ira Weiny <ira.weiny@...el.com>, Dan Williams <dan.j.williams@...el.com>,
	Jonathan Cameron <jonathan.cameron@...wei.com>, Smita Koralahalli
	<Smita.KoralahalliChannabasappa@....com>, Shiju Jose <shiju.jose@...wei.com>
CC: Dan Carpenter <dan.carpenter@...aro.org>, Yazen Ghannam
	<yazen.ghannam@....com>, Davidlohr Bueso <dave@...olabs.net>, Dave Jiang
	<dave.jiang@...el.com>, Alison Schofield <alison.schofield@...el.com>, Vishal
 Verma <vishal.l.verma@...el.com>, Ard Biesheuvel <ardb@...nel.org>,
	<linux-efi@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<linux-cxl@...r.kernel.org>, Ira Weiny <ira.weiny@...el.com>, "Rafael J.
 Wysocki" <rafael@...nel.org>, Tony Luck <tony.luck@...el.com>, "Borislav
 Petkov" <bp@...en8.de>
Subject: Re: [PATCH 4/4] ras/events: Trace CXL CPER events even without the
 CXL stack loaded

Ira Weiny wrote:
> If CXL is solely managed by firmware (including HDM configuration and
> event processing via firmware first) it is possible to run the system
> without the CXL software loaded.  In this case no CXL callback will be
> loaded and CXL CPER errors will not be processed at all.
> 
> In this case memory device and region (HPA) information is missing but
> omitting the error completely is not friendly for such a user.  Some
> device information is available in the generic event which could prove
> useful to a user.
> 
> Utilize the local work item to trace a generic CXL CPER event.
> 
> Duplicate the pattern of decoding the CXL event header to aid in adding
> future trace points if needed.  This was an easy lift from the CXL trace
> points.  But stop at header decoding only because this is an unlikely
> configuration for the system.  Further decoding can be obtained with
> user space tools or added later if needed.
> 
> Cc: Ard Biesheuvel <ardb@...nel.org>
> Cc: "Rafael J. Wysocki" <rafael@...nel.org>
> Cc: Tony Luck <tony.luck@...el.com>
> Cc: Borislav Petkov <bp@...en8.de>
> Suggested-by: Dan Williams <dan.j.williams@...el.com>
> Signed-off-by: Ira Weiny <ira.weiny@...el.com>
> ---
>  drivers/acpi/apei/ghes.c |  5 ++-
>  include/ras/ras_event.h  | 90 ++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 94 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> index f433f4eae888..9ac323cbf195 100644
> --- a/drivers/acpi/apei/ghes.c
> +++ b/drivers/acpi/apei/ghes.c
> @@ -729,7 +729,10 @@ static void cxl_cper_local_fn(struct work_struct *work)
>  
>  	while (kfifo_out_spinlocked(&cxl_cper_fifo, &wd, 1,
>  			&cxl_cper_read_lock)) {
> -		/* drop msg */
> +		struct cxl_cper_event_rec *rec = &wd.rec;
> +		union cxl_event *evt = &rec->event;
> +
> +		trace_cper_cxl_gen_event(rec, &evt->generic);

So it was confusing to read the empty stub function 2 patches back when this
change was coming, and basic reporting of CXL event does not need the
workqueue indirection. Note that EDAC triggers trace events directly in
the atomic notifier chain, so CXL could do the same.

>  static DECLARE_WORK(cxl_local_work, cxl_cper_local_fn);
> diff --git a/include/ras/ras_event.h b/include/ras/ras_event.h
> index cbd3ddd7c33d..319faf552b65 100644
> --- a/include/ras/ras_event.h
> +++ b/include/ras/ras_event.h

This is more heavywieght than I was expecting and defeats the purpose of
centralizing advanced decode in the CXL driver itself.

I would expect this to be just the tracing equivalent of the
ignore_section logic in cper_estatus_print_section().