* [kvm-unit-tests PATCH 0/8] nVMX: Clean up __enter_guest() and co.
@ 2020-03-12 23:27 Sean Christopherson
2020-03-12 23:27 ` [kvm-unit-tests PATCH 1/8] nVMX: Eliminate superfluous entry_failure_handler() wrapper Sean Christopherson
` (8 more replies)
0 siblings, 9 replies; 12+ messages in thread
From: Sean Christopherson @ 2020-03-12 23:27 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kvm
Start chipping away at the crustiness in the nVMX tests by refactoring
"struct vmentry_failure" into "struct vmentry_result", with the full
VM-Exit stored in vmentry_result. Capturing the exit reason allows for a
variety of cleanups and consolidations.
This series really only dives into the v1 tests. I'd like to also clean
up the v2 tests, e.g. take the expected exit reason in enter_guest() so
that the expected behavior is more obvious, but that's a more invasive
cleanup for another day.
Sean Christopherson (8):
nVMX: Eliminate superfluous entry_failure_handler() wrapper
nVMX: Refactor VM-Entry "failure" struct into "result"
nVMX: Consolidate non-canonical code in test_canonical()
nVMX: Drop redundant check for guest termination
nVMX: Expose __enter_guest() and consolidate guest state test code
nVMX: Pass exit reason union to v1 exit handlers
nVMX: Pass exit reason union to is_hypercall()
nVMX: Pass exit reason enum to print_vmexit_info()
x86/vmx.c | 191 +++++++++++--------------
x86/vmx.h | 50 +++++--
x86/vmx_tests.c | 366 +++++++++++++++++++-----------------------------
3 files changed, 263 insertions(+), 344 deletions(-)
--
2.24.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* [kvm-unit-tests PATCH 1/8] nVMX: Eliminate superfluous entry_failure_handler() wrapper
2020-03-12 23:27 [kvm-unit-tests PATCH 0/8] nVMX: Clean up __enter_guest() and co Sean Christopherson
@ 2020-03-12 23:27 ` Sean Christopherson
2020-03-12 23:27 ` [kvm-unit-tests PATCH 2/8] nVMX: Refactor VM-Entry "failure" struct into "result" Sean Christopherson
` (7 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Sean Christopherson @ 2020-03-12 23:27 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kvm
Check and invoke the current entry failure handler directly from
vmx_run() to eliminate an unnecessary layer and its stale comment.
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
---
x86/vmx.c | 19 +++----------------
1 file changed, 3 insertions(+), 16 deletions(-)
diff --git a/x86/vmx.c b/x86/vmx.c
index 0f2521b..99c3791 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -1631,21 +1631,6 @@ static int exit_handler(void)
return ret;
}
-/*
- * Called if vmlaunch or vmresume fails.
- * @early - failure due to "VMX controls and host-state area" (26.2)
- * @vmlaunch - was this a vmlaunch or vmresume
- * @rflags - host rflags
- */
-static int
-entry_failure_handler(struct vmentry_failure *failure)
-{
- if (current->entry_failure_handler)
- return current->entry_failure_handler(failure);
- else
- return VMX_TEST_EXIT;
-}
-
/*
* Tries to enter the guest. Returns true if entry succeeded. Otherwise,
* populates @failure.
@@ -1704,8 +1689,10 @@ static int vmx_run(void)
*/
launched = 1;
ret = exit_handler();
+ } else if (current->entry_failure_handler) {
+ ret = current->entry_failure_handler(failure);
} else {
- ret = entry_failure_handler(&failure);
+ ret = VMX_TEST_EXIT;
}
switch (ret) {
--
2.24.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [kvm-unit-tests PATCH 2/8] nVMX: Refactor VM-Entry "failure" struct into "result"
2020-03-12 23:27 [kvm-unit-tests PATCH 0/8] nVMX: Clean up __enter_guest() and co Sean Christopherson
2020-03-12 23:27 ` [kvm-unit-tests PATCH 1/8] nVMX: Eliminate superfluous entry_failure_handler() wrapper Sean Christopherson
@ 2020-03-12 23:27 ` Sean Christopherson
2020-03-18 23:40 ` Krish Sadhukhan
2020-03-12 23:27 ` [kvm-unit-tests PATCH 3/8] nVMX: Consolidate non-canonical code in test_canonical() Sean Christopherson
` (6 subsequent siblings)
8 siblings, 1 reply; 12+ messages in thread
From: Sean Christopherson @ 2020-03-12 23:27 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kvm
Rename "struct vmentry_failure" to "struct vmentry_result" and add the
full VM-Exit reason to the result. Implement exit_reason as a union so
that tests can easily pull out the parts of interest, e.g. basic exit
reason, whether VM-Entry failed, etc...
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
---
x86/vmx.c | 128 ++++++++++++++++++++++++++----------------------
x86/vmx.h | 39 ++++++++++++---
x86/vmx_tests.c | 24 ++++-----
3 files changed, 112 insertions(+), 79 deletions(-)
diff --git a/x86/vmx.c b/x86/vmx.c
index 99c3791..da17807 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -607,11 +607,14 @@ void print_vmexit_info()
regs.r12, regs.r13, regs.r14, regs.r15);
}
-void
-print_vmentry_failure_info(struct vmentry_failure *failure) {
- if (failure->early) {
- printf("Early %s failure: ", failure->instr);
- switch (failure->flags & VMX_ENTRY_FLAGS) {
+void print_vmentry_failure_info(struct vmentry_result *result)
+{
+ if (result->entered)
+ return;
+
+ if (result->vm_fail) {
+ printf("VM-Fail on %s: ", result->instr);
+ switch (result->flags & VMX_ENTRY_FLAGS) {
case X86_EFLAGS_CF:
printf("current-VMCS pointer is not valid.\n");
break;
@@ -620,16 +623,15 @@ print_vmentry_failure_info(struct vmentry_failure *failure) {
vmcs_read(VMX_INST_ERROR));
break;
default:
- printf("unexpected flags %lx!\n", failure->flags);
+ printf("unexpected flags %lx!\n", result->flags);
}
} else {
- u64 reason = vmcs_read(EXI_REASON);
u64 qual = vmcs_read(EXI_QUALIFICATION);
- printf("Non-early %s failure (reason=%#lx, qual=%#lx): ",
- failure->instr, reason, qual);
+ printf("VM-Exit failure on %s (reason=%#x, qual=%#lx): ",
+ result->instr, result->exit_reason.full, qual);
- switch (reason & 0xff) {
+ switch (result->exit_reason.basic) {
case VMX_FAIL_STATE:
printf("invalid guest state\n");
break;
@@ -640,14 +642,14 @@ print_vmentry_failure_info(struct vmentry_failure *failure) {
printf("machine-check event\n");
break;
default:
- printf("unexpected basic exit reason %ld\n",
- reason & 0xff);
+ printf("unexpected basic exit reason %u\n",
+ result->exit_reason.basic);
}
- if (!(reason & VMX_ENTRY_FAILURE))
+ if (!result->exit_reason.failed_vmentry)
printf("\tVMX_ENTRY_FAILURE BIT NOT SET!\n");
- if (reason & 0x7fff0000)
+ if (result->exit_reason.full & 0x7fff0000)
printf("\tRESERVED BITS SET!\n");
}
}
@@ -1632,12 +1634,12 @@ static int exit_handler(void)
}
/*
- * Tries to enter the guest. Returns true if entry succeeded. Otherwise,
- * populates @failure.
+ * Tries to enter the guest, populates @result with VM-Fail, VM-Exit, entered,
+ * etc...
*/
-static void vmx_enter_guest(struct vmentry_failure *failure)
+static void vmx_enter_guest(struct vmentry_result *result)
{
- failure->early = 0;
+ memset(result, 0, sizeof(*result));
in_guest = 1;
asm volatile (
@@ -1654,35 +1656,35 @@ static void vmx_enter_guest(struct vmentry_failure *failure)
SAVE_GPR_C
"pushf\n\t"
"pop %%rdi\n\t"
- "mov %%rdi, %[failure_flags]\n\t"
- "movl $1, %[failure_early]\n\t"
+ "mov %%rdi, %[vm_fail_flags]\n\t"
+ "movl $1, %[vm_fail]\n\t"
"jmp 3f\n\t"
"vmx_return:\n\t"
SAVE_GPR_C
"3: \n\t"
- : [failure_early]"+m"(failure->early),
- [failure_flags]"=m"(failure->flags)
+ : [vm_fail]"+m"(result->vm_fail),
+ [vm_fail_flags]"=m"(result->flags)
: [launched]"m"(launched), [HOST_RSP]"i"(HOST_RSP)
: "rdi", "memory", "cc"
);
in_guest = 0;
- failure->vmlaunch = !launched;
- failure->instr = launched ? "vmresume" : "vmlaunch";
+ result->vmlaunch = !launched;
+ result->instr = launched ? "vmresume" : "vmlaunch";
+ result->exit_reason.full = result->vm_fail ? 0xdead :
+ vmcs_read(EXI_REASON);
+ result->entered = !result->vm_fail &&
+ !result->exit_reason.failed_vmentry;
}
static int vmx_run(void)
{
+ struct vmentry_result result;
+ u32 ret;
+
while (1) {
- u32 ret;
- bool entered;
- struct vmentry_failure failure;
-
- vmx_enter_guest(&failure);
- entered = !failure.early &&
- !(vmcs_read(EXI_REASON) & VMX_ENTRY_FAILURE);
-
- if (entered) {
+ vmx_enter_guest(&result);
+ if (result.entered) {
/*
* VMCS isn't in "launched" state if there's been any
* entry failure (early or otherwise).
@@ -1690,7 +1692,7 @@ static int vmx_run(void)
launched = 1;
ret = exit_handler();
} else if (current->entry_failure_handler) {
- ret = current->entry_failure_handler(failure);
+ ret = current->entry_failure_handler(&result);
} else {
ret = VMX_TEST_EXIT;
}
@@ -1705,15 +1707,15 @@ static int vmx_run(void)
break;
default:
printf("ERROR : Invalid %s_handler return val %d.\n",
- entered ? "exit" : "entry_failure",
+ result.entered ? "exit" : "entry_failure",
ret);
break;
}
- if (entered)
+ if (result.entered)
print_vmexit_info();
else
- print_vmentry_failure_info(&failure);
+ print_vmentry_failure_info(&result);
abort();
}
}
@@ -1845,7 +1847,7 @@ static void check_for_guest_termination(void)
* 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).
*/
-static void __enter_guest(u8 abort_flag, struct vmentry_failure *failure)
+static void __enter_guest(u8 abort_flag, struct vmentry_result *result)
{
TEST_ASSERT_MSG(v2_guest_main,
"Never called test_set_guest_func!");
@@ -1853,24 +1855,32 @@ static void __enter_guest(u8 abort_flag, struct vmentry_failure *failure)
TEST_ASSERT_MSG(!guest_finished,
"Called enter_guest() after guest returned.");
- 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)) {
+ vmx_enter_guest(result);
- print_vmentry_failure_info(failure);
- abort();
+ if (result->vm_fail) {
+ if (abort_flag & ABORT_ON_EARLY_VMENTRY_FAIL)
+ goto do_abort;
+ return;
}
-
- if (!failure->early && !(vmcs_read(EXI_REASON) & VMX_ENTRY_FAILURE)) {
- launched = 1;
- check_for_guest_termination();
+ if (result->exit_reason.failed_vmentry) {
+ if ((abort_flag & ABORT_ON_INVALID_GUEST_STATE) ||
+ result->exit_reason.basic != VMX_FAIL_STATE)
+ goto do_abort;
+ return;
}
+
+ launched = 1;
+ check_for_guest_termination();
+ return;
+
+do_abort:
+ print_vmentry_failure_info(result);
+ abort();
}
void enter_guest_with_bad_controls(void)
{
- struct vmentry_failure failure = {0};
+ struct vmentry_result result;
TEST_ASSERT_MSG(v2_guest_main,
"Never called test_set_guest_func!");
@@ -1878,10 +1888,10 @@ void enter_guest_with_bad_controls(void)
TEST_ASSERT_MSG(!guest_finished,
"Called enter_guest() after guest returned.");
- __enter_guest(ABORT_ON_INVALID_GUEST_STATE, &failure);
- report(failure.early, "failure occurred early");
- report((failure.flags & VMX_ENTRY_FLAGS) == X86_EFLAGS_ZF,
- "FLAGS set correctly");
+ __enter_guest(ABORT_ON_INVALID_GUEST_STATE, &result);
+ report(result.vm_fail, "VM-Fail occurred as expected");
+ report((result.flags & VMX_ENTRY_FLAGS) == X86_EFLAGS_ZF,
+ "FLAGS set correctly on VM-Fail");
report(vmcs_read(VMX_INST_ERROR) == VMXERR_ENTRY_INVALID_CONTROL_FIELD,
"VM-Inst Error # is %d (VM entry with invalid control field(s))",
VMXERR_ENTRY_INVALID_CONTROL_FIELD);
@@ -1893,23 +1903,23 @@ void enter_guest_with_bad_controls(void)
* unexpectedly succeed, it's nice to check whether the guest has
* terminated, to reduce the number of error messages.
*/
- if (!failure.early)
+ if (!result.vm_fail)
check_for_guest_termination();
}
void enter_guest(void)
{
- struct vmentry_failure failure = {0};
+ struct vmentry_result result;
__enter_guest(ABORT_ON_EARLY_VMENTRY_FAIL |
- ABORT_ON_INVALID_GUEST_STATE, &failure);
+ ABORT_ON_INVALID_GUEST_STATE, &result);
}
void enter_guest_with_invalid_guest_state(void)
{
- struct vmentry_failure failure = {0};
+ struct vmentry_result result;
- __enter_guest(ABORT_ON_EARLY_VMENTRY_FAIL, &failure);
+ __enter_guest(ABORT_ON_EARLY_VMENTRY_FAIL, &result);
}
extern struct vmx_test vmx_tests[];
@@ -1924,6 +1934,8 @@ test_wanted(const char *name, const char *filters[], int filter_count)
char *c;
const char *n;
+ printf("filter = %s, test = %s\n", filters[0], name);
+
/* Replace spaces with underscores. */
n = name;
c = &clean_name[0];
diff --git a/x86/vmx.h b/x86/vmx.h
index 6adf091..c4a0fb4 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -44,13 +44,38 @@ struct regs {
u64 rflags;
};
-struct vmentry_failure {
- /* Did a vmlaunch or vmresume fail? */
- bool vmlaunch;
+struct vmentry_result {
/* Instruction mnemonic (for convenience). */
const char *instr;
- /* Did the instruction return right away, or did we jump to HOST_RIP? */
- bool early;
+ /* Did the test attempt vmlaunch or vmresume? */
+ bool vmlaunch;
+ /* Did the instruction VM-Fail? */
+ bool vm_fail;
+ /* Did the VM-Entry fully enter the guest? */
+ bool entered;
+ /* VM-Exit reason, valid iff !vm_fail */
+ union {
+ struct {
+ u32 basic : 16;
+ u32 reserved16 : 1;
+ u32 reserved17 : 1;
+ u32 reserved18 : 1;
+ u32 reserved19 : 1;
+ u32 reserved20 : 1;
+ u32 reserved21 : 1;
+ u32 reserved22 : 1;
+ u32 reserved23 : 1;
+ u32 reserved24 : 1;
+ u32 reserved25 : 1;
+ u32 reserved26 : 1;
+ u32 enclave_mode : 1;
+ u32 smi_pending_mtf : 1;
+ u32 smi_from_vmx_root : 1;
+ u32 reserved30 : 1;
+ u32 failed_vmentry : 1;
+ };
+ u32 full;
+ } exit_reason;
/* Contents of [re]flags after failed entry. */
unsigned long flags;
};
@@ -62,7 +87,7 @@ struct vmx_test {
int (*exit_handler)(void);
void (*syscall_handler)(u64 syscall_no);
struct regs guest_regs;
- int (*entry_failure_handler)(struct vmentry_failure *failure);
+ int (*entry_failure_handler)(struct vmentry_result *result);
struct vmcs *vmcs;
int exits;
/* Alternative test interface. */
@@ -800,7 +825,7 @@ void init_vmx(u64 *vmxon_region);
const char *exit_reason_description(u64 reason);
void print_vmexit_info(void);
-void print_vmentry_failure_info(struct vmentry_failure *failure);
+void print_vmentry_failure_info(struct vmentry_result *result);
void ept_sync(int type, u64 eptp);
void vpid_sync(int type, u16 vpid);
void install_ept_entry(unsigned long *pml4, int pte_level,
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index a7abd63..c4077b1 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -1996,24 +1996,22 @@ static int msr_switch_exit_handler(void)
return VMX_TEST_EXIT;
}
-static int msr_switch_entry_failure(struct vmentry_failure *failure)
+static int msr_switch_entry_failure(struct vmentry_result *result)
{
- ulong reason;
-
- if (failure->early) {
- printf("ERROR %s: early exit\n", __func__);
+ if (result->vm_fail) {
+ printf("ERROR %s: VM-Fail on %s\n", __func__, result->instr);
return VMX_TEST_EXIT;
}
- reason = vmcs_read(EXI_REASON);
- if (reason == (VMX_ENTRY_FAILURE | VMX_FAIL_MSR) &&
+ if (result->exit_reason.failed_vmentry &&
+ result->exit_reason.basic == VMX_FAIL_MSR &&
vmx_get_test_stage() == 3) {
report(vmcs_read(EXI_QUALIFICATION) == 1,
"VM entry MSR load: try to load FS_BASE");
return VMX_TEST_VMEXIT;
}
- printf("ERROR %s: unexpected stage=%u or reason=%lu\n",
- __func__, vmx_get_test_stage(), reason);
+ printf("ERROR %s: unexpected stage=%u or reason=%x\n",
+ __func__, vmx_get_test_stage(), result->exit_reason.full);
return VMX_TEST_EXIT;
}
@@ -9428,12 +9426,10 @@ static int invalid_msr_exit_handler(void)
return VMX_TEST_EXIT;
}
-static int invalid_msr_entry_failure(struct vmentry_failure *failure)
+static int invalid_msr_entry_failure(struct vmentry_result *result)
{
- ulong reason;
-
- reason = vmcs_read(EXI_REASON);
- report(reason == (0x80000000u | VMX_FAIL_MSR), "Invalid MSR load");
+ report(result->exit_reason.failed_vmentry &&
+ result->exit_reason.basic == VMX_FAIL_MSR, "Invalid MSR load");
return VMX_TEST_VMEXIT;
}
--
2.24.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [kvm-unit-tests PATCH 3/8] nVMX: Consolidate non-canonical code in test_canonical()
2020-03-12 23:27 [kvm-unit-tests PATCH 0/8] nVMX: Clean up __enter_guest() and co Sean Christopherson
2020-03-12 23:27 ` [kvm-unit-tests PATCH 1/8] nVMX: Eliminate superfluous entry_failure_handler() wrapper Sean Christopherson
2020-03-12 23:27 ` [kvm-unit-tests PATCH 2/8] nVMX: Refactor VM-Entry "failure" struct into "result" Sean Christopherson
@ 2020-03-12 23:27 ` Sean Christopherson
2020-03-12 23:27 ` [kvm-unit-tests PATCH 4/8] nVMX: Drop redundant check for guest termination Sean Christopherson
` (5 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Sean Christopherson @ 2020-03-12 23:27 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kvm
Refactor test_canonical() to provide a single flow for the non-canonical
path. Practically speaking, its extremely unlikely the field being
tested already has a non-canonical address, and even less likely that
it's anything other than NONCANONICAL. I.e. the added complexity
doesn't come with added coverage.
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
---
x86/vmx_tests.c | 40 +++++++++++++++++-----------------------
1 file changed, 17 insertions(+), 23 deletions(-)
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index c4077b1..ac02b9d 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -7411,6 +7411,11 @@ static void test_canonical(u64 field, const char * field_name, bool host)
{
u64 addr_saved = vmcs_read(field);
+ /*
+ * Use the existing value if possible. Writing a random canonical
+ * value is not an option as doing so would corrupt the field being
+ * tested and likely hose the test.
+ */
if (is_canonical(addr_saved)) {
if (host) {
report_prefix_pushf("%s %lx", field_name, addr_saved);
@@ -7422,33 +7427,22 @@ static void test_canonical(u64 field, const char * field_name, bool host)
VMX_VMCALL, addr_saved,
field_name);
}
+ }
- vmcs_write(field, NONCANONICAL);
+ vmcs_write(field, NONCANONICAL);
- if (host) {
- report_prefix_pushf("%s %llx", field_name, NONCANONICAL);
- test_vmx_vmlaunch(VMXERR_ENTRY_INVALID_HOST_STATE_FIELD);
- report_prefix_pop();
- } else {
- enter_guest_with_invalid_guest_state();
- report_guest_state_test("Test canonical address",
- VMX_FAIL_STATE | VMX_ENTRY_FAILURE,
- NONCANONICAL, field_name);
- }
-
- vmcs_write(field, addr_saved);
+ if (host) {
+ report_prefix_pushf("%s %llx", field_name, NONCANONICAL);
+ test_vmx_vmlaunch(VMXERR_ENTRY_INVALID_HOST_STATE_FIELD);
+ report_prefix_pop();
} else {
- if (host) {
- report_prefix_pushf("%s %llx", field_name, NONCANONICAL);
- test_vmx_vmlaunch(VMXERR_ENTRY_INVALID_HOST_STATE_FIELD);
- report_prefix_pop();
- } else {
- enter_guest_with_invalid_guest_state();
- report_guest_state_test("Test canonical address",
- VMX_FAIL_STATE | VMX_ENTRY_FAILURE,
- NONCANONICAL, field_name);
- }
+ enter_guest_with_invalid_guest_state();
+ report_guest_state_test("Test non-canonical address",
+ VMX_FAIL_STATE | VMX_ENTRY_FAILURE,
+ NONCANONICAL, field_name);
}
+
+ vmcs_write(field, addr_saved);
}
#define TEST_RPL_TI_FLAGS(reg, name) \
--
2.24.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [kvm-unit-tests PATCH 4/8] nVMX: Drop redundant check for guest termination
2020-03-12 23:27 [kvm-unit-tests PATCH 0/8] nVMX: Clean up __enter_guest() and co Sean Christopherson
` (2 preceding siblings ...)
2020-03-12 23:27 ` [kvm-unit-tests PATCH 3/8] nVMX: Consolidate non-canonical code in test_canonical() Sean Christopherson
@ 2020-03-12 23:27 ` Sean Christopherson
2020-03-12 23:27 ` [kvm-unit-tests PATCH 5/8] nVMX: Expose __enter_guest() and consolidate guest state test code Sean Christopherson
` (4 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Sean Christopherson @ 2020-03-12 23:27 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kvm
Remove the check_for_guest_termination() call in
enter_guest_with_bad_controls() as __enter_guest() unconditionally
performs the check if VM-Enter is successful (and aborts on failed
VM-Entry for the ...bad_controls() variant).
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
---
x86/vmx.c | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/x86/vmx.c b/x86/vmx.c
index da17807..d92350d 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -1895,16 +1895,6 @@ void enter_guest_with_bad_controls(void)
report(vmcs_read(VMX_INST_ERROR) == VMXERR_ENTRY_INVALID_CONTROL_FIELD,
"VM-Inst Error # is %d (VM entry with invalid control field(s))",
VMXERR_ENTRY_INVALID_CONTROL_FIELD);
-
- /*
- * This if statement shouldn't fire, as the entire premise of this
- * function is that VM entry is expected to fail, rather than succeed
- * and execute to termination. However, if the VM entry does
- * unexpectedly succeed, it's nice to check whether the guest has
- * terminated, to reduce the number of error messages.
- */
- if (!result.vm_fail)
- check_for_guest_termination();
}
void enter_guest(void)
--
2.24.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [kvm-unit-tests PATCH 5/8] nVMX: Expose __enter_guest() and consolidate guest state test code
2020-03-12 23:27 [kvm-unit-tests PATCH 0/8] nVMX: Clean up __enter_guest() and co Sean Christopherson
` (3 preceding siblings ...)
2020-03-12 23:27 ` [kvm-unit-tests PATCH 4/8] nVMX: Drop redundant check for guest termination Sean Christopherson
@ 2020-03-12 23:27 ` Sean Christopherson
2020-03-12 23:27 ` [kvm-unit-tests PATCH 6/8] nVMX: Pass exit reason union to v1 exit handlers Sean Christopherson
` (3 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Sean Christopherson @ 2020-03-12 23:27 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kvm
Expose __enter_guest() outside of vmx.c and use it in a new wrapper for
testing guest state. Handling both success and failure paths in a
common helper eliminates a lot of boilerplate code in the tests
themselves.
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
---
x86/vmx.c | 12 +----
x86/vmx.h | 5 ++-
x86/vmx_tests.c | 115 +++++++++++++++---------------------------------
3 files changed, 40 insertions(+), 92 deletions(-)
diff --git a/x86/vmx.c b/x86/vmx.c
index d92350d..1c837f0 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -1840,14 +1840,11 @@ static void check_for_guest_termination(void)
}
}
-#define ABORT_ON_EARLY_VMENTRY_FAIL 0x1
-#define ABORT_ON_INVALID_GUEST_STATE 0x2
-
/*
* 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).
*/
-static void __enter_guest(u8 abort_flag, struct vmentry_result *result)
+void __enter_guest(u8 abort_flag, struct vmentry_result *result)
{
TEST_ASSERT_MSG(v2_guest_main,
"Never called test_set_guest_func!");
@@ -1905,13 +1902,6 @@ void enter_guest(void)
ABORT_ON_INVALID_GUEST_STATE, &result);
}
-void enter_guest_with_invalid_guest_state(void)
-{
- struct vmentry_result result;
-
- __enter_guest(ABORT_ON_EARLY_VMENTRY_FAIL, &result);
-}
-
extern struct vmx_test vmx_tests[];
static bool
diff --git a/x86/vmx.h b/x86/vmx.h
index c4a0fb4..73979f7 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -855,9 +855,12 @@ bool ept_huge_pages_supported(int level);
bool ept_execute_only_supported(void);
bool ept_ad_bits_supported(void);
+#define ABORT_ON_EARLY_VMENTRY_FAIL 0x1
+#define ABORT_ON_INVALID_GUEST_STATE 0x2
+
+void __enter_guest(u8 abort_flag, struct vmentry_result *result);
void enter_guest(void);
void enter_guest_with_bad_controls(void);
-void enter_guest_with_invalid_guest_state(void);
typedef void (*test_guest_func)(void);
typedef void (*test_teardown_func)(void *data);
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index ac02b9d..5befcd3 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -5239,23 +5239,25 @@ static void guest_state_test_main(void)
asm volatile("fnop");
}
-static void advance_guest_state_test(void)
+static void test_guest_state(const char *test, bool xfail, u64 field,
+ const char * field_name)
{
- u32 reason = vmcs_read(EXI_REASON);
- if (! (reason & 0x80000000)) {
- u64 guest_rip = vmcs_read(GUEST_RIP);
- u32 insn_len = vmcs_read(EXI_INST_LEN);
- vmcs_write(GUEST_RIP, guest_rip + insn_len);
- }
-}
+ struct vmentry_result result;
+ u8 abort_flags;
-static void report_guest_state_test(const char *test, u32 xreason,
- u64 field, const char * field_name)
-{
- u32 reason = vmcs_read(EXI_REASON);
+ abort_flags = ABORT_ON_EARLY_VMENTRY_FAIL;
+ if (!xfail)
+ abort_flags = ABORT_ON_INVALID_GUEST_STATE;
+
+ __enter_guest(abort_flags, &result);
+
+ report(result.exit_reason.failed_vmentry == xfail &&
+ ((xfail && result.exit_reason.basic == VMX_FAIL_STATE) ||
+ (!xfail && result.exit_reason.basic == VMX_VMCALL)),
+ "%s, %s %lx", test, field_name, field);
- report(reason == xreason, "%s, %s %lx", test, field_name, field);
- advance_guest_state_test();
+ if (!result.exit_reason.failed_vmentry)
+ skip_exit_insn();
}
/*
@@ -6911,16 +6913,7 @@ static void test_efer_vmlaunch(u32 fld, bool ok)
else
test_vmx_vmlaunch(VMXERR_ENTRY_INVALID_HOST_STATE_FIELD);
} else {
- if (ok) {
- enter_guest();
- report(vmcs_read(EXI_REASON) == VMX_VMCALL,
- "vmlaunch succeeds");
- } else {
- enter_guest_with_invalid_guest_state();
- report(vmcs_read(EXI_REASON) == (VMX_ENTRY_FAILURE | VMX_FAIL_STATE),
- "vmlaunch fails");
- }
- advance_guest_state_test();
+ test_guest_state("EFER test", !ok, GUEST_EFER, "GUEST_EFER");
}
}
@@ -7124,10 +7117,8 @@ static void test_pat(u32 field, const char * field_name, u32 ctrl_field,
report_prefix_pop();
} else { // GUEST_PAT
- enter_guest();
- report_guest_state_test("ENT_LOAD_PAT enabled",
- VMX_VMCALL, val,
- "GUEST_PAT");
+ test_guest_state("ENT_LOAD_PAT enabled", false,
+ val, "GUEST_PAT");
}
}
}
@@ -7151,21 +7142,9 @@ static void test_pat(u32 field, const char * field_name, u32 ctrl_field,
report_prefix_pop();
} else { // GUEST_PAT
- if (i == 0x2 || i == 0x3 || i >= 0x8) {
- enter_guest_with_invalid_guest_state();
- report_guest_state_test("ENT_LOAD_PAT "
- "enabled",
- VMX_FAIL_STATE | VMX_ENTRY_FAILURE,
- val,
- "GUEST_PAT");
- } else {
- enter_guest();
- report_guest_state_test("ENT_LOAD_PAT "
- "enabled",
- VMX_VMCALL,
- val,
- "GUEST_PAT");
- }
+ error = (i == 0x2 || i == 0x3 || i >= 0x8);
+ test_guest_state("ENT_LOAD_PAT enabled", !!error,
+ val, "GUEST_PAT");
}
}
@@ -7254,14 +7233,9 @@ static void test_pgc_vmlaunch(u32 xerror, u32 xreason, bool xfail, bool host)
report(success != xfail, "vmlaunch succeeded");
}
} else {
- if (xfail) {
- enter_guest_with_invalid_guest_state();
- } else {
- enter_guest();
- }
- report_guest_state_test("load GUEST_PERF_GLOBAL_CTRL",
- xreason, GUEST_PERF_GLOBAL_CTRL,
- "GUEST_PERF_GLOBAL_CTRL");
+ test_guest_state("load GUEST_PERF_GLOBAL_CTRL", xfail,
+ GUEST_PERF_GLOBAL_CTRL,
+ "GUEST_PERF_GLOBAL_CTRL");
}
}
@@ -7422,10 +7396,8 @@ static void test_canonical(u64 field, const char * field_name, bool host)
test_vmx_vmlaunch(0);
report_prefix_pop();
} else {
- enter_guest();
- report_guest_state_test("Test canonical address",
- VMX_VMCALL, addr_saved,
- field_name);
+ test_guest_state("Test canonical address", false,
+ addr_saved, field_name);
}
}
@@ -7436,10 +7408,8 @@ static void test_canonical(u64 field, const char * field_name, bool host)
test_vmx_vmlaunch(VMXERR_ENTRY_INVALID_HOST_STATE_FIELD);
report_prefix_pop();
} else {
- enter_guest_with_invalid_guest_state();
- report_guest_state_test("Test non-canonical address",
- VMX_FAIL_STATE | VMX_ENTRY_FAILURE,
- NONCANONICAL, field_name);
+ test_guest_state("Test non-canonical address", true,
+ NONCANONICAL, field_name);
}
vmcs_write(field, addr_saved);
@@ -7626,9 +7596,8 @@ static void test_guest_dr7(void)
for (i = 0; i < 64; i++) {
val = 1ull << i;
vmcs_write(GUEST_DR7, val);
- enter_guest();
- report_guest_state_test("ENT_LOAD_DBGCTLS disabled",
- VMX_VMCALL, val, "GUEST_DR7");
+ test_guest_state("ENT_LOAD_DBGCTLS disabled", false,
+ val, "GUEST_DR7");
}
}
if (ctrl_enter_rev.clr & ENT_LOAD_DBGCTLS) {
@@ -7636,15 +7605,8 @@ static void test_guest_dr7(void)
for (i = 0; i < 64; i++) {
val = 1ull << i;
vmcs_write(GUEST_DR7, val);
- if (i < 32)
- enter_guest();
- else
- enter_guest_with_invalid_guest_state();
- report_guest_state_test("ENT_LOAD_DBGCTLS enabled",
- i < 32 ? VMX_VMCALL :
- VMX_ENTRY_FAILURE |
- VMX_FAIL_STATE,
- val, "GUEST_DR7");
+ test_guest_state("ENT_LOAD_DBGCTLS enabled", i >= 32,
+ val, "GUEST_DR7");
}
}
vmcs_write(GUEST_DR7, dr7_saved);
@@ -9516,17 +9478,10 @@ static void atomic_switch_msrs_test(int count)
assert_exit_reason(VMX_VMCALL);
skip_exit_vmcall();
} else {
- u32 exit_reason;
- u32 exit_reason_want;
u32 exit_qual;
- enter_guest_with_invalid_guest_state();
-
- exit_reason = vmcs_read(EXI_REASON);
- exit_reason_want = VMX_FAIL_MSR | VMX_ENTRY_FAILURE;
- report(exit_reason == exit_reason_want,
- "exit_reason, %u, is %u.", exit_reason,
- exit_reason_want);
+ test_guest_state("Invalid MSR Load Count", true, count,
+ "ENT_MSR_LD_CNT");
exit_qual = vmcs_read(EXI_QUALIFICATION);
report(exit_qual == max_allowed + 1, "exit_qual, %u, is %u.",
--
2.24.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [kvm-unit-tests PATCH 6/8] nVMX: Pass exit reason union to v1 exit handlers
2020-03-12 23:27 [kvm-unit-tests PATCH 0/8] nVMX: Clean up __enter_guest() and co Sean Christopherson
` (4 preceding siblings ...)
2020-03-12 23:27 ` [kvm-unit-tests PATCH 5/8] nVMX: Expose __enter_guest() and consolidate guest state test code Sean Christopherson
@ 2020-03-12 23:27 ` Sean Christopherson
2020-03-12 23:27 ` [kvm-unit-tests PATCH 7/8] nVMX: Pass exit reason union to is_hypercall() Sean Christopherson
` (2 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Sean Christopherson @ 2020-03-12 23:27 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kvm
Pass the recently introduce "union exit_reason" to the v1 exit handlers
and use it in lieu of a manual VMREAD of the exit reason.
Opportunistically fix a variety of warts in the handlers, e.g. grabbing
only bits 7:0 of the exit reason. Modify the "Unknown exit reason"
prints to display the exit reason in hex format to make a failed
VM-Entry more recognizable.
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
---
x86/vmx.c | 6 +-
x86/vmx.h | 48 ++++++++--------
x86/vmx_tests.c | 149 +++++++++++++++++++-----------------------------
3 files changed, 88 insertions(+), 115 deletions(-)
diff --git a/x86/vmx.c b/x86/vmx.c
index 1c837f0..35d7fc7 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -1618,7 +1618,7 @@ void test_skip(const char *msg)
abort();
}
-static int exit_handler(void)
+static int exit_handler(union exit_reason exit_reason)
{
int ret;
@@ -1627,7 +1627,7 @@ static int exit_handler(void)
if (is_hypercall())
ret = handle_hypercall();
else
- ret = current->exit_handler();
+ ret = current->exit_handler(exit_reason);
vmcs_write(GUEST_RFLAGS, regs.rflags);
return ret;
@@ -1690,7 +1690,7 @@ static int vmx_run(void)
* entry failure (early or otherwise).
*/
launched = 1;
- ret = exit_handler();
+ ret = exit_handler(result.exit_reason);
} else if (current->entry_failure_handler) {
ret = current->entry_failure_handler(&result);
} else {
diff --git a/x86/vmx.h b/x86/vmx.h
index 73979f7..b79cbc1 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -44,6 +44,29 @@ struct regs {
u64 rflags;
};
+union exit_reason {
+ struct {
+ u32 basic : 16;
+ u32 reserved16 : 1;
+ u32 reserved17 : 1;
+ u32 reserved18 : 1;
+ u32 reserved19 : 1;
+ u32 reserved20 : 1;
+ u32 reserved21 : 1;
+ u32 reserved22 : 1;
+ u32 reserved23 : 1;
+ u32 reserved24 : 1;
+ u32 reserved25 : 1;
+ u32 reserved26 : 1;
+ u32 enclave_mode : 1;
+ u32 smi_pending_mtf : 1;
+ u32 smi_from_vmx_root : 1;
+ u32 reserved30 : 1;
+ u32 failed_vmentry : 1;
+ };
+ u32 full;
+};
+
struct vmentry_result {
/* Instruction mnemonic (for convenience). */
const char *instr;
@@ -54,28 +77,7 @@ struct vmentry_result {
/* Did the VM-Entry fully enter the guest? */
bool entered;
/* VM-Exit reason, valid iff !vm_fail */
- union {
- struct {
- u32 basic : 16;
- u32 reserved16 : 1;
- u32 reserved17 : 1;
- u32 reserved18 : 1;
- u32 reserved19 : 1;
- u32 reserved20 : 1;
- u32 reserved21 : 1;
- u32 reserved22 : 1;
- u32 reserved23 : 1;
- u32 reserved24 : 1;
- u32 reserved25 : 1;
- u32 reserved26 : 1;
- u32 enclave_mode : 1;
- u32 smi_pending_mtf : 1;
- u32 smi_from_vmx_root : 1;
- u32 reserved30 : 1;
- u32 failed_vmentry : 1;
- };
- u32 full;
- } exit_reason;
+ union exit_reason exit_reason;
/* Contents of [re]flags after failed entry. */
unsigned long flags;
};
@@ -84,7 +86,7 @@ struct vmx_test {
const char *name;
int (*init)(struct vmcs *vmcs);
void (*guest_main)(void);
- int (*exit_handler)(void);
+ int (*exit_handler)(union exit_reason exit_reason);
void (*syscall_handler)(u64 syscall_no);
struct regs guest_regs;
int (*entry_failure_handler)(struct vmentry_result *result);
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index 5befcd3..f46a0b9 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -58,7 +58,7 @@ static void basic_guest_main(void)
report(1, "Basic VMX test");
}
-static int basic_exit_handler(void)
+static int basic_exit_handler(union exit_reason exit_reason)
{
report(0, "Basic VMX test");
print_vmexit_info();
@@ -83,14 +83,11 @@ static void vmenter_main(void)
report((rax == 0xFFFF) && (rsp == resume_rsp), "test vmresume");
}
-static int vmenter_exit_handler(void)
+static int vmenter_exit_handler(union exit_reason exit_reason)
{
- u64 guest_rip;
- ulong reason;
+ u64 guest_rip = vmcs_read(GUEST_RIP);
- guest_rip = vmcs_read(GUEST_RIP);
- reason = vmcs_read(EXI_REASON) & 0xff;
- switch (reason) {
+ switch (exit_reason.basic) {
case VMX_VMCALL:
if (regs.rax != 0xABCD) {
report(0, "test vmresume");
@@ -152,18 +149,16 @@ static void preemption_timer_main(void)
vmcall();
}
-static int preemption_timer_exit_handler(void)
+static int preemption_timer_exit_handler(union exit_reason exit_reason)
{
bool guest_halted;
u64 guest_rip;
- ulong reason;
u32 insn_len;
u32 ctrl_exit;
guest_rip = vmcs_read(GUEST_RIP);
- reason = vmcs_read(EXI_REASON) & 0xff;
insn_len = vmcs_read(EXI_INST_LEN);
- switch (reason) {
+ switch (exit_reason.basic) {
case VMX_PREEMPT:
switch (vmx_get_test_stage()) {
case 1:
@@ -240,7 +235,7 @@ static int preemption_timer_exit_handler(void)
}
break;
default:
- report(false, "Unknown exit reason, %ld", reason);
+ report(false, "Unknown exit reason, 0x%x", exit_reason.full);
print_vmexit_info();
}
vmcs_write(PIN_CONTROLS, vmcs_read(PIN_CONTROLS) & ~PIN_PREEMPT);
@@ -335,15 +330,13 @@ static void test_ctrl_pat_main(void)
report(guest_ia32_pat == ia32_pat, "Entry load PAT");
}
-static int test_ctrl_pat_exit_handler(void)
+static int test_ctrl_pat_exit_handler(union exit_reason exit_reason)
{
u64 guest_rip;
- ulong reason;
u64 guest_pat;
guest_rip = vmcs_read(GUEST_RIP);
- reason = vmcs_read(EXI_REASON) & 0xff;
- switch (reason) {
+ switch (exit_reason.basic) {
case VMX_VMCALL:
guest_pat = vmcs_read(GUEST_PAT);
if (!(ctrl_exit_rev.clr & EXI_SAVE_PAT)) {
@@ -361,7 +354,7 @@ static int test_ctrl_pat_exit_handler(void)
vmcs_write(GUEST_RIP, guest_rip + 3);
return VMX_TEST_RESUME;
default:
- printf("ERROR : Undefined exit reason, reason = %ld.\n", reason);
+ printf("ERROR : Unknown exit reason, 0x%x.\n", exit_reason.full);
break;
}
return VMX_TEST_VMEXIT;
@@ -403,15 +396,13 @@ static void test_ctrl_efer_main(void)
report(guest_ia32_efer == ia32_efer, "Entry load EFER");
}
-static int test_ctrl_efer_exit_handler(void)
+static int test_ctrl_efer_exit_handler(union exit_reason exit_reason)
{
u64 guest_rip;
- ulong reason;
u64 guest_efer;
guest_rip = vmcs_read(GUEST_RIP);
- reason = vmcs_read(EXI_REASON) & 0xff;
- switch (reason) {
+ switch (exit_reason.basic) {
case VMX_VMCALL:
guest_efer = vmcs_read(GUEST_EFER);
if (!(ctrl_exit_rev.clr & EXI_SAVE_EFER)) {
@@ -431,7 +422,7 @@ static int test_ctrl_efer_exit_handler(void)
vmcs_write(GUEST_RIP, guest_rip + 3);
return VMX_TEST_RESUME;
default:
- printf("ERROR : Undefined exit reason, reason = %ld.\n", reason);
+ printf("ERROR : Unknown exit reason, 0x%x.\n", exit_reason.full);
break;
}
return VMX_TEST_VMEXIT;
@@ -529,18 +520,16 @@ static void cr_shadowing_main(void)
"Write shadowing different X86_CR4_DE");
}
-static int cr_shadowing_exit_handler(void)
+static int cr_shadowing_exit_handler(union exit_reason exit_reason)
{
u64 guest_rip;
- ulong reason;
u32 insn_len;
u32 exit_qual;
guest_rip = vmcs_read(GUEST_RIP);
- reason = vmcs_read(EXI_REASON) & 0xff;
insn_len = vmcs_read(EXI_INST_LEN);
exit_qual = vmcs_read(EXI_QUALIFICATION);
- switch (reason) {
+ switch (exit_reason.basic) {
case VMX_VMCALL:
switch (vmx_get_test_stage()) {
case 0:
@@ -624,7 +613,7 @@ static int cr_shadowing_exit_handler(void)
vmcs_write(GUEST_RIP, guest_rip + insn_len);
return VMX_TEST_RESUME;
default:
- report(false, "Unknown exit reason, %ld", reason);
+ report(false, "Unknown exit reason, 0x%x", exit_reason.full);
print_vmexit_info();
}
return VMX_TEST_VMEXIT;
@@ -694,17 +683,16 @@ static void iobmp_main(void)
"I/O bitmap - unconditional exiting");
}
-static int iobmp_exit_handler(void)
+static int iobmp_exit_handler(union exit_reason exit_reason)
{
u64 guest_rip;
- ulong reason, exit_qual;
+ ulong exit_qual;
u32 insn_len, ctrl_cpu0;
guest_rip = vmcs_read(GUEST_RIP);
- reason = vmcs_read(EXI_REASON) & 0xff;
exit_qual = vmcs_read(EXI_QUALIFICATION);
insn_len = vmcs_read(EXI_INST_LEN);
- switch (reason) {
+ switch (exit_reason.basic) {
case VMX_IO:
switch (vmx_get_test_stage()) {
case 0:
@@ -784,7 +772,7 @@ static int iobmp_exit_handler(void)
return VMX_TEST_RESUME;
default:
printf("guest_rip = %#lx\n", guest_rip);
- printf("\tERROR : Undefined exit reason, reason = %ld.\n", reason);
+ printf("\tERROR : Unknown exit reason, 0x%x\n", exit_reason.full);
break;
}
return VMX_TEST_VMEXIT;
@@ -989,22 +977,20 @@ static void insn_intercept_main(void)
}
}
-static int insn_intercept_exit_handler(void)
+static int insn_intercept_exit_handler(union exit_reason exit_reason)
{
u64 guest_rip;
- u32 reason;
ulong exit_qual;
u32 insn_len;
u32 insn_info;
bool pass;
guest_rip = vmcs_read(GUEST_RIP);
- reason = vmcs_read(EXI_REASON) & 0xff;
exit_qual = vmcs_read(EXI_QUALIFICATION);
insn_len = vmcs_read(EXI_INST_LEN);
insn_info = vmcs_read(EXI_INST_INFO);
- if (reason == VMX_VMCALL) {
+ if (exit_reason.basic == VMX_VMCALL) {
u32 val = 0;
if (insn_table[cur_insn].type == INSN_CPU0)
@@ -1023,7 +1009,7 @@ static int insn_intercept_exit_handler(void)
vmcs_write(CPU_EXEC_CTRL1, val | ctrl_cpu_rev[1].set);
} else {
pass = (cur_insn * 2 == vmx_get_test_stage()) &&
- insn_table[cur_insn].reason == reason;
+ insn_table[cur_insn].reason == exit_reason.full;
if (insn_table[cur_insn].test_field & FIELD_EXIT_QUAL &&
insn_table[cur_insn].exit_qual != exit_qual)
pass = false;
@@ -1275,16 +1261,15 @@ static bool invept_test(int type, u64 eptp)
return true;
}
-static int pml_exit_handler(void)
+static int pml_exit_handler(union exit_reason exit_reason)
{
u16 index, count;
- ulong reason = vmcs_read(EXI_REASON) & 0xff;
u64 *pmlbuf = pml_log;
u64 guest_rip = vmcs_read(GUEST_RIP);;
u64 guest_cr3 = vmcs_read(GUEST_CR3);
u32 insn_len = vmcs_read(EXI_INST_LEN);
- switch (reason) {
+ switch (exit_reason.basic) {
case VMX_VMCALL:
switch (vmx_get_test_stage()) {
case 0:
@@ -1315,27 +1300,25 @@ static int pml_exit_handler(void)
vmcs_write(GUEST_PML_INDEX, PML_INDEX - 1);
return VMX_TEST_RESUME;
default:
- report(false, "Unknown exit reason, %ld", reason);
+ report(false, "Unknown exit reason, 0x%x", exit_reason.full);
print_vmexit_info();
}
return VMX_TEST_VMEXIT;
}
-static int ept_exit_handler_common(bool have_ad)
+static int ept_exit_handler_common(union exit_reason exit_reason, bool have_ad)
{
u64 guest_rip;
u64 guest_cr3;
- ulong reason;
u32 insn_len;
u32 exit_qual;
static unsigned long data_page1_pte, data_page1_pte_pte, memaddr_pte;
guest_rip = vmcs_read(GUEST_RIP);
guest_cr3 = vmcs_read(GUEST_CR3);
- reason = vmcs_read(EXI_REASON) & 0xff;
insn_len = vmcs_read(EXI_INST_LEN);
exit_qual = vmcs_read(EXI_QUALIFICATION);
- switch (reason) {
+ switch (exit_reason.basic) {
case VMX_VMCALL:
switch (vmx_get_test_stage()) {
case 0:
@@ -1483,15 +1466,15 @@ static int ept_exit_handler_common(bool have_ad)
}
return VMX_TEST_RESUME;
default:
- report(false, "Unknown exit reason, %ld", reason);
+ report(false, "Unknown exit reason, 0x%x", exit_reason.full);
print_vmexit_info();
}
return VMX_TEST_VMEXIT;
}
-static int ept_exit_handler(void)
+static int ept_exit_handler(union exit_reason exit_reason)
{
- return ept_exit_handler_common(false);
+ return ept_exit_handler_common(exit_reason, false);
}
static int eptad_init(struct vmcs *vmcs)
@@ -1556,9 +1539,9 @@ static void eptad_main(void)
ept_common();
}
-static int eptad_exit_handler(void)
+static int eptad_exit_handler(union exit_reason exit_reason)
{
- return ept_exit_handler_common(true);
+ return ept_exit_handler_common(exit_reason, true);
}
static bool invvpid_test(int type, u16 vpid)
@@ -1609,17 +1592,15 @@ static void vpid_main(void)
report(vmx_get_test_stage() == 5, "INVVPID ALL");
}
-static int vpid_exit_handler(void)
+static int vpid_exit_handler(union exit_reason exit_reason)
{
u64 guest_rip;
- ulong reason;
u32 insn_len;
guest_rip = vmcs_read(GUEST_RIP);
- reason = vmcs_read(EXI_REASON) & 0xff;
insn_len = vmcs_read(EXI_INST_LEN);
- switch (reason) {
+ switch (exit_reason.basic) {
case VMX_VMCALL:
switch(vmx_get_test_stage()) {
case 0:
@@ -1643,7 +1624,7 @@ static int vpid_exit_handler(void)
vmcs_write(GUEST_RIP, guest_rip + insn_len);
return VMX_TEST_RESUME;
default:
- report(false, "Unknown exit reason, %ld", reason);
+ report(false, "Unknown exit reason, 0x%x", exit_reason.full);
print_vmexit_info();
}
return VMX_TEST_VMEXIT;
@@ -1761,13 +1742,12 @@ static void interrupt_main(void)
report(timer_fired, "Inject an event to a halted guest");
}
-static int interrupt_exit_handler(void)
+static int interrupt_exit_handler(union exit_reason exit_reason)
{
u64 guest_rip = vmcs_read(GUEST_RIP);
- ulong reason = vmcs_read(EXI_REASON) & 0xff;
u32 insn_len = vmcs_read(EXI_INST_LEN);
- switch (reason) {
+ switch (exit_reason.basic) {
case VMX_VMCALL:
switch (vmx_get_test_stage()) {
case 0:
@@ -1815,7 +1795,7 @@ static int interrupt_exit_handler(void)
vmcs_write(GUEST_ACTV_STATE, ACTV_ACTIVE);
return VMX_TEST_RESUME;
default:
- report(false, "Unknown exit reason, %ld", reason);
+ report(false, "Unknown exit reason, 0x%x", exit_reason.full);
print_vmexit_info();
}
@@ -1886,9 +1866,8 @@ static void dbgctls_main(void)
report(vmx_get_test_stage() == 4, "Don't save debug controls");
}
-static int dbgctls_exit_handler(void)
+static int dbgctls_exit_handler(union exit_reason exit_reason)
{
- unsigned int reason = vmcs_read(EXI_REASON) & 0xff;
u32 insn_len = vmcs_read(EXI_INST_LEN);
u64 guest_rip = vmcs_read(GUEST_RIP);
u64 dr7, debugctl;
@@ -1896,7 +1875,7 @@ static int dbgctls_exit_handler(void)
asm volatile("mov %%dr7,%0" : "=r" (dr7));
debugctl = rdmsr(MSR_IA32_DEBUGCTLMSR);
- switch (reason) {
+ switch (exit_reason.basic) {
case VMX_VMCALL:
switch (vmx_get_test_stage()) {
case 0:
@@ -1929,7 +1908,7 @@ static int dbgctls_exit_handler(void)
vmcs_write(GUEST_RIP, guest_rip + insn_len);
return VMX_TEST_RESUME;
default:
- report(false, "Unknown exit reason, %d", reason);
+ report(false, "Unknown exit reason, %d", exit_reason.full);
print_vmexit_info();
}
return VMX_TEST_VMEXIT;
@@ -1977,12 +1956,9 @@ static void msr_switch_main(void)
vmcall();
}
-static int msr_switch_exit_handler(void)
+static int msr_switch_exit_handler(union exit_reason exit_reason)
{
- ulong reason;
-
- reason = vmcs_read(EXI_REASON);
- if (reason == VMX_VMCALL && vmx_get_test_stage() == 2) {
+ if (exit_reason.basic == VMX_VMCALL && vmx_get_test_stage() == 2) {
report(exit_msr_store[0].value == MSR_MAGIC + 1,
"VM exit MSR store");
report(rdmsr(MSR_KERNEL_GS_BASE) == MSR_MAGIC + 2,
@@ -1991,8 +1967,8 @@ static int msr_switch_exit_handler(void)
entry_msr_load[0].index = MSR_FS_BASE;
return VMX_TEST_RESUME;
}
- printf("ERROR %s: unexpected stage=%u or reason=%lu\n",
- __func__, vmx_get_test_stage(), reason);
+ printf("ERROR %s: unexpected stage=%u or reason=0x%x\n",
+ __func__, vmx_get_test_stage(), exit_reason.full);
return VMX_TEST_EXIT;
}
@@ -2031,12 +2007,9 @@ static void vmmcall_main(void)
report(0, "VMMCALL");
}
-static int vmmcall_exit_handler(void)
+static int vmmcall_exit_handler(union exit_reason exit_reason)
{
- ulong reason;
-
- reason = vmcs_read(EXI_REASON);
- switch (reason) {
+ switch (exit_reason.basic) {
case VMX_VMCALL:
printf("here\n");
report(0, "VMMCALL triggers #UD");
@@ -2046,7 +2019,7 @@ static int vmmcall_exit_handler(void)
"VMMCALL triggers #UD");
break;
default:
- report(false, "Unknown exit reason, %ld", reason);
+ report(false, "Unknown exit reason, 0x%x", exit_reason.full);
print_vmexit_info();
}
@@ -2098,11 +2071,9 @@ static void disable_rdtscp_main(void)
vmcall();
}
-static int disable_rdtscp_exit_handler(void)
+static int disable_rdtscp_exit_handler(union exit_reason exit_reason)
{
- unsigned int reason = vmcs_read(EXI_REASON) & 0xff;
-
- switch (reason) {
+ switch (exit_reason.basic) {
case VMX_VMCALL:
switch (vmx_get_test_stage()) {
case 0:
@@ -2120,7 +2091,7 @@ static int disable_rdtscp_exit_handler(void)
break;
default:
- report(false, "Unknown exit reason, %d", reason);
+ report(false, "Unknown exit reason, 0x%x", exit_reason.full);
print_vmexit_info();
}
return VMX_TEST_VMEXIT;
@@ -2137,12 +2108,12 @@ static void int3_guest_main(void)
asm volatile ("int3");
}
-static int int3_exit_handler(void)
+static int int3_exit_handler(union exit_reason exit_reason)
{
- u32 reason = vmcs_read(EXI_REASON);
u32 intr_info = vmcs_read(EXI_INTR_INFO);
- report(reason == VMX_EXC_NMI && (intr_info & INTR_INFO_VALID_MASK) &&
+ report(exit_reason.basic == VMX_EXC_NMI &&
+ (intr_info & INTR_INFO_VALID_MASK) &&
(intr_info & INTR_INFO_VECTOR_MASK) == BP_VECTOR &&
((intr_info & INTR_INFO_INTR_TYPE_MASK) >>
INTR_INFO_INTR_TYPE_SHIFT) == VMX_INTR_TYPE_SOFT_EXCEPTION,
@@ -2186,12 +2157,12 @@ into:
__builtin_unreachable();
}
-static int into_exit_handler(void)
+static int into_exit_handler(union exit_reason exit_reason)
{
- u32 reason = vmcs_read(EXI_REASON);
u32 intr_info = vmcs_read(EXI_INTR_INFO);
- report(reason == VMX_EXC_NMI && (intr_info & INTR_INFO_VALID_MASK) &&
+ report(exit_reason.basic == VMX_EXC_NMI &&
+ (intr_info & INTR_INFO_VALID_MASK) &&
(intr_info & INTR_INFO_VECTOR_MASK) == OF_VECTOR &&
((intr_info & INTR_INFO_INTR_TYPE_MASK) >>
INTR_INFO_INTR_TYPE_SHIFT) == VMX_INTR_TYPE_SOFT_EXCEPTION,
@@ -2206,7 +2177,7 @@ static void exit_monitor_from_l2_main(void)
exit(0);
}
-static int exit_monitor_from_l2_handler(void)
+static int exit_monitor_from_l2_handler(union exit_reason exit_reason)
{
report(false, "The guest should have killed the VMM");
return VMX_TEST_EXIT;
@@ -9375,7 +9346,7 @@ static void invalid_msr_main(void)
report(0, "Invalid MSR load");
}
-static int invalid_msr_exit_handler(void)
+static int invalid_msr_exit_handler(union exit_reason exit_reason)
{
report(0, "Invalid MSR load");
print_vmexit_info();
--
2.24.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [kvm-unit-tests PATCH 7/8] nVMX: Pass exit reason union to is_hypercall()
2020-03-12 23:27 [kvm-unit-tests PATCH 0/8] nVMX: Clean up __enter_guest() and co Sean Christopherson
` (5 preceding siblings ...)
2020-03-12 23:27 ` [kvm-unit-tests PATCH 6/8] nVMX: Pass exit reason union to v1 exit handlers Sean Christopherson
@ 2020-03-12 23:27 ` Sean Christopherson
2020-03-12 23:27 ` [kvm-unit-tests PATCH 8/8] nVMX: Pass exit reason enum to print_vmexit_info() Sean Christopherson
2020-03-14 10:35 ` [kvm-unit-tests PATCH 0/8] nVMX: Clean up __enter_guest() and co Paolo Bonzini
8 siblings, 0 replies; 12+ messages in thread
From: Sean Christopherson @ 2020-03-12 23:27 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kvm
Pass the exit reason captured in VM-Entry results into __enter_guest()'s
helpers to simplify code and eliminate extra VMREADS.
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
---
x86/vmx.c | 19 +++++++------------
1 file changed, 7 insertions(+), 12 deletions(-)
diff --git a/x86/vmx.c b/x86/vmx.c
index 35d7fc7..f7f9665 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -1553,15 +1553,10 @@ static void __attribute__((__used__)) hypercall(u32 hypercall_no)
asm volatile("vmcall\n\t");
}
-static bool is_hypercall(void)
+static bool is_hypercall(union exit_reason exit_reason)
{
- ulong reason, hyper_bit;
-
- reason = vmcs_read(EXI_REASON) & 0xff;
- hyper_bit = hypercall_field & HYPERCALL_BIT;
- if (reason == VMX_VMCALL && hyper_bit)
- return true;
- return false;
+ return exit_reason.basic == VMX_VMCALL &&
+ (hypercall_field & HYPERCALL_BIT);
}
static int handle_hypercall(void)
@@ -1624,7 +1619,7 @@ static int exit_handler(union exit_reason exit_reason)
current->exits++;
regs.rflags = vmcs_read(GUEST_RFLAGS);
- if (is_hypercall())
+ if (is_hypercall(exit_reason))
ret = handle_hypercall();
else
ret = current->exit_handler(exit_reason);
@@ -1816,9 +1811,9 @@ void test_set_guest(test_guest_func func)
v2_guest_main = func;
}
-static void check_for_guest_termination(void)
+static void check_for_guest_termination(union exit_reason exit_reason)
{
- if (is_hypercall()) {
+ if (is_hypercall(exit_reason)) {
int ret;
ret = handle_hypercall();
@@ -1867,7 +1862,7 @@ void __enter_guest(u8 abort_flag, struct vmentry_result *result)
}
launched = 1;
- check_for_guest_termination();
+ check_for_guest_termination(result->exit_reason);
return;
do_abort:
--
2.24.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [kvm-unit-tests PATCH 8/8] nVMX: Pass exit reason enum to print_vmexit_info()
2020-03-12 23:27 [kvm-unit-tests PATCH 0/8] nVMX: Clean up __enter_guest() and co Sean Christopherson
` (6 preceding siblings ...)
2020-03-12 23:27 ` [kvm-unit-tests PATCH 7/8] nVMX: Pass exit reason union to is_hypercall() Sean Christopherson
@ 2020-03-12 23:27 ` Sean Christopherson
2020-03-14 10:35 ` [kvm-unit-tests PATCH 0/8] nVMX: Clean up __enter_guest() and co Paolo Bonzini
8 siblings, 0 replies; 12+ messages in thread
From: Sean Christopherson @ 2020-03-12 23:27 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kvm
Take the exit reason as a parameter when printing VM-Exit info instead
of rereading it from the VMCS. Opportunistically clean up the related
printing.
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
---
x86/vmx.c | 9 ++++-----
x86/vmx.h | 2 +-
x86/vmx_tests.c | 46 +++++++++++++++++++++++-----------------------
3 files changed, 28 insertions(+), 29 deletions(-)
diff --git a/x86/vmx.c b/x86/vmx.c
index f7f9665..4c47eec 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -585,17 +585,16 @@ const char *exit_reason_description(u64 reason)
return exit_reason_descriptions[reason] ? : "(unused)";
}
-void print_vmexit_info()
+void print_vmexit_info(union exit_reason exit_reason)
{
u64 guest_rip, guest_rsp;
- ulong reason = vmcs_read(EXI_REASON) & 0xff;
ulong exit_qual = vmcs_read(EXI_QUALIFICATION);
guest_rip = vmcs_read(GUEST_RIP);
guest_rsp = vmcs_read(GUEST_RSP);
printf("VMEXIT info:\n");
- printf("\tvmexit reason = %ld\n", reason);
+ printf("\tvmexit reason = %u\n", exit_reason.basic);
+ printf("\tfailed vmentry = %u\n", !!exit_reason.failed_vmentry);
printf("\texit qualification = %#lx\n", exit_qual);
- printf("\tBit 31 of reason = %lx\n", (vmcs_read(EXI_REASON) >> 31) & 1);
printf("\tguest_rip = %#lx\n", guest_rip);
printf("\tRAX=%#lx RBX=%#lx RCX=%#lx RDX=%#lx\n",
regs.rax, regs.rbx, regs.rcx, regs.rdx);
@@ -1708,7 +1707,7 @@ static int vmx_run(void)
}
if (result.entered)
- print_vmexit_info();
+ print_vmexit_info(result.exit_reason);
else
print_vmentry_failure_info(&result);
abort();
diff --git a/x86/vmx.h b/x86/vmx.h
index b79cbc1..e6ee776 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -826,7 +826,7 @@ void enable_vmx(void);
void init_vmx(u64 *vmxon_region);
const char *exit_reason_description(u64 reason);
-void print_vmexit_info(void);
+void print_vmexit_info(union exit_reason exit_reason);
void print_vmentry_failure_info(struct vmentry_result *result);
void ept_sync(int type, u64 eptp);
void vpid_sync(int type, u16 vpid);
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index f46a0b9..2b8ce03 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -61,7 +61,7 @@ static void basic_guest_main(void)
static int basic_exit_handler(union exit_reason exit_reason)
{
report(0, "Basic VMX test");
- print_vmexit_info();
+ print_vmexit_info(exit_reason);
return VMX_TEST_EXIT;
}
@@ -98,7 +98,7 @@ static int vmenter_exit_handler(union exit_reason exit_reason)
return VMX_TEST_RESUME;
default:
report(0, "test vmresume");
- print_vmexit_info();
+ print_vmexit_info(exit_reason);
}
return VMX_TEST_VMEXIT;
}
@@ -187,7 +187,7 @@ static int preemption_timer_exit_handler(union exit_reason exit_reason)
break;
default:
report(false, "Invalid stage.");
- print_vmexit_info();
+ print_vmexit_info(exit_reason);
break;
}
break;
@@ -230,13 +230,13 @@ static int preemption_timer_exit_handler(union exit_reason exit_reason)
// Should not reach here
report(false, "unexpected stage, %d",
vmx_get_test_stage());
- print_vmexit_info();
+ print_vmexit_info(exit_reason);
return VMX_TEST_VMEXIT;
}
break;
default:
report(false, "Unknown exit reason, 0x%x", exit_reason.full);
- print_vmexit_info();
+ print_vmexit_info(exit_reason);
}
vmcs_write(PIN_CONTROLS, vmcs_read(PIN_CONTROLS) & ~PIN_PREEMPT);
return VMX_TEST_VMEXIT;
@@ -568,7 +568,7 @@ static int cr_shadowing_exit_handler(union exit_reason exit_reason)
// Should not reach here
report(false, "unexpected stage, %d",
vmx_get_test_stage());
- print_vmexit_info();
+ print_vmexit_info(exit_reason);
return VMX_TEST_VMEXIT;
}
vmcs_write(GUEST_RIP, guest_rip + insn_len);
@@ -607,14 +607,14 @@ static int cr_shadowing_exit_handler(union exit_reason exit_reason)
// Should not reach here
report(false, "unexpected stage, %d",
vmx_get_test_stage());
- print_vmexit_info();
+ print_vmexit_info(exit_reason);
return VMX_TEST_VMEXIT;
}
vmcs_write(GUEST_RIP, guest_rip + insn_len);
return VMX_TEST_RESUME;
default:
report(false, "Unknown exit reason, 0x%x", exit_reason.full);
- print_vmexit_info();
+ print_vmexit_info(exit_reason);
}
return VMX_TEST_VMEXIT;
}
@@ -744,7 +744,7 @@ static int iobmp_exit_handler(union exit_reason exit_reason)
// Should not reach here
report(false, "unexpected stage, %d",
vmx_get_test_stage());
- print_vmexit_info();
+ print_vmexit_info(exit_reason);
return VMX_TEST_VMEXIT;
}
vmcs_write(GUEST_RIP, guest_rip + insn_len);
@@ -765,7 +765,7 @@ static int iobmp_exit_handler(union exit_reason exit_reason)
// Should not reach here
report(false, "unexpected stage, %d",
vmx_get_test_stage());
- print_vmexit_info();
+ print_vmexit_info(exit_reason);
return VMX_TEST_VMEXIT;
}
vmcs_write(GUEST_RIP, guest_rip + insn_len);
@@ -1290,7 +1290,7 @@ static int pml_exit_handler(union exit_reason exit_reason)
default:
report(false, "unexpected stage, %d.",
vmx_get_test_stage());
- print_vmexit_info();
+ print_vmexit_info(exit_reason);
return VMX_TEST_VMEXIT;
}
vmcs_write(GUEST_RIP, guest_rip + insn_len);
@@ -1301,7 +1301,7 @@ static int pml_exit_handler(union exit_reason exit_reason)
return VMX_TEST_RESUME;
default:
report(false, "Unknown exit reason, 0x%x", exit_reason.full);
- print_vmexit_info();
+ print_vmexit_info(exit_reason);
}
return VMX_TEST_VMEXIT;
}
@@ -1386,7 +1386,7 @@ static int ept_exit_handler_common(union exit_reason exit_reason, bool have_ad)
default:
report(false, "ERROR - unexpected stage, %d.",
vmx_get_test_stage());
- print_vmexit_info();
+ print_vmexit_info(exit_reason);
return VMX_TEST_VMEXIT;
}
vmcs_write(GUEST_RIP, guest_rip + insn_len);
@@ -1405,7 +1405,7 @@ static int ept_exit_handler_common(union exit_reason exit_reason, bool have_ad)
default:
report(false, "ERROR - unexpected stage, %d.",
vmx_get_test_stage());
- print_vmexit_info();
+ print_vmexit_info(exit_reason);
return VMX_TEST_VMEXIT;
}
return VMX_TEST_RESUME;
@@ -1461,13 +1461,13 @@ static int ept_exit_handler_common(union exit_reason exit_reason, bool have_ad)
// Should not reach here
report(false, "ERROR : unexpected stage, %d",
vmx_get_test_stage());
- print_vmexit_info();
+ print_vmexit_info(exit_reason);
return VMX_TEST_VMEXIT;
}
return VMX_TEST_RESUME;
default:
report(false, "Unknown exit reason, 0x%x", exit_reason.full);
- print_vmexit_info();
+ print_vmexit_info(exit_reason);
}
return VMX_TEST_VMEXIT;
}
@@ -1618,14 +1618,14 @@ static int vpid_exit_handler(union exit_reason exit_reason)
default:
report(false, "ERROR: unexpected stage, %d",
vmx_get_test_stage());
- print_vmexit_info();
+ print_vmexit_info(exit_reason);
return VMX_TEST_VMEXIT;
}
vmcs_write(GUEST_RIP, guest_rip + insn_len);
return VMX_TEST_RESUME;
default:
report(false, "Unknown exit reason, 0x%x", exit_reason.full);
- print_vmexit_info();
+ print_vmexit_info(exit_reason);
}
return VMX_TEST_VMEXIT;
}
@@ -1796,7 +1796,7 @@ static int interrupt_exit_handler(union exit_reason exit_reason)
return VMX_TEST_RESUME;
default:
report(false, "Unknown exit reason, 0x%x", exit_reason.full);
- print_vmexit_info();
+ print_vmexit_info(exit_reason);
}
return VMX_TEST_VMEXIT;
@@ -1909,7 +1909,7 @@ static int dbgctls_exit_handler(union exit_reason exit_reason)
return VMX_TEST_RESUME;
default:
report(false, "Unknown exit reason, %d", exit_reason.full);
- print_vmexit_info();
+ print_vmexit_info(exit_reason);
}
return VMX_TEST_VMEXIT;
}
@@ -2020,7 +2020,7 @@ static int vmmcall_exit_handler(union exit_reason exit_reason)
break;
default:
report(false, "Unknown exit reason, 0x%x", exit_reason.full);
- print_vmexit_info();
+ print_vmexit_info(exit_reason);
}
return VMX_TEST_VMEXIT;
@@ -2092,7 +2092,7 @@ static int disable_rdtscp_exit_handler(union exit_reason exit_reason)
default:
report(false, "Unknown exit reason, 0x%x", exit_reason.full);
- print_vmexit_info();
+ print_vmexit_info(exit_reason);
}
return VMX_TEST_VMEXIT;
}
@@ -9349,7 +9349,7 @@ static void invalid_msr_main(void)
static int invalid_msr_exit_handler(union exit_reason exit_reason)
{
report(0, "Invalid MSR load");
- print_vmexit_info();
+ print_vmexit_info(exit_reason);
return VMX_TEST_EXIT;
}
--
2.24.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [kvm-unit-tests PATCH 0/8] nVMX: Clean up __enter_guest() and co.
2020-03-12 23:27 [kvm-unit-tests PATCH 0/8] nVMX: Clean up __enter_guest() and co Sean Christopherson
` (7 preceding siblings ...)
2020-03-12 23:27 ` [kvm-unit-tests PATCH 8/8] nVMX: Pass exit reason enum to print_vmexit_info() Sean Christopherson
@ 2020-03-14 10:35 ` Paolo Bonzini
8 siblings, 0 replies; 12+ messages in thread
From: Paolo Bonzini @ 2020-03-14 10:35 UTC (permalink / raw)
To: Sean Christopherson; +Cc: kvm
On 13/03/20 00:27, Sean Christopherson wrote:
> Start chipping away at the crustiness in the nVMX tests by refactoring
> "struct vmentry_failure" into "struct vmentry_result", with the full
> VM-Exit stored in vmentry_result. Capturing the exit reason allows for a
> variety of cleanups and consolidations.
>
> This series really only dives into the v1 tests. I'd like to also clean
> up the v2 tests, e.g. take the expected exit reason in enter_guest() so
> that the expected behavior is more obvious, but that's a more invasive
> cleanup for another day.
>
> Sean Christopherson (8):
> nVMX: Eliminate superfluous entry_failure_handler() wrapper
> nVMX: Refactor VM-Entry "failure" struct into "result"
> nVMX: Consolidate non-canonical code in test_canonical()
> nVMX: Drop redundant check for guest termination
> nVMX: Expose __enter_guest() and consolidate guest state test code
> nVMX: Pass exit reason union to v1 exit handlers
> nVMX: Pass exit reason union to is_hypercall()
> nVMX: Pass exit reason enum to print_vmexit_info()
>
> x86/vmx.c | 191 +++++++++++--------------
> x86/vmx.h | 50 +++++--
> x86/vmx_tests.c | 366 +++++++++++++++++++-----------------------------
> 3 files changed, 263 insertions(+), 344 deletions(-)
>
Queued, thanks.
Paolo
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [kvm-unit-tests PATCH 2/8] nVMX: Refactor VM-Entry "failure" struct into "result"
2020-03-12 23:27 ` [kvm-unit-tests PATCH 2/8] nVMX: Refactor VM-Entry "failure" struct into "result" Sean Christopherson
@ 2020-03-18 23:40 ` Krish Sadhukhan
2020-03-19 9:56 ` Paolo Bonzini
0 siblings, 1 reply; 12+ messages in thread
From: Krish Sadhukhan @ 2020-03-18 23:40 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini; +Cc: kvm
On 3/12/20 4:27 PM, Sean Christopherson wrote:
> Rename "struct vmentry_failure" to "struct vmentry_result" and add the
> full VM-Exit reason to the result. Implement exit_reason as a union so
> that tests can easily pull out the parts of interest, e.g. basic exit
> reason, whether VM-Entry failed, etc...
>
> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> ---
> x86/vmx.c | 128 ++++++++++++++++++++++++++----------------------
> x86/vmx.h | 39 ++++++++++++---
> x86/vmx_tests.c | 24 ++++-----
> 3 files changed, 112 insertions(+), 79 deletions(-)
>
> diff --git a/x86/vmx.c b/x86/vmx.c
> index 99c3791..da17807 100644
> --- a/x86/vmx.c
> +++ b/x86/vmx.c
> @@ -607,11 +607,14 @@ void print_vmexit_info()
> regs.r12, regs.r13, regs.r14, regs.r15);
> }
>
> -void
> -print_vmentry_failure_info(struct vmentry_failure *failure) {
> - if (failure->early) {
> - printf("Early %s failure: ", failure->instr);
> - switch (failure->flags & VMX_ENTRY_FLAGS) {
> +void print_vmentry_failure_info(struct vmentry_result *result)
> +{
> + if (result->entered)
> + return;
> +
> + if (result->vm_fail) {
> + printf("VM-Fail on %s: ", result->instr);
> + switch (result->flags & VMX_ENTRY_FLAGS) {
> case X86_EFLAGS_CF:
> printf("current-VMCS pointer is not valid.\n");
> break;
> @@ -620,16 +623,15 @@ print_vmentry_failure_info(struct vmentry_failure *failure) {
> vmcs_read(VMX_INST_ERROR));
> break;
> default:
> - printf("unexpected flags %lx!\n", failure->flags);
> + printf("unexpected flags %lx!\n", result->flags);
> }
> } else {
> - u64 reason = vmcs_read(EXI_REASON);
> u64 qual = vmcs_read(EXI_QUALIFICATION);
>
> - printf("Non-early %s failure (reason=%#lx, qual=%#lx): ",
> - failure->instr, reason, qual);
> + printf("VM-Exit failure on %s (reason=%#x, qual=%#lx): ",
> + result->instr, result->exit_reason.full, qual);
>
> - switch (reason & 0xff) {
> + switch (result->exit_reason.basic) {
> case VMX_FAIL_STATE:
> printf("invalid guest state\n");
> break;
> @@ -640,14 +642,14 @@ print_vmentry_failure_info(struct vmentry_failure *failure) {
> printf("machine-check event\n");
> break;
> default:
> - printf("unexpected basic exit reason %ld\n",
> - reason & 0xff);
> + printf("unexpected basic exit reason %u\n",
> + result->exit_reason.basic);
> }
>
> - if (!(reason & VMX_ENTRY_FAILURE))
> + if (!result->exit_reason.failed_vmentry)
> printf("\tVMX_ENTRY_FAILURE BIT NOT SET!\n");
>
> - if (reason & 0x7fff0000)
> + if (result->exit_reason.full & 0x7fff0000)
> printf("\tRESERVED BITS SET!\n");
> }
> }
> @@ -1632,12 +1634,12 @@ static int exit_handler(void)
> }
>
> /*
> - * Tries to enter the guest. Returns true if entry succeeded. Otherwise,
> - * populates @failure.
> + * Tries to enter the guest, populates @result with VM-Fail, VM-Exit, entered,
> + * etc...
> */
> -static void vmx_enter_guest(struct vmentry_failure *failure)
> +static void vmx_enter_guest(struct vmentry_result *result)
> {
> - failure->early = 0;
> + memset(result, 0, sizeof(*result));
>
> in_guest = 1;
> asm volatile (
> @@ -1654,35 +1656,35 @@ static void vmx_enter_guest(struct vmentry_failure *failure)
> SAVE_GPR_C
> "pushf\n\t"
> "pop %%rdi\n\t"
> - "mov %%rdi, %[failure_flags]\n\t"
> - "movl $1, %[failure_early]\n\t"
> + "mov %%rdi, %[vm_fail_flags]\n\t"
> + "movl $1, %[vm_fail]\n\t"
> "jmp 3f\n\t"
> "vmx_return:\n\t"
> SAVE_GPR_C
> "3: \n\t"
> - : [failure_early]"+m"(failure->early),
> - [failure_flags]"=m"(failure->flags)
> + : [vm_fail]"+m"(result->vm_fail),
> + [vm_fail_flags]"=m"(result->flags)
> : [launched]"m"(launched), [HOST_RSP]"i"(HOST_RSP)
> : "rdi", "memory", "cc"
> );
> in_guest = 0;
>
> - failure->vmlaunch = !launched;
> - failure->instr = launched ? "vmresume" : "vmlaunch";
> + result->vmlaunch = !launched;
> + result->instr = launched ? "vmresume" : "vmlaunch";
> + result->exit_reason.full = result->vm_fail ? 0xdead :
> + vmcs_read(EXI_REASON);
> + result->entered = !result->vm_fail &&
> + !result->exit_reason.failed_vmentry;
> }
>
> static int vmx_run(void)
> {
> + struct vmentry_result result;
> + u32 ret;
> +
> while (1) {
> - u32 ret;
> - bool entered;
> - struct vmentry_failure failure;
> -
> - vmx_enter_guest(&failure);
> - entered = !failure.early &&
> - !(vmcs_read(EXI_REASON) & VMX_ENTRY_FAILURE);
> -
> - if (entered) {
> + vmx_enter_guest(&result);
> + if (result.entered) {
> /*
> * VMCS isn't in "launched" state if there's been any
> * entry failure (early or otherwise).
> @@ -1690,7 +1692,7 @@ static int vmx_run(void)
> launched = 1;
> ret = exit_handler();
> } else if (current->entry_failure_handler) {
> - ret = current->entry_failure_handler(failure);
> + ret = current->entry_failure_handler(&result);
> } else {
> ret = VMX_TEST_EXIT;
> }
> @@ -1705,15 +1707,15 @@ static int vmx_run(void)
> break;
> default:
> printf("ERROR : Invalid %s_handler return val %d.\n",
> - entered ? "exit" : "entry_failure",
> + result.entered ? "exit" : "entry_failure",
> ret);
> break;
> }
>
> - if (entered)
> + if (result.entered)
> print_vmexit_info();
> else
> - print_vmentry_failure_info(&failure);
> + print_vmentry_failure_info(&result);
> abort();
> }
> }
> @@ -1845,7 +1847,7 @@ static void check_for_guest_termination(void)
> * 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).
> */
> -static void __enter_guest(u8 abort_flag, struct vmentry_failure *failure)
> +static void __enter_guest(u8 abort_flag, struct vmentry_result *result)
> {
> TEST_ASSERT_MSG(v2_guest_main,
> "Never called test_set_guest_func!");
> @@ -1853,24 +1855,32 @@ static void __enter_guest(u8 abort_flag, struct vmentry_failure *failure)
> TEST_ASSERT_MSG(!guest_finished,
> "Called enter_guest() after guest returned.");
>
> - 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)) {
> + vmx_enter_guest(result);
>
> - print_vmentry_failure_info(failure);
> - abort();
> + if (result->vm_fail) {
> + if (abort_flag & ABORT_ON_EARLY_VMENTRY_FAIL)
> + goto do_abort;
> + return;
> }
> -
> - if (!failure->early && !(vmcs_read(EXI_REASON) & VMX_ENTRY_FAILURE)) {
> - launched = 1;
> - check_for_guest_termination();
> + if (result->exit_reason.failed_vmentry) {
> + if ((abort_flag & ABORT_ON_INVALID_GUEST_STATE) ||
> + result->exit_reason.basic != VMX_FAIL_STATE)
> + goto do_abort;
> + return;
> }
> +
> + launched = 1;
> + check_for_guest_termination();
> + return;
> +
> +do_abort:
> + print_vmentry_failure_info(result);
> + abort();
> }
>
> void enter_guest_with_bad_controls(void)
> {
> - struct vmentry_failure failure = {0};
> + struct vmentry_result result;
>
> TEST_ASSERT_MSG(v2_guest_main,
> "Never called test_set_guest_func!");
> @@ -1878,10 +1888,10 @@ void enter_guest_with_bad_controls(void)
> TEST_ASSERT_MSG(!guest_finished,
> "Called enter_guest() after guest returned.");
>
> - __enter_guest(ABORT_ON_INVALID_GUEST_STATE, &failure);
> - report(failure.early, "failure occurred early");
> - report((failure.flags & VMX_ENTRY_FLAGS) == X86_EFLAGS_ZF,
> - "FLAGS set correctly");
> + __enter_guest(ABORT_ON_INVALID_GUEST_STATE, &result);
> + report(result.vm_fail, "VM-Fail occurred as expected");
> + report((result.flags & VMX_ENTRY_FLAGS) == X86_EFLAGS_ZF,
> + "FLAGS set correctly on VM-Fail");
> report(vmcs_read(VMX_INST_ERROR) == VMXERR_ENTRY_INVALID_CONTROL_FIELD,
> "VM-Inst Error # is %d (VM entry with invalid control field(s))",
> VMXERR_ENTRY_INVALID_CONTROL_FIELD);
> @@ -1893,23 +1903,23 @@ void enter_guest_with_bad_controls(void)
> * unexpectedly succeed, it's nice to check whether the guest has
> * terminated, to reduce the number of error messages.
> */
> - if (!failure.early)
> + if (!result.vm_fail)
> check_for_guest_termination();
> }
>
> void enter_guest(void)
> {
> - struct vmentry_failure failure = {0};
> + struct vmentry_result result;
>
> __enter_guest(ABORT_ON_EARLY_VMENTRY_FAIL |
> - ABORT_ON_INVALID_GUEST_STATE, &failure);
> + ABORT_ON_INVALID_GUEST_STATE, &result);
> }
>
> void enter_guest_with_invalid_guest_state(void)
> {
> - struct vmentry_failure failure = {0};
> + struct vmentry_result result;
>
> - __enter_guest(ABORT_ON_EARLY_VMENTRY_FAIL, &failure);
> + __enter_guest(ABORT_ON_EARLY_VMENTRY_FAIL, &result);
> }
>
> extern struct vmx_test vmx_tests[];
> @@ -1924,6 +1934,8 @@ test_wanted(const char *name, const char *filters[], int filter_count)
> char *c;
> const char *n;
>
> + printf("filter = %s, test = %s\n", filters[0], name);
> +
> /* Replace spaces with underscores. */
> n = name;
> c = &clean_name[0];
> diff --git a/x86/vmx.h b/x86/vmx.h
> index 6adf091..c4a0fb4 100644
> --- a/x86/vmx.h
> +++ b/x86/vmx.h
> @@ -44,13 +44,38 @@ struct regs {
> u64 rflags;
> };
>
> -struct vmentry_failure {
> - /* Did a vmlaunch or vmresume fail? */
> - bool vmlaunch;
> +struct vmentry_result {
> /* Instruction mnemonic (for convenience). */
> const char *instr;
> - /* Did the instruction return right away, or did we jump to HOST_RIP? */
> - bool early;
> + /* Did the test attempt vmlaunch or vmresume? */
> + bool vmlaunch;
> + /* Did the instruction VM-Fail? */
> + bool vm_fail;
I still like the old name, "failure_early". To me, "vm_fail" and
"failed_vmentry" sound similar and confusing.
The SDM calls this type of failure as "early failure" which is denoted
by an (instruction) error number, in order to distinguish it from the
failure that happens during guest state checking/loading. So, probably a
better naming is "vm_early_failure" or "vm_fail_early". Or may be,
"vm_instr_error" ?
> + /* Did the VM-Entry fully enter the guest? */
> + bool entered;
> + /* VM-Exit reason, valid iff !vm_fail */
> + union {
> + struct {
> + u32 basic : 16;
> + u32 reserved16 : 1;
> + u32 reserved17 : 1;
> + u32 reserved18 : 1;
> + u32 reserved19 : 1;
> + u32 reserved20 : 1;
> + u32 reserved21 : 1;
> + u32 reserved22 : 1;
> + u32 reserved23 : 1;
> + u32 reserved24 : 1;
> + u32 reserved25 : 1;
> + u32 reserved26 : 1;
> + u32 enclave_mode : 1;
> + u32 smi_pending_mtf : 1;
> + u32 smi_from_vmx_root : 1;
> + u32 reserved30 : 1;
> + u32 failed_vmentry : 1;
> + };
> + u32 full;
> + } exit_reason;
> /* Contents of [re]flags after failed entry. */
> unsigned long flags;
> };
> @@ -62,7 +87,7 @@ struct vmx_test {
> int (*exit_handler)(void);
> void (*syscall_handler)(u64 syscall_no);
> struct regs guest_regs;
> - int (*entry_failure_handler)(struct vmentry_failure *failure);
> + int (*entry_failure_handler)(struct vmentry_result *result);
> struct vmcs *vmcs;
> int exits;
> /* Alternative test interface. */
> @@ -800,7 +825,7 @@ void init_vmx(u64 *vmxon_region);
>
> const char *exit_reason_description(u64 reason);
> void print_vmexit_info(void);
> -void print_vmentry_failure_info(struct vmentry_failure *failure);
> +void print_vmentry_failure_info(struct vmentry_result *result);
> void ept_sync(int type, u64 eptp);
> void vpid_sync(int type, u16 vpid);
> void install_ept_entry(unsigned long *pml4, int pte_level,
> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
> index a7abd63..c4077b1 100644
> --- a/x86/vmx_tests.c
> +++ b/x86/vmx_tests.c
> @@ -1996,24 +1996,22 @@ static int msr_switch_exit_handler(void)
> return VMX_TEST_EXIT;
> }
>
> -static int msr_switch_entry_failure(struct vmentry_failure *failure)
> +static int msr_switch_entry_failure(struct vmentry_result *result)
> {
> - ulong reason;
> -
> - if (failure->early) {
> - printf("ERROR %s: early exit\n", __func__);
> + if (result->vm_fail) {
> + printf("ERROR %s: VM-Fail on %s\n", __func__, result->instr);
> return VMX_TEST_EXIT;
> }
>
> - reason = vmcs_read(EXI_REASON);
> - if (reason == (VMX_ENTRY_FAILURE | VMX_FAIL_MSR) &&
> + if (result->exit_reason.failed_vmentry &&
> + result->exit_reason.basic == VMX_FAIL_MSR &&
> vmx_get_test_stage() == 3) {
> report(vmcs_read(EXI_QUALIFICATION) == 1,
> "VM entry MSR load: try to load FS_BASE");
> return VMX_TEST_VMEXIT;
> }
> - printf("ERROR %s: unexpected stage=%u or reason=%lu\n",
> - __func__, vmx_get_test_stage(), reason);
> + printf("ERROR %s: unexpected stage=%u or reason=%x\n",
> + __func__, vmx_get_test_stage(), result->exit_reason.full);
> return VMX_TEST_EXIT;
> }
>
> @@ -9428,12 +9426,10 @@ static int invalid_msr_exit_handler(void)
> return VMX_TEST_EXIT;
> }
>
> -static int invalid_msr_entry_failure(struct vmentry_failure *failure)
> +static int invalid_msr_entry_failure(struct vmentry_result *result)
> {
> - ulong reason;
> -
> - reason = vmcs_read(EXI_REASON);
> - report(reason == (0x80000000u | VMX_FAIL_MSR), "Invalid MSR load");
> + report(result->exit_reason.failed_vmentry &&
> + result->exit_reason.basic == VMX_FAIL_MSR, "Invalid MSR load");
> return VMX_TEST_VMEXIT;
> }
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [kvm-unit-tests PATCH 2/8] nVMX: Refactor VM-Entry "failure" struct into "result"
2020-03-18 23:40 ` Krish Sadhukhan
@ 2020-03-19 9:56 ` Paolo Bonzini
0 siblings, 0 replies; 12+ messages in thread
From: Paolo Bonzini @ 2020-03-19 9:56 UTC (permalink / raw)
To: Krish Sadhukhan, Sean Christopherson; +Cc: kvm
On 19/03/20 00:40, Krish Sadhukhan wrote:
>>
>> + /* Did the test attempt vmlaunch or vmresume? */
>> + bool vmlaunch;
>> + /* Did the instruction VM-Fail? */
>> + bool vm_fail;
>
>
> I still like the old name, "failure_early". To me, "vm_fail" and
> "failed_vmentry" sound similar and confusing.
>
> The SDM calls this type of failure as "early failure" which is denoted
> by an (instruction) error number, in order to distinguish it from the
> failure that happens during guest state checking/loading. So, probably a
> better naming is "vm_early_failure" or "vm_fail_early". Or may be,
> "vm_instr_error" ?
This is actually what the SDM calls VMfailValid (we should never get to
VMfailInvalid in these tests), so vm_fail is appropriate.
failed_vmentry is what the SDM calls "VM-entry failure".
I agree that the names are similar and confusing, but there's some value
in keeping them close to the SDM.
Paolo
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2020-03-19 9:56 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-12 23:27 [kvm-unit-tests PATCH 0/8] nVMX: Clean up __enter_guest() and co Sean Christopherson
2020-03-12 23:27 ` [kvm-unit-tests PATCH 1/8] nVMX: Eliminate superfluous entry_failure_handler() wrapper Sean Christopherson
2020-03-12 23:27 ` [kvm-unit-tests PATCH 2/8] nVMX: Refactor VM-Entry "failure" struct into "result" Sean Christopherson
2020-03-18 23:40 ` Krish Sadhukhan
2020-03-19 9:56 ` Paolo Bonzini
2020-03-12 23:27 ` [kvm-unit-tests PATCH 3/8] nVMX: Consolidate non-canonical code in test_canonical() Sean Christopherson
2020-03-12 23:27 ` [kvm-unit-tests PATCH 4/8] nVMX: Drop redundant check for guest termination Sean Christopherson
2020-03-12 23:27 ` [kvm-unit-tests PATCH 5/8] nVMX: Expose __enter_guest() and consolidate guest state test code Sean Christopherson
2020-03-12 23:27 ` [kvm-unit-tests PATCH 6/8] nVMX: Pass exit reason union to v1 exit handlers Sean Christopherson
2020-03-12 23:27 ` [kvm-unit-tests PATCH 7/8] nVMX: Pass exit reason union to is_hypercall() Sean Christopherson
2020-03-12 23:27 ` [kvm-unit-tests PATCH 8/8] nVMX: Pass exit reason enum to print_vmexit_info() Sean Christopherson
2020-03-14 10:35 ` [kvm-unit-tests PATCH 0/8] nVMX: Clean up __enter_guest() and co Paolo Bonzini
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).