linux-kernel - Re: [PATCH v2 07/16] x86/mce: Define BSP-only init

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250220143602.GB589698@yaz-khff2.amd.com>
Date: Thu, 20 Feb 2025 09:36:02 -0500
From: Yazen Ghannam <yazen.ghannam@....com>
To: "Zhuo, Qiuxu" <qiuxu.zhuo@...el.com>
Cc: "x86@...nel.org" <x86@...nel.org>, "Luck, Tony" <tony.luck@...el.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-edac@...r.kernel.org" <linux-edac@...r.kernel.org>,
	"Smita.KoralahalliChannabasappa@....com" <Smita.KoralahalliChannabasappa@....com>
Subject: Re: [PATCH v2 07/16] x86/mce: Define BSP-only init

On Thu, Feb 20, 2025 at 01:37:37AM +0000, Zhuo, Qiuxu wrote:
> > From: Yazen Ghannam <yazen.ghannam@....com>
> > [...]
> > > > --- a/arch/x86/kernel/cpu/mce/core.c
> > > > +++ b/arch/x86/kernel/cpu/mce/core.c
> > > > [...]
> > > > +/* Called only on the boot CPU. */
> > > > +void cpu_mca_init(struct cpuinfo_x86 *c) {
> > > > +	u64 cap;
> > > > +
> > > > +	if (!mce_available(c))
> > > > +		return;
> > > > +
> > > > +	mce_flags.overflow_recov =
> > > > cpu_feature_enabled(X86_FEATURE_OVERFLOW_RECOV);
> > > > +	mce_flags.succor	 =
> > > > cpu_feature_enabled(X86_FEATURE_SUCCOR);
> > > > +	mce_flags.smca		 =
> > > > cpu_feature_enabled(X86_FEATURE_SMCA);
> > >
> > > 1. Before this patch set, the above code was executed only if the following
> > >     condition was true. Do we still need this check?
> > >
> > >     if (c->x86_vendor == X86_VENDOR_AMD || c->x86_vendor ==
> > X86_VENDOR_HYGON)
> > >     {
> > >           The above code.
> > >      }
> > 
> > I don't think so. Feature checks should be independent of vendor, so the
> > vendor checks are redundant.
> 
> Without the vendor check, the Intel CPUs will also run this code (which they should
> *NOT* ) and may mistakenly set those global AMD-specific flags, which may result in
> Intel CPUs performing AMD-specific error handling during the machine check.
> 

It's okay if Intel CPUs run this code, because they don't support these
features. The feature flags are generally derived from CPUID bits, and
x86 vendors do try to make sure they are unique and not
overloaded/redefined between vendors.

The same is true in reverse. It's okay if AMD CPUs run code to check for
Intel-specific features. The feature checks will be false, and
feature-specific code will not be used.

Thanks,
Yazen