linux-kernel - Re: [PATCH 7/9] x86/mce: Unify AMD DFR handler with MCA Polling

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: <20240604110528.GRZl70-MFo-EikWRHs@fat_crate.local>
Date: Tue, 4 Jun 2024 13:05:28 +0200
From: Borislav Petkov <bp@...en8.de>
To: Yazen Ghannam <yazen.ghannam@....com>
Cc: linux-edac@...r.kernel.org, linux-kernel@...r.kernel.org,
	tony.luck@...el.com, x86@...nel.org, avadhut.naik@....com,
	john.allen@....com
Subject: Re: [PATCH 7/9] x86/mce: Unify AMD DFR handler with MCA Polling

On Thu, May 23, 2024 at 10:56:39AM -0500, Yazen Ghannam wrote:
> +static bool smca_log_poll_error(struct mce *m, u32 *status_reg)

That handing of *status_reg back'n'forth just to clear it in the end is
not nice. Let's get rid of it:

---
diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c
index 0a9cff329487..a0ba82fe6de3 100644
--- a/arch/x86/kernel/cpu/mce/core.c
+++ b/arch/x86/kernel/cpu/mce/core.c
@@ -669,7 +669,7 @@ static void reset_thr_limit(unsigned int bank)
 
 DEFINE_PER_CPU(unsigned, mce_poll_count);
 
-static bool smca_log_poll_error(struct mce *m, u32 *status_reg)
+static bool smca_log_poll_error(struct mce *m, u32 status_reg)
 {
 	/*
 	 * If this is a deferred error found in MCA_STATUS, then clear
@@ -686,8 +686,8 @@ static bool smca_log_poll_error(struct mce *m, u32 *status_reg)
 	 * If the MCA_DESTAT register has valid data, then use
 	 * it as the status register.
 	 */
-	*status_reg = MSR_AMD64_SMCA_MCx_DESTAT(m->bank);
-	m->status = mce_rdmsrl(*status_reg);
+	status_reg = MSR_AMD64_SMCA_MCx_DESTAT(m->bank);
+	m->status = mce_rdmsrl(status_reg);
 
 	if (!(m->status & MCI_STATUS_VAL))
 		return false;
@@ -695,6 +695,8 @@ static bool smca_log_poll_error(struct mce *m, u32 *status_reg)
 	if (m->status & MCI_STATUS_ADDRV)
 		m->addr = mce_rdmsrl(MSR_AMD64_SMCA_MCx_DEADDR(m->bank));
 
+	mce_wrmsrl(status_reg, 0);
+
 	return true;
 }
 
@@ -714,7 +716,7 @@ static bool ser_log_poll_error(struct mce *m)
 	return false;
 }
 
-static bool log_poll_error(enum mcp_flags flags, struct mce *m, u32 *status_reg)
+static bool log_poll_error(enum mcp_flags flags, struct mce *m, u32 status_reg)
 {
 	if (mce_flags.smca)
 		return smca_log_poll_error(m, status_reg);
@@ -789,7 +791,7 @@ void machine_check_poll(enum mcp_flags flags, mce_banks_t *b)
 		if (!mca_cfg.cmci_disabled)
 			mce_track_storm(&m);
 
-		if (!log_poll_error(flags, &m, &status_reg))
+		if (!log_poll_error(flags, &m, status_reg))
 			continue;
 
 		if (flags & MCP_DONTLOG)


-- 
Regards/Gruss,
    Boris.

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