On 6/26/2024 06:57, Borislav Petkov wrote:
> On Tue, Jun 25, 2024 at 02:56:23PM -0500, Avadhut Naik wrote:
>> From: Yazen Ghannam <yazen.ghannam@xxxxxxx>
>>
>> ACPI Boot Error Record Table (BERT) is being used by the kernel to
>> report errors that occurred in a previous boot. On some modern AMD
>> systems, these very errors within the BERT are reported through the
>> x86 Common Platform Error Record (CPER) format which consists of one
>> or more Processor Context Information Structures. These context
>> structures provide a starting address and represent an x86 MSR range
>> in which the data constitutes a contiguous set of MSRs starting from,
>> and including the starting address.
>>
>> It's common, for AMD systems that implement this behavior, that the
>> MSR range represents the MCAX register space used for the Scalable MCA
>> feature. The apei_smca_report_x86_error() function decodes and passes
>> this information through the MCE notifier chain. However, this function
>> assumes a fixed register size based on the original HW/FW implementation.
>>
>> This assumption breaks with the addition of two new MCAX registers viz.
>> MCA_SYND1 and MCA_SYND2. These registers are added at the end of the
>> MCAX register space, so they won't be included when decoding the CPER
>> data.
>>
>> Rework apei_smca_report_x86_error() to support a variable register array
>> size. This covers any case where the MSR context information starts at
>> the MCAX address for MCA_STATUS and ends at any other register within
>> the MCAX register space.
>>
>> Add code comments indicating the MCAX register at each offset.
>>
>> [Yazen: Add Avadhut as co-developer for wrapper changes.]
>>
>> Co-developed-by: Avadhut Naik <avadhut.naik@xxxxxxx>
>> Signed-off-by: Avadhut Naik <avadhut.naik@xxxxxxx>
>> Signed-off-by: Yazen Ghannam <yazen.ghannam@xxxxxxx>
>
> This needs Avadhut's SOB after Yazen's.
>
Will do. Will change to the below format:
Co-developed-by: Avadhut Naik <avadhut.naik@xxxxxxx>
Signed-off-by: Yazen Ghannam <yazen.ghannam@xxxxxxx>
Signed-off-by: Avadhut Naik <avadhut.naik@xxxxxxx>
> Touchups ontop:
>
> diff --git a/arch/x86/kernel/cpu/mce/apei.c b/arch/x86/kernel/cpu/mce/apei.c
> index 7a15f0ca1bd1..6bbeb29125a9 100644
> --- a/arch/x86/kernel/cpu/mce/apei.c
> +++ b/arch/x86/kernel/cpu/mce/apei.c
> @@ -69,7 +69,7 @@ EXPORT_SYMBOL_GPL(apei_mce_report_mem_error);
> int apei_smca_report_x86_error(struct cper_ia_proc_ctx *ctx_info, u64 lapic_id)
> {
> const u64 *i_mce = ((const u64 *) (ctx_info + 1));
> - unsigned int cpu, num_registers;
> + unsigned int cpu, num_regs;
> struct mce_hw_err err;
> struct mce *m = &err.m;
>
> @@ -93,10 +93,10 @@ int apei_smca_report_x86_error(struct cper_ia_proc_ctx *ctx_info, u64 lapic_id)
> /*
> * The number of registers in the register array is determined by
> * Register Array Size/8 as defined in UEFI spec v2.8, sec N.2.4.2.2.
> - * Ensure that the array size includes at least 1 register.
> + * Sanity-check registers array size.
> */
> - num_registers = ctx_info->reg_arr_size >> 3;
> - if (!num_registers)
> + num_regs = ctx_info->reg_arr_size >> 3;
> + if (!num_regs)
> return -EINVAL;
>
> mce_setup(m);
> @@ -118,13 +118,12 @@ int apei_smca_report_x86_error(struct cper_ia_proc_ctx *ctx_info, u64 lapic_id)
> /*
> * The SMCA register layout is fixed and includes 16 registers.
> * The end of the array may be variable, but the beginning is known.
> - * Switch on the number of registers. Cap the number of registers to
> - * expected max (15).
> + * Cap the number of registers to expected max (15).
> */
> - if (num_registers > 15)
> - num_registers = 15;
> + if (num_regs > 15)
> + num_regs = 15;
>
> - switch (num_registers) {
> + switch (num_regs) {
> /* MCA_SYND2 */
> case 15:
> err.vi.amd.synd2 = *(i_mce + 14);
>
Will incorporate these.
--
Thanks,
Avadhut Naik
