linux-kernel - Re: [PATCH v2 3/4] x86/mce/apei: Handle variable register array size

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240626115641.GPZnwB-QEGYCoI_Fv3@fat_crate.local>
Date: Wed, 26 Jun 2024 13:57:24 +0200
From: Borislav Petkov <bp@...en8.de>
To: Avadhut Naik <avadhut.naik@....com>
Cc: x86@...nel.org, linux-edac@...r.kernel.org,
	linux-trace-kernel@...r.kernel.org, linux-acpi@...r.kernel.org,
	linux-kernel@...r.kernel.org, tony.luck@...el.com,
	rafael@...nel.org, tglx@...utronix.de, mingo@...hat.com,
	rostedt@...dmis.org, lenb@...nel.org, mchehab@...nel.org,
	james.morse@....com, airlied@...il.com, yazen.ghannam@....com,
	john.allen@....com, avadnaik@....com
Subject: Re: [PATCH v2 3/4] x86/mce/apei: Handle variable register array size

On Tue, Jun 25, 2024 at 02:56:23PM -0500, Avadhut Naik wrote:
> From: Yazen Ghannam <yazen.ghannam@....com>
> 
> 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@....com>
> Signed-off-by: Avadhut Naik <avadhut.naik@....com>
> Signed-off-by: Yazen Ghannam <yazen.ghannam@....com>

This needs Avadhut's SOB after Yazen's.

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);

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette