Re: [PATCH v3 04/19] x86/intel: Initialize IA32_FEATURE_CONTROL MSR at boot

Sean Christopherson
2019-11-19T05:03:26Z

On Tue, Nov 19, 2019 at 05:41:49PM +1300, Kai Huang wrote:
> On Mon, 2019-11-18 at 19:12 -0800, Sean Christopherson wrote:
> > + /*
> > + * Enable VMX if and only if the kernel may do VMXON at some point,
> > + * i.e. KVM is enabled, to avoid unnecessarily adding an attack vector
> > + * for the kernel, e.g. using VMX to hide malicious code.
> > + */
> > + if (cpu_has(c, X86_FEATURE_VMX) && IS_ENABLED(CONFIG_KVM)) {

Hmm, this should more specifically be CONFIG_KVM_INTEL.

> > + msr |= FEAT_CTL_VMX_ENABLED_OUTSIDE_SMX;
> > + if (tboot_enabled())
> > + msr |= FEAT_CTL_VMX_ENABLED_INSIDE_SMX;
> > + }
>
> Why not also take this chance to enable SGX? Or it will come with SGX patch
> series?

The latter. Similar to the KVM check, this shouldn't opt in to SGX unless
the kernel is capable of using SGX.

> > + wrmsrl(MSR_IA32_FEATURE_CONTROL, msr);
> > +}
> > diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
> > index 4a900804a023..b7c6ed0b40b6 100644
> > --- a/arch/x86/kernel/cpu/intel.c
> > +++ b/arch/x86/kernel/cpu/intel.c
> > @@ -755,6 +755,8 @@ static void init_intel(struct cpuinfo_x86 *c)
> > /* Work around errata */
> > srat_detect_node(c);
> >
> > + init_feature_control_msr(c);
>
> Will this compile if you disable CONFIG_X86_FEATURE_CONTROL_MSR?
>
> Provide an empty one in cpu.h if the config is not enabled?

CONFIG_X86_FEATURE_CONTROL_MSR can't be disabled manually, it's selected
by CPU_SUP_INTEL (and by Zhaoxin/Centaur for their relevant patches).