On 2/25/2020 3:54 PM, Jacob Keller wrote:
I reverted the suggested commit and added some prints:
[ 26.056398] X86_FEATURE_MSR_IA32_FEAT_CTL is enabled [ 26.062426] X86_FEATURE_VMX is enabled [ 26.066923] kvm: disabled by bios
So the old code flow is finding KVM to be disabled, but both features are set...
The code that sets this is run first:
Feb 25 15:46:05 jbrandeb-saw1 kernel: x86/cpu: FEAT_CTL_LOCKED is set Feb 25 15:46:05 jbrandeb-saw1 kernel: x86/cpu: FEAT_CTL_VMX_ENABLED_INSIDE_SMX is unset Feb 25 15:46:05 jbrandeb-saw1 kernel: x86/cpu: FEAT_CTL_VMX_ENABLED_OUTSIDE_SMX is unset Feb 25 15:46:05 jbrandeb-saw1 kernel: x86/cpu: MSR locked by bios Feb 25 15:46:05 jbrandeb-saw1 kernel: x86/cpu: VMX (outside TXT) disabled by BIOS Feb 25 15:46:05 jbrandeb-saw1 kernel: x86/cpu: disabling X86_FEATURE_VMX
But somehow... it is still set later...
So there's something weird going on. Maybe "boot_cpu_has" in the vmx_disabled_by_bios is wrong? Hmm.
I added even more pr_warns, giving me the following diff after reverting the suggested commit:
diff --git a/arch/x86/kernel/cpu/feat_ctl.c b/arch/x86/kernel/cpu/feat_ctl.c index 0268185bef94..a86619acab80 100644 --- a/arch/x86/kernel/cpu/feat_ctl.c +++ b/arch/x86/kernel/cpu/feat_ctl.c @@ -97,13 +97,27 @@ void init_ia32_feat_ctl(struct cpuinfo_x86 *c) bool tboot = tboot_enabled(); u64 msr;
pr_warn("before X86_FEATURE_MSR_IA32_FEAT_CTL is %s\n",cpu_has(c, X86_FEATURE_MSR_IA32_FEAT_CTL) ? "enabled" : "disabled");pr_warn("before X86_FEATURE_VMX is %s\n",cpu_has(c, X86_FEATURE_VMX) ? "enabled" : "disabled");if (rdmsrl_safe(MSR_IA32_FEAT_CTL, &msr)) { clear_cpu_cap(c, X86_FEATURE_VMX); return; }
if (msr & FEAT_CTL_LOCKED)
pr_warn("FEAT_CTL_LOCKED is %s\n",msr & FEAT_CTL_LOCKED ? "set" : "unset");pr_warn("FEAT_CTL_VMX_ENABLED_INSIDE_SMX is %s\n",msr & FEAT_CTL_VMX_ENABLED_INSIDE_SMX ? "set" : "unset");pr_warn("FEAT_CTL_VMX_ENABLED_OUTSIDE_SMX is %s\n",msr & FEAT_CTL_VMX_ENABLED_OUTSIDE_SMX ? "set" : "unset");if (msr & FEAT_CTL_LOCKED) {pr_warn("MSR locked by bios\n"); goto update_caps;} /* * Ignore whatever value BIOS left in the MSR to avoid enabling random@@ -136,10 +150,16 @@ void init_ia32_feat_ctl(struct cpuinfo_x86 *c) if (IS_ENABLED(CONFIG_KVM_INTEL)) pr_err_once("VMX (%s TXT) disabled by BIOS\n", tboot ? "inside" : "outside");
pr_warn("disabling X86_FEATURE_VMX\n"); clear_cpu_cap(c, X86_FEATURE_VMX); } else {#ifdef CONFIG_X86_VMX_FEATURE_NAMES init_vmx_capabilities(c); #endif }
pr_warn("after X86_FEATURE_MSR_IA32_FEAT_CTL is %s\n",cpu_has(c, X86_FEATURE_MSR_IA32_FEAT_CTL) ? "enabled" : "disabled");pr_warn("after X86_FEATURE_VMX is %s\n",cpu_has(c, X86_FEATURE_VMX) ? "enabled" : "disabled");} diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index a2e18e60c2db..550f8d556251 100644 --- a/arch/x86/kvm/vmx/vmx.c +++ b/arch/x86/kvm/vmx/vmx.c @@ -2222,6 +2222,16 @@ static __init int vmx_disabled_by_bios(void) { u64 msr;
pr_warn("boot X86_FEATURE_MSR_IA32_FEAT_CTL is %s\n",boot_cpu_has(X86_FEATURE_MSR_IA32_FEAT_CTL) ? "enabled" : "disabled");pr_warn("boot X86_FEATURE_VMX is %s\n",boot_cpu_has(X86_FEATURE_VMX) ? "enabled" : "disabled");pr_warn("this_cpu X86_FEATURE_MSR_IA32_FEAT_CTL is %s\n",this_cpu_has(X86_FEATURE_MSR_IA32_FEAT_CTL) ? "enabled" : "disabled");pr_warn("this_cpu X86_FEATURE_VMX is %s\n",this_cpu_has(X86_FEATURE_VMX) ? "enabled" : "disabled");rdmsrl(MSR_IA32_FEAT_CTL, msr); if (unlikely(!(msr & FEAT_CTL_LOCKED)))
With this, I see the following output for each CPU, starting with boot CPU:
Feb 25 16:35:59 jbrandeb-saw1 kernel: x86/cpu: before X86_FEATURE_MSR_IA32_FEAT_CTL is disabled Feb 25 16:35:59 jbrandeb-saw1 kernel: x86/cpu: before X86_FEATURE_VMX is enabled Feb 25 16:35:59 jbrandeb-saw1 kernel: x86/cpu: FEAT_CTL_LOCKED is set Feb 25 16:35:59 jbrandeb-saw1 kernel: x86/cpu: FEAT_CTL_VMX_ENABLED_INSIDE_SMX is unset Feb 25 16:35:59 jbrandeb-saw1 kernel: x86/cpu: FEAT_CTL_VMX_ENABLED_OUTSIDE_SMX is unset Feb 25 16:35:59 jbrandeb-saw1 kernel: x86/cpu: MSR locked by bios Feb 25 16:35:59 jbrandeb-saw1 kernel: x86/cpu: VMX (outside TXT) disabled by BIOS Feb 25 16:35:59 jbrandeb-saw1 kernel: x86/cpu: disabling X86_FEATURE_VMX Feb 25 16:35:59 jbrandeb-saw1 kernel: x86/cpu: after X86_FEATURE_MSR_IA32_FEAT_CTL is enabled Feb 25 16:35:59 jbrandeb-saw1 kernel: x86/cpu: after X86_FEATURE_VMX is disabled
And for each of the SMP CPUs:
Feb 25 16:35:59 jbrandeb-saw1 kernel: x86/cpu: before X86_FEATURE_MSR_IA32_FEAT_CTL is disabled Feb 25 16:35:59 jbrandeb-saw1 kernel: x86/cpu: before X86_FEATURE_VMX is enabled Feb 25 16:35:59 jbrandeb-saw1 kernel: x86/cpu: FEAT_CTL_LOCKED is set Feb 25 16:35:59 jbrandeb-saw1 kernel: x86/cpu: FEAT_CTL_VMX_ENABLED_INSIDE_SMX is unset Feb 25 16:35:59 jbrandeb-saw1 kernel: x86/cpu: FEAT_CTL_VMX_ENABLED_OUTSIDE_SMX is unset Feb 25 16:35:59 jbrandeb-saw1 kernel: x86/cpu: MSR locked by bios Feb 25 16:35:59 jbrandeb-saw1 kernel: x86/cpu: disabling X86_FEATURE_VMX Feb 25 16:35:59 jbrandeb-saw1 kernel: x86/cpu: after X86_FEATURE_MSR_IA32_FEAT_CTL is enabled Feb 25 16:35:59 jbrandeb-saw1 kernel: x86/cpu: after X86_FEATURE_VMX is disabled
But when we finally go to check kvm:
Feb 25 16:36:06 jbrandeb-saw1 kernel: boot X86_FEATURE_MSR_IA32_FEAT_CTL is enabled Feb 25 16:36:06 jbrandeb-saw1 kernel: boot X86_FEATURE_VMX is enabled Feb 25 16:36:06 jbrandeb-saw1 kernel: this_cpu X86_FEATURE_MSR_IA32_FEAT_CTL is enabled Feb 25 16:36:06 jbrandeb-saw1 kernel: this_cpu X86_FEATURE_VMX is enabled
I tried checking both boot and this_cpu, just in case.
Somehow the things are being restored/re-enabled. I can't figure out where this even happens. At a glance it's not even obvious to me where the original features get set, and nothing seems to obviously set these flags....
Thanks, Jake