kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jacob Keller <jacob.e.keller@intel.com>
To: Sean Christopherson <sean.j.christopherson@intel.com>
Cc: TonyWWang-oc@zhaoxin.com, acme@kernel.org,
	alexander.shishkin@linux.intel.com, bp@alien8.de, bp@suse.de,
	hpa@zytor.com, jacob.jun.pan@linux.intel.com,
	jarkko.sakkinen@linux.intel.com, jmattson@google.com,
	jolsa@redhat.com, joro@8bytes.org, kvm@vger.kernel.org,
	lenb@kernel.org, linux-edac@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org,
	linux-pm@vger.kernel.org, mark.rutland@arm.com, mingo@redhat.com,
	namhyung@kernel.org, pbonzini@redhat.com, peterz@infradead.org,
	rkrcmar@redhat.com, shuah@kernel.org, tglx@linutronix.de,
	tony.luck@intel.com, vkuznets@redhat.com, wanpengli@tencent.com,
	x86@kernel.org
Subject: Re: [PATCH v5 13/19] x86/cpufeatures: Add flag to track whether MSR IA32_FEAT_CTL is configured
Date: Tue, 25 Feb 2020 16:41:47 -0800	[thread overview]
Message-ID: <ee8e2b4d-9448-48e0-feb9-410059577fe3@intel.com> (raw)
In-Reply-To: <5434303a-0742-3811-fd14-6445d296c0f0@intel.com>

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


  reply	other threads:[~2020-02-26  0:41 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-21  4:44 [PATCH v5 00/19] x86/cpu: Clean up handling of VMX features Sean Christopherson
2019-12-21  4:44 ` [PATCH v5 01/19] x86/msr-index: Clean up bit defines for IA32_FEATURE_CONTROL MSR Sean Christopherson
2019-12-21  4:44 ` [PATCH v5 02/19] selftests: kvm: Replace manual MSR defs with common msr-index.h Sean Christopherson
2019-12-21  4:44 ` [PATCH v5 03/19] tools arch x86: Sync msr-index.h from kernel sources Sean Christopherson
2019-12-21  4:44 ` [PATCH v5 04/19] x86/intel: Initialize IA32_FEAT_CTL MSR at boot Sean Christopherson
2019-12-21  4:44 ` [PATCH v5 05/19] x86/mce: WARN once if IA32_FEAT_CTL MSR is left unlocked Sean Christopherson
2019-12-21  4:45 ` [PATCH v5 06/19] x86/centaur: Use common IA32_FEAT_CTL MSR initialization Sean Christopherson
2019-12-21  4:45 ` [PATCH v5 07/19] x86/zhaoxin: " Sean Christopherson
2019-12-21  4:45 ` [PATCH v5 08/19] x86/cpu: Clear VMX feature flag if VMX is not fully enabled Sean Christopherson
2019-12-21  4:45 ` [PATCH v5 09/19] x86/vmx: Introduce VMX_FEATURES_* Sean Christopherson
2019-12-21  4:45 ` [PATCH v5 10/19] x86/cpu: Detect VMX features on Intel, Centaur and Zhaoxin CPUs Sean Christopherson
2019-12-21  4:45 ` [PATCH v5 11/19] x86/cpu: Print VMX flags in /proc/cpuinfo using VMX_FEATURES_* Sean Christopherson
2019-12-21  4:45 ` [PATCH v5 12/19] x86/cpu: Set synthetic VMX cpufeatures during init_ia32_feat_ctl() Sean Christopherson
2019-12-21  4:45 ` [PATCH v5 13/19] x86/cpufeatures: Add flag to track whether MSR IA32_FEAT_CTL is configured Sean Christopherson
2020-02-25 21:49   ` Jacob Keller
2020-02-25 22:12     ` Sean Christopherson
2020-02-25 22:52       ` Jacob Keller
2020-02-25 23:29         ` Sean Christopherson
2020-02-25 23:54           ` Jacob Keller
2020-02-26  0:41             ` Jacob Keller [this message]
2020-02-26  0:42             ` Sean Christopherson
2020-02-26  0:58               ` Jacob Keller
2020-02-26 20:41                 ` Jacob Keller
2020-02-26 20:57                   ` Sean Christopherson
2020-02-26 21:03                     ` Jacob Keller
2020-02-26 21:25                       ` Sean Christopherson
2020-02-26 21:53                         ` Jacob Keller
2020-02-27  2:12     ` Sean Christopherson
2020-02-27  4:20       ` Huang, Kai
2020-02-27 18:09       ` Jacob Keller
2019-12-21  4:45 ` [PATCH v5 14/19] KVM: VMX: Drop initialization of IA32_FEAT_CTL MSR Sean Christopherson
2019-12-21  4:45 ` [PATCH v5 15/19] KVM: VMX: Use VMX feature flag to query BIOS enabling Sean Christopherson
2019-12-21  4:45 ` [PATCH v5 16/19] KVM: VMX: Check for full VMX support when verifying CPU compatibility Sean Christopherson
2019-12-21  4:45 ` [PATCH v5 17/19] KVM: VMX: Use VMX_FEATURE_* flags to define VMCS control bits Sean Christopherson
2020-01-13 18:32   ` Borislav Petkov
2020-01-13 18:37     ` Sean Christopherson
2020-01-13 18:38       ` Borislav Petkov
2020-01-13 18:42         ` Sean Christopherson
2020-01-13 18:52           ` [PATCH] KVM: VMX: Rename define to CPU_BASED_USE_TSC_OFFSETTING Borislav Petkov
2020-01-13 20:16             ` Sean Christopherson
2020-01-14  9:31               ` Borislav Petkov
2020-01-14 17:27                 ` Sean Christopherson
2019-12-21  4:45 ` [PATCH v5 18/19] perf/x86: Provide stubs of KVM helpers for non-Intel CPUs Sean Christopherson
2019-12-21  4:45 ` [PATCH v5 19/19] KVM: VMX: Allow KVM_INTEL when building for Centaur and/or Zhaoxin CPUs Sean Christopherson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ee8e2b4d-9448-48e0-feb9-410059577fe3@intel.com \
    --to=jacob.e.keller@intel.com \
    --cc=TonyWWang-oc@zhaoxin.com \
    --cc=acme@kernel.org \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=bp@alien8.de \
    --cc=bp@suse.de \
    --cc=hpa@zytor.com \
    --cc=jacob.jun.pan@linux.intel.com \
    --cc=jarkko.sakkinen@linux.intel.com \
    --cc=jmattson@google.com \
    --cc=jolsa@redhat.com \
    --cc=joro@8bytes.org \
    --cc=kvm@vger.kernel.org \
    --cc=lenb@kernel.org \
    --cc=linux-edac@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rkrcmar@redhat.com \
    --cc=sean.j.christopherson@intel.com \
    --cc=shuah@kernel.org \
    --cc=tglx@linutronix.de \
    --cc=tony.luck@intel.com \
    --cc=vkuznets@redhat.com \
    --cc=wanpengli@tencent.com \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).