Isaku, when you report a bug, it would be great to include the folks who
authored and worked on the original patch that introduced the bug. I've
gone ahead and done that for you here.
On 10/10/23 01:35, isaku.yamahata@xxxxxxxxx wrote:
> From: Isaku Yamahata <isaku.yamahata@xxxxxxxxx>
>
> When initializing x86 MCE injection framework, it checks if hardware mce
> injection is available or not. When it's not available on AMD, set the
> boolean variable to false to not use it. The variable is on by default and
> the feature is AMD specific based on the code.
>
> Because the variable is default on, it is true on Intel platform (probably
> on other non-AMD x86 platform). It results in unchecked msr access of
> MSR_K7_HWCR=0xc0010015 when injecting MCE on Intel platform. (Probably on
> other x86 platform.)
>
> Make the variable of by default, and set the variable on when the hardware
> feature is usable.
Gah, I'm finding that changelog impenetrable. Here's what's missing:
* The entirety of check_hw_inj_possible() is for AMD hardware:
X86_FEATURE_SMCA, the MSRs, hw_injection_possible, everything.
* Only AMD systems with SMCA support hardware error injection
(anything other than "echo sw > /sys/kernel/debug/mce-inject/flags")
* That AMD-only restriction is enforced by 'hw_injection_possible'
* 'hw_injection_possible' is true by default and only set to false in
check_hw_inj_possible() ... the AMD-only code
The end result is that everyone except SMCA-enabled AMD systems (Intel
included) leaves hw_injection_possible=true. They are free to try and
inject hardware errors. If they do, they'll get errors when writing to
the MSRs.
To fix this, make disable hw_injection_possible by default. Only enable
it on SMCA hardware that actually succeeds in ... whatever:
wrmsrl_safe(mca_msr_reg(bank, MCA_STATUS), status);
rdmsrl_safe(mca_msr_reg(bank, MCA_STATUS), &status);
is doing.
... and don't do it at the top of the function. Why bother setting it
to true only to disable it a moment later?
Do something like the following instead.diff --git a/arch/x86/kernel/cpu/mce/inject.c b/arch/x86/kernel/cpu/mce/inject.c
index 4d8d4bcf915d..01ee886d8540 100644
--- a/arch/x86/kernel/cpu/mce/inject.c
+++ b/arch/x86/kernel/cpu/mce/inject.c
@@ -33,7 +33,7 @@
#include "internal.h"
-static bool hw_injection_possible = true;
+static bool hw_injection_possible;
/*
* Collect all the MCi_XXX settings
@@ -748,9 +748,10 @@ static void check_hw_inj_possible(void)
rdmsrl_safe(mca_msr_reg(bank, MCA_STATUS), &status);
if (!status) {
- hw_injection_possible = false;
pr_warn("Platform does not allow *hardware* error injection."
"Try using APEI EINJ instead.\n");
+ } else {
+ hw_injection_possible = true;
}
toggle_hw_mce_inject(cpu, false);
