[PATCH] kvm: nVMX: Check memory operand to INVVPID

[PATCH] kvm: nVMX: Check memory operand to INVVPID

[Jim Mattson]

The memory operand fetched for INVVPID is 128 bits. Bits 63:16 are
reserved and must be zero.  Otherwise, the instruction fails with
VMfail(Invalid operand to INVEPT/INVVPID).  If the INVVPID_TYPE is 0
(individual address invalidation), then bits 127:64 must be in
canonical form, or the instruction fails with VMfail(Invalid operand
to INVEPT/INVVPID).

Signed-off-by: Jim Mattson <jmattson@google.com>
---
 arch/x86/kvm/vmx.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 42db3eb2d13b..9c34a98cc051 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -7651,7 +7651,11 @@ static int handle_invvpid(struct kvm_vcpu *vcpu)
 	unsigned long type, types;
 	gva_t gva;
 	struct x86_exception e;
-	int vpid;
+	struct {
+		u64 vpid : 16;
+		u64 rsvd : 48;
+		u64 gla;
+	} operand;
 
 	if (!(vmx->nested.nested_vmx_secondary_ctls_high &
 	      SECONDARY_EXEC_ENABLE_VPID) ||
@@ -7681,17 +7685,28 @@ static int handle_invvpid(struct kvm_vcpu *vcpu)
 	if (get_vmx_mem_address(vcpu, vmcs_readl(EXIT_QUALIFICATION),
 			vmx_instruction_info, false, &gva))
 		return 1;
-	if (kvm_read_guest_virt(&vcpu->arch.emulate_ctxt, gva, &vpid,
-				sizeof(u32), &e)) {
+	if (kvm_read_guest_virt(&vcpu->arch.emulate_ctxt, gva, &operand,
+				sizeof(operand), &e)) {
 		kvm_inject_page_fault(vcpu, &e);
 		return 1;
 	}
+	if (operand.rsvd) {
+		nested_vmx_failValid(vcpu,
+			VMXERR_INVALID_OPERAND_TO_INVEPT_INVVPID);
+		return kvm_skip_emulated_instruction(vcpu);
+	}
 
 	switch (type) {
 	case VMX_VPID_EXTENT_INDIVIDUAL_ADDR:
+		if (is_noncanonical_address(operand.gla)) {
+			nested_vmx_failValid(vcpu,
+				VMXERR_INVALID_OPERAND_TO_INVEPT_INVVPID);
+			return kvm_skip_emulated_instruction(vcpu);
+		}
+		/* fall through */
 	case VMX_VPID_EXTENT_SINGLE_CONTEXT:
 	case VMX_VPID_EXTENT_SINGLE_NON_GLOBAL:
-		if (!vpid) {
+		if (!operand.vpid) {
 			nested_vmx_failValid(vcpu,
 				VMXERR_INVALID_OPERAND_TO_INVEPT_INVVPID);
 			return kvm_skip_emulated_instruction(vcpu);
-- 
2.13.1.611.g7e3b11ae1-goog

Re: [PATCH] kvm: nVMX: Check memory operand to INVVPID

[David Hildenbrand]

On 27.06.2017 19:59, Jim Mattson wrote:
> The memory operand fetched for INVVPID is 128 bits. Bits 63:16 are
> reserved and must be zero.  Otherwise, the instruction fails with
> VMfail(Invalid operand to INVEPT/INVVPID).  If the INVVPID_TYPE is 0
> (individual address invalidation), then bits 127:64 must be in
> canonical form, or the instruction fails with VMfail(Invalid operand
> to INVEPT/INVVPID).
> 
> Signed-off-by: Jim Mattson <jmattson@google.com>
> ---
>  arch/x86/kvm/vmx.c | 23 +++++++++++++++++++----
>  1 file changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 42db3eb2d13b..9c34a98cc051 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -7651,7 +7651,11 @@ static int handle_invvpid(struct kvm_vcpu *vcpu)
>  	unsigned long type, types;
>  	gva_t gva;
>  	struct x86_exception e;
> -	int vpid;
> +	struct {
> +		u64 vpid : 16;
> +		u64 rsvd : 48;
> +		u64 gla;
> +	} operand;
>  
>  	if (!(vmx->nested.nested_vmx_secondary_ctls_high &
>  	      SECONDARY_EXEC_ENABLE_VPID) ||
> @@ -7681,17 +7685,28 @@ static int handle_invvpid(struct kvm_vcpu *vcpu)
>  	if (get_vmx_mem_address(vcpu, vmcs_readl(EXIT_QUALIFICATION),
>  			vmx_instruction_info, false, &gva))
>  		return 1;
> -	if (kvm_read_guest_virt(&vcpu->arch.emulate_ctxt, gva, &vpid,
> -				sizeof(u32), &e)) {
> +	if (kvm_read_guest_virt(&vcpu->arch.emulate_ctxt, gva, &operand,
> +				sizeof(operand), &e)) {
>  		kvm_inject_page_fault(vcpu, &e);
>  		return 1;
>  	}
> +	if (operand.rsvd) {
> +		nested_vmx_failValid(vcpu,
> +			VMXERR_INVALID_OPERAND_TO_INVEPT_INVVPID);
> +		return kvm_skip_emulated_instruction(vcpu);
> +	}
>  
>  	switch (type) {
>  	case VMX_VPID_EXTENT_INDIVIDUAL_ADDR:
> +		if (is_noncanonical_address(operand.gla)) {
> +			nested_vmx_failValid(vcpu,
> +				VMXERR_INVALID_OPERAND_TO_INVEPT_INVVPID);
> +			return kvm_skip_emulated_instruction(vcpu);
> +		}
> +		/* fall through */
>  	case VMX_VPID_EXTENT_SINGLE_CONTEXT:
>  	case VMX_VPID_EXTENT_SINGLE_NON_GLOBAL:
> -		if (!vpid) {
> +		if (!operand.vpid) {
>  			nested_vmx_failValid(vcpu,
>  				VMXERR_INVALID_OPERAND_TO_INVEPT_INVVPID);
>  			return kvm_skip_emulated_instruction(vcpu);
> 

Looks good to me!

Reviewed-by: David Hildenbrand <david@redhat.com>


-- 

Thanks,

David

Re: [PATCH] kvm: nVMX: Check memory operand to INVVPID

[Paolo Bonzini]

On 27/06/2017 19:59, Jim Mattson wrote:
> The memory operand fetched for INVVPID is 128 bits. Bits 63:16 are
> reserved and must be zero.  Otherwise, the instruction fails with
> VMfail(Invalid operand to INVEPT/INVVPID).  If the INVVPID_TYPE is 0
> (individual address invalidation), then bits 127:64 must be in
> canonical form, or the instruction fails with VMfail(Invalid operand
> to INVEPT/INVVPID).
> 
> Signed-off-by: Jim Mattson <jmattson@google.com>
> ---
>  arch/x86/kvm/vmx.c | 23 +++++++++++++++++++----
>  1 file changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 42db3eb2d13b..9c34a98cc051 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -7651,7 +7651,11 @@ static int handle_invvpid(struct kvm_vcpu *vcpu)
>  	unsigned long type, types;
>  	gva_t gva;
>  	struct x86_exception e;
> -	int vpid;
> +	struct {
> +		u64 vpid : 16;
> +		u64 rsvd : 48;

I prefer using an u64 here (and checking the reserved bits via "&
~0xFFFF").  I find bit-endianness confusing because bits are in the
opposite order of the declarations (unlike byte-endianness, bits within
one word are usually drawn high to low).

Paolo

> +		u64 gla;
> +	} operand;
>  
>  	if (!(vmx->nested.nested_vmx_secondary_ctls_high &
>  	      SECONDARY_EXEC_ENABLE_VPID) ||
> @@ -7681,17 +7685,28 @@ static int handle_invvpid(struct kvm_vcpu *vcpu)
>  	if (get_vmx_mem_address(vcpu, vmcs_readl(EXIT_QUALIFICATION),
>  			vmx_instruction_info, false, &gva))
>  		return 1;
> -	if (kvm_read_guest_virt(&vcpu->arch.emulate_ctxt, gva, &vpid,
> -				sizeof(u32), &e)) {
> +	if (kvm_read_guest_virt(&vcpu->arch.emulate_ctxt, gva, &operand,
> +				sizeof(operand), &e)) {
>  		kvm_inject_page_fault(vcpu, &e);
>  		return 1;
>  	}
> +	if (operand.rsvd) {
> +		nested_vmx_failValid(vcpu,
> +			VMXERR_INVALID_OPERAND_TO_INVEPT_INVVPID);
> +		return kvm_skip_emulated_instruction(vcpu);
> +	}
>  
>  	switch (type) {
>  	case VMX_VPID_EXTENT_INDIVIDUAL_ADDR:
> +		if (is_noncanonical_address(operand.gla)) {
> +			nested_vmx_failValid(vcpu,
> +				VMXERR_INVALID_OPERAND_TO_INVEPT_INVVPID);
> +			return kvm_skip_emulated_instruction(vcpu);
> +		}
> +		/* fall through */
>  	case VMX_VPID_EXTENT_SINGLE_CONTEXT:
>  	case VMX_VPID_EXTENT_SINGLE_NON_GLOBAL:
> -		if (!vpid) {
> +		if (!operand.vpid) {
>  			nested_vmx_failValid(vcpu,
>  				VMXERR_INVALID_OPERAND_TO_INVEPT_INVVPID);
>  			return kvm_skip_emulated_instruction(vcpu);
> 

[PATCH v2] kvm: nVMX: Check memory operand to INVVPID

[Jim Mattson]

The memory operand fetched for INVVPID is 128 bits. Bits 63:16 are
reserved and must be zero.  Otherwise, the instruction fails with
VMfail(Invalid operand to INVEPT/INVVPID).  If the INVVPID_TYPE is 0
(individual address invalidation), then bits 127:64 must be in
canonical form, or the instruction fails with VMfail(Invalid operand
to INVEPT/INVVPID).

Signed-off-by: Jim Mattson <jmattson@google.com>
---
 arch/x86/kvm/vmx.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 42db3eb2d13b..3c80a2fec5c6 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -7651,7 +7651,10 @@ static int handle_invvpid(struct kvm_vcpu *vcpu)
 	unsigned long type, types;
 	gva_t gva;
 	struct x86_exception e;
-	int vpid;
+	struct {
+		u64 vpid;
+		u64 gla;
+	} operand;
 
 	if (!(vmx->nested.nested_vmx_secondary_ctls_high &
 	      SECONDARY_EXEC_ENABLE_VPID) ||
@@ -7681,17 +7684,28 @@ static int handle_invvpid(struct kvm_vcpu *vcpu)
 	if (get_vmx_mem_address(vcpu, vmcs_readl(EXIT_QUALIFICATION),
 			vmx_instruction_info, false, &gva))
 		return 1;
-	if (kvm_read_guest_virt(&vcpu->arch.emulate_ctxt, gva, &vpid,
-				sizeof(u32), &e)) {
+	if (kvm_read_guest_virt(&vcpu->arch.emulate_ctxt, gva, &operand,
+				sizeof(operand), &e)) {
 		kvm_inject_page_fault(vcpu, &e);
 		return 1;
 	}
+	if (operand.vpid >> 16) {
+		nested_vmx_failValid(vcpu,
+			VMXERR_INVALID_OPERAND_TO_INVEPT_INVVPID);
+		return kvm_skip_emulated_instruction(vcpu);
+	}
 
 	switch (type) {
 	case VMX_VPID_EXTENT_INDIVIDUAL_ADDR:
+		if (is_noncanonical_address(operand.gla)) {
+			nested_vmx_failValid(vcpu,
+				VMXERR_INVALID_OPERAND_TO_INVEPT_INVVPID);
+			return kvm_skip_emulated_instruction(vcpu);
+		}
+		/* fall through */
 	case VMX_VPID_EXTENT_SINGLE_CONTEXT:
 	case VMX_VPID_EXTENT_SINGLE_NON_GLOBAL:
-		if (!vpid) {
+		if (!operand.vpid) {
 			nested_vmx_failValid(vcpu,
 				VMXERR_INVALID_OPERAND_TO_INVEPT_INVVPID);
 			return kvm_skip_emulated_instruction(vcpu);
-- 
2.13.2.725.g09c95d1e9-goog

Re: [PATCH v2] kvm: nVMX: Check memory operand to INVVPID

[Paolo Bonzini]

On 28/06/2017 18:37, Jim Mattson wrote:
> The memory operand fetched for INVVPID is 128 bits. Bits 63:16 are
> reserved and must be zero.  Otherwise, the instruction fails with
> VMfail(Invalid operand to INVEPT/INVVPID).  If the INVVPID_TYPE is 0
> (individual address invalidation), then bits 127:64 must be in
> canonical form, or the instruction fails with VMfail(Invalid operand
> to INVEPT/INVVPID).
> 
> Signed-off-by: Jim Mattson <jmattson@google.com>
> ---
>  arch/x86/kvm/vmx.c | 22 ++++++++++++++++++----
>  1 file changed, 18 insertions(+), 4 deletions(-)

Thanks, looks good.  Will apply, kvm-unit-tests patches are always
welcome. :)

Paolo

> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 42db3eb2d13b..3c80a2fec5c6 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -7651,7 +7651,10 @@ static int handle_invvpid(struct kvm_vcpu *vcpu)
>  	unsigned long type, types;
>  	gva_t gva;
>  	struct x86_exception e;
> -	int vpid;
> +	struct {
> +		u64 vpid;
> +		u64 gla;
> +	} operand;
>  
>  	if (!(vmx->nested.nested_vmx_secondary_ctls_high &
>  	      SECONDARY_EXEC_ENABLE_VPID) ||
> @@ -7681,17 +7684,28 @@ static int handle_invvpid(struct kvm_vcpu *vcpu)
>  	if (get_vmx_mem_address(vcpu, vmcs_readl(EXIT_QUALIFICATION),
>  			vmx_instruction_info, false, &gva))
>  		return 1;
> -	if (kvm_read_guest_virt(&vcpu->arch.emulate_ctxt, gva, &vpid,
> -				sizeof(u32), &e)) {
> +	if (kvm_read_guest_virt(&vcpu->arch.emulate_ctxt, gva, &operand,
> +				sizeof(operand), &e)) {
>  		kvm_inject_page_fault(vcpu, &e);
>  		return 1;
>  	}
> +	if (operand.vpid >> 16) {
> +		nested_vmx_failValid(vcpu,
> +			VMXERR_INVALID_OPERAND_TO_INVEPT_INVVPID);
> +		return kvm_skip_emulated_instruction(vcpu);
> +	}
>  
>  	switch (type) {
>  	case VMX_VPID_EXTENT_INDIVIDUAL_ADDR:
> +		if (is_noncanonical_address(operand.gla)) {
> +			nested_vmx_failValid(vcpu,
> +				VMXERR_INVALID_OPERAND_TO_INVEPT_INVVPID);
> +			return kvm_skip_emulated_instruction(vcpu);
> +		}
> +		/* fall through */
>  	case VMX_VPID_EXTENT_SINGLE_CONTEXT:
>  	case VMX_VPID_EXTENT_SINGLE_NON_GLOBAL:
> -		if (!vpid) {
> +		if (!operand.vpid) {
>  			nested_vmx_failValid(vcpu,
>  				VMXERR_INVALID_OPERAND_TO_INVEPT_INVVPID);
>  			return kvm_skip_emulated_instruction(vcpu);
> 

Re: [PATCH v2] kvm: nVMX: Check memory operand to INVVPID

[David Hildenbrand]

On 28.06.2017 18:37, Jim Mattson wrote:
> The memory operand fetched for INVVPID is 128 bits. Bits 63:16 are
> reserved and must be zero.  Otherwise, the instruction fails with
> VMfail(Invalid operand to INVEPT/INVVPID).  If the INVVPID_TYPE is 0
> (individual address invalidation), then bits 127:64 must be in
> canonical form, or the instruction fails with VMfail(Invalid operand
> to INVEPT/INVVPID).
> 
> Signed-off-by: Jim Mattson <jmattson@google.com>
> ---

My r-b still holds :)

-- 

Thanks,

David

Re: [PATCH v2] kvm: nVMX: Check memory operand to INVVPID

[Paolo Bonzini]

On 29/06/2017 00:41, Jim Mattson wrote:
>     Will apply, kvm-unit-tests patches are always
>     welcome. :)
> 
> Working on a crossport of the test. Bear with me.

No problem.

David, any news on the EPT without A/D bugfix, too?

Paolo

[kvm-unit-tests PATCH 1/2] Move vmx_{on,off} into vmx.h

[Jim Mattson]

Make these functions available to callers outside of vmx.c

Signed-off-by: Jim Mattson <jmattson@google.com>
---
 x86/vmx.c | 19 -------------------
 x86/vmx.h | 21 +++++++++++++++++++++
 2 files changed, 21 insertions(+), 19 deletions(-)

diff --git a/x86/vmx.c b/x86/vmx.c
index f7a34d20ab6a..aff13b430a67 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -424,25 +424,6 @@ static void __attribute__((__used__)) syscall_handler(u64 syscall_no)
 		current->syscall_handler(syscall_no);
 }
 
-static inline int vmx_on()
-{
-	bool ret;
-	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;
-	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;
-}
-
 static const char * const exit_reason_descriptions[] = {
 	[VMX_EXC_NMI]		= "VMX_EXC_NMI",
 	[VMX_EXTINT]		= "VMX_EXTINT",
diff --git a/x86/vmx.h b/x86/vmx.h
index 392979ba8ae8..cbba62e46959 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -585,10 +585,31 @@ extern union vmx_ctrl_msr ctrl_exit_rev;
 extern union vmx_ctrl_msr ctrl_enter_rev;
 extern union vmx_ept_vpid  ept_vpid;
 
+extern u64 *vmxon_region;
+
 void vmx_set_test_stage(u32 s);
 u32 vmx_get_test_stage(void);
 void vmx_inc_test_stage(void);
 
+static int vmx_on(void)
+{
+	bool ret;
+	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 int vmx_off(void)
+{
+	bool ret;
+	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;
+}
+
 static inline int make_vmcs_current(struct vmcs *vmcs)
 {
 	bool ret;
-- 
2.13.2.725.g09c95d1e9-goog

[kvm-unit-tests PATCH 2/2] Add basic invvpid test

[Jim Mattson]

Tests only for success/failure of invvpid. Does not test actual
invvpid functionality.

Signed-off-by: Jim Mattson <jmattson@google.com>
---
 lib/x86/processor.h |   5 ++
 x86/unittests.cfg   |   6 ++
 x86/vmx.c           |   6 +-
 x86/vmx.h           |  24 ++++---
 x86/vmx_tests.c     | 194 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 5 files changed, 219 insertions(+), 16 deletions(-)

diff --git a/lib/x86/processor.h b/lib/x86/processor.h
index 895d992a6943..8839923e9207 100644
--- a/lib/x86/processor.h
+++ b/lib/x86/processor.h
@@ -430,4 +430,9 @@ static inline void write_pkru(u32 pkru)
         : : "a" (eax), "c" (ecx), "d" (edx));
 }
 
+static inline bool is_canonical(u64 addr)
+{
+	return (s64)(addr << 16) >> 16 == addr;
+}
+
 #endif
diff --git a/x86/unittests.cfg b/x86/unittests.cfg
index 5ab46671d631..c9858159c657 100644
--- a/x86/unittests.cfg
+++ b/x86/unittests.cfg
@@ -494,6 +494,12 @@ extra_params = -cpu host,+vmx -m 2048 -append ept_access_test_force_2m_page
 arch = x86_64
 groups = vmx
 
+[vmx_invvpid]
+file = vmx.flat
+extra_params = -cpu host,+vmx -m 2048 -append invvpid_test_v2
+arch = x86_64
+groups = vmx
+
 [debug]
 file = debug.flat
 arch = x86_64
diff --git a/x86/vmx.c b/x86/vmx.c
index aff13b430a67..f71d71f64f46 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -987,9 +987,9 @@ bool ept_ad_bits_supported(void)
 void vpid_sync(int type, u16 vpid)
 {
 	switch(type) {
-	case INVVPID_SINGLE:
-		if (ept_vpid.val & VPID_CAP_INVVPID_SINGLE) {
-			invvpid(INVVPID_SINGLE, vpid, 0);
+	case INVVPID_CONTEXT_GLOBAL:
+		if (ept_vpid.val & VPID_CAP_INVVPID_CXTGLB) {
+			invvpid(INVVPID_CONTEXT_GLOBAL, vpid, 0);
 			break;
 		}
 	case INVVPID_ALL:
diff --git a/x86/vmx.h b/x86/vmx.h
index cbba62e46959..42d7f0a4e5e3 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -13,6 +13,11 @@ struct vmcs {
 	char data[0];
 };
 
+struct invvpid_operand {
+	u64 vpid;
+	u64 gla;
+};
+
 struct regs {
 	u64 rax;
 	u64 rcx;
@@ -537,8 +542,10 @@ enum vm_instruction_error_number {
 #define EPT_CAP_INVEPT_ALL	(1ull << 26)
 #define EPT_CAP_AD_FLAG		(1ull << 21)
 #define VPID_CAP_INVVPID	(1ull << 32)
-#define VPID_CAP_INVVPID_SINGLE	(1ull << 41)
-#define VPID_CAP_INVVPID_ALL	(1ull << 42)
+#define VPID_CAP_INVVPID_ADDR   (1ull << 40)
+#define VPID_CAP_INVVPID_CXTGLB (1ull << 41)
+#define VPID_CAP_INVVPID_ALL    (1ull << 42)
+#define VPID_CAP_INVVPID_CXTLOC	(1ull << 43)
 
 #define PAGE_SIZE_2M		(512 * PAGE_SIZE)
 #define PAGE_SIZE_1G		(512 * PAGE_SIZE_2M)
@@ -569,9 +576,10 @@ enum vm_instruction_error_number {
 #define INVEPT_SINGLE		1
 #define INVEPT_GLOBAL		2
 
-#define INVVPID_SINGLE_ADDRESS	0
-#define INVVPID_SINGLE		1
+#define INVVPID_ADDR            0
+#define INVVPID_CONTEXT_GLOBAL	1
 #define INVVPID_ALL		2
+#define INVVPID_CONTEXT_LOCAL	3
 
 #define ACTV_ACTIVE		0
 #define ACTV_HLT		1
@@ -687,16 +695,12 @@ static inline bool invept(unsigned long type, u64 eptp)
 	return ret;
 }
 
-static inline bool invvpid(unsigned long type, u16 vpid, u64 gva)
+static inline bool invvpid(unsigned long type, u64 vpid, u64 gla)
 {
 	bool ret;
 	u64 rflags = read_rflags() | X86_EFLAGS_CF | X86_EFLAGS_ZF;
 
-	struct {
-		u64 vpid : 16;
-		u64 rsvd : 48;
-		u64 gva;
-	} operand = {vpid, 0, gva};
+	struct invvpid_operand operand = {vpid, gla};
 	asm volatile("push %1; popf; invvpid %2, %3; setbe %0"
 		     : "=q" (ret) : "r" (rflags), "m"(operand),"r"(type) : "cc");
 	return ret;
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index 03e4ad43eba3..5043652291c6 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -13,6 +13,10 @@
 #include "apic.h"
 #include "types.h"
 
+#define NONCANONICAL            0xaaaaaaaaaaaaaaaaull
+
+#define VPID_CAP_INVVPID_TYPES_SHIFT 40
+
 u64 ia32_pat;
 u64 ia32_efer;
 void *io_bitmap_a, *io_bitmap_b;
@@ -25,6 +29,15 @@ void *data_page1, *data_page2;
 void *pml_log;
 #define PML_INDEX 512
 
+static inline unsigned ffs(unsigned x)
+{
+	int pos = -1;
+
+	__asm__ __volatile__("bsf %1, %%eax; cmovnz %%eax, %0"
+			     : "+r"(pos) : "rm"(x) : "eax");
+	return pos + 1;
+}
+
 static inline void vmcall()
 {
 	asm volatile("vmcall");
@@ -1375,7 +1388,8 @@ bool invvpid_test(int type, u16 vpid)
 {
 	bool ret, supported;
 
-	supported = ept_vpid.val & (VPID_CAP_INVVPID_SINGLE >> INVVPID_SINGLE << type);
+	supported = ept_vpid.val &
+		(VPID_CAP_INVVPID_ADDR >> INVVPID_ADDR << type);
 	ret = invvpid(type, vpid, 0);
 
 	if (ret == !supported)
@@ -1432,11 +1446,11 @@ static int vpid_exit_handler()
 	case VMX_VMCALL:
 		switch(vmx_get_test_stage()) {
 		case 0:
-			if (!invvpid_test(INVVPID_SINGLE_ADDRESS, 1))
+			if (!invvpid_test(INVVPID_ADDR, 1))
 				vmx_inc_test_stage();
 			break;
 		case 2:
-			if (!invvpid_test(INVVPID_SINGLE, 1))
+			if (!invvpid_test(INVVPID_CONTEXT_GLOBAL, 1))
 				vmx_inc_test_stage();
 			break;
 		case 4:
@@ -2916,6 +2930,178 @@ static void ept_access_test_force_2m_page(void)
 	ept_misconfig_at_level_mkhuge(true, 2, EPT_PRESENT, EPT_WA);
 }
 
+static bool invvpid_valid(u64 type, u64 vpid, u64 gla)
+{
+	u64 msr = rdmsr(MSR_IA32_VMX_EPT_VPID_CAP);
+
+	TEST_ASSERT(msr & VPID_CAP_INVVPID);
+
+	if (type < INVVPID_ADDR || type > INVVPID_CONTEXT_LOCAL)
+		return false;
+
+	if (!(msr & (1ull << (type + VPID_CAP_INVVPID_TYPES_SHIFT))))
+		return false;
+
+	if (vpid >> 16)
+		return false;
+
+	if (type != INVVPID_ALL && !vpid)
+		return false;
+
+	if (type == INVVPID_ADDR && !is_canonical(gla))
+		return false;
+
+	return true;
+}
+
+static void try_invvpid(u64 type, u64 vpid, u64 gla)
+{
+	int rc;
+	bool valid = invvpid_valid(type, vpid, gla);
+	u64 expected = valid ? VMXERR_UNSUPPORTED_VMCS_COMPONENT
+		: VMXERR_INVALID_OPERAND_TO_INVEPT_INVVPID;
+	/*
+	 * Set VMX_INST_ERROR to VMXERR_UNVALID_VMCS_COMPONENT, so
+	 * that we can tell if it is updated by INVVPID.
+	 */
+	vmcs_read(~0);
+	rc = invvpid(type, vpid, gla);
+	report("INVVPID type %ld VPID %lx GLA %lx %s",
+	       !rc == valid, type, vpid, gla,
+	       valid ? "passes" : "fails");
+	report("After %s INVVPID, VMX_INST_ERR is %d (actual %d)",
+	       vmcs_read(VMX_INST_ERROR) == expected,
+	       rc ? "failed" : "successful",
+	       expected, vmcs_read(VMX_INST_ERROR));
+}
+
+static void ds_invvpid(void *data)
+{
+	u64 msr = rdmsr(MSR_IA32_VMX_EPT_VPID_CAP);
+	u64 type = ffs(msr >> VPID_CAP_INVVPID_TYPES_SHIFT) - 1;
+
+	TEST_ASSERT(type >= INVVPID_ADDR && type <= INVVPID_CONTEXT_LOCAL);
+	asm volatile("invvpid %0, %1"
+		     :
+		     : "m"(*(struct invvpid_operand *)data),
+		       "r"(type));
+}
+
+/*
+ * The SS override is ignored in 64-bit mode, so we use an addressing
+ * mode with %rsp as the base register to generate an implicit SS
+ * reference.
+ */
+static void ss_invvpid(void *data)
+{
+	u64 msr = rdmsr(MSR_IA32_VMX_EPT_VPID_CAP);
+	u64 type = ffs(msr >> VPID_CAP_INVVPID_TYPES_SHIFT) - 1;
+
+	TEST_ASSERT(type >= INVVPID_ADDR && type <= INVVPID_CONTEXT_LOCAL);
+	asm volatile("sub %%rsp,%0; invvpid (%%rsp,%0,1), %1"
+		     : "+r"(data)
+		     : "r"(type));
+}
+
+static void invvpid_test_gp(void)
+{
+	bool fault;
+
+	fault = test_for_exception(GP_VECTOR, &ds_invvpid,
+				   (void *)NONCANONICAL);
+	report("INVVPID with non-canonical DS operand raises #GP", fault);
+}
+
+static void invvpid_test_ss(void)
+{
+	bool fault;
+
+	fault = test_for_exception(SS_VECTOR, &ss_invvpid,
+				   (void *)NONCANONICAL);
+	report("INVVPID with non-canonical SS operand raises #SS", fault);
+}
+
+static void invvpid_test_pf(void)
+{
+	void *vpage = alloc_vpage();
+	bool fault;
+
+	fault = test_for_exception(PF_VECTOR, &ds_invvpid, vpage);
+	report("INVVPID with unmapped operand raises #PF", fault);
+}
+
+static void invvpid_test_not_in_vmx_operation(void)
+{
+	bool fault;
+
+	TEST_ASSERT(!vmx_off());
+	fault = test_for_exception(UD_VECTOR, &ds_invvpid, NULL);
+	report("INVVPID outside of VMX operation raises #UD", fault);
+	TEST_ASSERT(!vmx_on());
+}
+
+/*
+ * This does not test real-address mode, virtual-8086 mode, protected mode,
+ * compatibility mode, or CPL > 0.
+ */
+static void invvpid_test_v2(void)
+{
+	u64 msr;
+	int i;
+	unsigned types = 0;
+	unsigned type;
+
+	if (!(ctrl_cpu_rev[0].clr & CPU_SECONDARY) ||
+	    !(ctrl_cpu_rev[1].clr & CPU_VPID))
+		test_skip("VPID not supported");
+
+	msr = rdmsr(MSR_IA32_VMX_EPT_VPID_CAP);
+
+	if (!(msr & VPID_CAP_INVVPID))
+		test_skip("INVVPID not supported.\n");
+
+	if (msr & VPID_CAP_INVVPID_ADDR)
+		types |= 1u << INVVPID_ADDR;
+	if (msr & VPID_CAP_INVVPID_CXTGLB)
+		types |= 1u << INVVPID_CONTEXT_GLOBAL;
+	if (msr & VPID_CAP_INVVPID_ALL)
+		types |= 1u << INVVPID_ALL;
+	if (msr & VPID_CAP_INVVPID_CXTLOC)
+		types |= 1u << INVVPID_CONTEXT_LOCAL;
+
+	if (!types)
+		test_skip("No INVVPID types supported.\n");
+
+	for (i = -127; i < 128; i++)
+		try_invvpid(i, 0xffff, 0);
+
+	/*
+	 * VPID must not be more than 16 bits.
+	 */
+	for (i = 0; i < 64; i++)
+		for (type = 0; type < 4; type++)
+			if (types & (1u << type))
+				try_invvpid(type, 1ul << i, 0);
+
+	/*
+	 * VPID must not be zero, except for "all contexts."
+	 */
+	for (type = 0; type < 4; type++)
+		if (types & (1u << type))
+			try_invvpid(type, 0, 0);
+
+	/*
+	 * The gla operand is only validated for single-address INVVPID.
+	 */
+	if (types & (1u << INVVPID_ADDR))
+		try_invvpid(INVVPID_ADDR, 0xffff, NONCANONICAL);
+
+	invvpid_test_gp();
+	invvpid_test_ss();
+	invvpid_test_pf();
+	invvpid_test_not_in_vmx_operation();
+}
+
 #define TEST(name) { #name, .v2 = name }
 
 /* name/init/guest_main/exit_handler/syscall_handler/guest_regs */
@@ -2977,5 +3163,7 @@ struct vmx_test vmx_tests[] = {
 	TEST(ept_access_test_paddr_read_execute_ad_enabled),
 	TEST(ept_access_test_paddr_not_present_page_fault),
 	TEST(ept_access_test_force_2m_page),
+	/* Opcode tests. */
+	TEST(invvpid_test_v2),
 	{ NULL, NULL, NULL, NULL, NULL, {0} },
 };
-- 
2.13.2.725.g09c95d1e9-goog

Re: [PATCH v2] kvm: nVMX: Check memory operand to INVVPID

[David Matlack]

On Thu, Jun 29, 2017 at 1:22 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 29/06/2017 00:41, Jim Mattson wrote:
>>     Will apply, kvm-unit-tests patches are always
>>     welcome. :)
>>
>> Working on a crossport of the test. Bear with me.
>
> No problem.
>
> David, any news on the EPT without A/D bugfix, too?

Thanks for the reminder. Peter's aiming to get it out today or
tomorrow. Sorry for the delay!

>
> Paolo

[kvm-unit-tests PATCH v2 1/4] Save/restore handler in test_for_exception

[Jim Mattson]

The default handler for #DE, #UD, and #GP is check_exception_table.
Test_for_exception should restore the original handler before
returning, rather than blindly clobbering it with NULL.

Signed-off-by: Jim Mattson <jmattson@google.com>
---
 lib/x86/desc.c | 16 ++++++++++------
 lib/x86/desc.h |  4 +++-
 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/lib/x86/desc.c b/lib/x86/desc.c
index 402204ddcac4..830c5d127dbc 100644
--- a/lib/x86/desc.c
+++ b/lib/x86/desc.c
@@ -117,13 +117,16 @@ static void check_exception_table(struct ex_regs *regs)
     unhandled_exception(regs, false);
 }
 
-static void (*exception_handlers[32])(struct ex_regs *regs);
+static handler exception_handlers[32];
 
-
-void handle_exception(u8 v, void (*func)(struct ex_regs *regs))
+handler handle_exception(u8 v, handler fn)
 {
+	handler old;
+
+	old = exception_handlers[v];
 	if (v < 32)
-		exception_handlers[v] = func;
+		exception_handlers[v] = fn;
+	return old;
 }
 
 #ifndef __x86_64__
@@ -390,14 +393,15 @@ static void exception_handler(struct ex_regs *regs)
 bool test_for_exception(unsigned int ex, void (*trigger_func)(void *data),
 			void *data)
 {
+	handler old;
 	jmp_buf jmpbuf;
 	int ret;
 
-	handle_exception(ex, exception_handler);
+	old = handle_exception(ex, exception_handler);
 	ret = set_exception_jmpbuf(jmpbuf);
 	if (ret == 0)
 		trigger_func(data);
-	handle_exception(ex, NULL);
+	handle_exception(ex, old);
 	return ret;
 }
 
diff --git a/lib/x86/desc.h b/lib/x86/desc.h
index be52fd4e1d92..a2500e0f6006 100644
--- a/lib/x86/desc.h
+++ b/lib/x86/desc.h
@@ -20,6 +20,8 @@ struct ex_regs {
     unsigned long rflags;
 };
 
+typedef void (*handler)(struct ex_regs *regs);
+
 typedef struct {
 	u16 prev;
 	u16 res1;
@@ -153,7 +155,7 @@ void set_idt_dpl(int vec, u16 dpl);
 void set_gdt_entry(int sel, u32 base,  u32 limit, u8 access, u8 gran);
 void set_intr_alt_stack(int e, void *fn);
 void print_current_tss_info(void);
-void handle_exception(u8 v, void (*func)(struct ex_regs *regs));
+handler handle_exception(u8 v, handler fn);
 
 bool test_for_exception(unsigned int ex, void (*trigger_func)(void *data),
 			void *data);
-- 
2.13.2.725.g09c95d1e9-goog

[kvm-unit-tests PATCH v2 2/4] Specify %cs for exception_handler iret

[Jim Mattson]

The exception handler longjmp should happen in the code segment of the
exception handler rather than in the code segment of the exception
(particularly when the exception occurs in compatibility mode).

Signed-off-by: Jim Mattson <jmattson@google.com>
---
 lib/x86/desc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/x86/desc.c b/lib/x86/desc.c
index 830c5d127dbc..fc6a67eb0a19 100644
--- a/lib/x86/desc.c
+++ b/lib/x86/desc.c
@@ -388,6 +388,7 @@ static void exception_handler(struct ex_regs *regs)
 	/* longjmp must happen after iret, so do not do it now.  */
 	exception = true;
 	regs->rip = (unsigned long)&exception_handler_longjmp;
+	regs->cs = read_cs();
 }
 
 bool test_for_exception(unsigned int ex, void (*trigger_func)(void *data),
-- 
2.13.2.725.g09c95d1e9-goog

[kvm-unit-tests PATCH v2 3/4] Move vmx_{on,off} into vmx.h

[Jim Mattson]

Make these functions available to callers outside of vmx.c

Signed-off-by: Jim Mattson <jmattson@google.com>
---
 x86/vmx.c | 19 -------------------
 x86/vmx.h | 21 +++++++++++++++++++++
 2 files changed, 21 insertions(+), 19 deletions(-)

diff --git a/x86/vmx.c b/x86/vmx.c
index f7a34d20ab6a..aff13b430a67 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -424,25 +424,6 @@ static void __attribute__((__used__)) syscall_handler(u64 syscall_no)
 		current->syscall_handler(syscall_no);
 }
 
-static inline int vmx_on()
-{
-	bool ret;
-	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;
-	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;
-}
-
 static const char * const exit_reason_descriptions[] = {
 	[VMX_EXC_NMI]		= "VMX_EXC_NMI",
 	[VMX_EXTINT]		= "VMX_EXTINT",
diff --git a/x86/vmx.h b/x86/vmx.h
index 392979ba8ae8..cbba62e46959 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -585,10 +585,31 @@ extern union vmx_ctrl_msr ctrl_exit_rev;
 extern union vmx_ctrl_msr ctrl_enter_rev;
 extern union vmx_ept_vpid  ept_vpid;
 
+extern u64 *vmxon_region;
+
 void vmx_set_test_stage(u32 s);
 u32 vmx_get_test_stage(void);
 void vmx_inc_test_stage(void);
 
+static int vmx_on(void)
+{
+	bool ret;
+	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 int vmx_off(void)
+{
+	bool ret;
+	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;
+}
+
 static inline int make_vmcs_current(struct vmcs *vmcs)
 {
 	bool ret;
-- 
2.13.2.725.g09c95d1e9-goog

[kvm-unit-tests PATCH v2 4/4] Add basic invvpid test

[Jim Mattson]

Tests only for success/failure of invvpid. Does not test actual
invvpid functionality.

Signed-off-by: Jim Mattson <jmattson@google.com>
---
 lib/x86/processor.h |   5 ++
 x86/unittests.cfg   |   6 ++
 x86/vmx.c           |   6 +-
 x86/vmx.h           |  24 +++---
 x86/vmx_tests.c     | 225 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 5 files changed, 250 insertions(+), 16 deletions(-)

diff --git a/lib/x86/processor.h b/lib/x86/processor.h
index 895d992a6943..8839923e9207 100644
--- a/lib/x86/processor.h
+++ b/lib/x86/processor.h
@@ -430,4 +430,9 @@ static inline void write_pkru(u32 pkru)
         : : "a" (eax), "c" (ecx), "d" (edx));
 }
 
+static inline bool is_canonical(u64 addr)
+{
+	return (s64)(addr << 16) >> 16 == addr;
+}
+
 #endif
diff --git a/x86/unittests.cfg b/x86/unittests.cfg
index 5ab46671d631..c9858159c657 100644
--- a/x86/unittests.cfg
+++ b/x86/unittests.cfg
@@ -494,6 +494,12 @@ extra_params = -cpu host,+vmx -m 2048 -append ept_access_test_force_2m_page
 arch = x86_64
 groups = vmx
 
+[vmx_invvpid]
+file = vmx.flat
+extra_params = -cpu host,+vmx -m 2048 -append invvpid_test_v2
+arch = x86_64
+groups = vmx
+
 [debug]
 file = debug.flat
 arch = x86_64
diff --git a/x86/vmx.c b/x86/vmx.c
index aff13b430a67..f71d71f64f46 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -987,9 +987,9 @@ bool ept_ad_bits_supported(void)
 void vpid_sync(int type, u16 vpid)
 {
 	switch(type) {
-	case INVVPID_SINGLE:
-		if (ept_vpid.val & VPID_CAP_INVVPID_SINGLE) {
-			invvpid(INVVPID_SINGLE, vpid, 0);
+	case INVVPID_CONTEXT_GLOBAL:
+		if (ept_vpid.val & VPID_CAP_INVVPID_CXTGLB) {
+			invvpid(INVVPID_CONTEXT_GLOBAL, vpid, 0);
 			break;
 		}
 	case INVVPID_ALL:
diff --git a/x86/vmx.h b/x86/vmx.h
index cbba62e46959..42d7f0a4e5e3 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -13,6 +13,11 @@ struct vmcs {
 	char data[0];
 };
 
+struct invvpid_operand {
+	u64 vpid;
+	u64 gla;
+};
+
 struct regs {
 	u64 rax;
 	u64 rcx;
@@ -537,8 +542,10 @@ enum vm_instruction_error_number {
 #define EPT_CAP_INVEPT_ALL	(1ull << 26)
 #define EPT_CAP_AD_FLAG		(1ull << 21)
 #define VPID_CAP_INVVPID	(1ull << 32)
-#define VPID_CAP_INVVPID_SINGLE	(1ull << 41)
-#define VPID_CAP_INVVPID_ALL	(1ull << 42)
+#define VPID_CAP_INVVPID_ADDR   (1ull << 40)
+#define VPID_CAP_INVVPID_CXTGLB (1ull << 41)
+#define VPID_CAP_INVVPID_ALL    (1ull << 42)
+#define VPID_CAP_INVVPID_CXTLOC	(1ull << 43)
 
 #define PAGE_SIZE_2M		(512 * PAGE_SIZE)
 #define PAGE_SIZE_1G		(512 * PAGE_SIZE_2M)
@@ -569,9 +576,10 @@ enum vm_instruction_error_number {
 #define INVEPT_SINGLE		1
 #define INVEPT_GLOBAL		2
 
-#define INVVPID_SINGLE_ADDRESS	0
-#define INVVPID_SINGLE		1
+#define INVVPID_ADDR            0
+#define INVVPID_CONTEXT_GLOBAL	1
 #define INVVPID_ALL		2
+#define INVVPID_CONTEXT_LOCAL	3
 
 #define ACTV_ACTIVE		0
 #define ACTV_HLT		1
@@ -687,16 +695,12 @@ static inline bool invept(unsigned long type, u64 eptp)
 	return ret;
 }
 
-static inline bool invvpid(unsigned long type, u16 vpid, u64 gva)
+static inline bool invvpid(unsigned long type, u64 vpid, u64 gla)
 {
 	bool ret;
 	u64 rflags = read_rflags() | X86_EFLAGS_CF | X86_EFLAGS_ZF;
 
-	struct {
-		u64 vpid : 16;
-		u64 rsvd : 48;
-		u64 gva;
-	} operand = {vpid, 0, gva};
+	struct invvpid_operand operand = {vpid, gla};
 	asm volatile("push %1; popf; invvpid %2, %3; setbe %0"
 		     : "=q" (ret) : "r" (rflags), "m"(operand),"r"(type) : "cc");
 	return ret;
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index 03e4ad43eba3..ffc913573fb4 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -13,6 +13,10 @@
 #include "apic.h"
 #include "types.h"
 
+#define NONCANONICAL            0xaaaaaaaaaaaaaaaaull
+
+#define VPID_CAP_INVVPID_TYPES_SHIFT 40
+
 u64 ia32_pat;
 u64 ia32_efer;
 void *io_bitmap_a, *io_bitmap_b;
@@ -25,6 +29,15 @@ void *data_page1, *data_page2;
 void *pml_log;
 #define PML_INDEX 512
 
+static inline unsigned ffs(unsigned x)
+{
+	int pos = -1;
+
+	__asm__ __volatile__("bsf %1, %%eax; cmovnz %%eax, %0"
+			     : "+r"(pos) : "rm"(x) : "eax");
+	return pos + 1;
+}
+
 static inline void vmcall()
 {
 	asm volatile("vmcall");
@@ -1375,7 +1388,8 @@ bool invvpid_test(int type, u16 vpid)
 {
 	bool ret, supported;
 
-	supported = ept_vpid.val & (VPID_CAP_INVVPID_SINGLE >> INVVPID_SINGLE << type);
+	supported = ept_vpid.val &
+		(VPID_CAP_INVVPID_ADDR >> INVVPID_ADDR << type);
 	ret = invvpid(type, vpid, 0);
 
 	if (ret == !supported)
@@ -1432,11 +1446,11 @@ static int vpid_exit_handler()
 	case VMX_VMCALL:
 		switch(vmx_get_test_stage()) {
 		case 0:
-			if (!invvpid_test(INVVPID_SINGLE_ADDRESS, 1))
+			if (!invvpid_test(INVVPID_ADDR, 1))
 				vmx_inc_test_stage();
 			break;
 		case 2:
-			if (!invvpid_test(INVVPID_SINGLE, 1))
+			if (!invvpid_test(INVVPID_CONTEXT_GLOBAL, 1))
 				vmx_inc_test_stage();
 			break;
 		case 4:
@@ -2916,6 +2930,209 @@ static void ept_access_test_force_2m_page(void)
 	ept_misconfig_at_level_mkhuge(true, 2, EPT_PRESENT, EPT_WA);
 }
 
+static bool invvpid_valid(u64 type, u64 vpid, u64 gla)
+{
+	u64 msr = rdmsr(MSR_IA32_VMX_EPT_VPID_CAP);
+
+	TEST_ASSERT(msr & VPID_CAP_INVVPID);
+
+	if (type < INVVPID_ADDR || type > INVVPID_CONTEXT_LOCAL)
+		return false;
+
+	if (!(msr & (1ull << (type + VPID_CAP_INVVPID_TYPES_SHIFT))))
+		return false;
+
+	if (vpid >> 16)
+		return false;
+
+	if (type != INVVPID_ALL && !vpid)
+		return false;
+
+	if (type == INVVPID_ADDR && !is_canonical(gla))
+		return false;
+
+	return true;
+}
+
+static void try_invvpid(u64 type, u64 vpid, u64 gla)
+{
+	int rc;
+	bool valid = invvpid_valid(type, vpid, gla);
+	u64 expected = valid ? VMXERR_UNSUPPORTED_VMCS_COMPONENT
+		: VMXERR_INVALID_OPERAND_TO_INVEPT_INVVPID;
+	/*
+	 * Set VMX_INST_ERROR to VMXERR_UNVALID_VMCS_COMPONENT, so
+	 * that we can tell if it is updated by INVVPID.
+	 */
+	vmcs_read(~0);
+	rc = invvpid(type, vpid, gla);
+	report("INVVPID type %ld VPID %lx GLA %lx %s",
+	       !rc == valid, type, vpid, gla,
+	       valid ? "passes" : "fails");
+	report("After %s INVVPID, VMX_INST_ERR is %d (actual %d)",
+	       vmcs_read(VMX_INST_ERROR) == expected,
+	       rc ? "failed" : "successful",
+	       expected, vmcs_read(VMX_INST_ERROR));
+}
+
+static void ds_invvpid(void *data)
+{
+	u64 msr = rdmsr(MSR_IA32_VMX_EPT_VPID_CAP);
+	u64 type = ffs(msr >> VPID_CAP_INVVPID_TYPES_SHIFT) - 1;
+
+	TEST_ASSERT(type >= INVVPID_ADDR && type <= INVVPID_CONTEXT_LOCAL);
+	asm volatile("invvpid %0, %1"
+		     :
+		     : "m"(*(struct invvpid_operand *)data),
+		       "r"(type));
+}
+
+/*
+ * The SS override is ignored in 64-bit mode, so we use an addressing
+ * mode with %rsp as the base register to generate an implicit SS
+ * reference.
+ */
+static void ss_invvpid(void *data)
+{
+	u64 msr = rdmsr(MSR_IA32_VMX_EPT_VPID_CAP);
+	u64 type = ffs(msr >> VPID_CAP_INVVPID_TYPES_SHIFT) - 1;
+
+	TEST_ASSERT(type >= INVVPID_ADDR && type <= INVVPID_CONTEXT_LOCAL);
+	asm volatile("sub %%rsp,%0; invvpid (%%rsp,%0,1), %1"
+		     : "+r"(data)
+		     : "r"(type));
+}
+
+static void invvpid_test_gp(void)
+{
+	bool fault;
+
+	fault = test_for_exception(GP_VECTOR, &ds_invvpid,
+				   (void *)NONCANONICAL);
+	report("INVVPID with non-canonical DS operand raises #GP", fault);
+}
+
+static void invvpid_test_ss(void)
+{
+	bool fault;
+
+	fault = test_for_exception(SS_VECTOR, &ss_invvpid,
+				   (void *)NONCANONICAL);
+	report("INVVPID with non-canonical SS operand raises #SS", fault);
+}
+
+static void invvpid_test_pf(void)
+{
+	void *vpage = alloc_vpage();
+	bool fault;
+
+	fault = test_for_exception(PF_VECTOR, &ds_invvpid, vpage);
+	report("INVVPID with unmapped operand raises #PF", fault);
+}
+
+static void try_compat_invvpid(void *unused)
+{
+	struct far_pointer32 fp = {
+		.offset = (uintptr_t)&&invvpid,
+		.selector = KERNEL_CS32,
+	};
+	register uintptr_t rsp asm("rsp");
+
+	TEST_ASSERT_MSG(fp.offset == (uintptr_t)&&invvpid,
+			"Code address too high.");
+	TEST_ASSERT_MSG(rsp == (u32)rsp, "Stack address too high.");
+
+	asm goto ("lcall *%0" : : "m" (fp) : "rax" : invvpid);
+	return;
+invvpid:
+	asm volatile (".code32;"
+		      "invvpid (%eax), %eax;"
+		      "lret;"
+		      ".code64");
+	__builtin_unreachable();
+}
+
+static void invvpid_test_compatibility_mode(void)
+{
+	bool fault;
+
+	fault = test_for_exception(UD_VECTOR, &try_compat_invvpid, NULL);
+	report("Compatibility mode INVVPID raises #UD", fault);
+}
+
+static void invvpid_test_not_in_vmx_operation(void)
+{
+	bool fault;
+
+	TEST_ASSERT(!vmx_off());
+	fault = test_for_exception(UD_VECTOR, &ds_invvpid, NULL);
+	report("INVVPID outside of VMX operation raises #UD", fault);
+	TEST_ASSERT(!vmx_on());
+}
+
+/*
+ * This does not test real-address mode, virtual-8086 mode, protected mode,
+ * or CPL > 0.
+ */
+static void invvpid_test_v2(void)
+{
+	u64 msr;
+	int i;
+	unsigned types = 0;
+	unsigned type;
+
+	if (!(ctrl_cpu_rev[0].clr & CPU_SECONDARY) ||
+	    !(ctrl_cpu_rev[1].clr & CPU_VPID))
+		test_skip("VPID not supported");
+
+	msr = rdmsr(MSR_IA32_VMX_EPT_VPID_CAP);
+
+	if (!(msr & VPID_CAP_INVVPID))
+		test_skip("INVVPID not supported.\n");
+
+	if (msr & VPID_CAP_INVVPID_ADDR)
+		types |= 1u << INVVPID_ADDR;
+	if (msr & VPID_CAP_INVVPID_CXTGLB)
+		types |= 1u << INVVPID_CONTEXT_GLOBAL;
+	if (msr & VPID_CAP_INVVPID_ALL)
+		types |= 1u << INVVPID_ALL;
+	if (msr & VPID_CAP_INVVPID_CXTLOC)
+		types |= 1u << INVVPID_CONTEXT_LOCAL;
+
+	if (!types)
+		test_skip("No INVVPID types supported.\n");
+
+	for (i = -127; i < 128; i++)
+		try_invvpid(i, 0xffff, 0);
+
+	/*
+	 * VPID must not be more than 16 bits.
+	 */
+	for (i = 0; i < 64; i++)
+		for (type = 0; type < 4; type++)
+			if (types & (1u << type))
+				try_invvpid(type, 1ul << i, 0);
+
+	/*
+	 * VPID must not be zero, except for "all contexts."
+	 */
+	for (type = 0; type < 4; type++)
+		if (types & (1u << type))
+			try_invvpid(type, 0, 0);
+
+	/*
+	 * The gla operand is only validated for single-address INVVPID.
+	 */
+	if (types & (1u << INVVPID_ADDR))
+		try_invvpid(INVVPID_ADDR, 0xffff, NONCANONICAL);
+
+	invvpid_test_gp();
+	invvpid_test_ss();
+	invvpid_test_pf();
+	invvpid_test_compatibility_mode();
+	invvpid_test_not_in_vmx_operation();
+}
+
 #define TEST(name) { #name, .v2 = name }
 
 /* name/init/guest_main/exit_handler/syscall_handler/guest_regs */
@@ -2977,5 +3194,7 @@ struct vmx_test vmx_tests[] = {
 	TEST(ept_access_test_paddr_read_execute_ad_enabled),
 	TEST(ept_access_test_paddr_not_present_page_fault),
 	TEST(ept_access_test_force_2m_page),
+	/* Opcode tests. */
+	TEST(invvpid_test_v2),
 	{ NULL, NULL, NULL, NULL, NULL, {0} },
 };
-- 
2.13.2.725.g09c95d1e9-goog

Re: [kvm-unit-tests PATCH v2 1/4] Save/restore handler in test_for_exception

[Paolo Bonzini]

TOn 29/06/2017 20:46, Jim Mattson wrote:
> The default handler for #DE, #UD, and #GP is check_exception_table.
> Test_for_exception should restore the original handler before
> returning, rather than blindly clobbering it with NULL.
> 
> Signed-off-by: Jim Mattson <jmattson@google.com>

Thanks, series tested and applied.

Paolo

> ---
>  lib/x86/desc.c | 16 ++++++++++------
>  lib/x86/desc.h |  4 +++-
>  2 files changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/lib/x86/desc.c b/lib/x86/desc.c
> index 402204ddcac4..830c5d127dbc 100644
> --- a/lib/x86/desc.c
> +++ b/lib/x86/desc.c
> @@ -117,13 +117,16 @@ static void check_exception_table(struct ex_regs *regs)
>      unhandled_exception(regs, false);
>  }
>  
> -static void (*exception_handlers[32])(struct ex_regs *regs);
> +static handler exception_handlers[32];
>  
> -
> -void handle_exception(u8 v, void (*func)(struct ex_regs *regs))
> +handler handle_exception(u8 v, handler fn)
>  {
> +	handler old;
> +
> +	old = exception_handlers[v];
>  	if (v < 32)
> -		exception_handlers[v] = func;
> +		exception_handlers[v] = fn;
> +	return old;
>  }
>  
>  #ifndef __x86_64__
> @@ -390,14 +393,15 @@ static void exception_handler(struct ex_regs *regs)
>  bool test_for_exception(unsigned int ex, void (*trigger_func)(void *data),
>  			void *data)
>  {
> +	handler old;
>  	jmp_buf jmpbuf;
>  	int ret;
>  
> -	handle_exception(ex, exception_handler);
> +	old = handle_exception(ex, exception_handler);
>  	ret = set_exception_jmpbuf(jmpbuf);
>  	if (ret == 0)
>  		trigger_func(data);
> -	handle_exception(ex, NULL);
> +	handle_exception(ex, old);
>  	return ret;
>  }
>  
> diff --git a/lib/x86/desc.h b/lib/x86/desc.h
> index be52fd4e1d92..a2500e0f6006 100644
> --- a/lib/x86/desc.h
> +++ b/lib/x86/desc.h
> @@ -20,6 +20,8 @@ struct ex_regs {
>      unsigned long rflags;
>  };
>  
> +typedef void (*handler)(struct ex_regs *regs);
> +
>  typedef struct {
>  	u16 prev;
>  	u16 res1;
> @@ -153,7 +155,7 @@ void set_idt_dpl(int vec, u16 dpl);
>  void set_gdt_entry(int sel, u32 base,  u32 limit, u8 access, u8 gran);
>  void set_intr_alt_stack(int e, void *fn);
>  void print_current_tss_info(void);
> -void handle_exception(u8 v, void (*func)(struct ex_regs *regs));
> +handler handle_exception(u8 v, handler fn);
>  
>  bool test_for_exception(unsigned int ex, void (*trigger_func)(void *data),
>  			void *data);
> 

Re: [PATCH v2] kvm: nVMX: Check memory operand to INVVPID

[Peter Feiner]

On Thu, Jun 29, 2017 at 11:14 AM, David Matlack <dmatlack@google.com> wrote:
> On Thu, Jun 29, 2017 at 1:22 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> On 29/06/2017 00:41, Jim Mattson wrote:
>>>     Will apply, kvm-unit-tests patches are always
>>>     welcome. :)
>>>
>>> Working on a crossport of the test. Bear with me.
>>
>> No problem.
>>
>> David, any news on the EPT without A/D bugfix, too?
>
> Thanks for the reminder. Peter's aiming to get it out today or
> tomorrow. Sorry for the delay!

I could send the patches out but they aren't working yet :-) Debugging
the bitrot between Google's kernel & upstream is taking longer than I
had hoped.

Since I'm on vacation next week, I won't have it working until
sometime during the week of July 10.

Again, sorry for the delay getting started on this.

Peter