[PATCH kvm-unit-tests v2 0/3] More nvmx unit tests changes

[PATCH kvm-unit-tests v2 0/3] More nvmx unit tests changes

[Bandan Das]

A couple more updates to test_vmxon, test_vmptrld
and test_vmclear based on kvm commits -

3573e22cfecaac83f82ef4f6847d90e466fc8e10
KVM: nVMX: additional checks on vmxon region

96ec146330d18a938b4773be8d6dd1f93399507c
KVM: nVMX: fail on invalid vmclear/vmptrld pointer

v2:
 - 1/3(New): Move the writing of flags register inside asm blocks

Bandan Das (3):
  VMX: move write_rflags inside asm blocks for vmx instructions
  VMX: Add more checks to test_vmxon
  VMX: Updated test_vmclear and test_vmptrld

 lib/x86/processor.h |   7 ---
 x86/vmx.c           | 121 +++++++++++++++++++++++++++++++++++++++++-----------
 x86/vmx.h           |  10 ++++-
 3 files changed, 103 insertions(+), 35 deletions(-)

-- 
1.8.3.1

[PATCH kvm-unit-tests v2 1/3] VMX: move write_rflags inside asm blocks for vmx instructions

[Bandan Das]

This makes sure that the flags register doesn't get accidentally
overwritten due to compiler optimizations or other means

Signed-off-by: Bandan Das <bsd@redhat.com>
---
 lib/x86/processor.h |  7 -------
 x86/vmx.c           | 31 ++++++++++---------------------
 x86/vmx.h           | 10 ++++++++--
 3 files changed, 18 insertions(+), 30 deletions(-)

diff --git a/lib/x86/processor.h b/lib/x86/processor.h
index 7fc1026..f5f1c82 100644
--- a/lib/x86/processor.h
+++ b/lib/x86/processor.h
@@ -351,11 +351,4 @@ static inline void safe_halt(void)
 	asm volatile("sti; hlt");
 }
 
-#ifdef __x86_64__
-static inline void write_rflags(u64 r)
-{
-	asm volatile("push %0; popf\n\t" : : "q"(r) : "cc");
-}
-#endif
-
 #endif
diff --git a/x86/vmx.c b/x86/vmx.c
index 1182eef..768d07f 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -65,8 +65,10 @@ extern void *guest_entry;
 static int make_vmcs_current(struct vmcs *vmcs)
 {
 	bool ret;
+	u64 rflags = read_rflags() | X86_EFLAGS_CF | X86_EFLAGS_ZF;
 
-	asm volatile ("vmptrld %1; setbe %0" : "=q" (ret) : "m" (vmcs) : "cc");
+	asm volatile ("push %1; popf; vmptrld %2; setbe %0"
+		      : "=q" (ret) : "q" (rflags), "m" (vmcs) : "cc");
 	return ret;
 }
 
@@ -92,16 +94,19 @@ static void __attribute__((__used__)) syscall_handler(u64 syscall_no)
 static inline int vmx_on()
 {
 	bool ret;
-	asm volatile ("vmxon %1; setbe %0\n\t"
-		: "=q"(ret) : "m"(vmxon_region) : "cc");
+	u64 rflags = read_rflags() | X86_EFLAGS_CF | X86_EFLAGS_ZF;
+	asm volatile ("push %1; popf; vmxon %2; setbe %0\n\t"
+		      : "=q" (ret) : "q" (rflags), "m" (vmxon_region) : "cc");
 	return ret;
 }
 
 static inline int vmx_off()
 {
 	bool ret;
-	asm volatile("vmxoff; setbe %0\n\t"
-		: "=q"(ret) : : "cc");
+	u64 rflags = read_rflags() | X86_EFLAGS_CF | X86_EFLAGS_ZF;
+
+	asm volatile("push %1; popf; vmxoff; setbe %0\n\t"
+		     : "=q"(ret) : "q" (rflags) : "cc");
 	return ret;
 }
 
@@ -129,20 +134,13 @@ void print_vmexit_info()
 
 static void test_vmclear(void)
 {
-	u64 rflags;
-
-	rflags = read_rflags() | X86_EFLAGS_CF | X86_EFLAGS_ZF;
-	write_rflags(rflags);
 	report("test vmclear", vmcs_clear(vmcs_root) == 0);
 }
 
 static void test_vmxoff(void)
 {
 	int ret;
-	u64 rflags;
 
-	rflags = read_rflags() | X86_EFLAGS_CF | X86_EFLAGS_ZF;
-	write_rflags(rflags);
 	ret = vmx_off();
 	report("test vmxoff", !ret);
 }
@@ -599,10 +597,7 @@ static int test_vmx_feature_control(void)
 static int test_vmxon(void)
 {
 	int ret;
-	u64 rflags;
 
-	rflags = read_rflags() | X86_EFLAGS_CF | X86_EFLAGS_ZF;
-	write_rflags(rflags);
 	ret = vmx_on();
 	report("test vmxon", !ret);
 	return ret;
@@ -610,27 +605,21 @@ static int test_vmxon(void)
 
 static void test_vmptrld(void)
 {
-	u64 rflags;
 	struct vmcs *vmcs;
 
 	vmcs = alloc_page();
 	vmcs->revision_id = basic.revision;
-	rflags = read_rflags() | X86_EFLAGS_CF | X86_EFLAGS_ZF;
-	write_rflags(rflags);
 	report("test vmptrld", make_vmcs_current(vmcs) == 0);
 }
 
 static void test_vmptrst(void)
 {
-	u64 rflags;
 	int ret;
 	struct vmcs *vmcs1, *vmcs2;
 
 	vmcs1 = alloc_page();
 	memset(vmcs1, 0, PAGE_SIZE);
 	init_vmcs(&vmcs1);
-	rflags = read_rflags() | X86_EFLAGS_CF | X86_EFLAGS_ZF;
-	write_rflags(rflags);
 	ret = vmcs_save(&vmcs2);
 	report("test vmptrst", (!ret) && (vmcs1 == vmcs2));
 }
diff --git a/x86/vmx.h b/x86/vmx.h
index 26dd161..86bdf67 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -2,6 +2,7 @@
 #define __VMX_H
 
 #include "libcflat.h"
+#include "processor.h"
 
 struct vmcs {
 	u32 revision_id; /* vmcs revision identifier */
@@ -515,7 +516,10 @@ extern union vmx_ept_vpid  ept_vpid;
 static inline int vmcs_clear(struct vmcs *vmcs)
 {
 	bool ret;
-	asm volatile ("vmclear %1; setbe %0" : "=q" (ret) : "m" (vmcs) : "cc");
+	u64 rflags = read_rflags() | X86_EFLAGS_CF | X86_EFLAGS_ZF;
+
+	asm volatile ("push %1; popf; vmclear %2; setbe %0"
+		      : "=q" (ret) : "q" (rflags), "m" (vmcs) : "cc");
 	return ret;
 }
 
@@ -537,8 +541,10 @@ static inline int vmcs_write(enum Encoding enc, u64 val)
 static inline int vmcs_save(struct vmcs **vmcs)
 {
 	bool ret;
+	u64 rflags = read_rflags() | X86_EFLAGS_CF | X86_EFLAGS_ZF;
 
-	asm volatile ("vmptrst %1; setbe %0" : "=q" (ret) : "m" (*vmcs) : "cc");
+	asm volatile ("push %1; popf; vmptrst %2; setbe %0"
+		      : "=q" (ret) : "q" (rflags), "m" (*vmcs) : "cc");
 	return ret;
 }
 
-- 
1.8.3.1

[PATCH kvm-unit-tests v2 2/3] VMX: Add more checks to test_vmxon

[Bandan Das]

Verify that vmon fails with unaligned vmxon region or
any bits set beyong the physical address width. Also verify
failure with an invalid revision identifier.

Signed-off-by: Bandan Das <bsd@redhat.com>
---
 x86/vmx.c | 40 +++++++++++++++++++++++++++++++++++++---
 1 file changed, 37 insertions(+), 3 deletions(-)

diff --git a/x86/vmx.c b/x86/vmx.c
index 768d07f..13e2be8 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -37,7 +37,7 @@
 #include "smp.h"
 #include "io.h"
 
-u32 *vmxon_region;
+u64 *vmxon_region;
 struct vmcs *vmcs_root;
 u32 vpid_cnt;
 void *guest_stack, *guest_syscall_stack;
@@ -596,10 +596,44 @@ static int test_vmx_feature_control(void)
 
 static int test_vmxon(void)
 {
-	int ret;
+	int ret, ret1;
+	u64 *tmp_region = vmxon_region;
+	int width = cpuid(0x80000008).a & 0xff;
+
+	/* Unaligned page access */
+	vmxon_region = (u64 *)((intptr_t)vmxon_region + 1);
+	ret1 = vmx_on();
+	report("test vmxon with unaligned vmxon region", ret1);
+	if (!ret1) {
+		ret = 1;
+		goto out;
+	}
+
+	/* gpa bits beyond physical address width are set*/
+	vmxon_region = (u64 *)((intptr_t)tmp_region | ((u64)1 << (width+1)));
+	ret1 = vmx_on();
+	report("test vmxon with bits set beyond physical address width", ret1);
+	if (!ret1) {
+		ret = 1;
+		goto out;
+	}
 
+	/* invalid revision indentifier */
+	vmxon_region = tmp_region;
+	*vmxon_region = 0xba9da9;
+	ret1 = vmx_on();
+	report("test vmxon with invalid revision identifier", ret1);
+	if (!ret1) {
+		ret = 1;
+		goto out;
+	}
+
+	/* and finally a valid region */
+	*vmxon_region = basic.revision;
 	ret = vmx_on();
-	report("test vmxon", !ret);
+	report("test vmxon with valid vmxon region", !ret);
+
+out:
 	return ret;
 }
 
-- 
1.8.3.1

[PATCH kvm-unit-tests v2 3/3] VMX: Updated test_vmclear and test_vmptrld

[Bandan Das]

Check if the vmcs pointer is not aligned to page size,
and if bits beyond physical address width are set. Also,
vmclear and vmptrld should fail if the vmxon region is
supplied instead of the vmcs

Signed-off-by: Bandan Das <bsd@redhat.com>
---
 x86/vmx.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 49 insertions(+), 3 deletions(-)

diff --git a/x86/vmx.c b/x86/vmx.c
index 13e2be8..b29fc05 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -134,7 +134,35 @@ void print_vmexit_info()
 
 static void test_vmclear(void)
 {
-	report("test vmclear", vmcs_clear(vmcs_root) == 0);
+	struct vmcs *tmp_root;
+	int width = cpuid(0x80000008).a & 0xff;
+
+	/*
+	 * Note- The tests below do not necessarily have a
+	 * valid VMCS, but that's ok since the invalid vmcs
+	 * is only used for a specific test and is discarded
+	 * without touching its contents
+	 */
+
+	/* Unaligned page access */
+	tmp_root = (struct vmcs *)((intptr_t)vmcs_root + 1);
+	report("test vmclear with unaligned vmcs",
+	       vmcs_clear(tmp_root) == 1);
+
+	/* gpa bits beyond physical address width are set*/
+	tmp_root = (struct vmcs *)((intptr_t)vmcs_root |
+				   ((u64)1 << (width+1)));
+	report("test vmclear with vmcs address bits set beyond physical address width",
+	       vmcs_clear(tmp_root) == 1);
+
+	/* Pass VMXON region */
+	tmp_root = (struct vmcs *)vmxon_region;
+	report("test vmclear with vmxon region",
+	       vmcs_clear(tmp_root) == 1);
+
+	/* Valid VMCS */
+	report("test vmclear with valid vmcs region", vmcs_clear(vmcs_root) == 0);
+
 }
 
 static void test_vmxoff(void)
@@ -639,11 +667,29 @@ out:
 
 static void test_vmptrld(void)
 {
-	struct vmcs *vmcs;
+	struct vmcs *vmcs, *tmp_root;
+	int width = cpuid(0x80000008).a & 0xff;
 
 	vmcs = alloc_page();
 	vmcs->revision_id = basic.revision;
-	report("test vmptrld", make_vmcs_current(vmcs) == 0);
+
+	/* Unaligned page access */
+	tmp_root = (struct vmcs *)((intptr_t)vmcs + 1);
+	report("test vmptrld with unaligned vmcs",
+	       vmcs_clear(tmp_root) == 1);
+
+	/* gpa bits beyond physical address width are set*/
+	tmp_root = (struct vmcs *)((intptr_t)vmcs |
+				   ((u64)1 << (width+1)));
+	report("test vmptrld with vmcs address bits set beyond physical address width",
+	       vmcs_clear(tmp_root) == 1);
+
+	/* Pass VMXON region */
+	tmp_root = (struct vmcs *)vmxon_region;
+	report("test vmptrld with vmxon region",
+	       vmcs_clear(tmp_root) == 1);
+
+	report("test vmptrld with valid vmcs region", make_vmcs_current(vmcs) == 0);
 }
 
 static void test_vmptrst(void)
-- 
1.8.3.1

Re: [PATCH kvm-unit-tests v2 0/3] More nvmx unit tests changes

[Paolo Bonzini]

Il 09/06/2014 23:04, Bandan Das ha scritto:
> A couple more updates to test_vmxon, test_vmptrld
> and test_vmclear based on kvm commits -
>
> 3573e22cfecaac83f82ef4f6847d90e466fc8e10
> KVM: nVMX: additional checks on vmxon region
>
> 96ec146330d18a938b4773be8d6dd1f93399507c
> KVM: nVMX: fail on invalid vmclear/vmptrld pointer
>
> v2:
>  - 1/3(New): Move the writing of flags register inside asm blocks
>
> Bandan Das (3):
>   VMX: move write_rflags inside asm blocks for vmx instructions
>   VMX: Add more checks to test_vmxon
>   VMX: Updated test_vmclear and test_vmptrld
>
>  lib/x86/processor.h |   7 ---
>  x86/vmx.c           | 121 +++++++++++++++++++++++++++++++++++++++++-----------
>  x86/vmx.h           |  10 ++++-
>  3 files changed, 103 insertions(+), 35 deletions(-)
>

Sooner or later, we probably want to return the flags so that you can 
test for CF=1 or ZF=1.  For now, I've applied the patch.  Thanks!

Paolo

Re: [PATCH kvm-unit-tests v2 0/3] More nvmx unit tests changes

[Paolo Bonzini]

Il 09/06/2014 23:04, Bandan Das ha scritto:
> A couple more updates to test_vmxon, test_vmptrld
> and test_vmclear based on kvm commits -
>
> 3573e22cfecaac83f82ef4f6847d90e466fc8e10
> KVM: nVMX: additional checks on vmxon region
>
> 96ec146330d18a938b4773be8d6dd1f93399507c
> KVM: nVMX: fail on invalid vmclear/vmptrld pointer
>
> v2:
>  - 1/3(New): Move the writing of flags register inside asm blocks
>
> Bandan Das (3):
>   VMX: move write_rflags inside asm blocks for vmx instructions
>   VMX: Add more checks to test_vmxon
>   VMX: Updated test_vmclear and test_vmptrld
>
>  lib/x86/processor.h |   7 ---
>  x86/vmx.c           | 121 +++++++++++++++++++++++++++++++++++++++++-----------
>  x86/vmx.h           |  10 ++++-
>  3 files changed, 103 insertions(+), 35 deletions(-)
>

Applied, thanks.

Paolo