2022-08-31 08:01:16

by Justin He

Subject: [PATCH v4 4/8] ghes: Introduce a helper ghes_edac_preferred()

Introduce a flag ghes_present to differentiate between the system ROM
really not offering GHES vs. the ghes module not running. ghes-present
is latched on ghes_probe() and never unlatched since ACPI GHES table
is static.

Introduce a helper ghes_edac_preferred() and it is true if the
platform-check passes.

Suggested-by: Toshi Kani <[email protected]>
Signed-off-by: Jia He <[email protected]>
---
drivers/acpi/apei/ghes.c | 29 +++++++++++++++++++++++++++++
include/acpi/ghes.h | 2 ++
2 files changed, 31 insertions(+)

diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index 6bc9c78f916b..d798a58fbbd9 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -117,6 +117,8 @@ bool ghes_edac_force_load;
module_param_named(edac_force_load, ghes_edac_force_load, bool, 0);
EXPORT_SYMBOL(ghes_edac_force_load);

+static bool ghes_present;
+
/*
* All error sources notified with HED (Hardware Error Device) share a
* single notifier callback, so they need to be linked and checked one
@@ -1388,6 +1390,8 @@ static int ghes_probe(struct platform_device *ghes_dev)

ghes_edac_register(ghes, &ghes_dev->dev);

+ ghes_present = true;
+
/* Handle any pending errors right away */
spin_lock_irqsave(&ghes_notify_lock_irq, flags);
ghes_proc(ghes);
@@ -1511,6 +1515,31 @@ void __init acpi_ghes_init(void)
pr_info(GHES_PFX "Failed to enable APEI firmware first mode.\n");
}

+/*
+ * Known x86 systems that prefer GHES error reporting:
+ */
+static struct acpi_platform_list plat_list[] = {
+ {"HPE ", "Server ", 0, ACPI_SIG_FADT, all_versions},
+ { } /* End */
+};
+
+bool ghes_edac_preferred(void)
+{
+ int idx = -1;
+
+ if (!ghes_present)
+ return false;
+
+ if (IS_ENABLED(CONFIG_X86)) {
+ idx = acpi_match_platform_list(plat_list);
+ if (idx < 0 && !ghes_edac_force_load)
+ return false;
+ }
+
+ return true;
+}
+EXPORT_SYMBOL_GPL(ghes_edac_preferred);
+
void ghes_register_report_chain(struct notifier_block *nb)
{
atomic_notifier_chain_register(&ghes_report_chain, nb);
diff --git a/include/acpi/ghes.h b/include/acpi/ghes.h
index 5cbd38b6e4e1..7ce91c0ff3eb 100644
--- a/include/acpi/ghes.h
+++ b/include/acpi/ghes.h
@@ -80,6 +80,7 @@ int ghes_edac_register(struct ghes *ghes, struct device *dev);

void ghes_edac_unregister(struct ghes *ghes);

+bool ghes_edac_preferred(void);
#else
static inline int ghes_edac_register(struct ghes *ghes, struct device *dev)
{
@@ -89,6 +90,7 @@ static inline int ghes_edac_register(struct ghes *ghes, struct device *dev)
static inline void ghes_edac_unregister(struct ghes *ghes)
{
}
+static bool ghes_edac_preferred(void) { return false; }
#endif

static inline int acpi_hest_get_version(struct acpi_hest_generic_data *gdata)
--
2.25.1

2022-09-02 15:55:50

by Kani, Toshimitsu

Subject: RE: [PATCH v4 4/8] ghes: Introduce a helper ghes_edac_preferred()

On Wednesday, August 31, 2022 1:40 AM, Jia He wrote:
> Introduce a flag ghes_present to differentiate between the system ROM
> really not offering GHES vs. the ghes module not running. ghes-present
> is latched on ghes_probe() and never unlatched since ACPI GHES table
> is static.
>
> Introduce a helper ghes_edac_preferred() and it is true if the
> platform-check passes.

I'd suggest to rephase a bit, something like:
==
Introduce a helper ghes_edac_preferred(), which returns true when
ACPI GHES table is present and the platform-check passes on the system.
ghes_present is set once in ghes_probe() since ACPI GHES table is static.
==

Otherwise, the patch looks good to me.
Reviewed-by: Toshi Kani <[email protected]>

Toshi