Messages in this thread | | | From | Justin He <> | Subject | RE: [PATCH 2/2] EDAC/ghes: Modularize ghes_edac driver to remove the dependency on ghes | Date | Mon, 15 Aug 2022 01:32:40 +0000 |
| |
Hi Boris and Toshi
> -----Original Message----- > From: Borislav Petkov <bp@alien8.de> > Sent: Friday, August 12, 2022 10:46 PM > To: Justin He <Justin.He@arm.com> > Cc: Ard Biesheuvel <ardb@kernel.org>; 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>; 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>; toshi.kani@hpe.com; > stable@kernel.org > Subject: Re: [PATCH 2/2] EDAC/ghes: Modularize ghes_edac driver to remove > the dependency on ghes > > On Thu, Aug 11, 2022 at 09:17:13AM +0000, Jia He wrote: > > diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c index > > d91ad378c00d..ed6519f3d45b 100644 > > --- a/drivers/acpi/apei/ghes.c > > +++ b/drivers/acpi/apei/ghes.c > > @@ -94,6 +94,8 @@ > > #define FIX_APEI_GHES_SDEI_CRITICAL __end_of_fixed_addresses > > #endif > > > > +ATOMIC_NOTIFIER_HEAD(ghes_report_chain); > > static. You need function wrappers which call the notifier from the module. > Ok
> Also, why atomic? x86_mce_decoder_chain is a blocking one. > But is ghes_proc_in_irq() in the irq context which can invoke the notifier? I described it in the commit log: >To resolve the build dependency of ghes_edac_report_mem_error() for ghes, >introduce a notifier which is registered by ghes_edac module. atomic >notifier is used because ghes_proc_in_irq() is in the irq context.
> Also, the whole notifier adding thing needs to be a separate patch. Ok, I will try to cleanly split it
> > > @@ -1497,3 +1504,37 @@ void __init acpi_ghes_init(void) > > else > > 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(bool force) { > > + int idx = -1; > > + > > + if (IS_ENABLED(CONFIG_X86)) { > > + idx = acpi_match_platform_list(plat_list); > > + if (idx < 0 && !force) > > + return NULL; > > + } > > + > > + return &ghes_devs; > > +} > > +EXPORT_SYMBOL_GPL(ghes_get_devices); > > And yes, as Toshi points out, the other EDAC drivers - sb_edac, skx_edac and > amd64_edac - should call this function in their init functions so that it can get > selected which driver to load on HPE server platforms. > > Also in a separate patch pls. > Ok, thanks
-- Cheers, Justin (Jia He)
| |