Message-ID: <20250811115238.0000272b@huawei.com>
Date: Mon, 11 Aug 2025 11:52:38 +0100
From: Jonathan Cameron <Jonathan.Cameron@...wei.com>
To: Mauro Carvalho Chehab <mchehab+huawei@...nel.org>
CC: Daniel Ferguson <danielf@...amperecomputing.com>, Ard Biesheuvel
	<ardb@...nel.org>, Jonathan Corbet <corbet@....net>, "Rafael J. Wysocki"
	<rafael@...nel.org>, Len Brown <lenb@...nel.org>, James Morse
	<james.morse@....com>, Tony Luck <tony.luck@...el.com>, Borislav Petkov
	<bp@...en8.de>, <linux-doc@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<linux-acpi@...r.kernel.org>, <linux-efi@...r.kernel.org>,
	<linux-edac@...r.kernel.org>, Jason Tian <jason@...amperecomputing.com>,
	Shengwei Luo <luoshengwei@...wei.com>, Shiju Jose <shiju.jose@...wei.com>
Subject: Re: [PATCH v4 1/5] RAS: Report all ARM processor CPER information
 to userspace

On Sat, 9 Aug 2025 17:55:19 +0200
Mauro Carvalho Chehab <mchehab+huawei@...nel.org> wrote:

> Em Fri, 8 Aug 2025 16:22:09 +0100
> Jonathan Cameron <Jonathan.Cameron@...wei.com> escreveu:
> 
> > On Tue, 05 Aug 2025 11:35:38 -0700
> > Daniel Ferguson <danielf@...amperecomputing.com> wrote:
> >   
> > > From: Jason Tian <jason@...amperecomputing.com>
> > > 
> > > The ARM processor CPER record was added in UEFI v2.6 and remained
> > > unchanged up to v2.10.
> > > 
> > > Yet, the original arm_event trace code added by
> > > 
> > >   e9279e83ad1f ("trace, ras: add ARM processor error trace event")
> > > 
> > > is incomplete, as it only traces some fields of UAPI 2.6 table N.16, not
> > > exporting any information from tables N.17 to N.29 of the record.
> > > 
> > > This is not enough for the user to be able to figure out what has
> > > exactly happened or to take appropriate action.
> > > 
> > > According to the UEFI v2.9 specification chapter N2.4.4, the ARM
> > > processor error section includes:
> > > 
> > > - several (ERR_INFO_NUM) ARM processor error information structures
> > >   (Tables N.17 to N.20);
> > > - several (CONTEXT_INFO_NUM) ARM processor context information
> > >   structures (Tables N.21 to N.29);
> > > - several vendor specific error information structures. The
> > >   size is given by Section Length minus the size of the other
> > >   fields.
> > > 
> > > In addition, it also exports two fields that are parsed by the GHES
> > > driver when firmware reports it, e.g.:
> > > 
> > > - error severity
> > > - CPU logical index
> > > 
> > > Report all of these information to userspace via a the ARM tracepoint so
> > > that userspace can properly record the error and take decisions related
> > > to CPU core isolation according to error severity and other info.
> > > 
> > > The updated ARM trace event now contains the following fields:
> > > 
> > > ======================================  =============================
> > > UEFI field on table N.16                ARM Processor trace fields
> > > ======================================  =============================
> > > Validation                              handled when filling data for
> > >                                         affinity MPIDR and running
> > >                                         state.
> > > ERR_INFO_NUM                            pei_len
> > > CONTEXT_INFO_NUM                        ctx_len
> > > Section Length                          indirectly reported by
> > >                                         pei_len, ctx_len and oem_len
> > > Error affinity level                    affinity
> > > MPIDR_EL1                               mpidr
> > > MIDR_EL1                                midr
> > > Running State                           running_state
> > > PSCI State                              psci_state
> > > Processor Error Information Structure   pei_err - count at pei_len
> > > Processor Context                       ctx_err- count at ctx_len
> > > Vendor Specific Error Info              oem - count at oem_len
> > > ======================================  =============================
> > > 
> > > It should be noted that decoding of tables N.17 to N.29, if needed, will
> > > be handled in userspace. That gives more flexibility, as there won't be
> > > any need to flood the kernel with micro-architecture specific error
> > > decoding.
> > > 
> > > Also, decoding the other fields require a complex logic, and should be
> > > done for each of the several values inside the record field.  So, let
> > > userspace daemons like rasdaemon decode them, parsing such tables and
> > > having vendor-specific micro-architecture-specific decoders.
> > > 
> > >   [mchehab: modified description, solved merge conflicts and fixed coding style]
> > > 
> > > Fixes: e9279e83ad1f ("trace, ras: add ARM processor error trace event")
> > >     
> > 
> > Fixes tag is part of the main tag block so no blank line here.
> > There are at least some scripts running on the kernel tree that trip
> > up on this (and one that moans at the submitter ;)
> > 
> > I'd also add something to explain the SoB sequence for the curious.
> >   
> > > Co-developed-by: Jason Tian <jason@...amperecomputing.com>    
> > 
> > Jason's the Author, so shouldn't have a Co-dev tag.
> > There is some info on this in
> > https://docs.kernel.org/process/submitting-patches.html  
> 
> My understanding is that all co-authors shall have co-developed-by
> and SoB. Anyway, doesn't matter much in practice, I guess.

Nope.  In the description the docs say "in addition to the author
attribute in the From: tag"  There are also examples where there
isn't a Co-dev for the From author including the subtle question of
ordering if someone else posts that patch.

I have a vague recollection one of the scripts checking linux-next
might moan about this. 

> 
> >   
> > > Signed-off-by: Jason Tian <jason@...amperecomputing.com>
> > > Co-developed-by: Shengwei Luo <luoshengwei@...wei.com>
> > > Signed-off-by: Shengwei Luo <luoshengwei@...wei.com>
> > > Co-developed-by: Daniel Ferguson <danielf@...amperecomputing.com>
> > > Signed-off-by: Daniel Ferguson <danielf@...amperecomputing.com>    
> > 
> > As person submitting I'd normally expect your SoB last.
> >   
> > > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@...nel.org>    
> > 
> > I guess this is because Mauro posted an earlier version in which
> > case this is arguably correct, but likely to confuse.
> > For cases like this I add comments.  
> 
> If the patch is identical, and it is just a resubmission,
> I would keep the original order.
> 
> Otherwise, if Daniel did some changes at the code (except for a
> trivial rebase stuff), better to move his co-developed-by/SoB to
> the end, eventually adding:
> 
> [Daniel: <did something>] before the custody chain block.

Docs are clear that sender of the patch must be last SoB.
That's also checked by the some of the tag block check scripts
I think.  There's an example of exactly this case in the in the
submitting-patches.rst file (last one in the section that talks
about Co-developed-by.

For meaning I don't care that much, but keeping to the rigid
rules in that doc makes it easier for scripts to check for the
more important stuff like whether all necessary SoB are there.

Jonathan

> 
> Thanks,
> Mauro
> 

Powered by blists - more mailing lists