Re: [PATCH 1/3] x86/cpu: Process all CPUID dependencies after identifying CPU info

[Sean Christopherson <seanjc@google.com>]

On Thu, Dec 08, 2022, Borislav Petkov wrote:
> On Sat, Dec 03, 2022 at 12:37:43AM +0000, Sean Christopherson wrote:
> > Process all CPUID dependencies to ensure that a dependent is disabled if
> > one or more of its parent features is unsupported.
> 
> Just out of curiosity: this is some weird guest configuration, right?

No, it's also relevant for bare metal.

> Not addressing a real hw issue...

But it's not really a hardware issue either.  More like an admin/user issue.

The problem is that if a kernel is built for subset of CPU types, e.g. just Intel
or just Centaur, and then booted on an "unsupported" CPU type, init_ia32_feat_ctl()
will never be invoked because ->c_init() will point a default_init(), and so the
kernel never checks MSR_IA32_FEAT_CTL to see if VMX and/or SGX are fully enabled.

E.g. if someone booted an "unsupported" kernel and also disabled VMX in BIOS, then
the CPU will enumerate support for VMX in CPUID, but attempting to actually enable
VMX will fail due to VMX being disabled in MSR_IA32_FEAT_CTL.

> > diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> > index bf4ac1cb93d7..094fc69dba63 100644
> > --- a/arch/x86/kernel/cpu/common.c
> > +++ b/arch/x86/kernel/cpu/common.c
> > @@ -1887,6 +1887,12 @@ static void identify_cpu(struct cpuinfo_x86 *c)
> >  
> >  	ppin_init(c);
> >  
> > +	/*
> > +	 * Apply CPUID dependencies to ensure dependent features are disabled
> > +	 * if a parent feature is unsupported but wasn't explicitly disabled.
> > +	 */
> > +	apply_cpuid_deps(c);
> 
> I'd probably call that resolve_cpuid_deps()...

"resolve" works for me.