[PATCH v3 0/2] OvmfPkg/ResetVector: replace open-coded constants with fixed PCDs

[PATCH v3 0/2] OvmfPkg/ResetVector: replace open-coded constants with fixed PCDs

[Laszlo Ersek]

Thanks to Jordan's old series

  abb158ded41f BaseTools: Add rules to build NASM source file into a
               binary
  5a1f324d946c UefiCpuPkg: Support building VTF0 ResetVector during the
               EDK II build
  eee1d2ca9078 UefiCpuPkg VTF0 X64: Build page tables in NASM code
  9b9fdbfa7059 OvmfPkg: Support building OVMF's ResetVector during the
               EDK II build
  497cbb530a58 OvmfPkg: Build OVMF ResetVector during EDK II build
               process
  70e46f44cd13 OvmfPkg/ResetVector: Remove pre-built binaries
  3449f56dac9c UefiCpuPkg: Add ResetVector/FixupVtf

we can now use fixed PCDs in NASMB source code.

Patch #1 removes the unused ResetVectorCode.asm file, suggested by
Marvin.

Patch #2 is Marvin's v2 patch, simplified.

Repo:   https://github.com/lersek/edk2.git
Branch: reset_vector_pcd_marv_v3

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Marvin Häuser <Marvin.Haeuser@outlook.com>

Thanks
Laszlo

Laszlo Ersek (1):
  OvmfPkg/ResetVector: remove unused file "ResetVectorCode.asm"

Marvin Häuser (1):
  OvmfPkg/ResetVector: Depend on PCD values of the page tables.

 OvmfPkg/ResetVector/ResetVector.inf       |  5 ++
 OvmfPkg/ResetVector/Ia32/PageTables64.asm | 22 ++++----
 OvmfPkg/ResetVector/ResetVector.nasmb     | 12 ++++-
 OvmfPkg/ResetVector/ResetVectorCode.asm   | 53 --------------------
 4 files changed, 25 insertions(+), 67 deletions(-)
 delete mode 100644 OvmfPkg/ResetVector/ResetVectorCode.asm

-- 
2.9.2

[PATCH v3 1/2] OvmfPkg/ResetVector: remove unused file "ResetVectorCode.asm"

[Laszlo Ersek]

In commit 70e46f44cd13 ("OvmfPkg/ResetVector: Remove pre-built binaries")
we removed the last (or only) reference to "ResetVectorCode.asm"; namely,
the "OvmfPkg/ResetVector/Build.py" script. We should have removed
"ResetVectorCode.asm" too -- do it now.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Marvin Häuser <Marvin.Haeuser@outlook.com>
Fixes: 70e46f44cd13337106036b67ac9f79b9e4237e79
Suggested-by: Marvin Häuser <Marvin.Haeuser@outlook.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 OvmfPkg/ResetVector/ResetVectorCode.asm | 53 --------------------
 1 file changed, 53 deletions(-)

diff --git a/OvmfPkg/ResetVector/ResetVectorCode.asm b/OvmfPkg/ResetVector/ResetVectorCode.asm
deleted file mode 100644
index 052c821f212c..000000000000
--- a/OvmfPkg/ResetVector/ResetVectorCode.asm
+++ /dev/null
@@ -1,53 +0,0 @@
-;------------------------------------------------------------------------------
-; @file
-; This file includes all other code files to assemble the reset vector code
-;
-; Copyright (c) 2008 - 2013, Intel Corporation. All rights reserved.<BR>
-; This program and the accompanying materials
-; are licensed and made available under the terms and conditions of the BSD License
-; which accompanies this distribution.  The full text of the license may be found at
-; http://opensource.org/licenses/bsd-license.php
-;
-; THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
-; WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
-;
-;------------------------------------------------------------------------------
-
-%ifdef ARCH_IA32
-  %ifdef ARCH_X64
-    %error "Only one of ARCH_IA32 or ARCH_X64 can be defined."
-  %endif
-%elifdef ARCH_X64
-%else
-  %error "Either ARCH_IA32 or ARCH_X64 must be defined."
-%endif
-
-%include "CommonMacros.inc"
-
-%include "PostCodes.inc"
-
-%ifdef DEBUG_NONE
-  %include "DebugDisabled.asm"
-%elifdef DEBUG_PORT80
-  %include "Port80Debug.asm"
-%elifdef DEBUG_SERIAL
-  %include "SerialDebug.asm"
-%else
-  %error "No debug type was specified."
-%endif
-
-%include "Ia32/SearchForBfvBase.asm"
-%include "Ia32/SearchForSecEntry.asm"
-
-%ifdef ARCH_X64
-%include "Ia32/Flat32ToFlat64.asm"
-%include "Ia32/PageTables64.asm"
-%endif
-
-%include "Ia16/Real16ToFlat32.asm"
-%include "Ia16/Init16.asm"
-
-%include "Main.asm"
-
-%include "Ia16/ResetVectorVtf0.asm"
-
-- 
2.9.2

[PATCH v3 2/2] OvmfPkg/ResetVector: Depend on PCD values of the page tables.

[Laszlo Ersek]

From: Marvin Häuser <Marvin.Haeuser@outlook.com>

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.

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 <Marvin.Haeuser@outlook.com>
[lersek@redhat.com: simplify patch]
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Marvin Häuser <Marvin.Haeuser@outlook.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 OvmfPkg/ResetVector/ResetVector.inf       |  5 +++++
 OvmfPkg/ResetVector/Ia32/PageTables64.asm | 22 +++++++++-----------
 OvmfPkg/ResetVector/ResetVector.nasmb     | 12 +++++++++--
 3 files changed, 25 insertions(+), 14 deletions(-)

diff --git a/OvmfPkg/ResetVector/ResetVector.inf b/OvmfPkg/ResetVector/ResetVector.inf
index 46610d243ecf..82df438db0dc 100644
--- a/OvmfPkg/ResetVector/ResetVector.inf
+++ b/OvmfPkg/ResetVector/ResetVector.inf
@@ -30,8 +30,13 @@ [Sources]
 
 [Packages]
   MdePkg/MdePkg.dec
+  OvmfPkg/OvmfPkg.dec
   UefiCpuPkg/UefiCpuPkg.dec
 
 [BuildOptions]
    *_*_IA32_NASMB_FLAGS = -I$(WORKSPACE)/UefiCpuPkg/ResetVector/Vtf0/
    *_*_X64_NASMB_FLAGS = -I$(WORKSPACE)/UefiCpuPkg/ResetVector/Vtf0/
+
+[FixedPcd]
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesBase ## CONSUMES
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesSize ## CONSUMES
diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
index b5a4cf8d7187..25c9ebd5f73e 100644
--- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
+++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
@@ -44,10 +44,8 @@ BITS    32
 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.
+    ; For OVMF, build some initial page tables at
+    ; PT_ADDR (0) .. PT_ADDR (0x6000).
     ;
     ; At the end of PEI, the pages tables will be rebuilt into a
     ; more permanent location by DxeIpl.
@@ -56,21 +54,21 @@ SetCr3ForPageTables64:
     mov     ecx, 6 * 0x1000 / 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 => 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 => 4GB)
@@ -81,13 +79,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
     loop    pageTableEntriesLoop
 
     ;
     ; Set CR3 now that the paging structures are available
     ;
-    mov     eax, 0x800000
+    mov     eax, PT_ADDR (0)
     mov     cr3, eax
 
     OneTimeCallRet SetCr3ForPageTables64
diff --git a/OvmfPkg/ResetVector/ResetVector.nasmb b/OvmfPkg/ResetVector/ResetVector.nasmb
index 31ac06ae4a8c..65f39a7d2d52 100644
--- a/OvmfPkg/ResetVector/ResetVector.nasmb
+++ b/OvmfPkg/ResetVector/ResetVector.nasmb
@@ -53,8 +53,16 @@
 %include "Ia32/SearchForSecEntry.asm"
 
 %ifdef ARCH_X64
-%include "Ia32/Flat32ToFlat64.asm"
-%include "Ia32/PageTables64.asm"
+  #include <AutoGen.h>
+
+  %if (FixedPcdGet32 (PcdOvmfSecPageTablesSize) != 0x6000)
+    %error "This implementation inherently depends on PcdOvmfSecPageTablesSize"
+  %endif
+
+  %define PT_ADDR(Offset) (FixedPcdGet32 (PcdOvmfSecPageTablesBase) + (Offset))
+
+  %include "Ia32/Flat32ToFlat64.asm"
+  %include "Ia32/PageTables64.asm"
 %endif
 
 %include "Ia16/Real16ToFlat32.asm"
-- 
2.9.2