[PATCH v5 0/3] x86/mce: Handle error injection failure in mce-inject module

[PATCH v5 0/3] x86/mce: Handle error injection failure in mce-inject module

[Smita Koralahalli]

This set of patches handles a scenario of silent hw error injection
failure on mce-inject module and returns appropriate error code to
userspace.

Error injection fails if the platform enforces write ignored behavior on
status registers and the second patch checks for writes ignored from
MCA_STATUS register and returns appropriate error code to user.

The first patch does some cleanup by replacing existing struct i_mce
with inject_desc and including error field as its member.

The last patch assigns and returns the error code to userspace when none
of the CPUs are online.

Smita Koralahalli (3):
  x86/mce/mce-inject: Replace struct i_mce with struct inject_desc
  x86/mce: Check for writes ignored in MCA_STATUS register
  x86/mce/mce-inject: Return appropriate error code if CPUs are offline

 arch/x86/kernel/cpu/mce/inject.c   | 124 +++++++++++++++++++----------
 arch/x86/kernel/cpu/mce/internal.h |   2 +-
 2 files changed, 82 insertions(+), 44 deletions(-)

-- 
2.17.1

[PATCH v5 1/3] x86/mce/mce-inject: Replace struct i_mce with struct inject_desc

[Smita Koralahalli]

Replace the existing struct i_mce with struct inject_desc. Extend the
struct to include "error" field. This error field will be useful to
return error codes to userspace when error injection fails.

Suggested-by: Borislav Petkov <bp@suse.de>
Signed-off-by: Borislav Petkov <bp@suse.de>
Signed-off-by: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>
Link: https://lkml.kernel.org/r/Yk267A1MKOo2AlXQ@zn.tnic
---
 arch/x86/kernel/cpu/mce/inject.c | 94 ++++++++++++++++++--------------
 1 file changed, 52 insertions(+), 42 deletions(-)

diff --git a/arch/x86/kernel/cpu/mce/inject.c b/arch/x86/kernel/cpu/mce/inject.c
index 5fbd7ffb3233..05581b718529 100644
--- a/arch/x86/kernel/cpu/mce/inject.c
+++ b/arch/x86/kernel/cpu/mce/inject.c
@@ -33,10 +33,12 @@
 
 #include "internal.h"
 
-/*
- * Collect all the MCi_XXX settings
- */
-static struct mce i_mce;
+/* Collect all the MCi_XXX settings */
+static struct inject_desc {
+	struct mce m;
+	int err;
+} inj_desc;
+
 static struct dentry *dfs_inj;
 
 #define MAX_FLAG_OPT_SIZE	4
@@ -110,9 +112,11 @@ static int inj_ipid_set(void *data, u64 val)
 
 DEFINE_SIMPLE_ATTRIBUTE(ipid_fops, inj_ipid_get, inj_ipid_set, "%llx\n");
 
-static void setup_inj_struct(struct mce *m)
+static void setup_inj_struct(void)
 {
-	memset(m, 0, sizeof(struct mce));
+	struct mce *m = &inj_desc.m;
+
+	memset(&inj_desc, 0, sizeof(struct inject_desc));
 
 	m->cpuvendor = boot_cpu_data.x86_vendor;
 	m->time	     = ktime_get_real_seconds();
@@ -470,56 +474,57 @@ static void toggle_nb_mca_mst_cpu(u16 nid)
 		       __func__, PCI_FUNC(F3->devfn), NBCFG);
 }
 
-static void prepare_msrs(void *info)
+static void prepare_msrs(void *unused)
 {
-	struct mce m = *(struct mce *)info;
-	u8 b = m.bank;
+	struct mce *m = &inj_desc.m;
+	u8 b = inj_desc.m.bank;
 
-	wrmsrl(MSR_IA32_MCG_STATUS, m.mcgstatus);
+	wrmsrl(MSR_IA32_MCG_STATUS, m->mcgstatus);
 
 	if (boot_cpu_has(X86_FEATURE_SMCA)) {
-		if (m.inject_flags == DFR_INT_INJ) {
-			wrmsrl(MSR_AMD64_SMCA_MCx_DESTAT(b), m.status);
-			wrmsrl(MSR_AMD64_SMCA_MCx_DEADDR(b), m.addr);
+		if (m->inject_flags == DFR_INT_INJ) {
+			wrmsrl(MSR_AMD64_SMCA_MCx_DESTAT(b), m->status);
+			wrmsrl(MSR_AMD64_SMCA_MCx_DEADDR(b), m->addr);
 		} else {
-			wrmsrl(MSR_AMD64_SMCA_MCx_STATUS(b), m.status);
-			wrmsrl(MSR_AMD64_SMCA_MCx_ADDR(b), m.addr);
+			wrmsrl(MSR_AMD64_SMCA_MCx_STATUS(b), m->status);
+			wrmsrl(MSR_AMD64_SMCA_MCx_ADDR(b), m->addr);
 		}
 
-		wrmsrl(MSR_AMD64_SMCA_MCx_MISC(b), m.misc);
-		wrmsrl(MSR_AMD64_SMCA_MCx_SYND(b), m.synd);
+		wrmsrl(MSR_AMD64_SMCA_MCx_MISC(b), m->misc);
+		wrmsrl(MSR_AMD64_SMCA_MCx_SYND(b), m->synd);
 	} else {
-		wrmsrl(MSR_IA32_MCx_STATUS(b), m.status);
-		wrmsrl(MSR_IA32_MCx_ADDR(b), m.addr);
-		wrmsrl(MSR_IA32_MCx_MISC(b), m.misc);
+		wrmsrl(MSR_IA32_MCx_STATUS(b), m->status);
+		wrmsrl(MSR_IA32_MCx_ADDR(b), m->addr);
+		wrmsrl(MSR_IA32_MCx_MISC(b), m->misc);
 	}
 }
 
-static void do_inject(void)
+static int do_inject(void)
 {
+	struct mce *m = &inj_desc.m;
+	unsigned int cpu = m->extcpu;
 	u64 mcg_status = 0;
-	unsigned int cpu = i_mce.extcpu;
-	u8 b = i_mce.bank;
+	u8 b = m->bank;
 
-	i_mce.tsc = rdtsc_ordered();
+	m->tsc = rdtsc_ordered();
 
-	i_mce.status |= MCI_STATUS_VAL;
+	m->status |= MCI_STATUS_VAL;
 
-	if (i_mce.misc)
-		i_mce.status |= MCI_STATUS_MISCV;
+	if (m->misc)
+		m->status |= MCI_STATUS_MISCV;
 
-	if (i_mce.synd)
-		i_mce.status |= MCI_STATUS_SYNDV;
+	if (m->synd)
+		m->status |= MCI_STATUS_SYNDV;
 
 	if (inj_type == SW_INJ) {
-		mce_log(&i_mce);
-		return;
+		mce_log(m);
+		return 0;
 	}
 
 	/* prep MCE global settings for the injection */
 	mcg_status = MCG_STATUS_MCIP | MCG_STATUS_EIPV;
 
-	if (!(i_mce.status & MCI_STATUS_PCC))
+	if (!(m->status & MCI_STATUS_PCC))
 		mcg_status |= MCG_STATUS_RIPV;
 
 	/*
@@ -528,8 +533,8 @@ static void do_inject(void)
 	 * - MCx_STATUS[UC] cleared: deferred errors are _not_ UC
 	 */
 	if (inj_type == DFR_INT_INJ) {
-		i_mce.status |= MCI_STATUS_DEFERRED;
-		i_mce.status &= ~MCI_STATUS_UC;
+		m->status |= MCI_STATUS_DEFERRED;
+		m->status &= ~MCI_STATUS_UC;
 	}
 
 	/*
@@ -550,12 +555,15 @@ static void do_inject(void)
 
 	toggle_hw_mce_inject(cpu, true);
 
-	i_mce.mcgstatus = mcg_status;
-	i_mce.inject_flags = inj_type;
-	smp_call_function_single(cpu, prepare_msrs, &i_mce, 0);
+	m->mcgstatus = mcg_status;
+	m->inject_flags = inj_type;
+	smp_call_function_single(cpu, prepare_msrs, NULL, 0);
 
 	toggle_hw_mce_inject(cpu, false);
 
+	if (inj_desc.err)
+		goto err;
+
 	switch (inj_type) {
 	case DFR_INT_INJ:
 		smp_call_function_single(cpu, trigger_dfr_int, NULL, 0);
@@ -570,6 +578,7 @@ static void do_inject(void)
 err:
 	cpus_read_unlock();
 
+	return inj_desc.err;
 }
 
 /*
@@ -580,6 +589,7 @@ static int inj_bank_set(void *data, u64 val)
 {
 	struct mce *m = (struct mce *)data;
 	u8 n_banks;
+	int err;
 	u64 cap;
 
 	/* Get bank count on target CPU so we can handle non-uniform values. */
@@ -619,12 +629,12 @@ static int inj_bank_set(void *data, u64 val)
 	}
 
 inject:
-	do_inject();
+	err = do_inject();
 
 	/* Reset injection struct */
-	setup_inj_struct(&i_mce);
+	setup_inj_struct();
 
-	return 0;
+	return err;
 }
 
 MCE_INJECT_GET(bank);
@@ -714,7 +724,7 @@ static void __init debugfs_init(void)
 
 	for (i = 0; i < ARRAY_SIZE(dfs_fls); i++)
 		debugfs_create_file(dfs_fls[i].name, dfs_fls[i].perm, dfs_inj,
-				    &i_mce, dfs_fls[i].fops);
+				    &inj_desc.m, dfs_fls[i].fops);
 }
 
 static int __init inject_init(void)
@@ -727,7 +737,7 @@ static int __init inject_init(void)
 	register_nmi_handler(NMI_LOCAL, mce_raise_notify, 0, "mce_notify");
 	mce_register_injector_chain(&inject_nb);
 
-	setup_inj_struct(&i_mce);
+	setup_inj_struct();
 
 	pr_info("Machine check injector initialized\n");
 
-- 
2.17.1

[PATCH v5 2/3] x86/mce: Check for writes ignored in MCA_STATUS register

[Smita Koralahalli]

According to Section 2.1.16.3 under HWCR[McStatusWrEn] in "PPR for AMD
Family 19h, Model 01h, Revision B1 Processors - 55898 Rev 0.35 - Feb 5,
2021", the status register may sometimes enforce write ignored behavior
independent of the value of HWCR[McStatusWrEn] depending on the platform
settings.

Hence, evaluate for writes ignored for MCA_STATUS before doing hw error
injection. If true, return the appropriate error code to userspace.

Introduce "hw_injection_possible" flag to return early on subsequent
hw error injections.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=206537
Signed-off-by: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>
---
Link:
https://lkml.kernel.org/r/20220214233640.70510-2-Smita.KoralahalliChannabasappa@amd.com

v2:
	msr_ops -> mca_msr_reg().
	simulation -> injection.
	pr_info() -> pr_err().
	Aligned on ",".
v3:
	Removed "x86/mce: Use mca_msr_reg() in prepare_msrs()" patch.
	and made changes on the existing MCx_{STATUS, ADDR, MISC} macros.
v4:
	Simplified the code by just checking for writes ignored behavior in
	MCA_STATUS register.
	Introduced prepare_mca_status() and performed writes ignored checks
	inside the function.
	Rephrased error message.
v5:
	Replaced i_mce with inject_desc.
	Introduction of hw_injection_possible flag to return early if HW
	error injections are not possible.
---
 arch/x86/kernel/cpu/mce/inject.c   | 25 +++++++++++++++++++++++++
 arch/x86/kernel/cpu/mce/internal.h |  2 +-
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/mce/inject.c b/arch/x86/kernel/cpu/mce/inject.c
index 05581b718529..cce068a4478c 100644
--- a/arch/x86/kernel/cpu/mce/inject.c
+++ b/arch/x86/kernel/cpu/mce/inject.c
@@ -33,6 +33,8 @@
 
 #include "internal.h"
 
+static bool hw_injection_possible = true;
+
 /* Collect all the MCi_XXX settings */
 static struct inject_desc {
 	struct mce m;
@@ -474,11 +476,29 @@ static void toggle_nb_mca_mst_cpu(u16 nid)
 		       __func__, PCI_FUNC(F3->devfn), NBCFG);
 }
 
+static bool prepare_mca_status(void)
+{
+	u32 status_reg = mca_msr_reg(inj_desc.m.bank, MCA_STATUS);
+	u64 status_val = inj_desc.m.status;
+
+	wrmsrl(status_reg, status_val);
+	rdmsrl(status_reg, status_val);
+
+	return status_val == inj_desc.m.status;
+}
+
 static void prepare_msrs(void *unused)
 {
 	struct mce *m = &inj_desc.m;
 	u8 b = inj_desc.m.bank;
 
+	if (!prepare_mca_status()) {
+		pr_err("Platform does not allow error injection, try using APEI EINJ instead.\n");
+		inj_desc.err = -EINVAL;
+		hw_injection_possible = false;
+		return;
+	}
+
 	wrmsrl(MSR_IA32_MCG_STATUS, m->mcgstatus);
 
 	if (boot_cpu_has(X86_FEATURE_SMCA)) {
@@ -521,6 +541,11 @@ static int do_inject(void)
 		return 0;
 	}
 
+	if (!hw_injection_possible) {
+		pr_err("SW-only injection possible on this platform");
+		return -EINVAL;
+	}
+
 	/* prep MCE global settings for the injection */
 	mcg_status = MCG_STATUS_MCIP | MCG_STATUS_EIPV;
 
diff --git a/arch/x86/kernel/cpu/mce/internal.h b/arch/x86/kernel/cpu/mce/internal.h
index 4ae0e603f7fa..7e03f5b7f6bd 100644
--- a/arch/x86/kernel/cpu/mce/internal.h
+++ b/arch/x86/kernel/cpu/mce/internal.h
@@ -211,7 +211,7 @@ noinstr u64 mce_rdmsrl(u32 msr);
 
 static __always_inline u32 mca_msr_reg(int bank, enum mca_msr reg)
 {
-	if (mce_flags.smca) {
+	if (cpu_feature_enabled(X86_FEATURE_SMCA)) {
 		switch (reg) {
 		case MCA_CTL:	 return MSR_AMD64_SMCA_MCx_CTL(bank);
 		case MCA_ADDR:	 return MSR_AMD64_SMCA_MCx_ADDR(bank);
-- 
2.17.1

[PATCH v5 3/3] x86/mce/mce-inject: Return appropriate error code if CPUs are offline

[Smita Koralahalli]

Assign appropriate error code when no CPUs are available online.

Return this error code with appropriate message to user when injection
fails.

Signed-off-by: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>
Reviewed-by: Yazen Ghannam <yazen.ghannam@amd.com>
---
Link:
https://lkml.kernel.org/r/20220214233640.70510-3-Smita.KoralahalliChannabasappa@amd.com

v2:
	Added pr_err() along with error code.
v3:
	Rephrased the statement: No online CPUs available for error
	injection -> Chosen CPU is not online.
v4:
	Prefixed "mce-inject" so the user knows that the message is
	coming from this module.
	Printed CPU number along with the error message.
v5:
	Used the new descriptor for writing errors.
---
 arch/x86/kernel/cpu/mce/inject.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/mce/inject.c b/arch/x86/kernel/cpu/mce/inject.c
index cce068a4478c..efae0e938293 100644
--- a/arch/x86/kernel/cpu/mce/inject.c
+++ b/arch/x86/kernel/cpu/mce/inject.c
@@ -575,8 +575,11 @@ static int do_inject(void)
 	}
 
 	cpus_read_lock();
-	if (!cpu_online(cpu))
+	if (!cpu_online(cpu)) {
+		pr_err("mce-inject: Chosen CPU %d is not online\n", cpu);
+		inj_desc.err = -ENODEV;
 		goto err;
+	}
 
 	toggle_hw_mce_inject(cpu, true);
 
-- 
2.17.1