2022-09-29 02:59:13

by Justin He

[permalink] [raw]
Subject: [PATCH v7 0/8] Make ghes_edac a proper module

Commit dc4e8c07e9e2 ("ACPI: APEI: explicit init of HEST and GHES in
apci_init()") introduced a bug that ghes_edac_register() would be invoked
before edac_init(). Because at that time, the bus "edac" hasn't been even
registered, this created sysfs /devices/mc0 instead of
/sys/devices/system/edac/mc/mc0 on an Ampere eMag server.

The proper solution is to make ghes_edac a proper module.

Changelog:
v7:
- remove the ghes_edac_preferred and ghes_present (suggested by Borislav)
- adjust the patch splitting, no major functional changes
- remove the r-b tag in those changed patches
v6:https://www.spinics.net/lists/kernel/msg4511453.html
- no code changes from v5 patches
- add the reviewed and acked by from Toshi
- describe the removal of ghes_edac_force_enable checking in Patch 05
v5: https://www.spinics.net/lists/kernel/msg4502787.html
- add the review-by from Toshi for patch 04 and 06
- refine the commit msg
- remove the unconditional set of ghes_edac_force_enable on Arm
v4: https://lore.kernel.org/lkml/[email protected]/
- move the kernel boot option to ghes module parameter
- collapse th ghes_present and ghes_edac_preferred into one patch
v3: https://lore.kernel.org/lkml/[email protected]/
- refine the commit logs
- introduce ghes preferred and present flag (by Toshi)
- move force_load to setup parameter
- add the ghes_edac_preferred() check for x86/Arm edac drivers
v2: https://lore.kernel.org/lkml/[email protected]/
- add acked-by tag of Patch 1 from Ard
- split the notifier patch
- add 2 patch to get regular drivers selected when ghes edac is not loaded
- fix an errno in igen6 driver
- add a patch to fix the sparse warning of ghes
- refine the commit logs
v1: https://lore.kernel.org/lkml/[email protected]/

Jia He (8):
efi/cper: export several helpers for ghes_edac to use
EDAC/ghes: Add a notifier for reporting memory errors
EDAC:ghes: Move ghes_edac.force_load to ghes module parameter
ghes: Introduce a helper ghes_get_devices()
EDAC/ghes: Make ghes_edac a proper module to remove the dependency on
ghes
EDAC: Add the ghes_get_devices() check for chipset-specific edac
drivers
apei/ghes: Use unrcu_pointer for cmpxchg
EDAC/igen6: Return consistent errno when another edac driver is
enabled

drivers/acpi/apei/ghes.c | 70 +++++++++++++++++++++++++---
drivers/edac/Kconfig | 4 +-
drivers/edac/amd64_edac.c | 3 ++
drivers/edac/armada_xp_edac.c | 3 ++
drivers/edac/edac_module.h | 1 +
drivers/edac/ghes_edac.c | 83 ++++++++++++++++++++++------------
drivers/edac/i10nm_base.c | 3 ++
drivers/edac/igen6_edac.c | 5 +-
drivers/edac/layerscape_edac.c | 3 ++
drivers/edac/pnd2_edac.c | 3 ++
drivers/edac/sb_edac.c | 3 ++
drivers/edac/skx_base.c | 3 ++
drivers/edac/thunderx_edac.c | 3 ++
drivers/edac/xgene_edac.c | 3 ++
drivers/firmware/efi/cper.c | 3 ++
include/acpi/apei.h | 2 +
include/acpi/ghes.h | 34 ++++----------
17 files changed, 166 insertions(+), 63 deletions(-)

--
2.25.1


2022-09-29 02:59:41

by Justin He

[permalink] [raw]
Subject: [PATCH v7 6/8] EDAC: Add the ghes_get_devices() check for chipset-specific edac drivers

Add ghes_get_devices() check for chipset-specific edac drivers to ensure
that ghes_edac is used on the platform where ghes_edac is preferred over
chipset-specific edac driver.

Unlike the existing edac_get_owner() check, the ghes_get_devices()
check works independent to the module_init ordering.

Suggested-by: Toshi Kani <[email protected]>
Signed-off-by: Jia He <[email protected]>
---
drivers/edac/amd64_edac.c | 3 +++
drivers/edac/armada_xp_edac.c | 3 +++
drivers/edac/edac_module.h | 1 +
drivers/edac/i10nm_base.c | 3 +++
drivers/edac/igen6_edac.c | 3 +++
drivers/edac/layerscape_edac.c | 3 +++
drivers/edac/pnd2_edac.c | 3 +++
drivers/edac/sb_edac.c | 3 +++
drivers/edac/skx_base.c | 3 +++
drivers/edac/thunderx_edac.c | 3 +++
drivers/edac/xgene_edac.c | 3 +++
11 files changed, 31 insertions(+)

diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c
index 2f854feeeb23..e3318e5575a3 100644
--- a/drivers/edac/amd64_edac.c
+++ b/drivers/edac/amd64_edac.c
@@ -4329,6 +4329,9 @@ static int __init amd64_edac_init(void)
int err = -ENODEV;
int i;

+ if (ghes_get_devices())
+ return -EBUSY;
+
owner = edac_get_owner();
if (owner && strncmp(owner, EDAC_MOD_STR, sizeof(EDAC_MOD_STR)))
return -EBUSY;
diff --git a/drivers/edac/armada_xp_edac.c b/drivers/edac/armada_xp_edac.c
index 038abbb83f4b..c4bd2fb9c46b 100644
--- a/drivers/edac/armada_xp_edac.c
+++ b/drivers/edac/armada_xp_edac.c
@@ -599,6 +599,9 @@ static int __init armada_xp_edac_init(void)
{
int res;

+ if (ghes_get_devices())
+ return -EBUSY;
+
/* only polling is supported */
edac_op_state = EDAC_OPSTATE_POLL;

diff --git a/drivers/edac/edac_module.h b/drivers/edac/edac_module.h
index 96f6de0c8ff6..3826f82de487 100644
--- a/drivers/edac/edac_module.h
+++ b/drivers/edac/edac_module.h
@@ -11,6 +11,7 @@
#ifndef __EDAC_MODULE_H__
#define __EDAC_MODULE_H__

+#include <acpi/ghes.h>
#include "edac_mc.h"
#include "edac_pci.h"
#include "edac_device.h"
diff --git a/drivers/edac/i10nm_base.c b/drivers/edac/i10nm_base.c
index 6cf50ee0b77c..75211ee4cd12 100644
--- a/drivers/edac/i10nm_base.c
+++ b/drivers/edac/i10nm_base.c
@@ -548,6 +548,9 @@ static int __init i10nm_init(void)

edac_dbg(2, "\n");

+ if (ghes_get_devices())
+ return -EBUSY;
+
owner = edac_get_owner();
if (owner && strncmp(owner, EDAC_MOD_STR, sizeof(EDAC_MOD_STR)))
return -EBUSY;
diff --git a/drivers/edac/igen6_edac.c b/drivers/edac/igen6_edac.c
index a07bbfd075d0..d33c666221f9 100644
--- a/drivers/edac/igen6_edac.c
+++ b/drivers/edac/igen6_edac.c
@@ -1271,6 +1271,9 @@ static int __init igen6_init(void)

edac_dbg(2, "\n");

+ if (ghes_get_devices())
+ return -EBUSY;
+
owner = edac_get_owner();
if (owner && strncmp(owner, EDAC_MOD_STR, sizeof(EDAC_MOD_STR)))
return -ENODEV;
diff --git a/drivers/edac/layerscape_edac.c b/drivers/edac/layerscape_edac.c
index 94cac7686a56..35ceaca578e1 100644
--- a/drivers/edac/layerscape_edac.c
+++ b/drivers/edac/layerscape_edac.c
@@ -38,6 +38,9 @@ static int __init fsl_ddr_mc_init(void)
{
int res;

+ if (ghes_get_devices())
+ return -EBUSY;
+
/* make sure error reporting method is sane */
switch (edac_op_state) {
case EDAC_OPSTATE_POLL:
diff --git a/drivers/edac/pnd2_edac.c b/drivers/edac/pnd2_edac.c
index a20b299f1202..2b306f2cc605 100644
--- a/drivers/edac/pnd2_edac.c
+++ b/drivers/edac/pnd2_edac.c
@@ -1528,6 +1528,9 @@ static int __init pnd2_init(void)

edac_dbg(2, "\n");

+ if (ghes_get_devices())
+ return -EBUSY;
+
owner = edac_get_owner();
if (owner && strncmp(owner, EDAC_MOD_STR, sizeof(EDAC_MOD_STR)))
return -EBUSY;
diff --git a/drivers/edac/sb_edac.c b/drivers/edac/sb_edac.c
index 9678ab97c7ac..2c860adf54a7 100644
--- a/drivers/edac/sb_edac.c
+++ b/drivers/edac/sb_edac.c
@@ -3506,6 +3506,9 @@ static int __init sbridge_init(void)

edac_dbg(2, "\n");

+ if (ghes_get_devices())
+ return -EBUSY;
+
owner = edac_get_owner();
if (owner && strncmp(owner, EDAC_MOD_STR, sizeof(EDAC_MOD_STR)))
return -EBUSY;
diff --git a/drivers/edac/skx_base.c b/drivers/edac/skx_base.c
index 1abc020d49ab..80a7334111b1 100644
--- a/drivers/edac/skx_base.c
+++ b/drivers/edac/skx_base.c
@@ -653,6 +653,9 @@ static int __init skx_init(void)

edac_dbg(2, "\n");

+ if (ghes_get_devices())
+ return -EBUSY;
+
owner = edac_get_owner();
if (owner && strncmp(owner, EDAC_MOD_STR, sizeof(EDAC_MOD_STR)))
return -EBUSY;
diff --git a/drivers/edac/thunderx_edac.c b/drivers/edac/thunderx_edac.c
index f13674081cb6..0bcd9f02c84a 100644
--- a/drivers/edac/thunderx_edac.c
+++ b/drivers/edac/thunderx_edac.c
@@ -2114,6 +2114,9 @@ static int __init thunderx_edac_init(void)
{
int rc = 0;

+ if (ghes_get_devices())
+ return -EBUSY;
+
rc = pci_register_driver(&thunderx_lmc_driver);
if (rc)
return rc;
diff --git a/drivers/edac/xgene_edac.c b/drivers/edac/xgene_edac.c
index 54081403db4f..c52b9dd9154c 100644
--- a/drivers/edac/xgene_edac.c
+++ b/drivers/edac/xgene_edac.c
@@ -2004,6 +2004,9 @@ static int __init xgene_edac_init(void)
{
int rc;

+ if (ghes_get_devices())
+ return -EBUSY;
+
/* Make sure error reporting method is sane */
switch (edac_op_state) {
case EDAC_OPSTATE_POLL:
--
2.25.1

2022-09-29 03:00:28

by Justin He

[permalink] [raw]
Subject: [PATCH v7 7/8] apei/ghes: Use unrcu_pointer for cmpxchg

ghes_estatus_caches should be add rcu annotation to avoid sparse warnings.
drivers/acpi/apei/ghes.c:733:25: sparse: sparse: incompatible types in comparison expression (different address spaces):
drivers/acpi/apei/ghes.c:733:25: sparse: struct ghes_estatus_cache [noderef] __rcu *
drivers/acpi/apei/ghes.c:733:25: sparse: struct ghes_estatus_cache *
drivers/acpi/apei/ghes.c:813:25: sparse: sparse: incompatible types in comparison expression (different address spaces):
drivers/acpi/apei/ghes.c:813:25: sparse: struct ghes_estatus_cache [noderef] __rcu *
drivers/acpi/apei/ghes.c:813:25: sparse: struct ghes_estatus_cache *

unrcu_pointer is to strip the __rcu in cmpxchg.

Reported-by: kernel test robot <[email protected]>
Signed-off-by: Jia He <[email protected]>
---
drivers/acpi/apei/ghes.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index 11a503f2260d..803e9d9867ca 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -151,7 +151,7 @@ struct ghes_vendor_record_entry {
static struct gen_pool *ghes_estatus_pool;
static unsigned long ghes_estatus_pool_size_request;

-static struct ghes_estatus_cache *ghes_estatus_caches[GHES_ESTATUS_CACHES_SIZE];
+static struct ghes_estatus_cache __rcu *ghes_estatus_caches[GHES_ESTATUS_CACHES_SIZE];
static atomic_t ghes_estatus_cache_alloced;

static int ghes_panic_timeout __read_mostly = 30;
@@ -841,8 +841,9 @@ static void ghes_estatus_cache_add(
}
/* new_cache must be put into array after its contents are written */
smp_wmb();
- if (slot != -1 && cmpxchg(ghes_estatus_caches + slot,
- slot_cache, new_cache) == slot_cache) {
+ if (slot != -1 && unrcu_pointer(cmpxchg(ghes_estatus_caches + slot,
+ RCU_INITIALIZER(slot_cache),
+ RCU_INITIALIZER(new_cache))) == slot_cache) {
if (slot_cache)
call_rcu(&slot_cache->rcu, ghes_estatus_cache_rcu_free);
} else
--
2.25.1

2022-09-29 03:02:18

by Justin He

[permalink] [raw]
Subject: [PATCH v7 3/8] EDAC:ghes: Move ghes_edac.force_load to ghes module parameter

ghes_edac_register() is too late to set this module flag ghes_edac.force_load.
Also, other edac drivers should not be able to control this flag.

Move this flag to the module parameter in ghes instead.

Suggested-by: Toshi Kani <[email protected]>
Signed-off-by: Jia He <[email protected]>
Reviewed-by: Toshi Kani <[email protected]>
---
drivers/acpi/apei/ghes.c | 8 ++++++++
drivers/edac/ghes_edac.c | 10 +++-------
include/acpi/apei.h | 2 ++
3 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index 8cb65f757d06..b0a6445c6da2 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -109,6 +109,14 @@ static inline bool is_hest_type_generic_v2(struct ghes *ghes)
bool ghes_disable;
module_param_named(disable, ghes_disable, bool, 0);

+/*
+ * "ghes.edac_force_enable" forcibly enables ghes_edac and skips the platform
+ * check.
+ */
+bool ghes_edac_force_enable;
+EXPORT_SYMBOL(ghes_edac_force_enable);
+module_param_named(edac_force_enable, ghes_edac_force_enable, bool, 0);
+
/*
* All error sources notified with HED (Hardware Error Device) share a
* single notifier callback, so they need to be linked and checked one
diff --git a/drivers/edac/ghes_edac.c b/drivers/edac/ghes_edac.c
index 7b8d56a769f6..11a1b5e7e484 100644
--- a/drivers/edac/ghes_edac.c
+++ b/drivers/edac/ghes_edac.c
@@ -54,10 +54,6 @@ static DEFINE_MUTEX(ghes_reg_mutex);
*/
static DEFINE_SPINLOCK(ghes_lock);

-/* "ghes_edac.force_load=1" skips the platform check */
-static bool __read_mostly force_load;
-module_param(force_load, bool, 0);
-
static bool system_scanned;

/* Memory Device - Type 17 of SMBIOS spec */
@@ -408,10 +404,10 @@ int ghes_edac_register(struct ghes *ghes, struct device *dev)
if (IS_ENABLED(CONFIG_X86)) {
/* Check if safe to enable on this system */
idx = acpi_match_platform_list(plat_list);
- if (!force_load && idx < 0)
+ if (!ghes_edac_force_enable && idx < 0)
return -ENODEV;
} else {
- force_load = true;
+ ghes_edac_force_enable = true;
idx = 0;
}

@@ -535,7 +531,7 @@ void ghes_edac_unregister(struct ghes *ghes)
struct mem_ctl_info *mci;
unsigned long flags;

- if (!force_load)
+ if (!ghes_edac_force_enable)
return;

mutex_lock(&ghes_reg_mutex);
diff --git a/include/acpi/apei.h b/include/acpi/apei.h
index dc60f7db5524..ab310393766e 100644
--- a/include/acpi/apei.h
+++ b/include/acpi/apei.h
@@ -27,9 +27,11 @@ extern int hest_disable;
extern int erst_disable;
#ifdef CONFIG_ACPI_APEI_GHES
extern bool ghes_disable;
+extern bool ghes_edac_force_enable;
void __init acpi_ghes_init(void);
#else
#define ghes_disable 1
+#define ghes_edac_force_enable 0
static inline void acpi_ghes_init(void) { }
#endif

--
2.25.1

2022-09-29 03:15:36

by Justin He

[permalink] [raw]
Subject: [PATCH v7 4/8] ghes: Introduce a helper ghes_get_devices()

Introduce a helper ghes_get_devices(), which returns the dev list GHES
probed when the platform-check passes on the system.

Suggested-by: Borislav Petkov <[email protected]>
Signed-off-by: Jia He <[email protected]>
---
drivers/acpi/apei/ghes.c | 37 +++++++++++++++++++++++++++++++++++++
include/acpi/ghes.h | 6 ++++++
2 files changed, 43 insertions(+)

diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index b0a6445c6da2..cc0588fe20f5 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -128,6 +128,9 @@ module_param_named(edac_force_enable, ghes_edac_force_enable, bool, 0);
static LIST_HEAD(ghes_hed);
static DEFINE_MUTEX(ghes_list_mutex);

+static LIST_HEAD(ghes_devs);
+static DEFINE_MUTEX(ghes_devs_mutex);
+
/*
* Because the memory area used to transfer hardware error information
* from BIOS to Linux can be determined only in NMI, IRQ or timer
@@ -1388,6 +1391,10 @@ static int ghes_probe(struct platform_device *ghes_dev)

ghes_edac_register(ghes, &ghes_dev->dev);

+ mutex_lock(&ghes_devs_mutex);
+ list_add_tail(&ghes->elist, &ghes_devs);
+ mutex_unlock(&ghes_devs_mutex);
+
/* Handle any pending errors right away */
spin_lock_irqsave(&ghes_notify_lock_irq, flags);
ghes_proc(ghes);
@@ -1452,6 +1459,10 @@ static int ghes_remove(struct platform_device *ghes_dev)

ghes_edac_unregister(ghes);

+ mutex_lock(&ghes_devs_mutex);
+ list_del(&ghes->elist);
+ mutex_unlock(&ghes_devs_mutex);
+
kfree(ghes);

platform_set_drvdata(ghes_dev, NULL);
@@ -1508,6 +1519,32 @@ 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 */
+};
+
+struct list_head *ghes_get_devices(void)
+{
+ int idx = -1;
+
+ if (IS_ENABLED(CONFIG_X86)) {
+ idx = acpi_match_platform_list(plat_list);
+ if (idx < 0) {
+ if (!ghes_edac_force_enable)
+ return NULL;
+
+ pr_warn_once("Force-loading ghes_edac on an unsupported platform. You're on your own!\n");
+ }
+ }
+
+ return &ghes_devs;
+}
+EXPORT_SYMBOL_GPL(ghes_get_devices);
+
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..46d9c86199e9 100644
--- a/include/acpi/ghes.h
+++ b/include/acpi/ghes.h
@@ -27,6 +27,8 @@ struct ghes {
struct timer_list timer;
unsigned int irq;
};
+ struct device *dev;
+ struct list_head elist;
};

struct ghes_estatus_node {
@@ -69,6 +71,10 @@ int ghes_register_vendor_record_notifier(struct notifier_block *nb);
* @nb: pointer to the notifier_block structure of the vendor record handler.
*/
void ghes_unregister_vendor_record_notifier(struct notifier_block *nb);
+
+struct list_head *ghes_get_devices(void);
+#else
+static inline struct list_head *ghes_get_devices(void) { return NULL; }
#endif

int ghes_estatus_pool_init(int num_ghes);
--
2.25.1

2022-09-29 03:32:14

by Justin He

[permalink] [raw]
Subject: [PATCH v7 8/8] EDAC/igen6: Return consistent errno when another edac driver is enabled

Only a single edac driver can be enabled for EDAC MC. The igen6_init()
should be returned with EBUSY instead of ENODEV, which is consistent with
other edac drivers.

Signed-off-by: Jia He <[email protected]>
---
drivers/edac/igen6_edac.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/edac/igen6_edac.c b/drivers/edac/igen6_edac.c
index d33c666221f9..544dd19072ea 100644
--- a/drivers/edac/igen6_edac.c
+++ b/drivers/edac/igen6_edac.c
@@ -1276,7 +1276,7 @@ static int __init igen6_init(void)

owner = edac_get_owner();
if (owner && strncmp(owner, EDAC_MOD_STR, sizeof(EDAC_MOD_STR)))
- return -ENODEV;
+ return -EBUSY;

edac_op_state = EDAC_OPSTATE_NMI;

--
2.25.1

2022-10-05 15:54:45

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH v7 3/8] EDAC:ghes: Move ghes_edac.force_load to ghes module parameter

On Thu, Sep 29, 2022 at 02:37:21AM +0000, Jia He wrote:
> ghes_edac_register() is too late to set this module flag ghes_edac.force_load.
> Also, other edac drivers should not be able to control this flag.
>
> Move this flag to the module parameter in ghes instead.
>
> Suggested-by: Toshi Kani <[email protected]>
> Signed-off-by: Jia He <[email protected]>
> Reviewed-by: Toshi Kani <[email protected]>
> ---
> drivers/acpi/apei/ghes.c | 8 ++++++++
> drivers/edac/ghes_edac.c | 10 +++-------
> include/acpi/apei.h | 2 ++
> 3 files changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> index 8cb65f757d06..b0a6445c6da2 100644
> --- a/drivers/acpi/apei/ghes.c
> +++ b/drivers/acpi/apei/ghes.c
> @@ -109,6 +109,14 @@ static inline bool is_hest_type_generic_v2(struct ghes *ghes)
> bool ghes_disable;
> module_param_named(disable, ghes_disable, bool, 0);
>
> +/*
> + * "ghes.edac_force_enable" forcibly enables ghes_edac and skips the platform
> + * check.
> + */
> +bool ghes_edac_force_enable;
> +EXPORT_SYMBOL(ghes_edac_force_enable);
> +module_param_named(edac_force_enable, ghes_edac_force_enable, bool, 0);

Why is this exported?

In the exemplary patch I sent you, that thing is static.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2022-10-08 09:02:30

by Justin He

[permalink] [raw]
Subject: RE: [PATCH v7 3/8] EDAC:ghes: Move ghes_edac.force_load to ghes module parameter



> -----Original Message-----
> From: Borislav Petkov <[email protected]>
> Sent: Wednesday, October 5, 2022 11:14 PM
> To: Justin He <[email protected]>
> Cc: Len Brown <[email protected]>; James Morse <[email protected]>;
> Tony Luck <[email protected]>; Mauro Carvalho Chehab
> <[email protected]>; Robert Richter <[email protected]>; Robert Moore
> <[email protected]>; Qiuxu Zhuo <[email protected]>; Yazen
> Ghannam <[email protected]>; Jan Luebbe <[email protected]>;
> Khuong Dinh <[email protected]>; Kani Toshi
> <[email protected]>; Ard Biesheuvel <[email protected]>;
> [email protected]; [email protected];
> [email protected]; [email protected]; Rafael J . Wysocki
> <[email protected]>; Shuai Xue <[email protected]>; Jarkko
> Sakkinen <[email protected]>; [email protected]; nd <[email protected]>
> Subject: Re: [PATCH v7 3/8] EDAC:ghes: Move ghes_edac.force_load to ghes
> module parameter
>
> On Thu, Sep 29, 2022 at 02:37:21AM +0000, Jia He wrote:
> > ghes_edac_register() is too late to set this module flag ghes_edac.force_load.
> > Also, other edac drivers should not be able to control this flag.
> >
> > Move this flag to the module parameter in ghes instead.
> >
> > Suggested-by: Toshi Kani <[email protected]>
> > Signed-off-by: Jia He <[email protected]>
> > Reviewed-by: Toshi Kani <[email protected]>
> > ---
> > drivers/acpi/apei/ghes.c | 8 ++++++++ drivers/edac/ghes_edac.c | 10
> > +++-------
> > include/acpi/apei.h | 2 ++
> > 3 files changed, 13 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c index
> > 8cb65f757d06..b0a6445c6da2 100644
> > --- a/drivers/acpi/apei/ghes.c
> > +++ b/drivers/acpi/apei/ghes.c
> > @@ -109,6 +109,14 @@ static inline bool is_hest_type_generic_v2(struct
> > ghes *ghes) bool ghes_disable; module_param_named(disable,
> > ghes_disable, bool, 0);
> >
> > +/*
> > + * "ghes.edac_force_enable" forcibly enables ghes_edac and skips the
> > +platform
> > + * check.
> > + */
> > +bool ghes_edac_force_enable;
> > +EXPORT_SYMBOL(ghes_edac_force_enable);
> > +module_param_named(edac_force_enable, ghes_edac_force_enable, bool,
> > +0);
>
> Why is this exported?
>
> In the exemplary patch I sent you, that thing is static.
Sorry for the carelessness


--
Cheers,
Justin (Jia He)