From mboxrd@z Thu Jan 1 00:00:00 1970 From: Peter Feiner Subject: Re: [kvm-unit-tests PATCH 3/3] x86/vmx: get EPT at the last level Date: Thu, 29 Jun 2017 10:51:58 -0700 Message-ID: References: <20170629172647.22188-1-rkrcmar@redhat.com> <20170629172647.22188-4-rkrcmar@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Cc: kvm@vger.kernel.org, Paolo Bonzini , David Matlack To: =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= Return-path: Received: from mail-yw0-f178.google.com ([209.85.161.178]:36607 "EHLO mail-yw0-f178.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752609AbdF2Rv7 (ORCPT ); Thu, 29 Jun 2017 13:51:59 -0400 Received: by mail-yw0-f178.google.com with SMTP id t127so40133272ywc.3 for ; Thu, 29 Jun 2017 10:51:59 -0700 (PDT) In-Reply-To: <20170629172647.22188-4-rkrcmar@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On Thu, Jun 29, 2017 at 10:26 AM, Radim Kr=C4=8Dm=C3=A1=C5=99 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=C4=8Dm=C3=A1=C5=99 > --- > 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 =3D EPT_PAGE_LEVEL; ; --l) { > offset =3D (guest_addr >> EPT_LEVEL_SHIFT(l)) & EPT_PGDIR= _MASK; > iter_pte =3D pt[offset]; > - if (!(iter_pte & (EPT_PRESENT))) > - return false; > if (l =3D=3D level) > break; > if (l < 4 && (iter_pte & EPT_LARGE_PAGE)) > return false; > + if (!(iter_pte & (EPT_PRESENT))) > + return false; > pt =3D (unsigned long *)(iter_pte & EPT_ADDR_MASK); > } > offset =3D (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 =3D 1ul << PAGE_1G_ORDER; > unsigned long size =3D npages * PAGE_SIZE; > unsigned long *page_table =3D 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 t= he > * 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 =3D=3D 0= ); > + TEST_ASSERT(get_ept_pte(pml4, data->gpa + size - 1, 4, &pte) && p= te =3D=3D 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 expec= t get_ept_pte(pml4, data->gpa, 2, &pte) to return false. > install_ept(pml4, data->hpa, data->gpa, EPT_PRESENT); > > data->hva[0] =3D MAGIC_VAL_1; > -- > 2.13.2 >