2018-02-23 20:06:40

by Yazen Ghannam

Subject: [PATCH 0/8] Decode IA32/X64 CPER

From: Yazen Ghannam <[email protected]>

This series adds decoding for the IA32/X64 Common Platform Error Record.

Patch 1 fixes the IA32/X64 Processor Error Section definition to match
the UEFI spec.

Patches 2-8 add the new decoding. The patches incrementally add the
decoding starting from the top-level "Error Section". Hopefully, this
will make reviewing a bit easier compared to one large patch.

The formatting of the field names and options is taken from the UEFI
spec. I tried to keep everything the same to make searching easier.

The patches were written to the UEFI 2.7 spec though the definition of
the IA32/X64 CPER seems to be the same as when it was introduced in
the UEFI 2.1 spec.

Without basic decoding, users will be confused about what these
"Hardware Errors" mean. So I'm requesting this set to be applied to the
stable branches. This set applies to the v4.16. However, patch 2 will
have a conflict on older branches, so I'll send this set again with the
conflict fixed.

Thanks,
Yazen

Yazen Ghannam (8):
efi: Fix IA32/X64 Processor Error Record definition
efi: Decode IA32/X64 Processor Error Section
efi: Decode IA32/X64 Processor Error Info Structure
efi: Decode UEFI-defined IA32/X64 Error Structure GUIDs
efi: Decode IA32/X64 Cache, TLB, and Bus Check structures
efi: Decode additional IA32/X64 Bus Check fields
efi: Decode IA32/X64 MS Check structure
efi: Decode IA32/X64 Context Info structure

drivers/firmware/efi/Kconfig | 5 +
drivers/firmware/efi/Makefile | 2 +
drivers/firmware/efi/cper-x86.c | 371 ++++++++++++++++++++++++++++++++++++++++
drivers/firmware/efi/cper.c | 10 ++
include/linux/cper.h | 4 +-
5 files changed, 391 insertions(+), 1 deletion(-)
create mode 100644 drivers/firmware/efi/cper-x86.c

--
2.14.1

2018-02-23 20:05:37

by Yazen Ghannam

Subject: [PATCH 5/8] efi: Decode IA32/X64 Cache, TLB, and Bus Check structures

From: Yazen Ghannam <[email protected]>

Print the common fields of the Cache, TLB, and Bus check structures.The
fields of these three check types are the same except for a few more
fields in the Bus check structure. The remaining Bus check structure
fields will be decoded in a following patch.

Based on UEFI 2.7,
Table 257. IA32/X64 Cache Check Structure
Table 258. IA32/X64 TLB Check Structure
Table 259. IA32/X64 Bus Check Structure

Cc: <[email protected]> # 4.16.x
Signed-off-by: Yazen Ghannam <[email protected]>
---
drivers/firmware/efi/cper-x86.c | 101 +++++++++++++++++++++++++++++++++++++++-
1 file changed, 100 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/efi/cper-x86.c b/drivers/firmware/efi/cper-x86.c
index 3b86223bdb67..75f664043076 100644
--- a/drivers/firmware/efi/cper-x86.c
+++ b/drivers/firmware/efi/cper-x86.c
@@ -33,6 +33,25 @@
#define INFO_VALID_RESPONDER_ID BIT_ULL(3)
#define INFO_VALID_IP BIT_ULL(4)

+#define CHECK_VALID_TRANS_TYPE BIT_ULL(0)
+#define CHECK_VALID_OPERATION BIT_ULL(1)
+#define CHECK_VALID_LEVEL BIT_ULL(2)
+#define CHECK_VALID_PCC BIT_ULL(3)
+#define CHECK_VALID_UNCORRECTED BIT_ULL(4)
+#define CHECK_VALID_PRECISE_IP BIT_ULL(5)
+#define CHECK_VALID_RESTARTABLE_IP BIT_ULL(6)
+#define CHECK_VALID_OVERFLOW BIT_ULL(7)
+
+#define CHECK_VALID_BITS(check) ((check & GENMASK_ULL(15, 0)))
+#define CHECK_TRANS_TYPE(check) ((check & GENMASK_ULL(17, 16)) >> 16)
+#define CHECK_OPERATION(check) ((check & GENMASK_ULL(21, 18)) >> 18)
+#define CHECK_LEVEL(check) ((check & GENMASK_ULL(24, 22)) >> 22)
+#define CHECK_PCC BIT_ULL(25)
+#define CHECK_UNCORRECTED BIT_ULL(26)
+#define CHECK_PRECISE_IP BIT_ULL(27)
+#define CHECK_RESTARTABLE_IP BIT_ULL(28)
+#define CHECK_OVERFLOW BIT_ULL(29)
+
enum err_types {
ERR_TYPE_CACHE = 0,
ERR_TYPE_TLB,
@@ -55,11 +74,83 @@ static enum err_types cper_get_err_type(const guid_t *err_type)
return N_ERR_TYPES;
}

+static const char * const ia_check_trans_type_strs[] = {
+ "Instruction",
+ "Data Access",
+ "Generic",
+};
+
+static const char * const ia_check_op_strs[] = {
+ "generic error",
+ "generic read",
+ "generic write",
+ "data read",
+ "data write",
+ "instruction fetch",
+ "prefetch",
+ "eviction",
+ "snoop",
+};
+
+static inline void print_bool(char *str, const char *pfx, u64 check, u64 bit)
+{
+ printk("%s%s: %s\n", pfx, str, (check & bit) ? "true" : "false");
+}
+
+static void print_err_info(const char *pfx, enum err_types err_type, u64 check)
+{
+ u16 validation_bits = CHECK_VALID_BITS(check);
+
+ printk("%sValidation Bits: 0x%04x\n", pfx, validation_bits);
+
+ if (err_type == ERR_TYPE_MS)
+ return;
+
+ if (validation_bits & CHECK_VALID_TRANS_TYPE) {
+ u8 trans_type = CHECK_TRANS_TYPE(check);
+
+ printk("%sTransaction Type: %u, %s\n", pfx, trans_type,
+ trans_type < ARRAY_SIZE(ia_check_trans_type_strs) ?
+ ia_check_trans_type_strs[trans_type] : "unknown");
+ }
+
+ if (validation_bits & CHECK_VALID_OPERATION) {
+ u8 op = CHECK_OPERATION(check);
+
+ /*
+ * CACHE has more operation types than TLB or BUS, though the
+ * name and the order are the same.
+ */
+ u8 max_ops = (err_type == ERR_TYPE_CACHE) ? 9 : 7;
+
+ printk("%sOperation: %u, %s\n", pfx, op,
+ op < max_ops ? ia_check_op_strs[op] : "unknown");
+ }
+
+ if (validation_bits & CHECK_VALID_LEVEL)
+ printk("%sLevel: %llu\n", pfx, CHECK_LEVEL(check));
+
+ if (validation_bits & CHECK_VALID_PCC)
+ print_bool("Processor Context Corrupt", pfx, check, CHECK_PCC);
+
+ if (validation_bits & CHECK_VALID_UNCORRECTED)
+ print_bool("Uncorrected", pfx, check, CHECK_UNCORRECTED);
+
+ if (validation_bits & CHECK_VALID_PRECISE_IP)
+ print_bool("Precise IP", pfx, check, CHECK_PRECISE_IP);
+
+ if (validation_bits & CHECK_VALID_RESTARTABLE_IP)
+ print_bool("Restartable IP", pfx, check, CHECK_RESTARTABLE_IP);
+
+ if (validation_bits & CHECK_VALID_OVERFLOW)
+ print_bool("Overflow", pfx, check, CHECK_OVERFLOW);
+}
+
void cper_print_proc_ia(const char *pfx, const struct cper_sec_proc_ia *proc)
{
int i;
struct cper_ia_err_info *err_info;
- char newpfx[64];
+ char newpfx[64], infopfx[64];
enum err_types err_type;

printk("%sValidation Bits: 0x%016llx\n", pfx, proc->validation_bits);
@@ -93,6 +184,14 @@ void cper_print_proc_ia(const char *pfx, const struct cper_sec_proc_ia *proc)
if (err_info->validation_bits & INFO_VALID_CHECK_INFO) {
printk("%sCheck Information: 0x%016llx\n", newpfx,
err_info->check_info);
+
+ if (err_type < N_ERR_TYPES) {
+ snprintf(infopfx, sizeof(infopfx), "%s%s",
+ newpfx, INDENT_SP);
+
+ print_err_info(infopfx, err_type,
+ err_info->check_info);
+ }
}

if (err_info->validation_bits & INFO_VALID_TARGET_ID) {
--
2.14.1

2018-02-23 20:06:16

by Yazen Ghannam

Subject: [PATCH 4/8] efi: Decode UEFI-defined IA32/X64 Error Structure GUIDs

From: Yazen Ghannam <[email protected]>

For easier handling, match the known IA32/X64 error structure GUIDs to
enums.

Also, print out the name of the matching Error Structure Type.

GUIDs taken from UEFI 2.7 section N.2.4.2.1 IA32/X64 Processor Error
Information Structure.

Cc: <[email protected]> # 4.16.x
Signed-off-by: Yazen Ghannam <[email protected]>
---
drivers/firmware/efi/cper-x86.c | 41 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 41 insertions(+)

diff --git a/drivers/firmware/efi/cper-x86.c b/drivers/firmware/efi/cper-x86.c
index 9d608f742c98..3b86223bdb67 100644
--- a/drivers/firmware/efi/cper-x86.c
+++ b/drivers/firmware/efi/cper-x86.c
@@ -14,17 +14,53 @@
#define VALID_CPUID_INFO BIT_ULL(1)
#define VALID_PROC_ERR_INFO_NUM(bits) ((bits & GENMASK_ULL(7, 2)) >> 2)

+#define INFO_ERR_STRUCT_TYPE_CACHE \
+ GUID_INIT(0xA55701F5, 0xE3EF, 0x43DE, 0xAC, 0x72, 0x24, 0x9B, \
+ 0x57, 0x3F, 0xAD, 0x2C)
+#define INFO_ERR_STRUCT_TYPE_TLB \
+ GUID_INIT(0xFC06B535, 0x5E1F, 0x4562, 0x9F, 0x25, 0x0A, 0x3B, \
+ 0x9A, 0xDB, 0x63, 0xC3)
+#define INFO_ERR_STRUCT_TYPE_BUS \
+ GUID_INIT(0x1CF3F8B3, 0xC5B1, 0x49a2, 0xAA, 0x59, 0x5E, 0xEF, \
+ 0x92, 0xFF, 0xA6, 0x3C)
+#define INFO_ERR_STRUCT_TYPE_MS \
+ GUID_INIT(0x48AB7F57, 0xDC34, 0x4f6c, 0xA7, 0xD3, 0xB0, 0xB5, \
+ 0xB0, 0xA7, 0x43, 0x14)
+
#define INFO_VALID_CHECK_INFO BIT_ULL(0)
#define INFO_VALID_TARGET_ID BIT_ULL(1)
#define INFO_VALID_REQUESTOR_ID BIT_ULL(2)
#define INFO_VALID_RESPONDER_ID BIT_ULL(3)
#define INFO_VALID_IP BIT_ULL(4)

+enum err_types {
+ ERR_TYPE_CACHE = 0,
+ ERR_TYPE_TLB,
+ ERR_TYPE_BUS,
+ ERR_TYPE_MS,
+ N_ERR_TYPES
+};
+
+static enum err_types cper_get_err_type(const guid_t *err_type)
+{
+ if (guid_equal(err_type, &INFO_ERR_STRUCT_TYPE_CACHE))
+ return ERR_TYPE_CACHE;
+ else if (guid_equal(err_type, &INFO_ERR_STRUCT_TYPE_TLB))
+ return ERR_TYPE_TLB;
+ else if (guid_equal(err_type, &INFO_ERR_STRUCT_TYPE_BUS))
+ return ERR_TYPE_BUS;
+ else if (guid_equal(err_type, &INFO_ERR_STRUCT_TYPE_MS))
+ return ERR_TYPE_MS;
+ else
+ return N_ERR_TYPES;
+}
+
void cper_print_proc_ia(const char *pfx, const struct cper_sec_proc_ia *proc)
{
int i;
struct cper_ia_err_info *err_info;
char newpfx[64];
+ enum err_types err_type;

printk("%sValidation Bits: 0x%016llx\n", pfx, proc->validation_bits);

@@ -46,6 +82,11 @@ void cper_print_proc_ia(const char *pfx, const struct cper_sec_proc_ia *proc)
printk("%sError Structure Type: %pUl\n", newpfx,
&err_info->err_type);

+ err_type = cper_get_err_type(&err_info->err_type);
+ printk("%sError Structure Type: %s\n", newpfx,
+ err_type < ARRAY_SIZE(cper_proc_error_type_strs) ?
+ cper_proc_error_type_strs[err_type] : "unknown");
+
printk("%sValidation Bits: 0x%016llx\n",
newpfx, err_info->validation_bits);

--
2.14.1

2018-02-23 20:06:21

by Yazen Ghannam

Subject: [PATCH 6/8] efi: Decode additional IA32/X64 Bus Check fields

From: Yazen Ghannam <[email protected]>

The "Participation Type", "Time Out", and "Address Space" fields are
unique to the IA32/X64 Bus Check structure. Print these fields.

Based on UEFI 2.7 Table 259. IA32/X64 Bus Check Structure

Cc: <[email protected]> # 4.16.x
Signed-off-by: Yazen Ghannam <[email protected]>
---
drivers/firmware/efi/cper-x86.c | 44 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 44 insertions(+)

diff --git a/drivers/firmware/efi/cper-x86.c b/drivers/firmware/efi/cper-x86.c
index 75f664043076..df4cf6221d8e 100644
--- a/drivers/firmware/efi/cper-x86.c
+++ b/drivers/firmware/efi/cper-x86.c
@@ -42,6 +42,10 @@
#define CHECK_VALID_RESTARTABLE_IP BIT_ULL(6)
#define CHECK_VALID_OVERFLOW BIT_ULL(7)

+#define CHECK_VALID_BUS_PART_TYPE BIT_ULL(8)
+#define CHECK_VALID_BUS_TIME_OUT BIT_ULL(9)
+#define CHECK_VALID_BUS_ADDR_SPACE BIT_ULL(10)
+
#define CHECK_VALID_BITS(check) ((check & GENMASK_ULL(15, 0)))
#define CHECK_TRANS_TYPE(check) ((check & GENMASK_ULL(17, 16)) >> 16)
#define CHECK_OPERATION(check) ((check & GENMASK_ULL(21, 18)) >> 18)
@@ -52,6 +56,10 @@
#define CHECK_RESTARTABLE_IP BIT_ULL(28)
#define CHECK_OVERFLOW BIT_ULL(29)

+#define CHECK_BUS_PART_TYPE(check) ((check & GENMASK_ULL(31, 30)) >> 30)
+#define CHECK_BUS_TIME_OUT BIT_ULL(32)
+#define CHECK_BUS_ADDR_SPACE(check) ((check & GENMASK_ULL(34, 33)) >> 33)
+
enum err_types {
ERR_TYPE_CACHE = 0,
ERR_TYPE_TLB,
@@ -92,6 +100,20 @@ static const char * const ia_check_op_strs[] = {
"snoop",
};

+static const char * const ia_check_bus_part_type_strs[] = {
+ "Local Processor originated request",
+ "Local Processor responded to request",
+ "Local Processor observed",
+ "Generic",
+};
+
+static const char * const ia_check_bus_addr_space_strs[] = {
+ "Memory Access",
+ "Reserved",
+ "I/O",
+ "Other Transaction",
+};
+
static inline void print_bool(char *str, const char *pfx, u64 check, u64 bit)
{
printk("%s%s: %s\n", pfx, str, (check & bit) ? "true" : "false");
@@ -144,6 +166,28 @@ static void print_err_info(const char *pfx, enum err_types err_type, u64 check)

if (validation_bits & CHECK_VALID_OVERFLOW)
print_bool("Overflow", pfx, check, CHECK_OVERFLOW);
+
+ if (err_type != ERR_TYPE_BUS)
+ return;
+
+ if (validation_bits & CHECK_VALID_BUS_PART_TYPE) {
+ u8 part_type = CHECK_BUS_PART_TYPE(check);
+
+ printk("%sParticipation Type: %u, %s\n", pfx, part_type,
+ part_type < ARRAY_SIZE(ia_check_bus_part_type_strs) ?
+ ia_check_bus_part_type_strs[part_type] : "unknown");
+ }
+
+ if (validation_bits & CHECK_VALID_BUS_TIME_OUT)
+ print_bool("Time Out", pfx, check, CHECK_BUS_TIME_OUT);
+
+ if (validation_bits & CHECK_VALID_BUS_ADDR_SPACE) {
+ u8 addr_space = CHECK_BUS_ADDR_SPACE(check);
+
+ printk("%sAddress Space: %u, %s\n", pfx, addr_space,
+ addr_space < ARRAY_SIZE(ia_check_bus_addr_space_strs) ?
+ ia_check_bus_addr_space_strs[addr_space] : "unknown");
+ }
}

void cper_print_proc_ia(const char *pfx, const struct cper_sec_proc_ia *proc)
--
2.14.1

2018-02-23 20:06:21

by Yazen Ghannam

Subject: [PATCH 7/8] efi: Decode IA32/X64 MS Check structure

From: Yazen Ghannam <[email protected]>

The IA32/X64 MS Check structure varies from the other Check structures
in the the bit positions of its fields, and it includes an additional
"Error Type" field.

Decode the MS Check structure in a separate function.

Based on UEFI 2.7 Table 260. IA32/X64 MS Check Field Description.

Cc: <[email protected]> # 4.16.x
Signed-off-by: Yazen Ghannam <[email protected]>
---
drivers/firmware/efi/cper-x86.c | 55 ++++++++++++++++++++++++++++++++++++++++-
1 file changed, 54 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/efi/cper-x86.c b/drivers/firmware/efi/cper-x86.c
index df4cf6221d8e..02b1b424f537 100644
--- a/drivers/firmware/efi/cper-x86.c
+++ b/drivers/firmware/efi/cper-x86.c
@@ -60,6 +60,20 @@
#define CHECK_BUS_TIME_OUT BIT_ULL(32)
#define CHECK_BUS_ADDR_SPACE(check) ((check & GENMASK_ULL(34, 33)) >> 33)

+#define CHECK_VALID_MS_ERR_TYPE BIT_ULL(0)
+#define CHECK_VALID_MS_PCC BIT_ULL(1)
+#define CHECK_VALID_MS_UNCORRECTED BIT_ULL(2)
+#define CHECK_VALID_MS_PRECISE_IP BIT_ULL(3)
+#define CHECK_VALID_MS_RESTARTABLE_IP BIT_ULL(4)
+#define CHECK_VALID_MS_OVERFLOW BIT_ULL(5)
+
+#define CHECK_MS_ERR_TYPE(check) ((check & GENMASK_ULL(18, 16)) >> 16)
+#define CHECK_MS_PCC BIT_ULL(19)
+#define CHECK_MS_UNCORRECTED BIT_ULL(20)
+#define CHECK_MS_PRECISE_IP BIT_ULL(21)
+#define CHECK_MS_RESTARTABLE_IP BIT_ULL(22)
+#define CHECK_MS_OVERFLOW BIT_ULL(23)
+
enum err_types {
ERR_TYPE_CACHE = 0,
ERR_TYPE_TLB,
@@ -114,19 +128,58 @@ static const char * const ia_check_bus_addr_space_strs[] = {
"Other Transaction",
};

+static const char * const ia_check_ms_error_type_strs[] = {
+ "No Error",
+ "Unclassified",
+ "Microcode ROM Parity Error",
+ "External Error",
+ "FRC Error",
+ "Internal Unclassified",
+};
+
static inline void print_bool(char *str, const char *pfx, u64 check, u64 bit)
{
printk("%s%s: %s\n", pfx, str, (check & bit) ? "true" : "false");
}

+static void print_err_info_ms(const char *pfx, u16 validation_bits, u64 check)
+{
+ if (validation_bits & CHECK_VALID_MS_ERR_TYPE) {
+ u8 err_type = CHECK_MS_ERR_TYPE(check);
+
+ printk("%sError Type: %u, %s\n", pfx, err_type,
+ err_type < ARRAY_SIZE(ia_check_ms_error_type_strs) ?
+ ia_check_ms_error_type_strs[err_type] : "unknown");
+ }
+
+ if (validation_bits & CHECK_VALID_MS_PCC)
+ print_bool("Processor Context Corrupt", pfx, check, CHECK_MS_PCC);
+
+ if (validation_bits & CHECK_VALID_MS_UNCORRECTED)
+ print_bool("Uncorrected", pfx, check, CHECK_MS_UNCORRECTED);
+
+ if (validation_bits & CHECK_VALID_MS_PRECISE_IP)
+ print_bool("Precise IP", pfx, check, CHECK_MS_PRECISE_IP);
+
+ if (validation_bits & CHECK_VALID_MS_RESTARTABLE_IP)
+ print_bool("Restartable IP", pfx, check, CHECK_MS_RESTARTABLE_IP);
+
+ if (validation_bits & CHECK_VALID_MS_OVERFLOW)
+ print_bool("Overflow", pfx, check, CHECK_MS_OVERFLOW);
+}
+
static void print_err_info(const char *pfx, enum err_types err_type, u64 check)
{
u16 validation_bits = CHECK_VALID_BITS(check);

printk("%sValidation Bits: 0x%04x\n", pfx, validation_bits);

+ /*
+ * The MS Check structure varies a lot from the others, so use a
+ * separate function for decoding.
+ */
if (err_type == ERR_TYPE_MS)
- return;
+ return print_err_info_ms(pfx, validation_bits, check);

if (validation_bits & CHECK_VALID_TRANS_TYPE) {
u8 trans_type = CHECK_TRANS_TYPE(check);
--
2.14.1

2018-02-23 20:06:28

by Yazen Ghannam

Subject: [PATCH 8/8] efi: Decode IA32/X64 Context Info structure

From: Yazen Ghannam <[email protected]>

Print the fields of the IA32/X64 Context Information structure.

Print the "Register Array" as raw values. Some context types are defined
in the UEFI spec, so more detailed decoded may be added in the future.

Based on UEFI 2.7 section N.2.4.2.2 IA32/X64 Processor Context
Information Structure.

Cc: <[email protected]> # 4.16.x
Signed-off-by: Yazen Ghannam <[email protected]>
---
drivers/firmware/efi/cper-x86.c | 55 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 55 insertions(+)

diff --git a/drivers/firmware/efi/cper-x86.c b/drivers/firmware/efi/cper-x86.c
index 02b1b424f537..bb6cef0b5e53 100644
--- a/drivers/firmware/efi/cper-x86.c
+++ b/drivers/firmware/efi/cper-x86.c
@@ -13,6 +13,7 @@
#define VALID_LAPIC_ID BIT_ULL(0)
#define VALID_CPUID_INFO BIT_ULL(1)
#define VALID_PROC_ERR_INFO_NUM(bits) ((bits & GENMASK_ULL(7, 2)) >> 2)
+#define VALID_PROC_CNXT_INFO_NUM(bits) ((bits & GENMASK_ULL(13, 8)) >> 8)

#define INFO_ERR_STRUCT_TYPE_CACHE \
GUID_INIT(0xA55701F5, 0xE3EF, 0x43DE, 0xAC, 0x72, 0x24, 0x9B, \
@@ -74,6 +75,9 @@
#define CHECK_MS_RESTARTABLE_IP BIT_ULL(22)
#define CHECK_MS_OVERFLOW BIT_ULL(23)

+#define CTX_TYPE_MSR 1
+#define CTX_TYPE_MMREG 7
+
enum err_types {
ERR_TYPE_CACHE = 0,
ERR_TYPE_TLB,
@@ -137,6 +141,17 @@ static const char * const ia_check_ms_error_type_strs[] = {
"Internal Unclassified",
};

+static const char * const ia_reg_ctx_strs[] = {
+ "Unclassified Data",
+ "MSR Registers (Machine Check and other MSRs)",
+ "32-bit Mode Execution Context",
+ "64-bit Mode Execution Context",
+ "FXSAVE Context",
+ "32-bit Mode Debug Registers (DR0-DR7)",
+ "64-bit Mode Debug Registers (DR0-DR7)",
+ "Memory Mapped Registers",
+};
+
static inline void print_bool(char *str, const char *pfx, u64 check, u64 bit)
{
printk("%s%s: %s\n", pfx, str, (check & bit) ? "true" : "false");
@@ -247,8 +262,10 @@ void cper_print_proc_ia(const char *pfx, const struct cper_sec_proc_ia *proc)
{
int i;
struct cper_ia_err_info *err_info;
+ struct cper_ia_proc_ctx *ctx_info;
char newpfx[64], infopfx[64];
enum err_types err_type;
+ unsigned int max_ctx_type = ARRAY_SIZE(ia_reg_ctx_strs) - 1;

printk("%sValidation Bits: 0x%016llx\n", pfx, proc->validation_bits);

@@ -313,4 +330,42 @@ void cper_print_proc_ia(const char *pfx, const struct cper_sec_proc_ia *proc)

err_info++;
}
+
+ ctx_info = (struct cper_ia_proc_ctx *)err_info;
+ for (i = 0; i < VALID_PROC_CNXT_INFO_NUM(proc->validation_bits); i++) {
+ int size = sizeof(*ctx_info) + ctx_info->reg_arr_size;
+ int groupsize = 4;
+
+ printk("%sContext Information Structure %d:\n", pfx, i);
+
+ if (ctx_info->reg_ctx_type > max_ctx_type) {
+ printk("%sInvalid Register Context Type: %d (max: %d)\n",
+ newpfx, ctx_info->reg_ctx_type, max_ctx_type);
+ goto next_ctx;
+ }
+
+ printk("%sRegister Context Type: %s\n", newpfx,
+ ia_reg_ctx_strs[ctx_info->reg_ctx_type]);
+
+ printk("%sRegister Array Size: 0x%04x\n", newpfx,
+ ctx_info->reg_arr_size);
+
+ if (ctx_info->reg_ctx_type == CTX_TYPE_MSR) {
+ groupsize = 8; /* MSRs are 8 bytes wide. */
+ printk("%sMSR Address: 0x%08x\n", newpfx,
+ ctx_info->msr_addr);
+ }
+
+ if (ctx_info->reg_ctx_type == CTX_TYPE_MMREG) {
+ printk("%sMM Register Address: 0x%016llx\n", newpfx,
+ ctx_info->mm_reg_addr);
+ }
+
+ printk("%sRegister Array:\n", newpfx);
+ print_hex_dump(newpfx, "", DUMP_PREFIX_OFFSET, 16, groupsize,
+ (ctx_info + 1), ctx_info->reg_arr_size, 0);
+
+next_ctx:
+ ctx_info = (struct cper_ia_proc_ctx *)((long)ctx_info + size);
+ }
}
--
2.14.1

2018-02-23 20:06:29

by Yazen Ghannam

Subject: [PATCH 1/8] efi: Fix IA32/X64 Processor Error Record definition

From: Yazen Ghannam <[email protected]>

Based on UEFI 2.7 Table 255. Processor Error Record, the "Local APIC_ID"
field is 8 bytes but Linux defines this field as 1 byte.

Fix this in the struct cper_sec_proc_ia definition.

Cc: <[email protected]> # 4.16.x
Signed-off-by: Yazen Ghannam <[email protected]>
---
include/linux/cper.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/cper.h b/include/linux/cper.h
index d14ef4e77c8a..4b5f8459b403 100644
--- a/include/linux/cper.h
+++ b/include/linux/cper.h
@@ -381,7 +381,7 @@ struct cper_sec_proc_generic {
/* IA32/X64 Processor Error Section */
struct cper_sec_proc_ia {
__u64 validation_bits;
- __u8 lapic_id;
+ __u64 lapic_id;
__u8 cpuid[48];
};

--
2.14.1

2018-02-23 20:06:50

by Yazen Ghannam

Subject: [PATCH 2/8] efi: Decode IA32/X64 Processor Error Section

From: Yazen Ghannam <[email protected]>

Recognize the IA32/X64 Processor Error Section.

Do the section decoding in a new "cper-x86.c" file and add this to the
Makefile depending on a new "UEFI_CPER_X86" config option.

Print the Local APIC ID and CPUID info from the Processor Error Record.

The "Processor Error Info" and "Processor Context" fields will be
decoded in following patches.

Based on UEFI 2.7 Table 255. Processor Error Record.

Cc: <[email protected]> # 4.16.x
Signed-off-by: Yazen Ghannam <[email protected]>
---
drivers/firmware/efi/Kconfig | 5 +++++
drivers/firmware/efi/Makefile | 2 ++
drivers/firmware/efi/cper-x86.c | 26 ++++++++++++++++++++++++++
drivers/firmware/efi/cper.c | 10 ++++++++++
include/linux/cper.h | 2 ++
5 files changed, 45 insertions(+)
create mode 100644 drivers/firmware/efi/cper-x86.c

diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig
index 3098410abad8..8b684147835d 100644
--- a/drivers/firmware/efi/Kconfig
+++ b/drivers/firmware/efi/Kconfig
@@ -174,6 +174,11 @@ config UEFI_CPER_ARM
depends on UEFI_CPER && ( ARM || ARM64 )
default y

+config UEFI_CPER_X86
+ bool
+ depends on UEFI_CPER && ( X86_32 || X86_64 )
+ default y
+
config EFI_DEV_PATH_PARSER
bool
depends on ACPI
diff --git a/drivers/firmware/efi/Makefile b/drivers/firmware/efi/Makefile
index cb805374f4bc..7cf8d75ebe51 100644
--- a/drivers/firmware/efi/Makefile
+++ b/drivers/firmware/efi/Makefile
@@ -31,3 +31,5 @@ obj-$(CONFIG_ARM) += $(arm-obj-y)
obj-$(CONFIG_ARM64) += $(arm-obj-y)
obj-$(CONFIG_EFI_CAPSULE_LOADER) += capsule-loader.o
obj-$(CONFIG_UEFI_CPER_ARM) += cper-arm.o
+
+obj-$(CONFIG_UEFI_CPER_X86) += cper-x86.o
diff --git a/drivers/firmware/efi/cper-x86.c b/drivers/firmware/efi/cper-x86.c
new file mode 100644
index 000000000000..b50ee3cdf637
--- /dev/null
+++ b/drivers/firmware/efi/cper-x86.c
@@ -0,0 +1,26 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright (C) 2018, Advanced Micro Devices, Inc.
+// Copyright (C) 2018, Yazen Ghannam <[email protected]>
+
+#include <linux/cper.h>
+
+/*
+ * We don't need a "CPER_IA" prefix since these are all locally defined.
+ * This will save us a lot of line space.
+ */
+#define VALID_LAPIC_ID BIT_ULL(0)
+#define VALID_CPUID_INFO BIT_ULL(1)
+
+void cper_print_proc_ia(const char *pfx, const struct cper_sec_proc_ia *proc)
+{
+ printk("%sValidation Bits: 0x%016llx\n", pfx, proc->validation_bits);
+
+ if (proc->validation_bits & VALID_LAPIC_ID)
+ printk("%sLocal APIC_ID: 0x%llx\n", pfx, proc->lapic_id);
+
+ if (proc->validation_bits & VALID_CPUID_INFO) {
+ printk("%sCPUID Info:\n", pfx);
+ print_hex_dump(pfx, "", DUMP_PREFIX_OFFSET, 16, 4, proc->cpuid,
+ sizeof(proc->cpuid), 0);
+ }
+}
diff --git a/drivers/firmware/efi/cper.c b/drivers/firmware/efi/cper.c
index c165933ebf38..5a59b582c9aa 100644
--- a/drivers/firmware/efi/cper.c
+++ b/drivers/firmware/efi/cper.c
@@ -469,6 +469,16 @@ cper_estatus_print_section(const char *pfx, struct acpi_hest_generic_data *gdata
cper_print_proc_arm(newpfx, arm_err);
else
goto err_section_too_small;
+#endif
+#if defined(CONFIG_UEFI_CPER_X86)
+ } else if (guid_equal(sec_type, &CPER_SEC_PROC_IA)) {
+ struct cper_sec_proc_ia *ia_err = acpi_hest_get_payload(gdata);
+
+ printk("%ssection_type: IA32/X64 processor error\n", newpfx);
+ if (gdata->error_data_length >= sizeof(*ia_err))
+ cper_print_proc_ia(newpfx, ia_err);
+ else
+ goto err_section_too_small;
#endif
} else {
const void *err = acpi_hest_get_payload(gdata);
diff --git a/include/linux/cper.h b/include/linux/cper.h
index 4b5f8459b403..9c703a0abe6e 100644
--- a/include/linux/cper.h
+++ b/include/linux/cper.h
@@ -551,5 +551,7 @@ const char *cper_mem_err_unpack(struct trace_seq *,
struct cper_mem_err_compact *);
void cper_print_proc_arm(const char *pfx,
const struct cper_sec_proc_arm *proc);
+void cper_print_proc_ia(const char *pfx,
+ const struct cper_sec_proc_ia *proc);

#endif
--
2.14.1

2018-02-23 20:07:46

by Yazen Ghannam

Subject: [PATCH 3/8] efi: Decode IA32/X64 Processor Error Info Structure

From: Yazen Ghannam <[email protected]>

Print the fields in the IA32/X64 Processor Error Info Structure.

Based on UEFI 2.7 Table 256. IA32/X64 Processor Error Information
Structure.

Cc: <[email protected]> # 4.16.x
Signed-off-by: Yazen Ghannam <[email protected]>
---
drivers/firmware/efi/cper-x86.c | 53 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 53 insertions(+)

diff --git a/drivers/firmware/efi/cper-x86.c b/drivers/firmware/efi/cper-x86.c
index b50ee3cdf637..9d608f742c98 100644
--- a/drivers/firmware/efi/cper-x86.c
+++ b/drivers/firmware/efi/cper-x86.c
@@ -4,15 +4,28 @@

#include <linux/cper.h>

+#define INDENT_SP " "
+
/*
* We don't need a "CPER_IA" prefix since these are all locally defined.
* This will save us a lot of line space.
*/
#define VALID_LAPIC_ID BIT_ULL(0)
#define VALID_CPUID_INFO BIT_ULL(1)
+#define VALID_PROC_ERR_INFO_NUM(bits) ((bits & GENMASK_ULL(7, 2)) >> 2)
+
+#define INFO_VALID_CHECK_INFO BIT_ULL(0)
+#define INFO_VALID_TARGET_ID BIT_ULL(1)
+#define INFO_VALID_REQUESTOR_ID BIT_ULL(2)
+#define INFO_VALID_RESPONDER_ID BIT_ULL(3)
+#define INFO_VALID_IP BIT_ULL(4)

void cper_print_proc_ia(const char *pfx, const struct cper_sec_proc_ia *proc)
{
+ int i;
+ struct cper_ia_err_info *err_info;
+ char newpfx[64];
+
printk("%sValidation Bits: 0x%016llx\n", pfx, proc->validation_bits);

if (proc->validation_bits & VALID_LAPIC_ID)
@@ -23,4 +36,44 @@ void cper_print_proc_ia(const char *pfx, const struct cper_sec_proc_ia *proc)
print_hex_dump(pfx, "", DUMP_PREFIX_OFFSET, 16, 4, proc->cpuid,
sizeof(proc->cpuid), 0);
}
+
+ snprintf(newpfx, sizeof(newpfx), "%s%s", pfx, INDENT_SP);
+
+ err_info = (struct cper_ia_err_info *)(proc + 1);
+ for (i = 0; i < VALID_PROC_ERR_INFO_NUM(proc->validation_bits); i++) {
+ printk("%sError Information Structure %d:\n", pfx, i);
+
+ printk("%sError Structure Type: %pUl\n", newpfx,
+ &err_info->err_type);
+
+ printk("%sValidation Bits: 0x%016llx\n",
+ newpfx, err_info->validation_bits);
+
+ if (err_info->validation_bits & INFO_VALID_CHECK_INFO) {
+ printk("%sCheck Information: 0x%016llx\n", newpfx,
+ err_info->check_info);
+ }
+
+ if (err_info->validation_bits & INFO_VALID_TARGET_ID) {
+ printk("%sTarget Identifier: 0x%016llx\n",
+ newpfx, err_info->target_id);
+ }
+
+ if (err_info->validation_bits & INFO_VALID_REQUESTOR_ID) {
+ printk("%sRequestor Identifier: 0x%016llx\n",
+ newpfx, err_info->requestor_id);
+ }
+
+ if (err_info->validation_bits & INFO_VALID_RESPONDER_ID) {
+ printk("%sResponder Identifier: 0x%016llx\n",
+ newpfx, err_info->responder_id);
+ }
+
+ if (err_info->validation_bits & INFO_VALID_IP) {
+ printk("%sInstruction Pointer: 0x%016llx\n",
+ newpfx, err_info->ip);
+ }
+
+ err_info++;
+ }
}
--
2.14.1

2018-02-24 16:41:27

by Ard Biesheuvel

Subject: Re: [PATCH 2/8] efi: Decode IA32/X64 Processor Error Section

On 23 February 2018 at 20:03, Yazen Ghannam <[email protected]> wrote:
> From: Yazen Ghannam <[email protected]>
>
> Recognize the IA32/X64 Processor Error Section.
>
> Do the section decoding in a new "cper-x86.c" file and add this to the
> Makefile depending on a new "UEFI_CPER_X86" config option.
>
> Print the Local APIC ID and CPUID info from the Processor Error Record.
>
> The "Processor Error Info" and "Processor Context" fields will be
> decoded in following patches.
>
> Based on UEFI 2.7 Table 255. Processor Error Record.
>
> Cc: <[email protected]> # 4.16.x
> Signed-off-by: Yazen Ghannam <[email protected]>
> ---
> drivers/firmware/efi/Kconfig | 5 +++++
> drivers/firmware/efi/Makefile | 2 ++
> drivers/firmware/efi/cper-x86.c | 26 ++++++++++++++++++++++++++
> drivers/firmware/efi/cper.c | 10 ++++++++++
> include/linux/cper.h | 2 ++
> 5 files changed, 45 insertions(+)
> create mode 100644 drivers/firmware/efi/cper-x86.c
>
> diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig
> index 3098410abad8..8b684147835d 100644
> --- a/drivers/firmware/efi/Kconfig
> +++ b/drivers/firmware/efi/Kconfig
> @@ -174,6 +174,11 @@ config UEFI_CPER_ARM
> depends on UEFI_CPER && ( ARM || ARM64 )
> default y
>
> +config UEFI_CPER_X86
> + bool
> + depends on UEFI_CPER && ( X86_32 || X86_64 )

Just 'UEFI_CPER && X86' should be sufficient, no?

> + default y
> +
> config EFI_DEV_PATH_PARSER
> bool
> depends on ACPI
> diff --git a/drivers/firmware/efi/Makefile b/drivers/firmware/efi/Makefile
> index cb805374f4bc..7cf8d75ebe51 100644
> --- a/drivers/firmware/efi/Makefile
> +++ b/drivers/firmware/efi/Makefile
> @@ -31,3 +31,5 @@ obj-$(CONFIG_ARM) += $(arm-obj-y)
> obj-$(CONFIG_ARM64) += $(arm-obj-y)
> obj-$(CONFIG_EFI_CAPSULE_LOADER) += capsule-loader.o
> obj-$(CONFIG_UEFI_CPER_ARM) += cper-arm.o
> +

Spurious newline

> +obj-$(CONFIG_UEFI_CPER_X86) += cper-x86.o
> diff --git a/drivers/firmware/efi/cper-x86.c b/drivers/firmware/efi/cper-x86.c
> new file mode 100644
> index 000000000000..b50ee3cdf637
> --- /dev/null
> +++ b/drivers/firmware/efi/cper-x86.c
> @@ -0,0 +1,26 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (C) 2018, Advanced Micro Devices, Inc.
> +// Copyright (C) 2018, Yazen Ghannam <[email protected]>
> +

Do you really own the copyright for code that you write on behalf of
your employer?

> +#include <linux/cper.h>
> +
> +/*
> + * We don't need a "CPER_IA" prefix since these are all locally defined.
> + * This will save us a lot of line space.
> + */
> +#define VALID_LAPIC_ID BIT_ULL(0)
> +#define VALID_CPUID_INFO BIT_ULL(1)
> +
> +void cper_print_proc_ia(const char *pfx, const struct cper_sec_proc_ia *proc)
> +{
> + printk("%sValidation Bits: 0x%016llx\n", pfx, proc->validation_bits);
> +
> + if (proc->validation_bits & VALID_LAPIC_ID)
> + printk("%sLocal APIC_ID: 0x%llx\n", pfx, proc->lapic_id);
> +
> + if (proc->validation_bits & VALID_CPUID_INFO) {
> + printk("%sCPUID Info:\n", pfx);
> + print_hex_dump(pfx, "", DUMP_PREFIX_OFFSET, 16, 4, proc->cpuid,
> + sizeof(proc->cpuid), 0);
> + }
> +}
> diff --git a/drivers/firmware/efi/cper.c b/drivers/firmware/efi/cper.c
> index c165933ebf38..5a59b582c9aa 100644
> --- a/drivers/firmware/efi/cper.c
> +++ b/drivers/firmware/efi/cper.c
> @@ -469,6 +469,16 @@ cper_estatus_print_section(const char *pfx, struct acpi_hest_generic_data *gdata
> cper_print_proc_arm(newpfx, arm_err);
> else
> goto err_section_too_small;
> +#endif
> +#if defined(CONFIG_UEFI_CPER_X86)
> + } else if (guid_equal(sec_type, &CPER_SEC_PROC_IA)) {
> + struct cper_sec_proc_ia *ia_err = acpi_hest_get_payload(gdata);
> +
> + printk("%ssection_type: IA32/X64 processor error\n", newpfx);
> + if (gdata->error_data_length >= sizeof(*ia_err))
> + cper_print_proc_ia(newpfx, ia_err);
> + else
> + goto err_section_too_small;
> #endif
> } else {
> const void *err = acpi_hest_get_payload(gdata);
> diff --git a/include/linux/cper.h b/include/linux/cper.h
> index 4b5f8459b403..9c703a0abe6e 100644
> --- a/include/linux/cper.h
> +++ b/include/linux/cper.h
> @@ -551,5 +551,7 @@ const char *cper_mem_err_unpack(struct trace_seq *,
> struct cper_mem_err_compact *);
> void cper_print_proc_arm(const char *pfx,
> const struct cper_sec_proc_arm *proc);
> +void cper_print_proc_ia(const char *pfx,
> + const struct cper_sec_proc_ia *proc);
>
> #endif
> --
> 2.14.1
>

2018-02-24 16:41:35

by Ard Biesheuvel

Subject: Re: [PATCH 3/8] efi: Decode IA32/X64 Processor Error Info Structure

On 23 February 2018 at 20:03, Yazen Ghannam <[email protected]> wrote:
> From: Yazen Ghannam <[email protected]>
>
> Print the fields in the IA32/X64 Processor Error Info Structure.
>
> Based on UEFI 2.7 Table 256. IA32/X64 Processor Error Information
> Structure.
>
> Cc: <[email protected]> # 4.16.x
> Signed-off-by: Yazen Ghannam <[email protected]>
> ---
> drivers/firmware/efi/cper-x86.c | 53 +++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 53 insertions(+)
>
> diff --git a/drivers/firmware/efi/cper-x86.c b/drivers/firmware/efi/cper-x86.c
> index b50ee3cdf637..9d608f742c98 100644
> --- a/drivers/firmware/efi/cper-x86.c
> +++ b/drivers/firmware/efi/cper-x86.c
> @@ -4,15 +4,28 @@
>
> #include <linux/cper.h>
>
> +#define INDENT_SP " "
> +
> /*
> * We don't need a "CPER_IA" prefix since these are all locally defined.
> * This will save us a lot of line space.
> */
> #define VALID_LAPIC_ID BIT_ULL(0)
> #define VALID_CPUID_INFO BIT_ULL(1)
> +#define VALID_PROC_ERR_INFO_NUM(bits) ((bits & GENMASK_ULL(7, 2)) >> 2)
> +

Parens around 'bits' please

> +#define INFO_VALID_CHECK_INFO BIT_ULL(0)
> +#define INFO_VALID_TARGET_ID BIT_ULL(1)
> +#define INFO_VALID_REQUESTOR_ID BIT_ULL(2)
> +#define INFO_VALID_RESPONDER_ID BIT_ULL(3)
> +#define INFO_VALID_IP BIT_ULL(4)
>
> void cper_print_proc_ia(const char *pfx, const struct cper_sec_proc_ia *proc)
> {
> + int i;
> + struct cper_ia_err_info *err_info;
> + char newpfx[64];
> +
> printk("%sValidation Bits: 0x%016llx\n", pfx, proc->validation_bits);
>
> if (proc->validation_bits & VALID_LAPIC_ID)
> @@ -23,4 +36,44 @@ void cper_print_proc_ia(const char *pfx, const struct cper_sec_proc_ia *proc)
> print_hex_dump(pfx, "", DUMP_PREFIX_OFFSET, 16, 4, proc->cpuid,
> sizeof(proc->cpuid), 0);
> }
> +
> + snprintf(newpfx, sizeof(newpfx), "%s%s", pfx, INDENT_SP);
> +
> + err_info = (struct cper_ia_err_info *)(proc + 1);
> + for (i = 0; i < VALID_PROC_ERR_INFO_NUM(proc->validation_bits); i++) {
> + printk("%sError Information Structure %d:\n", pfx, i);
> +
> + printk("%sError Structure Type: %pUl\n", newpfx,
> + &err_info->err_type);
> +

The indentation is a bit awkward here. Could you please align followup
lines with the character following the ( on the first line?

> + printk("%sValidation Bits: 0x%016llx\n",
> + newpfx, err_info->validation_bits);
> +
> + if (err_info->validation_bits & INFO_VALID_CHECK_INFO) {
> + printk("%sCheck Information: 0x%016llx\n", newpfx,
> + err_info->check_info);
> + }
> +
> + if (err_info->validation_bits & INFO_VALID_TARGET_ID) {
> + printk("%sTarget Identifier: 0x%016llx\n",
> + newpfx, err_info->target_id);
> + }
> +
> + if (err_info->validation_bits & INFO_VALID_REQUESTOR_ID) {
> + printk("%sRequestor Identifier: 0x%016llx\n",
> + newpfx, err_info->requestor_id);
> + }
> +
> + if (err_info->validation_bits & INFO_VALID_RESPONDER_ID) {
> + printk("%sResponder Identifier: 0x%016llx\n",
> + newpfx, err_info->responder_id);
> + }
> +
> + if (err_info->validation_bits & INFO_VALID_IP) {
> + printk("%sInstruction Pointer: 0x%016llx\n",
> + newpfx, err_info->ip);
> + }
> +
> + err_info++;
> + }
> }
> --
> 2.14.1
>

2018-02-24 16:43:24

by Ard Biesheuvel

Subject: Re: [PATCH 4/8] efi: Decode UEFI-defined IA32/X64 Error Structure GUIDs

On 23 February 2018 at 20:03, Yazen Ghannam <[email protected]> wrote:
> From: Yazen Ghannam <[email protected]>
>
> For easier handling, match the known IA32/X64 error structure GUIDs to
> enums.
>
> Also, print out the name of the matching Error Structure Type.
>
> GUIDs taken from UEFI 2.7 section N.2.4.2.1 IA32/X64 Processor Error
> Information Structure.
>
> Cc: <[email protected]> # 4.16.x
> Signed-off-by: Yazen Ghannam <[email protected]>
> ---
> drivers/firmware/efi/cper-x86.c | 41 +++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 41 insertions(+)
>
> diff --git a/drivers/firmware/efi/cper-x86.c b/drivers/firmware/efi/cper-x86.c
> index 9d608f742c98..3b86223bdb67 100644
> --- a/drivers/firmware/efi/cper-x86.c
> +++ b/drivers/firmware/efi/cper-x86.c
> @@ -14,17 +14,53 @@
> #define VALID_CPUID_INFO BIT_ULL(1)
> #define VALID_PROC_ERR_INFO_NUM(bits) ((bits & GENMASK_ULL(7, 2)) >> 2)
>
> +#define INFO_ERR_STRUCT_TYPE_CACHE \
> + GUID_INIT(0xA55701F5, 0xE3EF, 0x43DE, 0xAC, 0x72, 0x24, 0x9B, \
> + 0x57, 0x3F, 0xAD, 0x2C)
> +#define INFO_ERR_STRUCT_TYPE_TLB \
> + GUID_INIT(0xFC06B535, 0x5E1F, 0x4562, 0x9F, 0x25, 0x0A, 0x3B, \
> + 0x9A, 0xDB, 0x63, 0xC3)
> +#define INFO_ERR_STRUCT_TYPE_BUS \
> + GUID_INIT(0x1CF3F8B3, 0xC5B1, 0x49a2, 0xAA, 0x59, 0x5E, 0xEF, \
> + 0x92, 0xFF, 0xA6, 0x3C)
> +#define INFO_ERR_STRUCT_TYPE_MS \
> + GUID_INIT(0x48AB7F57, 0xDC34, 0x4f6c, 0xA7, 0xD3, 0xB0, 0xB5, \
> + 0xB0, 0xA7, 0x43, 0x14)
> +
> #define INFO_VALID_CHECK_INFO BIT_ULL(0)
> #define INFO_VALID_TARGET_ID BIT_ULL(1)
> #define INFO_VALID_REQUESTOR_ID BIT_ULL(2)
> #define INFO_VALID_RESPONDER_ID BIT_ULL(3)
> #define INFO_VALID_IP BIT_ULL(4)
>
> +enum err_types {
> + ERR_TYPE_CACHE = 0,
> + ERR_TYPE_TLB,
> + ERR_TYPE_BUS,
> + ERR_TYPE_MS,
> + N_ERR_TYPES
> +};
> +
> +static enum err_types cper_get_err_type(const guid_t *err_type)
> +{
> + if (guid_equal(err_type, &INFO_ERR_STRUCT_TYPE_CACHE))
> + return ERR_TYPE_CACHE;
> + else if (guid_equal(err_type, &INFO_ERR_STRUCT_TYPE_TLB))
> + return ERR_TYPE_TLB;
> + else if (guid_equal(err_type, &INFO_ERR_STRUCT_TYPE_BUS))
> + return ERR_TYPE_BUS;
> + else if (guid_equal(err_type, &INFO_ERR_STRUCT_TYPE_MS))
> + return ERR_TYPE_MS;
> + else
> + return N_ERR_TYPES;
> +}
> +
> void cper_print_proc_ia(const char *pfx, const struct cper_sec_proc_ia *proc)
> {
> int i;
> struct cper_ia_err_info *err_info;
> char newpfx[64];
> + enum err_types err_type;
>

Can you make this an u8 please? The signedness of an enum is not well
defined, and you only check the upper bound below.

> printk("%sValidation Bits: 0x%016llx\n", pfx, proc->validation_bits);
>
> @@ -46,6 +82,11 @@ void cper_print_proc_ia(const char *pfx, const struct cper_sec_proc_ia *proc)
> printk("%sError Structure Type: %pUl\n", newpfx,
> &err_info->err_type);
>
> + err_type = cper_get_err_type(&err_info->err_type);
> + printk("%sError Structure Type: %s\n", newpfx,
> + err_type < ARRAY_SIZE(cper_proc_error_type_strs) ?
> + cper_proc_error_type_strs[err_type] : "unknown");
> +
> printk("%sValidation Bits: 0x%016llx\n",
> newpfx, err_info->validation_bits);
>
> --
> 2.14.1
>

2018-02-24 16:44:24

by Ard Biesheuvel

Subject: Re: [PATCH 5/8] efi: Decode IA32/X64 Cache, TLB, and Bus Check structures

On 23 February 2018 at 20:03, Yazen Ghannam <[email protected]> wrote:
> From: Yazen Ghannam <[email protected]>
>
> Print the common fields of the Cache, TLB, and Bus check structures.The
> fields of these three check types are the same except for a few more
> fields in the Bus check structure. The remaining Bus check structure
> fields will be decoded in a following patch.
>
> Based on UEFI 2.7,
> Table 257. IA32/X64 Cache Check Structure
> Table 258. IA32/X64 TLB Check Structure
> Table 259. IA32/X64 Bus Check Structure
>
> Cc: <[email protected]> # 4.16.x
> Signed-off-by: Yazen Ghannam <[email protected]>
> ---
> drivers/firmware/efi/cper-x86.c | 101 +++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 100 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/firmware/efi/cper-x86.c b/drivers/firmware/efi/cper-x86.c
> index 3b86223bdb67..75f664043076 100644
> --- a/drivers/firmware/efi/cper-x86.c
> +++ b/drivers/firmware/efi/cper-x86.c
> @@ -33,6 +33,25 @@
> #define INFO_VALID_RESPONDER_ID BIT_ULL(3)
> #define INFO_VALID_IP BIT_ULL(4)
>
> +#define CHECK_VALID_TRANS_TYPE BIT_ULL(0)
> +#define CHECK_VALID_OPERATION BIT_ULL(1)
> +#define CHECK_VALID_LEVEL BIT_ULL(2)
> +#define CHECK_VALID_PCC BIT_ULL(3)
> +#define CHECK_VALID_UNCORRECTED BIT_ULL(4)
> +#define CHECK_VALID_PRECISE_IP BIT_ULL(5)
> +#define CHECK_VALID_RESTARTABLE_IP BIT_ULL(6)
> +#define CHECK_VALID_OVERFLOW BIT_ULL(7)
> +
> +#define CHECK_VALID_BITS(check) ((check & GENMASK_ULL(15, 0)))
> +#define CHECK_TRANS_TYPE(check) ((check & GENMASK_ULL(17, 16)) >> 16)
> +#define CHECK_OPERATION(check) ((check & GENMASK_ULL(21, 18)) >> 18)
> +#define CHECK_LEVEL(check) ((check & GENMASK_ULL(24, 22)) >> 22)

Parens around 'check' please

> +#define CHECK_PCC BIT_ULL(25)
> +#define CHECK_UNCORRECTED BIT_ULL(26)
> +#define CHECK_PRECISE_IP BIT_ULL(27)
> +#define CHECK_RESTARTABLE_IP BIT_ULL(28)
> +#define CHECK_OVERFLOW BIT_ULL(29)
> +
> enum err_types {
> ERR_TYPE_CACHE = 0,
> ERR_TYPE_TLB,
> @@ -55,11 +74,83 @@ static enum err_types cper_get_err_type(const guid_t *err_type)
> return N_ERR_TYPES;
> }
>
> +static const char * const ia_check_trans_type_strs[] = {
> + "Instruction",
> + "Data Access",
> + "Generic",
> +};
> +
> +static const char * const ia_check_op_strs[] = {
> + "generic error",
> + "generic read",
> + "generic write",
> + "data read",
> + "data write",
> + "instruction fetch",
> + "prefetch",
> + "eviction",
> + "snoop",
> +};
> +
> +static inline void print_bool(char *str, const char *pfx, u64 check, u64 bit)
> +{
> + printk("%s%s: %s\n", pfx, str, (check & bit) ? "true" : "false");
> +}
> +
> +static void print_err_info(const char *pfx, enum err_types err_type, u64 check)

u8 for err_type please

> +{
> + u16 validation_bits = CHECK_VALID_BITS(check);
> +
> + printk("%sValidation Bits: 0x%04x\n", pfx, validation_bits);
> +
> + if (err_type == ERR_TYPE_MS)
> + return;
> +
> + if (validation_bits & CHECK_VALID_TRANS_TYPE) {
> + u8 trans_type = CHECK_TRANS_TYPE(check);
> +
> + printk("%sTransaction Type: %u, %s\n", pfx, trans_type,
> + trans_type < ARRAY_SIZE(ia_check_trans_type_strs) ?
> + ia_check_trans_type_strs[trans_type] : "unknown");
> + }
> +
> + if (validation_bits & CHECK_VALID_OPERATION) {
> + u8 op = CHECK_OPERATION(check);
> +
> + /*
> + * CACHE has more operation types than TLB or BUS, though the
> + * name and the order are the same.
> + */
> + u8 max_ops = (err_type == ERR_TYPE_CACHE) ? 9 : 7;
> +
> + printk("%sOperation: %u, %s\n", pfx, op,
> + op < max_ops ? ia_check_op_strs[op] : "unknown");
> + }
> +
> + if (validation_bits & CHECK_VALID_LEVEL)
> + printk("%sLevel: %llu\n", pfx, CHECK_LEVEL(check));
> +
> + if (validation_bits & CHECK_VALID_PCC)
> + print_bool("Processor Context Corrupt", pfx, check, CHECK_PCC);
> +
> + if (validation_bits & CHECK_VALID_UNCORRECTED)
> + print_bool("Uncorrected", pfx, check, CHECK_UNCORRECTED);
> +
> + if (validation_bits & CHECK_VALID_PRECISE_IP)
> + print_bool("Precise IP", pfx, check, CHECK_PRECISE_IP);
> +
> + if (validation_bits & CHECK_VALID_RESTARTABLE_IP)
> + print_bool("Restartable IP", pfx, check, CHECK_RESTARTABLE_IP);
> +
> + if (validation_bits & CHECK_VALID_OVERFLOW)
> + print_bool("Overflow", pfx, check, CHECK_OVERFLOW);
> +}
> +
> void cper_print_proc_ia(const char *pfx, const struct cper_sec_proc_ia *proc)
> {
> int i;
> struct cper_ia_err_info *err_info;
> - char newpfx[64];
> + char newpfx[64], infopfx[64];
> enum err_types err_type;
>
> printk("%sValidation Bits: 0x%016llx\n", pfx, proc->validation_bits);
> @@ -93,6 +184,14 @@ void cper_print_proc_ia(const char *pfx, const struct cper_sec_proc_ia *proc)
> if (err_info->validation_bits & INFO_VALID_CHECK_INFO) {
> printk("%sCheck Information: 0x%016llx\n", newpfx,
> err_info->check_info);
> +
> + if (err_type < N_ERR_TYPES) {
> + snprintf(infopfx, sizeof(infopfx), "%s%s",
> + newpfx, INDENT_SP);
> +
> + print_err_info(infopfx, err_type,
> + err_info->check_info);
> + }
> }
>
> if (err_info->validation_bits & INFO_VALID_TARGET_ID) {
> --
> 2.14.1
>

2018-02-24 16:45:16

by Ard Biesheuvel

Subject: Re: [PATCH 6/8] efi: Decode additional IA32/X64 Bus Check fields

On 23 February 2018 at 20:03, Yazen Ghannam <[email protected]> wrote:
> From: Yazen Ghannam <[email protected]>
>
> The "Participation Type", "Time Out", and "Address Space" fields are
> unique to the IA32/X64 Bus Check structure. Print these fields.
>
> Based on UEFI 2.7 Table 259. IA32/X64 Bus Check Structure
>
> Cc: <[email protected]> # 4.16.x
> Signed-off-by: Yazen Ghannam <[email protected]>
> ---
> drivers/firmware/efi/cper-x86.c | 44 +++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 44 insertions(+)
>
> diff --git a/drivers/firmware/efi/cper-x86.c b/drivers/firmware/efi/cper-x86.c
> index 75f664043076..df4cf6221d8e 100644
> --- a/drivers/firmware/efi/cper-x86.c
> +++ b/drivers/firmware/efi/cper-x86.c
> @@ -42,6 +42,10 @@
> #define CHECK_VALID_RESTARTABLE_IP BIT_ULL(6)
> #define CHECK_VALID_OVERFLOW BIT_ULL(7)
>
> +#define CHECK_VALID_BUS_PART_TYPE BIT_ULL(8)
> +#define CHECK_VALID_BUS_TIME_OUT BIT_ULL(9)
> +#define CHECK_VALID_BUS_ADDR_SPACE BIT_ULL(10)
> +
> #define CHECK_VALID_BITS(check) ((check & GENMASK_ULL(15, 0)))
> #define CHECK_TRANS_TYPE(check) ((check & GENMASK_ULL(17, 16)) >> 16)
> #define CHECK_OPERATION(check) ((check & GENMASK_ULL(21, 18)) >> 18)
> @@ -52,6 +56,10 @@
> #define CHECK_RESTARTABLE_IP BIT_ULL(28)
> #define CHECK_OVERFLOW BIT_ULL(29)
>
> +#define CHECK_BUS_PART_TYPE(check) ((check & GENMASK_ULL(31, 30)) >> 30)
> +#define CHECK_BUS_TIME_OUT BIT_ULL(32)
> +#define CHECK_BUS_ADDR_SPACE(check) ((check & GENMASK_ULL(34, 33)) >> 33)
> +

Parens around 'check' please

> enum err_types {
> ERR_TYPE_CACHE = 0,
> ERR_TYPE_TLB,
> @@ -92,6 +100,20 @@ static const char * const ia_check_op_strs[] = {
> "snoop",
> };
>
> +static const char * const ia_check_bus_part_type_strs[] = {
> + "Local Processor originated request",
> + "Local Processor responded to request",
> + "Local Processor observed",
> + "Generic",
> +};
> +
> +static const char * const ia_check_bus_addr_space_strs[] = {
> + "Memory Access",
> + "Reserved",
> + "I/O",
> + "Other Transaction",
> +};
> +
> static inline void print_bool(char *str, const char *pfx, u64 check, u64 bit)
> {
> printk("%s%s: %s\n", pfx, str, (check & bit) ? "true" : "false");
> @@ -144,6 +166,28 @@ static void print_err_info(const char *pfx, enum err_types err_type, u64 check)
>
> if (validation_bits & CHECK_VALID_OVERFLOW)
> print_bool("Overflow", pfx, check, CHECK_OVERFLOW);
> +
> + if (err_type != ERR_TYPE_BUS)
> + return;
> +
> + if (validation_bits & CHECK_VALID_BUS_PART_TYPE) {
> + u8 part_type = CHECK_BUS_PART_TYPE(check);
> +
> + printk("%sParticipation Type: %u, %s\n", pfx, part_type,
> + part_type < ARRAY_SIZE(ia_check_bus_part_type_strs) ?
> + ia_check_bus_part_type_strs[part_type] : "unknown");
> + }
> +
> + if (validation_bits & CHECK_VALID_BUS_TIME_OUT)
> + print_bool("Time Out", pfx, check, CHECK_BUS_TIME_OUT);
> +
> + if (validation_bits & CHECK_VALID_BUS_ADDR_SPACE) {
> + u8 addr_space = CHECK_BUS_ADDR_SPACE(check);
> +
> + printk("%sAddress Space: %u, %s\n", pfx, addr_space,
> + addr_space < ARRAY_SIZE(ia_check_bus_addr_space_strs) ?
> + ia_check_bus_addr_space_strs[addr_space] : "unknown");
> + }
> }
>
> void cper_print_proc_ia(const char *pfx, const struct cper_sec_proc_ia *proc)
> --
> 2.14.1
>

2018-02-24 16:46:04

by Ard Biesheuvel

Subject: Re: [PATCH 7/8] efi: Decode IA32/X64 MS Check structure

On 23 February 2018 at 20:03, Yazen Ghannam <[email protected]> wrote:
> From: Yazen Ghannam <[email protected]>
>
> The IA32/X64 MS Check structure varies from the other Check structures
> in the the bit positions of its fields, and it includes an additional
> "Error Type" field.
>
> Decode the MS Check structure in a separate function.
>
> Based on UEFI 2.7 Table 260. IA32/X64 MS Check Field Description.
>
> Cc: <[email protected]> # 4.16.x
> Signed-off-by: Yazen Ghannam <[email protected]>
> ---
> drivers/firmware/efi/cper-x86.c | 55 ++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 54 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/firmware/efi/cper-x86.c b/drivers/firmware/efi/cper-x86.c
> index df4cf6221d8e..02b1b424f537 100644
> --- a/drivers/firmware/efi/cper-x86.c
> +++ b/drivers/firmware/efi/cper-x86.c
> @@ -60,6 +60,20 @@
> #define CHECK_BUS_TIME_OUT BIT_ULL(32)
> #define CHECK_BUS_ADDR_SPACE(check) ((check & GENMASK_ULL(34, 33)) >> 33)
>
> +#define CHECK_VALID_MS_ERR_TYPE BIT_ULL(0)
> +#define CHECK_VALID_MS_PCC BIT_ULL(1)
> +#define CHECK_VALID_MS_UNCORRECTED BIT_ULL(2)
> +#define CHECK_VALID_MS_PRECISE_IP BIT_ULL(3)
> +#define CHECK_VALID_MS_RESTARTABLE_IP BIT_ULL(4)
> +#define CHECK_VALID_MS_OVERFLOW BIT_ULL(5)
> +
> +#define CHECK_MS_ERR_TYPE(check) ((check & GENMASK_ULL(18, 16)) >> 16)

Parens

> +#define CHECK_MS_PCC BIT_ULL(19)
> +#define CHECK_MS_UNCORRECTED BIT_ULL(20)
> +#define CHECK_MS_PRECISE_IP BIT_ULL(21)
> +#define CHECK_MS_RESTARTABLE_IP BIT_ULL(22)
> +#define CHECK_MS_OVERFLOW BIT_ULL(23)
> +
> enum err_types {
> ERR_TYPE_CACHE = 0,
> ERR_TYPE_TLB,
> @@ -114,19 +128,58 @@ static const char * const ia_check_bus_addr_space_strs[] = {
> "Other Transaction",
> };
>
> +static const char * const ia_check_ms_error_type_strs[] = {
> + "No Error",
> + "Unclassified",
> + "Microcode ROM Parity Error",
> + "External Error",
> + "FRC Error",
> + "Internal Unclassified",
> +};
> +
> static inline void print_bool(char *str, const char *pfx, u64 check, u64 bit)
> {
> printk("%s%s: %s\n", pfx, str, (check & bit) ? "true" : "false");
> }
>
> +static void print_err_info_ms(const char *pfx, u16 validation_bits, u64 check)
> +{
> + if (validation_bits & CHECK_VALID_MS_ERR_TYPE) {
> + u8 err_type = CHECK_MS_ERR_TYPE(check);
> +
> + printk("%sError Type: %u, %s\n", pfx, err_type,
> + err_type < ARRAY_SIZE(ia_check_ms_error_type_strs) ?
> + ia_check_ms_error_type_strs[err_type] : "unknown");
> + }
> +
> + if (validation_bits & CHECK_VALID_MS_PCC)
> + print_bool("Processor Context Corrupt", pfx, check, CHECK_MS_PCC);
> +
> + if (validation_bits & CHECK_VALID_MS_UNCORRECTED)
> + print_bool("Uncorrected", pfx, check, CHECK_MS_UNCORRECTED);
> +
> + if (validation_bits & CHECK_VALID_MS_PRECISE_IP)
> + print_bool("Precise IP", pfx, check, CHECK_MS_PRECISE_IP);
> +
> + if (validation_bits & CHECK_VALID_MS_RESTARTABLE_IP)
> + print_bool("Restartable IP", pfx, check, CHECK_MS_RESTARTABLE_IP);
> +
> + if (validation_bits & CHECK_VALID_MS_OVERFLOW)
> + print_bool("Overflow", pfx, check, CHECK_MS_OVERFLOW);
> +}
> +
> static void print_err_info(const char *pfx, enum err_types err_type, u64 check)
> {
> u16 validation_bits = CHECK_VALID_BITS(check);
>
> printk("%sValidation Bits: 0x%04x\n", pfx, validation_bits);
>
> + /*
> + * The MS Check structure varies a lot from the others, so use a
> + * separate function for decoding.
> + */
> if (err_type == ERR_TYPE_MS)
> - return;
> + return print_err_info_ms(pfx, validation_bits, check);
>
> if (validation_bits & CHECK_VALID_TRANS_TYPE) {
> u8 trans_type = CHECK_TRANS_TYPE(check);
> --
> 2.14.1
>

2018-02-24 16:46:26

by Ard Biesheuvel

Subject: Re: [PATCH 8/8] efi: Decode IA32/X64 Context Info structure

On 23 February 2018 at 20:03, Yazen Ghannam <[email protected]> wrote:
> From: Yazen Ghannam <[email protected]>
>
> Print the fields of the IA32/X64 Context Information structure.
>
> Print the "Register Array" as raw values. Some context types are defined
> in the UEFI spec, so more detailed decoded may be added in the future.
>
> Based on UEFI 2.7 section N.2.4.2.2 IA32/X64 Processor Context
> Information Structure.
>
> Cc: <[email protected]> # 4.16.x
> Signed-off-by: Yazen Ghannam <[email protected]>
> ---
> drivers/firmware/efi/cper-x86.c | 55 +++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 55 insertions(+)
>
> diff --git a/drivers/firmware/efi/cper-x86.c b/drivers/firmware/efi/cper-x86.c
> index 02b1b424f537..bb6cef0b5e53 100644
> --- a/drivers/firmware/efi/cper-x86.c
> +++ b/drivers/firmware/efi/cper-x86.c
> @@ -13,6 +13,7 @@
> #define VALID_LAPIC_ID BIT_ULL(0)
> #define VALID_CPUID_INFO BIT_ULL(1)
> #define VALID_PROC_ERR_INFO_NUM(bits) ((bits & GENMASK_ULL(7, 2)) >> 2)
> +#define VALID_PROC_CNXT_INFO_NUM(bits) ((bits & GENMASK_ULL(13, 8)) >> 8)

Parens

Also, CNXT isn't really idiomatic when abbreviating 'context' (and you
use CTX below as well)

>
> #define INFO_ERR_STRUCT_TYPE_CACHE \
> GUID_INIT(0xA55701F5, 0xE3EF, 0x43DE, 0xAC, 0x72, 0x24, 0x9B, \
> @@ -74,6 +75,9 @@
> #define CHECK_MS_RESTARTABLE_IP BIT_ULL(22)
> #define CHECK_MS_OVERFLOW BIT_ULL(23)
>
> +#define CTX_TYPE_MSR 1
> +#define CTX_TYPE_MMREG 7
> +
> enum err_types {
> ERR_TYPE_CACHE = 0,
> ERR_TYPE_TLB,
> @@ -137,6 +141,17 @@ static const char * const ia_check_ms_error_type_strs[] = {
> "Internal Unclassified",
> };
>
> +static const char * const ia_reg_ctx_strs[] = {
> + "Unclassified Data",
> + "MSR Registers (Machine Check and other MSRs)",
> + "32-bit Mode Execution Context",
> + "64-bit Mode Execution Context",
> + "FXSAVE Context",
> + "32-bit Mode Debug Registers (DR0-DR7)",
> + "64-bit Mode Debug Registers (DR0-DR7)",
> + "Memory Mapped Registers",
> +};
> +
> static inline void print_bool(char *str, const char *pfx, u64 check, u64 bit)
> {
> printk("%s%s: %s\n", pfx, str, (check & bit) ? "true" : "false");
> @@ -247,8 +262,10 @@ void cper_print_proc_ia(const char *pfx, const struct cper_sec_proc_ia *proc)
> {
> int i;
> struct cper_ia_err_info *err_info;
> + struct cper_ia_proc_ctx *ctx_info;
> char newpfx[64], infopfx[64];
> enum err_types err_type;
> + unsigned int max_ctx_type = ARRAY_SIZE(ia_reg_ctx_strs) - 1;
>
> printk("%sValidation Bits: 0x%016llx\n", pfx, proc->validation_bits);
>
> @@ -313,4 +330,42 @@ void cper_print_proc_ia(const char *pfx, const struct cper_sec_proc_ia *proc)
>
> err_info++;
> }
> +
> + ctx_info = (struct cper_ia_proc_ctx *)err_info;
> + for (i = 0; i < VALID_PROC_CNXT_INFO_NUM(proc->validation_bits); i++) {
> + int size = sizeof(*ctx_info) + ctx_info->reg_arr_size;
> + int groupsize = 4;
> +
> + printk("%sContext Information Structure %d:\n", pfx, i);
> +
> + if (ctx_info->reg_ctx_type > max_ctx_type) {
> + printk("%sInvalid Register Context Type: %d (max: %d)\n",
> + newpfx, ctx_info->reg_ctx_type, max_ctx_type);
> + goto next_ctx;
> + }
> +
> + printk("%sRegister Context Type: %s\n", newpfx,
> + ia_reg_ctx_strs[ctx_info->reg_ctx_type]);
> +
> + printk("%sRegister Array Size: 0x%04x\n", newpfx,
> + ctx_info->reg_arr_size);
> +
> + if (ctx_info->reg_ctx_type == CTX_TYPE_MSR) {
> + groupsize = 8; /* MSRs are 8 bytes wide. */
> + printk("%sMSR Address: 0x%08x\n", newpfx,
> + ctx_info->msr_addr);
> + }
> +
> + if (ctx_info->reg_ctx_type == CTX_TYPE_MMREG) {
> + printk("%sMM Register Address: 0x%016llx\n", newpfx,
> + ctx_info->mm_reg_addr);
> + }
> +
> + printk("%sRegister Array:\n", newpfx);
> + print_hex_dump(newpfx, "", DUMP_PREFIX_OFFSET, 16, groupsize,
> + (ctx_info + 1), ctx_info->reg_arr_size, 0);
> +
> +next_ctx:
> + ctx_info = (struct cper_ia_proc_ctx *)((long)ctx_info + size);
> + }
> }
> --
> 2.14.1
>

2018-02-24 16:48:00

by Ard Biesheuvel

Subject: Re: [PATCH 0/8] Decode IA32/X64 CPER

Hi Yazen,

On 23 February 2018 at 20:03, Yazen Ghannam <[email protected]> wrote:
> From: Yazen Ghannam <[email protected]>
>
> This series adds decoding for the IA32/X64 Common Platform Error Record.
>
> Patch 1 fixes the IA32/X64 Processor Error Section definition to match
> the UEFI spec.
>
> Patches 2-8 add the new decoding. The patches incrementally add the
> decoding starting from the top-level "Error Section". Hopefully, this
> will make reviewing a bit easier compared to one large patch.
>
> The formatting of the field names and options is taken from the UEFI
> spec. I tried to keep everything the same to make searching easier.
>
> The patches were written to the UEFI 2.7 spec though the definition of
> the IA32/X64 CPER seems to be the same as when it was introduced in
> the UEFI 2.1 spec.
>
> Without basic decoding, users will be confused about what these
> "Hardware Errors" mean. So I'm requesting this set to be applied to the
> stable branches. This set applies to the v4.16. However, patch 2 will
> have a conflict on older branches, so I'll send this set again with the
> conflict fixed.
>

These patches look mostly fine to me, with the exception of some minor nits.

I can queue this for v4.17 if you respin it, but I am not sending 400
lines of brand new error record parsing code to Greg for inclusion in
-stable, so please drop the cc stable tags

Thanks,
Ard.

2018-02-26 15:47:25

by Yazen Ghannam

Subject: RE: [PATCH 0/8] Decode IA32/X64 CPER

> -----Original Message-----
> From: Ard Biesheuvel [mailto:[email protected]]
> Sent: Saturday, February 24, 2018 11:47 AM
> To: Ghannam, Yazen <[email protected]>
> Cc: [email protected]; Linux Kernel Mailing List <linux-
> [email protected]>; Borislav Petkov <[email protected]>; the arch/x86
> maintainers <[email protected]>
> Subject: Re: [PATCH 0/8] Decode IA32/X64 CPER
>
> Hi Yazen,
>
> On 23 February 2018 at 20:03, Yazen Ghannam
> <[email protected]> wrote:
> > From: Yazen Ghannam <[email protected]>
> >
> > This series adds decoding for the IA32/X64 Common Platform Error Record.
> >
> > Patch 1 fixes the IA32/X64 Processor Error Section definition to match
> > the UEFI spec.
> >
> > Patches 2-8 add the new decoding. The patches incrementally add the
> > decoding starting from the top-level "Error Section". Hopefully, this
> > will make reviewing a bit easier compared to one large patch.
> >
> > The formatting of the field names and options is taken from the UEFI
> > spec. I tried to keep everything the same to make searching easier.
> >
> > The patches were written to the UEFI 2.7 spec though the definition of
> > the IA32/X64 CPER seems to be the same as when it was introduced in
> > the UEFI 2.1 spec.
> >
> > Without basic decoding, users will be confused about what these
> > "Hardware Errors" mean. So I'm requesting this set to be applied to the
> > stable branches. This set applies to the v4.16. However, patch 2 will
> > have a conflict on older branches, so I'll send this set again with the
> > conflict fixed.
> >
>
> These patches look mostly fine to me, with the exception of some minor nits.
>
> I can queue this for v4.17 if you respin it, but I am not sending 400
> lines of brand new error record parsing code to Greg for inclusion in
> -stable, so please drop the cc stable tags
>

Okay, will do.

Thanks,
Yazen

2018-02-26 15:57:32

by Yazen Ghannam

Subject: RE: [PATCH 2/8] efi: Decode IA32/X64 Processor Error Section

> -----Original Message-----
> From: [email protected] [mailto:linux-kernel-
> [email protected]] On Behalf Of Ard Biesheuvel
> Sent: Saturday, February 24, 2018 11:39 AM
> To: Ghannam, Yazen <[email protected]>
> Cc: [email protected]; Linux Kernel Mailing List <linux-
> [email protected]>; Borislav Petkov <[email protected]>; the arch/x86
> maintainers <[email protected]>
> Subject: Re: [PATCH 2/8] efi: Decode IA32/X64 Processor Error Section
>
...
> >
> > diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig
> > index 3098410abad8..8b684147835d 100644
> > --- a/drivers/firmware/efi/Kconfig
> > +++ b/drivers/firmware/efi/Kconfig
> > @@ -174,6 +174,11 @@ config UEFI_CPER_ARM
> > depends on UEFI_CPER && ( ARM || ARM64 )
> > default y
> >
> > +config UEFI_CPER_X86
> > + bool
> > + depends on UEFI_CPER && ( X86_32 || X86_64 )
>
> Just 'UEFI_CPER && X86' should be sufficient, no?
>

Yes.

> > + default y
> > +
> > config EFI_DEV_PATH_PARSER
> > bool
> > depends on ACPI
> > diff --git a/drivers/firmware/efi/Makefile b/drivers/firmware/efi/Makefile
> > index cb805374f4bc..7cf8d75ebe51 100644
> > --- a/drivers/firmware/efi/Makefile
> > +++ b/drivers/firmware/efi/Makefile
> > @@ -31,3 +31,5 @@ obj-$(CONFIG_ARM) += $(arm-obj-y)
> > obj-$(CONFIG_ARM64) += $(arm-obj-y)
> > obj-$(CONFIG_EFI_CAPSULE_LOADER) += capsule-loader.o
> > obj-$(CONFIG_UEFI_CPER_ARM) += cper-arm.o
> > +
>
> Spurious newline
>

Okay.

> > +obj-$(CONFIG_UEFI_CPER_X86) += cper-x86.o
> > diff --git a/drivers/firmware/efi/cper-x86.c b/drivers/firmware/efi/cper-
> x86.c
> > new file mode 100644
> > index 000000000000..b50ee3cdf637
> > --- /dev/null
> > +++ b/drivers/firmware/efi/cper-x86.c
> > @@ -0,0 +1,26 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +// Copyright (C) 2018, Advanced Micro Devices, Inc.
> > +// Copyright (C) 2018, Yazen Ghannam <[email protected]>
> > +
>
> Do you really own the copyright for code that you write on behalf of
> your employer?
>

I'll drop that line.

> > +#include <linux/cper.h>
> > +
> > +/*
> > + * We don't need a "CPER_IA" prefix since these are all locally defined.
> > + * This will save us a lot of line space.
> > + */
> > +#define VALID_LAPIC_ID BIT_ULL(0)
> > +#define VALID_CPUID_INFO BIT_ULL(1)
> > +
> > +void cper_print_proc_ia(const char *pfx, const struct cper_sec_proc_ia
> *proc)
> > +{
> > + printk("%sValidation Bits: 0x%016llx\n", pfx, proc->validation_bits);
> > +
> > + if (proc->validation_bits & VALID_LAPIC_ID)
> > + printk("%sLocal APIC_ID: 0x%llx\n", pfx, proc->lapic_id);
> > +
> > + if (proc->validation_bits & VALID_CPUID_INFO) {
> > + printk("%sCPUID Info:\n", pfx);
> > + print_hex_dump(pfx, "", DUMP_PREFIX_OFFSET, 16, 4, proc-
> >cpuid,
> > + sizeof(proc->cpuid), 0);
> > + }
> > +}
> > diff --git a/drivers/firmware/efi/cper.c b/drivers/firmware/efi/cper.c
> > index c165933ebf38..5a59b582c9aa 100644
> > --- a/drivers/firmware/efi/cper.c
> > +++ b/drivers/firmware/efi/cper.c
> > @@ -469,6 +469,16 @@ cper_estatus_print_section(const char *pfx, struct
> acpi_hest_generic_data *gdata
> > cper_print_proc_arm(newpfx, arm_err);
> > else
> > goto err_section_too_small;
> > +#endif
> > +#if defined(CONFIG_UEFI_CPER_X86)
> > + } else if (guid_equal(sec_type, &CPER_SEC_PROC_IA)) {
> > + struct cper_sec_proc_ia *ia_err = acpi_hest_get_payload(gdata);
> > +
> > + printk("%ssection_type: IA32/X64 processor error\n", newpfx);
> > + if (gdata->error_data_length >= sizeof(*ia_err))
> > + cper_print_proc_ia(newpfx, ia_err);
> > + else
> > + goto err_section_too_small;
> > #endif
> > } else {
> > const void *err = acpi_hest_get_payload(gdata);
> > diff --git a/include/linux/cper.h b/include/linux/cper.h
> > index 4b5f8459b403..9c703a0abe6e 100644
> > --- a/include/linux/cper.h
> > +++ b/include/linux/cper.h
> > @@ -551,5 +551,7 @@ const char *cper_mem_err_unpack(struct
> trace_seq *,
> > struct cper_mem_err_compact *);
> > void cper_print_proc_arm(const char *pfx,
> > const struct cper_sec_proc_arm *proc);
> > +void cper_print_proc_ia(const char *pfx,
> > + const struct cper_sec_proc_ia *proc);
> >
> > #endif
> > --
> > 2.14.1
> >

2018-02-26 16:01:30

by Yazen Ghannam

Subject: RE: [PATCH 3/8] efi: Decode IA32/X64 Processor Error Info Structure

> -----Original Message-----
> From: Ard Biesheuvel [mailto:[email protected]]
> Sent: Saturday, February 24, 2018 11:40 AM
> To: Ghannam, Yazen <[email protected]>
> Cc: [email protected]; Linux Kernel Mailing List <linux-
> [email protected]>; Borislav Petkov <[email protected]>; the arch/x86
> maintainers <[email protected]>
> Subject: Re: [PATCH 3/8] efi: Decode IA32/X64 Processor Error Info Structure
>
...
> >
> > diff --git a/drivers/firmware/efi/cper-x86.c b/drivers/firmware/efi/cper-
> x86.c
> > index b50ee3cdf637..9d608f742c98 100644
> > --- a/drivers/firmware/efi/cper-x86.c
> > +++ b/drivers/firmware/efi/cper-x86.c
> > @@ -4,15 +4,28 @@
> >
> > #include <linux/cper.h>
> >
> > +#define INDENT_SP " "
> > +
> > /*
> > * We don't need a "CPER_IA" prefix since these are all locally defined.
> > * This will save us a lot of line space.
> > */
> > #define VALID_LAPIC_ID BIT_ULL(0)
> > #define VALID_CPUID_INFO BIT_ULL(1)
> > +#define VALID_PROC_ERR_INFO_NUM(bits) ((bits & GENMASK_ULL(7, 2))
> >> 2)
> > +
>
> Parens around 'bits' please
>

Like this?

#define VALID_PROC_ERR_INFO_NUM(bits) (((bits) & GENMASK_ULL(7, 2)) >> 2)

I'll do the same for the others.

> > +#define INFO_VALID_CHECK_INFO BIT_ULL(0)
> > +#define INFO_VALID_TARGET_ID BIT_ULL(1)
> > +#define INFO_VALID_REQUESTOR_ID BIT_ULL(2)
> > +#define INFO_VALID_RESPONDER_ID BIT_ULL(3)
> > +#define INFO_VALID_IP BIT_ULL(4)
> >
> > void cper_print_proc_ia(const char *pfx, const struct cper_sec_proc_ia
> *proc)
> > {
> > + int i;
> > + struct cper_ia_err_info *err_info;
> > + char newpfx[64];
> > +
> > printk("%sValidation Bits: 0x%016llx\n", pfx, proc->validation_bits);
> >
> > if (proc->validation_bits & VALID_LAPIC_ID)
> > @@ -23,4 +36,44 @@ void cper_print_proc_ia(const char *pfx, const struct
> cper_sec_proc_ia *proc)
> > print_hex_dump(pfx, "", DUMP_PREFIX_OFFSET, 16, 4, proc-
> >cpuid,
> > sizeof(proc->cpuid), 0);
> > }
> > +
> > + snprintf(newpfx, sizeof(newpfx), "%s%s", pfx, INDENT_SP);
> > +
> > + err_info = (struct cper_ia_err_info *)(proc + 1);
> > + for (i = 0; i < VALID_PROC_ERR_INFO_NUM(proc->validation_bits); i++)
> {
> > + printk("%sError Information Structure %d:\n", pfx, i);
> > +
> > + printk("%sError Structure Type: %pUl\n", newpfx,
> > + &err_info->err_type);
> > +
>
> The indentation is a bit awkward here. Could you please align followup
> lines with the character following the ( on the first line?
>

Yes, will do.

> > + printk("%sValidation Bits: 0x%016llx\n",
> > + newpfx, err_info->validation_bits);
> > +
> > + if (err_info->validation_bits & INFO_VALID_CHECK_INFO) {
> > + printk("%sCheck Information: 0x%016llx\n", newpfx,
> > + err_info->check_info);
> > + }
> > +
> > + if (err_info->validation_bits & INFO_VALID_TARGET_ID) {
> > + printk("%sTarget Identifier: 0x%016llx\n",
> > + newpfx, err_info->target_id);
> > + }
> > +
> > + if (err_info->validation_bits & INFO_VALID_REQUESTOR_ID) {
> > + printk("%sRequestor Identifier: 0x%016llx\n",
> > + newpfx, err_info->requestor_id);
> > + }
> > +
> > + if (err_info->validation_bits & INFO_VALID_RESPONDER_ID) {
> > + printk("%sResponder Identifier: 0x%016llx\n",
> > + newpfx, err_info->responder_id);
> > + }
> > +
> > + if (err_info->validation_bits & INFO_VALID_IP) {
> > + printk("%sInstruction Pointer: 0x%016llx\n",
> > + newpfx, err_info->ip);
> > + }
> > +
> > + err_info++;
> > + }
> > }
> > --
> > 2.14.1
> >

2018-02-26 16:02:51

by Yazen Ghannam

Subject: RE: [PATCH 4/8] efi: Decode UEFI-defined IA32/X64 Error Structure GUIDs

> -----Original Message-----
> From: Ard Biesheuvel [mailto:[email protected]]
> Sent: Saturday, February 24, 2018 11:41 AM
> To: Ghannam, Yazen <[email protected]>
> Cc: [email protected]; Linux Kernel Mailing List <linux-
> [email protected]>; Borislav Petkov <[email protected]>; the arch/x86
> maintainers <[email protected]>
> Subject: Re: [PATCH 4/8] efi: Decode UEFI-defined IA32/X64 Error Structure
> GUIDs
>
> On 23 February 2018 at 20:03, Yazen Ghannam
> <[email protected]> wrote:
> > From: Yazen Ghannam <[email protected]>
> >
> > For easier handling, match the known IA32/X64 error structure GUIDs to
> > enums.
> >
> > Also, print out the name of the matching Error Structure Type.
> >
> > GUIDs taken from UEFI 2.7 section N.2.4.2.1 IA32/X64 Processor Error
> > Information Structure.
> >
> > Cc: <[email protected]> # 4.16.x
> > Signed-off-by: Yazen Ghannam <[email protected]>
> > ---
> > drivers/firmware/efi/cper-x86.c | 41
> +++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 41 insertions(+)
> >
> > diff --git a/drivers/firmware/efi/cper-x86.c b/drivers/firmware/efi/cper-
> x86.c
> > index 9d608f742c98..3b86223bdb67 100644
> > --- a/drivers/firmware/efi/cper-x86.c
> > +++ b/drivers/firmware/efi/cper-x86.c
> > @@ -14,17 +14,53 @@
...
> > void cper_print_proc_ia(const char *pfx, const struct cper_sec_proc_ia
> *proc)
> > {
> > int i;
> > struct cper_ia_err_info *err_info;
> > char newpfx[64];
> > + enum err_types err_type;
> >
>
> Can you make this an u8 please? The signedness of an enum is not well
> defined, and you only check the upper bound below.
>

Yes, will do.

Thanks,
Yazen

2018-02-26 16:06:52

by Ard Biesheuvel

Subject: Re: [PATCH 3/8] efi: Decode IA32/X64 Processor Error Info Structure

On 26 February 2018 at 16:00, Ghannam, Yazen <[email protected]> wrote:
>> -----Original Message-----
>> From: Ard Biesheuvel [mailto:[email protected]]
>> Sent: Saturday, February 24, 2018 11:40 AM
>> To: Ghannam, Yazen <[email protected]>
>> Cc: [email protected]; Linux Kernel Mailing List <linux-
>> [email protected]>; Borislav Petkov <[email protected]>; the arch/x86
>> maintainers <[email protected]>
>> Subject: Re: [PATCH 3/8] efi: Decode IA32/X64 Processor Error Info Structure
>>
> ...
>> >
>> > diff --git a/drivers/firmware/efi/cper-x86.c b/drivers/firmware/efi/cper-
>> x86.c
>> > index b50ee3cdf637..9d608f742c98 100644
>> > --- a/drivers/firmware/efi/cper-x86.c
>> > +++ b/drivers/firmware/efi/cper-x86.c
>> > @@ -4,15 +4,28 @@
>> >
>> > #include <linux/cper.h>
>> >
>> > +#define INDENT_SP " "
>> > +
>> > /*
>> > * We don't need a "CPER_IA" prefix since these are all locally defined.
>> > * This will save us a lot of line space.
>> > */
>> > #define VALID_LAPIC_ID BIT_ULL(0)
>> > #define VALID_CPUID_INFO BIT_ULL(1)
>> > +#define VALID_PROC_ERR_INFO_NUM(bits) ((bits & GENMASK_ULL(7, 2))
>> >> 2)
>> > +
>>
>> Parens around 'bits' please
>>
>
> Like this?
>
> #define VALID_PROC_ERR_INFO_NUM(bits) (((bits) & GENMASK_ULL(7, 2)) >> 2)
>

Yes. Your code currently does not pass expressions into these, but it
is good form to use parens here to make them future proof

> I'll do the same for the others.
>

Cheers

>> > +#define INFO_VALID_CHECK_INFO BIT_ULL(0)
>> > +#define INFO_VALID_TARGET_ID BIT_ULL(1)
>> > +#define INFO_VALID_REQUESTOR_ID BIT_ULL(2)
>> > +#define INFO_VALID_RESPONDER_ID BIT_ULL(3)
>> > +#define INFO_VALID_IP BIT_ULL(4)
>> >
>> > void cper_print_proc_ia(const char *pfx, const struct cper_sec_proc_ia
>> *proc)
>> > {
>> > + int i;
>> > + struct cper_ia_err_info *err_info;
>> > + char newpfx[64];
>> > +
>> > printk("%sValidation Bits: 0x%016llx\n", pfx, proc->validation_bits);
>> >
>> > if (proc->validation_bits & VALID_LAPIC_ID)
>> > @@ -23,4 +36,44 @@ void cper_print_proc_ia(const char *pfx, const struct
>> cper_sec_proc_ia *proc)
>> > print_hex_dump(pfx, "", DUMP_PREFIX_OFFSET, 16, 4, proc-
>> >cpuid,
>> > sizeof(proc->cpuid), 0);
>> > }
>> > +
>> > + snprintf(newpfx, sizeof(newpfx), "%s%s", pfx, INDENT_SP);
>> > +
>> > + err_info = (struct cper_ia_err_info *)(proc + 1);
>> > + for (i = 0; i < VALID_PROC_ERR_INFO_NUM(proc->validation_bits); i++)
>> {
>> > + printk("%sError Information Structure %d:\n", pfx, i);
>> > +
>> > + printk("%sError Structure Type: %pUl\n", newpfx,
>> > + &err_info->err_type);
>> > +
>>
>> The indentation is a bit awkward here. Could you please align followup
>> lines with the character following the ( on the first line?
>>
>
> Yes, will do.
>
>> > + printk("%sValidation Bits: 0x%016llx\n",
>> > + newpfx, err_info->validation_bits);
>> > +
>> > + if (err_info->validation_bits & INFO_VALID_CHECK_INFO) {
>> > + printk("%sCheck Information: 0x%016llx\n", newpfx,
>> > + err_info->check_info);
>> > + }
>> > +
>> > + if (err_info->validation_bits & INFO_VALID_TARGET_ID) {
>> > + printk("%sTarget Identifier: 0x%016llx\n",
>> > + newpfx, err_info->target_id);
>> > + }
>> > +
>> > + if (err_info->validation_bits & INFO_VALID_REQUESTOR_ID) {
>> > + printk("%sRequestor Identifier: 0x%016llx\n",
>> > + newpfx, err_info->requestor_id);
>> > + }
>> > +
>> > + if (err_info->validation_bits & INFO_VALID_RESPONDER_ID) {
>> > + printk("%sResponder Identifier: 0x%016llx\n",
>> > + newpfx, err_info->responder_id);
>> > + }
>> > +
>> > + if (err_info->validation_bits & INFO_VALID_IP) {
>> > + printk("%sInstruction Pointer: 0x%016llx\n",
>> > + newpfx, err_info->ip);
>> > + }
>> > +
>> > + err_info++;
>> > + }
>> > }
>> > --
>> > 2.14.1
>> >

2018-02-26 16:06:55

by Ard Biesheuvel

Subject: Re: [PATCH 8/8] efi: Decode IA32/X64 Context Info structure

On 26 February 2018 at 16:05, Ghannam, Yazen <[email protected]> wrote:
>> -----Original Message-----
>> From: Ard Biesheuvel [mailto:[email protected]]
>> Sent: Saturday, February 24, 2018 11:46 AM
>> To: Ghannam, Yazen <[email protected]>
>> Cc: [email protected]; Linux Kernel Mailing List <linux-
>> [email protected]>; Borislav Petkov <[email protected]>; the arch/x86
>> maintainers <[email protected]>
>> Subject: Re: [PATCH 8/8] efi: Decode IA32/X64 Context Info structure
>>
>> On 23 February 2018 at 20:03, Yazen Ghannam
>> <[email protected]> wrote:
>> > From: Yazen Ghannam <[email protected]>
>> >
>> > Print the fields of the IA32/X64 Context Information structure.
>> >
>> > Print the "Register Array" as raw values. Some context types are defined
>> > in the UEFI spec, so more detailed decoded may be added in the future.
>> >
>> > Based on UEFI 2.7 section N.2.4.2.2 IA32/X64 Processor Context
>> > Information Structure.
>> >
>> > Cc: <[email protected]> # 4.16.x
>> > Signed-off-by: Yazen Ghannam <[email protected]>
>> > ---
>> > drivers/firmware/efi/cper-x86.c | 55
>> +++++++++++++++++++++++++++++++++++++++++
>> > 1 file changed, 55 insertions(+)
>> >
>> > diff --git a/drivers/firmware/efi/cper-x86.c b/drivers/firmware/efi/cper-
>> x86.c
>> > index 02b1b424f537..bb6cef0b5e53 100644
>> > --- a/drivers/firmware/efi/cper-x86.c
>> > +++ b/drivers/firmware/efi/cper-x86.c
>> > @@ -13,6 +13,7 @@
>> > #define VALID_LAPIC_ID BIT_ULL(0)
>> > #define VALID_CPUID_INFO BIT_ULL(1)
>> > #define VALID_PROC_ERR_INFO_NUM(bits) ((bits & GENMASK_ULL(7, 2))
>> >> 2)
>> > +#define VALID_PROC_CNXT_INFO_NUM(bits) ((bits & GENMASK_ULL(13,
>> 8)) >> 8)
>>
>> Parens
>>
>> Also, CNXT isn't really idiomatic when abbreviating 'context' (and you
>> use CTX below as well)
>>
>
> The UEFI spec uses PROC_CONTEXT_INFO_NUM.
>
> Would you prefer 1 or 2 below?
> 1) VALID_PROC_CONTEXT_INFO_NUM
> 2) VALID_PROC_CTX_INFO_NUM
>

Let's go with 2) since you're already using CTX in numerous places.

2018-02-26 16:07:24

by Yazen Ghannam

Subject: RE: [PATCH 8/8] efi: Decode IA32/X64 Context Info structure

> -----Original Message-----
> From: Ard Biesheuvel [mailto:[email protected]]
> Sent: Saturday, February 24, 2018 11:46 AM
> To: Ghannam, Yazen <[email protected]>
> Cc: [email protected]; Linux Kernel Mailing List <linux-
> [email protected]>; Borislav Petkov <[email protected]>; the arch/x86
> maintainers <[email protected]>
> Subject: Re: [PATCH 8/8] efi: Decode IA32/X64 Context Info structure
>
> On 23 February 2018 at 20:03, Yazen Ghannam
> <[email protected]> wrote:
> > From: Yazen Ghannam <[email protected]>
> >
> > Print the fields of the IA32/X64 Context Information structure.
> >
> > Print the "Register Array" as raw values. Some context types are defined
> > in the UEFI spec, so more detailed decoded may be added in the future.
> >
> > Based on UEFI 2.7 section N.2.4.2.2 IA32/X64 Processor Context
> > Information Structure.
> >
> > Cc: <[email protected]> # 4.16.x
> > Signed-off-by: Yazen Ghannam <[email protected]>
> > ---
> > drivers/firmware/efi/cper-x86.c | 55
> +++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 55 insertions(+)
> >
> > diff --git a/drivers/firmware/efi/cper-x86.c b/drivers/firmware/efi/cper-
> x86.c
> > index 02b1b424f537..bb6cef0b5e53 100644
> > --- a/drivers/firmware/efi/cper-x86.c
> > +++ b/drivers/firmware/efi/cper-x86.c
> > @@ -13,6 +13,7 @@
> > #define VALID_LAPIC_ID BIT_ULL(0)
> > #define VALID_CPUID_INFO BIT_ULL(1)
> > #define VALID_PROC_ERR_INFO_NUM(bits) ((bits & GENMASK_ULL(7, 2))
> >> 2)
> > +#define VALID_PROC_CNXT_INFO_NUM(bits) ((bits & GENMASK_ULL(13,
> 8)) >> 8)
>
> Parens
>
> Also, CNXT isn't really idiomatic when abbreviating 'context' (and you
> use CTX below as well)
>

The UEFI spec uses PROC_CONTEXT_INFO_NUM.

Would you prefer 1 or 2 below?
1) VALID_PROC_CONTEXT_INFO_NUM
2) VALID_PROC_CTX_INFO_NUM

Thanks,
Yazen