From 09ee10143658cd021d879ead61ead72a196302b6 Mon Sep 17 00:00:00 2001 From: Don Zickus Date: Thu, 29 Mar 2012 16:11:15 -0400 Subject: watchdog, hpwdt: Remove priority option for NMI callback The NMI_UNKNOWN bucket only allows for one function to register to it. The reason for that is because only functions which can not determine if the NMI belongs to them or not should register and would like to assume/swallow any NMI they see. As a result it doesn't make sense to let more than one function like this register. In fact, letting a second function fail allows us to know that more than one function is going to swallow NMIs on the current system. This is better than silently being ignored. Therefore hpwdt's priority mechanism doesn't make sense any more. They will be always first on the NMI_UNKNOWN queue, if they register. Removing this parameter cleans up the code and simplifies things for the next patch which changes how nmis are registered. Signed-off-by: Don Zickus Cc: Thomas Mingarelli Cc: Wim Van Sebroeck Cc: Peter Zijlstra Cc: Linus Torvalds Cc: Andrew Morton Link: http://lkml.kernel.org/r/1333051877-15755-2-git-send-email-dzickus@redhat.com Signed-off-by: Ingo Molnar --- drivers/watchdog/hpwdt.c | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) (limited to 'drivers/watchdog') diff --git a/drivers/watchdog/hpwdt.c b/drivers/watchdog/hpwdt.c index cbc7ceef2786..4000b8038cac 100644 --- a/drivers/watchdog/hpwdt.c +++ b/drivers/watchdog/hpwdt.c @@ -147,7 +147,6 @@ struct cmn_registers { static unsigned int hpwdt_nmi_decoding; static unsigned int allow_kdump; -static unsigned int priority; /* hpwdt at end of die_notify list */ static unsigned int is_icru; static DEFINE_SPINLOCK(rom_lock); static void *cru_rom_addr; @@ -723,13 +722,9 @@ static int __devinit hpwdt_init_nmi_decoding(struct pci_dev *dev) } /* - * If the priority is set to 1, then we will be put first on the - * die notify list to handle a critical NMI. The default is to - * be last so other users of the NMI signal can function. + * Only one function can register for NMI_UNKNOWN */ - retval = register_nmi_handler(NMI_UNKNOWN, hpwdt_pretimeout, - (priority) ? NMI_FLAG_FIRST : 0, - "hpwdt"); + retval = register_nmi_handler(NMI_UNKNOWN, hpwdt_pretimeout, 0, "hpwdt"); if (retval != 0) { dev_warn(&dev->dev, "Unable to register a die notifier (err=%d).\n", @@ -740,10 +735,8 @@ static int __devinit hpwdt_init_nmi_decoding(struct pci_dev *dev) dev_info(&dev->dev, "HP Watchdog Timer Driver: NMI decoding initialized" - ", allow kernel dump: %s (default = 0/OFF)" - ", priority: %s (default = 0/LAST).\n", - (allow_kdump == 0) ? "OFF" : "ON", - (priority == 0) ? "LAST" : "FIRST"); + ", allow kernel dump: %s (default = 0/OFF)\n", + (allow_kdump == 0) ? "OFF" : "ON"); return 0; } @@ -881,10 +874,6 @@ MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default=" #ifdef CONFIG_HPWDT_NMI_DECODING module_param(allow_kdump, int, 0); MODULE_PARM_DESC(allow_kdump, "Start a kernel dump after NMI occurs"); - -module_param(priority, int, 0); -MODULE_PARM_DESC(priority, "The hpwdt driver handles NMIs first or last" - " (default = 0/Last)\n"); #endif /* !CONFIG_HPWDT_NMI_DECODING */ module_init(hpwdt_init); -- cgit v1.2.3 From 553222f3e81f18da31b2552e18dc519715198590 Mon Sep 17 00:00:00 2001 From: Don Zickus Date: Thu, 29 Mar 2012 16:11:16 -0400 Subject: x86/nmi: Add new NMI queues to deal with IO_CHK and SERR In discussions with Thomas Mingarelli about hpwdt, he explained to me some issues they were some when using their virtual NMI button to test the hpwdt driver. It turns out the virtual NMI button used on HP's machines do no send unknown NMIs but instead send IO_CHK NMIs. The way the kernel code is written, the hpwdt driver can not register itself against that type of NMI and therefore can not successfully capture system information before panic'ing. To solve this I created two new NMI queues to allow driver to register against the IO_CHK and SERR NMIs. Or in the hpwdt all three (if you include unknown NMIs too). The change is straightforward and just mimics what the unknown NMI does. Reported-and-tested-by: Thomas Mingarelli Signed-off-by: Don Zickus Cc: Peter Zijlstra Cc: Linus Torvalds Cc: Andrew Morton Link: http://lkml.kernel.org/r/1333051877-15755-3-git-send-email-dzickus@redhat.com Signed-off-by: Ingo Molnar --- arch/x86/include/asm/nmi.h | 2 ++ arch/x86/kernel/nmi.c | 18 ++++++++++++++++++ drivers/watchdog/hpwdt.c | 27 ++++++++++++++++++++------- 3 files changed, 40 insertions(+), 7 deletions(-) (limited to 'drivers/watchdog') diff --git a/arch/x86/include/asm/nmi.h b/arch/x86/include/asm/nmi.h index fd3f9f18cf3f..07162dfbff84 100644 --- a/arch/x86/include/asm/nmi.h +++ b/arch/x86/include/asm/nmi.h @@ -27,6 +27,8 @@ void arch_trigger_all_cpu_backtrace(void); enum { NMI_LOCAL=0, NMI_UNKNOWN, + NMI_SERR, + NMI_IO_CHECK, NMI_MAX }; diff --git a/arch/x86/kernel/nmi.c b/arch/x86/kernel/nmi.c index 47acaf319165..ac9c1b76df96 100644 --- a/arch/x86/kernel/nmi.c +++ b/arch/x86/kernel/nmi.c @@ -54,6 +54,14 @@ static struct nmi_desc nmi_desc[NMI_MAX] = .lock = __SPIN_LOCK_UNLOCKED(&nmi_desc[1].lock), .head = LIST_HEAD_INIT(nmi_desc[1].head), }, + { + .lock = __SPIN_LOCK_UNLOCKED(&nmi_desc[2].lock), + .head = LIST_HEAD_INIT(nmi_desc[2].head), + }, + { + .lock = __SPIN_LOCK_UNLOCKED(&nmi_desc[3].lock), + .head = LIST_HEAD_INIT(nmi_desc[3].head), + }, }; @@ -120,6 +128,8 @@ static int __setup_nmi(unsigned int type, struct nmiaction *action) * to manage expectations */ WARN_ON_ONCE(type == NMI_UNKNOWN && !list_empty(&desc->head)); + WARN_ON_ONCE(type == NMI_SERR && !list_empty(&desc->head)); + WARN_ON_ONCE(type == NMI_IO_CHECK && !list_empty(&desc->head)); /* * some handlers need to be executed first otherwise a fake @@ -212,6 +222,10 @@ EXPORT_SYMBOL_GPL(unregister_nmi_handler); static notrace __kprobes void pci_serr_error(unsigned char reason, struct pt_regs *regs) { + /* check to see if anyone registered against these types of errors */ + if (nmi_handle(NMI_SERR, regs, false)) + return; + pr_emerg("NMI: PCI system error (SERR) for reason %02x on CPU %d.\n", reason, smp_processor_id()); @@ -241,6 +255,10 @@ io_check_error(unsigned char reason, struct pt_regs *regs) { unsigned long i; + /* check to see if anyone registered against these types of errors */ + if (nmi_handle(NMI_IO_CHECK, regs, false)) + return; + pr_emerg( "NMI: IOCK error (debug interrupt?) for reason %02x on CPU %d.\n", reason, smp_processor_id()); diff --git a/drivers/watchdog/hpwdt.c b/drivers/watchdog/hpwdt.c index 4000b8038cac..6e414b501d58 100644 --- a/drivers/watchdog/hpwdt.c +++ b/drivers/watchdog/hpwdt.c @@ -725,19 +725,32 @@ static int __devinit hpwdt_init_nmi_decoding(struct pci_dev *dev) * Only one function can register for NMI_UNKNOWN */ retval = register_nmi_handler(NMI_UNKNOWN, hpwdt_pretimeout, 0, "hpwdt"); - if (retval != 0) { - dev_warn(&dev->dev, - "Unable to register a die notifier (err=%d).\n", - retval); - if (cru_rom_addr) - iounmap(cru_rom_addr); - } + if (retval) + goto error; + retval = register_nmi_handler(NMI_SERR, hpwdt_pretimeout, 0, "hpwdt"); + if (retval) + goto error1; + retval = register_nmi_handler(NMI_IO_CHECK, hpwdt_pretimeout, 0, "hpwdt"); + if (retval) + goto error2; dev_info(&dev->dev, "HP Watchdog Timer Driver: NMI decoding initialized" ", allow kernel dump: %s (default = 0/OFF)\n", (allow_kdump == 0) ? "OFF" : "ON"); return 0; + +error2: + unregister_nmi_handler(NMI_SERR, "hpwdt"); +error1: + unregister_nmi_handler(NMI_UNKNOWN, "hpwdt"); +error: + dev_warn(&dev->dev, + "Unable to register a die notifier (err=%d).\n", + retval); + if (cru_rom_addr) + iounmap(cru_rom_addr); + return retval; } static void hpwdt_exit_nmi_decoding(void) -- cgit v1.2.3