Re-order the testcases in test_host_addr_size() to guarantee that the host
CR4 and RIP values are either written or restored before each testcase.
If a test fails unexpectedly, running with a test value from a previous
testcase may cause all subsequent tests to also fail, e.g. if the CR4
PCIDE test fails, all of the RIP tests will fail because of the bad CR4.
This also "fixes" the noncanonical RIP testcase, as running with the bad
CR4 setup by the !PAE testcase would mask a missed noncanonical check.
[sean: Surprise! The bad CR4 is indeed masking a bug. I'm leaving it for
now and intentionally creating a failing testcase for a commit or two to
highlight the importance of cleaning up after testcases, and isolating
what is actually being tested.]
Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
---
x86/vmx_tests.c | 24 +++++++++++++++++-------
1 file changed, 17 insertions(+), 7 deletions(-)
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index 376d0a53..1a340242 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -7634,6 +7634,12 @@ static void test_host_addr_size(void)
report_prefix_pop();
}
+ vmcs_write(HOST_CR4, cr4_saved & ~X86_CR4_PAE);
+ report_prefix_pushf("\"CR4.PAE\" unset");
+ test_vmx_vmlaunch(VMXERR_ENTRY_INVALID_HOST_STATE_FIELD);
+ vmcs_write(HOST_CR4, cr4_saved);
+ report_prefix_pop();
+
for (i = 32; i <= 63; i = i + 4) {
tmp = rip_saved | 1ull << i;
vmcs_write(HOST_RIP, tmp);
@@ -7642,12 +7648,6 @@ static void test_host_addr_size(void)
report_prefix_pop();
}
- vmcs_write(HOST_CR4, cr4_saved & ~X86_CR4_PAE);
- report_prefix_pushf("\"CR4.PAE\" unset");
- test_vmx_vmlaunch(VMXERR_ENTRY_INVALID_HOST_STATE_FIELD);
- vmcs_write(HOST_CR4, cr4_saved);
- report_prefix_pop();
-
vmcs_write(HOST_RIP, NONCANONICAL);
report_prefix_pushf("HOST_RIP %llx", NONCANONICAL);
test_vmx_vmlaunch(VMXERR_ENTRY_INVALID_HOST_STATE_FIELD);
@@ -7657,7 +7657,17 @@ static void test_host_addr_size(void)
vmcs_write(HOST_RIP, rip_saved);
vmcs_write(HOST_CR4, cr4_saved);
- /* Restore host's active RIP and CR4 values. */
+ /*
+ * Restore host's active CR4 and RIP values by triggering a VM-Exit.
+ * The original CR4 and RIP values in the VMCS are restored between
+ * testcases as needed, but don't guarantee a VM-Exit and so the active
+ * CR4 and RIP may still hold a test value. Running with the test CR4
+ * and RIP values at some point is unavoidable, and the active values
+ * are unlikely to affect VM-Enter, so the above doen't force a VM-Exit
+ * between testcases. Note, if VM-Enter is surrounded by CALL+RET then
+ * the active RIP will already be restored, but that's also not
+ * guaranteed, and CR4 needs to be restored regardless.
+ */
report_prefix_pushf("restore host state");
test_vmx_vmlaunch(0);
report_prefix_pop();
--
2.42.0.283.g2d96d420d3-goog
