Add MCE resume under ia32

PATCH In review Shaohua Li 2 participants 6 messages Aug 23, 2005 – Aug 25, 2005

Share & tools

Shaohua Li

author

2005-08-23 01:58 UTC #0 lore raw

PATCH 1/1 Add MCE resume under ia32

Hi,
It's widely seen a MCE non-fatal error reported after resume. It seems MCE resume is lacked under ia32. This patch tries to fix the gap.

Signed-off-by Shaohua Li<[email protected]>

---

 linux-2.6.13-rc6-root/arch/i386/kernel/cpu/mcheck/k7.c      |    2 +-
 linux-2.6.13-rc6-root/arch/i386/kernel/cpu/mcheck/mce.c     |    4 ++--
 linux-2.6.13-rc6-root/arch/i386/kernel/cpu/mcheck/p4.c      |    4 ++--
 linux-2.6.13-rc6-root/arch/i386/kernel/cpu/mcheck/p5.c      |    2 +-
 linux-2.6.13-rc6-root/arch/i386/kernel/cpu/mcheck/p6.c      |    2 +-
 linux-2.6.13-rc6-root/arch/i386/kernel/cpu/mcheck/winchip.c |    2 +-
 linux-2.6.13-rc6-root/arch/i386/power/cpu.c                 |    5 +++++
 7 files changed, 13 insertions(+), 8 deletions(-)

arch/i386/kernel/cpu/mcheck/k7.c renamed from arch/i386/kernel/cpu/mcheck/k7.c~mcheck_resume +1 -1

diff -puN arch/i386/kernel/cpu/mcheck/k7.c~mcheck_resume arch/i386/kernel/cpu/mcheck/k7.c
@@ -69,7 +69,7 @@ static fastcall void k7_machine_check(st
 
 
 /* AMD K7 machine check is Intel like */
-void __devinit amd_mcheck_init(struct cpuinfo_x86 *c)
+void amd_mcheck_init(struct cpuinfo_x86 *c)
 {
 	u32 l, h;
 	int i;

arch/i386/kernel/cpu/mcheck/mce.c renamed from arch/i386/kernel/cpu/mcheck/mce.c~mcheck_resume +2 -2

diff -puN arch/i386/kernel/cpu/mcheck/mce.c~mcheck_resume arch/i386/kernel/cpu/mcheck/mce.c
@@ -16,7 +16,7 @@
 
 #include "mce.h"
 
-int mce_disabled __devinitdata = 0;
+int mce_disabled = 0;
 int nr_mce_banks;
 
 EXPORT_SYMBOL_GPL(nr_mce_banks);	/* non-fatal.o */

@@ -31,7 +31,7 @@ static fastcall void unexpected_machine_
 void fastcall (*machine_check_vector)(struct pt_regs *, long error_code) = unexpected_machine_check;
 
 /* This has to be run for each processor */
-void __devinit mcheck_init(struct cpuinfo_x86 *c)
+void mcheck_init(struct cpuinfo_x86 *c)
 {
 	if (mce_disabled==1)
 		return;

arch/i386/kernel/cpu/mcheck/p4.c renamed from arch/i386/kernel/cpu/mcheck/p4.c~mcheck_resume +2 -2

diff -puN arch/i386/kernel/cpu/mcheck/p4.c~mcheck_resume arch/i386/kernel/cpu/mcheck/p4.c
@@ -78,7 +78,7 @@ fastcall void smp_thermal_interrupt(stru
 }
 
 /* P4/Xeon Thermal regulation detect and init */
-static void __devinit intel_init_thermal(struct cpuinfo_x86 *c)
+static void intel_init_thermal(struct cpuinfo_x86 *c)
 {
 	u32 l, h;
 	unsigned int cpu = smp_processor_id();

@@ -232,7 +232,7 @@ static fastcall void intel_machine_check
 }
 
 
-void __devinit intel_p4_mcheck_init(struct cpuinfo_x86 *c)
+void intel_p4_mcheck_init(struct cpuinfo_x86 *c)
 {
 	u32 l, h;
 	int i;

arch/i386/kernel/cpu/mcheck/p5.c renamed from arch/i386/kernel/cpu/mcheck/p5.c~mcheck_resume +1 -1

diff -puN arch/i386/kernel/cpu/mcheck/p5.c~mcheck_resume arch/i386/kernel/cpu/mcheck/p5.c
@@ -29,7 +29,7 @@ static fastcall void pentium_machine_che
 }
 
 /* Set up machine check reporting for processors with Intel style MCE */
-void __devinit intel_p5_mcheck_init(struct cpuinfo_x86 *c)
+void intel_p5_mcheck_init(struct cpuinfo_x86 *c)
 {
 	u32 l, h;

arch/i386/kernel/cpu/mcheck/p6.c renamed from arch/i386/kernel/cpu/mcheck/p6.c~mcheck_resume +1 -1

diff -puN arch/i386/kernel/cpu/mcheck/p6.c~mcheck_resume arch/i386/kernel/cpu/mcheck/p6.c
@@ -80,7 +80,7 @@ static fastcall void intel_machine_check
 }
 
 /* Set up machine check reporting for processors with Intel style MCE */
-void __devinit intel_p6_mcheck_init(struct cpuinfo_x86 *c)
+void intel_p6_mcheck_init(struct cpuinfo_x86 *c)
 {
 	u32 l, h;
 	int i;

arch/i386/kernel/cpu/mcheck/winchip.c renamed from arch/i386/kernel/cpu/mcheck/winchip.c~mcheck_resume +1 -1

diff -puN arch/i386/kernel/cpu/mcheck/winchip.c~mcheck_resume arch/i386/kernel/cpu/mcheck/winchip.c
@@ -23,7 +23,7 @@ static fastcall void winchip_machine_che
 }
 
 /* Set up machine check reporting on the Winchip C6 series */
-void __devinit winchip_mcheck_init(struct cpuinfo_x86 *c)
+void winchip_mcheck_init(struct cpuinfo_x86 *c)
 {
 	u32 lo, hi;
 	machine_check_vector = winchip_machine_check;

arch/i386/power/cpu.c renamed from arch/i386/power/cpu.c~mcheck_resume +5 -0

diff -puN arch/i386/power/cpu.c~mcheck_resume arch/i386/power/cpu.c
@@ -104,6 +104,8 @@ static void fix_processor_context(void)
 
 }
 
+extern void mcheck_init(struct cpuinfo_x86 *c);
+
 void __restore_processor_state(struct saved_context *ctxt)
 {
 	/*

@@ -138,6 +140,9 @@ void __restore_processor_state(struct sa
 	fix_processor_context();
 	do_fpu_end();
 	mtrr_ap_init();
+#ifdef CONFIG_X86_MCE
+	mcheck_init(&boot_cpu_data);
+#endif
 }
 
 void restore_processor_state(void)

Pavel Machek

2005-08-23 10:33 UTC #1 lore raw

Hi!

Quoted 2 lines

It's widely seen a MCE non-fatal error reported after resume. It seems MCE resume is lacked under ia32. This patch tries to fix the gap.

Well, you patch seems like missing piece of puzzle, but:

a) we probably want to do it for x86-64, too, and

arch/i386/power/cpu.c static void fix_processor_context(void) · line 111

Quoted 12 lines

diff -puN arch/i386/power/cpu.c~mcheck_resume arch/i386/power/cpu.c
arch/i386/power/cpu.c renamed from arch/i386/power/cpu.c~mcheck_resume +2 -0
@@ -104,6 +104,8 @@ static void fix_processor_context(void)
 
 }
 
+extern void mcheck_init(struct cpuinfo_x86 *c);
+
 void __restore_processor_state(struct saved_context *ctxt)
 {
 	/*

this should go to some header file and most importantly

arch/i386/power/cpu.c void __restore_processor_state(struct sa · line 146

Quoted 8 lines

arch/i386/power/cpu.c renamed from arch/i386/power/cpu.c~mcheck_resume +3 -0

@@ -138,6 +140,9 @@ void __restore_processor_state(struct sa
 	fix_processor_context();
 	do_fpu_end();
 	mtrr_ap_init();
+#ifdef CONFIG_X86_MCE
+	mcheck_init(&boot_cpu_data);
+#endif
 }

c) can't we register MCEs like some kind of system device so that this kind of hooks is not neccessary?
Pavel

Signature · 1 line

if you have sharp zaurus hardware you don't need... you know my address

Shaohua Li

author

2005-08-24 01:14 UTC #2 lore raw

Quoted 6 lines · Pavel Machek

Quoted 2 lines
It's widely seen a MCE non-fatal error reported after resume. It seems MCE resume is lacked under ia32. This patch tries to fix the gap.
Well, you patch seems like missing piece of puzzle, but:
a) we probably want to do it for x86-64, too, and

x86-64 has resume support. It uses 'on_each_cpu' in resume method, which is known broken. We'd better fix it.

Quoted 8 lines

Quoted 8 lines
diff -puN arch/i386/power/cpu.c~mcheck_resume arch/i386/power/cpu.c
arch/i386/power/cpu.c renamed from arch/i386/power/cpu.c~mcheck_resume +0 -0
@@ -104,6 +104,8 @@ static void fix_processor_context(void)
 
 }
 

Quoted 8 lines

arch/i386/power/cpu.c static void fix_processor_context(void) · line 111
Quoted 5 lines
arch/i386/power/cpu.c renamed from arch/i386/power/cpu.c~mcheck_resume +2 -0
+extern void mcheck_init(struct cpuinfo_x86 *c);
+
 void __restore_processor_state(struct saved_context *ctxt)
 {
 	/*
this should go to some header file and most importantly

If you agree my other points, I'll do this.

Quoted 12 lines

arch/i386/power/cpu.c void __restore_processor_state(struct sa · line 146
Quoted 9 lines
arch/i386/power/cpu.c renamed from arch/i386/power/cpu.c~mcheck_resume +3 -0
@@ -138,6 +140,9 @@ void __restore_processor_state(struct sa
 	fix_processor_context();
 	do_fpu_end();
 	mtrr_ap_init();
+#ifdef CONFIG_X86_MCE
+	mcheck_init(&boot_cpu_data);
+#endif
 }
c) can't we register MCEs like some kind of system device so that this kind of hooks is not neccessary?

Like x86-64 does, right? In this way, we must register a device for each cpu. But APs directly call mcheck_init in resume time (cpuhotplug framework). Only BP requires to call the resume method, so I think restore_processor_state calls it might be cleaner.

Thanks,
Shaohua

Pavel Machek

2005-08-24 08:51 UTC #3 lore raw

Hi!

Quoted 15 lines

Quoted 7 lines
Quoted 7 lines
diff -puN arch/i386/power/cpu.c~mcheck_resume arch/i386/power/cpu.c
arch/i386/power/cpu.c renamed from arch/i386/power/cpu.c~mcheck_resume +0 -0
@@ -104,6 +104,8 @@ static void fix_processor_context(void)
 
 }
 
Quoted 8 lines
arch/i386/power/cpu.c static void fix_processor_context(void) · line 111
Quoted 5 lines
arch/i386/power/cpu.c renamed from arch/i386/power/cpu.c~mcheck_resume +2 -0
+extern void mcheck_init(struct cpuinfo_x86 *c);
+
 void __restore_processor_state(struct saved_context *ctxt)
 {
 	/*
this should go to some header file and most importantly

Quoted 1 line

If you agree my other points, I'll do this.

Ok.

Quoted 11 lines

Quoted 11 lines
arch/i386/power/cpu.c void __restore_processor_state(struct sa · line 146
Quoted 8 lines
arch/i386/power/cpu.c renamed from arch/i386/power/cpu.c~mcheck_resume +3 -0
@@ -138,6 +140,9 @@ void __restore_processor_state(struct sa
 	fix_processor_context();
 	do_fpu_end();
 	mtrr_ap_init();
+#ifdef CONFIG_X86_MCE
+	mcheck_init(&boot_cpu_data);
+#endif
 }
c) can't we register MCEs like some kind of system device so that this kind of hooks is not neccessary?

Quoted 4 lines

Like x86-64 does, right? In this way, we must register a device for each cpu. But APs directly call mcheck_init in resume time (cpuhotplug framework). Only BP requires to call the resume method, so I think restore_processor_state calls it might be cleaner.

Ahha, ok.
Pavel

Signature · 1 line

if you have sharp zaurus hardware you don't need... you know my address

Shaohua Li

author

2005-08-25 02:33 UTC #4 lore raw

Quoted 18 lines · Pavel Machek

Hi!
Quoted 16 lines
Quoted 7 lines
Quoted 7 lines
diff -puN arch/i386/power/cpu.c~mcheck_resume arch/i386/power/cpu.c
arch/i386/power/cpu.c renamed from arch/i386/power/cpu.c~mcheck_resume +0 -0
@@ -104,6 +104,8 @@ static void fix_processor_context(void)
 
 }
 
Quoted 8 lines
arch/i386/power/cpu.c static void fix_processor_context(void) · line 111
Quoted 5 lines
arch/i386/power/cpu.c renamed from arch/i386/power/cpu.c~mcheck_resume +2 -0
+extern void mcheck_init(struct cpuinfo_x86 *c);
+
 void __restore_processor_state(struct saved_context *ctxt)
 {
 	/*
this should go to some header file and most importantly
If you agree my other points, I'll do this.

Ok, updated one.
Reinitialize MCE to avoid MCE non-fatal warning after resume.

Signed-off-by Shaohua Li<[email protected]>

---

 linux-2.6.13-rc6-root/arch/i386/kernel/cpu/common.c         |    5 +----
 linux-2.6.13-rc6-root/arch/i386/kernel/cpu/mcheck/k7.c      |    2 +-
 linux-2.6.13-rc6-root/arch/i386/kernel/cpu/mcheck/mce.c     |    4 ++--
 linux-2.6.13-rc6-root/arch/i386/kernel/cpu/mcheck/p4.c      |    4 ++--
 linux-2.6.13-rc6-root/arch/i386/kernel/cpu/mcheck/p5.c      |    2 +-
 linux-2.6.13-rc6-root/arch/i386/kernel/cpu/mcheck/p6.c      |    2 +-
 linux-2.6.13-rc6-root/arch/i386/kernel/cpu/mcheck/winchip.c |    2 +-
 linux-2.6.13-rc6-root/arch/i386/power/cpu.c                 |    1 +
 linux-2.6.13-rc6-root/include/asm-i386/processor.h          |    6 ++++++
 9 files changed, 16 insertions(+), 12 deletions(-)

arch/i386/kernel/cpu/mcheck/k7.c renamed from arch/i386/kernel/cpu/mcheck/k7.c~mcheck_resume +1 -1

diff -puN arch/i386/kernel/cpu/mcheck/k7.c~mcheck_resume arch/i386/kernel/cpu/mcheck/k7.c
@@ -69,7 +69,7 @@ static fastcall void k7_machine_check(st
 
 
 /* AMD K7 machine check is Intel like */
-void __devinit amd_mcheck_init(struct cpuinfo_x86 *c)
+void amd_mcheck_init(struct cpuinfo_x86 *c)
 {
 	u32 l, h;
 	int i;

arch/i386/kernel/cpu/mcheck/mce.c renamed from arch/i386/kernel/cpu/mcheck/mce.c~mcheck_resume +2 -2

diff -puN arch/i386/kernel/cpu/mcheck/mce.c~mcheck_resume arch/i386/kernel/cpu/mcheck/mce.c
@@ -16,7 +16,7 @@
 
 #include "mce.h"
 
-int mce_disabled __devinitdata = 0;
+int mce_disabled = 0;
 int nr_mce_banks;
 
 EXPORT_SYMBOL_GPL(nr_mce_banks);	/* non-fatal.o */

@@ -31,7 +31,7 @@ static fastcall void unexpected_machine_
 void fastcall (*machine_check_vector)(struct pt_regs *, long error_code) = unexpected_machine_check;
 
 /* This has to be run for each processor */
-void __devinit mcheck_init(struct cpuinfo_x86 *c)
+void mcheck_init(struct cpuinfo_x86 *c)
 {
 	if (mce_disabled==1)
 		return;

arch/i386/kernel/cpu/mcheck/p4.c renamed from arch/i386/kernel/cpu/mcheck/p4.c~mcheck_resume +2 -2

diff -puN arch/i386/kernel/cpu/mcheck/p4.c~mcheck_resume arch/i386/kernel/cpu/mcheck/p4.c
@@ -78,7 +78,7 @@ fastcall void smp_thermal_interrupt(stru
 }
 
 /* P4/Xeon Thermal regulation detect and init */
-static void __devinit intel_init_thermal(struct cpuinfo_x86 *c)
+static void intel_init_thermal(struct cpuinfo_x86 *c)
 {
 	u32 l, h;
 	unsigned int cpu = smp_processor_id();

@@ -232,7 +232,7 @@ static fastcall void intel_machine_check
 }
 
 
-void __devinit intel_p4_mcheck_init(struct cpuinfo_x86 *c)
+void intel_p4_mcheck_init(struct cpuinfo_x86 *c)
 {
 	u32 l, h;
 	int i;

arch/i386/kernel/cpu/mcheck/p5.c renamed from arch/i386/kernel/cpu/mcheck/p5.c~mcheck_resume +1 -1

diff -puN arch/i386/kernel/cpu/mcheck/p5.c~mcheck_resume arch/i386/kernel/cpu/mcheck/p5.c
@@ -29,7 +29,7 @@ static fastcall void pentium_machine_che
 }
 
 /* Set up machine check reporting for processors with Intel style MCE */
-void __devinit intel_p5_mcheck_init(struct cpuinfo_x86 *c)
+void intel_p5_mcheck_init(struct cpuinfo_x86 *c)
 {
 	u32 l, h;

arch/i386/kernel/cpu/mcheck/p6.c renamed from arch/i386/kernel/cpu/mcheck/p6.c~mcheck_resume +1 -1

diff -puN arch/i386/kernel/cpu/mcheck/p6.c~mcheck_resume arch/i386/kernel/cpu/mcheck/p6.c
@@ -80,7 +80,7 @@ static fastcall void intel_machine_check
 }
 
 /* Set up machine check reporting for processors with Intel style MCE */
-void __devinit intel_p6_mcheck_init(struct cpuinfo_x86 *c)
+void intel_p6_mcheck_init(struct cpuinfo_x86 *c)
 {
 	u32 l, h;
 	int i;

arch/i386/kernel/cpu/mcheck/winchip.c renamed from arch/i386/kernel/cpu/mcheck/winchip.c~mcheck_resume +1 -1

diff -puN arch/i386/kernel/cpu/mcheck/winchip.c~mcheck_resume arch/i386/kernel/cpu/mcheck/winchip.c
@@ -23,7 +23,7 @@ static fastcall void winchip_machine_che
 }
 
 /* Set up machine check reporting on the Winchip C6 series */
-void __devinit winchip_mcheck_init(struct cpuinfo_x86 *c)
+void winchip_mcheck_init(struct cpuinfo_x86 *c)
 {
 	u32 lo, hi;
 	machine_check_vector = winchip_machine_check;

arch/i386/power/cpu.c renamed from arch/i386/power/cpu.c~mcheck_resume +1 -0

diff -puN arch/i386/power/cpu.c~mcheck_resume arch/i386/power/cpu.c
@@ -138,6 +138,7 @@ void __restore_processor_state(struct sa
 	fix_processor_context();
 	do_fpu_end();
 	mtrr_ap_init();
+	mcheck_init(&boot_cpu_data);
 }
 
 void restore_processor_state(void)

include/asm-i386/processor.h renamed from include/asm-i386/processor.h~mcheck_resume +6 -0

diff -puN include/asm-i386/processor.h~mcheck_resume include/asm-i386/processor.h
@@ -702,4 +702,10 @@ extern void mtrr_bp_init(void);
 #define mtrr_bp_init() do {} while (0)
 #endif
 
+#ifdef CONFIG_X86_MCE
+extern void mcheck_init(struct cpuinfo_x86 *c);
+#else
+#define mcheck_init(c) do {} while(0)
+#endif
+
 #endif /* __ASM_I386_PROCESSOR_H */

arch/i386/kernel/cpu/common.c renamed from arch/i386/kernel/cpu/common.c~mcheck_resume +1 -4

diff -puN arch/i386/kernel/cpu/common.c~mcheck_resume arch/i386/kernel/cpu/common.c
@@ -30,8 +30,6 @@ static int disable_x86_serial_nr __devin
 
 struct cpu_dev * cpu_devs[X86_VENDOR_NUM] = {};
 
-extern void mcheck_init(struct cpuinfo_x86 *c);
-
 extern int disable_pse;
 
 static void default_init(struct cpuinfo_x86 * c)

@@ -429,9 +427,8 @@ void __devinit identify_cpu(struct cpuin
 	}
 
 	/* Init Machine Check Exception if available. */
-#ifdef CONFIG_X86_MCE
 	mcheck_init(c);
-#endif
+
 	if (c == &boot_cpu_data)
 		sysenter_setup();
 	enable_sep_cpu();

Pavel Machek

2005-08-25 09:08 UTC #5 lore raw

Quoted 18 lines · Shaohua Li

Quoted 17 lines · Pavel Machek
Hi!
Quoted 15 lines
Quoted 15 lines
arch/i386/power/cpu.c static void fix_processor_context(void) · line 111
Quoted 12 lines
diff -puN arch/i386/power/cpu.c~mcheck_resume arch/i386/power/cpu.c
arch/i386/power/cpu.c renamed from arch/i386/power/cpu.c~mcheck_resume +2 -0
@@ -104,6 +104,8 @@ static void fix_processor_context(void)
 
 }
 
+extern void mcheck_init(struct cpuinfo_x86 *c);
+
 void __restore_processor_state(struct saved_context *ctxt)
 {
 	/*
this should go to some header file and most importantly

Quoted 3 lines

Quoted 1 line
Quoted 1 line
If you agree my other points, I'll do this.
Ok, updated one.
Reinitialize MCE to avoid MCE non-fatal warning after resume.

ACK.
Pavel

Signature · 1 line

if you have sharp zaurus hardware you don't need... you know my address