[PATCH 0/6] x86, NMI: die_notifier and default_do_nmi cleanups

[PATCH 0/6] x86, NMI: die_notifier and default_do_nmi cleanups

[Don Zickus]

This is another version of the NMI rewrite patches I sent previously.  I
take it those patches were too thick to be reviewed easily, so I broke out
the patches differently to make it easier to review.  Hopefully it doesn't
look as scary.

The bulk of the patch adds priorities to the die_chain notifiers for NMI
events and removes the DIE_NMI_IPI event.  There are some other cleanups
too, which include removing the restriction of the BSP cpu to process 
external NMIs.

Cheers,
Don

Don Zickus (5):
  x86: Convert some devices to use DIE_NMIUNKNOWN
  x86, NMI: Add priorities to handlers
  x86, NMI: Remove DIE_NMI_IPI
  x86, NMI: Allow NMI reason io port (0x61) to be processed on any CPU
  x86, NMI: Clean-up default_do_nmi()

Huang Ying (1):
  x86, NMI: Add NMI symbol constants and rename memory parity to PCI
    SERR

 arch/x86/include/asm/kdebug.h           |    1 -
 arch/x86/include/asm/mach_traps.h       |   12 +++-
 arch/x86/include/asm/nmi.h              |   20 ++++++
 arch/x86/kernel/apic/hw_nmi.c           |    3 +-
 arch/x86/kernel/apic/x2apic_uv_x.c      |    2 +-
 arch/x86/kernel/cpu/mcheck/mce-inject.c |    5 +-
 arch/x86/kernel/cpu/perf_event.c        |    3 +-
 arch/x86/kernel/kgdb.c                  |    6 +--
 arch/x86/kernel/reboot.c                |    5 +-
 arch/x86/kernel/traps.c                 |  102 +++++++++++++++---------------
 arch/x86/oprofile/nmi_int.c             |    3 +-
 arch/x86/oprofile/nmi_timer_int.c       |    2 +-
 drivers/char/ipmi/ipmi_watchdog.c       |    2 +-
 drivers/watchdog/hpwdt.c                |    2 +-
 14 files changed, 97 insertions(+), 71 deletions(-)

-- 
1.7.3.4

[PATCH 1/6] x86, NMI: Add NMI symbol constants and rename memory parity to PCI SERR

[Don Zickus]

From: Huang Ying <ying.huang@intel.com>

Replace the NMI related magic numbers with symbol constants.

memory parity error is only valid for IBM PC-AT, newer machine use
bit 7 (0x80) of 0x61 port for PCI SERR. While memory error is usually
reported via MCE. So corresponding function name and kernel log string
is changed.

But on some machines, PCI SERR line is still used to report memory
errors. This is used by EDAC, so corresponding EDAC call is reserved.

v3:

- Merge adding symbol constants and renaming patches

v2:

- EDAC call in pci_serr_error is reserved.

Signed-off-by: Huang Ying <ying.huang@intel.com>
Signed-off-by: Don Zickus <dzickus@redhat.com>
---
 arch/x86/include/asm/mach_traps.h |   12 ++++++++-
 arch/x86/kernel/traps.c           |   51 +++++++++++++++++++------------------
 2 files changed, 37 insertions(+), 26 deletions(-)

diff --git a/arch/x86/include/asm/mach_traps.h b/arch/x86/include/asm/mach_traps.h
index f792060..72a8b52 100644
--- a/arch/x86/include/asm/mach_traps.h
+++ b/arch/x86/include/asm/mach_traps.h
@@ -7,9 +7,19 @@
 
 #include <asm/mc146818rtc.h>
 
+#define NMI_REASON_PORT		0x61
+
+#define NMI_REASON_SERR		0x80
+#define NMI_REASON_IOCHK	0x40
+#define NMI_REASON_MASK		(NMI_REASON_SERR | NMI_REASON_IOCHK)
+
+#define NMI_REASON_CLEAR_SERR	0x04
+#define NMI_REASON_CLEAR_IOCHK	0x08
+#define NMI_REASON_CLEAR_MASK	0x0f
+
 static inline unsigned char get_nmi_reason(void)
 {
-	return inb(0x61);
+	return inb(NMI_REASON_PORT);
 }
 
 static inline void reassert_nmi(void)
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index c76aaca..c7fd1ce 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -310,15 +310,15 @@ static int __init setup_unknown_nmi_panic(char *str)
 __setup("unknown_nmi_panic", setup_unknown_nmi_panic);
 
 static notrace __kprobes void
-mem_parity_error(unsigned char reason, struct pt_regs *regs)
+pci_serr_error(unsigned char reason, struct pt_regs *regs)
 {
-	printk(KERN_EMERG
-		"Uhhuh. NMI received for unknown reason %02x on CPU %d.\n",
-			reason, smp_processor_id());
-
-	printk(KERN_EMERG
-		"You have some hardware problem, likely on the PCI bus.\n");
+	pr_emerg("NMI: PCI system error (SERR) for reason %02x on CPU %d.\n",
+		 reason, smp_processor_id());
 
+	/*
+	 * On some machines, PCI SERR line is used to report memory
+	 * errors. EDAC makes use of it.
+	 */
 #if defined(CONFIG_EDAC)
 	if (edac_handler_set()) {
 		edac_atomic_assert_error();
@@ -329,11 +329,11 @@ mem_parity_error(unsigned char reason, struct pt_regs *regs)
 	if (panic_on_unrecovered_nmi)
 		panic("NMI: Not continuing");
 
-	printk(KERN_EMERG "Dazed and confused, but trying to continue\n");
+	pr_emerg("Dazed and confused, but trying to continue\n");
 
-	/* Clear and disable the memory parity error line. */
-	reason = (reason & 0xf) | 4;
-	outb(reason, 0x61);
+	/* Clear and disable the PCI SERR error line. */
+	reason = (reason & NMI_REASON_CLEAR_MASK) | NMI_REASON_CLEAR_SERR;
+	outb(reason, NMI_REASON_PORT);
 }
 
 static notrace __kprobes void
@@ -341,15 +341,17 @@ io_check_error(unsigned char reason, struct pt_regs *regs)
 {
 	unsigned long i;
 
-	printk(KERN_EMERG "NMI: IOCK error (debug interrupt?)\n");
+	pr_emerg(
+	"NMI: IOCK error (debug interrupt?) for reason %02x on CPU %d.\n",
+		 reason, smp_processor_id());
 	show_registers(regs);
 
 	if (panic_on_io_nmi)
 		panic("NMI IOCK error: Not continuing");
 
 	/* Re-enable the IOCK line, wait for a few seconds */
-	reason = (reason & 0xf) | 8;
-	outb(reason, 0x61);
+	reason = (reason & NMI_REASON_CLEAR_MASK) | NMI_REASON_CLEAR_IOCHK;
+	outb(reason, NMI_REASON_PORT);
 
 	i = 20000;
 	while (--i) {
@@ -357,8 +359,8 @@ io_check_error(unsigned char reason, struct pt_regs *regs)
 		udelay(100);
 	}
 
-	reason &= ~8;
-	outb(reason, 0x61);
+	reason &= ~NMI_REASON_CLEAR_IOCHK;
+	outb(reason, NMI_REASON_PORT);
 }
 
 static notrace __kprobes void
@@ -377,15 +379,14 @@ unknown_nmi_error(unsigned char reason, struct pt_regs *regs)
 		return;
 	}
 #endif
-	printk(KERN_EMERG
-		"Uhhuh. NMI received for unknown reason %02x on CPU %d.\n",
-			reason, smp_processor_id());
+	pr_emerg("Uhhuh. NMI received for unknown reason %02x on CPU %d.\n",
+		 reason, smp_processor_id());
 
-	printk(KERN_EMERG "Do you have a strange power saving mode enabled?\n");
+	pr_emerg("Do you have a strange power saving mode enabled?\n");
 	if (unknown_nmi_panic || panic_on_unrecovered_nmi)
 		panic("NMI: Not continuing");
 
-	printk(KERN_EMERG "Dazed and confused, but trying to continue\n");
+	pr_emerg("Dazed and confused, but trying to continue\n");
 }
 
 static notrace __kprobes void default_do_nmi(struct pt_regs *regs)
@@ -399,7 +400,7 @@ static notrace __kprobes void default_do_nmi(struct pt_regs *regs)
 	if (!cpu)
 		reason = get_nmi_reason();
 
-	if (!(reason & 0xc0)) {
+	if (!(reason & NMI_REASON_MASK)) {
 		if (notify_die(DIE_NMI_IPI, "nmi_ipi", regs, reason, 2, SIGINT)
 								== NOTIFY_STOP)
 			return;
@@ -417,9 +418,9 @@ static notrace __kprobes void default_do_nmi(struct pt_regs *regs)
 		return;
 
 	/* AK: following checks seem to be broken on modern chipsets. FIXME */
-	if (reason & 0x80)
-		mem_parity_error(reason, regs);
-	if (reason & 0x40)
+	if (reason & NMI_REASON_SERR)
+		pci_serr_error(reason, regs);
+	if (reason & NMI_REASON_IOCHK)
 		io_check_error(reason, regs);
 #ifdef CONFIG_X86_32
 	/*
-- 
1.7.3.4

[PATCH 2/6] x86: Convert some devices to use DIE_NMIUNKNOWN

[Don Zickus]

They are a handful of places in the code that register a die_notifier
as a catch all in case no claims the NMI.  Unfortunately, they trigger
on events like DIE_NMI and DIE_NMI_IPI, which depending on when they
registered may collide with other handlers that have the ability to
determine if the NMI is theirs or not.

The function unknown_nmi_error() makes one last effort to walk the
die_chain when no one else has claimed the NMI before spitting out
messages that the NMI is unknown.

This is a better spot for these devices to execute any code without
colliding with the other handlers.

The two drivers modified are only compiled on x86 arches I believe, so
they shouldn't be affected by other arches that may not have
DIE_NMIUNKNOWN defined.

CC: Russ Anderson <rja@sgi.com>
CC: Corey Minyard <minyard@acm.org>
CC: openipmi-developer@lists.sourceforge.net
CC: dann frazier <dannf@hp.com>
Signed-off-by: Don Zickus <dzickus@redhat.com>
---
 arch/x86/kernel/apic/x2apic_uv_x.c |    2 +-
 drivers/char/ipmi/ipmi_watchdog.c  |    2 +-
 drivers/watchdog/hpwdt.c           |    2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/apic/x2apic_uv_x.c b/arch/x86/kernel/apic/x2apic_uv_x.c
index e6cde91..7b0d2f6 100644
--- a/arch/x86/kernel/apic/x2apic_uv_x.c
+++ b/arch/x86/kernel/apic/x2apic_uv_x.c
@@ -642,7 +642,7 @@ void __cpuinit uv_cpu_init(void)
  */
 int uv_handle_nmi(struct notifier_block *self, unsigned long reason, void *data)
 {
-	if (reason != DIE_NMI_IPI)
+	if (reason != DIE_NMIUNKNOWN)
 		return NOTIFY_OK;
 
 	if (in_crash_kexec)
diff --git a/drivers/char/ipmi/ipmi_watchdog.c b/drivers/char/ipmi/ipmi_watchdog.c
index f4d334f..320668f 100644
--- a/drivers/char/ipmi/ipmi_watchdog.c
+++ b/drivers/char/ipmi/ipmi_watchdog.c
@@ -1081,7 +1081,7 @@ ipmi_nmi(struct notifier_block *self, unsigned long val, void *data)
 {
 	struct die_args *args = data;
 
-	if (val != DIE_NMI)
+	if (val != DIE_NMIUNKNOWN)
 		return NOTIFY_OK;
 
 	/* Hack, if it's a memory or I/O error, ignore it. */
diff --git a/drivers/watchdog/hpwdt.c b/drivers/watchdog/hpwdt.c
index dea7b5b..24b966d 100644
--- a/drivers/watchdog/hpwdt.c
+++ b/drivers/watchdog/hpwdt.c
@@ -469,7 +469,7 @@ static int hpwdt_pretimeout(struct notifier_block *nb, unsigned long ulReason,
 	unsigned long rom_pl;
 	static int die_nmi_called;
 
-	if (ulReason != DIE_NMI && ulReason != DIE_NMI_IPI)
+	if (ulReason != DIE_NMIUNKNOWN)
 		goto out;
 
 	if (!hpwdt_nmi_decoding)
-- 
1.7.3.4

[PATCH 3/6] x86, NMI: Add priorities to handlers

[Don Zickus]

In order to consolidate the NMI die_chain events, we need to setup the priorities
for the die notifiers.

I started by defining a bunch of common priorities that can be used by the
notifier blocks.  Then I modified the notifier blocks to use the newly created
priorities.

Now that the priorities are straightened out, it should be easier to remove the
event DIE_NMI_IPI.

Signed-off-by: Don Zickus <dzickus@redhat.com>
---
 arch/x86/include/asm/nmi.h              |   20 ++++++++++++++++++++
 arch/x86/kernel/apic/hw_nmi.c           |    2 +-
 arch/x86/kernel/cpu/mcheck/mce-inject.c |    2 +-
 arch/x86/kernel/cpu/perf_event.c        |    2 +-
 arch/x86/kernel/kgdb.c                  |    2 +-
 arch/x86/kernel/reboot.c                |    2 ++
 arch/x86/oprofile/nmi_int.c             |    2 +-
 arch/x86/oprofile/nmi_timer_int.c       |    2 +-
 8 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/nmi.h b/arch/x86/include/asm/nmi.h
index c4021b9..fb82fb6 100644
--- a/arch/x86/include/asm/nmi.h
+++ b/arch/x86/include/asm/nmi.h
@@ -23,6 +23,26 @@ void arch_trigger_all_cpu_backtrace(void);
 #define arch_trigger_all_cpu_backtrace arch_trigger_all_cpu_backtrace
 #endif
 
+/*
+ * Define some priorities for the nmi notifier call chain.
+ *
+ * Create a local nmi bit that has a higher priority than
+ * external nmis, because the local ones are more frequent.
+ * 
+ * Also setup some default high/normal/low settings for
+ * subsystems to registers with.  Using 4 bits to seperate
+ * the priorities.  This can go alot higher if needed be.
+ */
+
+#define NMI_LOCAL_SHIFT		16	/* randomly picked */
+#define NMI_LOCAL_BIT		(1ULL << NMI_LOCAL_SHIFT)
+#define NMI_HIGH_PRIOR		(1ULL << 8)
+#define NMI_NORMAL_PRIOR	(1ULL << 4)
+#define NMI_LOW_PRIOR		(1ULL << 0)
+#define NMI_LOCAL_HIGH_PRIOR	(NMI_LOCAL_BIT | NMI_HIGH_PRIOR)
+#define NMI_LOCAL_NORMAL_PRIOR	(NMI_LOCAL_BIT | NMI_NORMAL_PRIOR)
+#define NMI_LOCAL_LOW_PRIOR	(NMI_LOCAL_BIT | NMI_LOW_PRIOR)
+
 void stop_nmi(void);
 void restart_nmi(void);
 
diff --git a/arch/x86/kernel/apic/hw_nmi.c b/arch/x86/kernel/apic/hw_nmi.c
index 72ec29e..8bc49f1 100644
--- a/arch/x86/kernel/apic/hw_nmi.c
+++ b/arch/x86/kernel/apic/hw_nmi.c
@@ -96,7 +96,7 @@ arch_trigger_all_cpu_backtrace_handler(struct notifier_block *self,
 static __read_mostly struct notifier_block backtrace_notifier = {
 	.notifier_call          = arch_trigger_all_cpu_backtrace_handler,
 	.next                   = NULL,
-	.priority               = 1
+	.priority               = NMI_LOCAL_LOW_PRIOR,
 };
 
 static int __init register_trigger_all_cpu_backtrace(void)
diff --git a/arch/x86/kernel/cpu/mcheck/mce-inject.c b/arch/x86/kernel/cpu/mcheck/mce-inject.c
index e7dbde7..59546c1 100644
--- a/arch/x86/kernel/cpu/mcheck/mce-inject.c
+++ b/arch/x86/kernel/cpu/mcheck/mce-inject.c
@@ -95,7 +95,7 @@ static int mce_raise_notify(struct notifier_block *self,
 
 static struct notifier_block mce_raise_nb = {
 	.notifier_call = mce_raise_notify,
-	.priority = 1000,
+	.priority = NMI_LOCAL_NORMAL_PRIOR,
 };
 
 /* Inject mce on current CPU */
diff --git a/arch/x86/kernel/cpu/perf_event.c b/arch/x86/kernel/cpu/perf_event.c
index 0a360d1..5e14b5e 100644
--- a/arch/x86/kernel/cpu/perf_event.c
+++ b/arch/x86/kernel/cpu/perf_event.c
@@ -1318,7 +1318,7 @@ perf_event_nmi_handler(struct notifier_block *self,
 static __read_mostly struct notifier_block perf_event_nmi_notifier = {
 	.notifier_call		= perf_event_nmi_handler,
 	.next			= NULL,
-	.priority		= 1
+	.priority		= NMI_LOCAL_LOW_PRIOR,
 };
 
 static struct event_constraint unconstrained;
diff --git a/arch/x86/kernel/kgdb.c b/arch/x86/kernel/kgdb.c
index cd21b65..983fb54 100644
--- a/arch/x86/kernel/kgdb.c
+++ b/arch/x86/kernel/kgdb.c
@@ -606,7 +606,7 @@ static struct notifier_block kgdb_notifier = {
 	/*
 	 * Lowest-prio notifier priority, we want to be notified last:
 	 */
-	.priority	= -INT_MAX,
+	.priority	= NMI_LOCAL_LOW_PRIOR,
 };
 
 /**
diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
index c495aa8..9c1c83e 100644
--- a/arch/x86/kernel/reboot.c
+++ b/arch/x86/kernel/reboot.c
@@ -778,6 +778,8 @@ static void smp_send_nmi_allbutself(void)
 
 static struct notifier_block crash_nmi_nb = {
 	.notifier_call = crash_nmi_callback,
+	/* we want to be the first one called */
+	.priority = NMI_LOCAL_HIGH_PRIOR+1,
 };
 
 /* Halt all other CPUs, calling the specified function on each of them
diff --git a/arch/x86/oprofile/nmi_int.c b/arch/x86/oprofile/nmi_int.c
index 358c8b9..6e84ea4 100644
--- a/arch/x86/oprofile/nmi_int.c
+++ b/arch/x86/oprofile/nmi_int.c
@@ -361,7 +361,7 @@ static void nmi_cpu_setup(void *dummy)
 static struct notifier_block profile_exceptions_nb = {
 	.notifier_call = profile_exceptions_notify,
 	.next = NULL,
-	.priority = 2
+	.priority = NMI_LOCAL_LOW_PRIOR,
 };
 
 static void nmi_cpu_restore_registers(struct op_msrs *msrs)
diff --git a/arch/x86/oprofile/nmi_timer_int.c b/arch/x86/oprofile/nmi_timer_int.c
index 0636dd9..720bf5a 100644
--- a/arch/x86/oprofile/nmi_timer_int.c
+++ b/arch/x86/oprofile/nmi_timer_int.c
@@ -38,7 +38,7 @@ static int profile_timer_exceptions_notify(struct notifier_block *self,
 static struct notifier_block profile_timer_exceptions_nb = {
 	.notifier_call = profile_timer_exceptions_notify,
 	.next = NULL,
-	.priority = 0
+	.priority = NMI_LOW_PRIOR,
 };
 
 static int timer_start(void)
-- 
1.7.3.4

[PATCH 4/6] x86, NMI: Remove DIE_NMI_IPI

[Don Zickus]

With priorities in place and no one really understanding the difference between
DIE_NMI and DIE_NMI_IPI, just remove DIE_NMI_IPI and convert everyone to DIE_NMI.

This also simplifies default_do_nmi() a little bit.  Instead of calling the
die_notifier in both the if and else part, just pull it out and call it before
the if-statement.  This has the side benefit of avoiding a call to the ioport
to see if there is an external NMI sitting around until after the (more frequent)
internal NMIs are dealt with.

Patch-Inspired-by: Huang Ying <ying.huang@intel.com>
Signed-off-by: Don Zickus <dzickus@redhat.com>
---
 arch/x86/include/asm/kdebug.h           |    1 -
 arch/x86/kernel/apic/hw_nmi.c           |    1 -
 arch/x86/kernel/cpu/mcheck/mce-inject.c |    3 ++-
 arch/x86/kernel/cpu/perf_event.c        |    1 -
 arch/x86/kernel/kgdb.c                  |    4 ----
 arch/x86/kernel/reboot.c                |    3 ++-
 arch/x86/kernel/traps.c                 |   19 ++++++++-----------
 arch/x86/oprofile/nmi_int.c             |    1 -
 8 files changed, 12 insertions(+), 21 deletions(-)

diff --git a/arch/x86/include/asm/kdebug.h b/arch/x86/include/asm/kdebug.h
index f23eb25..ca242d3 100644
--- a/arch/x86/include/asm/kdebug.h
+++ b/arch/x86/include/asm/kdebug.h
@@ -18,7 +18,6 @@ enum die_val {
 	DIE_TRAP,
 	DIE_GPF,
 	DIE_CALL,
-	DIE_NMI_IPI,
 	DIE_PAGE_FAULT,
 	DIE_NMIUNKNOWN,
 };
diff --git a/arch/x86/kernel/apic/hw_nmi.c b/arch/x86/kernel/apic/hw_nmi.c
index 8bc49f1..79fd43c 100644
--- a/arch/x86/kernel/apic/hw_nmi.c
+++ b/arch/x86/kernel/apic/hw_nmi.c
@@ -68,7 +68,6 @@ arch_trigger_all_cpu_backtrace_handler(struct notifier_block *self,
 
 	switch (cmd) {
 	case DIE_NMI:
-	case DIE_NMI_IPI:
 		break;
 
 	default:
diff --git a/arch/x86/kernel/cpu/mcheck/mce-inject.c b/arch/x86/kernel/cpu/mcheck/mce-inject.c
index 59546c1..a779719 100644
--- a/arch/x86/kernel/cpu/mcheck/mce-inject.c
+++ b/arch/x86/kernel/cpu/mcheck/mce-inject.c
@@ -25,6 +25,7 @@
 #include <linux/gfp.h>
 #include <asm/mce.h>
 #include <asm/apic.h>
+#include <asm/nmi.h>
 
 /* Update fake mce registers on current CPU. */
 static void inject_mce(struct mce *m)
@@ -83,7 +84,7 @@ static int mce_raise_notify(struct notifier_block *self,
 	struct die_args *args = (struct die_args *)data;
 	int cpu = smp_processor_id();
 	struct mce *m = &__get_cpu_var(injectm);
-	if (val != DIE_NMI_IPI || !cpumask_test_cpu(cpu, mce_inject_cpumask))
+	if (val != DIE_NMI || !cpumask_test_cpu(cpu, mce_inject_cpumask))
 		return NOTIFY_DONE;
 	cpumask_clear_cpu(cpu, mce_inject_cpumask);
 	if (m->inject_flags & MCJ_EXCEPTION)
diff --git a/arch/x86/kernel/cpu/perf_event.c b/arch/x86/kernel/cpu/perf_event.c
index 5e14b5e..c71bae4 100644
--- a/arch/x86/kernel/cpu/perf_event.c
+++ b/arch/x86/kernel/cpu/perf_event.c
@@ -1268,7 +1268,6 @@ perf_event_nmi_handler(struct notifier_block *self,
 
 	switch (cmd) {
 	case DIE_NMI:
-	case DIE_NMI_IPI:
 		break;
 	case DIE_NMIUNKNOWN:
 		this_nmi = percpu_read(irq_stat.__nmi_count);
diff --git a/arch/x86/kernel/kgdb.c b/arch/x86/kernel/kgdb.c
index 983fb54..1e8b583 100644
--- a/arch/x86/kernel/kgdb.c
+++ b/arch/x86/kernel/kgdb.c
@@ -525,10 +525,6 @@ static int __kgdb_notify(struct die_args *args, unsigned long cmd)
 		}
 		return NOTIFY_DONE;
 
-	case DIE_NMI_IPI:
-		/* Just ignore, we will handle the roundup on DIE_NMI. */
-		return NOTIFY_DONE;
-
 	case DIE_NMIUNKNOWN:
 		if (was_in_debug_nmi[raw_smp_processor_id()]) {
 			was_in_debug_nmi[raw_smp_processor_id()] = 0;
diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
index 9c1c83e..fc7aae1 100644
--- a/arch/x86/kernel/reboot.c
+++ b/arch/x86/kernel/reboot.c
@@ -18,6 +18,7 @@
 #include <asm/pci_x86.h>
 #include <asm/virtext.h>
 #include <asm/cpu.h>
+#include <asm/nmi.h>
 
 #ifdef CONFIG_X86_32
 # include <linux/ctype.h>
@@ -747,7 +748,7 @@ static int crash_nmi_callback(struct notifier_block *self,
 {
 	int cpu;
 
-	if (val != DIE_NMI_IPI)
+	if (val != DIE_NMI)
 		return NOTIFY_OK;
 
 	cpu = raw_smp_processor_id();
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index c7fd1ce..23f6ac0 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -394,6 +394,14 @@ static notrace __kprobes void default_do_nmi(struct pt_regs *regs)
 	unsigned char reason = 0;
 	int cpu;
 
+	/*
+	 * CPU-specific NMI must be processed before non-CPU-specific
+	 * NMI, otherwise we may lose it, because the CPU-specific
+	 * NMI can not be detected/processed on other CPUs.
+	 */
+	if (notify_die(DIE_NMI, "nmi", regs, 0, 2, SIGINT) == NOTIFY_STOP)
+		return;
+
 	cpu = smp_processor_id();
 
 	/* Only the BSP gets external NMIs from the system. */
@@ -401,21 +409,10 @@ static notrace __kprobes void default_do_nmi(struct pt_regs *regs)
 		reason = get_nmi_reason();
 
 	if (!(reason & NMI_REASON_MASK)) {
-		if (notify_die(DIE_NMI_IPI, "nmi_ipi", regs, reason, 2, SIGINT)
-								== NOTIFY_STOP)
-			return;
-
-#ifdef CONFIG_X86_LOCAL_APIC
-		if (notify_die(DIE_NMI, "nmi", regs, reason, 2, SIGINT)
-							== NOTIFY_STOP)
-			return;
-#endif
 		unknown_nmi_error(reason, regs);
 
 		return;
 	}
-	if (notify_die(DIE_NMI, "nmi", regs, reason, 2, SIGINT) == NOTIFY_STOP)
-		return;
 
 	/* AK: following checks seem to be broken on modern chipsets. FIXME */
 	if (reason & NMI_REASON_SERR)
diff --git a/arch/x86/oprofile/nmi_int.c b/arch/x86/oprofile/nmi_int.c
index 6e84ea4..e77ea0b 100644
--- a/arch/x86/oprofile/nmi_int.c
+++ b/arch/x86/oprofile/nmi_int.c
@@ -65,7 +65,6 @@ static int profile_exceptions_notify(struct notifier_block *self,
 
 	switch (val) {
 	case DIE_NMI:
-	case DIE_NMI_IPI:
 		if (ctr_running)
 			model->check_ctrs(args->regs, &__get_cpu_var(cpu_msrs));
 		else if (!nmi_enabled)
-- 
1.7.3.4

[PATCH 5/6] x86, NMI: Allow NMI reason io port (0x61) to be processed on any CPU

[Don Zickus]

In original NMI handler, NMI reason io port (0x61) is only processed
on BSP.  This makes it impossible to hot-remove BSP.  To solve the
issue, a raw spinlock is used to allow the port to be processed on any
CPU.

Originally-by: Huang Ying <ying.huang@intel.com>
Signed-off-by: Don Zickus <dzickus@redhat.com>
---
 arch/x86/kernel/traps.c |   16 ++++++++++------
 1 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 23f6ac0..613b3d2 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -84,6 +84,11 @@ EXPORT_SYMBOL_GPL(used_vectors);
 static int ignore_nmis;
 
 int unknown_nmi_panic;
+/*
+ * Prevent NMI reason port (0x61) being accessed simultaneously, can
+ * only be used in NMI handler.
+ */
+static DEFINE_RAW_SPINLOCK(nmi_reason_lock);
 
 static inline void conditional_sti(struct pt_regs *regs)
 {
@@ -392,7 +397,6 @@ unknown_nmi_error(unsigned char reason, struct pt_regs *regs)
 static notrace __kprobes void default_do_nmi(struct pt_regs *regs)
 {
 	unsigned char reason = 0;
-	int cpu;
 
 	/*
 	 * CPU-specific NMI must be processed before non-CPU-specific
@@ -402,13 +406,12 @@ static notrace __kprobes void default_do_nmi(struct pt_regs *regs)
 	if (notify_die(DIE_NMI, "nmi", regs, 0, 2, SIGINT) == NOTIFY_STOP)
 		return;
 
-	cpu = smp_processor_id();
-
-	/* Only the BSP gets external NMIs from the system. */
-	if (!cpu)
-		reason = get_nmi_reason();
+	/* Non-CPU-specific NMI: NMI sources can be processed on any CPU */
+	raw_spin_lock(&nmi_reason_lock);
+	reason = get_nmi_reason();
 
 	if (!(reason & NMI_REASON_MASK)) {
+		raw_spin_unlock(&nmi_reason_lock);
 		unknown_nmi_error(reason, regs);
 
 		return;
@@ -426,6 +429,7 @@ static notrace __kprobes void default_do_nmi(struct pt_regs *regs)
 	 */
 	reassert_nmi();
 #endif
+	raw_spin_unlock(&nmi_reason_lock);
 }
 
 dotraplinkage notrace __kprobes void
-- 
1.7.3.4

[PATCH 6/6] x86, NMI: Clean-up default_do_nmi()

[Don Zickus]

Just re-arrange the code a bit to make it easier to follow what is
going on.  Basically un-negating the if-statement and swapping the code
inside the if-statement with code outside.

No functional changes.

Originally-by: Huang Ying <ying.huang@intel.com>
Signed-off-by: Don Zickus <dzickus@redhat.com>
---
 arch/x86/kernel/traps.c |   30 ++++++++++++++----------------
 1 files changed, 14 insertions(+), 16 deletions(-)

diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 613b3d2..b9b6716 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -410,26 +410,24 @@ static notrace __kprobes void default_do_nmi(struct pt_regs *regs)
 	raw_spin_lock(&nmi_reason_lock);
 	reason = get_nmi_reason();
 
-	if (!(reason & NMI_REASON_MASK)) {
+	if (reason & NMI_REASON_MASK) {
+		if (reason & NMI_REASON_SERR)
+			pci_serr_error(reason, regs);
+		else if (reason & NMI_REASON_IOCHK)
+			io_check_error(reason, regs);
+#ifdef CONFIG_X86_32
+		/*
+		 * Reassert NMI in case it became active
+		 * meanwhile as it's edge-triggered:
+		 */
+		reassert_nmi();
+#endif
 		raw_spin_unlock(&nmi_reason_lock);
-		unknown_nmi_error(reason, regs);
-
 		return;
 	}
-
-	/* AK: following checks seem to be broken on modern chipsets. FIXME */
-	if (reason & NMI_REASON_SERR)
-		pci_serr_error(reason, regs);
-	if (reason & NMI_REASON_IOCHK)
-		io_check_error(reason, regs);
-#ifdef CONFIG_X86_32
-	/*
-	 * Reassert NMI in case it became active meanwhile
-	 * as it's edge-triggered:
-	 */
-	reassert_nmi();
-#endif
 	raw_spin_unlock(&nmi_reason_lock);
+
+	unknown_nmi_error(reason, regs);
 }
 
 dotraplinkage notrace __kprobes void
-- 
1.7.3.4

Re: [PATCH 0/6] x86, NMI: die_notifier and default_do_nmi cleanups

[Cyrill Gorcunov]

On 01/07/2011 12:18 AM, Don Zickus wrote:
> This is another version of the NMI rewrite patches I sent previously.  I
> take it those patches were too thick to be reviewed easily, so I broke out
> the patches differently to make it easier to review.  Hopefully it doesn't
> look as scary.
> 
> The bulk of the patch adds priorities to the die_chain notifiers for NMI
> events and removes the DIE_NMI_IPI event.  There are some other cleanups
> too, which include removing the restriction of the BSP cpu to process 
> external NMIs.
> 
> Cheers,
> Don
> 
> Don Zickus (5):
>   x86: Convert some devices to use DIE_NMIUNKNOWN
>   x86, NMI: Add priorities to handlers
>   x86, NMI: Remove DIE_NMI_IPI
>   x86, NMI: Allow NMI reason io port (0x61) to be processed on any CPU
>   x86, NMI: Clean-up default_do_nmi()
> 
> Huang Ying (1):
>   x86, NMI: Add NMI symbol constants and rename memory parity to PCI
>     SERR
> 

FWIW, the series looks good to me. Thanks Don.

-- 
    Cyrill

Re: [PATCH 0/6] x86, NMI: die_notifier and default_do_nmi cleanups

[Peter Zijlstra]

On Thu, 2011-01-06 at 16:18 -0500, Don Zickus wrote:
> This is another version of the NMI rewrite patches I sent previously.  I
> take it those patches were too thick to be reviewed easily, so I broke out
> the patches differently to make it easier to review.  Hopefully it doesn't
> look as scary.
> 
> The bulk of the patch adds priorities to the die_chain notifiers for NMI
> events and removes the DIE_NMI_IPI event.  There are some other cleanups
> too, which include removing the restriction of the BSP cpu to process 
> external NMIs.
> 
> Cheers,
> Don
> 
> Don Zickus (5):
>   x86: Convert some devices to use DIE_NMIUNKNOWN
>   x86, NMI: Add priorities to handlers
>   x86, NMI: Remove DIE_NMI_IPI
>   x86, NMI: Allow NMI reason io port (0x61) to be processed on any CPU
>   x86, NMI: Clean-up default_do_nmi()
> 
> Huang Ying (1):
>   x86, NMI: Add NMI symbol constants and rename memory parity to PCI
>     SERR
> 
>  arch/x86/include/asm/kdebug.h           |    1 -
>  arch/x86/include/asm/mach_traps.h       |   12 +++-
>  arch/x86/include/asm/nmi.h              |   20 ++++++
>  arch/x86/kernel/apic/hw_nmi.c           |    3 +-
>  arch/x86/kernel/apic/x2apic_uv_x.c      |    2 +-
>  arch/x86/kernel/cpu/mcheck/mce-inject.c |    5 +-
>  arch/x86/kernel/cpu/perf_event.c        |    3 +-
>  arch/x86/kernel/kgdb.c                  |    6 +--
>  arch/x86/kernel/reboot.c                |    5 +-
>  arch/x86/kernel/traps.c                 |  102 +++++++++++++++---------------
>  arch/x86/oprofile/nmi_int.c             |    3 +-
>  arch/x86/oprofile/nmi_timer_int.c       |    2 +-
>  drivers/char/ipmi/ipmi_watchdog.c       |    2 +-
>  drivers/watchdog/hpwdt.c                |    2 +-
>  14 files changed, 97 insertions(+), 71 deletions(-)

Acked-by: Peter Zijlstra <a.p.zijlstra@chello.nl>

Re: [PATCH 3/6] x86, NMI: Add priorities to handlers

[Peter Zijlstra]

On Thu, 2011-01-06 at 16:18 -0500, Don Zickus wrote:
> In order to consolidate the NMI die_chain events, we need to setup the priorities
> for the die notifiers.
> 
> I started by defining a bunch of common priorities that can be used by the
> notifier blocks.  Then I modified the notifier blocks to use the newly created
> priorities.
> 
> Now that the priorities are straightened out, it should be easier to remove the
> event DIE_NMI_IPI.
> 
> Signed-off-by: Don Zickus <dzickus@redhat.com>
> ---
>  arch/x86/include/asm/nmi.h              |   20 ++++++++++++++++++++
>  arch/x86/kernel/apic/hw_nmi.c           |    2 +-
>  arch/x86/kernel/cpu/mcheck/mce-inject.c |    2 +-
>  arch/x86/kernel/cpu/perf_event.c        |    2 +-
>  arch/x86/kernel/kgdb.c                  |    2 +-
>  arch/x86/kernel/reboot.c                |    2 ++
>  arch/x86/oprofile/nmi_int.c             |    2 +-
>  arch/x86/oprofile/nmi_timer_int.c       |    2 +-
>  8 files changed, 28 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/x86/include/asm/nmi.h b/arch/x86/include/asm/nmi.h
> index c4021b9..fb82fb6 100644
> --- a/arch/x86/include/asm/nmi.h
> +++ b/arch/x86/include/asm/nmi.h
> @@ -23,6 +23,26 @@ void arch_trigger_all_cpu_backtrace(void);
>  #define arch_trigger_all_cpu_backtrace arch_trigger_all_cpu_backtrace
>  #endif
>  
> +/*
> + * Define some priorities for the nmi notifier call chain.
> + *
> + * Create a local nmi bit that has a higher priority than
> + * external nmis, because the local ones are more frequent.
> + * 
> + * Also setup some default high/normal/low settings for
> + * subsystems to registers with.  Using 4 bits to seperate
> + * the priorities.  This can go alot higher if needed be.
> + */
> +
> +#define NMI_LOCAL_SHIFT		16	/* randomly picked */
> +#define NMI_LOCAL_BIT		(1ULL << NMI_LOCAL_SHIFT)
> +#define NMI_HIGH_PRIOR		(1ULL << 8)
> +#define NMI_NORMAL_PRIOR	(1ULL << 4)
> +#define NMI_LOW_PRIOR		(1ULL << 0)
> +#define NMI_LOCAL_HIGH_PRIOR	(NMI_LOCAL_BIT | NMI_HIGH_PRIOR)
> +#define NMI_LOCAL_NORMAL_PRIOR	(NMI_LOCAL_BIT | NMI_NORMAL_PRIOR)
> +#define NMI_LOCAL_LOW_PRIOR	(NMI_LOCAL_BIT | NMI_LOW_PRIOR)
> +
>  void stop_nmi(void);
>  void restart_nmi(void);
>  
 
>  static struct event_constraint unconstrained;
> diff --git a/arch/x86/kernel/kgdb.c b/arch/x86/kernel/kgdb.c
> index cd21b65..983fb54 100644
> --- a/arch/x86/kernel/kgdb.c
> +++ b/arch/x86/kernel/kgdb.c
> @@ -606,7 +606,7 @@ static struct notifier_block kgdb_notifier = {
>  	/*
>  	 * Lowest-prio notifier priority, we want to be notified last:
>  	 */
> -	.priority	= -INT_MAX,
> +	.priority	= NMI_LOCAL_LOW_PRIOR,
>  };
>  
>  /**


I had to add the below to make things build here.

---
Index: linux-2.6/arch/x86/kernel/kgdb.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/kgdb.c
+++ linux-2.6/arch/x86/kernel/kgdb.c
@@ -48,6 +48,7 @@
 #include <asm/apicdef.h>
 #include <asm/system.h>
 #include <asm/apic.h>
+#include <asm/nmi.h>
 
 struct dbg_reg_def_t dbg_reg_def[DBG_MAX_REG_NUM] =
 {

Re: [PATCH 3/6] x86, NMI: Add priorities to handlers

[Don Zickus]

On Fri, Jan 07, 2011 at 02:09:51PM +0100, Peter Zijlstra wrote:
> >  static struct event_constraint unconstrained;
> > diff --git a/arch/x86/kernel/kgdb.c b/arch/x86/kernel/kgdb.c
> > index cd21b65..983fb54 100644
> > --- a/arch/x86/kernel/kgdb.c
> > +++ b/arch/x86/kernel/kgdb.c
> > @@ -606,7 +606,7 @@ static struct notifier_block kgdb_notifier = {
> >  	/*
> >  	 * Lowest-prio notifier priority, we want to be notified last:
> >  	 */
> > -	.priority	= -INT_MAX,
> > +	.priority	= NMI_LOCAL_LOW_PRIOR,
> >  };
> >  
> >  /**
> 
> 
> I had to add the below to make things build here.
> 
> ---
> Index: linux-2.6/arch/x86/kernel/kgdb.c
> ===================================================================
> --- linux-2.6.orig/arch/x86/kernel/kgdb.c
> +++ linux-2.6/arch/x86/kernel/kgdb.c
> @@ -48,6 +48,7 @@
>  #include <asm/apicdef.h>
>  #include <asm/system.h>
>  #include <asm/apic.h>
> +#include <asm/nmi.h>
>  
>  struct dbg_reg_def_t dbg_reg_def[DBG_MAX_REG_NUM] =
>  {
> 

Doh.  I missed a spot.  Thanks for the catch.  I'll add that and repost.

Cheers,
Don

Re: [PATCH 3/6] x86, NMI: Add priorities to handlers

[Peter Zijlstra]

On Fri, 2011-01-07 at 09:43 -0500, Don Zickus wrote:

> > I had to add the below to make things build here.
> > 
> > ---
> > Index: linux-2.6/arch/x86/kernel/kgdb.c
> > ===================================================================
> > --- linux-2.6.orig/arch/x86/kernel/kgdb.c
> > +++ linux-2.6/arch/x86/kernel/kgdb.c
> > @@ -48,6 +48,7 @@
> >  #include <asm/apicdef.h>
> >  #include <asm/system.h>
> >  #include <asm/apic.h>
> > +#include <asm/nmi.h>
> >  
> >  struct dbg_reg_def_t dbg_reg_def[DBG_MAX_REG_NUM] =
> >  {
> > 
> 
> Doh.  I missed a spot.  Thanks for the catch.  I'll add that and repost.

I already fixed it up and send it Ingo-wards.

[tip:perf/core] x86, NMI: Add NMI symbol constants and rename memory parity to PCI SERR

[tip-bot for Huang Ying]

Commit-ID:  1c7b74d46fed530cca22a9a54140cdac2815c797
Gitweb:     http://git.kernel.org/tip/1c7b74d46fed530cca22a9a54140cdac2815c797
Author:     Huang Ying <ying.huang@intel.com>
AuthorDate: Thu, 6 Jan 2011 16:18:47 -0500
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Fri, 7 Jan 2011 15:08:51 +0100

x86, NMI: Add NMI symbol constants and rename memory parity to PCI SERR

Replace the NMI related magic numbers with symbol constants.

Memory parity error is only valid for IBM PC-AT, newer machine use
bit 7 (0x80) of 0x61 port for PCI SERR. While memory error is usually
reported via MCE. So corresponding function name and kernel log string
is changed.

But on some machines, PCI SERR line is still used to report memory
errors. This is used by EDAC, so corresponding EDAC call is reserved.

Signed-off-by: Huang Ying <ying.huang@intel.com>
Signed-off-by: Don Zickus <dzickus@redhat.com>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
LKML-Reference: <1294348732-15030-2-git-send-email-dzickus@redhat.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 arch/x86/include/asm/mach_traps.h |   12 ++++++++-
 arch/x86/kernel/traps.c           |   51 +++++++++++++++++++------------------
 2 files changed, 37 insertions(+), 26 deletions(-)

diff --git a/arch/x86/include/asm/mach_traps.h b/arch/x86/include/asm/mach_traps.h
index f792060..72a8b52 100644
--- a/arch/x86/include/asm/mach_traps.h
+++ b/arch/x86/include/asm/mach_traps.h
@@ -7,9 +7,19 @@
 
 #include <asm/mc146818rtc.h>
 
+#define NMI_REASON_PORT		0x61
+
+#define NMI_REASON_SERR		0x80
+#define NMI_REASON_IOCHK	0x40
+#define NMI_REASON_MASK		(NMI_REASON_SERR | NMI_REASON_IOCHK)
+
+#define NMI_REASON_CLEAR_SERR	0x04
+#define NMI_REASON_CLEAR_IOCHK	0x08
+#define NMI_REASON_CLEAR_MASK	0x0f
+
 static inline unsigned char get_nmi_reason(void)
 {
-	return inb(0x61);
+	return inb(NMI_REASON_PORT);
 }
 
 static inline void reassert_nmi(void)
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index c76aaca..c7fd1ce 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -310,15 +310,15 @@ static int __init setup_unknown_nmi_panic(char *str)
 __setup("unknown_nmi_panic", setup_unknown_nmi_panic);
 
 static notrace __kprobes void
-mem_parity_error(unsigned char reason, struct pt_regs *regs)
+pci_serr_error(unsigned char reason, struct pt_regs *regs)
 {
-	printk(KERN_EMERG
-		"Uhhuh. NMI received for unknown reason %02x on CPU %d.\n",
-			reason, smp_processor_id());
-
-	printk(KERN_EMERG
-		"You have some hardware problem, likely on the PCI bus.\n");
+	pr_emerg("NMI: PCI system error (SERR) for reason %02x on CPU %d.\n",
+		 reason, smp_processor_id());
 
+	/*
+	 * On some machines, PCI SERR line is used to report memory
+	 * errors. EDAC makes use of it.
+	 */
 #if defined(CONFIG_EDAC)
 	if (edac_handler_set()) {
 		edac_atomic_assert_error();
@@ -329,11 +329,11 @@ mem_parity_error(unsigned char reason, struct pt_regs *regs)
 	if (panic_on_unrecovered_nmi)
 		panic("NMI: Not continuing");
 
-	printk(KERN_EMERG "Dazed and confused, but trying to continue\n");
+	pr_emerg("Dazed and confused, but trying to continue\n");
 
-	/* Clear and disable the memory parity error line. */
-	reason = (reason & 0xf) | 4;
-	outb(reason, 0x61);
+	/* Clear and disable the PCI SERR error line. */
+	reason = (reason & NMI_REASON_CLEAR_MASK) | NMI_REASON_CLEAR_SERR;
+	outb(reason, NMI_REASON_PORT);
 }
 
 static notrace __kprobes void
@@ -341,15 +341,17 @@ io_check_error(unsigned char reason, struct pt_regs *regs)
 {
 	unsigned long i;
 
-	printk(KERN_EMERG "NMI: IOCK error (debug interrupt?)\n");
+	pr_emerg(
+	"NMI: IOCK error (debug interrupt?) for reason %02x on CPU %d.\n",
+		 reason, smp_processor_id());
 	show_registers(regs);
 
 	if (panic_on_io_nmi)
 		panic("NMI IOCK error: Not continuing");
 
 	/* Re-enable the IOCK line, wait for a few seconds */
-	reason = (reason & 0xf) | 8;
-	outb(reason, 0x61);
+	reason = (reason & NMI_REASON_CLEAR_MASK) | NMI_REASON_CLEAR_IOCHK;
+	outb(reason, NMI_REASON_PORT);
 
 	i = 20000;
 	while (--i) {
@@ -357,8 +359,8 @@ io_check_error(unsigned char reason, struct pt_regs *regs)
 		udelay(100);
 	}
 
-	reason &= ~8;
-	outb(reason, 0x61);
+	reason &= ~NMI_REASON_CLEAR_IOCHK;
+	outb(reason, NMI_REASON_PORT);
 }
 
 static notrace __kprobes void
@@ -377,15 +379,14 @@ unknown_nmi_error(unsigned char reason, struct pt_regs *regs)
 		return;
 	}
 #endif
-	printk(KERN_EMERG
-		"Uhhuh. NMI received for unknown reason %02x on CPU %d.\n",
-			reason, smp_processor_id());
+	pr_emerg("Uhhuh. NMI received for unknown reason %02x on CPU %d.\n",
+		 reason, smp_processor_id());
 
-	printk(KERN_EMERG "Do you have a strange power saving mode enabled?\n");
+	pr_emerg("Do you have a strange power saving mode enabled?\n");
 	if (unknown_nmi_panic || panic_on_unrecovered_nmi)
 		panic("NMI: Not continuing");
 
-	printk(KERN_EMERG "Dazed and confused, but trying to continue\n");
+	pr_emerg("Dazed and confused, but trying to continue\n");
 }
 
 static notrace __kprobes void default_do_nmi(struct pt_regs *regs)
@@ -399,7 +400,7 @@ static notrace __kprobes void default_do_nmi(struct pt_regs *regs)
 	if (!cpu)
 		reason = get_nmi_reason();
 
-	if (!(reason & 0xc0)) {
+	if (!(reason & NMI_REASON_MASK)) {
 		if (notify_die(DIE_NMI_IPI, "nmi_ipi", regs, reason, 2, SIGINT)
 								== NOTIFY_STOP)
 			return;
@@ -417,9 +418,9 @@ static notrace __kprobes void default_do_nmi(struct pt_regs *regs)
 		return;
 
 	/* AK: following checks seem to be broken on modern chipsets. FIXME */
-	if (reason & 0x80)
-		mem_parity_error(reason, regs);
-	if (reason & 0x40)
+	if (reason & NMI_REASON_SERR)
+		pci_serr_error(reason, regs);
+	if (reason & NMI_REASON_IOCHK)
 		io_check_error(reason, regs);
 #ifdef CONFIG_X86_32
 	/*

[tip:perf/core] x86: Convert some devices to use DIE_NMIUNKNOWN

[tip-bot for Don Zickus]

Commit-ID:  673a6092ce5f5bec45619b7a7f89cfcf8bcf3c41
Gitweb:     http://git.kernel.org/tip/673a6092ce5f5bec45619b7a7f89cfcf8bcf3c41
Author:     Don Zickus <dzickus@redhat.com>
AuthorDate: Thu, 6 Jan 2011 16:18:48 -0500
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Fri, 7 Jan 2011 15:08:52 +0100

x86: Convert some devices to use DIE_NMIUNKNOWN

They are a handful of places in the code that register a die_notifier
as a catch all in case no claims the NMI.  Unfortunately, they trigger
on events like DIE_NMI and DIE_NMI_IPI, which depending on when they
registered may collide with other handlers that have the ability to
determine if the NMI is theirs or not.

The function unknown_nmi_error() makes one last effort to walk the
die_chain when no one else has claimed the NMI before spitting out
messages that the NMI is unknown.

This is a better spot for these devices to execute any code without
colliding with the other handlers.

The two drivers modified are only compiled on x86 arches I believe, so
they shouldn't be affected by other arches that may not have
DIE_NMIUNKNOWN defined.

Signed-off-by: Don Zickus <dzickus@redhat.com>
Cc: Russ Anderson <rja@sgi.com>
Cc: Corey Minyard <minyard@acm.org>
Cc: openipmi-developer@lists.sourceforge.net
Cc: dann frazier <dannf@hp.com>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
LKML-Reference: <1294348732-15030-3-git-send-email-dzickus@redhat.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 arch/x86/kernel/apic/x2apic_uv_x.c |    2 +-
 drivers/char/ipmi/ipmi_watchdog.c  |    2 +-
 drivers/watchdog/hpwdt.c           |    2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/apic/x2apic_uv_x.c b/arch/x86/kernel/apic/x2apic_uv_x.c
index c1c52c3..927902d 100644
--- a/arch/x86/kernel/apic/x2apic_uv_x.c
+++ b/arch/x86/kernel/apic/x2apic_uv_x.c
@@ -639,7 +639,7 @@ void __cpuinit uv_cpu_init(void)
  */
 int uv_handle_nmi(struct notifier_block *self, unsigned long reason, void *data)
 {
-	if (reason != DIE_NMI_IPI)
+	if (reason != DIE_NMIUNKNOWN)
 		return NOTIFY_OK;
 
 	if (in_crash_kexec)
diff --git a/drivers/char/ipmi/ipmi_watchdog.c b/drivers/char/ipmi/ipmi_watchdog.c
index f4d334f..320668f 100644
--- a/drivers/char/ipmi/ipmi_watchdog.c
+++ b/drivers/char/ipmi/ipmi_watchdog.c
@@ -1081,7 +1081,7 @@ ipmi_nmi(struct notifier_block *self, unsigned long val, void *data)
 {
 	struct die_args *args = data;
 
-	if (val != DIE_NMI)
+	if (val != DIE_NMIUNKNOWN)
 		return NOTIFY_OK;
 
 	/* Hack, if it's a memory or I/O error, ignore it. */
diff --git a/drivers/watchdog/hpwdt.c b/drivers/watchdog/hpwdt.c
index dea7b5b..24b966d 100644
--- a/drivers/watchdog/hpwdt.c
+++ b/drivers/watchdog/hpwdt.c
@@ -469,7 +469,7 @@ static int hpwdt_pretimeout(struct notifier_block *nb, unsigned long ulReason,
 	unsigned long rom_pl;
 	static int die_nmi_called;
 
-	if (ulReason != DIE_NMI && ulReason != DIE_NMI_IPI)
+	if (ulReason != DIE_NMIUNKNOWN)
 		goto out;
 
 	if (!hpwdt_nmi_decoding)

[tip:perf/core] x86, NMI: Add priorities to handlers

[tip-bot for Don Zickus]

Commit-ID:  166d751479c6d4e5b17dfc1f204a9c4397c9b3f1
Gitweb:     http://git.kernel.org/tip/166d751479c6d4e5b17dfc1f204a9c4397c9b3f1
Author:     Don Zickus <dzickus@redhat.com>
AuthorDate: Thu, 6 Jan 2011 16:18:49 -0500
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Fri, 7 Jan 2011 15:08:52 +0100

x86, NMI: Add priorities to handlers

In order to consolidate the NMI die_chain events, we need to setup the priorities
for the die notifiers.

I started by defining a bunch of common priorities that can be used by the
notifier blocks.  Then I modified the notifier blocks to use the newly created
priorities.

Now that the priorities are straightened out, it should be easier to remove the
event DIE_NMI_IPI.

Signed-off-by: Don Zickus <dzickus@redhat.com>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
LKML-Reference: <1294348732-15030-4-git-send-email-dzickus@redhat.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 arch/x86/include/asm/nmi.h              |   20 ++++++++++++++++++++
 arch/x86/kernel/apic/hw_nmi.c           |    2 +-
 arch/x86/kernel/cpu/mcheck/mce-inject.c |    2 +-
 arch/x86/kernel/cpu/perf_event.c        |    2 +-
 arch/x86/kernel/kgdb.c                  |    3 ++-
 arch/x86/kernel/reboot.c                |    2 ++
 arch/x86/oprofile/nmi_int.c             |    2 +-
 arch/x86/oprofile/nmi_timer_int.c       |    2 +-
 8 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/nmi.h b/arch/x86/include/asm/nmi.h
index c4021b9..c76f5b9 100644
--- a/arch/x86/include/asm/nmi.h
+++ b/arch/x86/include/asm/nmi.h
@@ -23,6 +23,26 @@ void arch_trigger_all_cpu_backtrace(void);
 #define arch_trigger_all_cpu_backtrace arch_trigger_all_cpu_backtrace
 #endif
 
+/*
+ * Define some priorities for the nmi notifier call chain.
+ *
+ * Create a local nmi bit that has a higher priority than
+ * external nmis, because the local ones are more frequent.
+ *
+ * Also setup some default high/normal/low settings for
+ * subsystems to registers with.  Using 4 bits to seperate
+ * the priorities.  This can go alot higher if needed be.
+ */
+
+#define NMI_LOCAL_SHIFT		16	/* randomly picked */
+#define NMI_LOCAL_BIT		(1ULL << NMI_LOCAL_SHIFT)
+#define NMI_HIGH_PRIOR		(1ULL << 8)
+#define NMI_NORMAL_PRIOR	(1ULL << 4)
+#define NMI_LOW_PRIOR		(1ULL << 0)
+#define NMI_LOCAL_HIGH_PRIOR	(NMI_LOCAL_BIT | NMI_HIGH_PRIOR)
+#define NMI_LOCAL_NORMAL_PRIOR	(NMI_LOCAL_BIT | NMI_NORMAL_PRIOR)
+#define NMI_LOCAL_LOW_PRIOR	(NMI_LOCAL_BIT | NMI_LOW_PRIOR)
+
 void stop_nmi(void);
 void restart_nmi(void);
 
diff --git a/arch/x86/kernel/apic/hw_nmi.c b/arch/x86/kernel/apic/hw_nmi.c
index 72ec29e..8bc49f1 100644
--- a/arch/x86/kernel/apic/hw_nmi.c
+++ b/arch/x86/kernel/apic/hw_nmi.c
@@ -96,7 +96,7 @@ arch_trigger_all_cpu_backtrace_handler(struct notifier_block *self,
 static __read_mostly struct notifier_block backtrace_notifier = {
 	.notifier_call          = arch_trigger_all_cpu_backtrace_handler,
 	.next                   = NULL,
-	.priority               = 1
+	.priority               = NMI_LOCAL_LOW_PRIOR,
 };
 
 static int __init register_trigger_all_cpu_backtrace(void)
diff --git a/arch/x86/kernel/cpu/mcheck/mce-inject.c b/arch/x86/kernel/cpu/mcheck/mce-inject.c
index e7dbde7..59546c1 100644
--- a/arch/x86/kernel/cpu/mcheck/mce-inject.c
+++ b/arch/x86/kernel/cpu/mcheck/mce-inject.c
@@ -95,7 +95,7 @@ static int mce_raise_notify(struct notifier_block *self,
 
 static struct notifier_block mce_raise_nb = {
 	.notifier_call = mce_raise_notify,
-	.priority = 1000,
+	.priority = NMI_LOCAL_NORMAL_PRIOR,
 };
 
 /* Inject mce on current CPU */
diff --git a/arch/x86/kernel/cpu/perf_event.c b/arch/x86/kernel/cpu/perf_event.c
index 0a360d1..5e14b5e 100644
--- a/arch/x86/kernel/cpu/perf_event.c
+++ b/arch/x86/kernel/cpu/perf_event.c
@@ -1318,7 +1318,7 @@ perf_event_nmi_handler(struct notifier_block *self,
 static __read_mostly struct notifier_block perf_event_nmi_notifier = {
 	.notifier_call		= perf_event_nmi_handler,
 	.next			= NULL,
-	.priority		= 1
+	.priority		= NMI_LOCAL_LOW_PRIOR,
 };
 
 static struct event_constraint unconstrained;
diff --git a/arch/x86/kernel/kgdb.c b/arch/x86/kernel/kgdb.c
index cd21b65..d43c841 100644
--- a/arch/x86/kernel/kgdb.c
+++ b/arch/x86/kernel/kgdb.c
@@ -48,6 +48,7 @@
 #include <asm/apicdef.h>
 #include <asm/system.h>
 #include <asm/apic.h>
+#include <asm/nmi.h>
 
 struct dbg_reg_def_t dbg_reg_def[DBG_MAX_REG_NUM] =
 {
@@ -606,7 +607,7 @@ static struct notifier_block kgdb_notifier = {
 	/*
 	 * Lowest-prio notifier priority, we want to be notified last:
 	 */
-	.priority	= -INT_MAX,
+	.priority	= NMI_LOCAL_LOW_PRIOR,
 };
 
 /**
diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
index c495aa8..9c1c83e 100644
--- a/arch/x86/kernel/reboot.c
+++ b/arch/x86/kernel/reboot.c
@@ -778,6 +778,8 @@ static void smp_send_nmi_allbutself(void)
 
 static struct notifier_block crash_nmi_nb = {
 	.notifier_call = crash_nmi_callback,
+	/* we want to be the first one called */
+	.priority = NMI_LOCAL_HIGH_PRIOR+1,
 };
 
 /* Halt all other CPUs, calling the specified function on each of them
diff --git a/arch/x86/oprofile/nmi_int.c b/arch/x86/oprofile/nmi_int.c
index 358c8b9..6e84ea4 100644
--- a/arch/x86/oprofile/nmi_int.c
+++ b/arch/x86/oprofile/nmi_int.c
@@ -361,7 +361,7 @@ static void nmi_cpu_setup(void *dummy)
 static struct notifier_block profile_exceptions_nb = {
 	.notifier_call = profile_exceptions_notify,
 	.next = NULL,
-	.priority = 2
+	.priority = NMI_LOCAL_LOW_PRIOR,
 };
 
 static void nmi_cpu_restore_registers(struct op_msrs *msrs)
diff --git a/arch/x86/oprofile/nmi_timer_int.c b/arch/x86/oprofile/nmi_timer_int.c
index 0636dd9..720bf5a 100644
--- a/arch/x86/oprofile/nmi_timer_int.c
+++ b/arch/x86/oprofile/nmi_timer_int.c
@@ -38,7 +38,7 @@ static int profile_timer_exceptions_notify(struct notifier_block *self,
 static struct notifier_block profile_timer_exceptions_nb = {
 	.notifier_call = profile_timer_exceptions_notify,
 	.next = NULL,
-	.priority = 0
+	.priority = NMI_LOW_PRIOR,
 };
 
 static int timer_start(void)

[tip:perf/core] x86, NMI: Remove DIE_NMI_IPI

[tip-bot for Don Zickus]

Commit-ID:  c410b8307702c1e1f35be3fd868ad18e4ba0410f
Gitweb:     http://git.kernel.org/tip/c410b8307702c1e1f35be3fd868ad18e4ba0410f
Author:     Don Zickus <dzickus@redhat.com>
AuthorDate: Thu, 6 Jan 2011 16:18:50 -0500
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Fri, 7 Jan 2011 15:08:53 +0100

x86, NMI: Remove DIE_NMI_IPI

With priorities in place and no one really understanding the difference between
DIE_NMI and DIE_NMI_IPI, just remove DIE_NMI_IPI and convert everyone to DIE_NMI.

This also simplifies default_do_nmi() a little bit.  Instead of calling the
die_notifier in both the if and else part, just pull it out and call it before
the if-statement.  This has the side benefit of avoiding a call to the ioport
to see if there is an external NMI sitting around until after the (more frequent)
internal NMIs are dealt with.

Patch-Inspired-by: Huang Ying <ying.huang@intel.com>
Signed-off-by: Don Zickus <dzickus@redhat.com>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
LKML-Reference: <1294348732-15030-5-git-send-email-dzickus@redhat.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 arch/x86/include/asm/kdebug.h           |    1 -
 arch/x86/kernel/apic/hw_nmi.c           |    1 -
 arch/x86/kernel/cpu/mcheck/mce-inject.c |    3 ++-
 arch/x86/kernel/cpu/perf_event.c        |    1 -
 arch/x86/kernel/kgdb.c                  |    4 ----
 arch/x86/kernel/reboot.c                |    3 ++-
 arch/x86/kernel/traps.c                 |   19 ++++++++-----------
 arch/x86/oprofile/nmi_int.c             |    1 -
 8 files changed, 12 insertions(+), 21 deletions(-)

diff --git a/arch/x86/include/asm/kdebug.h b/arch/x86/include/asm/kdebug.h
index f23eb25..ca242d3 100644
--- a/arch/x86/include/asm/kdebug.h
+++ b/arch/x86/include/asm/kdebug.h
@@ -18,7 +18,6 @@ enum die_val {
 	DIE_TRAP,
 	DIE_GPF,
 	DIE_CALL,
-	DIE_NMI_IPI,
 	DIE_PAGE_FAULT,
 	DIE_NMIUNKNOWN,
 };
diff --git a/arch/x86/kernel/apic/hw_nmi.c b/arch/x86/kernel/apic/hw_nmi.c
index 8bc49f1..79fd43c 100644
--- a/arch/x86/kernel/apic/hw_nmi.c
+++ b/arch/x86/kernel/apic/hw_nmi.c
@@ -68,7 +68,6 @@ arch_trigger_all_cpu_backtrace_handler(struct notifier_block *self,
 
 	switch (cmd) {
 	case DIE_NMI:
-	case DIE_NMI_IPI:
 		break;
 
 	default:
diff --git a/arch/x86/kernel/cpu/mcheck/mce-inject.c b/arch/x86/kernel/cpu/mcheck/mce-inject.c
index 59546c1..a779719 100644
--- a/arch/x86/kernel/cpu/mcheck/mce-inject.c
+++ b/arch/x86/kernel/cpu/mcheck/mce-inject.c
@@ -25,6 +25,7 @@
 #include <linux/gfp.h>
 #include <asm/mce.h>
 #include <asm/apic.h>
+#include <asm/nmi.h>
 
 /* Update fake mce registers on current CPU. */
 static void inject_mce(struct mce *m)
@@ -83,7 +84,7 @@ static int mce_raise_notify(struct notifier_block *self,
 	struct die_args *args = (struct die_args *)data;
 	int cpu = smp_processor_id();
 	struct mce *m = &__get_cpu_var(injectm);
-	if (val != DIE_NMI_IPI || !cpumask_test_cpu(cpu, mce_inject_cpumask))
+	if (val != DIE_NMI || !cpumask_test_cpu(cpu, mce_inject_cpumask))
 		return NOTIFY_DONE;
 	cpumask_clear_cpu(cpu, mce_inject_cpumask);
 	if (m->inject_flags & MCJ_EXCEPTION)
diff --git a/arch/x86/kernel/cpu/perf_event.c b/arch/x86/kernel/cpu/perf_event.c
index 5e14b5e..c71bae4 100644
--- a/arch/x86/kernel/cpu/perf_event.c
+++ b/arch/x86/kernel/cpu/perf_event.c
@@ -1268,7 +1268,6 @@ perf_event_nmi_handler(struct notifier_block *self,
 
 	switch (cmd) {
 	case DIE_NMI:
-	case DIE_NMI_IPI:
 		break;
 	case DIE_NMIUNKNOWN:
 		this_nmi = percpu_read(irq_stat.__nmi_count);
diff --git a/arch/x86/kernel/kgdb.c b/arch/x86/kernel/kgdb.c
index d43c841..a413000 100644
--- a/arch/x86/kernel/kgdb.c
+++ b/arch/x86/kernel/kgdb.c
@@ -526,10 +526,6 @@ static int __kgdb_notify(struct die_args *args, unsigned long cmd)
 		}
 		return NOTIFY_DONE;
 
-	case DIE_NMI_IPI:
-		/* Just ignore, we will handle the roundup on DIE_NMI. */
-		return NOTIFY_DONE;
-
 	case DIE_NMIUNKNOWN:
 		if (was_in_debug_nmi[raw_smp_processor_id()]) {
 			was_in_debug_nmi[raw_smp_processor_id()] = 0;
diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
index 9c1c83e..fc7aae1 100644
--- a/arch/x86/kernel/reboot.c
+++ b/arch/x86/kernel/reboot.c
@@ -18,6 +18,7 @@
 #include <asm/pci_x86.h>
 #include <asm/virtext.h>
 #include <asm/cpu.h>
+#include <asm/nmi.h>
 
 #ifdef CONFIG_X86_32
 # include <linux/ctype.h>
@@ -747,7 +748,7 @@ static int crash_nmi_callback(struct notifier_block *self,
 {
 	int cpu;
 
-	if (val != DIE_NMI_IPI)
+	if (val != DIE_NMI)
 		return NOTIFY_OK;
 
 	cpu = raw_smp_processor_id();
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index c7fd1ce..23f6ac0 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -394,6 +394,14 @@ static notrace __kprobes void default_do_nmi(struct pt_regs *regs)
 	unsigned char reason = 0;
 	int cpu;
 
+	/*
+	 * CPU-specific NMI must be processed before non-CPU-specific
+	 * NMI, otherwise we may lose it, because the CPU-specific
+	 * NMI can not be detected/processed on other CPUs.
+	 */
+	if (notify_die(DIE_NMI, "nmi", regs, 0, 2, SIGINT) == NOTIFY_STOP)
+		return;
+
 	cpu = smp_processor_id();
 
 	/* Only the BSP gets external NMIs from the system. */
@@ -401,21 +409,10 @@ static notrace __kprobes void default_do_nmi(struct pt_regs *regs)
 		reason = get_nmi_reason();
 
 	if (!(reason & NMI_REASON_MASK)) {
-		if (notify_die(DIE_NMI_IPI, "nmi_ipi", regs, reason, 2, SIGINT)
-								== NOTIFY_STOP)
-			return;
-
-#ifdef CONFIG_X86_LOCAL_APIC
-		if (notify_die(DIE_NMI, "nmi", regs, reason, 2, SIGINT)
-							== NOTIFY_STOP)
-			return;
-#endif
 		unknown_nmi_error(reason, regs);
 
 		return;
 	}
-	if (notify_die(DIE_NMI, "nmi", regs, reason, 2, SIGINT) == NOTIFY_STOP)
-		return;
 
 	/* AK: following checks seem to be broken on modern chipsets. FIXME */
 	if (reason & NMI_REASON_SERR)
diff --git a/arch/x86/oprofile/nmi_int.c b/arch/x86/oprofile/nmi_int.c
index 6e84ea4..e77ea0b 100644
--- a/arch/x86/oprofile/nmi_int.c
+++ b/arch/x86/oprofile/nmi_int.c
@@ -65,7 +65,6 @@ static int profile_exceptions_notify(struct notifier_block *self,
 
 	switch (val) {
 	case DIE_NMI:
-	case DIE_NMI_IPI:
 		if (ctr_running)
 			model->check_ctrs(args->regs, &__get_cpu_var(cpu_msrs));
 		else if (!nmi_enabled)

[tip:perf/core] x86, NMI: Allow NMI reason io port (0x61) to be processed on any CPU

[tip-bot for Don Zickus]

Commit-ID:  ab846f13f69fa64f8ed69ce0c3e239e075910d23
Gitweb:     http://git.kernel.org/tip/ab846f13f69fa64f8ed69ce0c3e239e075910d23
Author:     Don Zickus <dzickus@redhat.com>
AuthorDate: Thu, 6 Jan 2011 16:18:51 -0500
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Fri, 7 Jan 2011 15:08:53 +0100

x86, NMI: Allow NMI reason io port (0x61) to be processed on any CPU

In original NMI handler, NMI reason io port (0x61) is only processed
on BSP.  This makes it impossible to hot-remove BSP.  To solve the
issue, a raw spinlock is used to allow the port to be processed on any
CPU.

Originally-by: Huang Ying <ying.huang@intel.com>
Signed-off-by: Don Zickus <dzickus@redhat.com>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
LKML-Reference: <1294348732-15030-6-git-send-email-dzickus@redhat.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 arch/x86/kernel/traps.c |   16 ++++++++++------
 1 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 23f6ac0..613b3d2 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -84,6 +84,11 @@ EXPORT_SYMBOL_GPL(used_vectors);
 static int ignore_nmis;
 
 int unknown_nmi_panic;
+/*
+ * Prevent NMI reason port (0x61) being accessed simultaneously, can
+ * only be used in NMI handler.
+ */
+static DEFINE_RAW_SPINLOCK(nmi_reason_lock);
 
 static inline void conditional_sti(struct pt_regs *regs)
 {
@@ -392,7 +397,6 @@ unknown_nmi_error(unsigned char reason, struct pt_regs *regs)
 static notrace __kprobes void default_do_nmi(struct pt_regs *regs)
 {
 	unsigned char reason = 0;
-	int cpu;
 
 	/*
 	 * CPU-specific NMI must be processed before non-CPU-specific
@@ -402,13 +406,12 @@ static notrace __kprobes void default_do_nmi(struct pt_regs *regs)
 	if (notify_die(DIE_NMI, "nmi", regs, 0, 2, SIGINT) == NOTIFY_STOP)
 		return;
 
-	cpu = smp_processor_id();
-
-	/* Only the BSP gets external NMIs from the system. */
-	if (!cpu)
-		reason = get_nmi_reason();
+	/* Non-CPU-specific NMI: NMI sources can be processed on any CPU */
+	raw_spin_lock(&nmi_reason_lock);
+	reason = get_nmi_reason();
 
 	if (!(reason & NMI_REASON_MASK)) {
+		raw_spin_unlock(&nmi_reason_lock);
 		unknown_nmi_error(reason, regs);
 
 		return;
@@ -426,6 +429,7 @@ static notrace __kprobes void default_do_nmi(struct pt_regs *regs)
 	 */
 	reassert_nmi();
 #endif
+	raw_spin_unlock(&nmi_reason_lock);
 }
 
 dotraplinkage notrace __kprobes void

[tip:perf/core] x86, NMI: Clean-up default_do_nmi()

[tip-bot for Don Zickus]

Commit-ID:  f2fd43954abc058586e95d4eb91e7a5477070704
Gitweb:     http://git.kernel.org/tip/f2fd43954abc058586e95d4eb91e7a5477070704
Author:     Don Zickus <dzickus@redhat.com>
AuthorDate: Thu, 6 Jan 2011 16:18:52 -0500
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Fri, 7 Jan 2011 15:08:53 +0100

x86, NMI: Clean-up default_do_nmi()

Just re-arrange the code a bit to make it easier to follow what is
going on.  Basically un-negating the if-statement and swapping the code
inside the if-statement with code outside.

No functional changes.

Originally-by: Huang Ying <ying.huang@intel.com>
Signed-off-by: Don Zickus <dzickus@redhat.com>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
LKML-Reference: <1294348732-15030-7-git-send-email-dzickus@redhat.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 arch/x86/kernel/traps.c |   30 ++++++++++++++----------------
 1 files changed, 14 insertions(+), 16 deletions(-)

diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 613b3d2..b9b6716 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -410,26 +410,24 @@ static notrace __kprobes void default_do_nmi(struct pt_regs *regs)
 	raw_spin_lock(&nmi_reason_lock);
 	reason = get_nmi_reason();
 
-	if (!(reason & NMI_REASON_MASK)) {
+	if (reason & NMI_REASON_MASK) {
+		if (reason & NMI_REASON_SERR)
+			pci_serr_error(reason, regs);
+		else if (reason & NMI_REASON_IOCHK)
+			io_check_error(reason, regs);
+#ifdef CONFIG_X86_32
+		/*
+		 * Reassert NMI in case it became active
+		 * meanwhile as it's edge-triggered:
+		 */
+		reassert_nmi();
+#endif
 		raw_spin_unlock(&nmi_reason_lock);
-		unknown_nmi_error(reason, regs);
-
 		return;
 	}
-
-	/* AK: following checks seem to be broken on modern chipsets. FIXME */
-	if (reason & NMI_REASON_SERR)
-		pci_serr_error(reason, regs);
-	if (reason & NMI_REASON_IOCHK)
-		io_check_error(reason, regs);
-#ifdef CONFIG_X86_32
-	/*
-	 * Reassert NMI in case it became active meanwhile
-	 * as it's edge-triggered:
-	 */
-	reassert_nmi();
-#endif
 	raw_spin_unlock(&nmi_reason_lock);
+
+	unknown_nmi_error(reason, regs);
 }
 
 dotraplinkage notrace __kprobes void

Re: [PATCH 3/6] x86, NMI: Add priorities to handlers

[Don Zickus]

On Fri, Jan 07, 2011 at 03:50:17PM +0100, Peter Zijlstra wrote:
> On Fri, 2011-01-07 at 09:43 -0500, Don Zickus wrote:
> 
> > > I had to add the below to make things build here.
> > > 
> > > ---
> > > Index: linux-2.6/arch/x86/kernel/kgdb.c
> > > ===================================================================
> > > --- linux-2.6.orig/arch/x86/kernel/kgdb.c
> > > +++ linux-2.6/arch/x86/kernel/kgdb.c
> > > @@ -48,6 +48,7 @@
> > >  #include <asm/apicdef.h>
> > >  #include <asm/system.h>
> > >  #include <asm/apic.h>
> > > +#include <asm/nmi.h>
> > >  
> > >  struct dbg_reg_def_t dbg_reg_def[DBG_MAX_REG_NUM] =
> > >  {
> > > 
> > 
> > Doh.  I missed a spot.  Thanks for the catch.  I'll add that and repost.
> 
> I already fixed it up and send it Ingo-wards.

Awesome.  Thanks for that!

Cheers,
Don

Re: [PATCH 5/6] x86, NMI: Allow NMI reason io port (0x61) to be processed on any CPU

[Maciej W. Rozycki]

On Thu, 6 Jan 2011, Don Zickus wrote:

> In original NMI handler, NMI reason io port (0x61) is only processed
> on BSP.  This makes it impossible to hot-remove BSP.  To solve the
> issue, a raw spinlock is used to allow the port to be processed on any
> CPU.
> 
> Originally-by: Huang Ying <ying.huang@intel.com>
> Signed-off-by: Don Zickus <dzickus@redhat.com>
> ---
>  arch/x86/kernel/traps.c |   16 ++++++++++------
>  1 files changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
> index 23f6ac0..613b3d2 100644
> --- a/arch/x86/kernel/traps.c
> +++ b/arch/x86/kernel/traps.c
> @@ -402,13 +406,12 @@ static notrace __kprobes void default_do_nmi(struct pt_regs *regs)
>  	if (notify_die(DIE_NMI, "nmi", regs, 0, 2, SIGINT) == NOTIFY_STOP)
>  		return;
>  
> -	cpu = smp_processor_id();
> -
> -	/* Only the BSP gets external NMIs from the system. */
> -	if (!cpu)
> -		reason = get_nmi_reason();
> +	/* Non-CPU-specific NMI: NMI sources can be processed on any CPU */
> +	raw_spin_lock(&nmi_reason_lock);
> +	reason = get_nmi_reason();
>  
>  	if (!(reason & NMI_REASON_MASK)) {
> +		raw_spin_unlock(&nmi_reason_lock);
>  		unknown_nmi_error(reason, regs);
>  
>  		return;

 [Catching up with old e-mail...]

 In line with the comment above that you're removing -- have you (or 
anyone else) adjusted code elsewhere so that external NMIs are actually 
delivered to processors other than the BSP?  I can't see such code in this 
series nor an explanation as to why it wouldn't be needed.

 For the record -- the piece of code above reflects our setup where the 
LINT1 input is enabled and configured for the NMI delivery mode on the BSP 
only and all the other processors have this line disabled in their local 
APIC units.  If system NMIs are to be handled after the removal of the 
BSP, then another processor has to be selected and configured for NMI 
reception.  Alternatively, all local units could have their LINT1 input 
enabled and arbitrate handling, although it would be quite disruptive as 
all the processors would take the interrupt if it happened.  OTOH it would 
be more fault-tolerant in the case of a CPU failure.  On a typical x86 box 
the system NMI cannot be routed to an I/O APIC input.

  Maciej

Re: [PATCH 5/6] x86, NMI: Allow NMI reason io port (0x61) to be processed on any CPU

[Don Zickus]

On Wed, Feb 23, 2011 at 02:39:58AM +0000, Maciej W. Rozycki wrote:
>  [Catching up with old e-mail...]
> 
>  In line with the comment above that you're removing -- have you (or 
> anyone else) adjusted code elsewhere so that external NMIs are actually 
> delivered to processors other than the BSP?  I can't see such code in this 
> series nor an explanation as to why it wouldn't be needed.
> 
>  For the record -- the piece of code above reflects our setup where the 
> LINT1 input is enabled and configured for the NMI delivery mode on the BSP 
> only and all the other processors have this line disabled in their local 
> APIC units.  If system NMIs are to be handled after the removal of the 
> BSP, then another processor has to be selected and configured for NMI 
> reception.  Alternatively, all local units could have their LINT1 input 
> enabled and arbitrate handling, although it would be quite disruptive as 
> all the processors would take the interrupt if it happened.  OTOH it would 
> be more fault-tolerant in the case of a CPU failure.  On a typical x86 box 
> the system NMI cannot be routed to an I/O APIC input.

Intel requested this.  From my understanding they are working on
technology to allow removal of the BSP and still having the system work
correctly.  This patch is probably just the first step of many.  But I
don't really know what they are working on nor what else they need.

Cheers,
Don

Re: [PATCH 5/6] x86, NMI: Allow NMI reason io port (0x61) to be processed on any CPU

[Cyrill Gorcunov]

On 02/23/2011 05:39 AM, Maciej W. Rozycki wrote:
...
>
>   [Catching up with old e-mail...]
>
>   In line with the comment above that you're removing -- have you (or
> anyone else) adjusted code elsewhere so that external NMIs are actually
> delivered to processors other than the BSP?  I can't see such code in this
> series nor an explanation as to why it wouldn't be needed.
>
>   For the record -- the piece of code above reflects our setup where the
> LINT1 input is enabled and configured for the NMI delivery mode on the BSP
> only and all the other processors have this line disabled in their local
> APIC units.  If system NMIs are to be handled after the removal of the
> BSP, then another processor has to be selected and configured for NMI
> reception.  Alternatively, all local units could have their LINT1 input
> enabled and arbitrate handling, although it would be quite disruptive as
> all the processors would take the interrupt if it happened.  OTOH it would
> be more fault-tolerant in the case of a CPU failure.  On a typical x86 box
> the system NMI cannot be routed to an I/O APIC input.
>
>    Maciej

   Hi Maciej, good catch! The code doesn't reconfig LVT. As just Don pointed
it might be Intel is working on something, dunno. Probably we better should
drop this patch for now (at least until LVT reconfig would not be implemented).

-- 
     Cyrill

Re: [PATCH 5/6] x86, NMI: Allow NMI reason io port (0x61) to be processed on any CPU

[huang ying]

Hi,

On Sat, Feb 26, 2011 at 4:02 PM, Cyrill Gorcunov <gorcunov@gmail.com> wrote:
> On 02/23/2011 05:39 AM, Maciej W. Rozycki wrote:
> ...
>>
>>  [Catching up with old e-mail...]
>>
>>  In line with the comment above that you're removing -- have you (or
>> anyone else) adjusted code elsewhere so that external NMIs are actually
>> delivered to processors other than the BSP?  I can't see such code in this
>> series nor an explanation as to why it wouldn't be needed.
>>
>>  For the record -- the piece of code above reflects our setup where the
>> LINT1 input is enabled and configured for the NMI delivery mode on the BSP
>> only and all the other processors have this line disabled in their local
>> APIC units.  If system NMIs are to be handled after the removal of the
>> BSP, then another processor has to be selected and configured for NMI
>> reception.  Alternatively, all local units could have their LINT1 input
>> enabled and arbitrate handling, although it would be quite disruptive as
>> all the processors would take the interrupt if it happened.  OTOH it would
>> be more fault-tolerant in the case of a CPU failure.  On a typical x86 box
>> the system NMI cannot be routed to an I/O APIC input.
>>
>>   Maciej
>
>  Hi Maciej, good catch! The code doesn't reconfig LVT. As just Don pointed
> it might be Intel is working on something, dunno. Probably we better should
> drop this patch for now (at least until LVT reconfig would not be
> implemented).

Why?  Without LVT reconfig, system with this patch can not work
properly?  This is just one of the steps to make CPU 0 hot-removable.
We must enable CPU 0 hot-removing in one step?

Best Regards,
Huang Ying

Re: [PATCH 5/6] x86, NMI: Allow NMI reason io port (0x61) to be processed on any CPU

[Cyrill Gorcunov]

On 02/26/2011 02:19 PM, huang ying wrote:
> Hi,
>
> On Sat, Feb 26, 2011 at 4:02 PM, Cyrill Gorcunov<gorcunov@gmail.com>  wrote:
>> On 02/23/2011 05:39 AM, Maciej W. Rozycki wrote:
>> ...
>>>
>>>   [Catching up with old e-mail...]
>>>
>>>   In line with the comment above that you're removing -- have you (or
>>> anyone else) adjusted code elsewhere so that external NMIs are actually
>>> delivered to processors other than the BSP?  I can't see such code in this
>>> series nor an explanation as to why it wouldn't be needed.
>>>
>>>   For the record -- the piece of code above reflects our setup where the
>>> LINT1 input is enabled and configured for the NMI delivery mode on the BSP
>>> only and all the other processors have this line disabled in their local
>>> APIC units.  If system NMIs are to be handled after the removal of the
>>> BSP, then another processor has to be selected and configured for NMI
>>> reception.  Alternatively, all local units could have their LINT1 input
>>> enabled and arbitrate handling, although it would be quite disruptive as
>>> all the processors would take the interrupt if it happened.  OTOH it would
>>> be more fault-tolerant in the case of a CPU failure.  On a typical x86 box
>>> the system NMI cannot be routed to an I/O APIC input.
>>>
>>>    Maciej
>>
>>   Hi Maciej, good catch! The code doesn't reconfig LVT. As just Don pointed
>> it might be Intel is working on something, dunno. Probably we better should
>> drop this patch for now (at least until LVT reconfig would not be
>> implemented).
>

   Hi Huang,

> Why?  Without LVT reconfig, system with this patch can not work
> properly?

   I guess we have a few nits here -- first an important comment were
removed which doesn't reflect what happens on hw level for real. At
least we should put it back just to not confuse people who read this
code, something like

	/*
	 * FIXME: Only BSP can see external NMI for now and hot-unplug
	 * for BSP is not yet implemented
	 */
	WARN_ON_ONCE(smp_processor_id());

   The reason for WARN_ON_ONCE here is that -- imagine the situation when
perf-nmi happens on one cpu with external nmi on BSP and for some reason
(say code on upper level is screwed\bogus or anything else) nmi-notifier
didn't handled it properly as result we might have a report like "SERR for
reason xx on CPU 1" while this cpu didn't see this signal at all. And then
due to locking ordering BSP will see unknown nmi while in real those nmi belongs
him and it was CPU 1 who observed erronious NMI from upper level. Note this
is theoretical scenario I never saw anything like this ;)

   And since LVT reconfig might not be that simple as we might imagine I think
having additional lock in nmi handling code is not good at all.

> This is just one of the steps to make CPU 0 hot-removable.
> We must enable CPU 0 hot-removing in one step?

   Not of course but as I said having additional lock here for free
is not that good until we have a serious reason for it.

   Though, I would be glad if I'm wrong in my conclusions ;)

-- 
     Cyrill

Re: [PATCH 5/6] x86, NMI: Allow NMI reason io port (0x61) to be processed on any CPU

[huang ying]

On Sat, Feb 26, 2011 at 8:34 PM, Cyrill Gorcunov <gorcunov@gmail.com> wrote:
[snip]
>> Why?  Without LVT reconfig, system with this patch can not work
>> properly?
>
>  I guess we have a few nits here -- first an important comment were
> removed which doesn't reflect what happens on hw level for real. At
> least we should put it back just to not confuse people who read this
> code, something like
>
>        /*
>         * FIXME: Only BSP can see external NMI for now and hot-unplug
>         * for BSP is not yet implemented
>         */
>        WARN_ON_ONCE(smp_processor_id());
>
>  The reason for WARN_ON_ONCE here is that -- imagine the situation when
> perf-nmi happens on one cpu with external nmi on BSP and for some reason
> (say code on upper level is screwed\bogus or anything else) nmi-notifier
> didn't handled it properly as result we might have a report like "SERR for
> reason xx on CPU 1" while this cpu didn't see this signal at all. And then
> due to locking ordering BSP will see unknown nmi while in real those nmi
> belongs
> him and it was CPU 1 who observed erronious NMI from upper level. Note this
> is theoretical scenario I never saw anything like this ;)

Yes.  That is possible, at least in theory.  But similar issue is
possible for original code too.  For example, On CPU 0,

1. perf NMI 1 triggered
2. NMI handler enter
3. perf NMI 2 triggered (1 NMI is pending)
4. perf NMI handler handled 2 events
5. NMI handler return
6. NMI handler enter (because of pending NMI)
7. external NMI triggered (another NMI is pending)
8. external NMI handler handled SERR
9. NMI handler return
10. NMI handler enter (because of pending NMI)
11. unknown NMI triggered

If my analysis is correct, this kind of issue can not be resolved even
if we revert to original code.

Best Regards,
Huang Ying

Re: [PATCH 5/6] x86, NMI: Allow NMI reason io port (0x61) to be processed on any CPU

[Cyrill Gorcunov]

On 02/26/2011 05:07 PM, huang ying wrote:
> On Sat, Feb 26, 2011 at 8:34 PM, Cyrill Gorcunov<gorcunov@gmail.com>  wrote:
> [snip]
>>> Why?  Without LVT reconfig, system with this patch can not work
>>> properly?
>>
>>   I guess we have a few nits here -- first an important comment were
>> removed which doesn't reflect what happens on hw level for real. At
>> least we should put it back just to not confuse people who read this
>> code, something like
>>
>>         /*
>>          * FIXME: Only BSP can see external NMI for now and hot-unplug
>>          * for BSP is not yet implemented
>>          */
>>         WARN_ON_ONCE(smp_processor_id());
>>
>>   The reason for WARN_ON_ONCE here is that -- imagine the situation when
>> perf-nmi happens on one cpu with external nmi on BSP and for some reason
>> (say code on upper level is screwed\bogus or anything else) nmi-notifier
>> didn't handled it properly as result we might have a report like "SERR for
>> reason xx on CPU 1" while this cpu didn't see this signal at all. And then
>> due to locking ordering BSP will see unknown nmi while in real those nmi
>> belongs
>> him and it was CPU 1 who observed erronious NMI from upper level. Note this
>> is theoretical scenario I never saw anything like this ;)
>
> Yes.  That is possible, at least in theory.  But similar issue is
> possible for original code too.  For example, On CPU 0,
>
> 1. perf NMI 1 triggered
> 2. NMI handler enter
> 3. perf NMI 2 triggered (1 NMI is pending)
> 4. perf NMI handler handled 2 events
> 5. NMI handler return
> 6. NMI handler enter (because of pending NMI)
> 7. external NMI triggered (another NMI is pending)
> 8. external NMI handler handled SERR
> 9. NMI handler return
> 10. NMI handler enter (because of pending NMI)
> 11. unknown NMI triggered
>
> If my analysis is correct, this kind of issue can not be resolved even
> if we revert to original code.
>
> Best Regards,
> Huang Ying

   Of course there is a way to hit unknown nmi if upper level is screwed (we may see this with p4
pmu on ht machine+kgdb which I didn't manage to fix yet) but with the former code an external nmi would
not ever be handled by cpu which apic is not configured as a listener regardless anything. Ie there was 1:1
mapping between extnmi observer and handler.

   Probably we should put question in another fashion, ie in the fasion of overall design -- who should be
responsible for handling external nmis, 1) the cpu which apic is configured to observe such nmis or 2) any cpu?
If we take 1) then no lock is needed and underlied code will report real cpu number who observed nmi. If
we take 2) then lock is needed but we need a big comment in default_do_nmi together with probably cpu number
fixed in serr\iochk printk's.

-- 
     Cyrill

Re: [PATCH 5/6] x86, NMI: Allow NMI reason io port (0x61) to be processed on any CPU

[huang ying]

On Sat, Feb 26, 2011 at 11:09 PM, Cyrill Gorcunov <gorcunov@gmail.com> wrote:
> On 02/26/2011 05:07 PM, huang ying wrote:
>>
>> On Sat, Feb 26, 2011 at 8:34 PM, Cyrill Gorcunov<gorcunov@gmail.com>
>>  wrote:
>> [snip]
>>>>
>>>> Why?  Without LVT reconfig, system with this patch can not work
>>>> properly?
>>>
>>>  I guess we have a few nits here -- first an important comment were
>>> removed which doesn't reflect what happens on hw level for real. At
>>> least we should put it back just to not confuse people who read this
>>> code, something like
>>>
>>>        /*
>>>         * FIXME: Only BSP can see external NMI for now and hot-unplug
>>>         * for BSP is not yet implemented
>>>         */
>>>        WARN_ON_ONCE(smp_processor_id());
>>>
>>>  The reason for WARN_ON_ONCE here is that -- imagine the situation when
>>> perf-nmi happens on one cpu with external nmi on BSP and for some reason
>>> (say code on upper level is screwed\bogus or anything else) nmi-notifier
>>> didn't handled it properly as result we might have a report like "SERR
>>> for
>>> reason xx on CPU 1" while this cpu didn't see this signal at all. And
>>> then
>>> due to locking ordering BSP will see unknown nmi while in real those nmi
>>> belongs
>>> him and it was CPU 1 who observed erronious NMI from upper level. Note
>>> this
>>> is theoretical scenario I never saw anything like this ;)
>>
>> Yes.  That is possible, at least in theory.  But similar issue is
>> possible for original code too.  For example, On CPU 0,
>>
>> 1. perf NMI 1 triggered
>> 2. NMI handler enter
>> 3. perf NMI 2 triggered (1 NMI is pending)
>> 4. perf NMI handler handled 2 events
>> 5. NMI handler return
>> 6. NMI handler enter (because of pending NMI)
>> 7. external NMI triggered (another NMI is pending)
>> 8. external NMI handler handled SERR
>> 9. NMI handler return
>> 10. NMI handler enter (because of pending NMI)
>> 11. unknown NMI triggered
>>
>> If my analysis is correct, this kind of issue can not be resolved even
>> if we revert to original code.
>>
>> Best Regards,
>> Huang Ying
>
>  Of course there is a way to hit unknown nmi if upper level is screwed (we
> may see this with p4
> pmu on ht machine+kgdb which I didn't manage to fix yet) but with the former
> code an external nmi would
> not ever be handled by cpu which apic is not configured as a listener
> regardless anything. Ie there was 1:1
> mapping between extnmi observer and handler.
>
>  Probably we should put question in another fashion, ie in the fasion of
> overall design -- who should be
> responsible for handling external nmis, 1) the cpu which apic is configured
> to observe such nmis or 2) any cpu?
> If we take 1) then no lock is needed and underlied code will report real cpu
> number who observed nmi. If
> we take 2) then lock is needed but we need a big comment in default_do_nmi
> together with probably cpu number
> fixed in serr\iochk printk's.

I am OK with both solutions.

Best Regards,
Huang Ying

Re: [PATCH 5/6] x86, NMI: Allow NMI reason io port (0x61) to be processed on any CPU

[Cyrill Gorcunov]

On 02/27/2011 04:01 AM, huang ying wrote:
...
>>
>>   Probably we should put question in another fashion, ie in the fasion of
>> overall design -- who should be
>> responsible for handling external nmis, 1) the cpu which apic is configured
>> to observe such nmis or 2) any cpu?
>> If we take 1) then no lock is needed and underlied code will report real cpu
>> number who observed nmi. If
>> we take 2) then lock is needed but we need a big comment in default_do_nmi
>> together with probably cpu number
>> fixed in serr\iochk printk's.
>
> I am OK with both solutions.
>
> Best Regards,
> Huang Ying

ok, lets see what others think on this thread

-- 
     Cyrill

Re: [PATCH 5/6] x86, NMI: Allow NMI reason io port (0x61) to be processed on any CPU

[Don Zickus]

On Sun, Feb 27, 2011 at 02:19:11PM +0300, Cyrill Gorcunov wrote:
> On 02/27/2011 04:01 AM, huang ying wrote:
> ...
> >>
> >>  Probably we should put question in another fashion, ie in the fasion of
> >>overall design -- who should be
> >>responsible for handling external nmis, 1) the cpu which apic is configured
> >>to observe such nmis or 2) any cpu?
> >>If we take 1) then no lock is needed and underlied code will report real cpu
> >>number who observed nmi. If
> >>we take 2) then lock is needed but we need a big comment in default_do_nmi
> >>together with probably cpu number
> >>fixed in serr\iochk printk's.
> >
> >I am OK with both solutions.
> >
> >Best Regards,
> >Huang Ying
> 
> ok, lets see what others think on this thread

I'm trying to figure out how this affects SGI's systems which currently
enable external NMIs to all cpu's in order to support their nmi button to
dump cpu stacks on a system hang
(arch/x86/kernel/apic/x2apic_uv_x.c::uv_nmi_init)

But feel free to post patches addressing your concerns as I am getting a
little lost in the all the concerns being thrown back and forth.

Cheers,
Don

Re: [PATCH 5/6] x86, NMI: Allow NMI reason io port (0x61) to be processed on any CPU

[Cyrill Gorcunov]

On 02/28/2011 09:37 PM, Don Zickus wrote:
> On Sun, Feb 27, 2011 at 02:19:11PM +0300, Cyrill Gorcunov wrote:
>> On 02/27/2011 04:01 AM, huang ying wrote:
>> ...
>>>>
>>>>   Probably we should put question in another fashion, ie in the fasion of
>>>> overall design -- who should be
>>>> responsible for handling external nmis, 1) the cpu which apic is configured
>>>> to observe such nmis or 2) any cpu?
>>>> If we take 1) then no lock is needed and underlied code will report real cpu
>>>> number who observed nmi. If
>>>> we take 2) then lock is needed but we need a big comment in default_do_nmi
>>>> together with probably cpu number
>>>> fixed in serr\iochk printk's.
>>>
>>> I am OK with both solutions.
>>>
>>> Best Regards,
>>> Huang Ying
>>
>> ok, lets see what others think on this thread
>
> I'm trying to figure out how this affects SGI's systems which currently
> enable external NMIs to all cpu's in order to support their nmi button to
> dump cpu stacks on a system hang
> (arch/x86/kernel/apic/x2apic_uv_x.c::uv_nmi_init)
>
> But feel free to post patches addressing your concerns as I am getting a
> little lost in the all the concerns being thrown back and forth.
>
> Cheers,
> Don

   I was planning to do so today but seems out of time at moment (thought I will
try ;), in particular I thought about dropping lock for a while and restore old
behaviour. *BUT* same time to put some big comment explaining why we do this
(so that Ying's work would not be wasted but rather deffered until proper apic
  reconfig implemented).

-- 
     Cyrill