diff mbox series

[kvm-unit-tests] x86: nVMX: Test VMPTRLD with operand outside of backed memory

Message ID 20181126234215.154241-1-jmattson@google.com (mailing list archive)
State New, archived
Headers show
Series [kvm-unit-tests] x86: nVMX: Test VMPTRLD with operand outside of backed memory | expand

Commit Message

Jim Mattson Nov. 26, 2018, 11:42 p.m. UTC 
Reads of unbacked addresses from the PCI bus should return all
ones. Since kvm's VMCS revision identifier isn't 0xffffffff, this
means that a VMPTRLD with an argument outside of backed memory should
set the VM-instruction error number to 11 (VMPTRLD with incorrect VMCS
revision identifier) if there is already a valid VMCS pointer loaded.

make_vmcs_current() has been modified to return all of the arithmetic
flags, so that the caller can distinguish VMfailInvalid from
VMfailValid.

Suggested-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Signed-off-by: Jim Mattson <jmattson@google.com>
Reviewed-by: Peter Shier <pshier@google.com>
---
 x86/vmx.c | 17 ++++++++++++++---
 x86/vmx.h | 10 ++++++----
 2 files changed, 20 insertions(+), 7 deletions(-)

Comments

Konrad Rzeszutek Wilk Nov. 29, 2018, 3:11 p.m. UTC | #1 
On Mon, Nov 26, 2018 at 03:42:15PM -0800, Jim Mattson wrote:
> Reads of unbacked addresses from the PCI bus should return all
> ones. Since kvm's VMCS revision identifier isn't 0xffffffff, this
> means that a VMPTRLD with an argument outside of backed memory should
> set the VM-instruction error number to 11 (VMPTRLD with incorrect VMCS
> revision identifier) if there is already a valid VMCS pointer loaded.
> 
> make_vmcs_current() has been modified to return all of the arithmetic
> flags, so that the caller can distinguish VMfailInvalid from
> VMfailValid.
> 
> Suggested-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Signed-off-by: Jim Mattson <jmattson@google.com>
> Reviewed-by: Peter Shier <pshier@google.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

Thank you!
> ---
>  x86/vmx.c | 17 ++++++++++++++---
>  x86/vmx.h | 10 ++++++----
>  2 files changed, 20 insertions(+), 7 deletions(-)
> 
> diff --git a/x86/vmx.c b/x86/vmx.c
> index 79a21d8..1318548 100644
> --- a/x86/vmx.c
> +++ b/x86/vmx.c
> @@ -31,6 +31,7 @@
>  #include "libcflat.h"
>  #include "processor.h"
>  #include "alloc_page.h"
> +#include "fwcfg.h"
>  #include "vm.h"
>  #include "desc.h"
>  #include "vmx.h"
> @@ -1374,23 +1375,33 @@ static void test_vmptrld(void)
>  	/* Unaligned page access */
>  	tmp_root = (struct vmcs *)((intptr_t)vmcs + 1);
>  	report("test vmptrld with unaligned vmcs",
> -	       make_vmcs_current(tmp_root) == 1);
> +	       make_vmcs_current(tmp_root) == VM_FAIL_INVALID);
>  
>  	/* 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",
> -	       make_vmcs_current(tmp_root) == 1);
> +	       make_vmcs_current(tmp_root) == VM_FAIL_INVALID);
>  
>  	/* Pass VMXON region */
>  	make_vmcs_current(vmcs);
>  	tmp_root = (struct vmcs *)vmxon_region;
>  	report("test vmptrld with vmxon region",
> -	       make_vmcs_current(tmp_root) == 1);
> +	       make_vmcs_current(tmp_root) == VM_FAIL_VALID);
>  	report("test vmptrld with vmxon region vm-instruction error",
>  	       vmcs_read(VMX_INST_ERROR) == VMXERR_VMPTRLD_VMXON_POINTER);
>  
> +	tmp_root = (struct vmcs *)(fwcfg_get_u64(FW_CFG_RAM_SIZE));
> +	if ((uintptr_t)tmp_root < (1ul << width)) {
> +		report("test vmptrld of unbacked physical address",
> +		       make_vmcs_current(tmp_root) == VM_FAIL_VALID);
> +		report("test vmptrld of unbacked physical address vm-instruction error",
> +		       vmcs_read(VMX_INST_ERROR) ==
> +		       VMXERR_VMPTRLD_INCORRECT_VMCS_REVISION_ID);
> +	}
> +
>  	report("test vmptrld with valid vmcs region", make_vmcs_current(vmcs) == 0);
> +
>  }
>  
>  static void test_vmptrst(void)
> diff --git a/x86/vmx.h b/x86/vmx.h
> index ba47455..6e3620b 100644
> --- a/x86/vmx.h
> +++ b/x86/vmx.h
> @@ -7,6 +7,9 @@
>  #include "asm/page.h"
>  #include "asm/io.h"
>  
> +#define VM_FAIL_INVALID	X86_EFLAGS_CF
> +#define VM_FAIL_VALID	X86_EFLAGS_ZF
> +
>  struct vmcs_hdr {
>  	u32 revision_id:31;
>  	u32 shadow_vmcs:1;
> @@ -680,12 +683,11 @@ static int vmx_off(void)
>  
>  static inline int make_vmcs_current(struct vmcs *vmcs)
>  {
> -	bool ret;
>  	u64 rflags = read_rflags() | X86_EFLAGS_CF | X86_EFLAGS_ZF;
>  
> -	asm volatile ("push %1; popf; vmptrld %2; setbe %0"
> -		      : "=q" (ret) : "q" (rflags), "m" (vmcs) : "cc");
> -	return ret;
> +	asm volatile ("push %1; popf; vmptrld %2; pushf; pop %0"
> +		      : "=r" (rflags) : "0" (rflags), "m" (vmcs) : "cc");
> +	return rflags & VMX_ENTRY_FLAGS;
>  }
>  
>  static inline int vmcs_clear(struct vmcs *vmcs)
> -- 
> 2.20.0.rc0.387.gc7a69e6b6c-goog
>
diff mbox series

Patch

diff --git a/x86/vmx.c b/x86/vmx.c
index 79a21d8..1318548 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -31,6 +31,7 @@ 
 #include "libcflat.h"
 #include "processor.h"
 #include "alloc_page.h"
+#include "fwcfg.h"
 #include "vm.h"
 #include "desc.h"
 #include "vmx.h"
@@ -1374,23 +1375,33 @@  static void test_vmptrld(void)
 	/* Unaligned page access */
 	tmp_root = (struct vmcs *)((intptr_t)vmcs + 1);
 	report("test vmptrld with unaligned vmcs",
-	       make_vmcs_current(tmp_root) == 1);
+	       make_vmcs_current(tmp_root) == VM_FAIL_INVALID);
 
 	/* 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",
-	       make_vmcs_current(tmp_root) == 1);
+	       make_vmcs_current(tmp_root) == VM_FAIL_INVALID);
 
 	/* Pass VMXON region */
 	make_vmcs_current(vmcs);
 	tmp_root = (struct vmcs *)vmxon_region;
 	report("test vmptrld with vmxon region",
-	       make_vmcs_current(tmp_root) == 1);
+	       make_vmcs_current(tmp_root) == VM_FAIL_VALID);
 	report("test vmptrld with vmxon region vm-instruction error",
 	       vmcs_read(VMX_INST_ERROR) == VMXERR_VMPTRLD_VMXON_POINTER);
 
+	tmp_root = (struct vmcs *)(fwcfg_get_u64(FW_CFG_RAM_SIZE));
+	if ((uintptr_t)tmp_root < (1ul << width)) {
+		report("test vmptrld of unbacked physical address",
+		       make_vmcs_current(tmp_root) == VM_FAIL_VALID);
+		report("test vmptrld of unbacked physical address vm-instruction error",
+		       vmcs_read(VMX_INST_ERROR) ==
+		       VMXERR_VMPTRLD_INCORRECT_VMCS_REVISION_ID);
+	}
+
 	report("test vmptrld with valid vmcs region", make_vmcs_current(vmcs) == 0);
+
 }
 
 static void test_vmptrst(void)
diff --git a/x86/vmx.h b/x86/vmx.h
index ba47455..6e3620b 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -7,6 +7,9 @@ 
 #include "asm/page.h"
 #include "asm/io.h"
 
+#define VM_FAIL_INVALID	X86_EFLAGS_CF
+#define VM_FAIL_VALID	X86_EFLAGS_ZF
+
 struct vmcs_hdr {
 	u32 revision_id:31;
 	u32 shadow_vmcs:1;
@@ -680,12 +683,11 @@  static int vmx_off(void)
 
 static inline int make_vmcs_current(struct vmcs *vmcs)
 {
-	bool ret;
 	u64 rflags = read_rflags() | X86_EFLAGS_CF | X86_EFLAGS_ZF;
 
-	asm volatile ("push %1; popf; vmptrld %2; setbe %0"
-		      : "=q" (ret) : "q" (rflags), "m" (vmcs) : "cc");
-	return ret;
+	asm volatile ("push %1; popf; vmptrld %2; pushf; pop %0"
+		      : "=r" (rflags) : "0" (rflags), "m" (vmcs) : "cc");
+	return rflags & VMX_ENTRY_FLAGS;
 }
 
 static inline int vmcs_clear(struct vmcs *vmcs)