Linux-EDAC Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2 00/16] x86/cpu: Clean up handling of VMX features
@ 2019-10-21 23:46 Sean Christopherson
  2019-10-21 23:56 ` [PATCH v2 02/16] x86/mce: WARN once if IA32_FEATURE_CONTROL MSR is left unlocked Sean Christopherson
  0 siblings, 1 reply; 3+ messages in thread
From: Sean Christopherson @ 2019-10-21 23:46 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
	Peter Zijlstra, Arnaldo Carvalho de Melo, Paolo Bonzini,
	Radim Krčmář,
	Tony Luck, Tony W Wang-oc
  Cc: H. Peter Anvin, Mark Rutland, Alexander Shishkin, Jiri Olsa,
	Namhyung Kim, Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li,
	Jim Mattson, Joerg Roedel, linux-kernel, kvm, linux-edac,
	Borislav Petkov, Jarkko Sakkinen

Clean up a handful of interrelated warts in the kernel's handling of VMX:

  - Enable VMX in IA32_FEATURE_CONTROL during boot instead of on-demand
    during KVM load to avoid future contention over IA32_FEATURE_CONTROL.

  - Rework VMX feature reporting so that it is accurate and up-to-date,
    now and in the future.

  - Consolidate code across CPUs that support VMX.

This series stems from two separate but related issues.  The first issue,
pointed out by Boris in the SGX enabling series[1], is that the kernel
currently doesn't ensure the IA32_FEATURE_CONTROL MSR is configured during
boot.  The second issue is that the kernel's reporting of VMX features is
stale, potentially inaccurate, and difficult to maintain.

Note, most non-x86 and non-KVM folks are cc'd only on the cover letter and
on relevant patches.

v2:
  - Rebase to latest tip/x86/cpu (1edae1ae6258, "x86/Kconfig: Enforce...)
  - Collect Jim's reviews.
  - Fix a typo in setting of EPT capabilities [TonyWWang-oc].
  - Remove defines for reserved VMX feature flags [Paolo].
  - Print the VMX features under "flags" and maintain all existing names
    to be backward compatible with the ABI [Paolo].
  - Create aggregate APIC features to report FLEXPRIORITY and APICV, so
    that the full feature *and* their associated individual features are
    printed, e.g. to aid in recognizing why an APIC feature isn't being
    used.
  - Fix a few copy paste errors in changelogs.


== IA32_FEATURE_CONTROL ==
Lack of IA32_FEATURE_CONTROL configuration during boot isn't a functional
issue in the current kernel as the majority of platforms set and lock
IA32_FEATURE_CONTROL in firmware.  And when the MSR is left unlocked, KVM
is the only subsystem that writes IA32_FEATURE_CONTROL.  That will change
if/when SGX support is enabled, as SGX will also want to fully enable
itself when IA32_FEATURE_CONTROL is unlocked.

== VMX Feature Reporting ==
VMX features are not enumerated via CPUID, but instead are enumerated
through VMX MSRs.  As a result, new VMX features are not automatically
reported via /proc/cpuinfo.

An attempt was made long ago to report interesting and/or meaningful VMX
features by synthesizing select features into a Linux-defined cpufeatures
word.  Synthetic feature flags worked for the initial purpose, but the
existence of the synthetic flags was forgotten almost immediately, e.g.
only one new flag (EPT A/D) has been added in the the decade since the
synthetic VMX features were introduced, while VMX and KVM have gained
support for many new features.

Placing the synthetic flags in x86_capability also allows them to be
queried via cpu_has() and company, which is misleading as the flags exist
purely for reporting via /proc/cpuinfo.  KVM, the only in-kernel user of
VMX, ignores the flags.

Last but not least, VMX features are reported in /proc/cpuinfo even
when VMX is unusable due to lack of enabling in IA32_FEATURE_CONTROL.

== Caveats ==
All of the testing of non-standard flows was done in a VM, as I don't
have a system that leaves IA32_FEATURE_CONTROL unlocked, or locks it with
VMX disabled.

The Centaur and Zhaoxin changes are somewhat speculative, as I haven't
confirmed they actually support IA32_FEATURE_CONTROL, or that they want to
gain "official" KVM support.  I assume they unofficially support KVM given
that both CPUs went through the effort of enumerating VMX features.  That
in turn would require them to support IA32_FEATURE_CONTROL since KVM will
fault and refuse to load if the MSR doesn't exist.

[1] https://lkml.kernel.org/r/20190925085156.GA3891@zn.tnic


Sean Christopherson (16):
  x86/intel: Initialize IA32_FEATURE_CONTROL MSR at boot
  x86/mce: WARN once if IA32_FEATURE_CONTROL MSR is left unlocked
  x86/centaur: Use common IA32_FEATURE_CONTROL MSR initialization
  x86/zhaoxin: Use common IA32_FEATURE_CONTROL MSR initialization
  KVM: VMX: Drop initialization of IA32_FEATURE_CONTROL MSR
  x86/cpu: Clear VMX feature flag if VMX is not fully enabled
  KVM: VMX: Use VMX feature flag to query BIOS enabling
  KVM: VMX: Check for full VMX support when verifying CPU compatibility
  x86/vmx: Introduce VMX_FEATURES_*
  x86/cpu: Detect VMX features on Intel, Centaur and Zhaoxin CPUs
  x86/cpu: Print VMX flags in /proc/cpuinfo using VMX_FEATURES_*
  x86/cpufeatures: Drop synthetic VMX feature flags
  KVM: VMX: Use VMX_FEATURE_* flags to define VMCS control bits
  x86/cpufeatures: Clean up synthetic virtualization flags
  perf/x86: Provide stubs of KVM helpers for non-Intel CPUs
  KVM: VMX: Allow KVM_INTEL when building for Centaur and/or Zhaoxin
    CPUs

 MAINTAINERS                           |   2 +-
 arch/x86/Kconfig.cpu                  |   8 ++
 arch/x86/boot/mkcpustr.c              |   1 +
 arch/x86/include/asm/cpufeatures.h    |  15 +---
 arch/x86/include/asm/perf_event.h     |  22 +++--
 arch/x86/include/asm/processor.h      |   4 +
 arch/x86/include/asm/vmx.h            | 105 ++++++++++++-----------
 arch/x86/include/asm/vmxfeatures.h    |  86 +++++++++++++++++++
 arch/x86/kernel/cpu/Makefile          |   6 +-
 arch/x86/kernel/cpu/centaur.c         |  35 +-------
 arch/x86/kernel/cpu/common.c          |   3 +
 arch/x86/kernel/cpu/cpu.h             |   4 +
 arch/x86/kernel/cpu/feature_control.c | 117 ++++++++++++++++++++++++++
 arch/x86/kernel/cpu/intel.c           |  49 +----------
 arch/x86/kernel/cpu/mce/intel.c       |   7 +-
 arch/x86/kernel/cpu/mkcapflags.sh     |  15 +++-
 arch/x86/kernel/cpu/proc.c            |  14 +++
 arch/x86/kernel/cpu/zhaoxin.c         |  35 +-------
 arch/x86/kvm/Kconfig                  |   9 +-
 arch/x86/kvm/vmx/vmx.c                |  41 ++-------
 20 files changed, 343 insertions(+), 235 deletions(-)
 create mode 100644 arch/x86/include/asm/vmxfeatures.h
 create mode 100644 arch/x86/kernel/cpu/feature_control.c

-- 
2.22.0


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH v2 02/16] x86/mce: WARN once if IA32_FEATURE_CONTROL MSR is left unlocked
  2019-10-21 23:46 [PATCH v2 00/16] x86/cpu: Clean up handling of VMX features Sean Christopherson
@ 2019-10-21 23:56 ` Sean Christopherson
  2019-10-25 14:22   ` Borislav Petkov
  0 siblings, 1 reply; 3+ messages in thread
From: Sean Christopherson @ 2019-10-21 23:56 UTC (permalink / raw)
  To: Tony Luck, Borislav Petkov, Thomas Gleixner, Ingo Molnar, x86
  Cc: H. Peter Anvin, linux-edac, linux-kernel, Vitaly Kuznetsov,
	Wanpeng Li, Jim Mattson, kvm, Sean Christopherson

WARN if the IA32_FEATURE_CONTROL MSR is somehow left unlocked now that
CPU initialization unconditionally locks the MSR.

Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
Cc: Wanpeng Li <wanpengli@tencent.com>
Cc: Jim Mattson <jmattson@google.com>
Cc: kvm@vger.kernel.org
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
---
 arch/x86/kernel/cpu/mce/intel.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/cpu/mce/intel.c b/arch/x86/kernel/cpu/mce/intel.c
index 88cd9598fa57..1008f14b803b 100644
--- a/arch/x86/kernel/cpu/mce/intel.c
+++ b/arch/x86/kernel/cpu/mce/intel.c
@@ -117,11 +117,10 @@ static bool lmce_supported(void)
 	 * generate a #GP fault.
 	 */
 	rdmsrl(MSR_IA32_FEATURE_CONTROL, tmp);
-	if ((tmp & (FEATURE_CONTROL_LOCKED | FEATURE_CONTROL_LMCE)) ==
-		   (FEATURE_CONTROL_LOCKED | FEATURE_CONTROL_LMCE))
-		return true;
+	if (WARN_ON_ONCE(!(tmp & FEATURE_CONTROL_LOCKED)))
+		return false;
 
-	return false;
+	return tmp & FEATURE_CONTROL_LMCE;
 }
 
 bool mce_intel_cmci_poll(void)
-- 
2.22.0


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2 02/16] x86/mce: WARN once if IA32_FEATURE_CONTROL MSR is left unlocked
  2019-10-21 23:56 ` [PATCH v2 02/16] x86/mce: WARN once if IA32_FEATURE_CONTROL MSR is left unlocked Sean Christopherson
@ 2019-10-25 14:22   ` Borislav Petkov
  0 siblings, 0 replies; 3+ messages in thread
From: Borislav Petkov @ 2019-10-25 14:22 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: Tony Luck, Thomas Gleixner, Ingo Molnar, x86, H. Peter Anvin,
	linux-edac, linux-kernel, Vitaly Kuznetsov, Wanpeng Li,
	Jim Mattson, kvm

On Mon, Oct 21, 2019 at 04:56:42PM -0700, Sean Christopherson wrote:
> WARN if the IA32_FEATURE_CONTROL MSR is somehow left unlocked now that
> CPU initialization unconditionally locks the MSR.
> 
> Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
> Cc: Wanpeng Li <wanpengli@tencent.com>
> Cc: Jim Mattson <jmattson@google.com>
> Cc: kvm@vger.kernel.org
> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> ---
>  arch/x86/kernel/cpu/mce/intel.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/mce/intel.c b/arch/x86/kernel/cpu/mce/intel.c
> index 88cd9598fa57..1008f14b803b 100644
> --- a/arch/x86/kernel/cpu/mce/intel.c
> +++ b/arch/x86/kernel/cpu/mce/intel.c
> @@ -117,11 +117,10 @@ static bool lmce_supported(void)
>  	 * generate a #GP fault.
>  	 */
>  	rdmsrl(MSR_IA32_FEATURE_CONTROL, tmp);
> -	if ((tmp & (FEATURE_CONTROL_LOCKED | FEATURE_CONTROL_LMCE)) ==
> -		   (FEATURE_CONTROL_LOCKED | FEATURE_CONTROL_LMCE))
> -		return true;
> +	if (WARN_ON_ONCE(!(tmp & FEATURE_CONTROL_LOCKED)))
> +		return false;
>  
> -	return false;
> +	return tmp & FEATURE_CONTROL_LMCE;
>  }
>  
>  bool mce_intel_cmci_poll(void)
> -- 

Reviewed-by: Borislav Petkov <bp@suse.de>

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-21 23:46 [PATCH v2 00/16] x86/cpu: Clean up handling of VMX features Sean Christopherson
2019-10-21 23:56 ` [PATCH v2 02/16] x86/mce: WARN once if IA32_FEATURE_CONTROL MSR is left unlocked Sean Christopherson
2019-10-25 14:22   ` Borislav Petkov

Linux-EDAC Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-edac/0 linux-edac/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-edac linux-edac/ https://lore.kernel.org/linux-edac \
		linux-edac@vger.kernel.org
	public-inbox-index linux-edac

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-edac


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git