diff mbox

[kvm-unit-tests,25/32] x86: ept assertions

Message ID 20170421005004.137260-26-dmatlack@google.com (mailing list archive)
State New, archived
Headers show

Commit Message

David Matlack April 21, 2017, 12:49 a.m. UTC 
From: Peter Feiner <pfeiner@google.com>

The upper bounds on level in set_ept_pte and install_ept_entry were
just wrong. There's nothing wrong with setting an EPT PTE at the
highest (512GiB) level.

Got rid of unused error return values and replaced them with
assertions. Nobody was checking the return values, so these functions
were silently failing.

Change-Id: I4db1e1c2a0c050f5f69cce28df460b4c8ce10d1c
Signed-off-by: David Matlack <dmatlack@google.com>
---
 x86/vmx.c | 23 ++++++++++++-----------
 x86/vmx.h |  2 +-
 2 files changed, 13 insertions(+), 12 deletions(-)

Comments

Paolo Bonzini April 21, 2017, 9:57 a.m. UTC | #1 
On 21/04/2017 02:49, David Matlack wrote:
> From: Peter Feiner <pfeiner@google.com>
> 
> The upper bounds on level in set_ept_pte and install_ept_entry were
> just wrong. There's nothing wrong with setting an EPT PTE at the
> highest (512GiB) level.

Right, I think the idea was that the biggest supported page size is 1GB,
not 512, but setting up "super-huge" EPT pages can even be desirable for
testing purposes.

Paolo

> Got rid of unused error return values and replaced them with
> assertions. Nobody was checking the return values, so these functions
> were silently failing.
> 
> Change-Id: I4db1e1c2a0c050f5f69cce28df460b4c8ce10d1c
> Signed-off-by: David Matlack <dmatlack@google.com>
diff mbox

Patch

diff --git a/x86/vmx.c b/x86/vmx.c
index cae650ae2e68..c003d5d11e63 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -724,6 +724,9 @@  void install_ept_entry(unsigned long *pml4,
 	unsigned long *pt = pml4;
 	unsigned offset;
 
+	/* EPT only uses 48 bits of GPA. */
+	assert(guest_addr < (1ul << 48));
+
 	for (level = EPT_PAGE_LEVEL; level > pte_level; --level) {
 		offset = (guest_addr >> EPT_LEVEL_SHIFT(level))
 				& EPT_PGDIR_MASK;
@@ -813,17 +816,17 @@  unsigned long get_ept_pte(unsigned long *pml4,
 	unsigned long *pt = pml4, pte;
 	unsigned offset;
 
-	if (level < 1 || level > 3)
-		return -1;
+	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 0;
+			return -1;
 		if (l == level)
 			break;
 		if (l < 4 && (pte & EPT_LARGE_PAGE))
-			return pte;
+			return -1;
 		pt = (unsigned long *)(pte & EPT_ADDR_MASK);
 	}
 	offset = (guest_addr >> EPT_LEVEL_SHIFT(l)) & EPT_PGDIR_MASK;
@@ -950,26 +953,24 @@  void ept_sync(int type, u64 eptp)
 	}
 }
 
-int set_ept_pte(unsigned long *pml4, unsigned long guest_addr,
-		int level, u64 pte_val)
+void set_ept_pte(unsigned long *pml4, unsigned long guest_addr,
+		 int level, u64 pte_val)
 {
 	int l;
 	unsigned long *pt = pml4;
 	unsigned offset;
 
-	if (level < 1 || level > 3)
-		return -1;
+	assert(level >= 1 && level <= 4);
+
 	for (l = EPT_PAGE_LEVEL; ; --l) {
 		offset = (guest_addr >> EPT_LEVEL_SHIFT(l)) & EPT_PGDIR_MASK;
 		if (l == level)
 			break;
-		if (!(pt[offset] & (EPT_PRESENT)))
-			return -1;
+		assert(pt[offset] & EPT_PRESENT);
 		pt = (unsigned long *)(pt[offset] & EPT_ADDR_MASK);
 	}
 	offset = (guest_addr >> EPT_LEVEL_SHIFT(l)) & EPT_PGDIR_MASK;
 	pt[offset] = pte_val;
-	return 0;
 }
 
 void vpid_sync(int type, u16 vpid)
diff --git a/x86/vmx.h b/x86/vmx.h
index 966fff653561..4b899aa69145 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -691,7 +691,7 @@  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);
-int set_ept_pte(unsigned long *pml4, unsigned long guest_addr,
+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,
 		  unsigned long guest_addr, int expected_gpa_ad,