All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Justin He <Justin.He@arm.com>
To: Borislav Petkov <bp@alien8.de>,
	"toshi.kani@hpe.com" <toshi.kani@hpe.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-acpi@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-edac@vger.kernel.org" <linux-edac@vger.kernel.org>,
	"devel@acpica.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" <linux-efi@vger.kernel.org>,
	nd <nd@arm.com>, "stable@kernel.org" <stable@kernel.org>
Subject: RE: [PATCH 2/2] EDAC/ghes: Modularize ghes_edac driver to remove the dependency on ghes
Date: Tue, 16 Aug 2022 02:20:19 +0000	[thread overview]
Message-ID: <DBBPR08MB45389A9DB098F1AC14C19074F76B9@DBBPR08MB4538.eurprd08.prod.outlook.com> (raw)
In-Reply-To: <YvZnrTrXhRn8FV3I@zn.tnic>

Hi Borislav & Toshi
Please see my questions below:

> -----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.
> 
> Also, why atomic? x86_mce_decoder_chain is a blocking one.
> 
> Also, the whole notifier adding thing needs to be a separate patch.
> 
> > @@ -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.
> 
I assume that all those edac drivers which used owner checking are
impacted, right? So the impacted list should be:
drivers/edac/pnd2_edac.c:1532:  if (owner && strncmp(owner, EDAC_MOD_STR, sizeof(EDAC_MOD_STR)))
drivers/edac/sb_edac.c:3513:    if (owner && strncmp(owner, EDAC_MOD_STR, sizeof(EDAC_MOD_STR)))
drivers/edac/amd64_edac.c:4333: if (owner && strncmp(owner, EDAC_MOD_STR, sizeof(EDAC_MOD_STR)))
drivers/edac/i10nm_base.c:552:  if (owner && strncmp(owner, EDAC_MOD_STR, sizeof(EDAC_MOD_STR)))
drivers/edac/skx_base.c:657:    if (owner && strncmp(owner, EDAC_MOD_STR, sizeof(EDAC_MOD_STR)))
drivers/edac/igen6_edac.c:1275: if (owner && strncmp(owner, EDAC_MOD_STR, sizeof(EDAC_MOD_STR)))

Furthermore, should I totally remove the owner check in above driver
XX_init()? Because after the new helper ghes_get_device() checking, those
drivers can't be initialized after ghes_edac is loaded. If ghes_edac is NOT
loaded, the edac_mc_owner check in edac_mc_add_mc_with_groups() can guarantee
that there is only one regular edac driver.

What do you think of it?

--
Cheers,
Justin (Jia He)

  parent reply	other threads:[~2022-08-16  6:58 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-11  9:17 [PATCH 0/2] Modularize ghes_edac driver Jia He
2022-08-11  9:17 ` [PATCH 1/2] efi/cper: export several helpers for ghes edac to use Jia He
2022-08-11 15:07   ` Ard Biesheuvel
2022-08-12 14:46   ` Borislav Petkov
2022-08-15  1:13     ` Justin He
2022-08-11  9:17 ` [PATCH 2/2] EDAC/ghes: Modularize ghes_edac driver to remove the dependency on ghes Jia He
2022-08-11 19:26   ` Kani, Toshi
2022-08-12 10:58   ` kernel test robot
2022-08-12 10:58     ` [Devel] " kernel test robot
2022-08-12 14:46   ` Borislav Petkov
2022-08-15  1:32     ` Justin He
2022-08-17  8:36       ` Borislav Petkov
2022-08-16  2:20     ` Justin He [this message]
2022-08-17  1:39       ` Kani, Toshi
2022-08-17  8:50         ` Borislav Petkov
2022-08-17 20:22           ` Kani, Toshi
2022-08-17 20:56             ` Borislav Petkov
2022-08-17 21:09               ` Kani, Toshi
2022-08-17 21:34                 ` Borislav Petkov
2022-08-18  1:24                   ` Justin He

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DBBPR08MB45389A9DB098F1AC14C19074F76B9@DBBPR08MB4538.eurprd08.prod.outlook.com \
    --to=justin.he@arm.com \
    --cc=James.Morse@arm.com \
    --cc=ardb@kernel.org \
    --cc=bp@alien8.de \
    --cc=devel@acpica.org \
    --cc=jarkko@kernel.org \
    --cc=lenb@kernel.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-edac@vger.kernel.org \
    --cc=linux-efi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=nd@arm.com \
    --cc=rafael@kernel.org \
    --cc=robert.moore@intel.com \
    --cc=rric@kernel.org \
    --cc=stable@kernel.org \
    --cc=tony.luck@intel.com \
    --cc=toshi.kani@hpe.com \
    --cc=xueshuai@linux.alibaba.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.