2008-07-07 23:10:25

by Andi Kleen

[permalink] [raw]
Subject: [PATCH] [0/10] Use 64bit x86 machine check code for 32bit too v2


This patchkit uses the 64bit machine check code which is better in many
ways on 32bit x86 too. This is also the basis for some future machine
check work.

The 64bit machine check code is in many ways much better than
the 32bit machine check code: it is more specification compliant,
is cleaner, only has a single code base versus one per CPU,
has better infrastructure for recovery, has a cleaner way to communicate
with user space etc. etc.

It requires testing especially on older systems (on newer
ones it should be already tested well in 64bit systems).

The patchkit contains several parts:
- It ports over a few needed quirks (for older Intel and older
AMD CPUs) to the 64bit kernel.
- It changes the 64bit code to be 32bit clean in its data structures
(mostly just unsigned long -> u64 where needed)
- It drops some unused functionality that cannot be easily implemented on 32bit
and didn't seem worth ifdefing

Tested by doing some software level error injection on a few
different machines

I request this code is merged into the appropiate tree for linux-next
for wider testing. It's not .27 ready, but hopefully .28, but it requires
wider exposure now.

v2: Fix compilation problems noted by hpa in some configurations
Fix strict_strtoul() conversion

-Andi


2008-07-07 23:10:41

by Andi Kleen

[permalink] [raw]
Subject: [PATCH] [2/10] MCE: Implement the PPro bank 0 quirk in the 64bit machine check code


Quoting the comment:

* SDM documents that on family 6 bank 0 should not be written
* because it aliases to another special BIOS controlled
* register.
* But it's not aliased anymore on model 0x1a+
* Don't ignore bank 0 completely because there could be a valid
* event later, merely don't write CTL0.

This is mostly a port on the 32bit code, except that 32bit
always didn't write it and didn't have the 0x1a heuristic. I checked
with the CPU designers that the quirk is not required starting with
this model.

Signed-off-by: Andi Kleen <[email protected]>

---
arch/x86/kernel/cpu/mcheck/mce_64.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)

Index: linux/arch/x86/kernel/cpu/mcheck/mce_64.c
===================================================================
--- linux.orig/arch/x86/kernel/cpu/mcheck/mce_64.c
+++ linux/arch/x86/kernel/cpu/mcheck/mce_64.c
@@ -433,6 +433,7 @@ static __init int periodic_mcheck_init(v
}
__initcall(periodic_mcheck_init);

+static int dont_init_bank0;

/*
* Initialize Machine Checks for a CPU.
@@ -462,7 +463,8 @@ static void mce_init(void *dummy)
wrmsr(MSR_IA32_MCG_CTL, 0xffffffff, 0xffffffff);

for (i = 0; i < banks; i++) {
- wrmsrl(MSR_IA32_MC0_CTL+4*i, bank[i]);
+ if (!(i == 0 && dont_init_bank0))
+ wrmsrl(MSR_IA32_MC0_CTL+4*i, bank[i]);
wrmsrl(MSR_IA32_MC0_STATUS+4*i, 0);
}
}
@@ -481,6 +483,18 @@ static void __cpuinit mce_cpu_quirks(str
by default and leave crap in there. Don't log. */
mce_bootlog = 0;
}
+ if (c->x86_vendor == X86_VENDOR_INTEL) {
+ /*
+ * SDM documents that on family 6 bank 0 should not be written
+ * because it aliases to another special BIOS controlled
+ * register.
+ * But it's not aliased anymore on model 0x1a+
+ * Don't ignore bank 0 completely because there could be a valid
+ * event later, merely don't write CTL0.
+ */
+ if (c->x86 == 6 && c->x86_model < 0x1A)
+ dont_init_bank0 = 1;
+ }

}

2008-07-07 23:11:23

by Andi Kleen

[permalink] [raw]
Subject: [PATCH] [3/10] MCE: Port K7 bank 0 quirk to 64bit mce code


Various K7 have broken bank 0s. Don't enable it by default

Port from the 32bit code.

Signed-off-by: Andi Kleen <[email protected]>

---
arch/x86/kernel/cpu/mcheck/mce_64.c | 6 ++++++
1 file changed, 6 insertions(+)

Index: linux/arch/x86/kernel/cpu/mcheck/mce_64.c
===================================================================
--- linux.orig/arch/x86/kernel/cpu/mcheck/mce_64.c
+++ linux/arch/x86/kernel/cpu/mcheck/mce_64.c
@@ -482,6 +482,12 @@ static void __cpuinit mce_cpu_quirks(str
/* Lots of broken BIOS around that don't clear them
by default and leave crap in there. Don't log. */
mce_bootlog = 0;
+ /*
+ * Various K7s with broken bank 0 around. Always disable
+ * by default.
+ */
+ if (c->x86 == 6)
+ bank[0] = 0;
}
if (c->x86_vendor == X86_VENDOR_INTEL) {
/*

2008-07-07 23:10:54

by Andi Kleen

[permalink] [raw]
Subject: [PATCH] [1/10] MCE: Make 64bit mce code 32bit clean v2


Mostly replace unsigned long with u64s if they need to contain 64bit
values.

Also replace the simple_strtouls with strict_strtouls to shut
up checkpatch warnings.

v2: Do the strict_strtoull conversions correctly.

Signed-off-by: Andi Kleen <[email protected]>

---
arch/x86/kernel/cpu/mcheck/mce_64.c | 12 ++++++------
include/asm-x86/mce.h | 24 ++++++++++++------------
2 files changed, 18 insertions(+), 18 deletions(-)

Index: linux/arch/x86/kernel/cpu/mcheck/mce_64.c
===================================================================
--- linux.orig/arch/x86/kernel/cpu/mcheck/mce_64.c
+++ linux/arch/x86/kernel/cpu/mcheck/mce_64.c
@@ -46,7 +46,7 @@ static int mce_dont_init;
*/
static int tolerant = 1;
static int banks;
-static unsigned long bank[NR_BANKS] = { [0 ... NR_BANKS-1] = ~0UL };
+static u64 bank[NR_BANKS] = { [0 ... NR_BANKS-1] = ~0UL };
static unsigned long notify_user;
static int rip_msr;
static int mce_bootlog = -1;
@@ -129,15 +129,15 @@ static void print_mce(struct mce *m)
"and contact your hardware vendor\n");
}

-static void mce_panic(char *msg, struct mce *backup, unsigned long start)
+static void mce_panic(char *msg, struct mce *backup, u64 start)
{
int i;

oops_begin();
for (i = 0; i < MCE_LOG_LEN; i++) {
- unsigned long tsc = mcelog.entry[i].tsc;
+ u64 tsc = mcelog.entry[i].tsc;

- if (time_before(tsc, start))
+ if (time_before64(tsc, start))
continue;
print_mce(&mcelog.entry[i]);
if (backup && mcelog.entry[i].tsc == backup->tsc)
@@ -754,15 +754,15 @@ DEFINE_PER_CPU(struct sys_device, device
/* Why are there no generic functions for this? */
#define ACCESSOR(name, var, start) \
static ssize_t show_ ## name(struct sys_device *s, char *buf) { \
- return sprintf(buf, "%lx\n", (unsigned long)var); \
+ return sprintf(buf, "%Lx\n", (u64)var); \
} \
static ssize_t set_ ## name(struct sys_device *s,const char *buf,size_t siz) { \
- char *end; \
- unsigned long new = simple_strtoul(buf, &end, 0); \
- if (end == buf) return -EINVAL; \
+ u64 new; \
+ int err = strict_strtoull(buf, 0, &new); \
+ if (err) return err; \
var = new; \
start; \
- return end-buf; \
+ return strlen(buf); \
} \
static SYSDEV_ATTR(name, 0644, show_ ## name, set_ ## name);

Index: linux/include/asm-x86/mce.h
===================================================================
--- linux.orig/include/asm-x86/mce.h
+++ linux/include/asm-x86/mce.h
@@ -10,19 +10,19 @@
* Machine Check support for x86
*/

-#define MCG_CTL_P (1UL<<8) /* MCG_CAP register available */
+#define MCG_CTL_P (1ULL<<8) /* MCG_CAP register available */

-#define MCG_STATUS_RIPV (1UL<<0) /* restart ip valid */
-#define MCG_STATUS_EIPV (1UL<<1) /* ip points to correct instruction */
-#define MCG_STATUS_MCIP (1UL<<2) /* machine check in progress */
-
-#define MCI_STATUS_VAL (1UL<<63) /* valid error */
-#define MCI_STATUS_OVER (1UL<<62) /* previous errors lost */
-#define MCI_STATUS_UC (1UL<<61) /* uncorrected error */
-#define MCI_STATUS_EN (1UL<<60) /* error enabled */
-#define MCI_STATUS_MISCV (1UL<<59) /* misc error reg. valid */
-#define MCI_STATUS_ADDRV (1UL<<58) /* addr reg. valid */
-#define MCI_STATUS_PCC (1UL<<57) /* processor context corrupt */
+#define MCG_STATUS_RIPV (1ULL<<0) /* restart ip valid */
+#define MCG_STATUS_EIPV (1ULL<<1) /* ip points to correct instruction */
+#define MCG_STATUS_MCIP (1ULL<<2) /* machine check in progress */
+
+#define MCI_STATUS_VAL (1ULL<<63) /* valid error */
+#define MCI_STATUS_OVER (1ULL<<62) /* previous errors lost */
+#define MCI_STATUS_UC (1ULL<<61) /* uncorrected error */
+#define MCI_STATUS_EN (1ULL<<60) /* error enabled */
+#define MCI_STATUS_MISCV (1ULL<<59) /* misc error reg. valid */
+#define MCI_STATUS_ADDRV (1ULL<<58) /* addr reg. valid */
+#define MCI_STATUS_PCC (1ULL<<57) /* processor context corrupt */

/* Fields are zero when not available */
struct mce {

2008-07-07 23:11:38

by Andi Kleen

[permalink] [raw]
Subject: [PATCH] [4/10] MCE: Call 64bit machine check through a call vector


The 32bit mce code still needs that to interface to the WinChip
and P5 machine check handlers. On 64bit it's not strictly needed, but
also doesn't really hurt.

This way avoids some special cases for 32bit.

Signed-off-by: Andi Kleen <[email protected]>

---
arch/x86/kernel/cpu/mcheck/mce_64.c | 9 +++++++++
arch/x86/kernel/entry_64.S | 2 +-
include/asm-x86/mce.h | 2 ++
3 files changed, 12 insertions(+), 1 deletion(-)

Index: linux/arch/x86/kernel/cpu/mcheck/mce_64.c
===================================================================
--- linux.orig/arch/x86/kernel/cpu/mcheck/mce_64.c
+++ linux/arch/x86/kernel/cpu/mcheck/mce_64.c
@@ -317,6 +317,13 @@ void do_machine_check(struct pt_regs * r
atomic_dec(&mce_entry);
}

+static void ignore_machine_check(struct pt_regs *regs, long error_code)
+{
+}
+
+void (*machine_check_vector)(struct pt_regs *regs, long error_code) =
+ ignore_machine_check;
+
#ifdef CONFIG_X86_MCE_INTEL
/***
* mce_log_therm_throt_event - Logs the thermal throttling event to mcelog
@@ -533,6 +540,8 @@ void __cpuinit mcheck_init(struct cpuinf
!mce_available(c))
return;

+ machine_check_vector = do_machine_check;
+
mce_init(NULL);
mce_cpu_features(c);
}
Index: linux/arch/x86/kernel/entry_64.S
===================================================================
--- linux.orig/arch/x86/kernel/entry_64.S
+++ linux/arch/x86/kernel/entry_64.S
@@ -1170,7 +1170,7 @@ ENTRY(machine_check)
INTR_FRAME
pushq $0
CFI_ADJUST_CFA_OFFSET 8
- paranoidentry do_machine_check
+ paranoidentry *machine_check_vector(%rip)
jmp paranoid_exit1
CFI_ENDPROC
END(machine_check)
Index: linux/include/asm-x86/mce.h
===================================================================
--- linux.orig/include/asm-x86/mce.h
+++ linux/include/asm-x86/mce.h
@@ -107,6 +107,8 @@ static inline void mce_amd_feature_init(

void mce_log_therm_throt_event(unsigned int cpu, __u64 status);

+extern void (*machine_check_vector)(struct pt_regs *, long error_code);
+
extern atomic_t mce_entry;

extern void do_machine_check(struct pt_regs *, long);

2008-07-07 23:12:19

by Andi Kleen

[permalink] [raw]
Subject: [PATCH] [6/10] MCE: Provide exit_idle dummy functions for 32bit.


At some point i386 should probably gain true idle notifiers again,
but not now.

This is required for the 64bit thermal/threshold interrupt handlers.

Signed-off-by: Andi Kleen <[email protected]>

---
include/asm-x86/idle.h | 5 +++++
1 file changed, 5 insertions(+)

Index: linux/include/asm-x86/idle.h
===================================================================
--- linux.orig/include/asm-x86/idle.h
+++ linux/include/asm-x86/idle.h
@@ -7,7 +7,12 @@
struct notifier_block;
void idle_notifier_register(struct notifier_block *n);

+#ifdef CONFIG_X86_64
void enter_idle(void);
void exit_idle(void);
+#else
+static inline void enter_idle(void) {}
+static inline void exit_idle(void) {}
+#endif

#endif

2008-07-07 23:11:53

by Andi Kleen

[permalink] [raw]
Subject: [PATCH] [5/10] MCE: Rename mce_dont_init on 64bit to mce_disabled


This way they are the same between 32bit and 64bit and no special
cases needed. The semantics were always the same.

Signed-off-by: Andi Kleen <[email protected]>

---
arch/x86/kernel/cpu/mcheck/mce_64.c | 8 ++++----
include/asm-x86/mce.h | 2 --
2 files changed, 4 insertions(+), 6 deletions(-)

Index: linux/arch/x86/kernel/cpu/mcheck/mce_64.c
===================================================================
--- linux.orig/arch/x86/kernel/cpu/mcheck/mce_64.c
+++ linux/arch/x86/kernel/cpu/mcheck/mce_64.c
@@ -35,7 +35,7 @@

atomic_t mce_entry;

-static int mce_dont_init;
+int mce_disabled __cpuinitdata;

/*
* Tolerant levels:
@@ -535,7 +535,7 @@ void __cpuinit mcheck_init(struct cpuinf

mce_cpu_quirks(c);

- if (mce_dont_init ||
+ if (mce_disabled ||
cpu_test_and_set(smp_processor_id(), mce_cpus) ||
!mce_available(c))
return;
@@ -722,7 +722,7 @@ void __init restart_mce(void)
*/
static int __init mcheck_disable(char *str)
{
- mce_dont_init = 1;
+ mce_disabled = 1;
return 1;
}

@@ -734,7 +734,7 @@ static int __init mcheck_disable(char *s
static int __init mcheck_enable(char *str)
{
if (!strcmp(str, "off"))
- mce_dont_init = 1;
+ mce_disabled = 1;
else if (!strcmp(str, "bootlog") || !strcmp(str,"nobootlog"))
mce_bootlog = str[0] == 'b';
else if (isdigit(str[0]))
Index: linux/include/asm-x86/mce.h
===================================================================
--- linux.orig/include/asm-x86/mce.h
+++ linux/include/asm-x86/mce.h
@@ -84,9 +84,7 @@ struct mce_log {

#ifdef __KERNEL__

-#ifdef CONFIG_X86_32
extern int mce_disabled;
-#else /* CONFIG_X86_32 */

#include <asm/atomic.h>

2008-07-07 23:12:38

by Andi Kleen

[permalink] [raw]
Subject: [PATCH] [7/10] MCE: Remove machine check handler idle notify on 64bit


i386 has no idle notifiers, but the 64bit machine check
code uses them to wake up mcelog from a fatal machine check
exception.

For corrected machine checks found by the poller or
threshold interrupts going through an idle notifier is not needed
because the wake_up can is just done directly and doesn't
need the idle notifier. It is only needed for logging
exceptions.

To be honest I never liked the idle notifier even though I signed off
on it. On closer investigation the code actually turned out to
be a nop. Right now machine check exceptions on x86 are always unrecoverable
(lead to panic due to PCC), which means we never execute the
idle notifier path.

Also if we implement support for non fatal machine check
exceptions there are better ways to implement that than to
go through the idle thread. Also in this case the machine check will be
logged anyways.

So remove the "mcelog wakeup through idle notifier" code
from 64bit.

This allows to compile the 64bit machine check handler on 32bit
which doesn't have idle notifiers.

Signed-off-by: Andi Kleen <[email protected]>

---
arch/x86/kernel/cpu/mcheck/mce_64.c | 23 +----------------------
arch/x86/kernel/signal_64.c | 6 ------
include/asm-x86/thread_info_64.h | 5 ++---
3 files changed, 3 insertions(+), 31 deletions(-)

Index: linux/arch/x86/kernel/cpu/mcheck/mce_64.c
===================================================================
--- linux.orig/arch/x86/kernel/cpu/mcheck/mce_64.c
+++ linux/arch/x86/kernel/cpu/mcheck/mce_64.c
@@ -305,9 +305,6 @@ void do_machine_check(struct pt_regs * r
}
}

- /* notify userspace ASAP */
- set_thread_flag(TIF_MCE_NOTIFY);
-
out:
/* the last thing we do is clear state */
for (i = 0; i < banks; i++)
@@ -389,12 +386,10 @@ static void mcheck_timer(struct work_str
/*
* This is only called from process context. This is where we do
* anything we need to alert userspace about new MCEs. This is called
- * directly from the poller and also from entry.S and idle, thanks to
- * TIF_MCE_NOTIFY.
+ * directly from the poller.
*/
int mce_notify_user(void)
{
- clear_thread_flag(TIF_MCE_NOTIFY);
if (test_and_clear_bit(0, &notify_user)) {
static unsigned long last_print;
unsigned long now = jiffies;
@@ -414,28 +409,12 @@ int mce_notify_user(void)
return 0;
}

-/* see if the idle task needs to notify userspace */
-static int
-mce_idle_callback(struct notifier_block *nfb, unsigned long action, void *junk)
-{
- /* IDLE_END should be safe - interrupts are back on */
- if (action == IDLE_END && test_thread_flag(TIF_MCE_NOTIFY))
- mce_notify_user();
-
- return NOTIFY_OK;
-}
-
-static struct notifier_block mce_idle_notifier = {
- .notifier_call = mce_idle_callback,
-};
-
static __init int periodic_mcheck_init(void)
{
next_interval = check_interval * HZ;
if (next_interval)
schedule_delayed_work(&mcheck_work,
round_jiffies_relative(next_interval));
- idle_notifier_register(&mce_idle_notifier);
return 0;
}
__initcall(periodic_mcheck_init);
Index: linux/include/asm-x86/thread_info_64.h
===================================================================
--- linux.orig/include/asm-x86/thread_info_64.h
+++ linux/include/asm-x86/thread_info_64.h
@@ -109,7 +109,7 @@ static inline struct thread_info *stack_
#define TIF_IRET 5 /* force IRET */
#define TIF_SYSCALL_AUDIT 7 /* syscall auditing active */
#define TIF_SECCOMP 8 /* secure computing */
-#define TIF_MCE_NOTIFY 10 /* notify userspace of an MCE */
+/* 10 free */
#define TIF_HRTICK_RESCHED 11 /* reprogram hrtick timer */
/* 16 free */
#define TIF_IA32 17 /* 32bit process */
@@ -132,7 +132,6 @@ static inline struct thread_info *stack_
#define _TIF_IRET (1 << TIF_IRET)
#define _TIF_SYSCALL_AUDIT (1 << TIF_SYSCALL_AUDIT)
#define _TIF_SECCOMP (1 << TIF_SECCOMP)
-#define _TIF_MCE_NOTIFY (1 << TIF_MCE_NOTIFY)
#define _TIF_HRTICK_RESCHED (1 << TIF_HRTICK_RESCHED)
#define _TIF_IA32 (1 << TIF_IA32)
#define _TIF_FORK (1 << TIF_FORK)
@@ -154,7 +153,7 @@ static inline struct thread_info *stack_
#define _TIF_ALLWORK_MASK (0x0000FFFF & ~_TIF_SECCOMP)

#define _TIF_DO_NOTIFY_MASK \
- (_TIF_SIGPENDING|_TIF_SINGLESTEP|_TIF_MCE_NOTIFY|_TIF_HRTICK_RESCHED)
+ (_TIF_SIGPENDING|_TIF_SINGLESTEP|_TIF_HRTICK_RESCHED)

/* flags to check in __switch_to() */
#define _TIF_WORK_CTXSW \
Index: linux/arch/x86/kernel/signal_64.c
===================================================================
--- linux.orig/arch/x86/kernel/signal_64.c
+++ linux/arch/x86/kernel/signal_64.c
@@ -493,12 +493,6 @@ void do_notify_resume(struct pt_regs *re
clear_thread_flag(TIF_SINGLESTEP);
}

-#ifdef CONFIG_X86_MCE
- /* notify userspace of pending MCEs */
- if (thread_info_flags & _TIF_MCE_NOTIFY)
- mce_notify_user();
-#endif /* CONFIG_X86_MCE */
-
/* deal with pending signal delivery */
if (thread_info_flags & _TIF_SIGPENDING)
do_signal(regs);

2008-07-07 23:13:20

by Andi Kleen

[permalink] [raw]
Subject: [PATCH] [9/10] Add missing includes in mce_intel_64.c


In some i386 configurations these are not implicitely included.

Signed-off-by: Andi Kleen <[email protected]>

Index: linux/arch/x86/kernel/cpu/mcheck/mce_intel_64.c
===================================================================
--- linux.orig/arch/x86/kernel/cpu/mcheck/mce_intel_64.c
+++ linux/arch/x86/kernel/cpu/mcheck/mce_intel_64.c
@@ -12,6 +12,8 @@
#include <asm/hw_irq.h>
#include <asm/idle.h>
#include <asm/therm_throt.h>
+#include <asm/apic.h>
+#include <asm/pda.h>

asmlinkage void smp_thermal_interrupt(void)
{

2008-07-07 23:12:57

by Andi Kleen

[permalink] [raw]
Subject: [PATCH] [8/10] MCE: Remove oops_begin() use in 64bit machine check


First 32bit doesn't have oops_begin, so it's a barrier of using
this code on 32bit.

On closer examination it turns out oops_begin is not
a good idea in a machine check panic anyways. All oops_begin
does it so check for recursive/parallel oopses and implement the
"wait on oops" heuristic. But there's actually no good reason
to lock machine checks against oopses or prevent them
from recursion. Also "wait on oops" does not really make
sense for a machine check too.

So just remove it.

Replace it with a manual bust_spinlocks/console_verbose.

Signed-off-by: Andi Kleen <[email protected]>

---
arch/x86/kernel/cpu/mcheck/mce_64.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

Index: linux/arch/x86/kernel/cpu/mcheck/mce_64.c
===================================================================
--- linux.orig/arch/x86/kernel/cpu/mcheck/mce_64.c
+++ linux/arch/x86/kernel/cpu/mcheck/mce_64.c
@@ -133,7 +133,8 @@ static void mce_panic(char *msg, struct
{
int i;

- oops_begin();
+ bust_spinlocks(1);
+ console_verbose();
for (i = 0; i < MCE_LOG_LEN; i++) {
u64 tsc = mcelog.entry[i].tsc;

2008-07-07 23:13:37

by Andi Kleen

[permalink] [raw]
Subject: [PATCH] [10/10] MCE: Use 64bit machine check code on 32bit v2


The 64bit machine check code is in many ways much better than
the 32bit machine check code: it is more specification compliant,
is cleaner, only has a single code base versus one per CPU,
has better infrastructure for recovery, has a cleaner way to communicate
with user space etc. etc.

Use the 64bit code for 32bit too.

This is the second attempt to do this. There was one a couple of years
ago to unify this code for 32bit and 64bit. Back then this ran into some
trouble with K7s and was reverted.

I believe this time the K7 problems (and some others) are addressed.
I went over the old handlers and was very careful to retain
all quirks.

But of course this needs a lot of testing on old systems. On newer
64bit capable systems I don't expect much problems because they have been
already tested with the 64bit kernel.

I made this a CONFIG for now that still allows to select the old
machine check code. This is mostly to make testing easier,
if someone runs into a problem we can ask them to try
with the CONFIG switched.

The new code is default y for more coverage.

Once there is confidence the 64bit code works well on older hardware
too the CONFIG_X86_OLD_MCE and the associated code can be easily
removed.

This causes a behaviour change for 32bit installations. They now
have to install the mcelog package to be able to log
corrected machine checks.

The 64bit machine check code only handles CPUs which support the
standard Intel machine check architecture described in the IA32 SDM.
The 32bit code has special support for some older CPUs which
have non standard machine check architectures, in particular
WinChip C3 and Intel P5. I made those a separate CONFIG option
and kept them for now. The WinChip variant could be probably
removed without too much pain, it doesn't really do anything
interesting. P5 is also disabled by default (like it
was before) because many motherboards have it miswired, but
according to Alan Cox a few embedded setups use that one.

v2: Correct inverted recommendation in help text for ANCIENT_MCE
(Bert Wesarg)

Signed-off-by: Andi Kleen <[email protected]>

---
arch/x86/Kconfig | 35 +++++++++++++++++++++++++++++++----
arch/x86/kernel/cpu/mcheck/Makefile | 6 ++++--
arch/x86/kernel/cpu/mcheck/ancient.h | 12 ++++++++++++
arch/x86/kernel/cpu/mcheck/mce.h | 2 --
arch/x86/kernel/cpu/mcheck/mce_32.c | 1 +
arch/x86/kernel/cpu/mcheck/mce_64.c | 16 ++++++++++++++++
include/asm-x86/mce.h | 8 --------
7 files changed, 64 insertions(+), 16 deletions(-)

Index: linux/include/asm-x86/mce.h
===================================================================
--- linux.orig/include/asm-x86/mce.h
+++ linux/include/asm-x86/mce.h
@@ -1,8 +1,6 @@
#ifndef _ASM_X86_MCE_H
#define _ASM_X86_MCE_H

-#ifdef __x86_64__
-
#include <asm/ioctls.h>
#include <asm/types.h>

@@ -80,8 +78,6 @@ struct mce_log {
#define K8_MCE_THRESHOLD_BANK_5 (MCE_THRESHOLD_BASE + 5 * 9)
#define K8_MCE_THRESHOLD_DRAM_ECC (MCE_THRESHOLD_BANK_4 + 0)

-#endif /* __x86_64__ */
-
#ifdef __KERNEL__

extern int mce_disabled;
@@ -112,10 +108,6 @@ extern atomic_t mce_entry;
extern void do_machine_check(struct pt_regs *, long);
extern int mce_notify_user(void);

-#endif /* !CONFIG_X86_32 */
-
-
-
#ifdef CONFIG_X86_MCE
extern void mcheck_init(struct cpuinfo_x86 *c);
#else
Index: linux/arch/x86/Kconfig
===================================================================
--- linux.orig/arch/x86/Kconfig
+++ linux/arch/x86/Kconfig
@@ -650,10 +650,37 @@ config X86_MCE
to disable it. MCE support simply ignores non-MCE processors like
the 386 and 486, so nearly everyone can say Y here.

+config X86_OLD_MCE
+ depends on X86_32
+ bool "Use legacy machine check code (will go away)"
+ default n
+ help
+ Use the old i386 machine check code. This is merely intended for testing
+ in a transition period. Try this if you run into any machine check
+ related software problems.
+ When in doubt say no.
+
+config X86_ANCIENT_MCE
+ depends on X86_32
+ bool "Support ancient machine check handler for very old CPUs"
+ default n
+ help
+ Include support for family 5 (Intel Pentium 1 and Centaur Winchip)
+ machine check code. Machine check handles uncorrected CPU errors.
+ Note that the P5 pentium support is disabled
+ by default and can be only enabled on special hardware.
+ The Winchip code doesn't do much.
+ If you're still sure you want it, say y, otherwise n is safe
+ for nearly everybody.
+
+config X86_NEW_MCE
+ bool
+ default y if (!X86_OLD_MCE && X86_32) || X86_64
+
config X86_MCE_INTEL
def_bool y
prompt "Intel MCE features"
- depends on X86_64 && X86_MCE && X86_LOCAL_APIC
+ depends on X86_NEW_MCE && X86_MCE && X86_LOCAL_APIC
help
Additional support for intel specific MCE features such as
the thermal monitor.
@@ -661,14 +688,14 @@ config X86_MCE_INTEL
config X86_MCE_AMD
def_bool y
prompt "AMD MCE features"
- depends on X86_64 && X86_MCE && X86_LOCAL_APIC
+ depends on X86_NEW_MCE && X86_MCE && X86_LOCAL_APIC && X86_64
help
Additional support for AMD specific MCE features such as
the DRAM Error Threshold.

config X86_MCE_NONFATAL
tristate "Check for non-fatal errors on AMD Athlon/Duron / Intel Pentium 4"
- depends on X86_32 && X86_MCE
+ depends on !X86_NEW_MCE && X86_MCE
help
Enabling this feature starts a timer that triggers every 5 seconds which
will look at the machine check registers to see if anything happened.
@@ -681,7 +708,7 @@ config X86_MCE_NONFATAL

config X86_MCE_P4THERMAL
bool "check for P4 thermal throttling interrupt."
- depends on X86_32 && X86_MCE && (X86_UP_APIC || SMP) && !X86_VISWS
+ depends on !X86_NEW_MCE && X86_MCE && (X86_UP_APIC || SMP) && !X86_VISWS
help
Enabling this feature will cause a message to be printed when the P4
enters thermal throttling.
Index: linux/arch/x86/kernel/cpu/mcheck/Makefile
===================================================================
--- linux.orig/arch/x86/kernel/cpu/mcheck/Makefile
+++ linux/arch/x86/kernel/cpu/mcheck/Makefile
@@ -1,6 +1,8 @@
-obj-y = mce_$(BITS).o therm_throt.o
+obj-y = therm_throt.o

-obj-$(CONFIG_X86_32) += k7.o p4.o p5.o p6.o winchip.o
+obj-$(CONFIG_X86_OLD_MCE) += mce_32.o k7.o p4.o p6.o
+obj-$(CONFIG_X86_NEW_MCE) += mce_64.o
+obj-$(CONFIG_X86_ANCIENT_MCE) += p5.o winchip.o
obj-$(CONFIG_X86_MCE_INTEL) += mce_intel_64.o
obj-$(CONFIG_X86_MCE_AMD) += mce_amd_64.o
obj-$(CONFIG_X86_MCE_NONFATAL) += non-fatal.o
Index: linux/arch/x86/kernel/cpu/mcheck/mce_64.c
===================================================================
--- linux.orig/arch/x86/kernel/cpu/mcheck/mce_64.c
+++ linux/arch/x86/kernel/cpu/mcheck/mce_64.c
@@ -29,6 +29,7 @@
#include <asm/uaccess.h>
#include <asm/smp.h>
#include <asm/idle.h>
+#include "ancient.h"

#define MISC_MCELOG_MINOR 227
#define NR_BANKS 6
@@ -491,6 +492,20 @@ static void __cpuinit mce_cpu_quirks(str

}

+static void __cpuinit mce_ancient_init(struct cpuinfo_x86 *c)
+{
+ if (c->x86 != 5)
+ return;
+ switch (c->x86_vendor) {
+ case X86_VENDOR_INTEL:
+ intel_p5_mcheck_init(c);
+ break;
+ case X86_VENDOR_CENTAUR:
+ winchip_mcheck_init(c);
+ break;
+ }
+}
+
static void __cpuinit mce_cpu_features(struct cpuinfo_x86 *c)
{
switch (c->x86_vendor) {
@@ -513,6 +528,7 @@ void __cpuinit mcheck_init(struct cpuinf
{
static cpumask_t mce_cpus = CPU_MASK_NONE;

+ mce_ancient_init(c);
mce_cpu_quirks(c);

if (mce_disabled ||
Index: linux/arch/x86/kernel/cpu/mcheck/ancient.h
===================================================================
--- /dev/null
+++ linux/arch/x86/kernel/cpu/mcheck/ancient.h
@@ -0,0 +1,12 @@
+#ifdef CONFIG_X86_ANCIENT_MCE
+void intel_p5_mcheck_init(struct cpuinfo_x86 *);
+void winchip_mcheck_init(struct cpuinfo_x86 *);
+#else
+static inline void intel_p5_mcheck_init(struct cpuinfo_x86 *c)
+{
+}
+
+static inline void winchip_mcheck_init(struct cpuinfo_x86 *c)
+{
+}
+#endif
Index: linux/arch/x86/kernel/cpu/mcheck/mce.h
===================================================================
--- linux.orig/arch/x86/kernel/cpu/mcheck/mce.h
+++ linux/arch/x86/kernel/cpu/mcheck/mce.h
@@ -3,9 +3,7 @@

void amd_mcheck_init(struct cpuinfo_x86 *c);
void intel_p4_mcheck_init(struct cpuinfo_x86 *c);
-void intel_p5_mcheck_init(struct cpuinfo_x86 *c);
void intel_p6_mcheck_init(struct cpuinfo_x86 *c);
-void winchip_mcheck_init(struct cpuinfo_x86 *c);

/* Call the installed machine check handler for this CPU setup. */
extern void (*machine_check_vector)(struct pt_regs *, long error_code);
Index: linux/arch/x86/kernel/cpu/mcheck/mce_32.c
===================================================================
--- linux.orig/arch/x86/kernel/cpu/mcheck/mce_32.c
+++ linux/arch/x86/kernel/cpu/mcheck/mce_32.c
@@ -15,6 +15,7 @@
#include <asm/mce.h>

#include "mce.h"
+#include "ancient.h"

int mce_disabled;
int nr_mce_banks;

2008-07-07 23:53:39

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH] [0/10] Use 64bit x86 machine check code for 32bit too v2

Applied to -tip as x86/unify-mce.

Thanks!

-hpa


Andi Kleen wrote:
> This patchkit uses the 64bit machine check code which is better in many
> ways on 32bit x86 too. This is also the basis for some future machine
> check work.
>
> The 64bit machine check code is in many ways much better than
> the 32bit machine check code: it is more specification compliant,
> is cleaner, only has a single code base versus one per CPU,
> has better infrastructure for recovery, has a cleaner way to communicate
> with user space etc. etc.
>
> It requires testing especially on older systems (on newer
> ones it should be already tested well in 64bit systems).
>
> The patchkit contains several parts:
> - It ports over a few needed quirks (for older Intel and older
> AMD CPUs) to the 64bit kernel.
> - It changes the 64bit code to be 32bit clean in its data structures
> (mostly just unsigned long -> u64 where needed)
> - It drops some unused functionality that cannot be easily implemented on 32bit
> and didn't seem worth ifdefing
>
> Tested by doing some software level error injection on a few
> different machines
>
> I request this code is merged into the appropiate tree for linux-next
> for wider testing. It's not .27 ready, but hopefully .28, but it requires
> wider exposure now.
>
> v2: Fix compilation problems noted by hpa in some configurations
> Fix strict_strtoul() conversion
>
> -Andi
>

2008-07-08 19:54:59

by Max Asbock

[permalink] [raw]
Subject: Re: [PATCH] [0/10] Use 64bit x86 machine check code for 32bit too v2

This patch set builds fine. I booted it and started some preliminary
testing by injecting errors through software. So far it looks good.

Max

On Mon, 2008-07-07 at 16:53 -0700, H. Peter Anvin wrote:
> Applied to -tip as x86/unify-mce.
>
> Thanks!
>
> -hpa
>
>
> Andi Kleen wrote:
> > This patchkit uses the 64bit machine check code which is better in many
> > ways on 32bit x86 too. This is also the basis for some future machine
> > check work.
> >
> > The 64bit machine check code is in many ways much better than
> > the 32bit machine check code: it is more specification compliant,
> > is cleaner, only has a single code base versus one per CPU,
> > has better infrastructure for recovery, has a cleaner way to communicate
> > with user space etc. etc.
> >
> > It requires testing especially on older systems (on newer
> > ones it should be already tested well in 64bit systems).
> >
> > The patchkit contains several parts:
> > - It ports over a few needed quirks (for older Intel and older
> > AMD CPUs) to the 64bit kernel.
> > - It changes the 64bit code to be 32bit clean in its data structures
> > (mostly just unsigned long -> u64 where needed)
> > - It drops some unused functionality that cannot be easily implemented on 32bit
> > and didn't seem worth ifdefing
> >
> > Tested by doing some software level error injection on a few
> > different machines
> >
> > I request this code is merged into the appropiate tree for linux-next
> > for wider testing. It's not .27 ready, but hopefully .28, but it requires
> > wider exposure now.
> >
> > v2: Fix compilation problems noted by hpa in some configurations
> > Fix strict_strtoul() conversion
> >
> > -Andi
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/