RE: [PATCH v3 2/8] efi: Decode IA32/X64 Processor Error Section
> -Original Message- > From: Borislav Petkov <b...@alien8.de> > Sent: Thursday, March 29, 2018 5:46 AM > To: Ghannam, Yazen <yazen.ghan...@amd.com> > Cc: linux-efi@vger.kernel.org; linux-ker...@vger.kernel.org; > ard.biesheu...@linaro.org; x...@kernel.org; tony.l...@intel.com > Subject: Re: [PATCH v3 2/8] efi: Decode IA32/X64 Processor Error Section > > On Sat, Mar 24, 2018 at 01:49:34PM -0500, Yazen Ghannam wrote: > > From: Yazen Ghannam <yazen.ghan...@amd.com> > > > > 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 252. Processor Error Record. > > > > Signed-off-by: Yazen Ghannam <yazen.ghan...@amd.com> > > --- > > Link: > > https://lkml.kernel.org/r/20180226193904.20532-3- > yazen.ghan...@amd.com > > > > v2->v3: > > * Fix table number in commit message. > > * Don't print raw validation bits. > > > > v1->v2: > > * Change config option depends to "X86" instead of "X86_32 || X64_64". > > * Remove extra newline in Makefile changes. > > * Drop author copyright line. > > > > drivers/firmware/efi/Kconfig| 5 + > > drivers/firmware/efi/Makefile | 1 + > > drivers/firmware/efi/cper-x86.c | 23 +++ > > I'd prefer if that were: > > drivers/firmware/efi/cper/x86.c > drivers/firmware/efi/cper/arm.c > > but that's Ard's call. > > > drivers/firmware/efi/cper.c | 10 ++ > > include/linux/cper.h| 2 ++ > > 5 files changed, 41 insertions(+) > > create mode 100644 drivers/firmware/efi/cper-x86.c > > > > diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig > > index 3098410abad8..781a4a337557 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 > > + 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..5f9f5039de50 100644 > > --- a/drivers/firmware/efi/Makefile > > +++ b/drivers/firmware/efi/Makefile > > @@ -31,3 +31,4 @@ 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 ..863f0cd2a0ff > > --- /dev/null > > +++ b/drivers/firmware/efi/cper-x86.c > > @@ -0,0 +1,23 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +// Copyright (C) 2018, Advanced Micro Devices, Inc. > > + > > +#include > > + > > +/* > > + * 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) > > +{ > > + 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); > > That's dumping 48 bytes of static information for every error! > Information which can be dumped only once when collecting system info. > Right, but here we just print the record as-is. We can do it different when this hooks into the MCA decoding. And the use-case at the moment is for BERT which shouldn't have very many errors. Thanks, Yazen
Re: [PATCH v3 2/8] efi: Decode IA32/X64 Processor Error Section
On Sat, Mar 24, 2018 at 01:49:34PM -0500, Yazen Ghannam wrote: > From: Yazen Ghannam> > 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 252. Processor Error Record. > > Signed-off-by: Yazen Ghannam > --- > Link: > https://lkml.kernel.org/r/20180226193904.20532-3-yazen.ghan...@amd.com > > v2->v3: > * Fix table number in commit message. > * Don't print raw validation bits. > > v1->v2: > * Change config option depends to "X86" instead of "X86_32 || X64_64". > * Remove extra newline in Makefile changes. > * Drop author copyright line. > > drivers/firmware/efi/Kconfig| 5 + > drivers/firmware/efi/Makefile | 1 + > drivers/firmware/efi/cper-x86.c | 23 +++ I'd prefer if that were: drivers/firmware/efi/cper/x86.c drivers/firmware/efi/cper/arm.c but that's Ard's call. > drivers/firmware/efi/cper.c | 10 ++ > include/linux/cper.h| 2 ++ > 5 files changed, 41 insertions(+) > create mode 100644 drivers/firmware/efi/cper-x86.c > > diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig > index 3098410abad8..781a4a337557 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 > + 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..5f9f5039de50 100644 > --- a/drivers/firmware/efi/Makefile > +++ b/drivers/firmware/efi/Makefile > @@ -31,3 +31,4 @@ 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 ..863f0cd2a0ff > --- /dev/null > +++ b/drivers/firmware/efi/cper-x86.c > @@ -0,0 +1,23 @@ > +// SPDX-License-Identifier: GPL-2.0 > +// Copyright (C) 2018, Advanced Micro Devices, Inc. > + > +#include > + > +/* > + * 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) > +{ > + 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); That's dumping 48 bytes of static information for every error! Information which can be dumped only once when collecting system info. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 2/8] efi: Decode IA32/X64 Processor Error Section
From: Yazen GhannamRecognize 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 252. Processor Error Record. Signed-off-by: Yazen Ghannam --- Link: https://lkml.kernel.org/r/20180226193904.20532-3-yazen.ghan...@amd.com v2->v3: * Fix table number in commit message. * Don't print raw validation bits. v1->v2: * Change config option depends to "X86" instead of "X86_32 || X64_64". * Remove extra newline in Makefile changes. * Drop author copyright line. drivers/firmware/efi/Kconfig| 5 + drivers/firmware/efi/Makefile | 1 + drivers/firmware/efi/cper-x86.c | 23 +++ drivers/firmware/efi/cper.c | 10 ++ include/linux/cper.h| 2 ++ 5 files changed, 41 insertions(+) create mode 100644 drivers/firmware/efi/cper-x86.c diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig index 3098410abad8..781a4a337557 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 + 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..5f9f5039de50 100644 --- a/drivers/firmware/efi/Makefile +++ b/drivers/firmware/efi/Makefile @@ -31,3 +31,4 @@ 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 ..863f0cd2a0ff --- /dev/null +++ b/drivers/firmware/efi/cper-x86.c @@ -0,0 +1,23 @@ +// SPDX-License-Identifier: GPL-2.0 +// Copyright (C) 2018, Advanced Micro Devices, Inc. + +#include + +/* + * 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) +{ + 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, _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 -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html