Skip to contentSkip to conversation

x86: nvmx: fix bug in __enter_guest()

PATCH Superseded Marc Orr 2 participants 4 messages Sep 20, 2019 – Sep 25, 2019

Subsystems (2)
KVM Library code

v6 of 7 Superseded Latest is v7 →

lore
Share & tools
lore

Marc Orr

author
#0 lore raw

PATCH v6 1/2 x86: nvmx: fix bug in __enter_guest()

__enter_guest() should only set the launched flag when a launch has succeeded. Thus, don't set the launched flag when the VMX_ENTRY_FAILURE, bit 31, is set in the VMCS exit reason.

Reviewed-by Krish Sadhukhan <[email protected]>
Reviewed-by Sean Christopherson <[email protected]>
Signed-off-by Marc Orr <[email protected]>

---
v5 -> v6
* No changes.

 x86/vmx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
x86/vmx.c +1 -1
index 6079420db33a..7313c78f15c2 100644
@@ -1820,7 +1820,7 @@ static void __enter_guest(u8 abort_flag, struct vmentry_failure *failure)
 		abort();
 	}
 
-	if (!failure->early) {
+	if (!failure->early && !(vmcs_read(EXI_REASON) & VMX_ENTRY_FAILURE)) {
 		launched = 1;
 		check_for_guest_termination();
 	}
Signature · 1 line
2.23.0.351.gc4317032e6-goog
author
#1 lore raw

PATCH v6 2/2 x86: nvmx: test max atomic switch MSRs

Excerise nested VMX's atomic MSR switch code (e.g., VM-entry MSR-load list) at the maximum number of MSRs supported, as described in the SDM, in the appendix chapter titled "MISCELLANEOUS DATA".

Suggested-by Jim Mattson <[email protected]>
Reviewed-by Krish Sadhukhan <[email protected]>
Signed-off-by Marc Orr <[email protected]>

---
v5 -> v6
* Replaced atomic_switch_msr_limit_test_guest() (which was supposed to vmcall() in v5!) w/ v2_null_test_guest() and updated exit handling logic accordingly.

 lib/alloc_page.c  |   5 ++
 lib/alloc_page.h  |   1 +
 x86/unittests.cfg |   2 +-
 x86/vmx_tests.c   | 123 ++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 130 insertions(+), 1 deletion(-)
lib/alloc_page.c +5 -0
index 97d13395ff08..ed236389537e 100644
@@ -53,6 +53,11 @@ void free_pages(void *mem, unsigned long size)
 	spin_unlock(&lock);
 }
 
+void free_pages_by_order(void *mem, unsigned long order)
+{
+	free_pages(mem, 1ul << (order + PAGE_SHIFT));
+}
+
 void *alloc_page()
 {
 	void *p;
lib/alloc_page.h +1 -0
index 5cdfec57a0a8..739a91def979 100644
@@ -14,5 +14,6 @@ void *alloc_page(void);
 void *alloc_pages(unsigned long order);
 void free_page(void *page);
 void free_pages(void *mem, unsigned long size);
+void free_pages_by_order(void *mem, unsigned long order);
 
 #endif
x86/unittests.cfg +1 -1
index 694ee3d42f3a..05122cf91ea1 100644
@@ -227,7 +227,7 @@ extra_params = -cpu qemu64,+umip
 
 [vmx]
 file = vmx.flat
-extra_params = -cpu host,+vmx -append "-exit_monitor_from_l2_test -ept_access* -vmx_smp* -vmx_vmcs_shadow_test"
+extra_params = -cpu host,+vmx -append "-exit_monitor_from_l2_test -ept_access* -vmx_smp* -vmx_vmcs_shadow_test -atomic_switch_overflow_msrs_test"
 arch = x86_64
 groups = vmx
x86/vmx_tests.c +123 -0

134 lines not on this page · raw message

Signature · 1 line
2.23.0.351.gc4317032e6-goog
#2 lore raw

Re: [kvm-unit-tests PATCH v6 2/2] x86: nvmx: test max atomic switch MSRs

Quoted 5 lines · Marc Orr
x86/vmx_tests.c +5 -0
+		u32 exit_reason;
+		u32 exit_reason_want;
+		u32 exit_qual;
+
+		enter_guest_with_invalid_guest_state();

x86/vmx_tests.c static int invalid_msr_entry_failure(struct vmentry_failure *failure) · line 8670

Quoted 10 lines
x86/vmx_tests.c +10 -0
+
+		exit_reason = vmcs_read(EXI_REASON);
+		exit_reason_want = VMX_FAIL_MSR | VMX_ENTRY_FAILURE;
+		report("exit_reason, %u, is %u.",
+		       exit_reason == exit_reason_want, exit_reason,
+		       exit_reason_want);
+
+		exit_qual = vmcs_read(EXI_QUALIFICATION);
+		report("exit_qual, %u, is %u.", exit_qual == max_allowed + 1,
+		       exit_qual, max_allowed + 1);

I'd stick with the more standard "val %u, expected %u" verbiage. E.g.:

	exit_reason, 34, is 35

versus

exit_reason 34, expected 35

The "is" part makes it sound like the value in the VMCS *is* 35.

author
#3 lore raw

Re: [kvm-unit-tests PATCH v6 2/2] x86: nvmx: test max atomic switch MSRs

Quoted 20 lines · Sean Christopherson
Quoted 5 lines · Marc Orr
x86/vmx_tests.c +5 -0
+             u32 exit_reason;
+             u32 exit_reason_want;
+             u32 exit_qual;
+
+             enter_guest_with_invalid_guest_state();

x86/vmx_tests.c static int invalid_msr_entry_failure(struct vmentry_failure *failure) · line 8670

Quoted 10 lines
x86/vmx_tests.c +10 -0
+
+             exit_reason = vmcs_read(EXI_REASON);
+             exit_reason_want = VMX_FAIL_MSR | VMX_ENTRY_FAILURE;
+             report("exit_reason, %u, is %u.",
+                    exit_reason == exit_reason_want, exit_reason,
+                    exit_reason_want);
+
+             exit_qual = vmcs_read(EXI_QUALIFICATION);
+             report("exit_qual, %u, is %u.", exit_qual == max_allowed + 1,
+                    exit_qual, max_allowed + 1);

I'd stick with the more standard "val %u, expected %u" verbiage. E.g.:

Quoted 7 lines
        exit_reason, 34, is 35

versus

exit_reason 34, expected 35

The "is" part makes it sound like the value in the VMCS *is* 35.

Done.