* [PATCH 1/2][kvm-unit-test nVMX]: Move the functionality of enter_guest() to __enter_guest() and make the former a wrapper of the latter
2019-04-18 21:39 [kvm-unit-test nVMX]: Test "load IA32_PAT" VM-entry control on vmentry of nested guests Krish Sadhukhan
@ 2019-04-18 21:39 ` Krish Sadhukhan
2019-04-18 21:39 ` [PATCH 2/2][kvm-unit-test nVMX]: Check "load IA32_PAT" VM-entry control on vmentry of nested guests Krish Sadhukhan
2019-05-08 17:43 ` [kvm-unit-test nVMX]: Test " Krish Sadhukhan
2 siblings, 0 replies; 6+ messages in thread
From: Krish Sadhukhan @ 2019-04-18 21:39 UTC (permalink / raw)
To: kvm; +Cc: rkrcmar, pbonzini, jmattson
Currently enter_guest() aborts on any type of vmentry failures (early
failures or invalid guest state). But the vmentry tests that want to
validate the guest state need to continue running in spite of vmentry
failures arising out of an invalid guest state.
This patch moves the functionality of enter_guest() to __enter_guest() which
accepts a parameter to conditionally abort the test as opposed to the behavior
of enter_guest(). enter_guest() now calls __enter_guest() and preserves
compatibility of existing callers which expect the test to be aborted on
any type of vmentry failures.
Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
Suggested-by: Jim Mattson <jmattson@google.com>
Reviewed-by: Karl Heubaum <karl.heubaum@oracle.com>
---
x86/vmx.c | 27 ++++++++++++++++++---------
x86/vmx.h | 4 ++++
2 files changed, 22 insertions(+), 9 deletions(-)
diff --git a/x86/vmx.c b/x86/vmx.c
index 6ba56bc..d29d21a 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -1583,10 +1583,10 @@ entry_failure_handler(struct vmentry_failure *failure)
}
/*
- * Tries to enter the guest. Returns true iff entry succeeded. Otherwise,
+ * Tries to enter the guest. Returns true if entry succeeded. Otherwise,
* populates @failure.
*/
-static bool vmx_enter_guest(struct vmentry_failure *failure)
+static void vmx_enter_guest(struct vmentry_failure *failure)
{
failure->early = 0;
@@ -1620,8 +1620,6 @@ static bool vmx_enter_guest(struct vmentry_failure *failure)
failure->vmlaunch = !launched;
failure->instr = launched ? "vmresume" : "vmlaunch";
-
- return !failure->early && !(vmcs_read(EXI_REASON) & VMX_ENTRY_FAILURE);
}
static int vmx_run(void)
@@ -1631,7 +1629,9 @@ static int vmx_run(void)
bool entered;
struct vmentry_failure failure;
- entered = vmx_enter_guest(&failure);
+ vmx_enter_guest(&failure);
+ entered = !failure.early &&
+ !(vmcs_read(EXI_REASON) & VMX_ENTRY_FAILURE);
if (entered) {
/*
@@ -1765,10 +1765,9 @@ void test_set_guest(test_guest_func func)
/*
* Enters the guest (or launches it for the first time). Error to call once the
- * guest has returned (i.e., run past the end of its guest() function). Also
- * aborts if guest entry fails.
+ * guest has returned (i.e., run past the end of its guest() function).
*/
-void enter_guest(void)
+void __enter_guest(u8 abort_flag)
{
struct vmentry_failure failure;
@@ -1778,7 +1777,11 @@ void enter_guest(void)
TEST_ASSERT_MSG(!guest_finished,
"Called enter_guest() after guest returned.");
- if (!vmx_enter_guest(&failure)) {
+ vmx_enter_guest(&failure);
+ if ((abort_flag & ABORT_ON_EARLY_VMENTRY_FAIL && failure.early) ||
+ (abort_flag & ABORT_ON_INVALID_GUEST_STATE &&
+ vmcs_read(EXI_REASON) & VMX_ENTRY_FAILURE)) {
+
print_vmentry_failure_info(&failure);
abort();
}
@@ -1807,6 +1810,12 @@ void enter_guest(void)
}
}
+void enter_guest(void)
+{
+ __enter_guest(ABORT_ON_EARLY_VMENTRY_FAIL |
+ ABORT_ON_INVALID_GUEST_STATE);
+}
+
extern struct vmx_test vmx_tests[];
static bool
diff --git a/x86/vmx.h b/x86/vmx.h
index 8a00f73..eefd5dc 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -645,6 +645,9 @@ enum vm_instruction_error_number {
#define VMCS_FIELD_RESERVED_SHIFT (15)
#define VMCS_FIELD_BIT_SIZE (BITS_PER_LONG)
+#define ABORT_ON_EARLY_VMENTRY_FAIL 0x1
+#define ABORT_ON_INVALID_GUEST_STATE 0x2
+
extern struct regs regs;
extern union vmx_basic basic;
@@ -830,6 +833,7 @@ bool ept_execute_only_supported(void);
bool ept_ad_bits_supported(void);
void enter_guest(void);
+void __enter_guest(u8);
typedef void (*test_guest_func)(void);
typedef void (*test_teardown_func)(void *data);
--
2.17.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/2][kvm-unit-test nVMX]: Check "load IA32_PAT" VM-entry control on vmentry of nested guests
2019-04-18 21:39 [kvm-unit-test nVMX]: Test "load IA32_PAT" VM-entry control on vmentry of nested guests Krish Sadhukhan
2019-04-18 21:39 ` [PATCH 1/2][kvm-unit-test nVMX]: Move the functionality of enter_guest() to __enter_guest() and make the former a wrapper of the latter Krish Sadhukhan
@ 2019-04-18 21:39 ` Krish Sadhukhan
2019-05-08 17:43 ` [kvm-unit-test nVMX]: Test " Krish Sadhukhan
2 siblings, 0 replies; 6+ messages in thread
From: Krish Sadhukhan @ 2019-04-18 21:39 UTC (permalink / raw)
To: kvm; +Cc: rkrcmar, pbonzini, jmattson
..to verify KVM performs the appropriate consistency checks for loading
IA32_PAT as part of running a nested guest.
According to section "Checking and Loading Guest State" in Intel SDM
vol 3C, the following check is performed on vmentry:
If the "load IA32_PAT" VM-entry control is 1, the value of the field
for the IA32_PAT MSR must be one that could be written by WRMSR
without fault at CPL 0. Specifically, each of the 8 bytes in the
field must have one of the values 0 (UC), 1 (WC), 4 (WT), 5 (WP),
6 (WB), or 7 (UC-).
Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
Reviewed-by: Karl Heubaum <karl.heubaum@oracle.com>
---
x86/vmx_tests.c | 140 +++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 121 insertions(+), 19 deletions(-)
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index 04b1aee..8186b4f 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -4995,10 +4995,37 @@ static void test_sysenter_field(u32 field, const char *name)
vmcs_write(field, addr_saved);
}
+static void guest_pat_main(void)
+{
+ while (1) {
+ if (vmx_get_test_stage() != 2)
+ vmcall();
+ else
+ break;
+ }
+
+ asm volatile("fnop");
+}
+
+static void report_guest_pat_test(const char *test, u32 xreason, u64 guest_pat)
+{
+ u32 reason = vmcs_read(EXI_REASON);
+ u64 guest_rip;
+ u32 insn_len;
+
+ report("%s, GUEST_PAT %lx", reason == xreason, test, guest_pat);
+
+ guest_rip = vmcs_read(GUEST_RIP);
+ insn_len = vmcs_read(EXI_INST_LEN);
+ if (! (reason & 0x80000021))
+ vmcs_write(GUEST_RIP, guest_rip + insn_len);
+}
+
/*
- * Since a PAT value higher than 8 will yield the same test result as that
- * of 8, we want to confine our tests only up to 8 in order to reduce
- * redundancy of tests and to avoid too many vmentries.
+ * PAT values higher than 8 are uninteresting since they're likely lumped
+ * in with "8". We only test values above 8 one bit at a time,
+ * in order to reduce the number of VM-Entries and keep the runtime
+ * reasonable.
*/
#define PAT_VAL_LIMIT 8
@@ -5010,34 +5037,77 @@ static void test_pat(u32 fld, const char * fld_name, u32 ctrl_fld, u64 ctrl_bit)
u32 j;
int error;
- vmcs_write(ctrl_fld, ctrl_saved & ~ctrl_bit);
- for (i = 0; i <= PAT_VAL_LIMIT; i++) {
+ vmcs_clear_bits(ctrl_fld, ctrl_bit);
+ if (fld == GUEST_PAT) {
+ vmx_set_test_stage(1);
+ test_set_guest(guest_pat_main);
+ }
+
+ for (i = 0; i < 256; i = (i < PAT_VAL_LIMIT) ? i + 1 : i * 2) {
/* Test PAT0..PAT7 fields */
- for (j = 0; j < 8; j++) {
+ for (j = 0; j < (i ? 8 : 1); j++) {
val = i << j * 8;
vmcs_write(fld, val);
- report_prefix_pushf("%s %lx", fld_name, val);
- test_vmx_vmlaunch(0, false);
- report_prefix_pop();
+ if (fld == HOST_PAT) {
+ report_prefix_pushf("%s %lx", fld_name, val);
+ test_vmx_vmlaunch(0, false);
+ report_prefix_pop();
+
+ } else { // GUEST_PAT
+ __enter_guest(ABORT_ON_EARLY_VMENTRY_FAIL);
+ report_guest_pat_test("ENT_LOAD_PAT enabled",
+ VMX_VMCALL, val);
+ }
}
}
- vmcs_write(ctrl_fld, ctrl_saved | ctrl_bit);
- for (i = 0; i <= PAT_VAL_LIMIT; i++) {
+ vmcs_set_bits(ctrl_fld, ctrl_bit);
+ for (i = 0; i < 256; i = (i < PAT_VAL_LIMIT) ? i + 1 : i * 2) {
/* Test PAT0..PAT7 fields */
- for (j = 0; j < 8; j++) {
+ for (j = 0; j < (i ? 8 : 1); j++) {
val = i << j * 8;
vmcs_write(fld, val);
- report_prefix_pushf("%s %lx", fld_name, val);
- if (i == 0x2 || i == 0x3 || i >= 0x8)
- error = VMXERR_ENTRY_INVALID_HOST_STATE_FIELD;
- else
- error = 0;
- test_vmx_vmlaunch(error, false);
- report_prefix_pop();
+
+ if (fld == HOST_PAT) {
+ report_prefix_pushf("%s %lx", fld_name, val);
+ if (i == 0x2 || i == 0x3 || i == 0x8)
+ error =
+ VMXERR_ENTRY_INVALID_HOST_STATE_FIELD;
+ else
+ error = 0;
+
+ test_vmx_vmlaunch(error, false);
+ report_prefix_pop();
+
+ } else { // GUEST_PAT
+ if (i == 0x2 || i == 0x3 || i == 0x8) {
+ __enter_guest(ABORT_ON_EARLY_VMENTRY_FAIL);
+ report_guest_pat_test("ENT_LOAD_PAT "
+ "enabled",
+ VMX_FAIL_STATE |
+ VMX_ENTRY_FAILURE,
+ val);
+ } else {
+ __enter_guest(ABORT_ON_EARLY_VMENTRY_FAIL);
+ report_guest_pat_test("ENT_LOAD_PAT "
+ "enabled",
+ VMX_VMCALL,
+ val);
+ }
+ }
+
}
}
+ if (fld == GUEST_PAT) {
+ /*
+ * Let the guest finish execution
+ */
+ vmx_set_test_stage(2);
+ vmcs_write(fld, pat_saved);
+ __enter_guest(ABORT_ON_EARLY_VMENTRY_FAIL);
+ }
+
vmcs_write(ctrl_fld, ctrl_saved);
vmcs_write(fld, pat_saved);
}
@@ -5083,6 +5153,37 @@ static void vmx_host_state_area_test(void)
test_load_host_pat();
}
+/*
+ * If the "load IA32_PAT" VM-entry control is 1, the value of the field
+ * for the IA32_PAT MSR must be one that could be written by WRMSR
+ * without fault at CPL 0. Specifically, each of the 8 bytes in the
+ * field must have one of the values 0 (UC), 1 (WC), 4 (WT), 5 (WP),
+ * 6 (WB), or 7 (UC-).
+ *
+ * [Intel SDM]
+ */
+static void test_load_guest_pat(void)
+{
+ /*
+ * "load IA32_PAT" VM-entry control
+ */
+ if (!(ctrl_exit_rev.clr & ENT_LOAD_PAT)) {
+ printf("\"Load-IA32-PAT\" entry control not supported\n");
+ return;
+ }
+
+ test_pat(GUEST_PAT, "GUEST_PAT", ENT_CONTROLS, ENT_LOAD_PAT);
+}
+
+/*
+ * Check that the virtual CPU checks the VMX Guest State Area as
+ * documented in the Intel SDM.
+ */
+static void vmx_guest_state_area_test(void)
+{
+ test_load_guest_pat();
+}
+
static bool valid_vmcs_for_vmentry(void)
{
struct vmcs *current_vmcs = NULL;
@@ -6505,6 +6606,7 @@ struct vmx_test vmx_tests[] = {
/* VM-entry tests */
TEST(vmx_controls_test),
TEST(vmx_host_state_area_test),
+ TEST(vmx_guest_state_area_test),
TEST(vmentry_movss_shadow_test),
/* APICv tests */
TEST(vmx_eoi_bitmap_ioapic_scan_test),
--
2.17.2
^ permalink raw reply related [flat|nested] 6+ messages in thread