diff mbox

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

Message ID 1402657380-18539-2-git-send-email-tomasz.nowicki@linaro.org (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Tomasz Nowicki June 13, 2014, 11:02 a.m. UTC
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      |   29 +--------------------
 include/acpi/apei.h           |    3 +++
 6 files changed, 76 insertions(+), 32 deletions(-)
 create mode 100644 arch/x86/kernel/acpi/apei.c

Comments

Robert Richter June 13, 2014, 1:54 p.m. UTC | #1
On 13.06.14 13:02:56, 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      |   29 +--------------------
>  include/acpi/apei.h           |    3 +++
>  6 files changed, 76 insertions(+), 32 deletions(-)
>  create mode 100644 arch/x86/kernel/acpi/apei.c

Looks fine to me.

-Robert
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Borislav Petkov June 19, 2014, 2:17 p.m. UTC | #2
On Fri, Jun 13, 2014 at 01:02:56PM +0200, 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>

...

> 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)

Arch-specific function names usually use the "arch_" prefix. Otherwise
it looks ok.
Tomasz Nowicki June 24, 2014, 9:01 a.m. UTC | #3
On 19.06.2014 16:17, Borislav Petkov wrote:
> On Fri, Jun 13, 2014 at 01:02:56PM +0200, 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>
>
> ...
>
>> 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)
>
> Arch-specific function names usually use the "arch_" prefix. Otherwise
> it looks ok.

True, will fix that.

Tomasz
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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..edf26bc 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,33 +127,7 @@  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)
-		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;
+	return apei_arch_enable_cmcff(hest_hdr, data);
 }
 
 struct ghes_arr {
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