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

Message-ID: <65e25486de5ea_31513a29492@iweiny-mobl.notmuch>
Date: Fri, 1 Mar 2024 14:19:50 -0800
From: Ira Weiny <ira.weiny@...el.com>
To: Dan Williams <dan.j.williams@...el.com>, Ira Weiny <ira.weiny@...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

Dan Williams wrote:
> Ira Weiny wrote:
> > 
> > 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.

Fair enough but it is not that heavy IMO.  It was mostly lifted from the
CXL traces.

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

I think it is fair to give the user some information.  I'll redo this
patch to be first and drop the extra work item.

Ira