Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1689523 > unrolled thread
| Started by | Toshi Kani <toshi.kani@hpe.com> |
|---|---|
| First post | 2017-07-18 00:10 +0200 |
| Last post | 2017-07-18 22:20 +0200 |
| Articles | 20 on this page of 78 — 8 participants |
Back to article view | Back to linux.kernel
[PATCH 0/3] enable ghes_edac on selected platforms Toshi Kani <toshi.kani@hpe.com> - 2017-07-18 00:10 +0200
[PATCH 2/3] intel_pstate: convert to use acpi_match_oemlist() Toshi Kani <toshi.kani@hpe.com> - 2017-07-18 00:10 +0200
[PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Toshi Kani <toshi.kani@hpe.com> - 2017-07-18 00:10 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Borislav Petkov <bp@alien8.de> - 2017-07-18 08:10 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Borislav Petkov <bp@alien8.de> - 2017-07-18 10:10 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-07-18 23:30 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Borislav Petkov <bp@alien8.de> - 2017-07-19 08:00 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-07-19 18:20 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Borislav Petkov <bp@alien8.de> - 2017-07-19 18:30 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-07-19 19:00 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Borislav Petkov <bp@alien8.de> - 2017-07-20 06:20 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-07-20 16:50 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Borislav Petkov <bp@alien8.de> - 2017-07-20 17:10 +0200
RE: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac "Luck, Tony" <tony.luck@intel.com> - 2017-07-20 19:00 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Borislav Petkov <bp@alien8.de> - 2017-07-20 19:10 +0200
RE: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac "Luck, Tony" <tony.luck@intel.com> - 2017-07-20 19:20 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Mauro Carvalho Chehab <mchehab@s-opensource.com> - 2017-07-20 20:20 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Aristeu Rozanski <aris@redhat.com> - 2017-07-19 21:00 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-07-19 22:20 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Borislav Petkov <bp@alien8.de> - 2017-07-20 06:30 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-07-18 22:00 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Mauro Carvalho Chehab <mchehab@s-opensource.com> - 2017-07-18 23:20 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Borislav Petkov <bp@alien8.de> - 2017-07-19 08:00 +0200
RE: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac "Luck, Tony" <tony.luck@intel.com> - 2017-07-19 17:20 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Borislav Petkov <bp@alien8.de> - 2017-07-19 18:00 +0200
RE: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac "Luck, Tony" <tony.luck@intel.com> - 2017-07-19 20:10 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-07-19 18:50 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Borislav Petkov <bp@alien8.de> - 2017-07-20 06:40 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-07-20 22:00 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Mauro Carvalho Chehab <mchehab@s-opensource.com> - 2017-07-20 22:20 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-07-20 23:10 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Borislav Petkov <bp@alien8.de> - 2017-07-21 15:40 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Borislav Petkov <bp@alien8.de> - 2017-07-21 15:50 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Borislav Petkov <bp@alien8.de> - 2017-07-21 17:20 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-07-21 17:40 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Mauro Carvalho Chehab <mchehab@s-opensource.com> - 2017-07-21 17:50 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-07-21 18:50 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Mauro Carvalho Chehab <mchehab@s-opensource.com> - 2017-07-21 19:10 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Borislav Petkov <bp@alien8.de> - 2017-07-21 19:30 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-07-21 20:50 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Borislav Petkov <bp@alien8.de> - 2017-07-22 08:30 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-07-24 17:00 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Borislav Petkov <bp@alien8.de> - 2017-07-24 17:10 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-07-24 17:30 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Borislav Petkov <bp@alien8.de> - 2017-07-24 17:40 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-07-24 18:00 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Borislav Petkov <bp@alien8.de> - 2017-07-24 18:40 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-07-24 19:50 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Boris Petkov <bp@alien8.de> - 2017-07-24 20:00 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-07-24 20:00 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Borislav Petkov <bp@alien8.de> - 2017-07-24 20:20 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Mauro Carvalho Chehab <mchehab@s-opensource.com> - 2017-07-24 20:00 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-07-24 20:20 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Mauro Carvalho Chehab <mchehab@s-opensource.com> - 2017-07-24 18:10 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Borislav Petkov <bp@alien8.de> - 2017-07-24 18:50 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Mauro Carvalho Chehab <mchehab@s-opensource.com> - 2017-07-24 20:20 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Borislav Petkov <bp@alien8.de> - 2017-07-24 20:40 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-07-26 01:10 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Borislav Petkov <bp@alien8.de> - 2017-07-21 18:00 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-07-21 18:40 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-07-21 17:20 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Mauro Carvalho Chehab <mchehab@s-opensource.com> - 2017-07-21 15:50 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Borislav Petkov <bp@alien8.de> - 2017-07-19 08:00 +0200
RE: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac "Luck, Tony" <tony.luck@intel.com> - 2017-07-19 00:20 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Borislav Petkov <bp@alien8.de> - 2017-07-19 08:10 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Jeffrey Hugo <jhugo@codeaurora.org> - 2017-07-18 16:50 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-07-18 17:40 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac Jeffrey Hugo <jhugo@codeaurora.org> - 2017-07-18 18:30 +0200
Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-07-18 18:50 +0200
[PATCH 1/3] ACPI / blacklist: add acpi_match_oemlist() interface Toshi Kani <toshi.kani@hpe.com> - 2017-07-18 00:10 +0200
Re: [PATCH 1/3] ACPI / blacklist: add acpi_match_oemlist() interface Borislav Petkov <bp@alien8.de> - 2017-07-18 07:40 +0200
Re: [PATCH 1/3] ACPI / blacklist: add acpi_match_oemlist() interface "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-07-18 17:50 +0200
Re: [PATCH 1/3] ACPI / blacklist: add acpi_match_oemlist() interface Borislav Petkov <bp@alien8.de> - 2017-07-18 18:50 +0200
Re: [PATCH 1/3] ACPI / blacklist: add acpi_match_oemlist() interface "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-07-18 19:30 +0200
Re: [PATCH 1/3] ACPI / blacklist: add acpi_match_oemlist() interface Borislav Petkov <bp@alien8.de> - 2017-07-18 19:50 +0200
Re: [PATCH 1/3] ACPI / blacklist: add acpi_match_oemlist() interface "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-07-18 21:00 +0200
Re: [PATCH 1/3] ACPI / blacklist: add acpi_match_oemlist() interface Borislav Petkov <bp@alien8.de> - 2017-07-18 21:40 +0200
Re: [PATCH 1/3] ACPI / blacklist: add acpi_match_oemlist() interface "Kani, Toshimitsu" <toshi.kani@hpe.com> - 2017-07-18 22:20 +0200
Page 1 of 4 [1] 2 3 4 Next page →
| From | Toshi Kani <toshi.kani@hpe.com> |
|---|---|
| Date | 2017-07-18 00:10 +0200 |
| Subject | [PATCH 0/3] enable ghes_edac on selected platforms |
| Message-ID | <u4mnf-2mh-3@gated-at.bofh.it> |
The ghes_edac driver was introduced in 2013 [1], but it has not been enabled by any distro yet. This is because the driver obtains error info from firmware interfaces, which are not properly implemented on many platforms. To get out from this situation, add a platform check to selectively enable the driver on the platforms that are known to have proper firmware implementation. Platform vendors can add their platforms to the list when they support ghes_edac. Patch 1 moves the platform check in acpi_blacklisted() to a common utility func, acpi_match_oemlist(). Patch 2 converts the intel_pstate driver to use acpi_match_oemlist(). Patch 3 introduces a platform check to the ghes_edac driver. --- Toshi Kani (3): 1/3 ACPI / blacklist: add acpi_match_oemlist() interface 2/3 intel_pstate: convert to use acpi_match_oemlist() 3/3 ghes_edac: add platform check to enable ghes_edac --- drivers/acpi/blacklist.c | 84 ++++++++---------------------------------- drivers/acpi/utils.c | 40 ++++++++++++++++++++ drivers/cpufreq/intel_pstate.c | 64 +++++++++++++------------------- drivers/edac/ghes_edac.c | 28 +++++++++++--- include/linux/acpi.h | 19 ++++++++++ 5 files changed, 122 insertions(+), 113 deletions(-)
[toc] | [next] | [standalone]
| From | Toshi Kani <toshi.kani@hpe.com> |
|---|---|
| Date | 2017-07-18 00:10 +0200 |
| Subject | [PATCH 2/3] intel_pstate: convert to use acpi_match_oemlist() |
| Message-ID | <u4mnf-2mh-17@gated-at.bofh.it> |
| In reply to | #1689523 |
Convert to use acpi_match_oemlist() for the platform type check.
There is no change in functionality.
Signed-off-by: Toshi Kani <toshi.kani@hpe.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Cc: Len Brown <lenb@kernel.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
drivers/cpufreq/intel_pstate.c | 64 ++++++++++++++++------------------------
1 file changed, 25 insertions(+), 39 deletions(-)
diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index b7fb8b7..8f7703c 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -2458,39 +2458,31 @@ enum {
PPC,
};
-struct hw_vendor_info {
- u16 valid;
- char oem_id[ACPI_OEM_ID_SIZE];
- char oem_table_id[ACPI_OEM_TABLE_ID_SIZE];
- int oem_pwr_table;
-};
-
/* Hardware vendor-specific info that has its own power management modes */
-static struct hw_vendor_info vendor_info[] __initdata = {
- {1, "HP ", "ProLiant", PSS},
- {1, "ORACLE", "X4-2 ", PPC},
- {1, "ORACLE", "X4-2L ", PPC},
- {1, "ORACLE", "X4-2B ", PPC},
- {1, "ORACLE", "X3-2 ", PPC},
- {1, "ORACLE", "X3-2L ", PPC},
- {1, "ORACLE", "X3-2B ", PPC},
- {1, "ORACLE", "X4470M2 ", PPC},
- {1, "ORACLE", "X4270M3 ", PPC},
- {1, "ORACLE", "X4270M2 ", PPC},
- {1, "ORACLE", "X4170M2 ", PPC},
- {1, "ORACLE", "X4170 M3", PPC},
- {1, "ORACLE", "X4275 M3", PPC},
- {1, "ORACLE", "X6-2 ", PPC},
- {1, "ORACLE", "Sudbury ", PPC},
- {0, "", ""},
+static struct acpi_oemlist oemlist[] __initdata = {
+ {"HP ", "ProLiant", 0, ACPI_SIG_FADT, all_versions, 0, PSS},
+ {"ORACLE", "X4-2 ", 0, ACPI_SIG_FADT, all_versions, 0, PPC},
+ {"ORACLE", "X4-2L ", 0, ACPI_SIG_FADT, all_versions, 0, PPC},
+ {"ORACLE", "X4-2B ", 0, ACPI_SIG_FADT, all_versions, 0, PPC},
+ {"ORACLE", "X3-2 ", 0, ACPI_SIG_FADT, all_versions, 0, PPC},
+ {"ORACLE", "X3-2L ", 0, ACPI_SIG_FADT, all_versions, 0, PPC},
+ {"ORACLE", "X3-2B ", 0, ACPI_SIG_FADT, all_versions, 0, PPC},
+ {"ORACLE", "X4470M2 ", 0, ACPI_SIG_FADT, all_versions, 0, PPC},
+ {"ORACLE", "X4270M3 ", 0, ACPI_SIG_FADT, all_versions, 0, PPC},
+ {"ORACLE", "X4270M2 ", 0, ACPI_SIG_FADT, all_versions, 0, PPC},
+ {"ORACLE", "X4170M2 ", 0, ACPI_SIG_FADT, all_versions, 0, PPC},
+ {"ORACLE", "X4170 M3", 0, ACPI_SIG_FADT, all_versions, 0, PPC},
+ {"ORACLE", "X4275 M3", 0, ACPI_SIG_FADT, all_versions, 0, PPC},
+ {"ORACLE", "X6-2 ", 0, ACPI_SIG_FADT, all_versions, 0, PPC},
+ {"ORACLE", "Sudbury ", 0, ACPI_SIG_FADT, all_versions, 0, PPC},
+ { } /* End */
};
static bool __init intel_pstate_platform_pwr_mgmt_exists(void)
{
- struct acpi_table_header hdr;
- struct hw_vendor_info *v_info;
const struct x86_cpu_id *id;
u64 misc_pwr;
+ int idx;
id = x86_match_cpu(intel_pstate_cpu_oob_ids);
if (id) {
@@ -2499,21 +2491,15 @@ static bool __init intel_pstate_platform_pwr_mgmt_exists(void)
return true;
}
- if (acpi_disabled ||
- ACPI_FAILURE(acpi_get_table_header(ACPI_SIG_FADT, 0, &hdr)))
+ idx = acpi_match_oemlist(oemlist);
+ if (idx < 0)
return false;
- for (v_info = vendor_info; v_info->valid; v_info++) {
- if (!strncmp(hdr.oem_id, v_info->oem_id, ACPI_OEM_ID_SIZE) &&
- !strncmp(hdr.oem_table_id, v_info->oem_table_id,
- ACPI_OEM_TABLE_ID_SIZE))
- switch (v_info->oem_pwr_table) {
- case PSS:
- return intel_pstate_no_acpi_pss();
- case PPC:
- return intel_pstate_has_acpi_ppc() &&
- (!force_load);
- }
+ switch (oemlist[idx].data) {
+ case PSS:
+ return intel_pstate_no_acpi_pss();
+ case PPC:
+ return intel_pstate_has_acpi_ppc() && !force_load;
}
return false;
[toc] | [prev] | [next] | [standalone]
| From | Toshi Kani <toshi.kani@hpe.com> |
|---|---|
| Date | 2017-07-18 00:10 +0200 |
| Subject | [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac |
| Message-ID | <u4mnf-2mh-21@gated-at.bofh.it> |
| In reply to | #1689523 |
The ghes_edac driver was introduced in 2013 [1], but it has not
been enabled by any distro yet. This driver obtains error info
from firmware interfaces, which are not properly implemented on
many platforms, as the driver always emits the messages below:
This EDAC driver relies on BIOS to enumerate memory and get error reports.
Unfortunately, not all BIOSes reflect the memory layout correctly
So, the end result of using this driver varies from vendor to vendor
If you find incorrect reports, please contact your hardware vendor
to correct its BIOS.
To get out from this situation, add a platform type check to
selectively enable the driver on the platforms that are known to
have proper firmware implementation. Platform vendors can add
their platforms to the list when they support ghes_edac.
"ghes_edac.any_oem=1" skips the platform type check.
[1]: https://lwn.net/Articles/538438/
Signed-off-by: Toshi Kani <toshi.kani@hpe.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
drivers/edac/ghes_edac.c | 28 +++++++++++++++++++++++-----
1 file changed, 23 insertions(+), 5 deletions(-)
diff --git a/drivers/edac/ghes_edac.c b/drivers/edac/ghes_edac.c
index 4e61a62..00588a3 100644
--- a/drivers/edac/ghes_edac.c
+++ b/drivers/edac/ghes_edac.c
@@ -34,6 +34,9 @@ static LIST_HEAD(ghes_reglist);
static DEFINE_MUTEX(ghes_edac_lock);
static int ghes_edac_mc_num;
+/* Set 1 to skip the platform check */
+static bool __read_mostly ghes_edac_any_oem;
+module_param_named(any_oem, ghes_edac_any_oem, bool, 0);
/* Memory Device - Type 17 of SMBIOS spec */
struct memdev_dmi_entry {
@@ -405,6 +408,15 @@ void ghes_edac_report_mem_error(struct ghes *ghes, int sev,
}
EXPORT_SYMBOL_GPL(ghes_edac_report_mem_error);
+/*
+ * Known systems that are safe to enable this module.
+ * "ghes_edac.any_oem=1" skips this check if necessary.
+ */
+static struct acpi_oemlist oemlist[] = {
+ {"HPE ", "Server ", 0, ACPI_SIG_FADT, all_versions},
+ { } /* End */
+};
+
int ghes_edac_register(struct ghes *ghes, struct device *dev)
{
bool fake = false;
@@ -413,6 +425,12 @@ int ghes_edac_register(struct ghes *ghes, struct device *dev)
struct edac_mc_layer layers[1];
struct ghes_edac_pvt *pvt;
struct ghes_edac_dimm_fill dimm_fill;
+ int idx;
+
+ /* Check if safe to enable on this system */
+ idx = acpi_match_oemlist(oemlist);
+ if (!ghes_edac_any_oem && idx < 0)
+ return 0;
/* Get the number of DIMMs */
dmi_walk(ghes_edac_count_dimms, &num_dimm);
@@ -456,7 +474,11 @@ int ghes_edac_register(struct ghes *ghes, struct device *dev)
mci->dev_name = "ghes";
if (!ghes_edac_mc_num) {
- if (!fake) {
+ if (fake) {
+ pr_info("This system has a very crappy BIOS: It doesn't even list the DIMMS.\n");
+ pr_info("Its SMBIOS info is wrong. It is doubtful that the error report would\n");
+ pr_info("work on such system. Use this driver with caution\n");
+ } else if (idx < 0) {
pr_info("This EDAC driver relies on BIOS to enumerate memory and get error reports.\n");
pr_info("Unfortunately, not all BIOSes reflect the memory layout correctly.\n");
pr_info("So, the end result of using this driver varies from vendor to vendor.\n");
@@ -464,10 +486,6 @@ int ghes_edac_register(struct ghes *ghes, struct device *dev)
pr_info("to correct its BIOS.\n");
pr_info("This system has %d DIMM sockets.\n",
num_dimm);
- } else {
- pr_info("This system has a very crappy BIOS: It doesn't even list the DIMMS.\n");
- pr_info("Its SMBIOS info is wrong. It is doubtful that the error report would\n");
- pr_info("work on such system. Use this driver with caution\n");
}
}
[toc] | [prev] | [next] | [standalone]
| From | Borislav Petkov <bp@alien8.de> |
|---|---|
| Date | 2017-07-18 08:10 +0200 |
| Subject | Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac |
| Message-ID | <u4tRM-7a0-29@gated-at.bofh.it> |
| In reply to | #1689528 |
On Mon, Jul 17, 2017 at 03:59:12PM -0600, Toshi Kani wrote:
> The ghes_edac driver was introduced in 2013 [1], but it has not
> been enabled by any distro yet. This driver obtains error info
> from firmware interfaces, which are not properly implemented on
> many platforms, as the driver always emits the messages below:
>
> This EDAC driver relies on BIOS to enumerate memory and get error reports.
> Unfortunately, not all BIOSes reflect the memory layout correctly
> So, the end result of using this driver varies from vendor to vendor
> If you find incorrect reports, please contact your hardware vendor
> to correct its BIOS.
>
> To get out from this situation, add a platform type check to
> selectively enable the driver on the platforms that are known to
> have proper firmware implementation. Platform vendors can add
> their platforms to the list when they support ghes_edac.
So maintaining whitelists for things has always been a PITA and we
should try to avoid it, if possible. (We can always do it if nothing
saner comes along.)
Now, below is a dirty patch converting ghes_edac to a normal module.
On systems where we have GHES, the firmware generally disables the
detection of the presence of ECC hardware, thus preventing the platform
EDAC driver from loading.
Let me clarify: I have an AMD HP box which, when GHES is enabled in
the BIOS, says that ECC is disabled in the memory controller and the
amd64_edac driver doesn't load for that memory controller.
And I think we should try this first: have the firmware disable
detection methods so that the platform drivers don't load.
Then, ghes_edac can be a simple module and no other driver would attempt
loading.
The question is: does the platform do this disabling now?
Tony, I'm looking at sb_edac and there we don't do something like that
or maybe I'm missing it.
Hmmm.
---
From: Borislav Petkov <bp@suse.de>
Date: Thu, 29 Jun 2017 10:28:32 +0200
Subject: [PATCH] WIP
Not-Signed-off-by: Borislav Petkov <bp@suse.de>
---
drivers/acpi/apei/ghes.c | 32 ++++++-----
drivers/edac/Kconfig | 4 +-
drivers/edac/edac_mc.h | 3 ++
drivers/edac/ghes_edac.c | 137 ++++++++++++++++++++++++-----------------------
include/acpi/ghes.h | 27 +---------
5 files changed, 98 insertions(+), 105 deletions(-)
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index d661d452b238..37cd698cacd2 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -140,6 +140,20 @@ static atomic_t ghes_estatus_cache_alloced;
static int ghes_panic_timeout __read_mostly = 30;
+static ATOMIC_NOTIFIER_HEAD(ghes_edac_chain);
+
+void ghes_register_edac_chain(struct notifier_block *nb)
+{
+ atomic_notifier_chain_register(&ghes_edac_chain, nb);
+}
+EXPORT_SYMBOL_GPL(ghes_register_edac_chain);
+
+void ghes_unregister_edac_chain(struct notifier_block *nb)
+{
+ atomic_notifier_chain_unregister(&ghes_edac_chain, nb);
+}
+EXPORT_SYMBOL_GPL(ghes_unregister_edac_chain);
+
static int ghes_ioremap_init(void)
{
ghes_ioremap_area = __get_vm_area(PAGE_SIZE * GHES_IOREMAP_PAGES,
@@ -461,11 +475,11 @@ static void ghes_handle_memory_failure(struct acpi_hest_generic_data *gdata, int
static void ghes_do_proc(struct ghes *ghes,
const struct acpi_hest_generic_status *estatus)
{
- int sev, sec_sev;
struct acpi_hest_generic_data *gdata;
guid_t *sec_type;
guid_t *fru_id = &NULL_UUID_LE;
char *fru_text = "";
+ int sev, sec_sev;
sev = ghes_severity(estatus->error_severity);
apei_estatus_for_each_section(estatus, gdata) {
@@ -480,7 +494,8 @@ static void ghes_do_proc(struct ghes *ghes,
if (guid_equal(sec_type, &CPER_SEC_PLATFORM_MEM)) {
struct cper_sec_mem_err *mem_err = acpi_hest_get_payload(gdata);
- ghes_edac_report_mem_error(ghes, sev, mem_err);
+
+ atomic_notifier_call_chain(&ghes_edac_chain, sev, &mem_err);
arch_apei_report_mem_error(sev, mem_err);
ghes_handle_memory_failure(gdata, sev);
@@ -1139,10 +1154,6 @@ static int ghes_probe(struct platform_device *ghes_dev)
goto err;
}
- rc = ghes_edac_register(ghes, &ghes_dev->dev);
- if (rc < 0)
- goto err;
-
switch (generic->notify.type) {
case ACPI_HEST_NOTIFY_POLLED:
setup_deferrable_timer(&ghes->timer, ghes_poll_func,
@@ -1155,13 +1166,13 @@ static int ghes_probe(struct platform_device *ghes_dev)
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;
+ goto err;
}
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;
+ goto err;
}
break;
@@ -1190,8 +1201,7 @@ static int ghes_probe(struct platform_device *ghes_dev)
ghes_proc(ghes);
return 0;
-err_edac_unreg:
- ghes_edac_unregister(ghes);
+
err:
if (ghes) {
ghes_fini(ghes);
@@ -1241,8 +1251,6 @@ static int ghes_remove(struct platform_device *ghes_dev)
ghes_fini(ghes);
- ghes_edac_unregister(ghes);
-
kfree(ghes);
platform_set_drvdata(ghes_dev, NULL);
diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
index 96afb2aeed18..fdd8278ca89a 100644
--- a/drivers/edac/Kconfig
+++ b/drivers/edac/Kconfig
@@ -53,8 +53,8 @@ config EDAC_DECODE_MCE
has been initialized.
config EDAC_GHES
- bool "Output ACPI APEI/GHES BIOS detected errors via EDAC"
- depends on ACPI_APEI_GHES && (EDAC=y)
+ tristate "Output ACPI APEI/GHES BIOS detected errors via EDAC"
+ depends on ACPI_APEI_GHES
help
Not all machines support hardware-driven error report. Some of those
provide a BIOS-driven error report mechanism via ACPI, using the
diff --git a/drivers/edac/edac_mc.h b/drivers/edac/edac_mc.h
index 5357800e418d..6d46f30dc657 100644
--- a/drivers/edac/edac_mc.h
+++ b/drivers/edac/edac_mc.h
@@ -60,6 +60,9 @@
#define edac_pci_printk(ctl, level, fmt, arg...) \
printk(level "EDAC PCI%d: " fmt, ctl->pci_idx, ##arg)
+#define edac_pr_err(fmt, arg...) edac_printk(KERN_ERR, "", fmt, ##arg)
+#define edac_pr_info(fmt, arg...) edac_printk(KERN_INFO, "", fmt, ##arg)
+
/* prefixes for edac_printk() and edac_mc_printk() */
#define EDAC_MC "MC"
#define EDAC_PCI "PCI"
diff --git a/drivers/edac/ghes_edac.c b/drivers/edac/ghes_edac.c
index 4e61a6229dd2..20fafc55eb2d 100644
--- a/drivers/edac/ghes_edac.c
+++ b/drivers/edac/ghes_edac.c
@@ -5,6 +5,9 @@
* License version 2.
*
* Copyright (c) 2013 by Mauro Carvalho Chehab
+ * (c) 2017 Borislav Petkov
+ *
+ * Borislav Petkov: turn it into a proper module.
*
* Red Hat Inc. http://www.redhat.com
*/
@@ -17,7 +20,14 @@
#include "edac_module.h"
#include <ras/ras_event.h>
-#define GHES_EDAC_REVISION " Ver: 1.0.0"
+#define GHES_EDAC_REVISION " Ver: 2.0.0"
+
+/*
+ * Hand it into EDAC's core so that we have a device to operate on.
+ */
+static struct device dummy_dev;
+
+struct ghes_edac_pvt *ghes_pvt;
struct ghes_edac_pvt {
struct list_head list;
@@ -30,11 +40,6 @@ struct ghes_edac_pvt {
char msg[80];
};
-static LIST_HEAD(ghes_reglist);
-static DEFINE_MUTEX(ghes_edac_lock);
-static int ghes_edac_mc_num;
-
-
/* Memory Device - Type 17 of SMBIOS spec */
struct memdev_dmi_entry {
u8 type;
@@ -165,24 +170,21 @@ static void ghes_edac_dmidecode(const struct dmi_header *dh, void *arg)
}
}
-void ghes_edac_report_mem_error(struct ghes *ghes, int sev,
- struct cper_sec_mem_err *mem_err)
+static int report_mem_error(struct notifier_block *nb, unsigned long sev, void *data)
{
+ struct cper_sec_mem_err *mem_err = data;
enum hw_event_mc_err_type type;
struct edac_raw_error_desc *e;
struct mem_ctl_info *mci;
- struct ghes_edac_pvt *pvt = NULL;
- char *p;
+ struct ghes_edac_pvt *pvt = ghes_pvt;
u8 grain_bits;
+ char *p;
- list_for_each_entry(pvt, &ghes_reglist, list) {
- if (ghes == pvt->ghes)
- break;
- }
if (!pvt) {
- pr_err("Internal error: Can't find EDAC structure\n");
- return;
+ edac_pr_err("Internal error: Can't find EDAC structure\n");
+ return NOTIFY_DONE;
}
+
mci = pvt->mci;
e = &mci->error_desc;
@@ -402,23 +404,40 @@ void ghes_edac_report_mem_error(struct ghes *ghes, int sev,
/* Report the error via EDAC API */
edac_raw_mc_handle_error(type, mci, e);
+
+ return NOTIFY_DONE;
}
-EXPORT_SYMBOL_GPL(ghes_edac_report_mem_error);
-int ghes_edac_register(struct ghes *ghes, struct device *dev)
+static struct notifier_block ghes_nb = {
+ .notifier_call = report_mem_error,
+};
+
+static const char * const fake_msg =
+"This EDAC driver relies on BIOS to enumerate memory and get error reports.\n"
+"Unfortunately, not all BIOSes reflect the memory layout correctly.\n"
+"So, the end result of using this driver varies from vendor to vendor.\n"
+"If you find incorrect reports, please contact your hardware vendor\n"
+"to correct its BIOS.";
+
+static const char * const super_crap_msg =
+"This system has a very crappy BIOS: It doesn't even list the DIMMS.\n"
+"Its SMBIOS info is wrong. It is doubtful that the error report would\n"
+"work on such system. Use this driver with caution.";
+
+static int __init ghes_edac_register(void)
{
+ struct ghes_edac_pvt *pvt = ghes_pvt;
bool fake = false;
int rc, num_dimm = 0;
struct mem_ctl_info *mci;
struct edac_mc_layer layers[1];
- struct ghes_edac_pvt *pvt;
struct ghes_edac_dimm_fill dimm_fill;
/* Get the number of DIMMs */
dmi_walk(ghes_edac_count_dimms, &num_dimm);
/* Check if we've got a bogus BIOS */
- if (num_dimm == 0) {
+ if (!num_dimm) {
fake = true;
num_dimm = 1;
}
@@ -431,21 +450,17 @@ int ghes_edac_register(struct ghes *ghes, struct device *dev)
* We need to serialize edac_mc_alloc() and edac_mc_add_mc(),
* to avoid duplicated memory controller numbers
*/
- mutex_lock(&ghes_edac_lock);
- mci = edac_mc_alloc(ghes_edac_mc_num, ARRAY_SIZE(layers), layers,
- sizeof(*pvt));
+ mci = edac_mc_alloc(1, ARRAY_SIZE(layers), layers, sizeof(*pvt));
if (!mci) {
- pr_info("Can't allocate memory for EDAC data\n");
- mutex_unlock(&ghes_edac_lock);
+ edac_pr_err("Can't allocate memory for EDAC data\n");
return -ENOMEM;
}
pvt = mci->pvt_info;
memset(pvt, 0, sizeof(*pvt));
- list_add_tail(&pvt->list, &ghes_reglist);
- pvt->ghes = ghes;
pvt->mci = mci;
- mci->pdev = dev;
+
+ mci->pdev = &dummy_dev;
mci->mtype_cap = MEM_FLAG_EMPTY;
mci->edac_ctl_cap = EDAC_FLAG_NONE;
@@ -455,21 +470,12 @@ int ghes_edac_register(struct ghes *ghes, struct device *dev)
mci->ctl_name = "ghes_edac";
mci->dev_name = "ghes";
- if (!ghes_edac_mc_num) {
- if (!fake) {
- pr_info("This EDAC driver relies on BIOS to enumerate memory and get error reports.\n");
- pr_info("Unfortunately, not all BIOSes reflect the memory layout correctly.\n");
- pr_info("So, the end result of using this driver varies from vendor to vendor.\n");
- pr_info("If you find incorrect reports, please contact your hardware vendor\n");
- pr_info("to correct its BIOS.\n");
- pr_info("This system has %d DIMM sockets.\n",
- num_dimm);
- } else {
- pr_info("This system has a very crappy BIOS: It doesn't even list the DIMMS.\n");
- pr_info("Its SMBIOS info is wrong. It is doubtful that the error report would\n");
- pr_info("work on such system. Use this driver with caution\n");
- }
- }
+ if (!fake)
+ edac_pr_info("%s\n", fake_msg);
+ else
+ edac_pr_info("%s\n", super_crap_msg);
+
+ edac_pr_info("This system has %d DIMM sockets.\n", num_dimm);
if (!fake) {
/*
@@ -478,13 +484,11 @@ int ghes_edac_register(struct ghes *ghes, struct device *dev)
* Keep it in blank for the other memory controllers, as
* there's no reliable way to properly credit each DIMM to
* the memory controller, as different BIOSes fill the
- * DMI bank location fields on different ways
+ * DMI bank location fields in different ways.
*/
- if (!ghes_edac_mc_num) {
- dimm_fill.count = 0;
- dimm_fill.mci = mci;
- dmi_walk(ghes_edac_dmidecode, &dimm_fill);
- }
+ dimm_fill.count = 0;
+ dimm_fill.mci = mci;
+ dmi_walk(ghes_edac_dmidecode, &dimm_fill);
} else {
struct dimm_info *dimm = EDAC_DIMM_PTR(mci->layers, mci->dimms,
mci->n_layers, 0, 0, 0);
@@ -498,30 +502,31 @@ int ghes_edac_register(struct ghes *ghes, struct device *dev)
rc = edac_mc_add_mc(mci);
if (rc < 0) {
- pr_info("Can't register at EDAC core\n");
+ edac_pr_err("Can't register with EDAC core\n");
edac_mc_free(mci);
- mutex_unlock(&ghes_edac_lock);
return -ENODEV;
}
- ghes_edac_mc_num++;
- mutex_unlock(&ghes_edac_lock);
+ ghes_register_edac_chain(&ghes_nb);
+
return 0;
}
-EXPORT_SYMBOL_GPL(ghes_edac_register);
+module_init(ghes_edac_register);
-void ghes_edac_unregister(struct ghes *ghes)
+static void __exit ghes_edac_unregister(void)
{
struct mem_ctl_info *mci;
- struct ghes_edac_pvt *pvt, *tmp;
-
- list_for_each_entry_safe(pvt, tmp, &ghes_reglist, list) {
- if (ghes == pvt->ghes) {
- mci = pvt->mci;
- edac_mc_del_mc(mci->pdev);
- edac_mc_free(mci);
- list_del(&pvt->list);
- }
- }
+
+ ghes_unregister_edac_chain(&ghes_nb);
+
+ mci = find_mci_by_dev(&dummy_dev);
+ WARN_ON(!mci);
+
+ edac_mc_del_mc(mci->pdev);
+ edac_mc_free(mci);
+
}
-EXPORT_SYMBOL_GPL(ghes_edac_unregister);
+module_exit(ghes_edac_unregister);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("GHES error decoding module - " GHES_EDAC_REVISION);
diff --git a/include/acpi/ghes.h b/include/acpi/ghes.h
index 9f26e01186ae..c02b8eb91bd6 100644
--- a/include/acpi/ghes.h
+++ b/include/acpi/ghes.h
@@ -51,31 +51,8 @@ enum {
GHES_SEV_PANIC = 0x3,
};
-/* From drivers/edac/ghes_edac.c */
-
-#ifdef CONFIG_EDAC_GHES
-void ghes_edac_report_mem_error(struct ghes *ghes, int sev,
- struct cper_sec_mem_err *mem_err);
-
-int ghes_edac_register(struct ghes *ghes, struct device *dev);
-
-void ghes_edac_unregister(struct ghes *ghes);
-
-#else
-static inline void ghes_edac_report_mem_error(struct ghes *ghes, int sev,
- struct cper_sec_mem_err *mem_err)
-{
-}
-
-static inline int ghes_edac_register(struct ghes *ghes, struct device *dev)
-{
- return 0;
-}
-
-static inline void ghes_edac_unregister(struct ghes *ghes)
-{
-}
-#endif
+void ghes_register_edac_chain(struct notifier_block *nb);
+void ghes_unregister_edac_chain(struct notifier_block *nb);
static inline int acpi_hest_get_version(struct acpi_hest_generic_data *gdata)
{
--
2.14.0.rc0
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
[toc] | [prev] | [next] | [standalone]
| From | Borislav Petkov <bp@alien8.de> |
|---|---|
| Date | 2017-07-18 10:10 +0200 |
| Subject | Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac |
| Message-ID | <u4vJU-8k9-17@gated-at.bofh.it> |
| In reply to | #1689752 |
On Tue, Jul 18, 2017 at 08:00:07AM +0200, Borislav Petkov wrote:
> And I think we should try this first: have the firmware disable
> detection methods so that the platform drivers don't load.
Btw, in looking at this more, what about the firmware-first thing?
I.e., the firmware-first detection with apei_osc_setup() at the end of
ghes_init().
Can we make ghes_edac loading dependent on that? I mean, that was *the*
predicate for exactly that - to have the firmware look at the errors
first. No need for platform whitelisting and so on.
I'd still decouple ghes_edac loading from ghes_probe() even though
loading the platform driver should've been done *after* the
firmware-first detection regardless.
So what we could do is make ghes_edac a normal module and have the
relevant x86 EDAC modules query FF mode and if enabled, fail loading.
Hmmm?
My gut feeling tells me I'm on the right track here but who knows...
Thx.
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
[toc] | [prev] | [next] | [standalone]
| From | "Kani, Toshimitsu" <toshi.kani@hpe.com> |
|---|---|
| Date | 2017-07-18 23:30 +0200 |
| Subject | Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac |
| Message-ID | <u4Ie5-7Cc-5@gated-at.bofh.it> |
| In reply to | #1689854 |
On Tue, 2017-07-18 at 10:08 +0200, Borislav Petkov wrote: > On Tue, Jul 18, 2017 at 08:00:07AM +0200, Borislav Petkov wrote: > > And I think we should try this first: have the firmware disable > > detection methods so that the platform drivers don't load. > > Btw, in looking at this more, what about the firmware-first thing? > > I.e., the firmware-first detection with apei_osc_setup() at the end > of ghes_init(). > > Can we make ghes_edac loading dependent on that? I mean, that was > *the* predicate for exactly that - to have the firmware look at the > errors first. No need for platform whitelisting and so on. I agree that 'osc_sb_apei_support_acked' should be checked when enabling ghes_edac. I do not know the details of existing issues, but it sounds unlikely that this will address all of them since bugs can be everywhere. For instance, ghes_edac relies on DMI/SMBIOS info, unlike other EDAC drivers, which can be buggy regardless of this _OSC info. > I'd still decouple ghes_edac loading from ghes_probe() even though > loading the platform driver should've been done *after* the > firmware-first detection regardless. > > So what we could do is make ghes_edac a normal module and have the > relevant x86 EDAC modules query FF mode and if enabled, fail loading. I agree that making ghes_edac as a normal module is a good thing, but I do not think it's going to solve this issue. Thanks, -Toshi
[toc] | [prev] | [next] | [standalone]
| From | Borislav Petkov <bp@alien8.de> |
|---|---|
| Date | 2017-07-19 08:00 +0200 |
| Subject | Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac |
| Message-ID | <u4QbF-4dp-27@gated-at.bofh.it> |
| In reply to | #1690666 |
On Tue, Jul 18, 2017 at 09:20:44PM +0000, Kani, Toshimitsu wrote:
> I agree that 'osc_sb_apei_support_acked' should be checked when
> enabling ghes_edac. I do not know the details of existing issues, but
> it sounds unlikely that this will address all of them since bugs can be
> everywhere.
No, see below.
> For instance, ghes_edac relies on DMI/SMBIOS info, unlike
> other EDAC drivers, which can be buggy regardless of this _OSC info.
That's the problem with firmware. You can't really fix it and it is
buggy as hell.
> I agree that making ghes_edac as a normal module is a good thing, but I
> do not think it's going to solve this issue.
Of course it will - if the firmware says it wants to look at the errors
first, then it gets to do so. This is the whole handling of hardware
errors in the firmware deal. I admit, sometimes it makes sense because
the firmware has the most intimate knowledge of the platform and, in
a perfect world, we won't ever need to have platform-specific EDAC
drivers.
But, we don't live in a perfect world. And the vendor execution of the
whole firmware-error-handling deal is an abomination at best.
So, if we realize that the firmware is buggy, we can use a platform list
to blacklist it (^hint hint^) and have a parameter to disable ghes_edac
from loading.
But we'll deal with that when we get to cross that bridge. Right now,
I'd like to do the loading spec-conform and not fiddle with white-,
black-, or any-other-color lists.
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
[toc] | [prev] | [next] | [standalone]
| From | "Kani, Toshimitsu" <toshi.kani@hpe.com> |
|---|---|
| Date | 2017-07-19 18:20 +0200 |
| Subject | Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac |
| Message-ID | <u4ZRD-2BD-3@gated-at.bofh.it> |
| In reply to | #1690998 |
On Wed, 2017-07-19 at 07:52 +0200, Borislav Petkov wrote: > On Tue, Jul 18, 2017 at 09:20:44PM +0000, Kani, Toshimitsu wrote: > > I agree that 'osc_sb_apei_support_acked' should be checked when > > enabling ghes_edac. I do not know the details of existing issues, > > but it sounds unlikely that this will address all of them since > > bugs can be everywhere. > > No, see below. > > > For instance, ghes_edac relies on DMI/SMBIOS info, unlike > > other EDAC drivers, which can be buggy regardless of this _OSC > > info. > > That's the problem with firmware. You can't really fix it and it is > buggy as hell. Right, and that's what I was told as an issue for ghes_edac. This is why this patch introduces a white-list to preclude all buggy firmwares that are unknown to us... > > I agree that making ghes_edac as a normal module is a good thing, > > but I do not think it's going to solve this issue. > > Of course it will - if the firmware says it wants to look at the > errors first, then it gets to do so. This is the whole handling of > hardware errors in the firmware deal. I admit, sometimes it makes > sense because the firmware has the most intimate knowledge of the > platform and, in a perfect world, we won't ever need to have > platform-specific EDAC drivers. > > But, we don't live in a perfect world. And the vendor execution of > the whole firmware-error-handling deal is an abomination at best. > > So, if we realize that the firmware is buggy, we can use a platform > list to blacklist it (^hint hint^) and have a parameter to disable > ghes_edac from loading. Setting blacklist needs us to enable ghes_edac and find all buggy firmwares to date. I think this is too disturbing for people who are happily using regular edac drivers today even though their platforms have GHES. > But we'll deal with that when we get to cross that bridge. Right now, > I'd like to do the loading spec-conform and not fiddle with white-, > black-, or any-other-color lists. I do prefer to avoid any white / black listing. But I do not see how it solves the buggy DMI/SMBIOS info as an example of firmware bugs we may have to deal with. Thanks, -Toshi
[toc] | [prev] | [next] | [standalone]
| From | Borislav Petkov <bp@alien8.de> |
|---|---|
| Date | 2017-07-19 18:30 +0200 |
| Subject | Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac |
| Message-ID | <u501k-2Gh-31@gated-at.bofh.it> |
| In reply to | #1691998 |
On Wed, Jul 19, 2017 at 04:10:07PM +0000, Kani, Toshimitsu wrote:
> I do prefer to avoid any white / black listing. But I do not see how
> it solves the buggy DMI/SMBIOS info as an example of firmware bugs we
> may have to deal with.
So how do you want to deal with this?
Maintain an evergrowing whitelist of platforms which are OK and then the
moment a new platform comes along, you send a patch to add it to that
whitelist?
I'm sure you can see the problems with that approach.
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
[toc] | [prev] | [next] | [standalone]
| From | "Kani, Toshimitsu" <toshi.kani@hpe.com> |
|---|---|
| Date | 2017-07-19 19:00 +0200 |
| Subject | Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac |
| Message-ID | <u50un-2TW-29@gated-at.bofh.it> |
| In reply to | #1692014 |
On Wed, 2017-07-19 at 18:22 +0200, Borislav Petkov wrote: > On Wed, Jul 19, 2017 at 04:10:07PM +0000, Kani, Toshimitsu wrote: > > I do prefer to avoid any white / black listing. But I do not see > > how it solves the buggy DMI/SMBIOS info as an example of firmware > > bugs we may have to deal with. > > So how do you want to deal with this? > > Maintain an evergrowing whitelist of platforms which are OK and then > the moment a new platform comes along, you send a patch to add it to > that whitelist? > > I'm sure you can see the problems with that approach. Since ghes_edac has not been used for a long time, I have a feeling that not so many vendors want to use it. In the case of HPE, we do not need to update with each platform since "HPE" "Server" will cover all platforms we need. Thanks, -Toshi
[toc] | [prev] | [next] | [standalone]
| From | Borislav Petkov <bp@alien8.de> |
|---|---|
| Date | 2017-07-20 06:20 +0200 |
| Subject | Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac |
| Message-ID | <u5b6p-1Y8-7@gated-at.bofh.it> |
| In reply to | #1692029 |
On Wed, Jul 19, 2017 at 04:56:17PM +0000, Kani, Toshimitsu wrote:
> Since ghes_edac has not been used for a long time, I have a feeling
> that not so many vendors want to use it. In the case of HPE, we do not
> need to update with each platform since "HPE" "Server" will cover all
> platforms we need.
Does the apei_osc_setup() detection with the uuid work on HP systems?
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
[toc] | [prev] | [next] | [standalone]
| From | "Kani, Toshimitsu" <toshi.kani@hpe.com> |
|---|---|
| Date | 2017-07-20 16:50 +0200 |
| Subject | Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac |
| Message-ID | <u5kW6-lk-19@gated-at.bofh.it> |
| In reply to | #1692396 |
On Thu, 2017-07-20 at 06:16 +0200, Borislav Petkov wrote: > On Wed, Jul 19, 2017 at 04:56:17PM +0000, Kani, Toshimitsu wrote: > > Since ghes_edac has not been used for a long time, I have a feeling > > that not so many vendors want to use it. In the case of HPE, we do > > not need to update with each platform since "HPE" "Server" will > > cover all platforms we need. > > Does the apei_osc_setup() detection with the uuid work on HP systems? Yes, the following message is shown on HP systems. Please note that WHEA is a Windows-defined interface. "GHES: APEI firmware first mode is enabled by APEI bit and WHEA _OSC." Thanks, -Toshi
[toc] | [prev] | [next] | [standalone]
| From | Borislav Petkov <bp@alien8.de> |
|---|---|
| Date | 2017-07-20 17:10 +0200 |
| Subject | Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac |
| Message-ID | <u5lfr-JN-5@gated-at.bofh.it> |
| In reply to | #1692994 |
On Thu, Jul 20, 2017 at 02:42:25PM +0000, Kani, Toshimitsu wrote:
> Yes, the following message is shown on HP systems. Please note that
> WHEA is a Windows-defined interface.
Ok, so let's couple ghes_edac loading to that and see how far we could
go. I guess we should add checks for that to the major x86 EDAC drivers
to not load and this way ghes_edac will be the only driver loading.
Tony, how does that sound?
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
[toc] | [prev] | [next] | [standalone]
| From | "Luck, Tony" <tony.luck@intel.com> |
|---|---|
| Date | 2017-07-20 19:00 +0200 |
| Subject | RE: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac |
| Message-ID | <u5mXU-1BO-9@gated-at.bofh.it> |
| In reply to | #1693010 |
>> Yes, the following message is shown on HP systems. Please note that >> WHEA is a Windows-defined interface. > > Ok, so let's couple ghes_edac loading to that and see how far we could > go. I guess we should add checks for that to the major x86 EDAC drivers > to not load and this way ghes_edac will be the only driver loading. > > Tony, how does that sound? Add a module parameter to those edac drivers that can override the check and let them load anyway. I'm not paranoid, I just assume that there is a BIOS out there that sets the OSC/WHEA bits, but isn't generating useful GHES logs. -Tony
[toc] | [prev] | [next] | [standalone]
| From | Borislav Petkov <bp@alien8.de> |
|---|---|
| Date | 2017-07-20 19:10 +0200 |
| Subject | Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac |
| Message-ID | <u5n7z-1TV-1@gated-at.bofh.it> |
| In reply to | #1693095 |
On Thu, Jul 20, 2017 at 04:55:59PM +0000, Luck, Tony wrote:
> Add a module parameter to those edac drivers that can override the check
> and let them load anyway. I'm not paranoid, I just assume that there is a BIOS
> out there that sets the OSC/WHEA bits, but isn't generating useful GHES logs.
Or add that parameter to edac_core.ko and let it control which EDAC
driver gets loaded? Something like
edac=ignore_ghes
or so. And then the other EDAC drivers query it.
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
[toc] | [prev] | [next] | [standalone]
| From | "Luck, Tony" <tony.luck@intel.com> |
|---|---|
| Date | 2017-07-20 19:20 +0200 |
| Subject | RE: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac |
| Message-ID | <u5nhg-1X1-13@gated-at.bofh.it> |
| In reply to | #1693097 |
> Or add that parameter to edac_core.ko and let it control which EDAC > driver gets loaded? Something like > > edac=ignore_ghes > > or so. And then the other EDAC drivers query it. Sure ... one central place is better than adding code to each driver. -Tony
[toc] | [prev] | [next] | [standalone]
| From | Mauro Carvalho Chehab <mchehab@s-opensource.com> |
|---|---|
| Date | 2017-07-20 20:20 +0200 |
| Subject | Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac |
| Message-ID | <u5odj-2wG-11@gated-at.bofh.it> |
| In reply to | #1693097 |
Em Thu, 20 Jul 2017 19:05:04 +0200 Borislav Petkov <bp@alien8.de> escreveu: > On Thu, Jul 20, 2017 at 04:55:59PM +0000, Luck, Tony wrote: > > Add a module parameter to those edac drivers that can override the check > > and let them load anyway. I'm not paranoid, I just assume that there is a BIOS > > out there that sets the OSC/WHEA bits, but isn't generating useful GHES logs. > > Or add that parameter to edac_core.ko and let it control which EDAC > driver gets loaded? Something like > > edac=ignore_ghes > > or so. And then the other EDAC drivers query it. Works for me. Thanks, Mauro
[toc] | [prev] | [next] | [standalone]
| From | Aristeu Rozanski <aris@redhat.com> |
|---|---|
| Date | 2017-07-19 21:00 +0200 |
| Subject | Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac |
| Message-ID | <u52mv-4bx-25@gated-at.bofh.it> |
| In reply to | #1692014 |
On Wed, Jul 19, 2017 at 06:22:04PM +0200, Borislav Petkov wrote: > On Wed, Jul 19, 2017 at 04:10:07PM +0000, Kani, Toshimitsu wrote: > > I do prefer to avoid any white / black listing. But I do not see how > > it solves the buggy DMI/SMBIOS info as an example of firmware bugs we > > may have to deal with. > > So how do you want to deal with this? > > Maintain an evergrowing whitelist of platforms which are OK and then the > moment a new platform comes along, you send a patch to add it to that > whitelist? That would also need to keep an eye on versions. A newer version of BIOS on a whitelisted platform might be broken. -- Aristeu
[toc] | [prev] | [next] | [standalone]
| From | "Kani, Toshimitsu" <toshi.kani@hpe.com> |
|---|---|
| Date | 2017-07-19 22:20 +0200 |
| Subject | Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac |
| Message-ID | <u53BT-5c3-1@gated-at.bofh.it> |
| In reply to | #1692110 |
On Wed, 2017-07-19 at 14:55 -0400, Aristeu Rozanski wrote: > On Wed, Jul 19, 2017 at 06:22:04PM +0200, Borislav Petkov wrote: > > On Wed, Jul 19, 2017 at 04:10:07PM +0000, Kani, Toshimitsu wrote: > > > I do prefer to avoid any white / black listing. But I do not see > > > how > > > it solves the buggy DMI/SMBIOS info as an example of firmware > > > bugs we > > > may have to deal with. > > > > So how do you want to deal with this? > > > > Maintain an evergrowing whitelist of platforms which are OK and > > then the moment a new platform comes along, you send a patch to add > > it to that whitelist? > > That would also need to keep an eye on versions. A newer version of > BIOS on a whitelisted platform might be broken. Right. I think a question comes to who broke a running system -- OS update or BIOS update. This whitelist attempts to protect the former case by not introducing ghes_edac on arbitrary platforms. The latter case should be vendor's responsibility. Thanks, -Toshi
[toc] | [prev] | [next] | [standalone]
| From | Borislav Petkov <bp@alien8.de> |
|---|---|
| Date | 2017-07-20 06:30 +0200 |
| Subject | Re: [PATCH 3/3] ghes_edac: add platform check to enable ghes_edac |
| Message-ID | <u5bg6-24G-3@gated-at.bofh.it> |
| In reply to | #1692110 |
On Wed, Jul 19, 2017 at 02:55:08PM -0400, Aristeu Rozanski wrote:
> That would also need to keep an eye on versions. A newer version of BIOS
> on a whitelisted platform might be broken.
Yeah, that would be a nasty, back-stabbing SNAFU.
So I'm thinking of adding a bunch of FW_ERR sanity checks to that whole
ghes_edac and ghes init code to hopefully catch issues during platform
validation. I.e., early enough for them to get fixed.
But that's the same problem as with UEFI - vendors need to try to boot
Linux on their platforms early enough.
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
[toc] | [prev] | [next] | [standalone]
Page 1 of 4 [1] 2 3 4 Next page →
Back to top | Article view | linux.kernel
csiph-web