linux-efi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
To: Ard Biesheuvel <ardb@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>, Tony Luck <tony.luck@intel.com>,
	James Morse <james.morse@arm.com>,
	Jonathan Cameron <Jonathan.Cameron@huawei.com>,
	Shiju Jose <shiju.jose@huawei.com>,
	linux-efi@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-edac@vger.kernel.org, Len Brown <lenb@kernel.org>,
	linux-acpi@vger.kernel.org
Subject: Re: [PATCH v4 0/3] Fix CPER issues related to UEFI 2.9A Errata
Date: Fri, 21 Jun 2024 17:26:45 +0200	[thread overview]
Message-ID: <20240621172645.21082af3@coco.lan> (raw)
In-Reply-To: <CAMj1kXHsyAhmV9K__pRh8cYJy-ed2-s5VLDE4GwMqNajvJE46w@mail.gmail.com>

Hi Ard,

Em Fri, 21 Jun 2024 09:45:16 +0200
Ard Biesheuvel <ardb@kernel.org> escreveu:

> On Thu, 20 Jun 2024 at 20:01, Mauro Carvalho Chehab
> <mchehab+huawei@kernel.org> wrote:
> >
> > The UEFI 2.9A errata makes clear how ARM processor type encoding should
> > be done: it is meant to be equal to Generic processor, using a bitmask.
> >
> > The current code assumes, for both generic and ARM processor types
> > that this is an integer, which is an incorrect assumption.
> >
> > Fix it. While here, also fix a compilation issue when using W=1.
> >
> > After the change, Kernel will properly decode receiving two errors at the same
> > message, as defined at UEFI spec:
> >
> > [   75.282430] Memory failure: 0x5cdfd: recovery action for free buddy page: Recovered
> > [   94.973081] {2}[Hardware Error]: Hardware error from APEI Generic Hardware Error Source: 1
> > [   94.973770] {2}[Hardware Error]: event severity: recoverable
> > [   94.974334] {2}[Hardware Error]:  Error 0, type: recoverable
> > [   94.974962] {2}[Hardware Error]:   section_type: ARM processor error
> > [   94.975586] {2}[Hardware Error]:   MIDR: 0x000000000000cd24
> > [   94.976202] {2}[Hardware Error]:   Multiprocessor Affinity Register (MPIDR): 0x000000000000ab12
> > [   94.977011] {2}[Hardware Error]:   error affinity level: 2
> > [   94.977593] {2}[Hardware Error]:   running state: 0x1
> > [   94.978135] {2}[Hardware Error]:   Power State Coordination Interface state: 4660
> > [   94.978884] {2}[Hardware Error]:   Error info structure 0:
> > [   94.979463] {2}[Hardware Error]:   num errors: 3
> > [   94.979971] {2}[Hardware Error]:    first error captured
> > [   94.980523] {2}[Hardware Error]:    propagated error captured
> > [   94.981110] {2}[Hardware Error]:    overflow occurred, error info is incomplete
> > [   94.981893] {2}[Hardware Error]:    error_type: 0x0006: cache error|TLB error
> > [   94.982606] {2}[Hardware Error]:    error_info: 0x000000000091000f
> > [   94.983249] {2}[Hardware Error]:     transaction type: Data Access
> > [   94.983891] {2}[Hardware Error]:     cache error, operation type: Data write
> > [   94.984559] {2}[Hardware Error]:     TLB error, operation type: Data write
> > [   94.985215] {2}[Hardware Error]:     cache level: 2
> > [   94.985749] {2}[Hardware Error]:     TLB level: 2
> > [   94.986277] {2}[Hardware Error]:     processor context not corrupted
> >
> > And the error code is properly decoded according with table N.17 from UEFI 2.10
> > spec:
> >
> >         [   94.981893] {2}[Hardware Error]:    error_type: 0x0006: cache error|TLB error
> >
> > Mauro Carvalho Chehab (3):
> >   efi/cper: Adjust infopfx size to accept an extra space
> >   efi/cper: Add a new helper function to print bitmasks
> >   efi/cper: align ARM CPER type with UEFI 2.9A/2.10 specs
> >  
> 
> Hello Mauro,
> 
> How this is v4 different from the preceding 3 revisions that you sent
> over the past 2 days?
> 
> I would expect an experienced maintainer like yourself to be familiar
> with the common practice here: please leave some time between sending
> revisions so people can take a look. And if there is a pressing need
> to deviate from this rule, at least put an explanation in the commit
> log of how the series differs from the preceding one.

Sorry, I'll add a version review on that. Basically I was missing a
test environment to do error injection. When I got it enabled, and fixed
to cope with UEFI 2.9A/2.10 expected behavior, I was able to discover 
some issues and to do some code improvements.

v1: 
- (tagged as RFC) was mostly to give a heads up that the current 
  implementation is not following the spec. It also touches
  only cper code.

v2:
- It fixes the way printks are handled on both cper_arm and ghes
  drivers;

v3:
- It adds a helper function to produce a buffer describing the
  error bits at cper's printk and ghes pr_warn_bitrated. It also
  fixes a W=1 error while building cper;

v4:
- The print function had some bugs on it, which was discovered with
  the help of an error injection tool I'm now using.

I have already another version ready to send. It does some code
cleanup and address the issues pointed by Tony and Jonathan. If you
prefer, I can hold it until Monday to give you some time to look
at it.

Thanks,
Mauro

      reply	other threads:[~2024-06-21 15:26 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-20 18:01 [PATCH v4 0/3] Fix CPER issues related to UEFI 2.9A Errata Mauro Carvalho Chehab
2024-06-20 18:01 ` [PATCH v4 1/3] efi/cper: Adjust infopfx size to accept an extra space Mauro Carvalho Chehab
2024-06-21  9:04   ` Jonathan Cameron
2024-06-20 18:01 ` [PATCH v4 2/3] efi/cper: Add a new helper function to print bitmasks Mauro Carvalho Chehab
2024-06-20 18:08   ` Luck, Tony
2024-06-21  9:20   ` Jonathan Cameron
2024-06-21  9:26     ` Jonathan Cameron
2024-06-21  9:39     ` Mauro Carvalho Chehab
2024-06-20 18:01 ` [PATCH v4 3/3] efi/cper: align ARM CPER type with UEFI 2.9A/2.10 specs Mauro Carvalho Chehab
2024-06-21  9:30   ` Jonathan Cameron
2024-06-21  9:47     ` Mauro Carvalho Chehab
2024-06-21  7:45 ` [PATCH v4 0/3] Fix CPER issues related to UEFI 2.9A Errata Ard Biesheuvel
2024-06-21 15:26   ` Mauro Carvalho Chehab [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240621172645.21082af3@coco.lan \
    --to=mchehab+huawei@kernel.org \
    --cc=Jonathan.Cameron@huawei.com \
    --cc=ardb@kernel.org \
    --cc=bp@alien8.de \
    --cc=james.morse@arm.com \
    --cc=lenb@kernel.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-edac@vger.kernel.org \
    --cc=linux-efi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=shiju.jose@huawei.com \
    --cc=tony.luck@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).