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

[Yazen Ghannam <yazen.ghannam@amd.com>]

On Thu, Feb 20, 2025 at 01:37:37AM +0000, Zhuo, Qiuxu wrote:
> > From: Yazen Ghannam <yazen.ghannam@amd.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