| Message ID | 20220912144005.212624-6-justin.he@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | Make ghes_edac a proper module | expand |
On Mon, Sep 12, 2022 at 02:40:02PM +0000, Jia He wrote: > 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" hadn't been even > registered, this created sysfs /devices/mc0 instead of > /sys/devices/system/edac/mc/mc0 on an Ampere eMag server. > > To remove the dependency of ghes_edac on ghes, make it a proper module. Use > a list to save the probing devices in ghes_probe(), and defer the > ghes_edac_register() to module_init() of the new ghes_edac module by > iterating over the devices list. > > The ghes_edac_force_enable check is not needed in ghes_edac_unregister() > since it has been checked in ghes_edac_init(). > > Co-developed-by: Borislav Petkov <bp@alien8.de> > Signed-off-by: Borislav Petkov <bp@alien8.de> > Signed-off-by: Jia He <justin.he@arm.com> > Fixes: dc4e8c07e9e2 ("ACPI: APEI: explicit init of HEST and GHES in apci_init()") > Cc: Shuai Xue <xueshuai@linux.alibaba.com> > Acked-by: Toshi Kani <toshi.kani@hpe.com> > --- > drivers/acpi/apei/ghes.c | 22 +++++++++++++-- > drivers/edac/Kconfig | 4 +-- > drivers/edac/ghes_edac.c | 59 +++++++++++++++++++++++----------------- > include/acpi/ghes.h | 24 ++++------------ > 4 files changed, 62 insertions(+), 47 deletions(-) So those last three patches look unnecessarily complex. You don't need to export ghes_edac_force_enable and you don't need that ghes_edac_preferred() thing. IOW, I'd like to see the below split into two patches: 1. Add ghes_get_devices() to ghes.c along with moving the ghes_edac_force_enable param to ghes.c 2. Add init() and exit() module functions and fixup Kconfig There's no need for ghes_present - ghes.c has either registered devices or not. And ghes_edac is preferred if ghes_get_devices() gives you a list of devices or not. IOW, it all boils down to whether the list returned is NULL or not. HTH. --- diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c index 8cb65f757d06..27c72b175e4b 100644 --- a/drivers/acpi/apei/ghes.c +++ b/drivers/acpi/apei/ghes.c @@ -109,6 +109,13 @@ 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. + */ +static bool 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 @@ -120,6 +127,9 @@ module_param_named(disable, ghes_disable, 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 @@ -1378,7 +1388,11 @@ static int ghes_probe(struct platform_device *ghes_dev) platform_set_drvdata(ghes_dev, ghes); - ghes_edac_register(ghes, &ghes_dev->dev); + ghes->dev = &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); @@ -1442,7 +1456,9 @@ static int ghes_remove(struct platform_device *ghes_dev) ghes_fini(ghes); - ghes_edac_unregister(ghes); + mutex_lock(&ghes_devs_mutex); + list_del(&ghes->elist); + mutex_unlock(&ghes_devs_mutex); kfree(ghes); @@ -1500,6 +1516,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/drivers/edac/Kconfig b/drivers/edac/Kconfig index 17562cf1fe97..df45db81858b 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 select UEFI_CPER help Not all machines support hardware-driven error report. Some of those diff --git a/drivers/edac/ghes_edac.c b/drivers/edac/ghes_edac.c index 7b8d56a769f6..8460108e8623 100644 --- a/drivers/edac/ghes_edac.c +++ b/drivers/edac/ghes_edac.c @@ -54,12 +54,10 @@ 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; +static struct list_head *ghes_devs; + /* Memory Device - Type 17 of SMBIOS spec */ struct memdev_dmi_entry { u8 type; @@ -387,34 +385,15 @@ static struct notifier_block ghes_edac_mem_err_nb = { .priority = 0, }; -/* - * Known systems that are safe to enable this module. - */ -static struct acpi_platform_list plat_list[] = { - {"HPE ", "Server ", 0, ACPI_SIG_FADT, all_versions}, - { } /* End */ -}; - -int ghes_edac_register(struct ghes *ghes, struct device *dev) +static int ghes_edac_register(struct device *dev) { bool fake = false; struct mem_ctl_info *mci; struct ghes_pvt *pvt; struct edac_mc_layer layers[1]; unsigned long flags; - int idx = -1; int rc = 0; - 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) - return -ENODEV; - } else { - force_load = true; - idx = 0; - } - /* finish another registration/unregistration instance first */ mutex_lock(&ghes_reg_mutex); @@ -458,15 +437,10 @@ int ghes_edac_register(struct ghes *ghes, struct device *dev) 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"); - 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", ghes_hw.num_dimms); } + pr_info("This system has %d DIMM sockets.\n", ghes_hw.num_dimms); + if (!fake) { struct dimm_info *src, *dst; int i = 0; @@ -530,14 +504,11 @@ int ghes_edac_register(struct ghes *ghes, struct device *dev) return rc; } -void ghes_edac_unregister(struct ghes *ghes) +static void ghes_edac_unregister(struct ghes *ghes) { struct mem_ctl_info *mci; unsigned long flags; - if (!force_load) - return; - mutex_lock(&ghes_reg_mutex); system_scanned = false; @@ -566,3 +537,32 @@ void ghes_edac_unregister(struct ghes *ghes) unlock: mutex_unlock(&ghes_reg_mutex); } + +static int __init ghes_edac_init(void) +{ + struct ghes *g, *g_tmp; + + ghes_devs = ghes_get_devices(); + if (!ghes_devs) + return -ENODEV; + + list_for_each_entry_safe(g, g_tmp, ghes_devs, elist) { + ghes_edac_register(g->dev); + } + + return 0; +} +module_init(ghes_edac_init); + +static void __exit ghes_edac_exit(void) +{ + struct ghes *g, *g_tmp; + + list_for_each_entry_safe(g, g_tmp, ghes_devs, elist) { + ghes_edac_unregister(g); + } +} +module_exit(ghes_edac_exit); + +MODULE_LICENSE("GPL"); +MODULE_DESCRIPTION("Output ACPI APEI/GHES BIOS detected errors module via EDAC"); diff --git a/include/acpi/ghes.h b/include/acpi/ghes.h index 5cbd38b6e4e1..2e785d3554d8 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,28 +71,14 @@ 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); -#endif - -int ghes_estatus_pool_init(int num_ghes); - -/* From drivers/edac/ghes_edac.c */ - -#ifdef CONFIG_EDAC_GHES -int ghes_edac_register(struct ghes *ghes, struct device *dev); - -void ghes_edac_unregister(struct ghes *ghes); +struct list_head *ghes_get_devices(void); #else -static inline int ghes_edac_register(struct ghes *ghes, struct device *dev) -{ - return -ENODEV; -} - -static inline void ghes_edac_unregister(struct ghes *ghes) -{ -} +static inline struct list_head *ghes_get_devices(void) { return NULL; } #endif +int ghes_estatus_pool_init(int num_ghes); + static inline int acpi_hest_get_version(struct acpi_hest_generic_data *gdata) { return gdata->revision >> 8;
Hi Borislav > -----Original Message----- > From: Borislav Petkov <bp@alien8.de> > Sent: Wednesday, September 21, 2022 1:22 AM > To: Justin He <Justin.He@arm.com> > Cc: Len Brown <lenb@kernel.org>; James Morse <James.Morse@arm.com>; > Tony Luck <tony.luck@intel.com>; Mauro Carvalho Chehab > <mchehab@kernel.org>; Robert Richter <rric@kernel.org>; Robert Moore > <robert.moore@intel.com>; Qiuxu Zhuo <qiuxu.zhuo@intel.com>; Yazen > Ghannam <yazen.ghannam@amd.com>; Jan Luebbe <jlu@pengutronix.de>; > Khuong Dinh <khuong@os.amperecomputing.com>; Kani Toshi > <toshi.kani@hpe.com>; Ard Biesheuvel <ardb@kernel.org>; > linux-acpi@vger.kernel.org; linux-kernel@vger.kernel.org; > linux-edac@vger.kernel.org; devel@acpica.org; Rafael J . Wysocki > <rafael@kernel.org>; Shuai Xue <xueshuai@linux.alibaba.com>; Jarkko > Sakkinen <jarkko@kernel.org>; linux-efi@vger.kernel.org; nd <nd@arm.com> > Subject: Re: [PATCH v6 5/8] EDAC/ghes: Make ghes_edac a proper module to > remove the dependency on ghes > > On Mon, Sep 12, 2022 at 02:40:02PM +0000, Jia He wrote: > > 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" > > hadn't been even registered, this created sysfs /devices/mc0 instead > > of > > /sys/devices/system/edac/mc/mc0 on an Ampere eMag server. > > > > To remove the dependency of ghes_edac on ghes, make it a proper > > module. Use a list to save the probing devices in ghes_probe(), and > > defer the > > ghes_edac_register() to module_init() of the new ghes_edac module by > > iterating over the devices list. > > > > The ghes_edac_force_enable check is not needed in > > ghes_edac_unregister() since it has been checked in ghes_edac_init(). > > > > Co-developed-by: Borislav Petkov <bp@alien8.de> > > Signed-off-by: Borislav Petkov <bp@alien8.de> > > Signed-off-by: Jia He <justin.he@arm.com> > > Fixes: dc4e8c07e9e2 ("ACPI: APEI: explicit init of HEST and GHES in > > apci_init()") > > Cc: Shuai Xue <xueshuai@linux.alibaba.com> > > Acked-by: Toshi Kani <toshi.kani@hpe.com> > > --- > > drivers/acpi/apei/ghes.c | 22 +++++++++++++-- > > drivers/edac/Kconfig | 4 +-- > > drivers/edac/ghes_edac.c | 59 > +++++++++++++++++++++++----------------- > > include/acpi/ghes.h | 24 ++++------------ > > 4 files changed, 62 insertions(+), 47 deletions(-) > > So those last three patches look unnecessarily complex. You don't need to > export ghes_edac_force_enable and you don't need that > ghes_edac_preferred() thing. > > IOW, I'd like to see the below split into two patches: > > 1. Add ghes_get_devices() to ghes.c along with moving the > ghes_edac_force_enable param to ghes.c > > 2. Add init() and exit() module functions and fixup Kconfig > > There's no need for ghes_present - ghes.c has either registered devices or > not. > If there is no ghes_present flag. What if ghes.disable is passed to kernel boot parameter and then ghes_edac is loaded by modprobe? Thus, ghes_edac can be loaded even if ghes is disabled. (ghes_dev list is null) The ghes_present is to avoid this case. What do u think of it? -- Cheers, Justin (Jia He)
On Thu, Sep 22, 2022 at 08:26:16AM +0000, Justin He wrote: > If there is no ghes_present flag. > What if ghes.disable is passed to kernel boot parameter and then ghes_edac is > loaded by modprobe? > Thus, ghes_edac can be loaded even if ghes is disabled. (ghes_dev list is null) Yes, and what happens if ghes_dev is NULL? The other drivers would do in their init function: struct list_head *ghes_devs; ghes_devs = ghes_get_devices(); if (ghes_devs) return -ENODEV; /* Continue init */ and then load in that case because user has disabled GHES and thus no ghes_edac either. So the platform-specific one loads. Right?
Hi Borislav > -----Original Message----- > From: Borislav Petkov <bp@alien8.de> > Sent: Friday, September 23, 2022 11:43 PM > To: Justin He <Justin.He@arm.com> > Cc: Len Brown <lenb@kernel.org>; James Morse <James.Morse@arm.com>; > Tony Luck <tony.luck@intel.com>; Mauro Carvalho Chehab > <mchehab@kernel.org>; Robert Richter <rric@kernel.org>; Robert Moore > <robert.moore@intel.com>; Qiuxu Zhuo <qiuxu.zhuo@intel.com>; Yazen > Ghannam <yazen.ghannam@amd.com>; Jan Luebbe <jlu@pengutronix.de>; > Khuong Dinh <khuong@os.amperecomputing.com>; Kani Toshi > <toshi.kani@hpe.com>; Ard Biesheuvel <ardb@kernel.org>; > linux-acpi@vger.kernel.org; linux-kernel@vger.kernel.org; > linux-edac@vger.kernel.org; devel@acpica.org; Rafael J . Wysocki > <rafael@kernel.org>; Shuai Xue <xueshuai@linux.alibaba.com>; Jarkko > Sakkinen <jarkko@kernel.org>; linux-efi@vger.kernel.org; nd <nd@arm.com> > Subject: Re: [PATCH v6 5/8] EDAC/ghes: Make ghes_edac a proper module to > remove the dependency on ghes > > On Thu, Sep 22, 2022 at 08:26:16AM +0000, Justin He wrote: > > If there is no ghes_present flag. > > What if ghes.disable is passed to kernel boot parameter and then > > ghes_edac is loaded by modprobe? > > Thus, ghes_edac can be loaded even if ghes is disabled. (ghes_dev list > > is null) > > Yes, and what happens if ghes_dev is NULL? > Okay, I am fine if it is allowed to register ghes_dev with > The other drivers would do in their init function: > > struct list_head *ghes_devs; > > ghes_devs = ghes_get_devices(); > if (ghes_devs) > return -ENODEV; > > /* Continue init */ > > and then load in that case because user has disabled GHES and thus no > ghes_edac either. So the platform-specific one loads. Okay, I am fine with removing ghes_present and ghes_edac_preferred if It is ok to register empty ghes_dev list without prompting an error -- Cheers, Justin (Jia He)
On Sun, Sep 25, 2022 at 03:07:12PM +0000, Justin He wrote: > Okay, I am fine with removing ghes_present and ghes_edac_preferred if > It is ok to register empty ghes_dev list without prompting an error What "prompting an error"? In that case, you simply return a NULL list which says that GHES hasn't been initialized...
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c index 0c2dbfffe22a..4af269b22bd4 100644 --- a/drivers/acpi/apei/ghes.c +++ b/drivers/acpi/apei/ghes.c @@ -130,6 +130,9 @@ static bool ghes_present; 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,7 +1391,11 @@ static int ghes_probe(struct platform_device *ghes_dev) platform_set_drvdata(ghes_dev, ghes); - ghes_edac_register(ghes, &ghes_dev->dev); + ghes->dev = &ghes_dev->dev; + + mutex_lock(&ghes_devs_mutex); + list_add_tail(&ghes->elist, &ghes_devs); + mutex_unlock(&ghes_devs_mutex); ghes_present = true; @@ -1454,7 +1461,9 @@ static int ghes_remove(struct platform_device *ghes_dev) ghes_fini(ghes); - ghes_edac_unregister(ghes); + mutex_lock(&ghes_devs_mutex); + list_del(&ghes->elist); + mutex_unlock(&ghes_devs_mutex); kfree(ghes); @@ -1537,6 +1546,15 @@ bool ghes_edac_preferred(void) } EXPORT_SYMBOL_GPL(ghes_edac_preferred); +struct list_head *ghes_get_devices(void) +{ + if (!ghes_edac_preferred()) + return NULL; + + 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/drivers/edac/Kconfig b/drivers/edac/Kconfig index 17562cf1fe97..df45db81858b 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 select UEFI_CPER help Not all machines support hardware-driven error report. Some of those diff --git a/drivers/edac/ghes_edac.c b/drivers/edac/ghes_edac.c index 11a1b5e7e484..129665896a80 100644 --- a/drivers/edac/ghes_edac.c +++ b/drivers/edac/ghes_edac.c @@ -56,6 +56,8 @@ static DEFINE_SPINLOCK(ghes_lock); static bool system_scanned; +static struct list_head *ghes_devs; + /* Memory Device - Type 17 of SMBIOS spec */ struct memdev_dmi_entry { u8 type; @@ -383,34 +385,15 @@ static struct notifier_block ghes_edac_mem_err_nb = { .priority = 0, }; -/* - * Known systems that are safe to enable this module. - */ -static struct acpi_platform_list plat_list[] = { - {"HPE ", "Server ", 0, ACPI_SIG_FADT, all_versions}, - { } /* End */ -}; - -int ghes_edac_register(struct ghes *ghes, struct device *dev) +static int ghes_edac_register(struct device *dev) { bool fake = false; struct mem_ctl_info *mci; struct ghes_pvt *pvt; struct edac_mc_layer layers[1]; unsigned long flags; - int idx = -1; int rc = 0; - if (IS_ENABLED(CONFIG_X86)) { - /* Check if safe to enable on this system */ - idx = acpi_match_platform_list(plat_list); - if (!ghes_edac_force_enable && idx < 0) - return -ENODEV; - } else { - ghes_edac_force_enable = true; - idx = 0; - } - /* finish another registration/unregistration instance first */ mutex_lock(&ghes_reg_mutex); @@ -454,7 +437,7 @@ int ghes_edac_register(struct ghes *ghes, struct device *dev) 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) { + } else if (ghes_edac_force_enable) { 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"); @@ -526,14 +509,11 @@ int ghes_edac_register(struct ghes *ghes, struct device *dev) return rc; } -void ghes_edac_unregister(struct ghes *ghes) +static void ghes_edac_unregister(struct ghes *ghes) { struct mem_ctl_info *mci; unsigned long flags; - if (!ghes_edac_force_enable) - return; - mutex_lock(&ghes_reg_mutex); system_scanned = false; @@ -562,3 +542,32 @@ void ghes_edac_unregister(struct ghes *ghes) unlock: mutex_unlock(&ghes_reg_mutex); } + +static int __init ghes_edac_init(void) +{ + struct ghes *g, *g_tmp; + + ghes_devs = ghes_get_devices(); + if (!ghes_devs) + return -ENODEV; + + list_for_each_entry_safe(g, g_tmp, ghes_devs, elist) { + ghes_edac_register(g->dev); + } + + return 0; +} +module_init(ghes_edac_init); + +static void __exit ghes_edac_exit(void) +{ + struct ghes *g, *g_tmp; + + list_for_each_entry_safe(g, g_tmp, ghes_devs, elist) { + ghes_edac_unregister(g); + } +} +module_exit(ghes_edac_exit); + +MODULE_LICENSE("GPL"); +MODULE_DESCRIPTION("Output ACPI APEI/GHES BIOS detected errors module via EDAC"); diff --git a/include/acpi/ghes.h b/include/acpi/ghes.h index 7ce91c0ff3eb..08f12dde99ae 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,30 +71,16 @@ 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); -#endif - -int ghes_estatus_pool_init(int num_ghes); - -/* From drivers/edac/ghes_edac.c */ - -#ifdef CONFIG_EDAC_GHES -int ghes_edac_register(struct ghes *ghes, struct device *dev); - -void ghes_edac_unregister(struct ghes *ghes); +struct list_head *ghes_get_devices(void); bool ghes_edac_preferred(void); #else -static inline int ghes_edac_register(struct ghes *ghes, struct device *dev) -{ - return -ENODEV; -} - -static inline void ghes_edac_unregister(struct ghes *ghes) -{ -} +static inline struct list_head *ghes_get_devices(void) { return NULL; } static bool ghes_edac_preferred(void) { return false; } #endif +int ghes_estatus_pool_init(int num_ghes); + static inline int acpi_hest_get_version(struct acpi_hest_generic_data *gdata) { return gdata->revision >> 8;
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" hadn't been even registered, this created sysfs /devices/mc0 instead of /sys/devices/system/edac/mc/mc0 on an Ampere eMag server. To remove the dependency of ghes_edac on ghes, make it a proper module. Use a list to save the probing devices in ghes_probe(), and defer the ghes_edac_register() to module_init() of the new ghes_edac module by iterating over the devices list. The ghes_edac_force_enable check is not needed in ghes_edac_unregister() since it has been checked in ghes_edac_init(). Co-developed-by: Borislav Petkov <bp@alien8.de> Signed-off-by: Borislav Petkov <bp@alien8.de> Signed-off-by: Jia He <justin.he@arm.com> Fixes: dc4e8c07e9e2 ("ACPI: APEI: explicit init of HEST and GHES in apci_init()") Cc: Shuai Xue <xueshuai@linux.alibaba.com> Acked-by: Toshi Kani <toshi.kani@hpe.com> --- drivers/acpi/apei/ghes.c | 22 +++++++++++++-- drivers/edac/Kconfig | 4 +-- drivers/edac/ghes_edac.c | 59 +++++++++++++++++++++++----------------- include/acpi/ghes.h | 24 ++++------------ 4 files changed, 62 insertions(+), 47 deletions(-)