[PATCH v2 0/5] APEI: Make APEI architecture independent.

[PATCH v2 0/5] APEI: Make APEI architecture independent.

[Tomasz Nowicki]

APEI is currently implemented so that it depends on x86 hardware.
The primary dependency is that GHES uses the x86 NMI for hardware
error notification and MCE for memory error handling. These patches
remove that dependency.

Other APEI features such as error reporting via external IRQ, error
serialization, or error injection, do not require changes to use them
on non-x86 architectures.

The following patch set eliminates the APEI Kconfig x86 dependency
by making these changes:
- treat NMI notification as GHES feature - CONFIG_ACPI_APEI_NMI
- reorganize GHES notification type init/deinitialization
  so that we can decide in run time whether NMI is supported or not
- identify architectural boxes and abstract it accordingly (NMI and MCE)
- rework ioremap for both IRQ and NMI context

NMI code is kept in ghes.c file since NMI and IRQ context are tightly coupled.

Note, these patches introduce no functional changes for x86. The NMI notification
feature is selected for x86 by default. Architectures that want to use this
feature should also provide NMI code infrastructure.

V1->V2
- address Borislav comment
- abstract arch-specific calls instead of wrapping into the #ifdef

Tomasz Nowicki (5):
  apei, mce: Factor out APEI architecture specific MCE calls.
  acpi, apei, ghes: Introduce ACPI_APEI_NMI to make NMI error
    notification a GHES feature.
  acpi, apei, ghes: Introduce more generic mechanism to init/deinit
    GHES error notifications.
  apei, ghes, nmi: Factor out NMI arch-specific calls.
  acpi, apei, ghes: Factor out ioremap virtual memory for IRQ and NMI
    context.

 arch/x86/kernel/acpi/Makefile |    1 +
 arch/x86/kernel/acpi/apei.c   |   87 ++++++++++++
 drivers/acpi/apei/Kconfig     |   10 +-
 drivers/acpi/apei/apei-base.c |   32 +++++
 drivers/acpi/apei/ghes.c      |  301 +++++++++++++++++++++++++----------------
 drivers/acpi/apei/hest.c      |   26 +---
 include/acpi/apei.h           |   11 ++
 7 files changed, 328 insertions(+), 140 deletions(-)
 create mode 100644 arch/x86/kernel/acpi/apei.c

-- 
1.7.9.5

[PATCH v2 1/5] apei, mce: Factor out APEI architecture specific MCE calls.

[Tomasz Nowicki]

This commit abstracts MCE calls and provides weak corresponding default
implementation for those architectures which do not need arch specific
actions. Each platform willing to do additional architectural actions
should provides desired function definition. It allows us to avoid wrap
code into #ifdef in generic code and prevent new platform from introducing
dummy stub function too.

Initially, there are two APEI arch-specific calls:
- apei_arch_enable_cmcff()
- apei_arch_report_mem_error()
Both interact with MCE driver for X86 architecture.

Signed-off-by: Tomasz Nowicki <tomasz.nowicki@linaro.org>
---
 arch/x86/kernel/acpi/Makefile |    1 +
 arch/x86/kernel/acpi/apei.c   |   56 +++++++++++++++++++++++++++++++++++++++++
 drivers/acpi/apei/apei-base.c |   13 ++++++++++
 drivers/acpi/apei/ghes.c      |    6 ++---
 drivers/acpi/apei/hest.c      |   26 +------------------
 include/acpi/apei.h           |    3 +++
 6 files changed, 76 insertions(+), 29 deletions(-)
 create mode 100644 arch/x86/kernel/acpi/apei.c

diff --git a/arch/x86/kernel/acpi/Makefile b/arch/x86/kernel/acpi/Makefile
index 163b225..3242e59 100644
--- a/arch/x86/kernel/acpi/Makefile
+++ b/arch/x86/kernel/acpi/Makefile
@@ -1,5 +1,6 @@
 obj-$(CONFIG_ACPI)		+= boot.o
 obj-$(CONFIG_ACPI_SLEEP)	+= sleep.o wakeup_$(BITS).o
+obj-$(CONFIG_ACPI_APEI)		+= apei.o
 
 ifneq ($(CONFIG_ACPI_PROCESSOR),)
 obj-y				+= cstate.o
diff --git a/arch/x86/kernel/acpi/apei.c b/arch/x86/kernel/acpi/apei.c
new file mode 100644
index 0000000..dca2852
--- /dev/null
+++ b/arch/x86/kernel/acpi/apei.c
@@ -0,0 +1,56 @@
+/*
+ * Arch-specific APEI-related functions.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <acpi/apei.h>
+
+#include <asm/mce.h>
+
+int apei_arch_enable_cmcff(struct acpi_hest_header *hest_hdr, void *data)
+{
+#ifdef CONFIG_X86_MCE
+	int i;
+	struct acpi_hest_ia_corrected *cmc;
+	struct acpi_hest_ia_error_bank *mc_bank;
+
+	if (hest_hdr->type != ACPI_HEST_TYPE_IA32_CORRECTED_CHECK)
+		return 0;
+
+	cmc = (struct acpi_hest_ia_corrected *)hest_hdr;
+	if (!cmc->enabled)
+		return 0;
+
+	/*
+	 * We expect HEST to provide a list of MC banks that report errors
+	 * in firmware first mode. Otherwise, return non-zero value to
+	 * indicate that we are done parsing HEST.
+	 */
+	if (!(cmc->flags & ACPI_HEST_FIRMWARE_FIRST) ||
+	    !cmc->num_hardware_banks)
+		return 1;
+
+	pr_info("HEST: Enabling Firmware First mode for corrected errors.\n");
+
+	mc_bank = (struct acpi_hest_ia_error_bank *)(cmc + 1);
+	for (i = 0; i < cmc->num_hardware_banks; i++, mc_bank++)
+		mce_disable_bank(mc_bank->bank_number);
+#endif
+	return 1;
+}
+
+void apei_arch_report_mem_error(int sev, struct cper_sec_mem_err *mem_err)
+{
+#ifdef CONFIG_X86_MCE
+	apei_mce_report_mem_error(sev, mem_err);
+#endif
+}
diff --git a/drivers/acpi/apei/apei-base.c b/drivers/acpi/apei/apei-base.c
index 8678dfe..4a11c1a 100644
--- a/drivers/acpi/apei/apei-base.c
+++ b/drivers/acpi/apei/apei-base.c
@@ -745,6 +745,19 @@ struct dentry *apei_get_debugfs_dir(void)
 }
 EXPORT_SYMBOL_GPL(apei_get_debugfs_dir);
 
+int __weak apei_arch_enable_cmcff(struct acpi_hest_header *hest_hdr,
+				  void *data)
+{
+	return 1;
+}
+EXPORT_SYMBOL_GPL(apei_arch_enable_cmcff);
+
+void __weak apei_arch_report_mem_error(int sev,
+				       struct cper_sec_mem_err *mem_err)
+{
+}
+EXPORT_SYMBOL_GPL(apei_arch_report_mem_error);
+
 int apei_osc_setup(void)
 {
 	static u8 whea_uuid_str[] = "ed855e0c-6c90-47bf-a62a-26de0fc5ad5c";
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index dab7cb7..e7dc5c6 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -49,7 +49,7 @@
 #include <linux/aer.h>
 
 #include <acpi/ghes.h>
-#include <asm/mce.h>
+#include <acpi/apei.h>
 #include <asm/tlbflush.h>
 #include <asm/nmi.h>
 
@@ -455,9 +455,7 @@ static void ghes_do_proc(struct ghes *ghes,
 			mem_err = (struct cper_sec_mem_err *)(gdata+1);
 			ghes_edac_report_mem_error(ghes, sev, mem_err);
 
-#ifdef CONFIG_X86_MCE
-			apei_mce_report_mem_error(sev, mem_err);
-#endif
+			apei_arch_report_mem_error(sev, mem_err);
 			ghes_handle_memory_failure(gdata, sev);
 		}
 #ifdef CONFIG_ACPI_APEI_PCIEAER
diff --git a/drivers/acpi/apei/hest.c b/drivers/acpi/apei/hest.c
index f5e37f3..9084767 100644
--- a/drivers/acpi/apei/hest.c
+++ b/drivers/acpi/apei/hest.c
@@ -36,7 +36,6 @@
 #include <linux/io.h>
 #include <linux/platform_device.h>
 #include <acpi/apei.h>
-#include <asm/mce.h>
 
 #include "apei-internal.h"
 
@@ -128,32 +127,9 @@ EXPORT_SYMBOL_GPL(apei_hest_parse);
  */
 static int __init hest_parse_cmc(struct acpi_hest_header *hest_hdr, void *data)
 {
-#ifdef CONFIG_X86_MCE
-	int i;
-	struct acpi_hest_ia_corrected *cmc;
-	struct acpi_hest_ia_error_bank *mc_bank;
-
-	if (hest_hdr->type != ACPI_HEST_TYPE_IA32_CORRECTED_CHECK)
-		return 0;
-
-	cmc = (struct acpi_hest_ia_corrected *)hest_hdr;
-	if (!cmc->enabled)
+	if (!apei_arch_enable_cmcff(hest_hdr, data))
 		return 0;
 
-	/*
-	 * We expect HEST to provide a list of MC banks that report errors
-	 * in firmware first mode. Otherwise, return non-zero value to
-	 * indicate that we are done parsing HEST.
-	 */
-	if (!(cmc->flags & ACPI_HEST_FIRMWARE_FIRST) || !cmc->num_hardware_banks)
-		return 1;
-
-	pr_info(HEST_PFX "Enabling Firmware First mode for corrected errors.\n");
-
-	mc_bank = (struct acpi_hest_ia_error_bank *)(cmc + 1);
-	for (i = 0; i < cmc->num_hardware_banks; i++, mc_bank++)
-		mce_disable_bank(mc_bank->bank_number);
-#endif
 	return 1;
 }
 
diff --git a/include/acpi/apei.h b/include/acpi/apei.h
index 04f349d..62b9d1c 100644
--- a/include/acpi/apei.h
+++ b/include/acpi/apei.h
@@ -42,5 +42,8 @@ ssize_t erst_read(u64 record_id, struct cper_record_header *record,
 		  size_t buflen);
 int erst_clear(u64 record_id);
 
+int apei_arch_enable_cmcff(struct acpi_hest_header *hest_hdr, void *data);
+void apei_arch_report_mem_error(int sev, struct cper_sec_mem_err *mem_err);
+
 #endif
 #endif
-- 
1.7.9.5

[PATCH v2 2/5] acpi, apei, ghes: Introduce ACPI_APEI_NMI to make NMI error notification a GHES feature.

[Tomasz Nowicki]

Currently APEI depends on x86 architecture. It is because of NMI hardware
error notification of GHES which is currently supported by x86 only.
However, many other APEI features can be still used perfectly by other
architectures.

This commit adds ACPI_APEI_NMI which will be used in next patches to isolate
NMI related code in ghes.c file. Only NMI error notification feature
depends on x86 and it is selected by default for x86 arch.

Signed-off-by: Tomasz Nowicki <tomasz.nowicki@linaro.org>
---
 drivers/acpi/apei/Kconfig |   10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/apei/Kconfig b/drivers/acpi/apei/Kconfig
index c4dac71..4b734a2 100644
--- a/drivers/acpi/apei/Kconfig
+++ b/drivers/acpi/apei/Kconfig
@@ -3,7 +3,6 @@ config ACPI_APEI
 	select MISC_FILESYSTEMS
 	select PSTORE
 	select UEFI_CPER
-	depends on X86
 	help
 	  APEI allows to report errors (for example from the chipset)
 	  to the operating system. This improves NMI handling
@@ -26,6 +25,15 @@ config ACPI_APEI_GHES
 	  by firmware to produce more valuable hardware error
 	  information for Linux.
 
+config ACPI_APEI_NMI
+	bool "NMI error notification support"
+	default y
+	depends on ACPI_APEI_GHES && X86
+	help
+	  Firmware first mode can use NMI notification mechanism to report errors
+	  to operating system. This feature is currently supported by X86
+	  architecture only.
+
 config ACPI_APEI_PCIEAER
 	bool "APEI PCIe AER logging/recovering support"
 	depends on ACPI_APEI && PCIEAER
-- 
1.7.9.5

[PATCH v2 3/5] acpi, apei, ghes: Introduce more generic mechanism to init/deinit GHES error notifications.

[Tomasz Nowicki]

Init/deinit of GHES error notifications are moved to corresponding
functions e.g. for SCI ghes_notify_init_sci{sci} and ghes_notify_remove_{sci}
which in turn are gathered to ghes_notify_tab table.
ghes_probe and ghes_remove check notification support based on
function reference pointer in ghes_notify_tab and call it with ghes argument.

This approach allows us to change address of a given error notification
init/deinit function call in ghes_notify_tab in runtime. In turn,
we will avoid #ifdef usage in common code for NMI which is currently
supported by x86.

Signed-off-by: Tomasz Nowicki <tomasz.nowicki@linaro.org>
---
 drivers/acpi/apei/ghes.c |  271 +++++++++++++++++++++++++++++-----------------
 1 file changed, 174 insertions(+), 97 deletions(-)

diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index e7dc5c6..750b04e 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -146,6 +146,14 @@ static struct irq_work ghes_proc_irq_work;
 struct ghes_estatus_cache *ghes_estatus_caches[GHES_ESTATUS_CACHES_SIZE];
 static atomic_t ghes_estatus_cache_alloced;
 
+struct ghes_notify_setup {
+	const char *notif_name;
+	int (*init_call)(struct ghes *ghes);
+	void (*remove_call)(struct ghes *ghes);
+};
+
+static struct ghes_notify_setup	ghes_notify_tab[];
+
 static int ghes_ioremap_init(void)
 {
 	ghes_ioremap_area = __get_vm_area(PAGE_SIZE * GHES_IOREMAP_PAGES,
@@ -811,6 +819,8 @@ static int ghes_notify_nmi(unsigned int cmd, struct pt_regs *regs)
 	int sev, sev_global = -1;
 	int ret = NMI_DONE;
 
+	BUG_ON(!IS_ENABLED(CONFIG_ACPI_APEI_NMI));
+
 	raw_spin_lock(&ghes_nmi_lock);
 	list_for_each_entry_rcu(ghes, &ghes_nmi, list) {
 		if (ghes_read_estatus(ghes, 1)) {
@@ -875,10 +885,6 @@ out:
 	return ret;
 }
 
-static struct notifier_block ghes_notifier_sci = {
-	.notifier_call = ghes_notify_sci,
-};
-
 static unsigned long ghes_esource_prealloc_size(
 	const struct acpi_hest_generic *generic)
 {
@@ -894,39 +900,169 @@ static unsigned long ghes_esource_prealloc_size(
 	return prealloc_size;
 }
 
+static int ghes_notify_init_nmi(struct ghes *ghes)
+{
+	unsigned long len;
+	int status = 0;
+
+	len = ghes_esource_prealloc_size(ghes->generic);
+	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");
+	list_add_rcu(&ghes->list, &ghes_nmi);
+	mutex_unlock(&ghes_list_mutex);
+
+	return status;
+}
+
+static void ghes_notify_remove_nmi(struct ghes *ghes)
+{
+	unsigned long len;
+
+	mutex_lock(&ghes_list_mutex);
+	list_del_rcu(&ghes->list);
+	if (list_empty(&ghes_nmi))
+		unregister_nmi_handler(NMI_LOCAL, "ghes");
+	mutex_unlock(&ghes_list_mutex);
+	/*
+	 * To synchronize with NMI handler, ghes can only be
+	 * freed after NMI handler finishes.
+	 */
+	synchronize_rcu();
+	len = ghes_esource_prealloc_size(ghes->generic);
+	ghes_estatus_pool_shrink(len);
+}
+
+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;
+}
+
+static int ghes_notify_init_polled(struct ghes *ghes)
+{
+	ghes->timer.function = ghes_poll_func;
+	ghes->timer.data = (unsigned long)ghes;
+	init_timer_deferrable(&ghes->timer);
+	ghes_add_timer(ghes);
+
+	return 0;
+}
+
+static void ghes_notify_remove_polled(struct ghes *ghes)
+{
+	del_timer_sync(&ghes->timer);
+}
+
+static int ghes_notify_init_external(struct ghes *ghes)
+{
+	int rc;
+
+	/* External interrupt vector is GSI */
+	rc = acpi_gsi_to_irq(ghes->generic->notify.vector, &ghes->irq);
+	if (rc) {
+		pr_err(GHES_PFX "Failed to map GSI to IRQ for generic hardware error source: %d\n",
+		       ghes->generic->header.source_id);
+		goto out;
+	}
+
+	rc = request_irq(ghes->irq, ghes_irq_func, 0, "GHES IRQ", ghes);
+	if (rc)
+		pr_err(GHES_PFX "Failed to register IRQ for generic hardware error source: %d\n",
+		       ghes->generic->header.source_id);
+
+out:
+	return rc;
+}
+
+static void ghes_notify_remove_external(struct ghes *ghes)
+{
+	free_irq(ghes->irq, ghes);
+}
+
+static struct notifier_block ghes_notifier_sci = {
+	.notifier_call = ghes_notify_sci,
+};
+
+static int ghes_notify_init_sci(struct ghes *ghes)
+{
+	mutex_lock(&ghes_list_mutex);
+	if (list_empty(&ghes_sci))
+		register_acpi_hed_notifier(&ghes_notifier_sci);
+	list_add_rcu(&ghes->list, &ghes_sci);
+	mutex_unlock(&ghes_list_mutex);
+
+	return 0;
+}
+
+static void ghes_notify_remove_sci(struct ghes *ghes)
+{
+	mutex_lock(&ghes_list_mutex);
+	list_del_rcu(&ghes->list);
+	if (list_empty(&ghes_sci))
+		unregister_acpi_hed_notifier(&ghes_notifier_sci);
+	mutex_unlock(&ghes_list_mutex);
+}
+
+static struct ghes_notify_setup
+	ghes_notify_tab[ACPI_HEST_NOTIFY_RESERVED] = {
+		[ACPI_HEST_NOTIFY_POLLED]   = {"POLLED",
+					       ghes_notify_init_polled,
+					       ghes_notify_remove_polled},
+		[ACPI_HEST_NOTIFY_EXTERNAL] = {"EXT_IRQ",
+					       ghes_notify_init_external,
+					       ghes_notify_remove_external},
+		[ACPI_HEST_NOTIFY_LOCAL]    = {"LOCAL_IRQ", NULL, NULL},
+		[ACPI_HEST_NOTIFY_SCI]      = {"SCI",
+					       ghes_notify_init_sci,
+					       ghes_notify_remove_sci},
+		[ACPI_HEST_NOTIFY_NMI]      = {"NMI", NULL, NULL},
+		[ACPI_HEST_NOTIFY_CMCI]     = {"CMCI", NULL, NULL},
+		[ACPI_HEST_NOTIFY_MCE]      = {"MCE", NULL, NULL},
+};
+
 static int ghes_probe(struct platform_device *ghes_dev)
 {
 	struct acpi_hest_generic *generic;
 	struct ghes *ghes = NULL;
-	unsigned long len;
 	int rc = -EINVAL;
+	int (*ghes_init_call)(struct ghes *ghes);
+	const char *ghes_notif_name;
+	u8 notify_type;
+
 
 	generic = *(struct acpi_hest_generic **)ghes_dev->dev.platform_data;
 	if (!generic->enabled)
 		return -ENODEV;
 
-	switch (generic->notify.type) {
-	case ACPI_HEST_NOTIFY_POLLED:
-	case ACPI_HEST_NOTIFY_EXTERNAL:
-	case ACPI_HEST_NOTIFY_SCI:
-	case ACPI_HEST_NOTIFY_NMI:
-		break;
-	case ACPI_HEST_NOTIFY_LOCAL:
-		pr_warning(GHES_PFX "Generic hardware error source: %d notified via local interrupt is not supported!\n",
-			   generic->header.source_id);
+	notify_type = generic->notify.type;
+	if (notify_type >= ACPI_HEST_NOTIFY_RESERVED) {
+		pr_warn(FW_WARN GHES_PFX "Unknown notification type: %u for generic hardware error source: %d\n",
+			notify_type, generic->header.source_id);
 		goto err;
-	default:
-		pr_warning(FW_WARN GHES_PFX "Unknown notification type: %u for generic hardware error source: %d\n",
-			   generic->notify.type, generic->header.source_id);
+	}
+
+	ghes_init_call = ghes_notify_tab[notify_type].init_call;
+	ghes_notif_name = ghes_notify_tab[notify_type].notif_name;
+	if (!ghes_init_call) {
+		pr_warn(GHES_PFX "Generic hardware error source: %d notified via %s is not supported!\n",
+			generic->header.source_id, ghes_notif_name);
 		goto err;
 	}
 
 	rc = -EIO;
 	if (generic->error_block_length <
 	    sizeof(struct acpi_generic_status)) {
-		pr_warning(FW_BUG GHES_PFX "Invalid error block length: %u for generic hardware error source: %d\n",
-			   generic->error_block_length,
-			   generic->header.source_id);
+		pr_warn(FW_BUG GHES_PFX "Invalid error block length: %u for generic hardware error source: %d\n",
+			generic->error_block_length,
+			generic->header.source_id);
 		goto err;
 	}
 	ghes = ghes_new(generic);
@@ -940,48 +1076,10 @@ static int ghes_probe(struct platform_device *ghes_dev)
 	if (rc < 0)
 		goto err;
 
-	switch (generic->notify.type) {
-	case ACPI_HEST_NOTIFY_POLLED:
-		ghes->timer.function = ghes_poll_func;
-		ghes->timer.data = (unsigned long)ghes;
-		init_timer_deferrable(&ghes->timer);
-		ghes_add_timer(ghes);
-		break;
-	case ACPI_HEST_NOTIFY_EXTERNAL:
-		/* External interrupt vector is GSI */
-		rc = acpi_gsi_to_irq(generic->notify.vector, &ghes->irq);
-		if (rc) {
-			pr_err(GHES_PFX "Failed to map GSI to IRQ for generic hardware error source: %d\n",
-			       generic->header.source_id);
-			goto err_edac_unreg;
-		}
-		rc = request_irq(ghes->irq, ghes_irq_func, 0, "GHES IRQ", ghes);
-		if (rc) {
-			pr_err(GHES_PFX "Failed to register IRQ for generic hardware error source: %d\n",
-			       generic->header.source_id);
-			goto err_edac_unreg;
-		}
-		break;
-	case ACPI_HEST_NOTIFY_SCI:
-		mutex_lock(&ghes_list_mutex);
-		if (list_empty(&ghes_sci))
-			register_acpi_hed_notifier(&ghes_notifier_sci);
-		list_add_rcu(&ghes->list, &ghes_sci);
-		mutex_unlock(&ghes_list_mutex);
-		break;
-	case ACPI_HEST_NOTIFY_NMI:
-		len = ghes_esource_prealloc_size(generic);
-		ghes_estatus_pool_expand(len);
-		mutex_lock(&ghes_list_mutex);
-		if (list_empty(&ghes_nmi))
-			register_nmi_handler(NMI_LOCAL, ghes_notify_nmi, 0,
-						"ghes");
-		list_add_rcu(&ghes->list, &ghes_nmi);
-		mutex_unlock(&ghes_list_mutex);
-		break;
-	default:
-		BUG();
-	}
+	rc = (*ghes_init_call)(ghes);
+	if (rc)
+		goto err_edac_unreg;
+
 	platform_set_drvdata(ghes_dev, ghes);
 
 	return 0;
@@ -999,45 +1097,24 @@ static int ghes_remove(struct platform_device *ghes_dev)
 {
 	struct ghes *ghes;
 	struct acpi_hest_generic *generic;
-	unsigned long len;
+	void (*ghes_remove_call)(struct ghes *ghes);
+	u8 notify_type;
 
 	ghes = platform_get_drvdata(ghes_dev);
-	generic = ghes->generic;
-
 	ghes->flags |= GHES_EXITING;
-	switch (generic->notify.type) {
-	case ACPI_HEST_NOTIFY_POLLED:
-		del_timer_sync(&ghes->timer);
-		break;
-	case ACPI_HEST_NOTIFY_EXTERNAL:
-		free_irq(ghes->irq, ghes);
-		break;
-	case ACPI_HEST_NOTIFY_SCI:
-		mutex_lock(&ghes_list_mutex);
-		list_del_rcu(&ghes->list);
-		if (list_empty(&ghes_sci))
-			unregister_acpi_hed_notifier(&ghes_notifier_sci);
-		mutex_unlock(&ghes_list_mutex);
-		break;
-	case ACPI_HEST_NOTIFY_NMI:
-		mutex_lock(&ghes_list_mutex);
-		list_del_rcu(&ghes->list);
-		if (list_empty(&ghes_nmi))
-			unregister_nmi_handler(NMI_LOCAL, "ghes");
-		mutex_unlock(&ghes_list_mutex);
-		/*
-		 * To synchronize with NMI handler, ghes can only be
-		 * freed after NMI handler finishes.
-		 */
-		synchronize_rcu();
-		len = ghes_esource_prealloc_size(generic);
-		ghes_estatus_pool_shrink(len);
-		break;
-	default:
-		BUG();
-		break;
+
+	generic = ghes->generic;
+	notify_type = generic->notify.type;
+	if (notify_type >= ACPI_HEST_NOTIFY_RESERVED) {
+		pr_warn(FW_WARN GHES_PFX "Unknown notification type: %u for generic hardware error source: %d\n",
+			notify_type, generic->header.source_id);
+		return -EINVAL;
 	}
 
+	ghes_remove_call = ghes_notify_tab[notify_type].remove_call;
+	if (ghes_remove_call)
+		(*ghes_remove_call)(ghes);
+
 	ghes_fini(ghes);
 
 	ghes_edac_unregister(ghes);
@@ -1075,7 +1152,7 @@ static int __init ghes_init(void)
 		return -EINVAL;
 	}
 
-	init_irq_work(&ghes_proc_irq_work, ghes_proc_in_irq);
+	ghes_init_nmi();
 
 	rc = ghes_ioremap_init();
 	if (rc)
-- 
1.7.9.5

[PATCH v2 4/5] apei, ghes, nmi: Factor out NMI arch-specific calls.

[Tomasz Nowicki]

Similar to MCE related patch, all NMI architectural calls are abstracted.
Also, we are providing corresponding X86 functions' content.

Signed-off-by: Tomasz Nowicki <tomasz.nowicki@linaro.org>
---
 arch/x86/kernel/acpi/apei.c   |   25 +++++++++++++++++++++++++
 drivers/acpi/apei/apei-base.c |   19 +++++++++++++++++++
 drivers/acpi/apei/ghes.c      |   14 ++++++--------
 include/acpi/apei.h           |    7 +++++++
 4 files changed, 57 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kernel/acpi/apei.c b/arch/x86/kernel/acpi/apei.c
index dca2852..221a3a6 100644
--- a/arch/x86/kernel/acpi/apei.c
+++ b/arch/x86/kernel/acpi/apei.c
@@ -12,9 +12,12 @@
  * GNU General Public License for more details.
  */
 
+#include <linux/kdebug.h>
 #include <acpi/apei.h>
 
 #include <asm/mce.h>
+#include <asm/nmi.h>
+#include <asm/tlbflush.h>
 
 int apei_arch_enable_cmcff(struct acpi_hest_header *hest_hdr, void *data)
 {
@@ -54,3 +57,25 @@ void apei_arch_report_mem_error(int sev, struct cper_sec_mem_err *mem_err)
 	apei_mce_report_mem_error(sev, mem_err);
 #endif
 }
+
+int arch_apei_register_nmi(int (*nmi_handler)(unsigned int, struct pt_regs *),
+			   const char *name)
+{
+	struct nmiaction apei_nmi_action = {
+		.handler = nmi_handler,
+		.name = name,
+		.flags = 0,
+	};
+
+	return __register_nmi_handler(NMI_LOCAL, &apei_nmi_action);
+}
+
+void arch_apei_unregister_nmi(const char *name)
+{
+	unregister_nmi_handler(NMI_LOCAL, name);
+}
+
+void arch_apei_nmi_oops_begin(void)
+{
+	oops_begin();
+}
diff --git a/drivers/acpi/apei/apei-base.c b/drivers/acpi/apei/apei-base.c
index 4a11c1a..78c2a26 100644
--- a/drivers/acpi/apei/apei-base.c
+++ b/drivers/acpi/apei/apei-base.c
@@ -758,6 +758,25 @@ void __weak apei_arch_report_mem_error(int sev,
 }
 EXPORT_SYMBOL_GPL(apei_arch_report_mem_error);
 
+int __weak arch_apei_register_nmi(
+	__attribute__((unused))	int (*nmi_handler)(unsigned int,
+						   struct pt_regs *),
+	const char *name)
+{
+	return -ENOSYS;
+}
+EXPORT_SYMBOL_GPL(arch_apei_register_nmi);
+
+void __weak arch_apei_unregister_nmi(const char *name)
+{
+}
+EXPORT_SYMBOL_GPL(arch_apei_unregister_nmi);
+
+void __weak arch_apei_nmi_oops_begin(void)
+{
+}
+EXPORT_SYMBOL_GPL(arch_apei_nmi_oops_begin);
+
 int apei_osc_setup(void)
 {
 	static u8 whea_uuid_str[] = "ed855e0c-6c90-47bf-a62a-26de0fc5ad5c";
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index 750b04e..3c42629 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -51,7 +51,6 @@
 #include <acpi/ghes.h>
 #include <acpi/apei.h>
 #include <asm/tlbflush.h>
-#include <asm/nmi.h>
 
 #include "apei-internal.h"
 
@@ -817,7 +816,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 +831,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);
@@ -909,8 +908,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);
 
@@ -924,7 +922,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
diff --git a/include/acpi/apei.h b/include/acpi/apei.h
index 62b9d1c..348e1ea 100644
--- a/include/acpi/apei.h
+++ b/include/acpi/apei.h
@@ -14,6 +14,9 @@
 #define APEI_ERST_CLEAR_RECORD		_IOW('E', 1, u64)
 #define APEI_ERST_GET_RECORD_COUNT	_IOR('E', 2, u32)
 
+#define APEI_NMI_DONE		0
+#define APEI_NMI_HANDLED	1
+
 #ifdef __KERNEL__
 
 extern bool hest_disable;
@@ -44,6 +47,10 @@ int erst_clear(u64 record_id);
 
 int apei_arch_enable_cmcff(struct acpi_hest_header *hest_hdr, void *data);
 void apei_arch_report_mem_error(int sev, struct cper_sec_mem_err *mem_err);
+int arch_apei_register_nmi(int (*nmi_handler)(unsigned int, struct pt_regs *),
+			   const char *name);
+void arch_apei_unregister_nmi(const char *name);
+void arch_apei_nmi_oops_begin(void);
 
 #endif
 #endif
-- 
1.7.9.5

[PATCH v2 5/5] acpi, apei, ghes: Factor out ioremap virtual memory for IRQ and NMI context.

[Tomasz Nowicki]

GHES currently maps two pages with atomic_ioremap.  From now
on, NMI is optional so there is no need to allocate an NMI page
for platforms without NMI support.

To make it possible to not use a second page, swap the existing
page order so that the IRQ context page is first, and the optional
NMI context page is second.  Then, use CONFIG_ACPI_APEI_NMI to decide
at runtime how many pages are to be allocated.  Finally, put in
sanity checks to avoid accessing unallocated memory.

Signed-off-by: Tomasz Nowicki <tomasz.nowicki@linaro.org>
---
 arch/x86/kernel/acpi/apei.c |    6 ++++++
 drivers/acpi/apei/ghes.c    |   16 ++++++++--------
 include/acpi/apei.h         |    1 +
 3 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kernel/acpi/apei.c b/arch/x86/kernel/acpi/apei.c
index 221a3a6..842d158 100644
--- a/arch/x86/kernel/acpi/apei.c
+++ b/arch/x86/kernel/acpi/apei.c
@@ -79,3 +79,9 @@ void arch_apei_nmi_oops_begin(void)
 {
 	oops_begin();
 }
+
+void arch_apei_flush_tlb_one(unsigned long addr)
+{
+	__flush_tlb_one(addr);
+}
+EXPORT_SYMBOL_GPL(arch_apei_flush_tlb_one);
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index 3c42629..55bf47f 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -113,12 +113,11 @@ static DEFINE_RAW_SPINLOCK(ghes_nmi_lock);
  */
 
 /*
- * Two virtual pages are used, one for NMI context, the other for
- * IRQ/PROCESS context
+ * Two virtual pages are used, one for IRQ/PROCESS context, the other for
+ * NMI context (optionally).
  */
-#define GHES_IOREMAP_PAGES		2
-#define GHES_IOREMAP_NMI_PAGE(base)	(base)
-#define GHES_IOREMAP_IRQ_PAGE(base)	((base) + PAGE_SIZE)
+#define GHES_IOREMAP_IRQ_PAGE(base)	(base)
+#define GHES_IOREMAP_NMI_PAGE(base)	((base) + PAGE_SIZE)
 
 /* virtual memory area for atomic ioremap */
 static struct vm_struct *ghes_ioremap_area;
@@ -155,7 +154,8 @@ static struct ghes_notify_setup	ghes_notify_tab[];
 
 static int ghes_ioremap_init(void)
 {
-	ghes_ioremap_area = __get_vm_area(PAGE_SIZE * GHES_IOREMAP_PAGES,
+	ghes_ioremap_area = __get_vm_area(
+		PAGE_SIZE * (IS_ENABLED(CONFIG_ACPI_APEI_NMI) ? 2 : 1),
 		VM_IOREMAP, VMALLOC_START, VMALLOC_END);
 	if (!ghes_ioremap_area) {
 		pr_err(GHES_PFX "Failed to allocate virtual memory area for atomic ioremap.\n");
@@ -199,7 +199,7 @@ static void ghes_iounmap_nmi(void __iomem *vaddr_ptr)
 
 	BUG_ON(vaddr != (unsigned long)GHES_IOREMAP_NMI_PAGE(base));
 	unmap_kernel_range_noflush(vaddr, PAGE_SIZE);
-	__flush_tlb_one(vaddr);
+	arch_apei_flush_tlb_one(vaddr);
 }
 
 static void ghes_iounmap_irq(void __iomem *vaddr_ptr)
@@ -209,7 +209,7 @@ static void ghes_iounmap_irq(void __iomem *vaddr_ptr)
 
 	BUG_ON(vaddr != (unsigned long)GHES_IOREMAP_IRQ_PAGE(base));
 	unmap_kernel_range_noflush(vaddr, PAGE_SIZE);
-	__flush_tlb_one(vaddr);
+	arch_apei_flush_tlb_one(vaddr);
 }
 
 static int ghes_estatus_pool_init(void)
diff --git a/include/acpi/apei.h b/include/acpi/apei.h
index 348e1ea..ff2bb7e 100644
--- a/include/acpi/apei.h
+++ b/include/acpi/apei.h
@@ -51,6 +51,7 @@ int arch_apei_register_nmi(int (*nmi_handler)(unsigned int, struct pt_regs *),
 			   const char *name);
 void arch_apei_unregister_nmi(const char *name);
 void arch_apei_nmi_oops_begin(void);
+void arch_apei_flush_tlb_one(unsigned long addr);
 
 #endif
 #endif
-- 
1.7.9.5

Re: [PATCH v2 0/5] APEI: Make APEI architecture independent.

[Tomasz Nowicki]

Hi,

I would like to kindly remind and request for comments on this patches.

Regards,
Tomasz

On 28.05.2014 09:39, Tomasz Nowicki wrote:
> APEI is currently implemented so that it depends on x86 hardware.
> The primary dependency is that GHES uses the x86 NMI for hardware
> error notification and MCE for memory error handling. These patches
> remove that dependency.
>
> Other APEI features such as error reporting via external IRQ, error
> serialization, or error injection, do not require changes to use them
> on non-x86 architectures.
>
> The following patch set eliminates the APEI Kconfig x86 dependency
> by making these changes:
> - treat NMI notification as GHES feature - CONFIG_ACPI_APEI_NMI
> - reorganize GHES notification type init/deinitialization
>    so that we can decide in run time whether NMI is supported or not
> - identify architectural boxes and abstract it accordingly (NMI and MCE)
> - rework ioremap for both IRQ and NMI context
>
> NMI code is kept in ghes.c file since NMI and IRQ context are tightly coupled.
>
> Note, these patches introduce no functional changes for x86. The NMI notification
> feature is selected for x86 by default. Architectures that want to use this
> feature should also provide NMI code infrastructure.
>
> V1->V2
> - address Borislav comment
> - abstract arch-specific calls instead of wrapping into the #ifdef
>
> Tomasz Nowicki (5):
>    apei, mce: Factor out APEI architecture specific MCE calls.
>    acpi, apei, ghes: Introduce ACPI_APEI_NMI to make NMI error
>      notification a GHES feature.
>    acpi, apei, ghes: Introduce more generic mechanism to init/deinit
>      GHES error notifications.
>    apei, ghes, nmi: Factor out NMI arch-specific calls.
>    acpi, apei, ghes: Factor out ioremap virtual memory for IRQ and NMI
>      context.
>
>   arch/x86/kernel/acpi/Makefile |    1 +
>   arch/x86/kernel/acpi/apei.c   |   87 ++++++++++++
>   drivers/acpi/apei/Kconfig     |   10 +-
>   drivers/acpi/apei/apei-base.c |   32 +++++
>   drivers/acpi/apei/ghes.c      |  301 +++++++++++++++++++++++++----------------
>   drivers/acpi/apei/hest.c      |   26 +---
>   include/acpi/apei.h           |   11 ++
>   7 files changed, 328 insertions(+), 140 deletions(-)
>   create mode 100644 arch/x86/kernel/acpi/apei.c
>

Re: [PATCH v2 1/5] apei, mce: Factor out APEI architecture specific MCE calls.

[Robert Richter]

On 28.05.14 09:39:26, Tomasz Nowicki wrote:
> This commit abstracts MCE calls and provides weak corresponding default
> implementation for those architectures which do not need arch specific
> actions. Each platform willing to do additional architectural actions
> should provides desired function definition. It allows us to avoid wrap
> code into #ifdef in generic code and prevent new platform from introducing
> dummy stub function too.
> 
> Initially, there are two APEI arch-specific calls:
> - apei_arch_enable_cmcff()
> - apei_arch_report_mem_error()
> Both interact with MCE driver for X86 architecture.
> 
> Signed-off-by: Tomasz Nowicki <tomasz.nowicki@linaro.org>
> ---
>  arch/x86/kernel/acpi/Makefile |    1 +
>  arch/x86/kernel/acpi/apei.c   |   56 +++++++++++++++++++++++++++++++++++++++++
>  drivers/acpi/apei/apei-base.c |   13 ++++++++++
>  drivers/acpi/apei/ghes.c      |    6 ++---
>  drivers/acpi/apei/hest.c      |   26 +------------------
>  include/acpi/apei.h           |    3 +++
>  6 files changed, 76 insertions(+), 29 deletions(-)
>  create mode 100644 arch/x86/kernel/acpi/apei.c

>  static int __init hest_parse_cmc(struct acpi_hest_header *hest_hdr, void *data)
>  {
> -#ifdef CONFIG_X86_MCE
> -	int i;
> -	struct acpi_hest_ia_corrected *cmc;
> -	struct acpi_hest_ia_error_bank *mc_bank;
> -
> -	if (hest_hdr->type != ACPI_HEST_TYPE_IA32_CORRECTED_CHECK)
> -		return 0;
> -
> -	cmc = (struct acpi_hest_ia_corrected *)hest_hdr;
> -	if (!cmc->enabled)
> +	if (!apei_arch_enable_cmcff(hest_hdr, data))
>  		return 0;

This could be a tail call:

	return apei_arch_enable_cmcff(...);

Otherwise the patch looks good to me.

-Robert

>  
> -	/*
> -	 * We expect HEST to provide a list of MC banks that report errors
> -	 * in firmware first mode. Otherwise, return non-zero value to
> -	 * indicate that we are done parsing HEST.
> -	 */
> -	if (!(cmc->flags & ACPI_HEST_FIRMWARE_FIRST) || !cmc->num_hardware_banks)
> -		return 1;
> -
> -	pr_info(HEST_PFX "Enabling Firmware First mode for corrected errors.\n");
> -
> -	mc_bank = (struct acpi_hest_ia_error_bank *)(cmc + 1);
> -	for (i = 0; i < cmc->num_hardware_banks; i++, mc_bank++)
> -		mce_disable_bank(mc_bank->bank_number);
> -#endif
>  	return 1;
>  }

Re: [PATCH v2 2/5] acpi, apei, ghes: Introduce ACPI_APEI_NMI to make NMI error notification a GHES feature.

[Robert Richter]

On 28.05.14 09:39:27, Tomasz Nowicki wrote:
> +config ACPI_APEI_NMI
> +	bool "NMI error notification support"
> +	default y
> +	depends on ACPI_APEI_GHES && X86
> +	help
> +	  Firmware first mode can use NMI notification mechanism to report errors
> +	  to operating system. This feature is currently supported by X86
> +	  architecture only.
> +

This selection is not necessary. I don't see why one should select
this at all. It's either there for an arch or not. Just use ARCH_HAS_
style and and hard select ARCH_HAS_ACPI_APEI_NMI for x86. There is no
need for the ACPI_APEI_GHES dependency then.

-Robert

Re: [PATCH v2 1/5] apei, mce: Factor out APEI architecture specific MCE calls.

[Tomasz Nowicki]

On 12.06.2014 12:23, Robert Richter wrote:
> On 28.05.14 09:39:26, Tomasz Nowicki wrote:
>> This commit abstracts MCE calls and provides weak corresponding default
>> implementation for those architectures which do not need arch specific
>> actions. Each platform willing to do additional architectural actions
>> should provides desired function definition. It allows us to avoid wrap
>> code into #ifdef in generic code and prevent new platform from introducing
>> dummy stub function too.
>>
>> Initially, there are two APEI arch-specific calls:
>> - apei_arch_enable_cmcff()
>> - apei_arch_report_mem_error()
>> Both interact with MCE driver for X86 architecture.
>>
>> Signed-off-by: Tomasz Nowicki <tomasz.nowicki@linaro.org>
>> ---
>>   arch/x86/kernel/acpi/Makefile |    1 +
>>   arch/x86/kernel/acpi/apei.c   |   56 +++++++++++++++++++++++++++++++++++++++++
>>   drivers/acpi/apei/apei-base.c |   13 ++++++++++
>>   drivers/acpi/apei/ghes.c      |    6 ++---
>>   drivers/acpi/apei/hest.c      |   26 +------------------
>>   include/acpi/apei.h           |    3 +++
>>   6 files changed, 76 insertions(+), 29 deletions(-)
>>   create mode 100644 arch/x86/kernel/acpi/apei.c
>
>>   static int __init hest_parse_cmc(struct acpi_hest_header *hest_hdr, void *data)
>>   {
>> -#ifdef CONFIG_X86_MCE
>> -	int i;
>> -	struct acpi_hest_ia_corrected *cmc;
>> -	struct acpi_hest_ia_error_bank *mc_bank;
>> -
>> -	if (hest_hdr->type != ACPI_HEST_TYPE_IA32_CORRECTED_CHECK)
>> -		return 0;
>> -
>> -	cmc = (struct acpi_hest_ia_corrected *)hest_hdr;
>> -	if (!cmc->enabled)
>> +	if (!apei_arch_enable_cmcff(hest_hdr, data))
>>   		return 0;
>
> This could be a tail call:
>
> 	return apei_arch_enable_cmcff(...);

Right. Will fix that.

>
> Otherwise the patch looks good to me.

Thanks.

Tomasz

Re: [PATCH v2 2/5] acpi, apei, ghes: Introduce ACPI_APEI_NMI to make NMI error notification a GHES feature.

[Tomasz Nowicki]

On 12.06.2014 12:32, Robert Richter wrote:
> On 28.05.14 09:39:27, Tomasz Nowicki wrote:
>> +config ACPI_APEI_NMI
>> +	bool "NMI error notification support"
>> +	default y
>> +	depends on ACPI_APEI_GHES && X86
>> +	help
>> +	  Firmware first mode can use NMI notification mechanism to report errors
>> +	  to operating system. This feature is currently supported by X86
>> +	  architecture only.
>> +
>
> This selection is not necessary. I don't see why one should select
> this at all. It's either there for an arch or not. Just use ARCH_HAS_
> style and and hard select ARCH_HAS_ACPI_APEI_NMI for x86. There is no
> need for the ACPI_APEI_GHES dependency then.
>

It does make sense to me, will address your comment appropriately.

Tomasz