[kvm-unit-tests PATCH 2/3] x86/vmx: fix detection of unmapped PTE

["Radim Krčmář" <rkrcmar@redhat.com>]

check_ept_ad() was testing get_ept_pte() for 0, which meant that the PTE
lookup failed.  dff740c00197 ("x86: ept assertions") changed the return
value in that case to -1, which broke the test.

Returning 0 and -1 is ambiguous, so let's return false instead and get
the PTE through a pointer argument.  This skips a test that was failing
before, because it was looking at invalid type (the meta -1) instead of
the pte.

Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
 I'm not sure if the case was supposed to be tested more, rather than
 called as "ok".
---
 x86/vmx.c       | 33 +++++++++++++++++++--------------
 x86/vmx.h       |  4 ++--
 x86/vmx_tests.c | 20 ++++++++++----------
 3 files changed, 31 insertions(+), 26 deletions(-)

diff --git a/x86/vmx.c b/x86/vmx.c
index 9189a66759ec..5e3832727f05 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -809,29 +809,30 @@ void setup_ept_range(unsigned long *pml4, unsigned long start,
 
 /* get_ept_pte : Get the PTE of a given level in EPT,
     @level == 1 means get the latest level*/
-unsigned long get_ept_pte(unsigned long *pml4,
-		unsigned long guest_addr, int level)
+bool get_ept_pte(unsigned long *pml4, unsigned long guest_addr, int level,
+		unsigned long *pte)
 {
 	int l;
-	unsigned long *pt = pml4, pte;
+	unsigned long *pt = pml4, iter_pte;
 	unsigned offset;
 
 	assert(level >= 1 && level <= 4);
 
 	for (l = EPT_PAGE_LEVEL; ; --l) {
 		offset = (guest_addr >> EPT_LEVEL_SHIFT(l)) & EPT_PGDIR_MASK;
-		pte = pt[offset];
-		if (!(pte & (EPT_PRESENT)))
-			return -1;
+		iter_pte = pt[offset];
+		if (!(iter_pte & (EPT_PRESENT)))
+			return false;
 		if (l == level)
 			break;
-		if (l < 4 && (pte & EPT_LARGE_PAGE))
-			return -1;
-		pt = (unsigned long *)(pte & EPT_ADDR_MASK);
+		if (l < 4 && (iter_pte & EPT_LARGE_PAGE))
+			return false;
+		pt = (unsigned long *)(iter_pte & EPT_ADDR_MASK);
 	}
 	offset = (guest_addr >> EPT_LEVEL_SHIFT(l)) & EPT_PGDIR_MASK;
-	pte = pt[offset];
-	return pte;
+	if (pte)
+		*pte = pt[offset];
+	return true;
 }
 
 static void clear_ept_ad_pte(unsigned long *pml4, unsigned long guest_addr)
@@ -894,9 +895,10 @@ void check_ept_ad(unsigned long *pml4, u64 guest_cr3,
 	for (l = EPT_PAGE_LEVEL; ; --l) {
 		offset = (guest_addr >> EPT_LEVEL_SHIFT(l)) & EPT_PGDIR_MASK;
 
-		ept_pte = get_ept_pte(pml4, (u64) &pt[offset], 1);
-		if (ept_pte == 0)
+		if (!get_ept_pte(pml4, (u64) &pt[offset], 1, &ept_pte)) {
+			printf("EPT - guest level %d page table is not mapped.\n", l);
 			return;
+		}
 
 		if (!bad_pt_ad) {
 			bad_pt_ad |= (ept_pte & (EPT_ACCESS_FLAG|EPT_DIRTY_FLAG)) != expected_pt_ad;
@@ -925,7 +927,10 @@ void check_ept_ad(unsigned long *pml4, u64 guest_cr3,
 	offset_in_page = guest_addr & ((1 << EPT_LEVEL_SHIFT(l)) - 1);
 	gpa = (pt[offset] & PT_ADDR_MASK) | (guest_addr & offset_in_page);
 
-	ept_pte = get_ept_pte(pml4, gpa, 1);
+	if (!get_ept_pte(pml4, gpa, 1, &ept_pte)) {
+		report("EPT - guest physical address is not mapped", false);
+		return;
+	}
 	report("EPT - guest physical address A=%d/D=%d",
 	       (ept_pte & (EPT_ACCESS_FLAG|EPT_DIRTY_FLAG)) == expected_gpa_ad,
 	       !!(expected_gpa_ad & EPT_ACCESS_FLAG),
diff --git a/x86/vmx.h b/x86/vmx.h
index 787466653f8b..efbb320f44c7 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -697,8 +697,8 @@ void install_ept(unsigned long *pml4, unsigned long phys,
 		unsigned long guest_addr, u64 perm);
 void setup_ept_range(unsigned long *pml4, unsigned long start,
 		     unsigned long len, int map_1g, int map_2m, u64 perm);
-unsigned long get_ept_pte(unsigned long *pml4,
-		unsigned long guest_addr, int level);
+bool get_ept_pte(unsigned long *pml4, unsigned long guest_addr, int level,
+		unsigned long *pte);
 void set_ept_pte(unsigned long *pml4, unsigned long guest_addr,
 		int level, u64 pte_val);
 void check_ept_ad(unsigned long *pml4, u64 guest_cr3,
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index 7be016ce4fbc..181c3c73cb60 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -1212,17 +1212,18 @@ static int ept_exit_handler_common(bool have_ad)
 			break;
 		case 3:
 			clear_ept_ad(pml4, guest_cr3, (unsigned long)data_page1);
-			data_page1_pte = get_ept_pte(pml4,
-				(unsigned long)data_page1, 1);
+			TEST_ASSERT(get_ept_pte(pml4, (unsigned long)data_page1,
+						1, &data_page1_pte));
 			set_ept_pte(pml4, (unsigned long)data_page1, 
 				1, data_page1_pte & ~EPT_PRESENT);
 			ept_sync(INVEPT_SINGLE, eptp);
 			break;
 		case 4:
-			data_page1_pte = get_ept_pte(pml4,
-				(unsigned long)data_page1, 2);
+			TEST_ASSERT(get_ept_pte(pml4, (unsigned long)data_page1,
+						2, &data_page1_pte));
 			data_page1_pte &= PAGE_MASK;
-			data_page1_pte_pte = get_ept_pte(pml4, data_page1_pte, 2);
+			TEST_ASSERT(get_ept_pte(pml4, data_page1_pte,
+						2, &data_page1_pte_pte));
 			set_ept_pte(pml4, data_page1_pte, 2,
 				data_page1_pte_pte & ~EPT_PRESENT);
 			ept_sync(INVEPT_SINGLE, eptp);
@@ -1267,7 +1268,7 @@ static int ept_exit_handler_common(bool have_ad)
 			if (exit_qual == (EPT_VLT_WR | EPT_VLT_LADDR_VLD |
 					EPT_VLT_PADDR))
 				vmx_inc_test_stage();
-			set_ept_pte(pml4, (unsigned long)data_page1, 
+			set_ept_pte(pml4, (unsigned long)data_page1,
 				1, data_page1_pte | (EPT_PRESENT));
 			ept_sync(INVEPT_SINGLE, eptp);
 			break;
@@ -2175,8 +2176,7 @@ static unsigned long ept_twiddle(unsigned long gpa, bool mkhuge, int level,
 	unsigned long pte;
 
 	/* Screw with the mapping at the requested level. */
-	orig_pte = get_ept_pte(pml4, gpa, level);
-	TEST_ASSERT(orig_pte != -1);
+	TEST_ASSERT(get_ept_pte(pml4, gpa, level, &orig_pte));
 	pte = orig_pte;
 	if (mkhuge)
 		pte = (orig_pte & ~EPT_ADDR_MASK) | data->hpa | EPT_LARGE_PAGE;
@@ -2603,8 +2603,8 @@ static void ept_access_test_setup(void)
 	 * Make sure nothing's mapped here so the tests that screw with the
 	 * pml4 entry don't inadvertently break something.
 	 */
-	TEST_ASSERT_EQ(get_ept_pte(pml4, data->gpa, 4), -1);
-	TEST_ASSERT_EQ(get_ept_pte(pml4, data->gpa + size - 1, 4), -1);
+	TEST_ASSERT(!get_ept_pte(pml4, data->gpa, 4, NULL));
+	TEST_ASSERT(!get_ept_pte(pml4, data->gpa + size - 1, 4, NULL));
 	install_ept(pml4, data->hpa, data->gpa, EPT_PRESENT);
 
 	data->hva[0] = MAGIC_VAL_1;
-- 
2.13.2