On 26.05.2014 15:45, Borislav Petkov wrote:
On Mon, May 26, 2014 at 03:26:06PM +0200, Tomasz Nowicki wrote:
Now I do follow :) Nicely done, I have applied your patch and indeed
there are more arch dependencies for !X86.
Not nicely enough, I guess :-)
diff --git a/arch/x86/include/asm/nmi.h b/arch/x86/include/asm/nmi.h
index 86f9301..0f03ab6 100644
--- a/arch/x86/include/asm/nmi.h
+++ b/arch/x86/include/asm/nmi.h
@@ -54,7 +54,18 @@ struct nmiaction {
int __register_nmi_handler(unsigned int, struct nmiaction *);
-void unregister_nmi_handler(unsigned int, const char *);
+void unregister_nmi_handler(unsigned int type, const char *name);
+
+static inline int arch_apei_register_nmi(nmi_handler_t fn,
+ const char *name)
+{
+ return register_nmi_handler(NMI_LOCAL, fn, 0, name);
+}
+
+static inline void arch_apei_unregister_nmi(const char *name)
+{
+ unregister_nmi_handler(NMI_LOCAL, name);
+}
I'm guessing you've added those wrappers so that you don't have to
export NMI_LOCAL?
Yep.
void stop_nmi(void);
void restart_nmi(void);
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index 35a44d9..84c79af 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -47,13 +47,11 @@
#include <linux/genalloc.h>
#include <linux/pci.h>
#include <linux/aer.h>
+#include <linux/nmi.h>
#include <acpi/ghes.h>
#include <asm/apei.h>
#include <asm/tlbflush.h>
-#ifdef CONFIG_ACPI_APEI_NMI
-#include <asm/nmi.h>
-#endif
#include "apei-internal.h"
@@ -718,7 +716,6 @@ static int ghes_notify_sci(struct notifier_block *this,
return ret;
}
-#ifdef CONFIG_ACPI_APEI_NMI
/*
* printk is not safe in NMI context. So in NMI handler, we allocate
* required memory from lock-less memory allocator
@@ -817,7 +814,7 @@ static int ghes_notify_nmi(unsigned int cmd, struct
pt_regs *regs)
{
struct ghes *ghes, *ghes_global = NULL;
int sev, sev_global = -1;
- int ret = NMI_DONE;
+ int ret = APEI_NMI_DONE;
BUG_ON(!IS_ENABLED(CONFIG_ACPI_APEI_NMI));
@@ -832,14 +829,14 @@ static int ghes_notify_nmi(unsigned int cmd, struct
pt_regs *regs)
sev_global = sev;
ghes_global = ghes;
}
- ret = NMI_HANDLED;
+ ret = APEI_NMI_HANDLED;
}
- if (ret == NMI_DONE)
+ if (ret == APEI_NMI_DONE)
goto out;
if (sev_global >= GHES_SEV_PANIC) {
- oops_begin();
+ arch_apei_nmi_oops_begin();
ghes_print_queued_estatus();
__ghes_print_estatus(KERN_EMERG, ghes_global->generic,
ghes_global->estatus);
@@ -914,7 +911,7 @@ static int ghes_notify_init_nmi(struct ghes *ghes)
ghes_estatus_pool_expand(len);
mutex_lock(&ghes_list_mutex);
if (list_empty(&ghes_nmi))
- status = register_nmi_handler(NMI_LOCAL, ghes_notify_nmi, 0, "ghes");
+ status = arch_apei_register_nmi(ghes_notify_nmi, "ghes");
list_add_rcu(&ghes->list, &ghes_nmi);
mutex_unlock(&ghes_list_mutex);
@@ -928,7 +925,7 @@ static void ghes_notify_remove_nmi(struct ghes *ghes)
mutex_lock(&ghes_list_mutex);
list_del_rcu(&ghes->list);
if (list_empty(&ghes_nmi))
- unregister_nmi_handler(NMI_LOCAL, "ghes");
+ arch_apei_unregister_nmi("ghes");
mutex_unlock(&ghes_list_mutex);
/*
* To synchronize with NMI handler, ghes can only be
@@ -941,17 +938,14 @@ static void ghes_notify_remove_nmi(struct ghes *ghes)
static void ghes_init_nmi(void)
{
+ if (!IS_ENABLED(CONFIG_ACPI_APEI_NMI))
+ return;
+
init_irq_work(&ghes_proc_irq_work, ghes_proc_in_irq);
ghes_notify_tab[ACPI_HEST_NOTIFY_NMI].init_call = ghes_notify_init_nmi;
ghes_notify_tab[ACPI_HEST_NOTIFY_NMI].remove_call =
ghes_notify_remove_nmi;
}
-#else
-static inline void ghes_init_nmi(void)
-{
-
-}
-#endif
static int ghes_notify_init_polled(struct ghes *ghes)
{
diff --git a/include/linux/nmi.h b/include/linux/nmi.h
index 084b4c5..1aa351c 100644
--- a/include/linux/nmi.h
+++ b/include/linux/nmi.h
@@ -56,4 +56,19 @@ extern int proc_dowatchdog(struct ctl_table *, int ,
void __user *, size_t *, loff_t *);
#endif
+#define APEI_NMI_DONE 0
+#define APEI_NMI_HANDLED 1
+
+#ifdef CONFIG_ACPI_APEI_NMI
+#include <asm/nmi.h>
+#define arch_apei_nmi_oops_begin() oops_begin()
+#else
+#define arch_apei_register_nmi(fn, n) ({ \
+ void __attribute__((unused)) *dummy = fn; \
Do we really need this dummy assignment? Wouldn't it be just fine to
simply have:
#define arch_apei_register_nmi(fn, n) ({ (-ENOSYS); })
For !X86 I've got compilator warning that ghes_notify_nmi is defined but
not used. I just want to make console a bit cleaner :)
Just a nitpick, though; otherwise, this looks nicely abstracted.
Thanks for your help!
Tomasz
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
