Re: [PATCH v5 13/19] x86/cpufeatures: Add flag to track whether MSR IA32_FEAT_CTL is configured

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux