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

From: Ghannam, Yazen
Date: Mon Feb 26 2018 - 10:56:07 EST


> -----Original Message-----
> From: linux-kernel-owner@xxxxxxxxxxxxxxx [mailto:linux-kernel-
> owner@xxxxxxxxxxxxxxx] On Behalf Of Ard Biesheuvel
> Sent: Saturday, February 24, 2018 11:39 AM
> To: Ghannam, Yazen <Yazen.Ghannam@xxxxxxx>
> Cc: linux-efi@xxxxxxxxxxxxxxx; Linux Kernel Mailing List <linux-
> kernel@xxxxxxxxxxxxxxx>; Borislav Petkov <bp@xxxxxxx>; the arch/x86
> maintainers <x86@xxxxxxxxxx>
> 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 <yazen.ghannam@xxxxxxx>
> > +
>
> 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
> >