Skip to contentSkip to conversation

x86: MCE optimization and cleanups

PATCH In review Joerg Roedel 5 participants 12 messages Oct 11, 2007 – Oct 12, 2007 x86

lore
Share & tools
lore

Joerg Roedel

author
#0 lore raw

cover x86: MCE optimization and cleanups

This patchset includes two patches.

1. Checks for the MCA fetures as early as possible and signedness fixup.
2. Minor coding style cleanup.

author
#1 lore raw

PATCH 1/2 x86: mce init optimization and signedness fixup

MCG_CAP never reports a negative count of available error-reporting banks. Therefore, make nr_mce_banks unsigned.
Check for MCA/MCE feature bits as early as possible.

Signed-off-by Christoph Egger <[email protected]>
Signed-off-by Joerg Roedel <[email protected]>
---
 arch/x86/kernel/cpu/mcheck/mce.c |   12 ++++++++++--
 arch/x86/kernel/cpu/mcheck/mce.h |    2 +-
 2 files changed, 11 insertions(+), 3 deletions(-)
arch/x86/kernel/cpu/mcheck/mce.c +10 -2
index 34c781e..e1ac4f7 100644
@@ -17,7 +17,7 @@
 #include "mce.h"
 
 int mce_disabled = 0;
-int nr_mce_banks;
+unsigned int nr_mce_banks;
 
 EXPORT_SYMBOL_GPL(nr_mce_banks);	/* non-fatal.o */
 

@@ -33,8 +33,16 @@ void fastcall (*machine_check_vector)(struct pt_regs *, long error_code) = unexp
 /* This has to be run for each processor */
 void mcheck_init(struct cpuinfo_x86 *c)
 {
-	if (mce_disabled==1)
+	if (mce_disabled == 1) {
+		printk(KERN_INFO "MCE support disabled by bootparam\n");
 		return;
+	}
+
+	if (!cpu_has(c, X86_FEATURE_MCA) || !cpu_has(c, X86_FEATURE_MCE)) {
+		printk(KERN_INFO "CPU%i: No machine check support available\n",
+			smp_processor_id());
+		return;
+	}
 
 	switch (c->x86_vendor) {
 		case X86_VENDOR_AMD:
arch/x86/kernel/cpu/mcheck/mce.h +1 -1
index 81fb6e2..9cbe812 100644
@@ -10,5 +10,5 @@ void winchip_mcheck_init(struct cpuinfo_x86 *c);
 /* Call the installed machine check handler for this CPU setup. */
 extern fastcall void (*machine_check_vector)(struct pt_regs *, long error_code);
 
-extern int nr_mce_banks;
+extern unsigned int nr_mce_banks;
Signature · 1 line
1.5.2.5
#2 lore raw

Re: [PATCH 1/2] x86: mce init optimization and signedness fixup

arch/x86/kernel/cpu/mcheck/mce.c void fastcall (*machine_check_vector)(struct pt_regs *, long error_code) = unexp · line 44

Quoted 9 lines · Joerg Roedel

MCG_CAP never reports a negative count of available error-reporting banks. Therefore, make nr_mce_banks unsigned.
Check for MCA/MCE feature bits as early as possible.

arch/x86/kernel/cpu/mcheck/mce.c +5 -0
+
+	if (!cpu_has(c, X86_FEATURE_MCA) || !cpu_has(c, X86_FEATURE_MCE)) {
+		printk(KERN_INFO "CPU%i: No machine check support available\n",
+			smp_processor_id());
+		return;

This breaks winchip MCE support.

tglx

#3 lore raw

Re: [PATCH 1/2] x86: mce init optimization and signedness fixup

Quoted 5 lines · On Thursday 11 October 2007 15:51:49 Thomas Gleixner wrote:
Quoted 4 lines · Joerg Roedel

MCG_CAP never reports a negative count of available error-reporting banks. Therefore, make nr_mce_banks unsigned.
Check for MCA/MCE feature bits as early as possible.

Quoted 7 lines

arch/x86/kernel/cpu/mcheck/mce.c void fastcall (*machine_check_vector)(struct pt_regs *, long error_code) = unexp · line 44

Quoted 5 lines
arch/x86/kernel/cpu/mcheck/mce.c +5 -0
+
+	if (!cpu_has(c, X86_FEATURE_MCA) || !cpu_has(c, X86_FEATURE_MCE)) {
+		printk(KERN_INFO "CPU%i: No machine check support available\n",
+			smp_processor_id());
+		return;

This breaks winchip MCE support.

First, what is a winchip? It sounds to be something windows specific. ;) Second, can you explain in which way MCE support gets broken, please?

If you actually see the printk message, then machine check support is not available on that cpu on that machine, right?

Signature · 12 lines
AMD Saxony, Dresden, Germany
Operating System Research Center

Legal Information:
AMD Saxony Limited Liability Company & Co. KG
Sitz (Geschäftsanschrift):
   Wilschdorfer Landstr. 101, 01109 Dresden, Deutschland
Registergericht Dresden: HRA 4896
vertretungsberechtigter Komplementär:
   AMD Saxony LLC (Sitz Wilmington, Delaware, USA)
Geschäftsführer der AMD Saxony LLC:
   Dr. Hans-R. Deppe, Thomas McCoy
#4 lore raw

Re: [PATCH 1/2] x86: mce init optimization and signedness fixup

Quoted 14 lines · Christoph Egger
Quoted 5 lines · On Thursday 11 October 2007 15:51:49 Thomas Gleixner wrote:
Quoted 4 lines · Joerg Roedel

MCG_CAP never reports a negative count of available error-reporting banks. Therefore, make nr_mce_banks unsigned.
Check for MCA/MCE feature bits as early as possible.

Quoted 7 lines

arch/x86/kernel/cpu/mcheck/mce.c void fastcall (*machine_check_vector)(struct pt_regs *, long error_code) = unexp · line 44

Quoted 5 lines
arch/x86/kernel/cpu/mcheck/mce.c +5 -0
+
+	if (!cpu_has(c, X86_FEATURE_MCA) || !cpu_has(c, X86_FEATURE_MCE)) {
+		printk(KERN_INFO "CPU%i: No machine check support available\n",
+			smp_processor_id());
+		return;

This breaks winchip MCE support.

Quoted 2 lines

First, what is a winchip? It sounds to be something windows specific. ;) Second, can you explain in which way MCE support gets broken, please?

First, winchip is the code name of Centaurs early x86 cpus.

Second, those beasts do not have FEATURE_MCA, but they have FEATURE_MCE, so they support the fatal exception, but not the non fatal check.

tglx

#5 lore raw

Re: [PATCH 1/2] x86: mce init optimization and signedness fixup

Quoted 15 lines · On Thursday 11 October 2007 16:55:36 Thomas Gleixner wrote:
Quoted 14 lines · Christoph Egger
Quoted 12 lines · On Thursday 11 October 2007 15:51:49 Thomas Gleixner wrote:

arch/x86/kernel/cpu/mcheck/mce.c void fastcall (*machine_check_vector)(struct pt_regs *, long error_code) = unexp · line 44

Quoted 9 lines · Joerg Roedel

MCG_CAP never reports a negative count of available error-reporting banks. Therefore, make nr_mce_banks unsigned.
Check for MCA/MCE feature bits as early as possible.

arch/x86/kernel/cpu/mcheck/mce.c +5 -0
+
+	if (!cpu_has(c, X86_FEATURE_MCA) || !cpu_has(c, X86_FEATURE_MCE)) {
+		printk(KERN_INFO "CPU%i: No machine check support available\n",
+			smp_processor_id());
+		return;

This breaks winchip MCE support.

Quoted 7 lines
Quoted 2 lines

First, what is a winchip? It sounds to be something windows specific. ;) Second, can you explain in which way MCE support gets broken, please?

First, winchip is the code name of Centaurs early x86 cpus.

Second, those beasts do not have FEATURE_MCA, but they have FEATURE_MCE, so they support the fatal exception, but not the non fatal check.

So when I change the above code snippet to:

+ if (!cpu_has(c, X86_FEATURE_MCE)) {
+ printk(KERN_INFO "CPU%i: No machine check support available\n",
+ smp_processor_id());
+ return;

Would this make the whole patch acceptable then?

Signature · 12 lines
AMD Saxony, Dresden, Germany
Operating System Research Center

Legal Information:
AMD Saxony Limited Liability Company & Co. KG
Sitz (Geschäftsanschrift):
   Wilschdorfer Landstr. 101, 01109 Dresden, Deutschland
Registergericht Dresden: HRA 4896
vertretungsberechtigter Komplementär:
   AMD Saxony LLC (Sitz Wilmington, Delaware, USA)
Geschäftsführer der AMD Saxony LLC:
   Dr. Hans-R. Deppe, Thomas McCoy
#6 lore raw

Re: [PATCH 1/2] x86: mce init optimization and signedness fixup

Quoted 24 lines · Christoph Egger
Quoted 8 lines · On Thursday 11 October 2007 16:55:36 Thomas Gleixner wrote:
Quoted 8 lines
Quoted 7 lines

arch/x86/kernel/cpu/mcheck/mce.c void fastcall (*machine_check_vector)(struct pt_regs *, long error_code) = unexp · line 44

Quoted 5 lines
arch/x86/kernel/cpu/mcheck/mce.c +5 -0
+
+	if (!cpu_has(c, X86_FEATURE_MCA) || !cpu_has(c, X86_FEATURE_MCE)) {
+		printk(KERN_INFO "CPU%i: No machine check support available\n",
+			smp_processor_id());
+		return;

This breaks winchip MCE support.

Quoted 7 lines
Quoted 2 lines

First, what is a winchip? It sounds to be something windows specific. ;) Second, can you explain in which way MCE support gets broken, please?

First, winchip is the code name of Centaurs early x86 cpus.

Second, those beasts do not have FEATURE_MCA, but they have FEATURE_MCE, so they support the fatal exception, but not the non fatal check.

So when I change the above code snippet to:

arch/x86/kernel/cpu/mcheck/mce.c +4 -0
+	if (!cpu_has(c, X86_FEATURE_MCE)) {
+		printk(KERN_INFO "CPU%i: No machine check support available\n",
+			smp_processor_id());
+		return;
Quoted 1 line

Would this make the whole patch acceptable then?

Yeah, but then we can clean up the extra checks for _MCE in the various cpu type init functions as well.

tglx

#7 lore raw

Re: [PATCH 1/2] x86: mce init optimization and signedness fixup

Quoted 25 lines · Thomas Gleixner
Quoted 24 lines · Christoph Egger
Quoted 8 lines · On Thursday 11 October 2007 16:55:36 Thomas Gleixner wrote:
Quoted 8 lines
Quoted 7 lines

arch/x86/kernel/cpu/mcheck/mce.c void fastcall (*machine_check_vector)(struct pt_regs *, long error_code) = unexp · line 44

Quoted 5 lines
arch/x86/kernel/cpu/mcheck/mce.c +5 -0
+
+	if (!cpu_has(c, X86_FEATURE_MCA) || !cpu_has(c, X86_FEATURE_MCE)) {
+		printk(KERN_INFO "CPU%i: No machine check support available\n",
+			smp_processor_id());
+		return;

This breaks winchip MCE support.

Quoted 7 lines
Quoted 2 lines

First, what is a winchip? It sounds to be something windows specific. ;) Second, can you explain in which way MCE support gets broken, please?

First, winchip is the code name of Centaurs early x86 cpus.

Second, those beasts do not have FEATURE_MCA, but they have FEATURE_MCE, so they support the fatal exception, but not the non fatal check.

So when I change the above code snippet to:

arch/x86/kernel/cpu/mcheck/mce.c +4 -0
+	if (!cpu_has(c, X86_FEATURE_MCE)) {
+		printk(KERN_INFO "CPU%i: No machine check support available\n",
+			smp_processor_id());
+		return;
Quoted 4 lines
Quoted 1 line

Would this make the whole patch acceptable then?

Yeah, but then we can clean up the extra checks for _MCE in the various cpu type init functions as well.

I question the value of adding the printk.
It's not a failure, there's nothing the user can do about it, and it adds no real value, just more noise to the dmesg.

Dave

Signature · 1 line
http://www.codemonkey.org.uk
#8 lore raw

Re: [PATCH 1/2] x86: mce init optimization and signedness fixup

In reply to Thomas Gleixner
Quoted 27 lines · On Thursday 11 October 2007 18:50:12 Thomas Gleixner wrote:
Quoted 26 lines · Christoph Egger
Quoted 12 lines · On Thursday 11 October 2007 16:55:36 Thomas Gleixner wrote:
Quoted 11 lines
Quoted 7 lines

arch/x86/kernel/cpu/mcheck/mce.c void fastcall (*machine_check_vector)(struct pt_regs *, long error_code) = unexp · line 44

Quoted 5 lines
(quoted diff) +2 -0
+
+	if (!cpu_has(c, X86_FEATURE_MCA) || !cpu_has(c,

X86_FEATURE_MCE)) { + printk(KERN_INFO "CPU%i: No machine check
support available\n", + smp_processor_id());

arch/x86/kernel/cpu/mcheck/mce.c +1 -0
+		return;

This breaks winchip MCE support.

First, what is a winchip? It sounds to be something windows specific. ;) Second, can you explain in which way MCE support gets broken, please?

Quoted 5 lines

First, winchip is the code name of Centaurs early x86 cpus.

Second, those beasts do not have FEATURE_MCA, but they have FEATURE_MCE, so they support the fatal exception, but not the non fatal check.

So when I change the above code snippet to:

arch/x86/kernel/cpu/mcheck/mce.c +4 -0
+	if (!cpu_has(c, X86_FEATURE_MCE)) {
+		printk(KERN_INFO "CPU%i: No machine check support available\n",
+			smp_processor_id());
+		return;
Quoted 4 lines
Quoted 1 line

Would this make the whole patch acceptable then?

Yeah, but then we can clean up the extra checks for _MCE in the various cpu type init functions as well.

Will you do the little modification in your tree after applying my patch or do you want me to resend the patch with that little modification.

Christoph

Signature · 12 lines
AMD Saxony, Dresden, Germany
Operating System Research Center

Legal Information:
AMD Saxony Limited Liability Company & Co. KG
Sitz (Geschäftsanschrift):
   Wilschdorfer Landstr. 101, 01109 Dresden, Deutschland
Registergericht Dresden: HRA 4896
vertretungsberechtigter Komplementär:
   AMD Saxony LLC (Sitz Wilmington, Delaware, USA)
Geschäftsführer der AMD Saxony LLC:
   Dr. Hans-R. Deppe, Thomas McCoy
#9 lore raw

Re: [PATCH 1/2] x86: mce init optimization and signedness fixup

In reply to Christoph Egger
Quoted 8 lines

So when I change the above code snippet to:

arch/x86/kernel/cpu/mcheck/mce.c +4 -0
+	if (!cpu_has(c, X86_FEATURE_MCE)) {
+		printk(KERN_INFO "CPU%i: No machine check support available\n",
+			smp_processor_id());
+		return;

Would this make the whole patch acceptable then?

I think the fundamental direction is wrong. "Do you have a machine check like facility" is a CPU specific question that belongs in the CPU specific code.

If some of that CPU specific code is wrong, fix it there.

Alan

#10 lore raw

Re: [PATCH 1/2] x86: mce init optimization and signedness fixup

In reply to Thomas Gleixner
Quoted 16 lines · On Thu, 11 Oct 2007 16:55:36 +0200 (CEST) Thomas Gleixner <[email protected]>…
Quoted 14 lines · Christoph Egger
Quoted 12 lines · On Thursday 11 October 2007 15:51:49 Thomas Gleixner wrote:

arch/x86/kernel/cpu/mcheck/mce.c void fastcall (*machine_check_vector)(struct pt_regs *, long error_code) = unexp · line 44

Quoted 9 lines · Joerg Roedel

MCG_CAP never reports a negative count of available error-reporting banks. Therefore, make nr_mce_banks unsigned.
Check for MCA/MCE feature bits as early as possible.

arch/x86/kernel/cpu/mcheck/mce.c +5 -0
+
+	if (!cpu_has(c, X86_FEATURE_MCA) || !cpu_has(c, X86_FEATURE_MCE)) {
+		printk(KERN_INFO "CPU%i: No machine check support available\n",
+			smp_processor_id());
+		return;

This breaks winchip MCE support.

Quoted 7 lines
Quoted 2 lines

First, what is a winchip? It sounds to be something windows specific. ;) Second, can you explain in which way MCE support gets broken, please?

First, winchip is the code name of Centaurs early x86 cpus.

Second, those beasts do not have FEATURE_MCA, but they have FEATURE_MCE, so they support the fatal exception, but not the non fatal check.

Ditto some of the original Intel Preventium processors which have limited MC support hidden in the magic 'Appendix H'

Alan

author
#11 lore raw

PATCH 2/2 x86: mce minor indent cleanup

In reply to Joerg Roedel

remove one indent level

Signed-off-by Christoph Egger <[email protected]>
Signed-off-by Joerg Roedel <[email protected]>
---
 arch/x86/kernel/cpu/mcheck/mce.c |   40 +++++++++++++++++++-------------------
 1 files changed, 20 insertions(+), 20 deletions(-)
arch/x86/kernel/cpu/mcheck/mce.c +20 -20
index e1ac4f7..bc61f1e 100644
@@ -45,26 +45,26 @@ void mcheck_init(struct cpuinfo_x86 *c)
 	}
 
 	switch (c->x86_vendor) {
-		case X86_VENDOR_AMD:
-			amd_mcheck_init(c);
-			break;
-
-		case X86_VENDOR_INTEL:
-			if (c->x86==5)
-				intel_p5_mcheck_init(c);
-			if (c->x86==6)
-				intel_p6_mcheck_init(c);
-			if (c->x86==15)
-				intel_p4_mcheck_init(c);
-			break;
-
-		case X86_VENDOR_CENTAUR:
-			if (c->x86==5)
-				winchip_mcheck_init(c);
-			break;
-
-		default:
-			break;
+	case X86_VENDOR_AMD:
+		amd_mcheck_init(c);
+		break;
+
+	case X86_VENDOR_INTEL:
+		if (c->x86==5)
+			intel_p5_mcheck_init(c);
+		if (c->x86==6)
+			intel_p6_mcheck_init(c);
+		if (c->x86==15)
+			intel_p4_mcheck_init(c);
+		break;
+
+	case X86_VENDOR_CENTAUR:
+		if (c->x86==5)
+			winchip_mcheck_init(c);
+		break;
+
+	default:
+		break;
 	}
 }
Signature · 1 line
1.5.2.5