* [PATCH v4 0/3] Fix CPER issues related to UEFI 2.9A Errata
@ 2024-06-20 18:01 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 7:45 ` [PATCH v4 0/3] Fix CPER issues related to UEFI 2.9A Errata Ard Biesheuvel
0 siblings, 2 replies; 6+ messages in thread
From: Mauro Carvalho Chehab @ 2024-06-20 18:01 UTC (permalink / raw)
Cc: Mauro Carvalho Chehab, Borislav Petkov, Tony Luck, James Morse,
Jonathan Cameron, Shiju Jose, linux-efi, linux-kernel, linux-edac,
Ard Biesheuvel, Len Brown, linux-acpi
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
drivers/acpi/apei/ghes.c | 10 +++---
drivers/firmware/efi/cper-arm.c | 48 ++++++++++++---------------
drivers/firmware/efi/cper.c | 59 +++++++++++++++++++++++++++++++++
include/linux/cper.h | 13 +++++---
4 files changed, 94 insertions(+), 36 deletions(-)
--
2.45.2
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v4 3/3] efi/cper: align ARM CPER type with UEFI 2.9A/2.10 specs
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 ` Mauro Carvalho Chehab
2024-06-21 9:30 ` Jonathan Cameron
2024-06-21 7:45 ` [PATCH v4 0/3] Fix CPER issues related to UEFI 2.9A Errata Ard Biesheuvel
1 sibling, 1 reply; 6+ messages in thread
From: Mauro Carvalho Chehab @ 2024-06-20 18:01 UTC (permalink / raw)
Cc: Mauro Carvalho Chehab, Borislav Petkov, James Morse,
Jonathan Cameron, Rafael J. Wysocki, Shiju Jose, Tony Luck,
Uwe Kleine-König, Alison Schofield, Ard Biesheuvel,
Dan Williams, Dave Jiang, Ira Weiny, Len Brown, Shuai Xue,
linux-acpi, linux-edac, linux-efi, linux-kernel
Up to UEFI spec, the type byte of CPER struct for ARM processor was
defined simply as:
Type at byte offset 4:
- Cache error
- TLB Error
- Bus Error
- Micro-architectural Error
All other values are reserved
Yet, there was no information about how this would be encoded.
Spec 2.9A errata corrected it by defining:
- Bit 1 - Cache Error
- Bit 2 - TLB Error
- Bit 3 - Bus Error
- Bit 4 - Micro-architectural Error
All other values are reserved
That actually aligns with the values already defined on older
versions at N.2.4.1. Generic Processor Error Section.
Spec 2.10 also preserve the same encoding as 2.9A
See: https://uefi.org/specs/UEFI/2.10/Apx_N_Common_Platform_Error_Record.html#arm-processor-error-information
Adjust CPER and GHES handling code for both generic and ARM
processors to properly handle UEFI 2.9A and 2.10 encoding.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
drivers/acpi/apei/ghes.c | 10 ++++---
drivers/firmware/efi/cper-arm.c | 46 ++++++++++++++-------------------
include/linux/cper.h | 10 +++----
3 files changed, 31 insertions(+), 35 deletions(-)
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index 623cc0cb4a65..093a2d0e49e7 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -533,6 +533,7 @@ static bool ghes_handle_arm_hw_error(struct acpi_hest_generic_data *gdata,
{
struct cper_sec_proc_arm *err = acpi_hest_get_payload(gdata);
int flags = sync ? MF_ACTION_REQUIRED : 0;
+ char error_type[120];
bool queued = false;
int sec_sev, i;
char *p;
@@ -546,9 +547,8 @@ static bool ghes_handle_arm_hw_error(struct acpi_hest_generic_data *gdata,
p = (char *)(err + 1);
for (i = 0; i < err->err_info_num; i++) {
struct cper_arm_err_info *err_info = (struct cper_arm_err_info *)p;
- bool is_cache = (err_info->type == CPER_ARM_CACHE_ERROR);
+ bool is_cache = err_info->type & CPER_ARM_CACHE_ERROR;
bool has_pa = (err_info->validation_bits & CPER_ARM_INFO_VALID_PHYSICAL_ADDR);
- const char *error_type = "unknown error";
/*
* The field (err_info->error_info & BIT(26)) is fixed to set to
@@ -562,8 +562,10 @@ static bool ghes_handle_arm_hw_error(struct acpi_hest_generic_data *gdata,
continue;
}
- if (err_info->type < ARRAY_SIZE(cper_proc_error_type_strs))
- error_type = cper_proc_error_type_strs[err_info->type];
+ cper_bits_to_str(error_type, sizeof(error_type), err_info->type,
+ cper_proc_error_type_strs,
+ ARRAY_SIZE(cper_proc_error_type_strs),
+ CPER_ARM_ERR_TYPE_MASK);
pr_warn_ratelimited(FW_WARN GHES_PFX
"Unhandled processor error type: %s\n",
diff --git a/drivers/firmware/efi/cper-arm.c b/drivers/firmware/efi/cper-arm.c
index d9bbcea0adf4..4c101a09fd80 100644
--- a/drivers/firmware/efi/cper-arm.c
+++ b/drivers/firmware/efi/cper-arm.c
@@ -93,15 +93,11 @@ static void cper_print_arm_err_info(const char *pfx, u32 type,
bool proc_context_corrupt, corrected, precise_pc, restartable_pc;
bool time_out, access_mode;
- /* If the type is unknown, bail. */
- if (type > CPER_ARM_MAX_TYPE)
- return;
-
/*
* Vendor type errors have error information values that are vendor
* specific.
*/
- if (type == CPER_ARM_VENDOR_ERROR)
+ if (type & CPER_ARM_VENDOR_ERROR)
return;
if (error_info & CPER_ARM_ERR_VALID_TRANSACTION_TYPE) {
@@ -116,43 +112,38 @@ static void cper_print_arm_err_info(const char *pfx, u32 type,
if (error_info & CPER_ARM_ERR_VALID_OPERATION_TYPE) {
op_type = ((error_info >> CPER_ARM_ERR_OPERATION_SHIFT)
& CPER_ARM_ERR_OPERATION_MASK);
- switch (type) {
- case CPER_ARM_CACHE_ERROR:
+ if (type & CPER_ARM_CACHE_ERROR) {
if (op_type < ARRAY_SIZE(arm_cache_err_op_strs)) {
- printk("%soperation type: %s\n", pfx,
+ printk("%scache error, operation type: %s\n", pfx,
arm_cache_err_op_strs[op_type]);
}
- break;
- case CPER_ARM_TLB_ERROR:
+ }
+ if (type & CPER_ARM_TLB_ERROR) {
if (op_type < ARRAY_SIZE(arm_tlb_err_op_strs)) {
- printk("%soperation type: %s\n", pfx,
+ printk("%sTLB error, operation type: %s\n", pfx,
arm_tlb_err_op_strs[op_type]);
}
- break;
- case CPER_ARM_BUS_ERROR:
+ }
+ if (type & CPER_ARM_BUS_ERROR) {
if (op_type < ARRAY_SIZE(arm_bus_err_op_strs)) {
- printk("%soperation type: %s\n", pfx,
+ printk("%sbus error, operation type: %s\n", pfx,
arm_bus_err_op_strs[op_type]);
}
- break;
}
}
if (error_info & CPER_ARM_ERR_VALID_LEVEL) {
level = ((error_info >> CPER_ARM_ERR_LEVEL_SHIFT)
& CPER_ARM_ERR_LEVEL_MASK);
- switch (type) {
- case CPER_ARM_CACHE_ERROR:
+ if (type & CPER_ARM_CACHE_ERROR)
printk("%scache level: %d\n", pfx, level);
- break;
- case CPER_ARM_TLB_ERROR:
+
+ if (type & CPER_ARM_TLB_ERROR)
printk("%sTLB level: %d\n", pfx, level);
- break;
- case CPER_ARM_BUS_ERROR:
+
+ if (type & CPER_ARM_BUS_ERROR)
printk("%saffinity level at which the bus error occurred: %d\n",
pfx, level);
- break;
- }
}
if (error_info & CPER_ARM_ERR_VALID_PROC_CONTEXT_CORRUPT) {
@@ -241,6 +232,7 @@ void cper_print_proc_arm(const char *pfx,
struct cper_arm_err_info *err_info;
struct cper_arm_ctx_info *ctx_info;
char newpfx[64], infopfx[65];
+ char error_type[120];
printk("%sMIDR: 0x%016llx\n", pfx, proc->midr);
@@ -289,9 +281,11 @@ void cper_print_proc_arm(const char *pfx,
newpfx);
}
- printk("%serror_type: %d, %s\n", newpfx, err_info->type,
- err_info->type < ARRAY_SIZE(cper_proc_error_type_strs) ?
- cper_proc_error_type_strs[err_info->type] : "unknown");
+ cper_bits_to_str(error_type, sizeof(error_type), err_info->type,
+ cper_proc_error_type_strs,
+ ARRAY_SIZE(cper_proc_error_type_strs),
+ CPER_ARM_ERR_TYPE_MASK);
+ printk("%serror_type: %s\n", newpfx, error_type);
if (err_info->validation_bits & CPER_ARM_INFO_VALID_ERR_INFO) {
printk("%serror_info: 0x%016llx\n", newpfx,
err_info->error_info);
diff --git a/include/linux/cper.h b/include/linux/cper.h
index 856e8f00a7fb..4fef462944d6 100644
--- a/include/linux/cper.h
+++ b/include/linux/cper.h
@@ -293,11 +293,11 @@ enum {
#define CPER_ARM_INFO_FLAGS_PROPAGATED BIT(2)
#define CPER_ARM_INFO_FLAGS_OVERFLOW BIT(3)
-#define CPER_ARM_CACHE_ERROR 0
-#define CPER_ARM_TLB_ERROR 1
-#define CPER_ARM_BUS_ERROR 2
-#define CPER_ARM_VENDOR_ERROR 3
-#define CPER_ARM_MAX_TYPE CPER_ARM_VENDOR_ERROR
+#define CPER_ARM_ERR_TYPE_MASK GENMASK(4,1)
+#define CPER_ARM_CACHE_ERROR BIT(1)
+#define CPER_ARM_TLB_ERROR BIT(2)
+#define CPER_ARM_BUS_ERROR BIT(3)
+#define CPER_ARM_VENDOR_ERROR BIT(4)
#define CPER_ARM_ERR_VALID_TRANSACTION_TYPE BIT(0)
#define CPER_ARM_ERR_VALID_OPERATION_TYPE BIT(1)
--
2.45.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v4 0/3] Fix CPER issues related to UEFI 2.9A Errata
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 3/3] efi/cper: align ARM CPER type with UEFI 2.9A/2.10 specs Mauro Carvalho Chehab
@ 2024-06-21 7:45 ` Ard Biesheuvel
2024-06-21 15:26 ` Mauro Carvalho Chehab
1 sibling, 1 reply; 6+ messages in thread
From: Ard Biesheuvel @ 2024-06-21 7:45 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Borislav Petkov, Tony Luck, James Morse, Jonathan Cameron,
Shiju Jose, linux-efi, linux-kernel, linux-edac, Len Brown,
linux-acpi
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.
Thanks,
Ard.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4 3/3] efi/cper: align ARM CPER type with UEFI 2.9A/2.10 specs
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
0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Cameron @ 2024-06-21 9:30 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Borislav Petkov, James Morse, Rafael J. Wysocki, Shiju Jose,
Tony Luck, Uwe Kleine-König, Alison Schofield,
Ard Biesheuvel, Dan Williams, Dave Jiang, Ira Weiny, Len Brown,
Shuai Xue, linux-acpi, linux-edac, linux-efi, linux-kernel
On Thu, 20 Jun 2024 20:01:46 +0200
Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote:
> Up to UEFI spec, the type byte of CPER struct for ARM processor was
> defined simply as:
>
> Type at byte offset 4:
>
> - Cache error
> - TLB Error
> - Bus Error
> - Micro-architectural Error
> All other values are reserved
>
> Yet, there was no information about how this would be encoded.
>
> Spec 2.9A errata corrected it by defining:
>
> - Bit 1 - Cache Error
> - Bit 2 - TLB Error
> - Bit 3 - Bus Error
> - Bit 4 - Micro-architectural Error
> All other values are reserved
>
> That actually aligns with the values already defined on older
> versions at N.2.4.1. Generic Processor Error Section.
>
> Spec 2.10 also preserve the same encoding as 2.9A
>
> See: https://uefi.org/specs/UEFI/2.10/Apx_N_Common_Platform_Error_Record.html#arm-processor-error-information
>
> Adjust CPER and GHES handling code for both generic and ARM
> processors to properly handle UEFI 2.9A and 2.10 encoding.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
I think you can avoid complexity of your masking solution.
Cost is we don't have that function print that there were reserved bits
set, but that could be easily handled at the caller including notifying
on bits above the defined range which might be helpful.
> diff --git a/drivers/firmware/efi/cper-arm.c b/drivers/firmware/efi/cper-arm.c
> index d9bbcea0adf4..4c101a09fd80 100644
> --- a/drivers/firmware/efi/cper-arm.c
> +++ b/drivers/firmware/efi/cper-arm.c
...
> if (error_info & CPER_ARM_ERR_VALID_PROC_CONTEXT_CORRUPT) {
> @@ -241,6 +232,7 @@ void cper_print_proc_arm(const char *pfx,
> struct cper_arm_err_info *err_info;
> struct cper_arm_ctx_info *ctx_info;
> char newpfx[64], infopfx[65];
> + char error_type[120];
>
> printk("%sMIDR: 0x%016llx\n", pfx, proc->midr);
>
> @@ -289,9 +281,11 @@ void cper_print_proc_arm(const char *pfx,
> newpfx);
> }
>
> - printk("%serror_type: %d, %s\n", newpfx, err_info->type,
> - err_info->type < ARRAY_SIZE(cper_proc_error_type_strs) ?
> - cper_proc_error_type_strs[err_info->type] : "unknown");
> + cper_bits_to_str(error_type, sizeof(error_type), err_info->type,
> + cper_proc_error_type_strs,
> + ARRAY_SIZE(cper_proc_error_type_strs),
> + CPER_ARM_ERR_TYPE_MASK);
Maybe drop this mask complexity and just use
FIELD_GET() to extract the relevant field with no shift from 0.
> + printk("%serror_type: %s\n", newpfx, error_type);
> if (err_info->validation_bits & CPER_ARM_INFO_VALID_ERR_INFO) {
> printk("%serror_info: 0x%016llx\n", newpfx,
> err_info->error_info);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4 3/3] efi/cper: align ARM CPER type with UEFI 2.9A/2.10 specs
2024-06-21 9:30 ` Jonathan Cameron
@ 2024-06-21 9:47 ` Mauro Carvalho Chehab
0 siblings, 0 replies; 6+ messages in thread
From: Mauro Carvalho Chehab @ 2024-06-21 9:47 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Borislav Petkov, James Morse, Rafael J. Wysocki, Shiju Jose,
Tony Luck, Uwe Kleine-König, Alison Schofield,
Ard Biesheuvel, Dan Williams, Dave Jiang, Ira Weiny, Len Brown,
Shuai Xue, linux-acpi, linux-edac, linux-efi, linux-kernel
Em Fri, 21 Jun 2024 10:30:50 +0100
Jonathan Cameron <Jonathan.Cameron@Huawei.com> escreveu:
> On Thu, 20 Jun 2024 20:01:46 +0200
> Mauro Carvalho Chehab <mchehab+huawei@kernel.org> wrote:
>
> > Up to UEFI spec, the type byte of CPER struct for ARM processor was
> > defined simply as:
> >
> > Type at byte offset 4:
> >
> > - Cache error
> > - TLB Error
> > - Bus Error
> > - Micro-architectural Error
> > All other values are reserved
> >
> > Yet, there was no information about how this would be encoded.
> >
> > Spec 2.9A errata corrected it by defining:
> >
> > - Bit 1 - Cache Error
> > - Bit 2 - TLB Error
> > - Bit 3 - Bus Error
> > - Bit 4 - Micro-architectural Error
> > All other values are reserved
> >
> > That actually aligns with the values already defined on older
> > versions at N.2.4.1. Generic Processor Error Section.
> >
> > Spec 2.10 also preserve the same encoding as 2.9A
> >
> > See: https://uefi.org/specs/UEFI/2.10/Apx_N_Common_Platform_Error_Record.html#arm-processor-error-information
> >
> > Adjust CPER and GHES handling code for both generic and ARM
> > processors to properly handle UEFI 2.9A and 2.10 encoding.
> >
> > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
>
> I think you can avoid complexity of your masking solution.
> Cost is we don't have that function print that there were reserved bits
> set, but that could be easily handled at the caller including notifying
> on bits above the defined range which might be helpful.
>
> > diff --git a/drivers/firmware/efi/cper-arm.c b/drivers/firmware/efi/cper-arm.c
> > index d9bbcea0adf4..4c101a09fd80 100644
> > --- a/drivers/firmware/efi/cper-arm.c
> > +++ b/drivers/firmware/efi/cper-arm.c
> ...
>
> > if (error_info & CPER_ARM_ERR_VALID_PROC_CONTEXT_CORRUPT) {
> > @@ -241,6 +232,7 @@ void cper_print_proc_arm(const char *pfx,
> > struct cper_arm_err_info *err_info;
> > struct cper_arm_ctx_info *ctx_info;
> > char newpfx[64], infopfx[65];
> > + char error_type[120];
> >
> > printk("%sMIDR: 0x%016llx\n", pfx, proc->midr);
> >
> > @@ -289,9 +281,11 @@ void cper_print_proc_arm(const char *pfx,
> > newpfx);
> > }
> >
> > - printk("%serror_type: %d, %s\n", newpfx, err_info->type,
> > - err_info->type < ARRAY_SIZE(cper_proc_error_type_strs) ?
> > - cper_proc_error_type_strs[err_info->type] : "unknown");
> > + cper_bits_to_str(error_type, sizeof(error_type), err_info->type,
> > + cper_proc_error_type_strs,
> > + ARRAY_SIZE(cper_proc_error_type_strs),
> > + CPER_ARM_ERR_TYPE_MASK);
>
> Maybe drop this mask complexity and just use
> FIELD_GET() to extract the relevant field with no shift from 0.
IMO not using the function will make the code here more complex, as the
same code needs to be duplicated on two places: here and at ghes, where
the error bits are printed using pr_warn_ratelimited():
cper_bits_to_str(error_type, sizeof(error_type), err_info->type,
cper_proc_error_type_strs,
ARRAY_SIZE(cper_proc_error_type_strs),
CPER_ARM_ERR_TYPE_MASK);
pr_warn_ratelimited(FW_WARN GHES_PFX
"Unhandled processor error type: %s\n",
Also, other parts of CPER uses cper_bits_print() for the same reason:
to have the common print code handled inside a function instead of
repeating the same print pattern everywhere.
> > + printk("%serror_type: %s\n", newpfx, error_type);
> > if (err_info->validation_bits & CPER_ARM_INFO_VALID_ERR_INFO) {
> > printk("%serror_info: 0x%016llx\n", newpfx,
> > err_info->error_info);
>
>
Regards,
Mauro
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4 0/3] Fix CPER issues related to UEFI 2.9A Errata
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
0 siblings, 0 replies; 6+ messages in thread
From: Mauro Carvalho Chehab @ 2024-06-21 15:26 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Borislav Petkov, Tony Luck, James Morse, Jonathan Cameron,
Shiju Jose, linux-efi, linux-kernel, linux-edac, Len Brown,
linux-acpi
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-06-21 15:26 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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 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 is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox