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 writing the MSR.
- 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.
The patches are based on tip/x86/cpu, commit
87d6021b8143 ("x86/math-emu: Limit MATH_EMULATION to 486SX compatibles")
Please let me know if you'd prefer not to receive the full patch set on
future versions of the series. I cc'd everyone on all patches to provide
the full picture, e.g. the motivation behind things like the perf patch.
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.
== 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/[email protected]
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 features as separate line item in /proc/cpuinfo
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 | 121 ++++++++++++++++++++++++++
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 | 106 ++++++++++++++++++++++
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 | 15 ++++
arch/x86/kernel/cpu/zhaoxin.c | 35 +-------
arch/x86/kvm/Kconfig | 9 +-
arch/x86/kvm/vmx/vmx.c | 41 ++-------
20 files changed, 368 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
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 <[email protected]>
---
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]);
+#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
--
2.22.0
Explicitly check the current CPU's VMX feature flag when verifying
compatibility across physical CPUs. This effectively adds a check on
IA32_FEATURE_CONTROL to ensure that VMX is fully enabled on all CPUs.
Signed-off-by: Sean Christopherson <[email protected]>
---
arch/x86/kvm/vmx/vmx.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 510f8a778fca..a482949063f2 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -6837,6 +6837,11 @@ static int __init vmx_check_processor_compat(void)
struct vmcs_config vmcs_conf;
struct vmx_capability vmx_cap;
+ if (!this_cpu_has(X86_FEATURE_VMX)) {
+ pr_err("kvm: VMX is disabled on CPU %d\n", smp_processor_id());
+ return -EIO;
+ }
+
if (setup_vmcs_config(&vmcs_conf, &vmx_cap) < 0)
return -EIO;
if (nested)
--
2.22.0
Use the recently added IA32_FEATURE_CONTROL MSR initialization sequence
to opportunstically enable VMX support when running on a Zhaoxin CPU.
Signed-off-by: Sean Christopherson <[email protected]>
---
arch/x86/Kconfig.cpu | 2 +-
arch/x86/kernel/cpu/zhaoxin.c | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/x86/Kconfig.cpu b/arch/x86/Kconfig.cpu
index 9e4e41424dc2..e78f39adae7b 100644
--- a/arch/x86/Kconfig.cpu
+++ b/arch/x86/Kconfig.cpu
@@ -389,7 +389,7 @@ config X86_DEBUGCTLMSR
config X86_FEATURE_CONTROL_MSR
def_bool y
- depends on CPU_SUP_INTEL || CPU_SUP_CENTAUR
+ depends on CPU_SUP_INTEL || CPU_SUP_CENTAUR || CPU_SUP_ZHAOXIN
menuconfig PROCESSOR_SELECT
bool "Supported processor vendors" if EXPERT
diff --git a/arch/x86/kernel/cpu/zhaoxin.c b/arch/x86/kernel/cpu/zhaoxin.c
index 8e6f2f4b4afe..01b05a4a5a85 100644
--- a/arch/x86/kernel/cpu/zhaoxin.c
+++ b/arch/x86/kernel/cpu/zhaoxin.c
@@ -141,6 +141,8 @@ static void init_zhaoxin(struct cpuinfo_x86 *c)
set_cpu_cap(c, X86_FEATURE_LFENCE_RDTSC);
#endif
+ init_feature_control_msr(c);
+
if (cpu_has(c, X86_FEATURE_VMX))
zhaoxin_detect_vmx_virtcap(c);
}
--
2.22.0
On Fri, Oct 4, 2019 at 2:56 PM Sean Christopherson
<[email protected]> wrote:
>
> Explicitly check the current CPU's VMX feature flag when verifying
> compatibility across physical CPUs. This effectively adds a check on
> IA32_FEATURE_CONTROL to ensure that VMX is fully enabled on all CPUs.
>
> Signed-off-by: Sean Christopherson <[email protected]>
Reviewed-by: Jim Mattson <[email protected]>
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 <[email protected]>
> ---
> 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
>
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;
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;
On Mon, Oct 7, 2019 at 11:57 PM Paolo Bonzini <[email protected]> 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.
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. :-(
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