[kvm-unit-tests PATCH] x86: Do not run vmx tests if feature is unsupported by CPU

* [kvm-unit-tests PATCH] x86: Do not run vmx tests if feature is unsupported by CPU
@ 2019-05-18 15:53 Nadav Amit
  2019-05-20 14:24 ` Paolo Bonzini
  0 siblings, 1 reply; 2+ messages in thread
From: Nadav Amit @ 2019-05-18 15:53 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: kvm, rkrcmar, Nadav Amit

Instruction tests of VMX should not be executed if the feature is
unsupported by the CPU. Even if the execution controls allow to trap
exits on the feature, the feature might be disabled, for example through
IA32_MISC_ENABLES. Therefore, checking that the feature is supported
through CPUID is needed.

Introduce a general mechanism to check that a feature is supported and
use it for monitor/mwait.

Signed-off-by: Nadav Amit <nadav.amit@gmail.com>
---
 x86/vmx_tests.c | 34 ++++++++++++++++++++++++++++------
 1 file changed, 28 insertions(+), 6 deletions(-)

diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index cade812..bdff938 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -842,6 +842,17 @@ extern void insn_rdseed(void);
 u32 cur_insn;
 u64 cr3;
 
+#define X86_FEATURE_MONITOR	(1 << 3)
+#define X86_FEATURE_MCE		(1 << 7)
+#define X86_FEATURE_PCID	(1 << 17)
+
+typedef bool (*supported_fn)(void);
+
+static bool monitor_supported(void)
+{
+	return cpuid(1).c & X86_FEATURE_MONITOR;
+}
+
 struct insn_table {
 	const char *name;
 	u32 flag;
@@ -853,6 +864,8 @@ struct insn_table {
 	// Use FIELD_EXIT_QUAL and FIELD_INSN_INFO to define
 	// which field need to be tested, reason is always tested
 	u32 test_field;
+	const supported_fn supported_fn;
+	u8 disabled;
 };
 
 /*
@@ -868,7 +881,7 @@ static struct insn_table insn_table[] = {
 	{"HLT",  CPU_HLT, insn_hlt, INSN_CPU0, 12, 0, 0, 0},
 	{"INVLPG", CPU_INVLPG, insn_invlpg, INSN_CPU0, 14,
 		0x12345678, 0, FIELD_EXIT_QUAL},
-	{"MWAIT", CPU_MWAIT, insn_mwait, INSN_CPU0, 36, 0, 0, 0},
+	{"MWAIT", CPU_MWAIT, insn_mwait, INSN_CPU0, 36, 0, 0, 0, &monitor_supported},
 	{"RDPMC", CPU_RDPMC, insn_rdpmc, INSN_CPU0, 15, 0, 0, 0},
 	{"RDTSC", CPU_RDTSC, insn_rdtsc, INSN_CPU0, 16, 0, 0, 0},
 	{"CR3 load", CPU_CR3_LOAD, insn_cr3_load, INSN_CPU0, 28, 0x3, 0,
@@ -881,7 +894,7 @@ static struct insn_table insn_table[] = {
 	{"CR8 store", CPU_CR8_STORE, insn_cr8_store, INSN_CPU0, 28, 0x18, 0,
 		FIELD_EXIT_QUAL},
 #endif
-	{"MONITOR", CPU_MONITOR, insn_monitor, INSN_CPU0, 39, 0, 0, 0},
+	{"MONITOR", CPU_MONITOR, insn_monitor, INSN_CPU0, 39, 0, 0, 0, &monitor_supported},
 	{"PAUSE", CPU_PAUSE, insn_pause, INSN_CPU0, 40, 0, 0, 0},
 	// Flags for Secondary Processor-Based VM-Execution Controls
 	{"WBINVD", CPU_WBINVD, insn_wbinvd, INSN_CPU1, 54, 0, 0, 0},
@@ -904,13 +917,19 @@ static struct insn_table insn_table[] = {
 
 static int insn_intercept_init(struct vmcs *vmcs)
 {
-	u32 ctrl_cpu;
+	u32 ctrl_cpu, cur_insn;
 
 	ctrl_cpu = ctrl_cpu_rev[0].set | CPU_SECONDARY;
 	ctrl_cpu &= ctrl_cpu_rev[0].clr;
 	vmcs_write(CPU_EXEC_CTRL0, ctrl_cpu);
 	vmcs_write(CPU_EXEC_CTRL1, ctrl_cpu_rev[1].set);
 	cr3 = read_cr3();
+
+	for (cur_insn = 0; insn_table[cur_insn].name != NULL; cur_insn++) {
+		if (insn_table[cur_insn].supported_fn == NULL)
+			continue;
+		insn_table[cur_insn].disabled = !insn_table[cur_insn].supported_fn();
+	}
 	return VMX_TEST_START;
 }
 
@@ -928,6 +947,12 @@ static void insn_intercept_main(void)
 			continue;
 		}
 
+		if (insn_table[cur_insn].disabled) {
+			printf("\tFeature required for %s is not supported.\n",
+			       insn_table[cur_insn].name);
+			continue;
+		}
+
 		if ((insn_table[cur_insn].type == INSN_CPU0 &&
 		     !(ctrl_cpu_rev[0].set & insn_table[cur_insn].flag)) ||
 		    (insn_table[cur_insn].type == INSN_CPU1 &&
@@ -6841,9 +6866,6 @@ static void vmentry_movss_shadow_test(void)
 	vmcs_write(GUEST_RFLAGS, X86_EFLAGS_FIXED);
 }
 
-#define X86_FEATURE_PCID       (1 << 17)
-#define X86_FEATURE_MCE        (1 << 7)
-
 static int write_cr4_checking(unsigned long val)
 {
 	asm volatile(ASM_TRY("1f")
-- 
2.17.1


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