Linux-EDAC Archive on lore.kernel.org
 help / color / Atom feed
From: Borislav Petkov <bp@alien8.de>
To: Sean Christopherson <sean.j.christopherson@intel.com>
Cc: "Thomas Gleixner" <tglx@linutronix.de>,
	"Ingo Molnar" <mingo@redhat.com>,
	x86@kernel.org, "H. Peter Anvin" <hpa@zytor.com>,
	"Peter Zijlstra" <peterz@infradead.org>,
	"Arnaldo Carvalho de Melo" <acme@kernel.org>,
	"Mark Rutland" <mark.rutland@arm.com>,
	"Alexander Shishkin" <alexander.shishkin@linux.intel.com>,
	"Jiri Olsa" <jolsa@redhat.com>,
	"Namhyung Kim" <namhyung@kernel.org>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Radim Krčmář" <rkrcmar@redhat.com>,
	"Vitaly Kuznetsov" <vkuznets@redhat.com>,
	"Wanpeng Li" <wanpengli@tencent.com>,
	"Jim Mattson" <jmattson@google.com>,
	"Joerg Roedel" <joro@8bytes.org>,
	"Tony Luck" <tony.luck@intel.com>,
	"Tony W Wang-oc" <TonyWWang-oc@zhaoxin.com>,
	"Len Brown" <lenb@kernel.org>, "Shuah Khan" <shuah@kernel.org>,
	linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
	linux-edac@vger.kernel.org, linux-pm@vger.kernel.org,
	linux-kselftest@vger.kernel.org,
	"Jarkko Sakkinen" <jarkko.sakkinen@linux.intel.com>
Subject: Re: [PATCH v4 10/19] x86/cpu: Detect VMX features on Intel, Centaur and Zhaoxin CPUs
Date: Thu, 12 Dec 2019 12:38:38 +0100
Message-ID: <20191212113838.GD4991@zn.tnic> (raw)
In-Reply-To: <20191128014016.4389-11-sean.j.christopherson@intel.com>

On Wed, Nov 27, 2019 at 05:40:07PM -0800, Sean Christopherson wrote:
> diff --git a/arch/x86/kernel/cpu/feat_ctl.c b/arch/x86/kernel/cpu/feat_ctl.c
> index a46c9e46f937..93268bde662a 100644
> --- a/arch/x86/kernel/cpu/feat_ctl.c
> +++ b/arch/x86/kernel/cpu/feat_ctl.c
> @@ -4,6 +4,72 @@
>  #include <asm/cpufeature.h>
>  #include <asm/msr-index.h>
>  #include <asm/processor.h>
> +#include <asm/vmx.h>
> +
> +#ifdef CONFIG_X86_VMX_FEATURE_NAMES
> +enum vmx_feature_leafs {
> +	MISC_FEATURES = 0,
> +	PRIMARY_PROC_CTLS,
> +	SECONDARY_PROC_CTLS,
> +	NR_VMX_FEATURE_WORDS,
> +};
> +
> +#define F(x) BIT(VMX_FEATURE_##x & 0x1f)

Eww, this F-thing has been always bugging me, especially if it means
something a little different each time:

arch/x86/crypto/blowfish-x86_64-asm_64.S:59:#define F() \
arch/x86/kernel/cpu/feat_ctl.c:17:#define F(x) BIT(VMX_FEATURE_##x & 0x1f)
arch/x86/kvm/cpuid.c:65:#define F(x) bit(X86_FEATURE_##x)
arch/x86/kvm/emulate.c:4393:#define F(_f, _e) { .flags = (_f) | Fastop, .u.fastop = (_e) }
arch/x86/kvm/svm.c:5927:#define F(x) bit(X86_FEATURE_##x)

I guess you can call yours VMX_F() or so, just so that it's name is
something different.

> +static void init_vmx_capabilities(struct cpuinfo_x86 *c)
> +{
> +	u32 supported, funcs, ept, vpid, ign;
> +
> +	BUILD_BUG_ON(NVMXINTS != NR_VMX_FEATURE_WORDS);
> +
> +	/*
> +	 * The high bits contain the allowed-1 settings, i.e. features that can
> +	 * be turned on.  The low bits contain the allowed-0 settings, i.e.
> +	 * features that can be turned off.  Ignore the allowed-0 settings,
> +	 * if a feature can be turned on then it's supported.
> +	 */
> +	rdmsr(MSR_IA32_VMX_PROCBASED_CTLS, ign, supported);
> +	c->vmx_capability[PRIMARY_PROC_CTLS] = supported;
> +
> +	rdmsr_safe(MSR_IA32_VMX_PROCBASED_CTLS2, &ign, &supported);
> +	c->vmx_capability[SECONDARY_PROC_CTLS] = supported;
> +
> +	rdmsr(MSR_IA32_VMX_PINBASED_CTLS, ign, supported);
> +	rdmsr_safe(MSR_IA32_VMX_VMFUNC, &ign, &funcs);
> +
> +	/*
> +	 * Except for EPT+VPID, which enumerates support for both in a single
> +	 * MSR, low for EPT, high for VPID.
> +	 */
> +	rdmsr_safe(MSR_IA32_VMX_EPT_VPID_CAP, &ept, &vpid);

Right, so this is a garden variety of rdmsr() and rdmsr_safe() and
the safe variant's retval needs to be checked, strictly speaking. It
probably doesn't matter here since you'll get 0s if it fails, which
means feature not supported, so all good.

But I guess you can still use rdmsr_safe() everywhere just so it doesn't
cause head scratching in the future, when one looks at that code.

> +#endif /* CONFIG_X86_VMX_FEATURE_NAMES */
>  
>  #undef pr_fmt
>  #define pr_fmt(fmt)	"x86/cpu: " fmt
> @@ -50,5 +116,9 @@ void init_ia32_feat_ctl(struct cpuinfo_x86 *c)
>  		pr_err_once("VMX (%s TXT) disabled by BIOS\n",
>  			    tboot ? "inside" : "outside");
>  		clear_cpu_cap(c, X86_FEATURE_VMX);
> +	} else {
> +#ifdef CONFIG_X86_VMX_FEATURE_NAMES
> +		init_vmx_capabilities(c);
> +#endif

Can't say that I'm happy about all that ifdeffery but I guess we need
to perpetuate this since X86_FEATURE_NAMES is there for embedded. In
practice, probably no one disables it...

-- 
Regards/Gruss,
    Boris.

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

  reply index

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-28  1:39 [PATCH v4 00/19] x86/cpu: Clean up handling of VMX features Sean Christopherson
2019-11-28  1:39 ` [PATCH v4 01/19] x86/msr-index: Clean up bit defines for IA32_FEATURE_CONTROL MSR Sean Christopherson
2019-11-28 21:07   ` kbuild test robot
2019-11-30 20:52   ` kbuild test robot
2019-12-02 19:06     ` Sean Christopherson
2019-12-12  9:25       ` Borislav Petkov
2019-12-12 17:48         ` Sean Christopherson
2019-12-12 18:19           ` Borislav Petkov
2019-11-28  1:39 ` [PATCH v4 02/19] selftests: kvm: Replace manual MSR defs with common msr-index.h Sean Christopherson
2019-11-28  1:40 ` [PATCH v4 03/19] tools arch x86: Sync msr-index.h from kernel sources Sean Christopherson
2019-11-28  1:40 ` [PATCH v4 04/19] x86/intel: Initialize IA32_FEAT_CTL MSR at boot Sean Christopherson
2019-11-28  1:40 ` [PATCH v4 05/19] x86/mce: WARN once if IA32_FEAT_CTL MSR is left unlocked Sean Christopherson
2019-11-28  1:40 ` [PATCH v4 06/19] x86/centaur: Use common IA32_FEAT_CTL MSR initialization Sean Christopherson
2019-11-28  1:40 ` [PATCH v4 07/19] x86/zhaoxin: " Sean Christopherson
2019-11-28  1:40 ` [PATCH v4 08/19] x86/cpu: Clear VMX feature flag if VMX is not fully enabled Sean Christopherson
2019-11-28  1:40 ` [PATCH v4 09/19] x86/vmx: Introduce VMX_FEATURES_* Sean Christopherson
2019-11-28  1:40 ` [PATCH v4 10/19] x86/cpu: Detect VMX features on Intel, Centaur and Zhaoxin CPUs Sean Christopherson
2019-12-12 11:38   ` Borislav Petkov [this message]
2019-12-12 17:55     ` Sean Christopherson
2019-12-12 18:24       ` Borislav Petkov
2019-11-28  1:40 ` [PATCH v4 11/19] x86/cpu: Print VMX flags in /proc/cpuinfo using VMX_FEATURES_* Sean Christopherson
2019-12-12 12:26   ` Borislav Petkov
2019-12-12 14:13     ` Paolo Bonzini
2019-12-12 15:52       ` Liran Alon
2019-12-12 15:57         ` Paolo Bonzini
2019-12-12 17:43           ` Sean Christopherson
2019-12-12 17:47             ` Paolo Bonzini
2019-12-12 17:52               ` Liran Alon
2019-12-12 17:57                 ` Jim Mattson
2019-12-12 18:04                   ` Liran Alon
2019-12-12 18:27                     ` Sean Christopherson
2019-12-12 18:32                 ` Borislav Petkov
2019-12-12 17:47             ` Liran Alon
2019-12-12 18:18       ` Sean Christopherson
2019-12-12 18:23         ` Paolo Bonzini
2019-12-21  3:48           ` Sean Christopherson
2019-11-28  1:40 ` [PATCH v4 12/19] x86/cpu: Set synthetic VMX cpufeatures during init_ia32_feat_ctl() Sean Christopherson
2019-11-28  1:40 ` [PATCH v4 13/19] x86/cpufeatures: Add flag to track whether MSR IA32_FEAT_CTL is configured Sean Christopherson
2019-11-28  1:40 ` [PATCH v4 14/19] KVM: VMX: Drop initialization of IA32_FEAT_CTL MSR Sean Christopherson
2019-11-28  1:40 ` [PATCH v4 15/19] KVM: VMX: Use VMX feature flag to query BIOS enabling Sean Christopherson
2019-11-28  1:40 ` [PATCH v4 16/19] KVM: VMX: Check for full VMX support when verifying CPU compatibility Sean Christopherson
2019-11-28  1:40 ` [PATCH v4 17/19] KVM: VMX: Use VMX_FEATURE_* flags to define VMCS control bits Sean Christopherson
2019-11-28  1:40 ` [PATCH v4 18/19] perf/x86: Provide stubs of KVM helpers for non-Intel CPUs Sean Christopherson
2019-11-28  1:40 ` [PATCH v4 19/19] KVM: VMX: Allow KVM_INTEL when building for Centaur and/or Zhaoxin CPUs Sean Christopherson
2019-12-12 14:07 ` [PATCH v4 00/19] x86/cpu: Clean up handling of VMX features Borislav Petkov
2019-12-21  3:44   ` Sean Christopherson

Reply instructions:

You may reply publically 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=20191212113838.GD4991@zn.tnic \
    --to=bp@alien8.de \
    --cc=TonyWWang-oc@zhaoxin.com \
    --cc=acme@kernel.org \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=hpa@zytor.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

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