Re: [PATCH 0/2] efi/cper, cxl: Decode CXL Protocol Errors CPER

[Dan Williams <dan.j.williams@intel.com>]

Jonathan Zhang (Infra) wrote:
> 
> 
> > On Oct 26, 2022, at 12:31 PM, Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com> wrote:
> > 
> > On 10/25/2022 5:11 PM, Dan Williams wrote:
> >> Smita Koralahalli wrote:
> >>> Hi Dan,
> >>> 
> >>> On 10/21/2022 3:18 PM, Dan Williams wrote:
> >>>> Hi Smita,
> >>>> 
> >>>> Smita Koralahalli wrote:
> >>>>> This series adds decoding for the CXL Protocol Errors Common Platform
> >>>>> Error Record.
> >>>> Be sure to copy Ard Biesheuvel <ardb@kernel.org>, added, on
> >>>> drivers/firmware/efi/ patches.
> >>>> 
> >>>> Along those lines, drivers/cxl/ developers have an idea of what is
> >>>> contained in the new CXL protocol error records and why Linux might want
> >>>> to decode them, others from outside drivers/cxl/ might not. It always
> >>>> helps to have a small summary of the benefit to end users of the
> >>>> motivation to apply a patch set.
> >>> Sure, will include in my v2.
> >>> 
> >>>>> Smita Koralahalli (2):
> >>>>>    efi/cper, cxl: Decode CXL Protocol Error Section
> >>>>>    efi/cper, cxl: Decode CXL Error Log
> >>>>> 
> >>>>>   drivers/firmware/efi/Makefile   |   2 +-
> >>>>>   drivers/firmware/efi/cper.c     |   9 +++
> >>>>>   drivers/firmware/efi/cper_cxl.c | 108 ++++++++++++++++++++++++++++++++
> >>>>>   drivers/firmware/efi/cper_cxl.h |  58 +++++++++++++++++
> >>>>>   include/linux/cxl_err.h         |  21 +++++++
> >>>>>   5 files changed, 197 insertions(+), 1 deletion(-)
> >>>> I notice no updates for the trace events in ghes_do_proc(), is that next
> >>>> in your queue? That's ok to be a follow-on after v2.
> >>> Sorry, if I haven't understood this right. Are you implying about the
> >>> "handling"
> >>> of cxl memory errors in ghes_do_proc() or is it just copying of CPER
> >>> entries to
> >>> tracepoints?
> >> Right now ghes_do_proc() will let the CXL CPER records fall through to
> >> log_non_standard_event(). Are you planning to add trace event decode
> >> there for CPER_SEC_CXL_PROT_ERR records?
> > 
> > Thanks! Yeah its a good idea to add. I did not think about this before.
> > I will send this as a separate patchset after v2.
> > 
> > I think with this cxl cper trace event support and Ira's patchset which traces
> > specific event record types via Get Event Record, we can start the userspace
> > handling probably in rasdaemon?
> Yes, I think this makes sense. rasdaemon could aggregate data and provide user
> with full picture:
> * Memory errors from both processor attached memory and CXL memory.
> * CXL protocol errors.
> * CXL device errors.
> Such errors may be handled either firmware first or OS first.

I have no concerns about rasdaemon subscribing to CXL RAS events, but
the nice thing about trace-events is that any number of subscribers can
attach to the event stream. So I expect cxl-cli to have a monitor of
these CXL specific events and that does not preclude rasdaemon from
also incorporating CXL events into its event list.