On Thu, Jun 29, 2017 at 10:26 AM, Radim Krčmář <rkrcmar@xxxxxxxxxx> wrote:
> vmx_EPT_AD_* tests mark the last level as non-present, but that doesn't
> mean we cannot look at A/D bits of that last level.
> This fixes "EPT - guest physical address is not mapped" in case 3.
>
> Signed-off-by: Radim Krčmář <rkrcmar@xxxxxxxxxx>
> ---
> x86/vmx.c | 4 ++--
> x86/vmx_tests.c | 5 +++--
> 2 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/x86/vmx.c b/x86/vmx.c
> index 5e3832727f05..56c2c079ebc5 100644
> --- a/x86/vmx.c
> +++ b/x86/vmx.c
> @@ -821,12 +821,12 @@ bool get_ept_pte(unsigned long *pml4, unsigned long guest_addr, int level,
> for (l = EPT_PAGE_LEVEL; ; --l) {
> offset = (guest_addr >> EPT_LEVEL_SHIFT(l)) & EPT_PGDIR_MASK;
> iter_pte = pt[offset];
> - if (!(iter_pte & (EPT_PRESENT)))
> - return false;
> if (l == level)
> break;
> if (l < 4 && (iter_pte & EPT_LARGE_PAGE))
> return false;
> + if (!(iter_pte & (EPT_PRESENT)))
> + return false;
> pt = (unsigned long *)(iter_pte & EPT_ADDR_MASK);
> }
> offset = (guest_addr >> EPT_LEVEL_SHIFT(l)) & EPT_PGDIR_MASK;
> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
> index 181c3c73cb60..567f7143b427 100644
> --- a/x86/vmx_tests.c
> +++ b/x86/vmx_tests.c
> @@ -2582,6 +2582,7 @@ static void ept_access_test_setup(void)
> unsigned long npages = 1ul << PAGE_1G_ORDER;
> unsigned long size = npages * PAGE_SIZE;
> unsigned long *page_table = current_page_table();
> + unsigned long pte;
>
> if (setup_ept(false))
> test_skip("EPT not supported");
> @@ -2603,8 +2604,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(!get_ept_pte(pml4, data->gpa, 4, NULL));
> - TEST_ASSERT(!get_ept_pte(pml4, data->gpa + size - 1, 4, NULL));
> + TEST_ASSERT(get_ept_pte(pml4, data->gpa, 4, &pte) && pte == 0);
> + TEST_ASSERT(get_ept_pte(pml4, data->gpa + size - 1, 4, &pte) && pte == 0);
This isn't right. The PML4 for 1 TiB shouldn't be present ("Make sure
nothing's mapped
here so the tests that screw with the pml4 entry don't inadvertently
break something."),
so the walk definitely shouldn't get to the leaf entry. I'd actually expect
get_ept_pte(pml4, data->gpa, 2, &pte) to return false.
> install_ept(pml4, data->hpa, data->gpa, EPT_PRESENT);
>
> data->hva[0] = MAGIC_VAL_1;
> --
> 2.13.2
>
