diff mbox series

x86: vmx: Fix "EPT violation - paging structure" test

Message ID 20200321050555.4212-1-namit@vmware.com (mailing list archive)
State New, archived
Headers show
Series x86: vmx: Fix "EPT violation - paging structure" test | expand

Commit Message

Nadav Amit March 21, 2020, 5:05 a.m. UTC 
Running the tests with more than 4GB of memory results in the following
failure:

  FAIL: EPT violation - paging structure

It appears that the test mistakenly used get_ept_pte() to retrieve the
guest PTE, but this function is intended for accessing EPT and not the
guest page tables. Use get_pte_level() instead of get_ept_pte().

Tested on bare-metal only.

Signed-off-by: Nadav Amit <namit@vmware.com>
---
 x86/vmx_tests.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

Comments

Nadav Amit March 23, 2020, 7:42 p.m. UTC | #1 
I mistakenly forgot the kvm-unit-tests in the patch subject. Sorry for that.


> On Mar 20, 2020, at 10:05 PM, Nadav Amit <namit@vmware.com> wrote:
> 
> Running the tests with more than 4GB of memory results in the following
> failure:
> 
>  FAIL: EPT violation - paging structure
> 
> It appears that the test mistakenly used get_ept_pte() to retrieve the
> guest PTE, but this function is intended for accessing EPT and not the
> guest page tables. Use get_pte_level() instead of get_ept_pte().
> 
> Tested on bare-metal only.
> 
> Signed-off-by: Nadav Amit <namit@vmware.com>
> ---
> x86/vmx_tests.c | 17 +++++++++--------
> 1 file changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
> index be5c952..1f97fe3 100644
> --- a/x86/vmx_tests.c
> +++ b/x86/vmx_tests.c
> @@ -1312,12 +1312,14 @@ static int ept_exit_handler_common(union exit_reason exit_reason, bool have_ad)
> 	u64 guest_cr3;
> 	u32 insn_len;
> 	u32 exit_qual;
> -	static unsigned long data_page1_pte, data_page1_pte_pte, memaddr_pte;
> +	static unsigned long data_page1_pte, data_page1_pte_pte, memaddr_pte,
> +			     guest_pte_addr;
> 
> 	guest_rip = vmcs_read(GUEST_RIP);
> 	guest_cr3 = vmcs_read(GUEST_CR3);
> 	insn_len = vmcs_read(EXI_INST_LEN);
> 	exit_qual = vmcs_read(EXI_QUALIFICATION);
> +	pteval_t *ptep;
> 	switch (exit_reason.basic) {
> 	case VMX_VMCALL:
> 		switch (vmx_get_test_stage()) {
> @@ -1364,12 +1366,11 @@ static int ept_exit_handler_common(union exit_reason exit_reason, bool have_ad)
> 			ept_sync(INVEPT_SINGLE, eptp);
> 			break;
> 		case 4:
> -			TEST_ASSERT(get_ept_pte(pml4, (unsigned long)data_page1,
> -						2, &data_page1_pte));
> -			data_page1_pte &= PAGE_MASK;
> -			TEST_ASSERT(get_ept_pte(pml4, data_page1_pte,
> -						2, &data_page1_pte_pte));
> -			set_ept_pte(pml4, data_page1_pte, 2,
> +			ptep = get_pte_level((pgd_t *)guest_cr3, data_page1, /*level=*/2);
> +			guest_pte_addr = virt_to_phys(ptep) & PAGE_MASK;
> +
> +			TEST_ASSERT(get_ept_pte(pml4, guest_pte_addr, 2, &data_page1_pte_pte));
> +			set_ept_pte(pml4, guest_pte_addr, 2,
> 				data_page1_pte_pte & ~EPT_PRESENT);
> 			ept_sync(INVEPT_SINGLE, eptp);
> 			break;
> @@ -1437,7 +1438,7 @@ static int ept_exit_handler_common(union exit_reason exit_reason, bool have_ad)
> 					  (have_ad ? EPT_VLT_WR : 0) |
> 					  EPT_VLT_LADDR_VLD))
> 				vmx_inc_test_stage();
> -			set_ept_pte(pml4, data_page1_pte, 2,
> +			set_ept_pte(pml4, guest_pte_addr, 2,
> 				data_page1_pte_pte | (EPT_PRESENT));
> 			ept_sync(INVEPT_SINGLE, eptp);
> 			break;
> — 
> 2.17.1
Paolo Bonzini March 31, 2020, 5:01 p.m. UTC | #2 
On 21/03/20 06:05, Nadav Amit wrote:
> Running the tests with more than 4GB of memory results in the following
> failure:
> 
>   FAIL: EPT violation - paging structure
> 
> It appears that the test mistakenly used get_ept_pte() to retrieve the
> guest PTE, but this function is intended for accessing EPT and not the
> guest page tables. Use get_pte_level() instead of get_ept_pte().
> 
> Tested on bare-metal only.
> 
> Signed-off-by: Nadav Amit <namit@vmware.com>
> ---
>  x86/vmx_tests.c | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
> index be5c952..1f97fe3 100644
> --- a/x86/vmx_tests.c
> +++ b/x86/vmx_tests.c
> @@ -1312,12 +1312,14 @@ static int ept_exit_handler_common(union exit_reason exit_reason, bool have_ad)
>  	u64 guest_cr3;
>  	u32 insn_len;
>  	u32 exit_qual;
> -	static unsigned long data_page1_pte, data_page1_pte_pte, memaddr_pte;
> +	static unsigned long data_page1_pte, data_page1_pte_pte, memaddr_pte,
> +			     guest_pte_addr;
>  
>  	guest_rip = vmcs_read(GUEST_RIP);
>  	guest_cr3 = vmcs_read(GUEST_CR3);
>  	insn_len = vmcs_read(EXI_INST_LEN);
>  	exit_qual = vmcs_read(EXI_QUALIFICATION);
> +	pteval_t *ptep;
>  	switch (exit_reason.basic) {
>  	case VMX_VMCALL:
>  		switch (vmx_get_test_stage()) {
> @@ -1364,12 +1366,11 @@ static int ept_exit_handler_common(union exit_reason exit_reason, bool have_ad)
>  			ept_sync(INVEPT_SINGLE, eptp);
>  			break;
>  		case 4:
> -			TEST_ASSERT(get_ept_pte(pml4, (unsigned long)data_page1,
> -						2, &data_page1_pte));
> -			data_page1_pte &= PAGE_MASK;
> -			TEST_ASSERT(get_ept_pte(pml4, data_page1_pte,
> -						2, &data_page1_pte_pte));
> -			set_ept_pte(pml4, data_page1_pte, 2,
> +			ptep = get_pte_level((pgd_t *)guest_cr3, data_page1, /*level=*/2);
> +			guest_pte_addr = virt_to_phys(ptep) & PAGE_MASK;
> +
> +			TEST_ASSERT(get_ept_pte(pml4, guest_pte_addr, 2, &data_page1_pte_pte));
> +			set_ept_pte(pml4, guest_pte_addr, 2,
>  				data_page1_pte_pte & ~EPT_PRESENT);
>  			ept_sync(INVEPT_SINGLE, eptp);
>  			break;
> @@ -1437,7 +1438,7 @@ static int ept_exit_handler_common(union exit_reason exit_reason, bool have_ad)
>  					  (have_ad ? EPT_VLT_WR : 0) |
>  					  EPT_VLT_LADDR_VLD))
>  				vmx_inc_test_stage();
> -			set_ept_pte(pml4, data_page1_pte, 2,
> +			set_ept_pte(pml4, guest_pte_addr, 2,
>  				data_page1_pte_pte | (EPT_PRESENT));
>  			ept_sync(INVEPT_SINGLE, eptp);
>  			break;
> 

Queued, thanks.

Paolo
diff mbox series

Patch

diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index be5c952..1f97fe3 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -1312,12 +1312,14 @@  static int ept_exit_handler_common(union exit_reason exit_reason, bool have_ad)
 	u64 guest_cr3;
 	u32 insn_len;
 	u32 exit_qual;
-	static unsigned long data_page1_pte, data_page1_pte_pte, memaddr_pte;
+	static unsigned long data_page1_pte, data_page1_pte_pte, memaddr_pte,
+			     guest_pte_addr;
 
 	guest_rip = vmcs_read(GUEST_RIP);
 	guest_cr3 = vmcs_read(GUEST_CR3);
 	insn_len = vmcs_read(EXI_INST_LEN);
 	exit_qual = vmcs_read(EXI_QUALIFICATION);
+	pteval_t *ptep;
 	switch (exit_reason.basic) {
 	case VMX_VMCALL:
 		switch (vmx_get_test_stage()) {
@@ -1364,12 +1366,11 @@  static int ept_exit_handler_common(union exit_reason exit_reason, bool have_ad)
 			ept_sync(INVEPT_SINGLE, eptp);
 			break;
 		case 4:
-			TEST_ASSERT(get_ept_pte(pml4, (unsigned long)data_page1,
-						2, &data_page1_pte));
-			data_page1_pte &= PAGE_MASK;
-			TEST_ASSERT(get_ept_pte(pml4, data_page1_pte,
-						2, &data_page1_pte_pte));
-			set_ept_pte(pml4, data_page1_pte, 2,
+			ptep = get_pte_level((pgd_t *)guest_cr3, data_page1, /*level=*/2);
+			guest_pte_addr = virt_to_phys(ptep) & PAGE_MASK;
+
+			TEST_ASSERT(get_ept_pte(pml4, guest_pte_addr, 2, &data_page1_pte_pte));
+			set_ept_pte(pml4, guest_pte_addr, 2,
 				data_page1_pte_pte & ~EPT_PRESENT);
 			ept_sync(INVEPT_SINGLE, eptp);
 			break;
@@ -1437,7 +1438,7 @@  static int ept_exit_handler_common(union exit_reason exit_reason, bool have_ad)
 					  (have_ad ? EPT_VLT_WR : 0) |
 					  EPT_VLT_LADDR_VLD))
 				vmx_inc_test_stage();
-			set_ept_pte(pml4, data_page1_pte, 2,
+			set_ept_pte(pml4, guest_pte_addr, 2,
 				data_page1_pte_pte | (EPT_PRESENT));
 			ept_sync(INVEPT_SINGLE, eptp);
 			break;