Received-SPF: pass (google.com: domain of linux-kernel+bounces-200488-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Tue, 4 Jun 2024 13:05:28 +0200
From: Borislav Petkov <bp@alien8.de>
To: Yazen Ghannam <yazen.ghannam@amd.com>
Cc: linux-edac@vger.kernel.org, linux-kernel@vger.kernel.org,
	tony.luck@intel.com, x86@kernel.org, avadhut.naik@amd.com,
	john.allen@amd.com
Subject: Re: [PATCH 7/9] x86/mce: Unify AMD DFR handler with MCA Polling
Message-ID: <20240604110528.GRZl70-MFo-EikWRHs@fat_crate.local>
References: <20240523155641.2805411-1-yazen.ghannam@amd.com>
 <20240523155641.2805411-8-yazen.ghannam@amd.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <20240523155641.2805411-8-yazen.ghannam@amd.com>

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