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@xxxxxxxxxx>
---
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
