From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 8C02981D40 for ; Thu, 3 Nov 2016 13:59:20 -0700 (PDT) Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by orsmga104.jf.intel.com with ESMTP; 03 Nov 2016 13:59:22 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,440,1473145200"; d="scan'208";a="27204617" Received: from poojakri-mobl.amr.corp.intel.com (HELO localhost) ([10.252.132.157]) by fmsmga005.fm.intel.com with ESMTP; 03 Nov 2016 13:59:22 -0700 MIME-Version: 1.0 To: =?utf-8?q?Marvin_H=C3=A4user?= , "edk2-devel@lists.01.org" Message-ID: <147820676171.25634.8108696649790140093@jljusten-ivb> From: Jordan Justen In-Reply-To: Cc: "lersek@redhat.com" References: User-Agent: alot/0.3.7 Date: Thu, 03 Nov 2016 13:59:21 -0700 Subject: Re: [PATCH v2] OvmfPkg/ResetVector: Depend on PCD values of the page tables. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 03 Nov 2016 20:59:20 -0000 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable On 2016-11-03 12:18:56, Marvin H=C3=A4user wrote: > Currently, the values of the page tables' address and size are > hard-coded in the ResetVector. This patch replaces this with a PCD > dependency for the NASM Reset Vector. The ASM Reset Vector remains > using a hard-coded value due to the lack of C preprocessing. > = > Checks for the size have been added to alert the developer to rewrite > the ASM according to the new size, if it has been changed. > = > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Marvin Haeuser > --- > OvmfPkg/ResetVector/Ia32/PageTables64.asm | 23 ++++++++++---------- > OvmfPkg/ResetVector/ResetVector.inf | 5 +++++ > OvmfPkg/ResetVector/ResetVector.nasmb | 7 ++++++ > OvmfPkg/ResetVector/ResetVectorCode.asm | 16 ++++++++++++++ > 4 files changed, 39 insertions(+), 12 deletions(-) > = > diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm b/OvmfPkg/ResetVec= tor/Ia32/PageTables64.asm > index b5a4cf8d7187..7ff394b1afc3 100644 > --- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm > +++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm > @@ -37,6 +37,8 @@ BITS 32 > PAGE_READ_WRITE + \ > PAGE_PRESENT) > = > +%define PT_ADDR(Offset) (OVMF_SEC_PAGE_TABLES_BASE + (Offset)) > + > = > ; > ; Modified: EAX, ECX > @@ -46,31 +48,28 @@ SetCr3ForPageTables64: > ; > ; For OVMF, build some initial page tables at 0x800000-0x806000. > ; > - ; This range should match with PcdOvmfSecPageTablesBase and > - ; PcdOvmfSecPageTablesSize which are declared in the FDF files. > - ; > ; At the end of PEI, the pages tables will be rebuilt into a > ; more permanent location by DxeIpl. > ; > = > - mov ecx, 6 * 0x1000 / 4 > + mov ecx, OVMF_SEC_PAGE_TABLES_SIZE / 4 > xor eax, eax > clearPageTablesMemoryLoop: > - mov dword[ecx * 4 + 0x800000 - 4], eax > + mov dword[ecx * 4 + PT_ADDR (0) - 4], eax > loop clearPageTablesMemoryLoop > = > ; > ; Top level Page Directory Pointers (1 * 512GB entry) > ; > - mov dword[0x800000], 0x801000 + PAGE_PDP_ATTR > + mov dword[PT_ADDR (0)], PT_ADDR (0x1000) + PAGE_PDP_ATTR > = > ; > ; Next level Page Directory Pointers (4 * 1GB entries =3D> 4GB) > ; > - mov dword[0x801000], 0x802000 + PAGE_PDP_ATTR > - mov dword[0x801008], 0x803000 + PAGE_PDP_ATTR > - mov dword[0x801010], 0x804000 + PAGE_PDP_ATTR > - mov dword[0x801018], 0x805000 + PAGE_PDP_ATTR > + mov dword[PT_ADDR (0x1000)], PT_ADDR (0x2000) + PAGE_PDP_ATTR > + mov dword[PT_ADDR (0x1008)], PT_ADDR (0x3000) + PAGE_PDP_ATTR > + mov dword[PT_ADDR (0x1010)], PT_ADDR (0x4000) + PAGE_PDP_ATTR > + mov dword[PT_ADDR (0x1018)], PT_ADDR (0x5000) + PAGE_PDP_ATTR > = > ; > ; Page Table Entries (2048 * 2MB entries =3D> 4GB) > @@ -81,13 +80,13 @@ pageTableEntriesLoop: > dec eax > shl eax, 21 > add eax, PAGE_2M_PDE_ATTR > - mov [ecx * 8 + 0x802000 - 8], eax > + mov [ecx * 8 + PT_ADDR (0x2000) - 8], eax I think the '- 8' should be in the PT_ADDR. This patch is basically Reviewed-by: Jordan Justen except, like Laszlo pointed out, I think that ResetVectorCode.asm should have been deleted in 70e46f44cd13. So, before this patch, I think we should add a patch to delete it. Then you won't need to modify it, and only the PCD's will be used. Would you like to make these changes, or would you like me to? Thanks, -Jordan > loop pageTableEntriesLoop > = > ; > ; Set CR3 now that the paging structures are available > ; > - mov eax, 0x800000 > + mov eax, OVMF_SEC_PAGE_TABLES_BASE > mov cr3, eax > = > OneTimeCallRet SetCr3ForPageTables64 > diff --git a/OvmfPkg/ResetVector/ResetVector.inf b/OvmfPkg/ResetVector/Re= setVector.inf > index 46610d243ecf..d1e5d4d9bdea 100644 > --- a/OvmfPkg/ResetVector/ResetVector.inf > +++ b/OvmfPkg/ResetVector/ResetVector.inf > @@ -29,9 +29,14 @@ [Sources] > ResetVector.nasmb > = > [Packages] > + OvmfPkg/OvmfPkg.dec > MdePkg/MdePkg.dec > UefiCpuPkg/UefiCpuPkg.dec > = > [BuildOptions] > *_*_IA32_NASMB_FLAGS =3D -I$(WORKSPACE)/UefiCpuPkg/ResetVector/Vtf0/ > *_*_X64_NASMB_FLAGS =3D -I$(WORKSPACE)/UefiCpuPkg/ResetVector/Vtf0/ > + > +[Pcd] > + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesBase > + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesSize > diff --git a/OvmfPkg/ResetVector/ResetVector.nasmb b/OvmfPkg/ResetVector/= ResetVector.nasmb > index 31ac06ae4a8c..62597ea95cf5 100644 > --- a/OvmfPkg/ResetVector/ResetVector.nasmb > +++ b/OvmfPkg/ResetVector/ResetVector.nasmb > @@ -53,6 +53,13 @@ > %include "Ia32/SearchForSecEntry.asm" > = > %ifdef ARCH_X64 > + #include > + %define OVMF_SEC_PAGE_TABLES_BASE FixedPcdGet32 (PcdOvmfSecPageTablesB= ase) > + %define OVMF_SEC_PAGE_TABLES_SIZE FixedPcdGet32 (PcdOvmfSecPageTablesS= ize) > + > + %if (FixedPcdGet32 (PcdOvmfSecPageTablesSize) !=3D 0x006000) > + %error "This implementation inherently depends on PcdOvmfSecPageTabl= esSize" > + %endif > %include "Ia32/Flat32ToFlat64.asm" > %include "Ia32/PageTables64.asm" > %endif > diff --git a/OvmfPkg/ResetVector/ResetVectorCode.asm b/OvmfPkg/ResetVecto= r/ResetVectorCode.asm > index 052c821f212c..1a1d92fb1415 100644 > --- a/OvmfPkg/ResetVector/ResetVectorCode.asm > +++ b/OvmfPkg/ResetVector/ResetVectorCode.asm > @@ -40,6 +40,22 @@ > %include "Ia32/SearchForSecEntry.asm" > = > %ifdef ARCH_X64 > + ; > + ; These values should match with PcdOvmfSecPageTablesBase and > + ; PcdOvmfSecPageTablesSize which are declared in the FDF files. > + ; > + > + %ifndef OVMF_SEC_PAGE_TABLES_BASE > + %define OVMF_SEC_PAGE_TABLES_BASE 0x800000 > + %endif > + > + %ifndef OVMF_SEC_PAGE_TABLES_SIZE > + %define OVMF_SEC_PAGE_TABLES_SIZE 6 * 0x1000 > + %endif > + > + %if (OVMF_SEC_PAGE_TABLES_SIZE !=3D 0x006000) > + %error "This implementation inherently depends on OVMF_SEC_PAGE_TABL= ES_SIZE" > + %endif > %include "Ia32/Flat32ToFlat64.asm" > %include "Ia32/PageTables64.asm" > %endif > -- = > 2.10.1.windows.1 >=20