[11/16] x86/cpu: Print VMX features as separate line item in /proc/cpuinfo
diff mbox series

Message ID 20191004215615.5479-12-sean.j.christopherson@intel.com
State New
Headers show
Series
  • x86/cpu: Clean up handling of VMX features
Related show

Commit Message

Sean Christopherson Oct. 4, 2019, 9:56 p.m. UTC
Add support for generating VMX feature names in capflags.c and printing
the resulting names in /proc/cpuinfo as "vmx flags" when VMX support is
detected.  Do not print VMX flags if no bits are set in word 0, which
includes Pin controls.  INTR and NMI exiting are fundamental pillars of
virtualization, if they're not supported then the CPU is broken, it does
not actually support VMX, or the kernel wasn't built with support for
the target CPU.

Remove all code which sets the synthetic VMX flags in cpufeatures so
that duplicate VMX features are not printed in "flags" and "vmx flags".

Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
---
 arch/x86/boot/mkcpustr.c          |  1 +
 arch/x86/kernel/cpu/Makefile      |  5 ++--
 arch/x86/kernel/cpu/centaur.c     | 35 ----------------------
 arch/x86/kernel/cpu/intel.c       | 49 -------------------------------
 arch/x86/kernel/cpu/mkcapflags.sh | 15 +++++++---
 arch/x86/kernel/cpu/proc.c        | 15 ++++++++++
 arch/x86/kernel/cpu/zhaoxin.c     | 35 ----------------------
 7 files changed, 30 insertions(+), 125 deletions(-)

Comments

Paolo Bonzini Oct. 7, 2019, 5:12 p.m. UTC | #1
On 04/10/19 23:56, Sean Christopherson wrote:
> Add support for generating VMX feature names in capflags.c and printing
> the resulting names in /proc/cpuinfo as "vmx flags" when VMX support is
> detected.  Do not print VMX flags if no bits are set in word 0, which
> includes Pin controls.  INTR and NMI exiting are fundamental pillars of
> virtualization, if they're not supported then the CPU is broken, it does
> not actually support VMX, or the kernel wasn't built with support for
> the target CPU.
> 
> Remove all code which sets the synthetic VMX flags in cpufeatures so
> that duplicate VMX features are not printed in "flags" and "vmx flags".
> 
> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> ---
>  arch/x86/boot/mkcpustr.c          |  1 +
>  arch/x86/kernel/cpu/Makefile      |  5 ++--
>  arch/x86/kernel/cpu/centaur.c     | 35 ----------------------
>  arch/x86/kernel/cpu/intel.c       | 49 -------------------------------
>  arch/x86/kernel/cpu/mkcapflags.sh | 15 +++++++---
>  arch/x86/kernel/cpu/proc.c        | 15 ++++++++++
>  arch/x86/kernel/cpu/zhaoxin.c     | 35 ----------------------
>  7 files changed, 30 insertions(+), 125 deletions(-)
> 
> diff --git a/arch/x86/boot/mkcpustr.c b/arch/x86/boot/mkcpustr.c
> index 9caa10e82217..da0ccc5de538 100644
> --- a/arch/x86/boot/mkcpustr.c
> +++ b/arch/x86/boot/mkcpustr.c
> @@ -15,6 +15,7 @@
>  #include "../include/asm/required-features.h"
>  #include "../include/asm/disabled-features.h"
>  #include "../include/asm/cpufeatures.h"
> +#include "../include/asm/vmxfeatures.h"
>  #include "../kernel/cpu/capflags.c"
>  
>  int main(void)
> diff --git a/arch/x86/kernel/cpu/Makefile b/arch/x86/kernel/cpu/Makefile
> index df5ad0cfe3e9..025cbfd45687 100644
> --- a/arch/x86/kernel/cpu/Makefile
> +++ b/arch/x86/kernel/cpu/Makefile
> @@ -54,11 +54,12 @@ obj-$(CONFIG_ACRN_GUEST)		+= acrn.o
>  
>  ifdef CONFIG_X86_FEATURE_NAMES
>  quiet_cmd_mkcapflags = MKCAP   $@
> -      cmd_mkcapflags = $(CONFIG_SHELL) $(srctree)/$(src)/mkcapflags.sh $< $@
> +      cmd_mkcapflags = $(CONFIG_SHELL) $(srctree)/$(src)/mkcapflags.sh $@ $^
>  
>  cpufeature = $(src)/../../include/asm/cpufeatures.h
> +vmxfeature = $(src)/../../include/asm/vmxfeatures.h
>  
> -$(obj)/capflags.c: $(cpufeature) $(src)/mkcapflags.sh FORCE
> +$(obj)/capflags.c: $(cpufeature) $(vmxfeature) $(src)/mkcapflags.sh FORCE
>  	$(call if_changed,mkcapflags)
>  endif
>  targets += capflags.c
> diff --git a/arch/x86/kernel/cpu/centaur.c b/arch/x86/kernel/cpu/centaur.c
> index a6ca4c31c1b6..be11c796926b 100644
> --- a/arch/x86/kernel/cpu/centaur.c
> +++ b/arch/x86/kernel/cpu/centaur.c
> @@ -18,13 +18,6 @@
>  #define RNG_ENABLED	(1 << 3)
>  #define RNG_ENABLE	(1 << 6)	/* MSR_VIA_RNG */
>  
> -#define X86_VMX_FEATURE_PROC_CTLS_TPR_SHADOW	0x00200000
> -#define X86_VMX_FEATURE_PROC_CTLS_VNMI		0x00400000
> -#define X86_VMX_FEATURE_PROC_CTLS_2ND_CTLS	0x80000000
> -#define X86_VMX_FEATURE_PROC_CTLS2_VIRT_APIC	0x00000001
> -#define X86_VMX_FEATURE_PROC_CTLS2_EPT		0x00000002
> -#define X86_VMX_FEATURE_PROC_CTLS2_VPID		0x00000020
> -
>  static void init_c3(struct cpuinfo_x86 *c)
>  {
>  	u32  lo, hi;
> @@ -119,31 +112,6 @@ static void early_init_centaur(struct cpuinfo_x86 *c)
>  	}
>  }
>  
> -static void centaur_detect_vmx_virtcap(struct cpuinfo_x86 *c)
> -{
> -	u32 vmx_msr_low, vmx_msr_high, msr_ctl, msr_ctl2;
> -
> -	rdmsr(MSR_IA32_VMX_PROCBASED_CTLS, vmx_msr_low, vmx_msr_high);
> -	msr_ctl = vmx_msr_high | vmx_msr_low;
> -
> -	if (msr_ctl & X86_VMX_FEATURE_PROC_CTLS_TPR_SHADOW)
> -		set_cpu_cap(c, X86_FEATURE_TPR_SHADOW);
> -	if (msr_ctl & X86_VMX_FEATURE_PROC_CTLS_VNMI)
> -		set_cpu_cap(c, X86_FEATURE_VNMI);
> -	if (msr_ctl & X86_VMX_FEATURE_PROC_CTLS_2ND_CTLS) {
> -		rdmsr(MSR_IA32_VMX_PROCBASED_CTLS2,
> -		      vmx_msr_low, vmx_msr_high);
> -		msr_ctl2 = vmx_msr_high | vmx_msr_low;
> -		if ((msr_ctl2 & X86_VMX_FEATURE_PROC_CTLS2_VIRT_APIC) &&
> -		    (msr_ctl & X86_VMX_FEATURE_PROC_CTLS_TPR_SHADOW))
> -			set_cpu_cap(c, X86_FEATURE_FLEXPRIORITY);
> -		if (msr_ctl2 & X86_VMX_FEATURE_PROC_CTLS2_EPT)
> -			set_cpu_cap(c, X86_FEATURE_EPT);
> -		if (msr_ctl2 & X86_VMX_FEATURE_PROC_CTLS2_VPID)
> -			set_cpu_cap(c, X86_FEATURE_VPID);
> -	}
> -}
> -
>  static void init_centaur(struct cpuinfo_x86 *c)
>  {
>  #ifdef CONFIG_X86_32
> @@ -251,9 +219,6 @@ static void init_centaur(struct cpuinfo_x86 *c)
>  #endif
>  
>  	init_feature_control_msr(c);
> -
> -	if (cpu_has(c, X86_FEATURE_VMX))
> -		centaur_detect_vmx_virtcap(c);
>  }
>  
>  #ifdef CONFIG_X86_32
> diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
> index 15d59224e2f8..594d2686ad52 100644
> --- a/arch/x86/kernel/cpu/intel.c
> +++ b/arch/x86/kernel/cpu/intel.c
> @@ -494,52 +494,6 @@ static void srat_detect_node(struct cpuinfo_x86 *c)
>  #endif
>  }
>  
> -static void detect_vmx_virtcap(struct cpuinfo_x86 *c)
> -{
> -	/* Intel VMX MSR indicated features */
> -#define X86_VMX_FEATURE_PROC_CTLS_TPR_SHADOW	0x00200000
> -#define X86_VMX_FEATURE_PROC_CTLS_VNMI		0x00400000
> -#define X86_VMX_FEATURE_PROC_CTLS_2ND_CTLS	0x80000000
> -#define X86_VMX_FEATURE_PROC_CTLS2_VIRT_APIC	0x00000001
> -#define X86_VMX_FEATURE_PROC_CTLS2_EPT		0x00000002
> -#define X86_VMX_FEATURE_PROC_CTLS2_VPID		0x00000020
> -#define x86_VMX_FEATURE_EPT_CAP_AD		0x00200000
> -
> -	u32 vmx_msr_low, vmx_msr_high, msr_ctl, msr_ctl2;
> -	u32 msr_vpid_cap, msr_ept_cap;
> -
> -	clear_cpu_cap(c, X86_FEATURE_TPR_SHADOW);
> -	clear_cpu_cap(c, X86_FEATURE_VNMI);
> -	clear_cpu_cap(c, X86_FEATURE_FLEXPRIORITY);
> -	clear_cpu_cap(c, X86_FEATURE_EPT);
> -	clear_cpu_cap(c, X86_FEATURE_VPID);
> -	clear_cpu_cap(c, X86_FEATURE_EPT_AD);
> -
> -	rdmsr(MSR_IA32_VMX_PROCBASED_CTLS, vmx_msr_low, vmx_msr_high);
> -	msr_ctl = vmx_msr_high | vmx_msr_low;
> -	if (msr_ctl & X86_VMX_FEATURE_PROC_CTLS_TPR_SHADOW)
> -		set_cpu_cap(c, X86_FEATURE_TPR_SHADOW);
> -	if (msr_ctl & X86_VMX_FEATURE_PROC_CTLS_VNMI)
> -		set_cpu_cap(c, X86_FEATURE_VNMI);
> -	if (msr_ctl & X86_VMX_FEATURE_PROC_CTLS_2ND_CTLS) {
> -		rdmsr(MSR_IA32_VMX_PROCBASED_CTLS2,
> -		      vmx_msr_low, vmx_msr_high);
> -		msr_ctl2 = vmx_msr_high | vmx_msr_low;
> -		if ((msr_ctl2 & X86_VMX_FEATURE_PROC_CTLS2_VIRT_APIC) &&
> -		    (msr_ctl & X86_VMX_FEATURE_PROC_CTLS_TPR_SHADOW))
> -			set_cpu_cap(c, X86_FEATURE_FLEXPRIORITY);
> -		if (msr_ctl2 & X86_VMX_FEATURE_PROC_CTLS2_EPT) {
> -			set_cpu_cap(c, X86_FEATURE_EPT);
> -			rdmsr(MSR_IA32_VMX_EPT_VPID_CAP,
> -			      msr_ept_cap, msr_vpid_cap);
> -			if (msr_ept_cap & x86_VMX_FEATURE_EPT_CAP_AD)
> -				set_cpu_cap(c, X86_FEATURE_EPT_AD);
> -		}
> -		if (msr_ctl2 & X86_VMX_FEATURE_PROC_CTLS2_VPID)
> -			set_cpu_cap(c, X86_FEATURE_VPID);
> -	}
> -}
> -
>  #define MSR_IA32_TME_ACTIVATE		0x982
>  
>  /* Helpers to access TME_ACTIVATE MSR */
> @@ -757,9 +711,6 @@ static void init_intel(struct cpuinfo_x86 *c)
>  
>  	init_feature_control_msr(c);
>  
> -	if (cpu_has(c, X86_FEATURE_VMX))
> -		detect_vmx_virtcap(c);
> -
>  	if (cpu_has(c, X86_FEATURE_TME))
>  		detect_tme(c);
>  
> diff --git a/arch/x86/kernel/cpu/mkcapflags.sh b/arch/x86/kernel/cpu/mkcapflags.sh
> index aed45b8895d5..1db560ed2ca3 100644
> --- a/arch/x86/kernel/cpu/mkcapflags.sh
> +++ b/arch/x86/kernel/cpu/mkcapflags.sh
> @@ -6,8 +6,7 @@
>  
>  set -e
>  
> -IN=$1
> -OUT=$2
> +OUT=$1
>  
>  dump_array()
>  {
> @@ -15,6 +14,7 @@ dump_array()
>  	SIZE=$2
>  	PFX=$3
>  	POSTFIX=$4
> +	IN=$5
>  
>  	PFX_SZ=$(echo $PFX | wc -c)
>  	TABS="$(printf '\t\t\t\t\t')"
> @@ -57,11 +57,18 @@ trap 'rm "$OUT"' EXIT
>  	echo "#endif"
>  	echo ""
>  
> -	dump_array "x86_cap_flags" "NCAPINTS*32" "X86_FEATURE_" ""
> +	dump_array "x86_cap_flags" "NCAPINTS*32" "X86_FEATURE_" "" $2
>  	echo ""
>  
> -	dump_array "x86_bug_flags" "NBUGINTS*32" "X86_BUG_" "NCAPINTS*32"
> +	dump_array "x86_bug_flags" "NBUGINTS*32" "X86_BUG_" "NCAPINTS*32" $2
> +	echo ""
>  
> +	echo "#ifdef CONFIG_X86_VMX_FEATURE_NAMES"
> +	echo "#ifndef _ASM_X86_VMXFEATURES_H"
> +	echo "#include <asm/vmxfeatures.h>"
> +	echo "#endif"
> +	dump_array "x86_vmx_flags" "NVMXINTS*32" "VMX_FEATURE_" "" $3
> +	echo "#endif /* CONFIG_X86_VMX_FEATURE_NAMES */"
>  ) > $OUT
>  
>  trap - EXIT
> diff --git a/arch/x86/kernel/cpu/proc.c b/arch/x86/kernel/cpu/proc.c
> index cb2e49810d68..4eec8889b0ff 100644
> --- a/arch/x86/kernel/cpu/proc.c
> +++ b/arch/x86/kernel/cpu/proc.c
> @@ -7,6 +7,10 @@
>  
>  #include "cpu.h"
>  
> +#ifdef CONFIG_X86_VMX_FEATURE_NAMES
> +extern const char * const x86_vmx_flags[NVMXINTS*32];
> +#endif
> +
>  /*
>   *	Get CPU information for use by the procfs.
>   */
> @@ -102,6 +106,17 @@ static int show_cpuinfo(struct seq_file *m, void *v)
>  		if (cpu_has(c, i) && x86_cap_flags[i] != NULL)
>  			seq_printf(m, " %s", x86_cap_flags[i]);

I'm afraid this is going to break some scripts in the wild.  I would
simply remove the seq_puts below.

Paolo

> +#ifdef CONFIG_X86_VMX_FEATURE_NAMES
> +	if (cpu_has(c, X86_FEATURE_VMX) && c->vmx_capability[0]) {
> +		seq_puts(m, "\nvmx flags\t:");
> +		for (i = 0; i < 32*NVMXINTS; i++) {
> +			if (test_bit(i, (unsigned long *)c->vmx_capability) &&
> +			    x86_vmx_flags[i] != NULL)
> +				seq_printf(m, " %s", x86_vmx_flags[i]);
> +		}
> +	}
> +#endif
> +
>  	seq_puts(m, "\nbugs\t\t:");
>  	for (i = 0; i < 32*NBUGINTS; i++) {
>  		unsigned int bug_bit = 32*NCAPINTS + i;
> diff --git a/arch/x86/kernel/cpu/zhaoxin.c b/arch/x86/kernel/cpu/zhaoxin.c
> index 01b05a4a5a85..edfc7cc4ec33 100644
> --- a/arch/x86/kernel/cpu/zhaoxin.c
> +++ b/arch/x86/kernel/cpu/zhaoxin.c
> @@ -16,13 +16,6 @@
>  #define RNG_ENABLED	(1 << 3)
>  #define RNG_ENABLE	(1 << 8)	/* MSR_ZHAOXIN_RNG */
>  
> -#define X86_VMX_FEATURE_PROC_CTLS_TPR_SHADOW	0x00200000
> -#define X86_VMX_FEATURE_PROC_CTLS_VNMI		0x00400000
> -#define X86_VMX_FEATURE_PROC_CTLS_2ND_CTLS	0x80000000
> -#define X86_VMX_FEATURE_PROC_CTLS2_VIRT_APIC	0x00000001
> -#define X86_VMX_FEATURE_PROC_CTLS2_EPT		0x00000002
> -#define X86_VMX_FEATURE_PROC_CTLS2_VPID		0x00000020
> -
>  static void init_zhaoxin_cap(struct cpuinfo_x86 *c)
>  {
>  	u32  lo, hi;
> @@ -89,31 +82,6 @@ static void early_init_zhaoxin(struct cpuinfo_x86 *c)
>  
>  }
>  
> -static void zhaoxin_detect_vmx_virtcap(struct cpuinfo_x86 *c)
> -{
> -	u32 vmx_msr_low, vmx_msr_high, msr_ctl, msr_ctl2;
> -
> -	rdmsr(MSR_IA32_VMX_PROCBASED_CTLS, vmx_msr_low, vmx_msr_high);
> -	msr_ctl = vmx_msr_high | vmx_msr_low;
> -
> -	if (msr_ctl & X86_VMX_FEATURE_PROC_CTLS_TPR_SHADOW)
> -		set_cpu_cap(c, X86_FEATURE_TPR_SHADOW);
> -	if (msr_ctl & X86_VMX_FEATURE_PROC_CTLS_VNMI)
> -		set_cpu_cap(c, X86_FEATURE_VNMI);
> -	if (msr_ctl & X86_VMX_FEATURE_PROC_CTLS_2ND_CTLS) {
> -		rdmsr(MSR_IA32_VMX_PROCBASED_CTLS2,
> -		      vmx_msr_low, vmx_msr_high);
> -		msr_ctl2 = vmx_msr_high | vmx_msr_low;
> -		if ((msr_ctl2 & X86_VMX_FEATURE_PROC_CTLS2_VIRT_APIC) &&
> -		    (msr_ctl & X86_VMX_FEATURE_PROC_CTLS_TPR_SHADOW))
> -			set_cpu_cap(c, X86_FEATURE_FLEXPRIORITY);
> -		if (msr_ctl2 & X86_VMX_FEATURE_PROC_CTLS2_EPT)
> -			set_cpu_cap(c, X86_FEATURE_EPT);
> -		if (msr_ctl2 & X86_VMX_FEATURE_PROC_CTLS2_VPID)
> -			set_cpu_cap(c, X86_FEATURE_VPID);
> -	}
> -}
> -
>  static void init_zhaoxin(struct cpuinfo_x86 *c)
>  {
>  	early_init_zhaoxin(c);
> @@ -142,9 +110,6 @@ static void init_zhaoxin(struct cpuinfo_x86 *c)
>  #endif
>  
>  	init_feature_control_msr(c);
> -
> -	if (cpu_has(c, X86_FEATURE_VMX))
> -		zhaoxin_detect_vmx_virtcap(c);
>  }
>  
>  #ifdef CONFIG_X86_32
>
Sean Christopherson Oct. 7, 2019, 7:56 p.m. UTC | #2
On Mon, Oct 07, 2019 at 07:12:37PM +0200, Paolo Bonzini wrote:
> On 04/10/19 23:56, Sean Christopherson wrote:
> > diff --git a/arch/x86/kernel/cpu/proc.c b/arch/x86/kernel/cpu/proc.c
> > index cb2e49810d68..4eec8889b0ff 100644
> > --- a/arch/x86/kernel/cpu/proc.c
> > +++ b/arch/x86/kernel/cpu/proc.c
> > @@ -7,6 +7,10 @@
> >  
> >  #include "cpu.h"
> >  
> > +#ifdef CONFIG_X86_VMX_FEATURE_NAMES
> > +extern const char * const x86_vmx_flags[NVMXINTS*32];
> > +#endif
> > +
> >  /*
> >   *	Get CPU information for use by the procfs.
> >   */
> > @@ -102,6 +106,17 @@ static int show_cpuinfo(struct seq_file *m, void *v)
> >  		if (cpu_has(c, i) && x86_cap_flags[i] != NULL)
> >  			seq_printf(m, " %s", x86_cap_flags[i]);
> 
> I'm afraid this is going to break some scripts in the wild.  I would
> simply remove the seq_puts below.

Can you elaborate?  I'm having trouble connecting the dots...

> Paolo
> 
> > +#ifdef CONFIG_X86_VMX_FEATURE_NAMES
> > +	if (cpu_has(c, X86_FEATURE_VMX) && c->vmx_capability[0]) {
> > +		seq_puts(m, "\nvmx flags\t:");
> > +		for (i = 0; i < 32*NVMXINTS; i++) {
> > +			if (test_bit(i, (unsigned long *)c->vmx_capability) &&
> > +			    x86_vmx_flags[i] != NULL)
> > +				seq_printf(m, " %s", x86_vmx_flags[i]);
> > +		}
> > +	}
> > +#endif
> > +
> >  	seq_puts(m, "\nbugs\t\t:");
> >  	for (i = 0; i < 32*NBUGINTS; i++) {
> >  		unsigned int bug_bit = 32*NCAPINTS + i;
Paolo Bonzini Oct. 8, 2019, 6:57 a.m. UTC | #3
On 07/10/19 21:56, Sean Christopherson wrote:
> On Mon, Oct 07, 2019 at 07:12:37PM +0200, Paolo Bonzini wrote:
>> On 04/10/19 23:56, Sean Christopherson wrote:
>>> diff --git a/arch/x86/kernel/cpu/proc.c b/arch/x86/kernel/cpu/proc.c
>>> index cb2e49810d68..4eec8889b0ff 100644
>>> --- a/arch/x86/kernel/cpu/proc.c
>>> +++ b/arch/x86/kernel/cpu/proc.c
>>> @@ -7,6 +7,10 @@
>>>  
>>>  #include "cpu.h"
>>>  
>>> +#ifdef CONFIG_X86_VMX_FEATURE_NAMES
>>> +extern const char * const x86_vmx_flags[NVMXINTS*32];
>>> +#endif
>>> +
>>>  /*
>>>   *	Get CPU information for use by the procfs.
>>>   */
>>> @@ -102,6 +106,17 @@ static int show_cpuinfo(struct seq_file *m, void *v)
>>>  		if (cpu_has(c, i) && x86_cap_flags[i] != NULL)
>>>  			seq_printf(m, " %s", x86_cap_flags[i]);
>>
>> I'm afraid this is going to break some scripts in the wild.  I would
>> simply remove the seq_puts below.
> 
> Can you elaborate?  I'm having trouble connecting the dots...

Somebody is bound to have scripts doing "grep ^flags.*ept /proc/cpuinfo"
or checking for VMX flags under some kind of "if (/^flags/)", so it's
safer not to separate VMX and non-VMX flags.

Paolo

>> Paolo
>>
>>> +#ifdef CONFIG_X86_VMX_FEATURE_NAMES
>>> +	if (cpu_has(c, X86_FEATURE_VMX) && c->vmx_capability[0]) {
>>> +		seq_puts(m, "\nvmx flags\t:");
>>> +		for (i = 0; i < 32*NVMXINTS; i++) {
>>> +			if (test_bit(i, (unsigned long *)c->vmx_capability) &&
>>> +			    x86_vmx_flags[i] != NULL)
>>> +				seq_printf(m, " %s", x86_vmx_flags[i]);
>>> +		}
>>> +	}
>>> +#endif
>>> +
>>>  	seq_puts(m, "\nbugs\t\t:");
>>>  	for (i = 0; i < 32*NBUGINTS; i++) {
>>>  		unsigned int bug_bit = 32*NCAPINTS + i;
Jim Mattson Oct. 8, 2019, 4:53 p.m. UTC | #4
On Mon, Oct 7, 2019 at 11:57 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 07/10/19 21:56, Sean Christopherson wrote:
> > On Mon, Oct 07, 2019 at 07:12:37PM +0200, Paolo Bonzini wrote:
> >> On 04/10/19 23:56, Sean Christopherson wrote:
> >>> diff --git a/arch/x86/kernel/cpu/proc.c b/arch/x86/kernel/cpu/proc.c
> >>> index cb2e49810d68..4eec8889b0ff 100644
> >>> --- a/arch/x86/kernel/cpu/proc.c
> >>> +++ b/arch/x86/kernel/cpu/proc.c
> >>> @@ -7,6 +7,10 @@
> >>>
> >>>  #include "cpu.h"
> >>>
> >>> +#ifdef CONFIG_X86_VMX_FEATURE_NAMES
> >>> +extern const char * const x86_vmx_flags[NVMXINTS*32];
> >>> +#endif
> >>> +
> >>>  /*
> >>>   * Get CPU information for use by the procfs.
> >>>   */
> >>> @@ -102,6 +106,17 @@ static int show_cpuinfo(struct seq_file *m, void *v)
> >>>             if (cpu_has(c, i) && x86_cap_flags[i] != NULL)
> >>>                     seq_printf(m, " %s", x86_cap_flags[i]);
> >>
> >> I'm afraid this is going to break some scripts in the wild.  I would
> >> simply remove the seq_puts below.
> >
> > Can you elaborate?  I'm having trouble connecting the dots...
>
> Somebody is bound to have scripts doing "grep ^flags.*ept /proc/cpuinfo"
> or checking for VMX flags under some kind of "if (/^flags/)", so it's
> safer not to separate VMX and non-VMX flags.

Yep. Not quite that exact syntax, but we do have, e.g.:

./x86/feature_check.sh ept

...and you can imagine what feature_check.sh does.
Sean Christopherson Oct. 9, 2019, 7:16 p.m. UTC | #5
On Tue, Oct 08, 2019 at 08:57:30AM +0200, Paolo Bonzini wrote:
> On 07/10/19 21:56, Sean Christopherson wrote:
> > On Mon, Oct 07, 2019 at 07:12:37PM +0200, Paolo Bonzini wrote:
> >> On 04/10/19 23:56, Sean Christopherson wrote:
> >>> diff --git a/arch/x86/kernel/cpu/proc.c b/arch/x86/kernel/cpu/proc.c
> >>> index cb2e49810d68..4eec8889b0ff 100644
> >>> --- a/arch/x86/kernel/cpu/proc.c
> >>> +++ b/arch/x86/kernel/cpu/proc.c
> >>> @@ -7,6 +7,10 @@
> >>>  
> >>>  #include "cpu.h"
> >>>  
> >>> +#ifdef CONFIG_X86_VMX_FEATURE_NAMES
> >>> +extern const char * const x86_vmx_flags[NVMXINTS*32];
> >>> +#endif
> >>> +
> >>>  /*
> >>>   *	Get CPU information for use by the procfs.
> >>>   */
> >>> @@ -102,6 +106,17 @@ static int show_cpuinfo(struct seq_file *m, void *v)
> >>>  		if (cpu_has(c, i) && x86_cap_flags[i] != NULL)
> >>>  			seq_printf(m, " %s", x86_cap_flags[i]);
> >>
> >> I'm afraid this is going to break some scripts in the wild.  I would
> >> simply remove the seq_puts below.
> > 
> > Can you elaborate?  I'm having trouble connecting the dots...
> 
> Somebody is bound to have scripts doing "grep ^flags.*ept /proc/cpuinfo"
> or checking for VMX flags under some kind of "if (/^flags/)", so it's
> safer not to separate VMX and non-VMX flags.

Are the names of the flags considered ABI?  If so, then the rename of
"vnmi" to "virtual_nmis" also needs to be dropped.  :-(
Paolo Bonzini Oct. 9, 2019, 9:13 p.m. UTC | #6
On 09/10/19 21:16, Sean Christopherson wrote:
> On Tue, Oct 08, 2019 at 08:57:30AM +0200, Paolo Bonzini wrote:
>> On 07/10/19 21:56, Sean Christopherson wrote:
>>> On Mon, Oct 07, 2019 at 07:12:37PM +0200, Paolo Bonzini wrote:
>>>> On 04/10/19 23:56, Sean Christopherson wrote:
>>>>> diff --git a/arch/x86/kernel/cpu/proc.c b/arch/x86/kernel/cpu/proc.c
>>>>> index cb2e49810d68..4eec8889b0ff 100644
>>>>> --- a/arch/x86/kernel/cpu/proc.c
>>>>> +++ b/arch/x86/kernel/cpu/proc.c
>>>>> @@ -7,6 +7,10 @@
>>>>>  
>>>>>  #include "cpu.h"
>>>>>  
>>>>> +#ifdef CONFIG_X86_VMX_FEATURE_NAMES
>>>>> +extern const char * const x86_vmx_flags[NVMXINTS*32];
>>>>> +#endif
>>>>> +
>>>>>  /*
>>>>>   *	Get CPU information for use by the procfs.
>>>>>   */
>>>>> @@ -102,6 +106,17 @@ static int show_cpuinfo(struct seq_file *m, void *v)
>>>>>  		if (cpu_has(c, i) && x86_cap_flags[i] != NULL)
>>>>>  			seq_printf(m, " %s", x86_cap_flags[i]);
>>>>
>>>> I'm afraid this is going to break some scripts in the wild.  I would
>>>> simply remove the seq_puts below.
>>>
>>> Can you elaborate?  I'm having trouble connecting the dots...
>>
>> Somebody is bound to have scripts doing "grep ^flags.*ept /proc/cpuinfo"
>> or checking for VMX flags under some kind of "if (/^flags/)", so it's
>> safer not to separate VMX and non-VMX flags.
> 
> Are the names of the flags considered ABI?  If so, then the rename of
> "vnmi" to "virtual_nmis" also needs to be dropped.  :-(

Yes, they are. :/

Paolo

Patch
diff mbox series

diff --git a/arch/x86/boot/mkcpustr.c b/arch/x86/boot/mkcpustr.c
index 9caa10e82217..da0ccc5de538 100644
--- a/arch/x86/boot/mkcpustr.c
+++ b/arch/x86/boot/mkcpustr.c
@@ -15,6 +15,7 @@ 
 #include "../include/asm/required-features.h"
 #include "../include/asm/disabled-features.h"
 #include "../include/asm/cpufeatures.h"
+#include "../include/asm/vmxfeatures.h"
 #include "../kernel/cpu/capflags.c"
 
 int main(void)
diff --git a/arch/x86/kernel/cpu/Makefile b/arch/x86/kernel/cpu/Makefile
index df5ad0cfe3e9..025cbfd45687 100644
--- a/arch/x86/kernel/cpu/Makefile
+++ b/arch/x86/kernel/cpu/Makefile
@@ -54,11 +54,12 @@  obj-$(CONFIG_ACRN_GUEST)		+= acrn.o
 
 ifdef CONFIG_X86_FEATURE_NAMES
 quiet_cmd_mkcapflags = MKCAP   $@
-      cmd_mkcapflags = $(CONFIG_SHELL) $(srctree)/$(src)/mkcapflags.sh $< $@
+      cmd_mkcapflags = $(CONFIG_SHELL) $(srctree)/$(src)/mkcapflags.sh $@ $^
 
 cpufeature = $(src)/../../include/asm/cpufeatures.h
+vmxfeature = $(src)/../../include/asm/vmxfeatures.h
 
-$(obj)/capflags.c: $(cpufeature) $(src)/mkcapflags.sh FORCE
+$(obj)/capflags.c: $(cpufeature) $(vmxfeature) $(src)/mkcapflags.sh FORCE
 	$(call if_changed,mkcapflags)
 endif
 targets += capflags.c
diff --git a/arch/x86/kernel/cpu/centaur.c b/arch/x86/kernel/cpu/centaur.c
index a6ca4c31c1b6..be11c796926b 100644
--- a/arch/x86/kernel/cpu/centaur.c
+++ b/arch/x86/kernel/cpu/centaur.c
@@ -18,13 +18,6 @@ 
 #define RNG_ENABLED	(1 << 3)
 #define RNG_ENABLE	(1 << 6)	/* MSR_VIA_RNG */
 
-#define X86_VMX_FEATURE_PROC_CTLS_TPR_SHADOW	0x00200000
-#define X86_VMX_FEATURE_PROC_CTLS_VNMI		0x00400000
-#define X86_VMX_FEATURE_PROC_CTLS_2ND_CTLS	0x80000000
-#define X86_VMX_FEATURE_PROC_CTLS2_VIRT_APIC	0x00000001
-#define X86_VMX_FEATURE_PROC_CTLS2_EPT		0x00000002
-#define X86_VMX_FEATURE_PROC_CTLS2_VPID		0x00000020
-
 static void init_c3(struct cpuinfo_x86 *c)
 {
 	u32  lo, hi;
@@ -119,31 +112,6 @@  static void early_init_centaur(struct cpuinfo_x86 *c)
 	}
 }
 
-static void centaur_detect_vmx_virtcap(struct cpuinfo_x86 *c)
-{
-	u32 vmx_msr_low, vmx_msr_high, msr_ctl, msr_ctl2;
-
-	rdmsr(MSR_IA32_VMX_PROCBASED_CTLS, vmx_msr_low, vmx_msr_high);
-	msr_ctl = vmx_msr_high | vmx_msr_low;
-
-	if (msr_ctl & X86_VMX_FEATURE_PROC_CTLS_TPR_SHADOW)
-		set_cpu_cap(c, X86_FEATURE_TPR_SHADOW);
-	if (msr_ctl & X86_VMX_FEATURE_PROC_CTLS_VNMI)
-		set_cpu_cap(c, X86_FEATURE_VNMI);
-	if (msr_ctl & X86_VMX_FEATURE_PROC_CTLS_2ND_CTLS) {
-		rdmsr(MSR_IA32_VMX_PROCBASED_CTLS2,
-		      vmx_msr_low, vmx_msr_high);
-		msr_ctl2 = vmx_msr_high | vmx_msr_low;
-		if ((msr_ctl2 & X86_VMX_FEATURE_PROC_CTLS2_VIRT_APIC) &&
-		    (msr_ctl & X86_VMX_FEATURE_PROC_CTLS_TPR_SHADOW))
-			set_cpu_cap(c, X86_FEATURE_FLEXPRIORITY);
-		if (msr_ctl2 & X86_VMX_FEATURE_PROC_CTLS2_EPT)
-			set_cpu_cap(c, X86_FEATURE_EPT);
-		if (msr_ctl2 & X86_VMX_FEATURE_PROC_CTLS2_VPID)
-			set_cpu_cap(c, X86_FEATURE_VPID);
-	}
-}
-
 static void init_centaur(struct cpuinfo_x86 *c)
 {
 #ifdef CONFIG_X86_32
@@ -251,9 +219,6 @@  static void init_centaur(struct cpuinfo_x86 *c)
 #endif
 
 	init_feature_control_msr(c);
-
-	if (cpu_has(c, X86_FEATURE_VMX))
-		centaur_detect_vmx_virtcap(c);
 }
 
 #ifdef CONFIG_X86_32
diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index 15d59224e2f8..594d2686ad52 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -494,52 +494,6 @@  static void srat_detect_node(struct cpuinfo_x86 *c)
 #endif
 }
 
-static void detect_vmx_virtcap(struct cpuinfo_x86 *c)
-{
-	/* Intel VMX MSR indicated features */
-#define X86_VMX_FEATURE_PROC_CTLS_TPR_SHADOW	0x00200000
-#define X86_VMX_FEATURE_PROC_CTLS_VNMI		0x00400000
-#define X86_VMX_FEATURE_PROC_CTLS_2ND_CTLS	0x80000000
-#define X86_VMX_FEATURE_PROC_CTLS2_VIRT_APIC	0x00000001
-#define X86_VMX_FEATURE_PROC_CTLS2_EPT		0x00000002
-#define X86_VMX_FEATURE_PROC_CTLS2_VPID		0x00000020
-#define x86_VMX_FEATURE_EPT_CAP_AD		0x00200000
-
-	u32 vmx_msr_low, vmx_msr_high, msr_ctl, msr_ctl2;
-	u32 msr_vpid_cap, msr_ept_cap;
-
-	clear_cpu_cap(c, X86_FEATURE_TPR_SHADOW);
-	clear_cpu_cap(c, X86_FEATURE_VNMI);
-	clear_cpu_cap(c, X86_FEATURE_FLEXPRIORITY);
-	clear_cpu_cap(c, X86_FEATURE_EPT);
-	clear_cpu_cap(c, X86_FEATURE_VPID);
-	clear_cpu_cap(c, X86_FEATURE_EPT_AD);
-
-	rdmsr(MSR_IA32_VMX_PROCBASED_CTLS, vmx_msr_low, vmx_msr_high);
-	msr_ctl = vmx_msr_high | vmx_msr_low;
-	if (msr_ctl & X86_VMX_FEATURE_PROC_CTLS_TPR_SHADOW)
-		set_cpu_cap(c, X86_FEATURE_TPR_SHADOW);
-	if (msr_ctl & X86_VMX_FEATURE_PROC_CTLS_VNMI)
-		set_cpu_cap(c, X86_FEATURE_VNMI);
-	if (msr_ctl & X86_VMX_FEATURE_PROC_CTLS_2ND_CTLS) {
-		rdmsr(MSR_IA32_VMX_PROCBASED_CTLS2,
-		      vmx_msr_low, vmx_msr_high);
-		msr_ctl2 = vmx_msr_high | vmx_msr_low;
-		if ((msr_ctl2 & X86_VMX_FEATURE_PROC_CTLS2_VIRT_APIC) &&
-		    (msr_ctl & X86_VMX_FEATURE_PROC_CTLS_TPR_SHADOW))
-			set_cpu_cap(c, X86_FEATURE_FLEXPRIORITY);
-		if (msr_ctl2 & X86_VMX_FEATURE_PROC_CTLS2_EPT) {
-			set_cpu_cap(c, X86_FEATURE_EPT);
-			rdmsr(MSR_IA32_VMX_EPT_VPID_CAP,
-			      msr_ept_cap, msr_vpid_cap);
-			if (msr_ept_cap & x86_VMX_FEATURE_EPT_CAP_AD)
-				set_cpu_cap(c, X86_FEATURE_EPT_AD);
-		}
-		if (msr_ctl2 & X86_VMX_FEATURE_PROC_CTLS2_VPID)
-			set_cpu_cap(c, X86_FEATURE_VPID);
-	}
-}
-
 #define MSR_IA32_TME_ACTIVATE		0x982
 
 /* Helpers to access TME_ACTIVATE MSR */
@@ -757,9 +711,6 @@  static void init_intel(struct cpuinfo_x86 *c)
 
 	init_feature_control_msr(c);
 
-	if (cpu_has(c, X86_FEATURE_VMX))
-		detect_vmx_virtcap(c);
-
 	if (cpu_has(c, X86_FEATURE_TME))
 		detect_tme(c);
 
diff --git a/arch/x86/kernel/cpu/mkcapflags.sh b/arch/x86/kernel/cpu/mkcapflags.sh
index aed45b8895d5..1db560ed2ca3 100644
--- a/arch/x86/kernel/cpu/mkcapflags.sh
+++ b/arch/x86/kernel/cpu/mkcapflags.sh
@@ -6,8 +6,7 @@ 
 
 set -e
 
-IN=$1
-OUT=$2
+OUT=$1
 
 dump_array()
 {
@@ -15,6 +14,7 @@  dump_array()
 	SIZE=$2
 	PFX=$3
 	POSTFIX=$4
+	IN=$5
 
 	PFX_SZ=$(echo $PFX | wc -c)
 	TABS="$(printf '\t\t\t\t\t')"
@@ -57,11 +57,18 @@  trap 'rm "$OUT"' EXIT
 	echo "#endif"
 	echo ""
 
-	dump_array "x86_cap_flags" "NCAPINTS*32" "X86_FEATURE_" ""
+	dump_array "x86_cap_flags" "NCAPINTS*32" "X86_FEATURE_" "" $2
 	echo ""
 
-	dump_array "x86_bug_flags" "NBUGINTS*32" "X86_BUG_" "NCAPINTS*32"
+	dump_array "x86_bug_flags" "NBUGINTS*32" "X86_BUG_" "NCAPINTS*32" $2
+	echo ""
 
+	echo "#ifdef CONFIG_X86_VMX_FEATURE_NAMES"
+	echo "#ifndef _ASM_X86_VMXFEATURES_H"
+	echo "#include <asm/vmxfeatures.h>"
+	echo "#endif"
+	dump_array "x86_vmx_flags" "NVMXINTS*32" "VMX_FEATURE_" "" $3
+	echo "#endif /* CONFIG_X86_VMX_FEATURE_NAMES */"
 ) > $OUT
 
 trap - EXIT
diff --git a/arch/x86/kernel/cpu/proc.c b/arch/x86/kernel/cpu/proc.c
index cb2e49810d68..4eec8889b0ff 100644
--- a/arch/x86/kernel/cpu/proc.c
+++ b/arch/x86/kernel/cpu/proc.c
@@ -7,6 +7,10 @@ 
 
 #include "cpu.h"
 
+#ifdef CONFIG_X86_VMX_FEATURE_NAMES
+extern const char * const x86_vmx_flags[NVMXINTS*32];
+#endif
+
 /*
  *	Get CPU information for use by the procfs.
  */
@@ -102,6 +106,17 @@  static int show_cpuinfo(struct seq_file *m, void *v)
 		if (cpu_has(c, i) && x86_cap_flags[i] != NULL)
 			seq_printf(m, " %s", x86_cap_flags[i]);
 
+#ifdef CONFIG_X86_VMX_FEATURE_NAMES
+	if (cpu_has(c, X86_FEATURE_VMX) && c->vmx_capability[0]) {
+		seq_puts(m, "\nvmx flags\t:");
+		for (i = 0; i < 32*NVMXINTS; i++) {
+			if (test_bit(i, (unsigned long *)c->vmx_capability) &&
+			    x86_vmx_flags[i] != NULL)
+				seq_printf(m, " %s", x86_vmx_flags[i]);
+		}
+	}
+#endif
+
 	seq_puts(m, "\nbugs\t\t:");
 	for (i = 0; i < 32*NBUGINTS; i++) {
 		unsigned int bug_bit = 32*NCAPINTS + i;
diff --git a/arch/x86/kernel/cpu/zhaoxin.c b/arch/x86/kernel/cpu/zhaoxin.c
index 01b05a4a5a85..edfc7cc4ec33 100644
--- a/arch/x86/kernel/cpu/zhaoxin.c
+++ b/arch/x86/kernel/cpu/zhaoxin.c
@@ -16,13 +16,6 @@ 
 #define RNG_ENABLED	(1 << 3)
 #define RNG_ENABLE	(1 << 8)	/* MSR_ZHAOXIN_RNG */
 
-#define X86_VMX_FEATURE_PROC_CTLS_TPR_SHADOW	0x00200000
-#define X86_VMX_FEATURE_PROC_CTLS_VNMI		0x00400000
-#define X86_VMX_FEATURE_PROC_CTLS_2ND_CTLS	0x80000000
-#define X86_VMX_FEATURE_PROC_CTLS2_VIRT_APIC	0x00000001
-#define X86_VMX_FEATURE_PROC_CTLS2_EPT		0x00000002
-#define X86_VMX_FEATURE_PROC_CTLS2_VPID		0x00000020
-
 static void init_zhaoxin_cap(struct cpuinfo_x86 *c)
 {
 	u32  lo, hi;
@@ -89,31 +82,6 @@  static void early_init_zhaoxin(struct cpuinfo_x86 *c)
 
 }
 
-static void zhaoxin_detect_vmx_virtcap(struct cpuinfo_x86 *c)
-{
-	u32 vmx_msr_low, vmx_msr_high, msr_ctl, msr_ctl2;
-
-	rdmsr(MSR_IA32_VMX_PROCBASED_CTLS, vmx_msr_low, vmx_msr_high);
-	msr_ctl = vmx_msr_high | vmx_msr_low;
-
-	if (msr_ctl & X86_VMX_FEATURE_PROC_CTLS_TPR_SHADOW)
-		set_cpu_cap(c, X86_FEATURE_TPR_SHADOW);
-	if (msr_ctl & X86_VMX_FEATURE_PROC_CTLS_VNMI)
-		set_cpu_cap(c, X86_FEATURE_VNMI);
-	if (msr_ctl & X86_VMX_FEATURE_PROC_CTLS_2ND_CTLS) {
-		rdmsr(MSR_IA32_VMX_PROCBASED_CTLS2,
-		      vmx_msr_low, vmx_msr_high);
-		msr_ctl2 = vmx_msr_high | vmx_msr_low;
-		if ((msr_ctl2 & X86_VMX_FEATURE_PROC_CTLS2_VIRT_APIC) &&
-		    (msr_ctl & X86_VMX_FEATURE_PROC_CTLS_TPR_SHADOW))
-			set_cpu_cap(c, X86_FEATURE_FLEXPRIORITY);
-		if (msr_ctl2 & X86_VMX_FEATURE_PROC_CTLS2_EPT)
-			set_cpu_cap(c, X86_FEATURE_EPT);
-		if (msr_ctl2 & X86_VMX_FEATURE_PROC_CTLS2_VPID)
-			set_cpu_cap(c, X86_FEATURE_VPID);
-	}
-}
-
 static void init_zhaoxin(struct cpuinfo_x86 *c)
 {
 	early_init_zhaoxin(c);
@@ -142,9 +110,6 @@  static void init_zhaoxin(struct cpuinfo_x86 *c)
 #endif
 
 	init_feature_control_msr(c);
-
-	if (cpu_has(c, X86_FEATURE_VMX))
-		zhaoxin_detect_vmx_virtcap(c);
 }
 
 #ifdef CONFIG_X86_32