Skip to contentSkip to conversation

x86, mce: CE in last bank prevents panic by unknown MCE

PATCH Applied Hidetoshi Seto 2 participants 3 messages Aug 26, 2009 – Sep 17, 2009 x86

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 x86, mce: CE in last bank prevents panic by unknown MCE

[based on tip/x86/mce]

If MCE handler is called but none of mces_seen have machine check event which might signal the MCE (i.e. event higher than MCE_KEEP_SEVERITY), panic with "Machine check from unknown source" will be taken since the MCE is assumed to be signaled from external agent or so.

Usually mces_seen never point MCE_KEEP_SEVERITY event such as CE. But it can happen because initial value of mces_seen is accidentally modified by mce_no_way_out() - in case if mce_no_way_out() run through all banks and the last bank has the CE, mces_seen points the CE and the "panic by unknown" will not be taken.

This patch fix this undesired behavior, and clarify the logic.

---
 arch/x86/kernel/cpu/mcheck/mce.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
arch/x86/kernel/cpu/mcheck/mce.c +3 -3
index 54bd1b2..7b485e9 100644
@@ -665,7 +665,7 @@ static void mce_reign(void)
 	 * No machine check event found. Must be some external
 	 * source or one CPU is hung. Panic.
 	 */
-	if (!m && tolerant < 3)
+	if (global_worst <= MCE_KEEP_SEVERITY && tolerant < 3)
 		mce_panic("Machine check from unknown source", NULL, NULL);
 
 	/*
@@ -889,11 +889,11 @@ void do_machine_check(struct pt_regs *regs, long error_code)
 	mce_setup(&m);
 
 	m.mcgstatus = mce_rdmsrl(MSR_IA32_MCG_STATUS);
-	no_way_out = mce_no_way_out(&m, &msg);
-
 	final = &__get_cpu_var(mces_seen);
 	*final = m;
 
+	no_way_out = mce_no_way_out(&m, &msg);
+
 	barrier();
 
 	/*
Signature · 1 line
1.6.4.1
Applied
#1 lore raw
Quoted 15 lines · Hidetoshi Seto

[based on tip/x86/mce]

If MCE handler is called but none of mces_seen have machine check event which might signal the MCE (i.e. event higher than MCE_KEEP_SEVERITY), panic with "Machine check from unknown source" will be taken since the MCE is assumed to be signaled from external agent or so.

Usually mces_seen never point MCE_KEEP_SEVERITY event such as CE. But it can happen because initial value of mces_seen is accidentally modified by mce_no_way_out() - in case if mce_no_way_out() run through all banks and the last bank has the CE, mces_seen points the CE and the "panic by unknown" will not be taken.

Quoted 8 lines

This patch fix this undesired behavior, and clarify the logic.

---
 arch/x86/kernel/cpu/mcheck/mce.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

applied, thanks!

Btw., i had a quick look at arch/x86/kernel/cpu/mcheck/mce-severity.c, and it is quite a pile of unclean, over-engineered crap really.

Would you be interested in sending me a patch that converts that to clean, proper C code that just checks the bits in a straightforward, readable way? We dont need that silly, unreadable table with the macro jungle and we definitely dont want to expose it via debugfs - the debugfs bits can be removed altogether.

[ Plus in the mce_severity() implementation please rename 'a' to at
least 'm' - that name choice for a variable shows zero taste. We dont program the kernel in BASIC with 'A', 'B' and 'C' variable names anymore. ]

Thanks,

Ingo

#2 lore raw
In reply to Hidetoshi Seto
Commit-ID:  680b6cfd3cee30a7d997d49430fb73af84523853
Gitweb:     http://git.kernel.org/tip/680b6cfd3cee30a7d997d49430fb73af84523853
Author:     Hidetoshi Seto <[email protected]>
AuthorDate: Wed, 26 Aug 2009 16:20:36 +0900
Committer:  Ingo Molnar <[email protected]>
CommitDate: Wed, 26 Aug 2009 20:21:11 +0200

x86, mce: CE in last bank prevents panic by unknown MCE

If MCE handler is called but none of mces_seen have machine check event which might signal the MCE (i.e. event higher than MCE_KEEP_SEVERITY), panic with "Machine check from unknown source" will be taken since the MCE is assumed to be signaled from external agent or so.

Usually mces_seen never point MCE_KEEP_SEVERITY event such as CE. But it can happen because initial value of mces_seen is accidentally modified by mce_no_way_out() - in case if mce_no_way_out() run through all banks and the last bank has the CE, mces_seen points the CE and the "panic by unknown" will not be taken.

This patch fixes this undesired behavior, and clarifies the logic.

Cc H. Peter Anvin <[email protected]>
Cc Andi Kleen <[email protected]>
Cc Jin Dongming <[email protected]>

LKML-Reference: <[email protected]>

---
 arch/x86/kernel/cpu/mcheck/mce.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)
arch/x86/kernel/cpu/mcheck/mce.c +4 -4
index 54bd1b2..325559d 100644
@@ -612,7 +612,7 @@ out:
  * This way we prevent any potential data corruption in a unrecoverable case
  * and also makes sure always all CPU's errors are examined.
  *
- * Also this detects the case of an machine check event coming from outer
+ * Also this detects the case of a machine check event coming from outer
  * space (not detected by any CPUs) In this case some external agent wants
  * us to shut down, so panic too.
  *
@@ -665,7 +665,7 @@ static void mce_reign(void)
 	 * No machine check event found. Must be some external
 	 * source or one CPU is hung. Panic.
 	 */
-	if (!m && tolerant < 3)
+	if (global_worst <= MCE_KEEP_SEVERITY && tolerant < 3)
 		mce_panic("Machine check from unknown source", NULL, NULL);
 
 	/*
@@ -889,11 +889,11 @@ void do_machine_check(struct pt_regs *regs, long error_code)
 	mce_setup(&m);
 
 	m.mcgstatus = mce_rdmsrl(MSR_IA32_MCG_STATUS);
-	no_way_out = mce_no_way_out(&m, &msg);
-
 	final = &__get_cpu_var(mces_seen);
 	*final = m;
 
+	no_way_out = mce_no_way_out(&m, &msg);
+
 	barrier();
 
 	/*