[PATCH v3 00/16] [Arm|ArmVirt|MdePkg|Embedded]Pkg: lift 40-bit IPA space limit

[PATCH v3 00/16] [Arm|ArmVirt|MdePkg|Embedded]Pkg: lift 40-bit IPA space limit

[Ard Biesheuvel]

This v3 subsumes and/or supersedes

[PATCH v2 00/13] ArmPkg, ArmVirtPkg: lift 40-bit IPA space limit
[PATCH] MdePkg/ProcessorBind.h AARCH64: limit MAX_ADDRESS to 48 bits
[PATCH v2 0/2] ArmVirtPkg: remove high peripheral space mapping

The ArmVirtQemu targets currently limit the size of the IPA space to
40 bits because that is all what KVM supports. However, this is about
to change, and so we need to update the code if we want to ensure that
our UEFI firmware builds can keep running on systems that set values
other than 40 (which could be > 40 or < 40)

This series refactors how we handle the maximum size of the physical
address space supported by the CPU in relation with the size of UEFI's
1:1 mapping and the size of the GCD memory space map, taking the following
observations into account:
- the range of the linear mapping can be tied to whatever the CPU supports
  (as long as it doesn't exceed what the architecture permits for 4k pages)
  since we mostly already use the maximum of 4 levels anyway, and there is
  no memory cost involved beyond that
- there is usually no point in mapping the entire address space, which does
  involve a memory cost
- the GCD memory space may be required to cover more than what UEFI can
  address itself, since it is the based for the UEFI memory map that is
  provided to the OS

Patches #1 and #2 remove some unused code to avoid having to fix it.

Patches #3 and #4 update ArmVirtQemu and ArmVirtQemuKernel to drop the high
peripheral space mapping, and map whatever may reside there explicitly
(currently only the ECAM space in practice, but the MMIO view of the PCI
I/O space is mapped explicitly as well)

Patch #5 was sent out before individually, and sets MAX_ADDRESS to the
maximum value AArch64 can map in UEFI which runs with 4k pages.

Patch #6 adds a helper to ArmLib to read the number of supported address
bits and take this into account in the page table code (#8), which allows
PcdPrePiCpuMemorySize to assume a value that exceeds the capabilities of
the CPU.

Patch #7 is mostly a cleanup patch, to switch to the new helper added in
patch #6. No functional changes intended.

Patches #9 to #12 modify building of the CPU hob (and thus the size of the
GCD memory space) based on the CPU capabilities rather than the value of 
PcdPrePiCpuMemorySize, which is dropped in the last patch.

Pacthes #13 and #14 remove some needless references to PcdPrePiCpuMemorySize

Patch #15 drops the overrides of PcdPrePiCpuMemorySize from all ArmVirtPkg
platforms.

Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Eric Auger <eric.auger@redhat.com>
Cc: Andrew Jones <drjones@redhat.com>
Cc: Philippe Mathieu-Daude <philmd@redhat.com>
Cc: Julien Grall <julien.grall@linaro.org>

Ard Biesheuvel (16):
  EmbeddedPkg/TemplateSec: remove unused module
  EmbeddedPkg/PrePiHobLib: drop CreateHobList() from library
  ArmVirtPkg/FdtPciHostBridgeLib: map ECAM and I/O spaces in GCD memory
    map
  ArmVirtPkg/QemuVirtMemInfoLib: remove 1:1 mapping of top of PA range
  MdePkg/ProcessorBind.h AARCH64: limit MAX_ADDRESS to 48 bits
  ArmPkg/ArmLib: add support for reading the max physical address space
    size
  ArmVirtPkg/XenVirtMemInfoLib: refactor reading of the PA space size
  ArmPkg/ArmMmuLib: take the CPU supported maximum PA space into account
  ArmPkg/CpuPei: base GCD memory space size on CPU's PA range
  ArmPlatformPkg/PrePi: base GCD memory space size on CPU's PA range
  ArmVirtPkg/PrePi: base GCD memory space size on CPU's PA range
  BeagleBoardPkg/PrePi: base GCD memory space size on CPU's PA range
  ArmPlatformPkg/PlatformPei: drop unused PCD references
  EmbeddedPkg/PrePiLib: drop unused PCD reference
  ArmVirtPkg: drop PcdPrePiCpuMemorySize assignments from all platforms
  EmbeddedPkg/EmbeddedPkg.dec: drop PcdPrePiCpuMemorySize declarations

 EmbeddedPkg/EmbeddedPkg.dec                   |  4 -
 ArmVirtPkg/ArmVirt.dsc.inc                    |  3 -
 ArmVirtPkg/ArmVirtQemu.dsc                    |  4 -
 ArmVirtPkg/ArmVirtQemuKernel.dsc              |  4 -
 ArmPkg/Drivers/CpuPei/CpuPei.inf              |  1 -
 ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf    |  3 -
 ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf     |  3 -
 ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf |  3 -
 ArmPlatformPkg/PlatformPei/PlatformPeim.inf   |  3 -
 ArmPlatformPkg/PrePi/PeiMPCore.inf            |  1 -
 ArmPlatformPkg/PrePi/PeiUniCore.inf           |  1 -
 .../FdtPciHostBridgeLib.inf                   |  1 +
 .../QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf |  7 --
 .../QemuVirtMemInfoPeiLib.inf                 |  7 --
 .../XenVirtMemInfoLib/XenVirtMemInfoLib.inf   |  6 --
 .../PrePi/ArmVirtPrePiUniCoreRelocatable.inf  |  1 -
 BeagleBoardPkg/PrePi/PeiUniCore.inf           |  1 -
 EmbeddedPkg/Library/PrePiLib/PrePiLib.inf     |  1 -
 EmbeddedPkg/TemplateSec/TemplateSec.inf       | 65 ----------------
 ArmPkg/Include/Library/ArmLib.h               |  6 ++
 EmbeddedPkg/Include/Library/PrePiLib.h        | 18 -----
 MdePkg/Include/AArch64/ProcessorBind.h        |  4 +-
 ArmPkg/Drivers/CpuPei/CpuPei.c                |  2 +-
 .../Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 11 ++-
 ArmPlatformPkg/PrePi/PrePi.c                  |  2 +-
 .../FdtPciHostBridgeLib/FdtPciHostBridgeLib.c | 46 ++++++++++-
 .../QemuVirtMemInfoLib/QemuVirtMemInfoLib.c   | 25 ++----
 .../XenVirtMemInfoLib/XenVirtMemInfoLib.c     | 11 ++-
 ArmVirtPkg/PrePi/PrePi.c                      |  2 +-
 BeagleBoardPkg/PrePi/PrePi.c                  |  2 +-
 EmbeddedPkg/Library/PrePiHobLib/Hob.c         | 41 ----------
 EmbeddedPkg/TemplateSec/TemplateSec.c         | 76 -------------------
 ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S | 17 +++++
 ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S     |  8 ++
 ArmPkg/Library/ArmLib/Arm/ArmLibSupport.asm   |  8 ++
 .../QemuVirtMemInfoLib/AArch64/PhysAddrTop.S  | 39 ----------
 .../QemuVirtMemInfoLib/Arm/PhysAddrTop.S      | 24 ------
 .../XenVirtMemInfoLib/AArch64/PhysAddrTop.S   | 39 ----------
 .../XenVirtMemInfoLib/Arm/PhysAddrTop.S       | 24 ------
 39 files changed, 110 insertions(+), 414 deletions(-)
 delete mode 100644 EmbeddedPkg/TemplateSec/TemplateSec.inf
 delete mode 100644 EmbeddedPkg/TemplateSec/TemplateSec.c
 delete mode 100644 ArmVirtPkg/Library/QemuVirtMemInfoLib/AArch64/PhysAddrTop.S
 delete mode 100644 ArmVirtPkg/Library/QemuVirtMemInfoLib/Arm/PhysAddrTop.S
 delete mode 100644 ArmVirtPkg/Library/XenVirtMemInfoLib/AArch64/PhysAddrTop.S
 delete mode 100644 ArmVirtPkg/Library/XenVirtMemInfoLib/Arm/PhysAddrTop.S

-- 
2.19.1

[PATCH v3 01/16] EmbeddedPkg/TemplateSec: remove unused module

[Ard Biesheuvel]

Remove this module: it is unused, and should not be used as an
example going forward.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 EmbeddedPkg/TemplateSec/TemplateSec.inf | 65 -----------------
 EmbeddedPkg/TemplateSec/TemplateSec.c   | 76 --------------------
 2 files changed, 141 deletions(-)

diff --git a/EmbeddedPkg/TemplateSec/TemplateSec.inf b/EmbeddedPkg/TemplateSec/TemplateSec.inf
deleted file mode 100644
index 3a63e59294d3..000000000000
--- a/EmbeddedPkg/TemplateSec/TemplateSec.inf
+++ /dev/null
@@ -1,65 +0,0 @@
-#/** @file
-#
-#    Component description file for DxeIpl module
-#
-#   The responsibility of this module is to load the DXE Core from a Firmware Volume. This implementation i used to load a 32-bit DXE Core.
-#
-#  Copyright (c) 2006 - 2008, 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.
-#
-#**/
-
-[Defines]
-  INF_VERSION                    = 0x00010005
-  BASE_NAME                      = TemplateSec
-  FILE_GUID                      = 1D6F730F-5A55-4078-869B-E0A18324BDC8
-  MODULE_TYPE                    = SEC
-  VERSION_STRING                 = 1.0
-
-
-#
-# The following information is for reference only and not required by the build tools.
-#
-#  VALID_ARCHITECTURES           = IA32 X64 ARM
-#
-
-[Sources.common]
-  TemplateSec.c
-
-[Sources.Ia32]
-#  Ia32/ResetVector.asm | MSFT
-#  Ia32/ResetVector.S   | GCC
-
-[Sources.X64]
-#  X64/ResetVector.asm | MSFT
-#  X64/ResetVector.S   | GCC
-
-[Sources.ARM]
-#  Arm/ResetVector.asm | RVCT
-#  Arm/ResetVector.S   | GCC
-
-[Packages]
-  MdePkg/MdePkg.dec
-  MdeModulePkg/MdeModulePkg.dec
-  EmbeddedPkg/EmbeddedPkg.dec
-
-
-[LibraryClasses]
-  BaseLib
-  DebugLib
-  BaseMemoryLib
-  UefiDecompressLib
-  PeCoffLib
-  CacheMaintenanceLib
-  PrePiLib
-
-[Pcd]
-  gEmbeddedTokenSpaceGuid.PcdEmbeddedFdBaseAddress
-  gEmbeddedTokenSpaceGuid.PcdEmbeddedFdSize
-
diff --git a/EmbeddedPkg/TemplateSec/TemplateSec.c b/EmbeddedPkg/TemplateSec/TemplateSec.c
deleted file mode 100644
index c63adbb6f90f..000000000000
--- a/EmbeddedPkg/TemplateSec/TemplateSec.c
+++ /dev/null
@@ -1,76 +0,0 @@
-/** @file
-
-  Copyright (c) 2008 - 2009, Apple Inc. 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.
-
-**/
-
-#include <PiPei.h>
-
-#include <Library/DebugLib.h>
-#include <Library/PrePiLib.h>
-#include <Library/PcdLib.h>
-
-#include <Ppi/GuidedSectionExtraction.h>
-
-VOID
-_ModuleEntryPoint (
-  VOID
-  )
-{
-}
-
-VOID
-CEntryPoint (
-  VOID    *MemoryBase,
-  UINTN   MemorySize,
-  VOID    *StackBase,
-  UINTN   StackSize
-  )
-{
-  EFI_PHYSICAL_ADDRESS  MemoryBegin;
-  UINT64                MemoryLength;
-  VOID                  *HobBase;
-
-  //
-  // Boot strap the C environment so the other library services will work properly.
-  //
-  MemoryBegin  = (EFI_PHYSICAL_ADDRESS)(UINTN)MemoryBase;
-  MemoryLength = (UINT64)MemorySize;
-  HobBase      = (VOID *)(UINTN)(FixedPcdGet32(PcdEmbeddedFdBaseAddress) + FixedPcdGet32(PcdEmbeddedFdSize));
-  CreateHobList (MemoryBase, MemorySize, HobBase, StackBase);
-
-  MemoryBegin  = (EFI_PHYSICAL_ADDRESS)(UINTN)StackBase;
-  MemoryLength = (UINT64)StackSize;
-  UpdateStackHob (MemoryBegin, MemoryLength);
-
-  DEBUG ((DEBUG_ERROR, "CEntryPoint (%x,%x,%x,%x)\n", MemoryBase, MemorySize, StackBase, StackSize));
-
-  //
-  // Add your C code stuff here....
-  //
-
-
-  //
-  // Load the DXE Core and transfer control to it
-  //
-
-  // Give the DXE Core access to our DEBUG and ASSERT infrastructure so this will work prior
-  // to the DXE version being loaded. Thus we close the debugging gap between phases.
-  AddDxeCoreReportStatusCodeCallback ();
-
-  //BuildFvHobs (PcdBfvBase, PcdBfvSize, NULL);
-
-  LoadDxeCoreFromFv (NULL, 0);
-
-  // DXE Core should always load and never return
-  ASSERT (FALSE);
-}
-
-- 
2.19.1

[PATCH v3 02/16] EmbeddedPkg/PrePiHobLib: drop CreateHobList() from library

[Ard Biesheuvel]

Drop the declaration and the implementation of CreateHoblist(),
which is not used anywhere.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 EmbeddedPkg/Include/Library/PrePiLib.h | 18 ---------
 EmbeddedPkg/Library/PrePiHobLib/Hob.c  | 41 --------------------
 2 files changed, 59 deletions(-)

diff --git a/EmbeddedPkg/Include/Library/PrePiLib.h b/EmbeddedPkg/Include/Library/PrePiLib.h
index cf70fca3b619..a857308ecec2 100644
--- a/EmbeddedPkg/Include/Library/PrePiLib.h
+++ b/EmbeddedPkg/Include/Library/PrePiLib.h
@@ -274,24 +274,6 @@ HobConstructor (
   IN VOID   *EfiFreeMemoryTop
   );
 
-/**
-  Retrieves the magic value from the PE/COFF header.
-
-  @param  Hdr             The buffer in which to return the PE32, PE32+, or TE header.
-
-  @return EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC - Image is PE32
-  @return EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC - Image is PE32+
-
-**/
-VOID
-CreateHobList (
-  IN VOID   *MemoryBegin,
-  IN UINTN  MemoryLength,
-  IN VOID   *HobBase,
-  IN VOID   *StackBase
-  );
-
-
 /**
   This service enables PEIMs to create various types of HOBs.
 
diff --git a/EmbeddedPkg/Library/PrePiHobLib/Hob.c b/EmbeddedPkg/Library/PrePiHobLib/Hob.c
index aff8ea05797b..ba16899a9184 100644
--- a/EmbeddedPkg/Library/PrePiHobLib/Hob.c
+++ b/EmbeddedPkg/Library/PrePiHobLib/Hob.c
@@ -175,47 +175,6 @@ BuildResourceDescriptorHob (
   Hob->ResourceLength    = NumberOfBytes;
 }
 
-/**
-
-
-**/
-VOID
-CreateHobList (
-  IN VOID   *MemoryBegin,
-  IN UINTN  MemoryLength,
-  IN VOID   *HobBase,
-  IN VOID   *StackBase
-  )
-{
-  EFI_HOB_HANDOFF_INFO_TABLE  *Hob;
-  EFI_RESOURCE_ATTRIBUTE_TYPE Attributes;
-
-  Hob = HobConstructor (MemoryBegin,MemoryLength,HobBase,StackBase);
-  SetHobList (Hob);
-
-  BuildCpuHob (PcdGet8 (PcdPrePiCpuMemorySize), PcdGet8 (PcdPrePiCpuIoSize));
-
-  Attributes =(
-    EFI_RESOURCE_ATTRIBUTE_PRESENT |
-    EFI_RESOURCE_ATTRIBUTE_INITIALIZED |
-    EFI_RESOURCE_ATTRIBUTE_TESTED |
-    EFI_RESOURCE_ATTRIBUTE_UNCACHEABLE |
-    EFI_RESOURCE_ATTRIBUTE_WRITE_COMBINEABLE |
-    EFI_RESOURCE_ATTRIBUTE_WRITE_THROUGH_CACHEABLE |
-    EFI_RESOURCE_ATTRIBUTE_WRITE_BACK_CACHEABLE
-  );
-
-  BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY, Attributes, (UINTN)MemoryBegin, MemoryLength);
-
-  BuildStackHob ((EFI_PHYSICAL_ADDRESS)(UINTN)StackBase, ((UINTN)MemoryBegin + MemoryLength) - (UINTN)StackBase);
-
-  if (FeaturePcdGet (PcdPrePiProduceMemoryTypeInformationHob)) {
-    // Optional feature that helps prevent EFI memory map fragmentation.
-    BuildMemoryTypeInformationHob ();
-  }
-}
-
-
 VOID
 EFIAPI
 BuildFvHobs (
-- 
2.19.1

[PATCH v3 03/16] ArmVirtPkg/FdtPciHostBridgeLib: map ECAM and I/O spaces in GCD memory map

[Ard Biesheuvel]

Up until now, we have been getting away with not declaring the ECAM
and translated I/O spaces at all in the GCD memory map, simply because
we map the entire address space with device attributes in the early PEI
code, and so the ECAM space will be mapped wherever it ends up.

Now that we are about to make changes to how ArmVirtQemu reasons
about the size of the address space, it would be better to get rid
of this mapping of the entire address space, since it can get
arbitrarily large without real benefit.

So start by mapping the ECAM and translated I/O spaces explicitly,
instead of relying on the early PEI mapping.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
 ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf |  1 +
 ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c   | 46 +++++++++++++++++++-
 2 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf
index 0995f4b7a156..4011336a353b 100644
--- a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf
+++ b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf
@@ -42,6 +42,7 @@ [Packages]
 [LibraryClasses]
   DebugLib
   DevicePathLib
+  DxeServicesTableLib
   MemoryAllocationLib
   PciPcdProducerLib
 
diff --git a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
index 5b9c887db35d..ebfa14a349f4 100644
--- a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
+++ b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
@@ -17,6 +17,7 @@
 #include <Library/PciHostBridgeLib.h>
 #include <Library/DebugLib.h>
 #include <Library/DevicePathLib.h>
+#include <Library/DxeServicesTableLib.h>
 #include <Library/MemoryAllocationLib.h>
 #include <Library/PcdLib.h>
 #include <Library/UefiBootServicesTableLib.h>
@@ -82,6 +83,33 @@ typedef struct {
 #define DTB_PCI_HOST_RANGE_IO           BIT24
 #define DTB_PCI_HOST_RANGE_TYPEMASK     (BIT31 | BIT30 | BIT29 | BIT25 | BIT24)
 
+STATIC
+EFI_STATUS
+MapGcdMmioSpace (
+  IN    UINT64    Base,
+  IN    UINT64    Size
+  )
+{
+  EFI_STATUS    Status;
+
+  Status = gDS->AddMemorySpace (EfiGcdMemoryTypeMemoryMappedIo, Base, Size,
+                  EFI_MEMORY_UC);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR,
+      "%a: failed to add GCD memory space for region [0x%Lx+0x%Lx)\n",
+      __FUNCTION__, Base, Size));
+    return Status;
+  }
+
+  Status = gDS->SetMemorySpaceAttributes (Base, Size, EFI_MEMORY_UC);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR,
+      "%a: failed to set memory space attributes for region [0x%Lx+0x%Lx)\n",
+      __FUNCTION__, Base, Size));
+  }
+  return Status;
+}
+
 STATIC
 EFI_STATUS
 ProcessPciHost (
@@ -266,7 +294,23 @@ ProcessPciHost (
     "Io[0x%Lx+0x%Lx)@0x%Lx Mem32[0x%Lx+0x%Lx)@0x0 Mem64[0x%Lx+0x%Lx)@0x0\n",
     __FUNCTION__, ConfigBase, ConfigSize, *BusMin, *BusMax, *IoBase, *IoSize,
     IoTranslation, *Mmio32Base, *Mmio32Size, *Mmio64Base, *Mmio64Size));
-  return EFI_SUCCESS;
+
+  // Map the ECAM space in the GCD memory map
+  Status = MapGcdMmioSpace (ConfigBase, ConfigSize);
+  ASSERT_EFI_ERROR (Status);
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  //
+  // Map the MMIO window that provides I/O access - the PCI host bridge code
+  // is not aware of this translation and so it will only map the I/O view
+  // in the GCD I/O map.
+  //
+  Status = MapGcdMmioSpace (*IoBase + IoTranslation, *IoSize);
+  ASSERT_EFI_ERROR (Status);
+
+  return Status;
 }
 
 STATIC PCI_ROOT_BRIDGE mRootBridge;
-- 
2.19.1

[PATCH v3 04/16] ArmVirtPkg/QemuVirtMemInfoLib: remove 1:1 mapping of top of PA range

[Ard Biesheuvel]

Currently, we map DRAM as EFI_MEMORY_WB, and the remainder of the
entire virtual address space is mapped with EFI_MEMORY_UC attributes,
regardless of whether any devices actually reside there.

Now that we are relaxing the address space limit to more than 40 bits,
mapping all that address space actually takes up more space in page
tables than we have so far made available as temporary RAM. So let's
get rid of the mapping rather than increasing the available RAM, given
that the mapping is not particularly useful anyway.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf    |  7 ----
 ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLib.inf |  7 ----
 ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c      | 25 +++----------
 ArmVirtPkg/Library/QemuVirtMemInfoLib/AArch64/PhysAddrTop.S     | 39 --------------------
 ArmVirtPkg/Library/QemuVirtMemInfoLib/Arm/PhysAddrTop.S         | 24 ------------
 5 files changed, 5 insertions(+), 97 deletions(-)

diff --git a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf
index c72a97f9e78a..5c5b841051ad 100644
--- a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf
+++ b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf
@@ -24,12 +24,6 @@ [Defines]
 [Sources]
   QemuVirtMemInfoLib.c
 
-[Sources.ARM]
-  Arm/PhysAddrTop.S
-
-[Sources.AARCH64]
-  AArch64/PhysAddrTop.S
-
 [Packages]
   ArmPkg/ArmPkg.dec
   ArmVirtPkg/ArmVirtPkg.dec
@@ -51,4 +45,3 @@ [Pcd]
 
 [FixedPcd]
   gArmTokenSpaceGuid.PcdFdSize
-  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
diff --git a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLib.inf b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLib.inf
index e4032d3efb53..d12089760b22 100644
--- a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLib.inf
+++ b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLib.inf
@@ -26,12 +26,6 @@ [Sources]
   QemuVirtMemInfoLib.c
   QemuVirtMemInfoPeiLibConstructor.c
 
-[Sources.ARM]
-  Arm/PhysAddrTop.S
-
-[Sources.AARCH64]
-  AArch64/PhysAddrTop.S
-
 [Packages]
   ArmPkg/ArmPkg.dec
   ArmVirtPkg/ArmVirtPkg.dec
@@ -55,4 +49,3 @@ [Pcd]
 [FixedPcd]
   gArmTokenSpaceGuid.PcdFdSize
   gArmVirtTokenSpaceGuid.PcdDeviceTreeInitialBaseAddress
-  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
diff --git a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c
index 760bcc169cf4..0285a11b1d77 100644
--- a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c
+++ b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c
@@ -21,11 +21,6 @@
 // Number of Virtual Memory Map Descriptors
 #define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS          5
 
-EFI_PHYSICAL_ADDRESS
-ArmGetPhysAddrTop (
-  VOID
-  );
-
 /**
   Return the Virtual Memory Map of your platform
 
@@ -45,7 +40,6 @@ ArmVirtGetMemoryMap (
   )
 {
   ARM_MEMORY_REGION_DESCRIPTOR  *VirtualMemoryTable;
-  UINT64                        TopOfMemory;
 
   ASSERT (VirtualMemoryMap != NULL);
 
@@ -78,23 +72,14 @@ ArmVirtGetMemoryMap (
   VirtualMemoryTable[1].Length       = VirtualMemoryTable[0].PhysicalBase;
   VirtualMemoryTable[1].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
 
-  // Peripheral space after DRAM
-  TopOfMemory = MIN (1ULL << FixedPcdGet8 (PcdPrePiCpuMemorySize),
-                     ArmGetPhysAddrTop ());
-  VirtualMemoryTable[2].PhysicalBase = VirtualMemoryTable[0].Length + VirtualMemoryTable[1].Length;
-  VirtualMemoryTable[2].VirtualBase  = VirtualMemoryTable[2].PhysicalBase;
-  VirtualMemoryTable[2].Length       = TopOfMemory -
-                                       VirtualMemoryTable[2].PhysicalBase;
-  VirtualMemoryTable[2].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
-
   // Remap the FD region as normal executable memory
-  VirtualMemoryTable[3].PhysicalBase = PcdGet64 (PcdFdBaseAddress);
-  VirtualMemoryTable[3].VirtualBase  = VirtualMemoryTable[3].PhysicalBase;
-  VirtualMemoryTable[3].Length       = FixedPcdGet32 (PcdFdSize);
-  VirtualMemoryTable[3].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK;
+  VirtualMemoryTable[2].PhysicalBase = PcdGet64 (PcdFdBaseAddress);
+  VirtualMemoryTable[2].VirtualBase  = VirtualMemoryTable[2].PhysicalBase;
+  VirtualMemoryTable[2].Length       = FixedPcdGet32 (PcdFdSize);
+  VirtualMemoryTable[2].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK;
 
   // End of Table
-  ZeroMem (&VirtualMemoryTable[4], sizeof (ARM_MEMORY_REGION_DESCRIPTOR));
+  ZeroMem (&VirtualMemoryTable[3], sizeof (ARM_MEMORY_REGION_DESCRIPTOR));
 
   *VirtualMemoryMap = VirtualMemoryTable;
 }
diff --git a/ArmVirtPkg/Library/QemuVirtMemInfoLib/AArch64/PhysAddrTop.S b/ArmVirtPkg/Library/QemuVirtMemInfoLib/AArch64/PhysAddrTop.S
deleted file mode 100644
index a1f6a194d59b..000000000000
--- a/ArmVirtPkg/Library/QemuVirtMemInfoLib/AArch64/PhysAddrTop.S
+++ /dev/null
@@ -1,39 +0,0 @@
-#
-#  Copyright (c) 2011-2013, ARM Limited. All rights reserved.
-#  Copyright (c) 2016-2017, Linaro Limited. All rights reserved.
-#
-#  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.
-#
-#
-
-#include <AsmMacroIoLibV8.h>
-
-//EFI_PHYSICAL_ADDRESS
-//GetPhysAddrTop (
-//  VOID
-//  );
-ASM_FUNC(ArmGetPhysAddrTop)
-  mrs   x0, id_aa64mmfr0_el1
-  adr   x1, .LPARanges
-  and   x0, x0, #7
-  ldrb  w1, [x1, x0]
-  mov   x0, #1
-  lsl   x0, x0, x1
-  ret
-
-//
-// Bits 0..2 of the AA64MFR0_EL1 system register encode the size of the
-// physical address space support on this CPU:
-// 0 == 32 bits, 1 == 36 bits, etc etc
-// 6 and 7 are reserved
-//
-.LPARanges:
-  .byte 32, 36, 40, 42, 44, 48, -1, -1
-
-ASM_FUNCTION_REMOVE_IF_UNREFERENCED
diff --git a/ArmVirtPkg/Library/QemuVirtMemInfoLib/Arm/PhysAddrTop.S b/ArmVirtPkg/Library/QemuVirtMemInfoLib/Arm/PhysAddrTop.S
deleted file mode 100644
index 9cd81529fb3d..000000000000
--- a/ArmVirtPkg/Library/QemuVirtMemInfoLib/Arm/PhysAddrTop.S
+++ /dev/null
@@ -1,24 +0,0 @@
-#
-#  Copyright (c) 2011-2013, ARM Limited. All rights reserved.
-#  Copyright (c) 2014-2017, Linaro Limited. All rights reserved.
-#
-#  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.
-#
-#
-
-#include <AsmMacroIoLib.h>
-
-//EFI_PHYSICAL_ADDRESS
-//GetPhysAddrTop (
-//  VOID
-//  );
-ASM_FUNC(ArmGetPhysAddrTop)
-  mov   r0, #0x00000000
-  mov   r1, #0x10000
-  bx    lr
-- 
2.19.1

[PATCH v3 05/16] MdePkg/ProcessorBind.h AARCH64: limit MAX_ADDRESS to 48 bits

[Ard Biesheuvel]

AArch64 supports the use of more than 48 bits for physical and/or
virtual addressing, but only if the page size is set to 64 KB,
which is not supported by UEFI. So redefine MAX_ADDRESS to cover
only 48 address bits.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
---
 MdePkg/Include/AArch64/ProcessorBind.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdePkg/Include/AArch64/ProcessorBind.h b/MdePkg/Include/AArch64/ProcessorBind.h
index 968c18f915ae..dad75df1c579 100644
--- a/MdePkg/Include/AArch64/ProcessorBind.h
+++ b/MdePkg/Include/AArch64/ProcessorBind.h
@@ -138,9 +138,9 @@ typedef INT64   INTN;
 #define MAX_2_BITS  0xC000000000000000ULL
 
 ///
-/// Maximum legal AARCH64  address
+/// Maximum legal AARCH64  address (48 bits for 4 KB page size)
 ///
-#define MAX_ADDRESS   0xFFFFFFFFFFFFFFFFULL
+#define MAX_ADDRESS   0xFFFFFFFFFFFFULL
 
 ///
 /// Maximum legal AArch64 INTN and UINTN values.
-- 
2.19.1

[PATCH v3 06/16] ArmPkg/ArmLib: add support for reading the max physical address space size

[Ard Biesheuvel]

Add a helper function that returns the maximum physical address space
size as supported by the current CPU.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 ArmPkg/Include/Library/ArmLib.h               |  6 ++++++
 ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S | 17 +++++++++++++++++
 ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S     |  8 ++++++++
 ArmPkg/Library/ArmLib/Arm/ArmLibSupport.asm   |  8 ++++++++
 4 files changed, 39 insertions(+)

diff --git a/ArmPkg/Include/Library/ArmLib.h b/ArmPkg/Include/Library/ArmLib.h
index ffda50e9d767..9a804c15fdb6 100644
--- a/ArmPkg/Include/Library/ArmLib.h
+++ b/ArmPkg/Include/Library/ArmLib.h
@@ -733,4 +733,10 @@ ArmWriteCntvOff (
   UINT64   Val
   );
 
+UINTN
+EFIAPI
+ArmGetPhysicalAddressBits (
+  VOID
+  );
+
 #endif // __ARM_LIB__
diff --git a/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S b/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
index 1ef2f61f5979..b7173e00b039 100644
--- a/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
+++ b/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
@@ -196,4 +196,21 @@ ASM_FUNC(ArmWriteSctlr)
 3:msr   sctlr_el3, x0
 4:ret
 
+ASM_FUNC(ArmGetPhysicalAddressBits)
+  mrs   x0, id_aa64mmfr0_el1
+  adr   x1, .LPARanges
+  and   x0, x0, #0xf
+  ldrb  w0, [x1, x0]
+  ret
+
+//
+// Bits 0..3 of the AA64MFR0_EL1 system register encode the size of the
+// physical address space support on this CPU:
+// 0 == 32 bits, 1 == 36 bits, etc etc
+// 7 and up are reserved
+//
+.LPARanges:
+  .byte 32, 36, 40, 42, 44, 48, 52,  0
+  .byte  0,  0,  0,  0,  0,  0,  0,  0
+
 ASM_FUNCTION_REMOVE_IF_UNREFERENCED
diff --git a/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S b/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S
index f2a517671f0a..0e9f9d0453e4 100644
--- a/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S
+++ b/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S
@@ -165,4 +165,12 @@ ASM_FUNC(ArmWriteCpuActlr)
   isb
   bx      lr
 
+ASM_FUNC (ArmGetPhysicalAddressBits)
+  mrc     p15, 0, r0, c0, c1, 4   // MMFR0
+  and     r0, r0, #0xf            // VMSA [3:0]
+  cmp     r0, #5                  // >= 5 implies LPAE support
+  movlt   r0, #32                 // 32 bits if no LPAE
+  movge   r0, #40                 // 40 bits if LPAE
+  bx      lr
+
 ASM_FUNCTION_REMOVE_IF_UNREFERENCED
diff --git a/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.asm b/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.asm
index 219140c22b13..3eb52875971d 100644
--- a/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.asm
+++ b/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.asm
@@ -169,4 +169,12 @@
   isb
   bx      lr
 
+ RVCT_ASM_EXPORT ArmGetPhysicalAddressBits
+  mrc     p15, 0, r0, c0, c1, 4   ; MMFR0
+  and     r0, r0, #0xf            ; VMSA [3:0]
+  cmp     r0, #5                  ; >= 5 implies LPAE support
+  movlt   r0, #32                 ; 32 bits if no LPAE
+  movge   r0, #40                 ; 40 bits if LPAE
+  bx      lr
+
   END
-- 
2.19.1

[PATCH v3 07/16] ArmVirtPkg/XenVirtMemInfoLib: refactor reading of the PA space size

[Ard Biesheuvel]

Use the new ArmLib helper to read the CPU's physical address limit
so we can drop our own homecooked one.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 ArmVirtPkg/Library/XenVirtMemInfoLib/XenVirtMemInfoLib.inf |  6 ---
 ArmVirtPkg/Library/XenVirtMemInfoLib/XenVirtMemInfoLib.c   | 11 +++---
 ArmVirtPkg/Library/XenVirtMemInfoLib/AArch64/PhysAddrTop.S | 39 --------------------
 ArmVirtPkg/Library/XenVirtMemInfoLib/Arm/PhysAddrTop.S     | 24 ------------
 4 files changed, 5 insertions(+), 75 deletions(-)

diff --git a/ArmVirtPkg/Library/XenVirtMemInfoLib/XenVirtMemInfoLib.inf b/ArmVirtPkg/Library/XenVirtMemInfoLib/XenVirtMemInfoLib.inf
index cd4c805a4db9..ae107810e927 100644
--- a/ArmVirtPkg/Library/XenVirtMemInfoLib/XenVirtMemInfoLib.inf
+++ b/ArmVirtPkg/Library/XenVirtMemInfoLib/XenVirtMemInfoLib.inf
@@ -24,12 +24,6 @@ [Defines]
 [Sources]
   XenVirtMemInfoLib.c
 
-[Sources.ARM]
-  Arm/PhysAddrTop.S
-
-[Sources.AARCH64]
-  AArch64/PhysAddrTop.S
-
 [Packages]
   ArmPkg/ArmPkg.dec
   ArmVirtPkg/ArmVirtPkg.dec
diff --git a/ArmVirtPkg/Library/XenVirtMemInfoLib/XenVirtMemInfoLib.c b/ArmVirtPkg/Library/XenVirtMemInfoLib/XenVirtMemInfoLib.c
index 88ff3167cbfd..6701dec50ea8 100644
--- a/ArmVirtPkg/Library/XenVirtMemInfoLib/XenVirtMemInfoLib.c
+++ b/ArmVirtPkg/Library/XenVirtMemInfoLib/XenVirtMemInfoLib.c
@@ -18,11 +18,6 @@
 
 STATIC ARM_MEMORY_REGION_DESCRIPTOR  mVirtualMemoryTable[2];
 
-EFI_PHYSICAL_ADDRESS
-ArmGetPhysAddrTop (
-  VOID
-  );
-
 /**
   Return the Virtual Memory Map of your platform
 
@@ -42,8 +37,12 @@ ArmVirtGetMemoryMap (
   OUT ARM_MEMORY_REGION_DESCRIPTOR   **VirtualMemoryMap
   )
 {
+  EFI_PHYSICAL_ADDRESS TopOfAddressSpace;
+
   ASSERT (VirtualMemoryMap != NULL);
 
+  TopOfAddressSpace = LShiftU64 (1ULL, ArmGetPhysicalAddressBits ());
+
   //
   // Map the entire physical memory space as cached. The only device
   // we care about is the GIC, which will be stage 2 mapped as a device
@@ -51,7 +50,7 @@ ArmVirtGetMemoryMap (
   //
   mVirtualMemoryTable[0].PhysicalBase = 0x0;
   mVirtualMemoryTable[0].VirtualBase  = 0x0;
-  mVirtualMemoryTable[0].Length       = ArmGetPhysAddrTop ();
+  mVirtualMemoryTable[0].Length       = TopOfAddressSpace;
   mVirtualMemoryTable[0].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK;
 
   mVirtualMemoryTable[1].PhysicalBase = 0x0;
diff --git a/ArmVirtPkg/Library/XenVirtMemInfoLib/AArch64/PhysAddrTop.S b/ArmVirtPkg/Library/XenVirtMemInfoLib/AArch64/PhysAddrTop.S
deleted file mode 100644
index a1f6a194d59b..000000000000
--- a/ArmVirtPkg/Library/XenVirtMemInfoLib/AArch64/PhysAddrTop.S
+++ /dev/null
@@ -1,39 +0,0 @@
-#
-#  Copyright (c) 2011-2013, ARM Limited. All rights reserved.
-#  Copyright (c) 2016-2017, Linaro Limited. All rights reserved.
-#
-#  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.
-#
-#
-
-#include <AsmMacroIoLibV8.h>
-
-//EFI_PHYSICAL_ADDRESS
-//GetPhysAddrTop (
-//  VOID
-//  );
-ASM_FUNC(ArmGetPhysAddrTop)
-  mrs   x0, id_aa64mmfr0_el1
-  adr   x1, .LPARanges
-  and   x0, x0, #7
-  ldrb  w1, [x1, x0]
-  mov   x0, #1
-  lsl   x0, x0, x1
-  ret
-
-//
-// Bits 0..2 of the AA64MFR0_EL1 system register encode the size of the
-// physical address space support on this CPU:
-// 0 == 32 bits, 1 == 36 bits, etc etc
-// 6 and 7 are reserved
-//
-.LPARanges:
-  .byte 32, 36, 40, 42, 44, 48, -1, -1
-
-ASM_FUNCTION_REMOVE_IF_UNREFERENCED
diff --git a/ArmVirtPkg/Library/XenVirtMemInfoLib/Arm/PhysAddrTop.S b/ArmVirtPkg/Library/XenVirtMemInfoLib/Arm/PhysAddrTop.S
deleted file mode 100644
index 9cd81529fb3d..000000000000
--- a/ArmVirtPkg/Library/XenVirtMemInfoLib/Arm/PhysAddrTop.S
+++ /dev/null
@@ -1,24 +0,0 @@
-#
-#  Copyright (c) 2011-2013, ARM Limited. All rights reserved.
-#  Copyright (c) 2014-2017, Linaro Limited. All rights reserved.
-#
-#  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.
-#
-#
-
-#include <AsmMacroIoLib.h>
-
-//EFI_PHYSICAL_ADDRESS
-//GetPhysAddrTop (
-//  VOID
-//  );
-ASM_FUNC(ArmGetPhysAddrTop)
-  mov   r0, #0x00000000
-  mov   r1, #0x10000
-  bx    lr
-- 
2.19.1

[PATCH v3 08/16] ArmPkg/ArmMmuLib: take the CPU supported maximum PA space into account

[Ard Biesheuvel]

In preparation of dropping PcdPrePiCpuMemorySize entirely, base the
maximum size of the identity map on the capabilities of the CPU.
Since that may exceed what is architecturally permitted when using
4 KB pages, take MAX_ADDRESS into account as well.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf       |  3 ---
 ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf        |  3 ---
 ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 11 +++++++++--
 3 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf b/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
index b9f264de8d26..246963361e45 100644
--- a/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
+++ b/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
@@ -40,8 +40,5 @@ [LibraryClasses]
   CacheMaintenanceLib
   MemoryAllocationLib
 
-[Pcd.AARCH64]
-  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
-
 [Pcd.ARM]
   gArmTokenSpaceGuid.PcdNormalMemoryNonshareableOverride
diff --git a/ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf b/ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf
index ecf13f790734..f689c193b862 100644
--- a/ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf
+++ b/ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf
@@ -35,6 +35,3 @@ [LibraryClasses]
   ArmLib
   CacheMaintenanceLib
   MemoryAllocationLib
-
-[Pcd.AARCH64]
-  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
index 4b62ecb6a476..5403b8d4070e 100644
--- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
+++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
@@ -604,8 +604,15 @@ ArmConfigureMmu (
     return EFI_INVALID_PARAMETER;
   }
 
-  // Cover the entire GCD memory space
-  MaxAddress = (1UL << PcdGet8 (PcdPrePiCpuMemorySize)) - 1;
+  //
+  // Limit the virtual address space to what we can actually use: UEFI
+  // mandates a 1:1 mapping, so no point in making the virtual address
+  // space larger than the physical address space. We also have to take
+  // into account the architectural limitations that result from UEFI's
+  // use of 4 KB pages.
+  //
+  MaxAddress = MIN (LShiftU64 (1ULL, ArmGetPhysicalAddressBits ()) - 1,
+                    MAX_ADDRESS);
 
   // Lookup the Table Level to get the information
   LookupAddresstoRootTable (MaxAddress, &T0SZ, &RootTableEntryCount);
-- 
2.19.1

[PATCH v3 09/16] ArmPkg/CpuPei: base GCD memory space size on CPU's PA range

[Ard Biesheuvel]

Derive the size of the GCD memory space map directly from the CPU's
information registers rather than from the PcdPrePiCpuMemorySize PCD,
which will be removed.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 ArmPkg/Drivers/CpuPei/CpuPei.inf | 1 -
 ArmPkg/Drivers/CpuPei/CpuPei.c   | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/ArmPkg/Drivers/CpuPei/CpuPei.inf b/ArmPkg/Drivers/CpuPei/CpuPei.inf
index eafccd600983..dcea012fd8f9 100644
--- a/ArmPkg/Drivers/CpuPei/CpuPei.inf
+++ b/ArmPkg/Drivers/CpuPei/CpuPei.inf
@@ -50,7 +50,6 @@ [Guids]
   gArmMpCoreInfoGuid
 
 [FixedPcd]
-  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
   gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize
 
 [Depex]
diff --git a/ArmPkg/Drivers/CpuPei/CpuPei.c b/ArmPkg/Drivers/CpuPei/CpuPei.c
index d54f42acfcc8..e63519ff6481 100644
--- a/ArmPkg/Drivers/CpuPei/CpuPei.c
+++ b/ArmPkg/Drivers/CpuPei/CpuPei.c
@@ -73,7 +73,7 @@ InitializeCpuPeim (
   ArmEnableBranchPrediction ();
 
   // Publish the CPU memory and io spaces sizes
-  BuildCpuHob (PcdGet8 (PcdPrePiCpuMemorySize), PcdGet8 (PcdPrePiCpuIoSize));
+  BuildCpuHob (ArmGetPhysicalAddressBits (), PcdGet8 (PcdPrePiCpuIoSize));
 
   // Only MP Core platform need to produce gArmMpCoreInfoPpiGuid
   Status = PeiServicesLocatePpi (&gArmMpCoreInfoPpiGuid, 0, NULL, (VOID**)&ArmMpCoreInfoPpi);
-- 
2.19.1

[PATCH v3 10/16] ArmPlatformPkg/PrePi: base GCD memory space size on CPU's PA range

[Ard Biesheuvel]

Derive the size of the GCD memory space map directly from the CPU's
information registers rather than from the PcdPrePiCpuMemorySize PCD,
which will be removed.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 ArmPlatformPkg/PrePi/PeiMPCore.inf  | 1 -
 ArmPlatformPkg/PrePi/PeiUniCore.inf | 1 -
 ArmPlatformPkg/PrePi/PrePi.c        | 2 +-
 3 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/ArmPlatformPkg/PrePi/PeiMPCore.inf b/ArmPlatformPkg/PrePi/PeiMPCore.inf
index 242b03175536..7e2ad6fc483d 100644
--- a/ArmPlatformPkg/PrePi/PeiMPCore.inf
+++ b/ArmPlatformPkg/PrePi/PeiMPCore.inf
@@ -97,7 +97,6 @@ [FixedPcd]
 
   gArmPlatformTokenSpaceGuid.PcdCoreCount
 
-  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
   gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize
 
   gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIReclaimMemory
diff --git a/ArmPlatformPkg/PrePi/PeiUniCore.inf b/ArmPlatformPkg/PrePi/PeiUniCore.inf
index a45cdef4ed91..26328b7e8f67 100644
--- a/ArmPlatformPkg/PrePi/PeiUniCore.inf
+++ b/ArmPlatformPkg/PrePi/PeiUniCore.inf
@@ -90,7 +90,6 @@ [FixedPcd]
 
   gArmPlatformTokenSpaceGuid.PcdCoreCount
 
-  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
   gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize
 
   gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIReclaimMemory
diff --git a/ArmPlatformPkg/PrePi/PrePi.c b/ArmPlatformPkg/PrePi/PrePi.c
index 02cff7ddc204..245bdded1eb3 100644
--- a/ArmPlatformPkg/PrePi/PrePi.c
+++ b/ArmPlatformPkg/PrePi/PrePi.c
@@ -115,7 +115,7 @@ PrePiMain (
   BuildStackHob (StacksBase, StacksSize);
 
   //TODO: Call CpuPei as a library
-  BuildCpuHob (PcdGet8 (PcdPrePiCpuMemorySize), PcdGet8 (PcdPrePiCpuIoSize));
+  BuildCpuHob (ArmGetPhysicalAddressBits (), PcdGet8 (PcdPrePiCpuIoSize));
 
   if (ArmIsMpCore ()) {
     // Only MP Core platform need to produce gArmMpCoreInfoPpiGuid
-- 
2.19.1

[PATCH v3 11/16] ArmVirtPkg/PrePi: base GCD memory space size on CPU's PA range

[Ard Biesheuvel]

Derive the size of the GCD memory space map directly from the CPU's
information registers rather than from the PcdPrePiCpuMemorySize PCD,
which will be removed.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf | 1 -
 ArmVirtPkg/PrePi/PrePi.c                            | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf b/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf
index 1587bd92f206..034ddb41cb48 100755
--- a/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf
+++ b/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf
@@ -85,7 +85,6 @@ [FixedPcd]
 
   gArmPlatformTokenSpaceGuid.PcdCoreCount
 
-  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
   gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize
 
   gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIReclaimMemory
diff --git a/ArmVirtPkg/PrePi/PrePi.c b/ArmVirtPkg/PrePi/PrePi.c
index f6abe2f2016b..61de6cfd4ae6 100755
--- a/ArmVirtPkg/PrePi/PrePi.c
+++ b/ArmVirtPkg/PrePi/PrePi.c
@@ -80,7 +80,7 @@ PrePiMain (
   BuildStackHob (StacksBase, StacksSize);
 
   //TODO: Call CpuPei as a library
-  BuildCpuHob (PcdGet8 (PcdPrePiCpuMemorySize), PcdGet8 (PcdPrePiCpuIoSize));
+  BuildCpuHob (ArmGetPhysicalAddressBits (), PcdGet8 (PcdPrePiCpuIoSize));
 
   // Set the Boot Mode
   SetBootMode (BOOT_WITH_FULL_CONFIGURATION);
-- 
2.19.1

[PATCH v3 12/16] BeagleBoardPkg/PrePi: base GCD memory space size on CPU's PA range

[Ard Biesheuvel]

Derive the size of the GCD memory space map directly from the CPU's
information registers rather than from the PcdPrePiCpuMemorySize PCD,
which will be removed.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 BeagleBoardPkg/PrePi/PeiUniCore.inf | 1 -
 BeagleBoardPkg/PrePi/PrePi.c        | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/BeagleBoardPkg/PrePi/PeiUniCore.inf b/BeagleBoardPkg/PrePi/PeiUniCore.inf
index 3d72bc5b46e1..53c71d8eafc2 100644
--- a/BeagleBoardPkg/PrePi/PeiUniCore.inf
+++ b/BeagleBoardPkg/PrePi/PeiUniCore.inf
@@ -86,7 +86,6 @@ [FixedPcd]
 
   gArmPlatformTokenSpaceGuid.PcdCoreCount
 
-  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
   gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize
 
   gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIReclaimMemory
diff --git a/BeagleBoardPkg/PrePi/PrePi.c b/BeagleBoardPkg/PrePi/PrePi.c
index 46f63f40c46e..bc9b0c80b84c 100644
--- a/BeagleBoardPkg/PrePi/PrePi.c
+++ b/BeagleBoardPkg/PrePi/PrePi.c
@@ -110,7 +110,7 @@ PrePiMain (
   BuildStackHob (StacksBase, StacksSize);
 
   //TODO: Call CpuPei as a library
-  BuildCpuHob (PcdGet8 (PcdPrePiCpuMemorySize), PcdGet8 (PcdPrePiCpuIoSize));
+  BuildCpuHob (ArmGetPhysicalAddressBits (), PcdGet8 (PcdPrePiCpuIoSize));
 
   // Store timer value logged at the beginning of firmware image execution
   Performance.ResetEnd = GetTimeInNanoSecond (StartTimeStamp);
-- 
2.19.1

[PATCH v3 13/16] ArmPlatformPkg/PlatformPei: drop unused PCD references

[Ard Biesheuvel]

Drop some PCD references that are not actually referenced from the
PlatformPei code.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf | 3 ---
 ArmPlatformPkg/PlatformPei/PlatformPeim.inf   | 3 ---
 2 files changed, 6 deletions(-)

diff --git a/ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf b/ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf
index 314789d0a990..23bb3f37e766 100644
--- a/ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf
+++ b/ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf
@@ -46,8 +46,5 @@ [FixedPcd]
   gArmTokenSpaceGuid.PcdFvBaseAddress
   gArmTokenSpaceGuid.PcdFvSize
 
-  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
-  gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize
-
 [depex]
   TRUE
diff --git a/ArmPlatformPkg/PlatformPei/PlatformPeim.inf b/ArmPlatformPkg/PlatformPei/PlatformPeim.inf
index 423b9ab858d1..4934baa838e1 100644
--- a/ArmPlatformPkg/PlatformPei/PlatformPeim.inf
+++ b/ArmPlatformPkg/PlatformPei/PlatformPeim.inf
@@ -57,9 +57,6 @@ [FixedPcd]
   gArmTokenSpaceGuid.PcdFvBaseAddress
   gArmTokenSpaceGuid.PcdFvSize
 
-  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
-  gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize
-
 [Depex]
   TRUE
 
-- 
2.19.1

[PATCH v3 14/16] EmbeddedPkg/PrePiLib: drop unused PCD reference

[Ard Biesheuvel]

Drop the reference to gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
which is never used.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 EmbeddedPkg/Library/PrePiLib/PrePiLib.inf | 1 -
 1 file changed, 1 deletion(-)

diff --git a/EmbeddedPkg/Library/PrePiLib/PrePiLib.inf b/EmbeddedPkg/Library/PrePiLib/PrePiLib.inf
index de68405098c0..3dba884b1f31 100644
--- a/EmbeddedPkg/Library/PrePiLib/PrePiLib.inf
+++ b/EmbeddedPkg/Library/PrePiLib/PrePiLib.inf
@@ -69,7 +69,6 @@ [Protocols]
 
 
 [FixedPcd.common]
-  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
   gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize
 
   gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIReclaimMemory
-- 
2.19.1

[PATCH v3 15/16] ArmVirtPkg: drop PcdPrePiCpuMemorySize assignments from all platforms

[Ard Biesheuvel]

PcdPrePiCpuMemorySize is no longer used so drop the PCD overrides
from all platform descriptions in ArmVirtPkg.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 ArmVirtPkg/ArmVirt.dsc.inc       | 3 ---
 ArmVirtPkg/ArmVirtQemu.dsc       | 4 ----
 ArmVirtPkg/ArmVirtQemuKernel.dsc | 4 ----
 3 files changed, 11 deletions(-)

diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
index 70a0ac4d786c..fbdb5c982604 100644
--- a/ArmVirtPkg/ArmVirt.dsc.inc
+++ b/ArmVirtPkg/ArmVirt.dsc.inc
@@ -388,9 +388,6 @@ [PcdsFixedAtBuild.common]
   #
   gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE
 
-[PcdsFixedAtBuild.ARM]
-  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|40
-
 [Components.common]
   #
   # Ramdisk support
diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
index 885c6b14b844..a107b6bb5104 100644
--- a/ArmVirtPkg/ArmVirtQemu.dsc
+++ b/ArmVirtPkg/ArmVirtQemu.dsc
@@ -143,10 +143,6 @@ [PcdsFixedAtBuild.common]
   gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|16
 
 [PcdsFixedAtBuild.AARCH64]
-  # KVM limits it IPA space to 40 bits (1 TB), so there is no need to
-  # support anything bigger, even if the host hardware does
-  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|40
-
   # Clearing BIT0 in this PCD prevents installing a 32-bit SMBIOS entry point,
   # if the entry point version is >= 3.0. AARCH64 OSes cannot assume the
   # presence of the 32-bit entry point anyway (because many AARCH64 systems
diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
index 434d6861a56f..d8fbf14e8f4e 100644
--- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
+++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
@@ -157,10 +157,6 @@ [PcdsFixedAtBuild.AARCH64]
   #
   gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|16
 
-  # KVM limits it IPA space to 40 bits (1 TB), so there is no need to
-  # support anything bigger, even if the host hardware does
-  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|40
-
 [PcdsDynamicDefault.common]
   gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|3
 
-- 
2.19.1

[PATCH v3 16/16] EmbeddedPkg/EmbeddedPkg.dec: drop PcdPrePiCpuMemorySize declarations

[Ard Biesheuvel]

PcdPrePiCpuMemorySize is no longer used so drop the declarations from
the package DEC file.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 EmbeddedPkg/EmbeddedPkg.dec | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/EmbeddedPkg/EmbeddedPkg.dec b/EmbeddedPkg/EmbeddedPkg.dec
index 28a143865d0e..ff5aab07d745 100644
--- a/EmbeddedPkg/EmbeddedPkg.dec
+++ b/EmbeddedPkg/EmbeddedPkg.dec
@@ -170,22 +170,18 @@ [PcdsFixedAtBuild.common]
   gEmbeddedTokenSpaceGuid.PcdAndroidBootDevicePath|L""|VOID*|0x00000057
 
 [PcdsFixedAtBuild.ARM]
-  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|32|UINT8|0x00000010
   gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|0|UINT8|0x00000011
 
   # ISP1761 USB OTG Controller
   gEmbeddedTokenSpaceGuid.PcdIsp1761BaseAddress|0|UINT32|0x00000021
 
 [PcdsFixedAtBuild.AARCH64]
-  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|48|UINT8|0x00000010
   gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|0|UINT8|0x00000011
 
 [PcdsFixedAtBuild.IA32]
-  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|36|UINT8|0x00000010
   gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|16|UINT8|0x00000011
 
 [PcdsFixedAtBuild.X64]
-  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|52|UINT8|0x00000010
   gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|16|UINT8|0x00000011
 
 [PcdsFixedAtBuild.common, PcdsDynamic.common]
-- 
2.19.1

Re: [PATCH v3 06/16] ArmPkg/ArmLib: add support for reading the max physical address space size

[Philippe Mathieu-Daudé]

On 28/11/18 15:33, Ard Biesheuvel wrote:
> Add a helper function that returns the maximum physical address space
> size as supported by the current CPU.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  ArmPkg/Include/Library/ArmLib.h               |  6 ++++++
>  ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S | 17 +++++++++++++++++
>  ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S     |  8 ++++++++
>  ArmPkg/Library/ArmLib/Arm/ArmLibSupport.asm   |  8 ++++++++
>  4 files changed, 39 insertions(+)
> 
> diff --git a/ArmPkg/Include/Library/ArmLib.h b/ArmPkg/Include/Library/ArmLib.h
> index ffda50e9d767..9a804c15fdb6 100644
> --- a/ArmPkg/Include/Library/ArmLib.h
> +++ b/ArmPkg/Include/Library/ArmLib.h
> @@ -733,4 +733,10 @@ ArmWriteCntvOff (
>    UINT64   Val
>    );
>  
> +UINTN
> +EFIAPI
> +ArmGetPhysicalAddressBits (
> +  VOID
> +  );
> +
>  #endif // __ARM_LIB__
> diff --git a/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S b/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
> index 1ef2f61f5979..b7173e00b039 100644
> --- a/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
> +++ b/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
> @@ -196,4 +196,21 @@ ASM_FUNC(ArmWriteSctlr)
>  3:msr   sctlr_el3, x0
>  4:ret
>  
> +ASM_FUNC(ArmGetPhysicalAddressBits)
> +  mrs   x0, id_aa64mmfr0_el1
> +  adr   x1, .LPARanges
> +  and   x0, x0, #0xf
> +  ldrb  w0, [x1, x0]
> +  ret
> +
> +//
> +// Bits 0..3 of the AA64MFR0_EL1 system register encode the size of the
> +// physical address space support on this CPU:
> +// 0 == 32 bits, 1 == 36 bits, etc etc
> +// 7 and up are reserved
> +//
> +.LPARanges:
> +  .byte 32, 36, 40, 42, 44, 48, 52,  0
> +  .byte  0,  0,  0,  0,  0,  0,  0,  0
> +
>  ASM_FUNCTION_REMOVE_IF_UNREFERENCED
> diff --git a/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S b/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S
> index f2a517671f0a..0e9f9d0453e4 100644
> --- a/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S
> +++ b/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S
> @@ -165,4 +165,12 @@ ASM_FUNC(ArmWriteCpuActlr)
>    isb
>    bx      lr
>  
> +ASM_FUNC (ArmGetPhysicalAddressBits)
> +  mrc     p15, 0, r0, c0, c1, 4   // MMFR0
> +  and     r0, r0, #0xf            // VMSA [3:0]
> +  cmp     r0, #5                  // >= 5 implies LPAE support
> +  movlt   r0, #32                 // 32 bits if no LPAE
> +  movge   r0, #40                 // 40 bits if LPAE
> +  bx      lr
> +
>  ASM_FUNCTION_REMOVE_IF_UNREFERENCED
> diff --git a/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.asm b/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.asm
> index 219140c22b13..3eb52875971d 100644
> --- a/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.asm
> +++ b/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.asm
> @@ -169,4 +169,12 @@
>    isb
>    bx      lr
>  
> + RVCT_ASM_EXPORT ArmGetPhysicalAddressBits
> +  mrc     p15, 0, r0, c0, c1, 4   ; MMFR0
> +  and     r0, r0, #0xf            ; VMSA [3:0]
> +  cmp     r0, #5                  ; >= 5 implies LPAE support
> +  movlt   r0, #32                 ; 32 bits if no LPAE
> +  movge   r0, #40                 ; 40 bits if LPAE
> +  bx      lr
> +
>    END
> 

Re: [PATCH v3 07/16] ArmVirtPkg/XenVirtMemInfoLib: refactor reading of the PA space size

[Philippe Mathieu-Daudé]

On 28/11/18 15:33, Ard Biesheuvel wrote:
> Use the new ArmLib helper to read the CPU's physical address limit
> so we can drop our own homecooked one.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  ArmVirtPkg/Library/XenVirtMemInfoLib/XenVirtMemInfoLib.inf |  6 ---
>  ArmVirtPkg/Library/XenVirtMemInfoLib/XenVirtMemInfoLib.c   | 11 +++---
>  ArmVirtPkg/Library/XenVirtMemInfoLib/AArch64/PhysAddrTop.S | 39 --------------------
>  ArmVirtPkg/Library/XenVirtMemInfoLib/Arm/PhysAddrTop.S     | 24 ------------

Good diffstat :)

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

>  4 files changed, 5 insertions(+), 75 deletions(-)
> 
> diff --git a/ArmVirtPkg/Library/XenVirtMemInfoLib/XenVirtMemInfoLib.inf b/ArmVirtPkg/Library/XenVirtMemInfoLib/XenVirtMemInfoLib.inf
> index cd4c805a4db9..ae107810e927 100644
> --- a/ArmVirtPkg/Library/XenVirtMemInfoLib/XenVirtMemInfoLib.inf
> +++ b/ArmVirtPkg/Library/XenVirtMemInfoLib/XenVirtMemInfoLib.inf
> @@ -24,12 +24,6 @@ [Defines]
>  [Sources]
>    XenVirtMemInfoLib.c
>  
> -[Sources.ARM]
> -  Arm/PhysAddrTop.S
> -
> -[Sources.AARCH64]
> -  AArch64/PhysAddrTop.S
> -
>  [Packages]
>    ArmPkg/ArmPkg.dec
>    ArmVirtPkg/ArmVirtPkg.dec
> diff --git a/ArmVirtPkg/Library/XenVirtMemInfoLib/XenVirtMemInfoLib.c b/ArmVirtPkg/Library/XenVirtMemInfoLib/XenVirtMemInfoLib.c
> index 88ff3167cbfd..6701dec50ea8 100644
> --- a/ArmVirtPkg/Library/XenVirtMemInfoLib/XenVirtMemInfoLib.c
> +++ b/ArmVirtPkg/Library/XenVirtMemInfoLib/XenVirtMemInfoLib.c
> @@ -18,11 +18,6 @@
>  
>  STATIC ARM_MEMORY_REGION_DESCRIPTOR  mVirtualMemoryTable[2];
>  
> -EFI_PHYSICAL_ADDRESS
> -ArmGetPhysAddrTop (
> -  VOID
> -  );
> -
>  /**
>    Return the Virtual Memory Map of your platform
>  
> @@ -42,8 +37,12 @@ ArmVirtGetMemoryMap (
>    OUT ARM_MEMORY_REGION_DESCRIPTOR   **VirtualMemoryMap
>    )
>  {
> +  EFI_PHYSICAL_ADDRESS TopOfAddressSpace;
> +
>    ASSERT (VirtualMemoryMap != NULL);
>  
> +  TopOfAddressSpace = LShiftU64 (1ULL, ArmGetPhysicalAddressBits ());
> +
>    //
>    // Map the entire physical memory space as cached. The only device
>    // we care about is the GIC, which will be stage 2 mapped as a device
> @@ -51,7 +50,7 @@ ArmVirtGetMemoryMap (
>    //
>    mVirtualMemoryTable[0].PhysicalBase = 0x0;
>    mVirtualMemoryTable[0].VirtualBase  = 0x0;
> -  mVirtualMemoryTable[0].Length       = ArmGetPhysAddrTop ();
> +  mVirtualMemoryTable[0].Length       = TopOfAddressSpace;
>    mVirtualMemoryTable[0].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK;
>  
>    mVirtualMemoryTable[1].PhysicalBase = 0x0;
> diff --git a/ArmVirtPkg/Library/XenVirtMemInfoLib/AArch64/PhysAddrTop.S b/ArmVirtPkg/Library/XenVirtMemInfoLib/AArch64/PhysAddrTop.S
> deleted file mode 100644
> index a1f6a194d59b..000000000000
> --- a/ArmVirtPkg/Library/XenVirtMemInfoLib/AArch64/PhysAddrTop.S
> +++ /dev/null
> @@ -1,39 +0,0 @@
> -#
> -#  Copyright (c) 2011-2013, ARM Limited. All rights reserved.
> -#  Copyright (c) 2016-2017, Linaro Limited. All rights reserved.
> -#
> -#  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.
> -#
> -#
> -
> -#include <AsmMacroIoLibV8.h>
> -
> -//EFI_PHYSICAL_ADDRESS
> -//GetPhysAddrTop (
> -//  VOID
> -//  );
> -ASM_FUNC(ArmGetPhysAddrTop)
> -  mrs   x0, id_aa64mmfr0_el1
> -  adr   x1, .LPARanges
> -  and   x0, x0, #7
> -  ldrb  w1, [x1, x0]
> -  mov   x0, #1
> -  lsl   x0, x0, x1
> -  ret
> -
> -//
> -// Bits 0..2 of the AA64MFR0_EL1 system register encode the size of the
> -// physical address space support on this CPU:
> -// 0 == 32 bits, 1 == 36 bits, etc etc
> -// 6 and 7 are reserved
> -//
> -.LPARanges:
> -  .byte 32, 36, 40, 42, 44, 48, -1, -1
> -
> -ASM_FUNCTION_REMOVE_IF_UNREFERENCED
> diff --git a/ArmVirtPkg/Library/XenVirtMemInfoLib/Arm/PhysAddrTop.S b/ArmVirtPkg/Library/XenVirtMemInfoLib/Arm/PhysAddrTop.S
> deleted file mode 100644
> index 9cd81529fb3d..000000000000
> --- a/ArmVirtPkg/Library/XenVirtMemInfoLib/Arm/PhysAddrTop.S
> +++ /dev/null
> @@ -1,24 +0,0 @@
> -#
> -#  Copyright (c) 2011-2013, ARM Limited. All rights reserved.
> -#  Copyright (c) 2014-2017, Linaro Limited. All rights reserved.
> -#
> -#  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.
> -#
> -#
> -
> -#include <AsmMacroIoLib.h>
> -
> -//EFI_PHYSICAL_ADDRESS
> -//GetPhysAddrTop (
> -//  VOID
> -//  );
> -ASM_FUNC(ArmGetPhysAddrTop)
> -  mov   r0, #0x00000000
> -  mov   r1, #0x10000
> -  bx    lr
> 

Re: [PATCH v3 08/16] ArmPkg/ArmMmuLib: take the CPU supported maximum PA space into account

[Philippe Mathieu-Daudé]

On 28/11/18 15:33, Ard Biesheuvel wrote:
> In preparation of dropping PcdPrePiCpuMemorySize entirely, base the
> maximum size of the identity map on the capabilities of the CPU.
> Since that may exceed what is architecturally permitted when using
> 4 KB pages, take MAX_ADDRESS into account as well.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf       |  3 ---
>  ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf        |  3 ---
>  ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 11 +++++++++--
>  3 files changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf b/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
> index b9f264de8d26..246963361e45 100644
> --- a/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
> +++ b/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
> @@ -40,8 +40,5 @@ [LibraryClasses]
>    CacheMaintenanceLib
>    MemoryAllocationLib
>  
> -[Pcd.AARCH64]
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
> -
>  [Pcd.ARM]
>    gArmTokenSpaceGuid.PcdNormalMemoryNonshareableOverride
> diff --git a/ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf b/ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf
> index ecf13f790734..f689c193b862 100644
> --- a/ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf
> +++ b/ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf
> @@ -35,6 +35,3 @@ [LibraryClasses]
>    ArmLib
>    CacheMaintenanceLib
>    MemoryAllocationLib
> -
> -[Pcd.AARCH64]
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
> diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> index 4b62ecb6a476..5403b8d4070e 100644
> --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> @@ -604,8 +604,15 @@ ArmConfigureMmu (
>      return EFI_INVALID_PARAMETER;
>    }
>  
> -  // Cover the entire GCD memory space
> -  MaxAddress = (1UL << PcdGet8 (PcdPrePiCpuMemorySize)) - 1;
> +  //
> +  // Limit the virtual address space to what we can actually use: UEFI
> +  // mandates a 1:1 mapping, so no point in making the virtual address
> +  // space larger than the physical address space. We also have to take
> +  // into account the architectural limitations that result from UEFI's
> +  // use of 4 KB pages.
> +  //
> +  MaxAddress = MIN (LShiftU64 (1ULL, ArmGetPhysicalAddressBits ()) - 1,
> +                    MAX_ADDRESS);
>  
>    // Lookup the Table Level to get the information
>    LookupAddresstoRootTable (MaxAddress, &T0SZ, &RootTableEntryCount);
> 

Re: [PATCH v3 09/16] ArmPkg/CpuPei: base GCD memory space size on CPU's PA range

[Philippe Mathieu-Daudé]

On 28/11/18 15:33, Ard Biesheuvel wrote:
> Derive the size of the GCD memory space map directly from the CPU's
> information registers rather than from the PcdPrePiCpuMemorySize PCD,
> which will be removed.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  ArmPkg/Drivers/CpuPei/CpuPei.inf | 1 -
>  ArmPkg/Drivers/CpuPei/CpuPei.c   | 2 +-
>  2 files changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/ArmPkg/Drivers/CpuPei/CpuPei.inf b/ArmPkg/Drivers/CpuPei/CpuPei.inf
> index eafccd600983..dcea012fd8f9 100644
> --- a/ArmPkg/Drivers/CpuPei/CpuPei.inf
> +++ b/ArmPkg/Drivers/CpuPei/CpuPei.inf
> @@ -50,7 +50,6 @@ [Guids]
>    gArmMpCoreInfoGuid
>  
>  [FixedPcd]
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
>    gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize
>  
>  [Depex]
> diff --git a/ArmPkg/Drivers/CpuPei/CpuPei.c b/ArmPkg/Drivers/CpuPei/CpuPei.c
> index d54f42acfcc8..e63519ff6481 100644
> --- a/ArmPkg/Drivers/CpuPei/CpuPei.c
> +++ b/ArmPkg/Drivers/CpuPei/CpuPei.c
> @@ -73,7 +73,7 @@ InitializeCpuPeim (
>    ArmEnableBranchPrediction ();
>  
>    // Publish the CPU memory and io spaces sizes

This comment is misleading, maybe 'sizes' -> 'addressable bits'?
(not related to this series)

> -  BuildCpuHob (PcdGet8 (PcdPrePiCpuMemorySize), PcdGet8 (PcdPrePiCpuIoSize));
> +  BuildCpuHob (ArmGetPhysicalAddressBits (), PcdGet8 (PcdPrePiCpuIoSize));

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

>  
>    // Only MP Core platform need to produce gArmMpCoreInfoPpiGuid
>    Status = PeiServicesLocatePpi (&gArmMpCoreInfoPpiGuid, 0, NULL, (VOID**)&ArmMpCoreInfoPpi);
> 

Re: [PATCH v3 10/16] ArmPlatformPkg/PrePi: base GCD memory space size on CPU's PA range

[Philippe Mathieu-Daudé]

On 28/11/18 15:33, Ard Biesheuvel wrote:
> Derive the size of the GCD memory space map directly from the CPU's
> information registers rather than from the PcdPrePiCpuMemorySize PCD,
> which will be removed.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  ArmPlatformPkg/PrePi/PeiMPCore.inf  | 1 -
>  ArmPlatformPkg/PrePi/PeiUniCore.inf | 1 -
>  ArmPlatformPkg/PrePi/PrePi.c        | 2 +-
>  3 files changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/ArmPlatformPkg/PrePi/PeiMPCore.inf b/ArmPlatformPkg/PrePi/PeiMPCore.inf
> index 242b03175536..7e2ad6fc483d 100644
> --- a/ArmPlatformPkg/PrePi/PeiMPCore.inf
> +++ b/ArmPlatformPkg/PrePi/PeiMPCore.inf
> @@ -97,7 +97,6 @@ [FixedPcd]
>  
>    gArmPlatformTokenSpaceGuid.PcdCoreCount
>  
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
>    gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize
>  
>    gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIReclaimMemory
> diff --git a/ArmPlatformPkg/PrePi/PeiUniCore.inf b/ArmPlatformPkg/PrePi/PeiUniCore.inf
> index a45cdef4ed91..26328b7e8f67 100644
> --- a/ArmPlatformPkg/PrePi/PeiUniCore.inf
> +++ b/ArmPlatformPkg/PrePi/PeiUniCore.inf
> @@ -90,7 +90,6 @@ [FixedPcd]
>  
>    gArmPlatformTokenSpaceGuid.PcdCoreCount
>  
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
>    gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize
>  
>    gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIReclaimMemory
> diff --git a/ArmPlatformPkg/PrePi/PrePi.c b/ArmPlatformPkg/PrePi/PrePi.c
> index 02cff7ddc204..245bdded1eb3 100644
> --- a/ArmPlatformPkg/PrePi/PrePi.c
> +++ b/ArmPlatformPkg/PrePi/PrePi.c
> @@ -115,7 +115,7 @@ PrePiMain (
>    BuildStackHob (StacksBase, StacksSize);
>  
>    //TODO: Call CpuPei as a library
> -  BuildCpuHob (PcdGet8 (PcdPrePiCpuMemorySize), PcdGet8 (PcdPrePiCpuIoSize));
> +  BuildCpuHob (ArmGetPhysicalAddressBits (), PcdGet8 (PcdPrePiCpuIoSize));
>  
>    if (ArmIsMpCore ()) {
>      // Only MP Core platform need to produce gArmMpCoreInfoPpiGuid
> 

Re: [PATCH v3 11/16] ArmVirtPkg/PrePi: base GCD memory space size on CPU's PA range

[Philippe Mathieu-Daudé]

On 28/11/18 15:33, Ard Biesheuvel wrote:
> Derive the size of the GCD memory space map directly from the CPU's
> information registers rather than from the PcdPrePiCpuMemorySize PCD,
> which will be removed.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf | 1 -
>  ArmVirtPkg/PrePi/PrePi.c                            | 2 +-
>  2 files changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf b/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf
> index 1587bd92f206..034ddb41cb48 100755
> --- a/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf
> +++ b/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf
> @@ -85,7 +85,6 @@ [FixedPcd]
>  
>    gArmPlatformTokenSpaceGuid.PcdCoreCount
>  
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
>    gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize
>  
>    gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIReclaimMemory
> diff --git a/ArmVirtPkg/PrePi/PrePi.c b/ArmVirtPkg/PrePi/PrePi.c
> index f6abe2f2016b..61de6cfd4ae6 100755
> --- a/ArmVirtPkg/PrePi/PrePi.c
> +++ b/ArmVirtPkg/PrePi/PrePi.c
> @@ -80,7 +80,7 @@ PrePiMain (
>    BuildStackHob (StacksBase, StacksSize);
>  
>    //TODO: Call CpuPei as a library
> -  BuildCpuHob (PcdGet8 (PcdPrePiCpuMemorySize), PcdGet8 (PcdPrePiCpuIoSize));
> +  BuildCpuHob (ArmGetPhysicalAddressBits (), PcdGet8 (PcdPrePiCpuIoSize));
>  
>    // Set the Boot Mode
>    SetBootMode (BOOT_WITH_FULL_CONFIGURATION);
> 

Re: [PATCH v3 12/16] BeagleBoardPkg/PrePi: base GCD memory space size on CPU's PA range

[Philippe Mathieu-Daudé]

On 28/11/18 15:33, Ard Biesheuvel wrote:
> Derive the size of the GCD memory space map directly from the CPU's
> information registers rather than from the PcdPrePiCpuMemorySize PCD,
> which will be removed.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  BeagleBoardPkg/PrePi/PeiUniCore.inf | 1 -
>  BeagleBoardPkg/PrePi/PrePi.c        | 2 +-
>  2 files changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/BeagleBoardPkg/PrePi/PeiUniCore.inf b/BeagleBoardPkg/PrePi/PeiUniCore.inf
> index 3d72bc5b46e1..53c71d8eafc2 100644
> --- a/BeagleBoardPkg/PrePi/PeiUniCore.inf
> +++ b/BeagleBoardPkg/PrePi/PeiUniCore.inf
> @@ -86,7 +86,6 @@ [FixedPcd]
>  
>    gArmPlatformTokenSpaceGuid.PcdCoreCount
>  
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
>    gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize
>  
>    gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIReclaimMemory
> diff --git a/BeagleBoardPkg/PrePi/PrePi.c b/BeagleBoardPkg/PrePi/PrePi.c
> index 46f63f40c46e..bc9b0c80b84c 100644
> --- a/BeagleBoardPkg/PrePi/PrePi.c
> +++ b/BeagleBoardPkg/PrePi/PrePi.c
> @@ -110,7 +110,7 @@ PrePiMain (
>    BuildStackHob (StacksBase, StacksSize);
>  
>    //TODO: Call CpuPei as a library
> -  BuildCpuHob (PcdGet8 (PcdPrePiCpuMemorySize), PcdGet8 (PcdPrePiCpuIoSize));
> +  BuildCpuHob (ArmGetPhysicalAddressBits (), PcdGet8 (PcdPrePiCpuIoSize));
>  
>    // Store timer value logged at the beginning of firmware image execution
>    Performance.ResetEnd = GetTimeInNanoSecond (StartTimeStamp);
> 

Re: [PATCH v3 04/16] ArmVirtPkg/QemuVirtMemInfoLib: remove 1:1 mapping of top of PA range

[Philippe Mathieu-Daudé]

On 28/11/18 15:33, Ard Biesheuvel wrote:
> Currently, we map DRAM as EFI_MEMORY_WB, and the remainder of the
> entire virtual address space is mapped with EFI_MEMORY_UC attributes,
> regardless of whether any devices actually reside there.
> 
> Now that we are relaxing the address space limit to more than 40 bits,
> mapping all that address space actually takes up more space in page
> tables than we have so far made available as temporary RAM. So let's
> get rid of the mapping rather than increasing the available RAM, given
> that the mapping is not particularly useful anyway.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf    |  7 ----
>  ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLib.inf |  7 ----
>  ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c      | 25 +++----------
>  ArmVirtPkg/Library/QemuVirtMemInfoLib/AArch64/PhysAddrTop.S     | 39 --------------------
>  ArmVirtPkg/Library/QemuVirtMemInfoLib/Arm/PhysAddrTop.S         | 24 ------------
>  5 files changed, 5 insertions(+), 97 deletions(-)
> 
> diff --git a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf
> index c72a97f9e78a..5c5b841051ad 100644
> --- a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf
> +++ b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf
> @@ -24,12 +24,6 @@ [Defines]
>  [Sources]
>    QemuVirtMemInfoLib.c
>  
> -[Sources.ARM]
> -  Arm/PhysAddrTop.S
> -
> -[Sources.AARCH64]
> -  AArch64/PhysAddrTop.S
> -
>  [Packages]
>    ArmPkg/ArmPkg.dec
>    ArmVirtPkg/ArmVirtPkg.dec
> @@ -51,4 +45,3 @@ [Pcd]
>  
>  [FixedPcd]
>    gArmTokenSpaceGuid.PcdFdSize
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
> diff --git a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLib.inf b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLib.inf
> index e4032d3efb53..d12089760b22 100644
> --- a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLib.inf
> +++ b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLib.inf
> @@ -26,12 +26,6 @@ [Sources]
>    QemuVirtMemInfoLib.c
>    QemuVirtMemInfoPeiLibConstructor.c
>  
> -[Sources.ARM]
> -  Arm/PhysAddrTop.S
> -
> -[Sources.AARCH64]
> -  AArch64/PhysAddrTop.S
> -
>  [Packages]
>    ArmPkg/ArmPkg.dec
>    ArmVirtPkg/ArmVirtPkg.dec
> @@ -55,4 +49,3 @@ [Pcd]
>  [FixedPcd]
>    gArmTokenSpaceGuid.PcdFdSize
>    gArmVirtTokenSpaceGuid.PcdDeviceTreeInitialBaseAddress
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
> diff --git a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c
> index 760bcc169cf4..0285a11b1d77 100644
> --- a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c
> +++ b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c
> @@ -21,11 +21,6 @@
>  // Number of Virtual Memory Map Descriptors
>  #define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS          5
>  
> -EFI_PHYSICAL_ADDRESS
> -ArmGetPhysAddrTop (
> -  VOID
> -  );
> -
>  /**
>    Return the Virtual Memory Map of your platform
>  
> @@ -45,7 +40,6 @@ ArmVirtGetMemoryMap (
>    )
>  {
>    ARM_MEMORY_REGION_DESCRIPTOR  *VirtualMemoryTable;
> -  UINT64                        TopOfMemory;
>  
>    ASSERT (VirtualMemoryMap != NULL);
>  
> @@ -78,23 +72,14 @@ ArmVirtGetMemoryMap (
>    VirtualMemoryTable[1].Length       = VirtualMemoryTable[0].PhysicalBase;
>    VirtualMemoryTable[1].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
>  
> -  // Peripheral space after DRAM
> -  TopOfMemory = MIN (1ULL << FixedPcdGet8 (PcdPrePiCpuMemorySize),
> -                     ArmGetPhysAddrTop ());
> -  VirtualMemoryTable[2].PhysicalBase = VirtualMemoryTable[0].Length + VirtualMemoryTable[1].Length;
> -  VirtualMemoryTable[2].VirtualBase  = VirtualMemoryTable[2].PhysicalBase;
> -  VirtualMemoryTable[2].Length       = TopOfMemory -
> -                                       VirtualMemoryTable[2].PhysicalBase;
> -  VirtualMemoryTable[2].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> -
>    // Remap the FD region as normal executable memory
> -  VirtualMemoryTable[3].PhysicalBase = PcdGet64 (PcdFdBaseAddress);
> -  VirtualMemoryTable[3].VirtualBase  = VirtualMemoryTable[3].PhysicalBase;
> -  VirtualMemoryTable[3].Length       = FixedPcdGet32 (PcdFdSize);
> -  VirtualMemoryTable[3].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK;
> +  VirtualMemoryTable[2].PhysicalBase = PcdGet64 (PcdFdBaseAddress);
> +  VirtualMemoryTable[2].VirtualBase  = VirtualMemoryTable[2].PhysicalBase;
> +  VirtualMemoryTable[2].Length       = FixedPcdGet32 (PcdFdSize);
> +  VirtualMemoryTable[2].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK;
>  
>    // End of Table
> -  ZeroMem (&VirtualMemoryTable[4], sizeof (ARM_MEMORY_REGION_DESCRIPTOR));
> +  ZeroMem (&VirtualMemoryTable[3], sizeof (ARM_MEMORY_REGION_DESCRIPTOR));
>  
>    *VirtualMemoryMap = VirtualMemoryTable;
>  }
> diff --git a/ArmVirtPkg/Library/QemuVirtMemInfoLib/AArch64/PhysAddrTop.S b/ArmVirtPkg/Library/QemuVirtMemInfoLib/AArch64/PhysAddrTop.S
> deleted file mode 100644
> index a1f6a194d59b..000000000000
> --- a/ArmVirtPkg/Library/QemuVirtMemInfoLib/AArch64/PhysAddrTop.S
> +++ /dev/null
> @@ -1,39 +0,0 @@
> -#
> -#  Copyright (c) 2011-2013, ARM Limited. All rights reserved.
> -#  Copyright (c) 2016-2017, Linaro Limited. All rights reserved.
> -#
> -#  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.
> -#
> -#
> -
> -#include <AsmMacroIoLibV8.h>
> -
> -//EFI_PHYSICAL_ADDRESS
> -//GetPhysAddrTop (
> -//  VOID
> -//  );
> -ASM_FUNC(ArmGetPhysAddrTop)
> -  mrs   x0, id_aa64mmfr0_el1
> -  adr   x1, .LPARanges
> -  and   x0, x0, #7
> -  ldrb  w1, [x1, x0]
> -  mov   x0, #1
> -  lsl   x0, x0, x1
> -  ret
> -
> -//
> -// Bits 0..2 of the AA64MFR0_EL1 system register encode the size of the
> -// physical address space support on this CPU:
> -// 0 == 32 bits, 1 == 36 bits, etc etc
> -// 6 and 7 are reserved
> -//
> -.LPARanges:
> -  .byte 32, 36, 40, 42, 44, 48, -1, -1
> -
> -ASM_FUNCTION_REMOVE_IF_UNREFERENCED
> diff --git a/ArmVirtPkg/Library/QemuVirtMemInfoLib/Arm/PhysAddrTop.S b/ArmVirtPkg/Library/QemuVirtMemInfoLib/Arm/PhysAddrTop.S
> deleted file mode 100644
> index 9cd81529fb3d..000000000000
> --- a/ArmVirtPkg/Library/QemuVirtMemInfoLib/Arm/PhysAddrTop.S
> +++ /dev/null
> @@ -1,24 +0,0 @@
> -#
> -#  Copyright (c) 2011-2013, ARM Limited. All rights reserved.
> -#  Copyright (c) 2014-2017, Linaro Limited. All rights reserved.
> -#
> -#  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.
> -#
> -#
> -
> -#include <AsmMacroIoLib.h>
> -
> -//EFI_PHYSICAL_ADDRESS
> -//GetPhysAddrTop (
> -//  VOID
> -//  );
> -ASM_FUNC(ArmGetPhysAddrTop)
> -  mov   r0, #0x00000000
> -  mov   r1, #0x10000
> -  bx    lr
> 

Re: [PATCH v3 01/16] EmbeddedPkg/TemplateSec: remove unused module

[Laszlo Ersek]

On 11/28/18 15:33, Ard Biesheuvel wrote:
> Remove this module: it is unused, and should not be used as an
> example going forward.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  EmbeddedPkg/TemplateSec/TemplateSec.inf | 65 -----------------
>  EmbeddedPkg/TemplateSec/TemplateSec.c   | 76 --------------------
>  2 files changed, 141 deletions(-)

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Re: [PATCH v3 02/16] EmbeddedPkg/PrePiHobLib: drop CreateHobList() from library

[Laszlo Ersek]

On 11/28/18 15:33, Ard Biesheuvel wrote:
> Drop the declaration and the implementation of CreateHoblist(),
> which is not used anywhere.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  EmbeddedPkg/Include/Library/PrePiLib.h | 18 ---------
>  EmbeddedPkg/Library/PrePiHobLib/Hob.c  | 41 --------------------
>  2 files changed, 59 deletions(-)
> 
> diff --git a/EmbeddedPkg/Include/Library/PrePiLib.h b/EmbeddedPkg/Include/Library/PrePiLib.h
> index cf70fca3b619..a857308ecec2 100644
> --- a/EmbeddedPkg/Include/Library/PrePiLib.h
> +++ b/EmbeddedPkg/Include/Library/PrePiLib.h
> @@ -274,24 +274,6 @@ HobConstructor (
>    IN VOID   *EfiFreeMemoryTop
>    );
>  
> -/**
> -  Retrieves the magic value from the PE/COFF header.
> -
> -  @param  Hdr             The buffer in which to return the PE32, PE32+, or TE header.
> -
> -  @return EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC - Image is PE32
> -  @return EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC - Image is PE32+
> -
> -**/
> -VOID
> -CreateHobList (
> -  IN VOID   *MemoryBegin,
> -  IN UINTN  MemoryLength,
> -  IN VOID   *HobBase,
> -  IN VOID   *StackBase
> -  );
> -
> -
>  /**
>    This service enables PEIMs to create various types of HOBs.
>  
> diff --git a/EmbeddedPkg/Library/PrePiHobLib/Hob.c b/EmbeddedPkg/Library/PrePiHobLib/Hob.c
> index aff8ea05797b..ba16899a9184 100644
> --- a/EmbeddedPkg/Library/PrePiHobLib/Hob.c
> +++ b/EmbeddedPkg/Library/PrePiHobLib/Hob.c
> @@ -175,47 +175,6 @@ BuildResourceDescriptorHob (
>    Hob->ResourceLength    = NumberOfBytes;
>  }
>  
> -/**
> -
> -
> -**/
> -VOID
> -CreateHobList (
> -  IN VOID   *MemoryBegin,
> -  IN UINTN  MemoryLength,
> -  IN VOID   *HobBase,
> -  IN VOID   *StackBase
> -  )
> -{
> -  EFI_HOB_HANDOFF_INFO_TABLE  *Hob;
> -  EFI_RESOURCE_ATTRIBUTE_TYPE Attributes;
> -
> -  Hob = HobConstructor (MemoryBegin,MemoryLength,HobBase,StackBase);
> -  SetHobList (Hob);
> -
> -  BuildCpuHob (PcdGet8 (PcdPrePiCpuMemorySize), PcdGet8 (PcdPrePiCpuIoSize));
> -
> -  Attributes =(
> -    EFI_RESOURCE_ATTRIBUTE_PRESENT |
> -    EFI_RESOURCE_ATTRIBUTE_INITIALIZED |
> -    EFI_RESOURCE_ATTRIBUTE_TESTED |
> -    EFI_RESOURCE_ATTRIBUTE_UNCACHEABLE |
> -    EFI_RESOURCE_ATTRIBUTE_WRITE_COMBINEABLE |
> -    EFI_RESOURCE_ATTRIBUTE_WRITE_THROUGH_CACHEABLE |
> -    EFI_RESOURCE_ATTRIBUTE_WRITE_BACK_CACHEABLE
> -  );
> -
> -  BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY, Attributes, (UINTN)MemoryBegin, MemoryLength);
> -
> -  BuildStackHob ((EFI_PHYSICAL_ADDRESS)(UINTN)StackBase, ((UINTN)MemoryBegin + MemoryLength) - (UINTN)StackBase);
> -
> -  if (FeaturePcdGet (PcdPrePiProduceMemoryTypeInformationHob)) {
> -    // Optional feature that helps prevent EFI memory map fragmentation.
> -    BuildMemoryTypeInformationHob ();
> -  }
> -}
> -
> -
>  VOID
>  EFIAPI
>  BuildFvHobs (
> 

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Re: [PATCH v3 03/16] ArmVirtPkg/FdtPciHostBridgeLib: map ECAM and I/O spaces in GCD memory map

[Laszlo Ersek]

On 11/28/18 15:33, Ard Biesheuvel wrote:
> Up until now, we have been getting away with not declaring the ECAM
> and translated I/O spaces at all in the GCD memory map, simply because
> we map the entire address space with device attributes in the early PEI
> code, and so the ECAM space will be mapped wherever it ends up.
> 
> Now that we are about to make changes to how ArmVirtQemu reasons
> about the size of the address space, it would be better to get rid
> of this mapping of the entire address space, since it can get
> arbitrarily large without real benefit.
> 
> So start by mapping the ECAM and translated I/O spaces explicitly,
> instead of relying on the early PEI mapping.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> ---
>  ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf |  1 +
>  ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c   | 46 +++++++++++++++++++-
>  2 files changed, 46 insertions(+), 1 deletion(-)
> 
> diff --git a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf
> index 0995f4b7a156..4011336a353b 100644
> --- a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf
> +++ b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf
> @@ -42,6 +42,7 @@ [Packages]
>  [LibraryClasses]
>    DebugLib
>    DevicePathLib
> +  DxeServicesTableLib
>    MemoryAllocationLib
>    PciPcdProducerLib
>  
> diff --git a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
> index 5b9c887db35d..ebfa14a349f4 100644
> --- a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
> +++ b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
> @@ -17,6 +17,7 @@
>  #include <Library/PciHostBridgeLib.h>
>  #include <Library/DebugLib.h>
>  #include <Library/DevicePathLib.h>
> +#include <Library/DxeServicesTableLib.h>
>  #include <Library/MemoryAllocationLib.h>
>  #include <Library/PcdLib.h>
>  #include <Library/UefiBootServicesTableLib.h>
> @@ -82,6 +83,33 @@ typedef struct {
>  #define DTB_PCI_HOST_RANGE_IO           BIT24
>  #define DTB_PCI_HOST_RANGE_TYPEMASK     (BIT31 | BIT30 | BIT29 | BIT25 | BIT24)
>  
> +STATIC
> +EFI_STATUS
> +MapGcdMmioSpace (
> +  IN    UINT64    Base,
> +  IN    UINT64    Size
> +  )
> +{
> +  EFI_STATUS    Status;
> +
> +  Status = gDS->AddMemorySpace (EfiGcdMemoryTypeMemoryMappedIo, Base, Size,
> +                  EFI_MEMORY_UC);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR,
> +      "%a: failed to add GCD memory space for region [0x%Lx+0x%Lx)\n",
> +      __FUNCTION__, Base, Size));
> +    return Status;
> +  }
> +
> +  Status = gDS->SetMemorySpaceAttributes (Base, Size, EFI_MEMORY_UC);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR,
> +      "%a: failed to set memory space attributes for region [0x%Lx+0x%Lx)\n",
> +      __FUNCTION__, Base, Size));
> +  }
> +  return Status;
> +}
> +
>  STATIC
>  EFI_STATUS
>  ProcessPciHost (
> @@ -266,7 +294,23 @@ ProcessPciHost (
>      "Io[0x%Lx+0x%Lx)@0x%Lx Mem32[0x%Lx+0x%Lx)@0x0 Mem64[0x%Lx+0x%Lx)@0x0\n",
>      __FUNCTION__, ConfigBase, ConfigSize, *BusMin, *BusMax, *IoBase, *IoSize,
>      IoTranslation, *Mmio32Base, *Mmio32Size, *Mmio64Base, *Mmio64Size));
> -  return EFI_SUCCESS;
> +
> +  // Map the ECAM space in the GCD memory map
> +  Status = MapGcdMmioSpace (ConfigBase, ConfigSize);
> +  ASSERT_EFI_ERROR (Status);
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  //
> +  // Map the MMIO window that provides I/O access - the PCI host bridge code
> +  // is not aware of this translation and so it will only map the I/O view
> +  // in the GCD I/O map.
> +  //
> +  Status = MapGcdMmioSpace (*IoBase + IoTranslation, *IoSize);
> +  ASSERT_EFI_ERROR (Status);
> +
> +  return Status;
>  }
>  
>  STATIC PCI_ROOT_BRIDGE mRootBridge;
> 

looks good, thanks!

Re: [PATCH v3 04/16] ArmVirtPkg/QemuVirtMemInfoLib: remove 1:1 mapping of top of PA range

[Laszlo Ersek]

On 11/28/18 15:33, Ard Biesheuvel wrote:
> Currently, we map DRAM as EFI_MEMORY_WB, and the remainder of the
> entire virtual address space is mapped with EFI_MEMORY_UC attributes,
> regardless of whether any devices actually reside there.
> 
> Now that we are relaxing the address space limit to more than 40 bits,
> mapping all that address space actually takes up more space in page
> tables than we have so far made available as temporary RAM. So let's
> get rid of the mapping rather than increasing the available RAM, given
> that the mapping is not particularly useful anyway.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf    |  7 ----
>  ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLib.inf |  7 ----
>  ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c      | 25 +++----------
>  ArmVirtPkg/Library/QemuVirtMemInfoLib/AArch64/PhysAddrTop.S     | 39 --------------------
>  ArmVirtPkg/Library/QemuVirtMemInfoLib/Arm/PhysAddrTop.S         | 24 ------------
>  5 files changed, 5 insertions(+), 97 deletions(-)

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks!
Laszlo

> 
> diff --git a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf
> index c72a97f9e78a..5c5b841051ad 100644
> --- a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf
> +++ b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf
> @@ -24,12 +24,6 @@ [Defines]
>  [Sources]
>    QemuVirtMemInfoLib.c
>  
> -[Sources.ARM]
> -  Arm/PhysAddrTop.S
> -
> -[Sources.AARCH64]
> -  AArch64/PhysAddrTop.S
> -
>  [Packages]
>    ArmPkg/ArmPkg.dec
>    ArmVirtPkg/ArmVirtPkg.dec
> @@ -51,4 +45,3 @@ [Pcd]
>  
>  [FixedPcd]
>    gArmTokenSpaceGuid.PcdFdSize
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
> diff --git a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLib.inf b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLib.inf
> index e4032d3efb53..d12089760b22 100644
> --- a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLib.inf
> +++ b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLib.inf
> @@ -26,12 +26,6 @@ [Sources]
>    QemuVirtMemInfoLib.c
>    QemuVirtMemInfoPeiLibConstructor.c
>  
> -[Sources.ARM]
> -  Arm/PhysAddrTop.S
> -
> -[Sources.AARCH64]
> -  AArch64/PhysAddrTop.S
> -
>  [Packages]
>    ArmPkg/ArmPkg.dec
>    ArmVirtPkg/ArmVirtPkg.dec
> @@ -55,4 +49,3 @@ [Pcd]
>  [FixedPcd]
>    gArmTokenSpaceGuid.PcdFdSize
>    gArmVirtTokenSpaceGuid.PcdDeviceTreeInitialBaseAddress
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
> diff --git a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c
> index 760bcc169cf4..0285a11b1d77 100644
> --- a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c
> +++ b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c
> @@ -21,11 +21,6 @@
>  // Number of Virtual Memory Map Descriptors
>  #define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS          5
>  
> -EFI_PHYSICAL_ADDRESS
> -ArmGetPhysAddrTop (
> -  VOID
> -  );
> -
>  /**
>    Return the Virtual Memory Map of your platform
>  
> @@ -45,7 +40,6 @@ ArmVirtGetMemoryMap (
>    )
>  {
>    ARM_MEMORY_REGION_DESCRIPTOR  *VirtualMemoryTable;
> -  UINT64                        TopOfMemory;
>  
>    ASSERT (VirtualMemoryMap != NULL);
>  
> @@ -78,23 +72,14 @@ ArmVirtGetMemoryMap (
>    VirtualMemoryTable[1].Length       = VirtualMemoryTable[0].PhysicalBase;
>    VirtualMemoryTable[1].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
>  
> -  // Peripheral space after DRAM
> -  TopOfMemory = MIN (1ULL << FixedPcdGet8 (PcdPrePiCpuMemorySize),
> -                     ArmGetPhysAddrTop ());
> -  VirtualMemoryTable[2].PhysicalBase = VirtualMemoryTable[0].Length + VirtualMemoryTable[1].Length;
> -  VirtualMemoryTable[2].VirtualBase  = VirtualMemoryTable[2].PhysicalBase;
> -  VirtualMemoryTable[2].Length       = TopOfMemory -
> -                                       VirtualMemoryTable[2].PhysicalBase;
> -  VirtualMemoryTable[2].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> -
>    // Remap the FD region as normal executable memory
> -  VirtualMemoryTable[3].PhysicalBase = PcdGet64 (PcdFdBaseAddress);
> -  VirtualMemoryTable[3].VirtualBase  = VirtualMemoryTable[3].PhysicalBase;
> -  VirtualMemoryTable[3].Length       = FixedPcdGet32 (PcdFdSize);
> -  VirtualMemoryTable[3].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK;
> +  VirtualMemoryTable[2].PhysicalBase = PcdGet64 (PcdFdBaseAddress);
> +  VirtualMemoryTable[2].VirtualBase  = VirtualMemoryTable[2].PhysicalBase;
> +  VirtualMemoryTable[2].Length       = FixedPcdGet32 (PcdFdSize);
> +  VirtualMemoryTable[2].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK;
>  
>    // End of Table
> -  ZeroMem (&VirtualMemoryTable[4], sizeof (ARM_MEMORY_REGION_DESCRIPTOR));
> +  ZeroMem (&VirtualMemoryTable[3], sizeof (ARM_MEMORY_REGION_DESCRIPTOR));
>  
>    *VirtualMemoryMap = VirtualMemoryTable;
>  }
> diff --git a/ArmVirtPkg/Library/QemuVirtMemInfoLib/AArch64/PhysAddrTop.S b/ArmVirtPkg/Library/QemuVirtMemInfoLib/AArch64/PhysAddrTop.S
> deleted file mode 100644
> index a1f6a194d59b..000000000000
> --- a/ArmVirtPkg/Library/QemuVirtMemInfoLib/AArch64/PhysAddrTop.S
> +++ /dev/null
> @@ -1,39 +0,0 @@
> -#
> -#  Copyright (c) 2011-2013, ARM Limited. All rights reserved.
> -#  Copyright (c) 2016-2017, Linaro Limited. All rights reserved.
> -#
> -#  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.
> -#
> -#
> -
> -#include <AsmMacroIoLibV8.h>
> -
> -//EFI_PHYSICAL_ADDRESS
> -//GetPhysAddrTop (
> -//  VOID
> -//  );
> -ASM_FUNC(ArmGetPhysAddrTop)
> -  mrs   x0, id_aa64mmfr0_el1
> -  adr   x1, .LPARanges
> -  and   x0, x0, #7
> -  ldrb  w1, [x1, x0]
> -  mov   x0, #1
> -  lsl   x0, x0, x1
> -  ret
> -
> -//
> -// Bits 0..2 of the AA64MFR0_EL1 system register encode the size of the
> -// physical address space support on this CPU:
> -// 0 == 32 bits, 1 == 36 bits, etc etc
> -// 6 and 7 are reserved
> -//
> -.LPARanges:
> -  .byte 32, 36, 40, 42, 44, 48, -1, -1
> -
> -ASM_FUNCTION_REMOVE_IF_UNREFERENCED
> diff --git a/ArmVirtPkg/Library/QemuVirtMemInfoLib/Arm/PhysAddrTop.S b/ArmVirtPkg/Library/QemuVirtMemInfoLib/Arm/PhysAddrTop.S
> deleted file mode 100644
> index 9cd81529fb3d..000000000000
> --- a/ArmVirtPkg/Library/QemuVirtMemInfoLib/Arm/PhysAddrTop.S
> +++ /dev/null
> @@ -1,24 +0,0 @@
> -#
> -#  Copyright (c) 2011-2013, ARM Limited. All rights reserved.
> -#  Copyright (c) 2014-2017, Linaro Limited. All rights reserved.
> -#
> -#  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.
> -#
> -#
> -
> -#include <AsmMacroIoLib.h>
> -
> -//EFI_PHYSICAL_ADDRESS
> -//GetPhysAddrTop (
> -//  VOID
> -//  );
> -ASM_FUNC(ArmGetPhysAddrTop)
> -  mov   r0, #0x00000000
> -  mov   r1, #0x10000
> -  bx    lr
> 

Re: [PATCH v3 05/16] MdePkg/ProcessorBind.h AARCH64: limit MAX_ADDRESS to 48 bits

[Laszlo Ersek]

On 11/28/18 15:33, Ard Biesheuvel wrote:
> AArch64 supports the use of more than 48 bits for physical and/or
> virtual addressing, but only if the page size is set to 64 KB,
> which is not supported by UEFI. So redefine MAX_ADDRESS to cover
> only 48 address bits.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
> ---
>  MdePkg/Include/AArch64/ProcessorBind.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/MdePkg/Include/AArch64/ProcessorBind.h b/MdePkg/Include/AArch64/ProcessorBind.h
> index 968c18f915ae..dad75df1c579 100644
> --- a/MdePkg/Include/AArch64/ProcessorBind.h
> +++ b/MdePkg/Include/AArch64/ProcessorBind.h
> @@ -138,9 +138,9 @@ typedef INT64   INTN;
>  #define MAX_2_BITS  0xC000000000000000ULL
>  
>  ///
> -/// Maximum legal AARCH64  address
> +/// Maximum legal AARCH64  address (48 bits for 4 KB page size)
>  ///
> -#define MAX_ADDRESS   0xFFFFFFFFFFFFFFFFULL
> +#define MAX_ADDRESS   0xFFFFFFFFFFFFULL
>  
>  ///
>  /// Maximum legal AArch64 INTN and UINTN values.
> 

Hmmm.

I bit the bullet and grepped the tree for MAX_ADDRESS.

The amount of hits is staggering. I can't audit all of them.

Generally, MAX_ADDRESS seems to be used in checks that prevent address
wrap-around. In that regard, this change looks valid.

I can't guarantee this change won't regress anything though. In the
previous posting of this patch, I asked Liming some questions (IIRC):

http://mid.mail-archive.com/6f1209fb-bb89-a70f-ba0e-3ebf2e12e459@redhat.com

It would be nice to see answers. :)

In addition:

(a) in "BaseTools/Source/C/Include/AArch64/ProcessorBind.h", we have
another instance of the macro definition. I suspect it should be kept in
sync.

(b) in "BaseTools/Source/C/Common/CommonLib.h", we have:

#define MAX_UINTN MAX_ADDRESS

which I think relies on (a), and hence it will be amusingly wrong after
we synchronize (a) with MdePkg.

(BTW, (b) is exactly the kind of assumption that scares me about this
patch.)


We're not much past the last stable tag (edk2-stable201811), so let's
hope there's going to be enough time to catch any regressions.

With (a) and (b) investigated / fixed up, I'd be willing to A-b this.
Cautiously :)

Anyway, this is for MdePkg, so my review is not required. (I certainly
do not intend to *oppose* this patch.)

Thanks
Laszlo

Re: [PATCH v3 06/16] ArmPkg/ArmLib: add support for reading the max physical address space size

[Laszlo Ersek]

On 11/28/18 15:33, Ard Biesheuvel wrote:
> Add a helper function that returns the maximum physical address space
> size as supported by the current CPU.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  ArmPkg/Include/Library/ArmLib.h               |  6 ++++++
>  ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S | 17 +++++++++++++++++
>  ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S     |  8 ++++++++
>  ArmPkg/Library/ArmLib/Arm/ArmLibSupport.asm   |  8 ++++++++
>  4 files changed, 39 insertions(+)
> 
> diff --git a/ArmPkg/Include/Library/ArmLib.h b/ArmPkg/Include/Library/ArmLib.h
> index ffda50e9d767..9a804c15fdb6 100644
> --- a/ArmPkg/Include/Library/ArmLib.h
> +++ b/ArmPkg/Include/Library/ArmLib.h
> @@ -733,4 +733,10 @@ ArmWriteCntvOff (
>    UINT64   Val
>    );
>  
> +UINTN
> +EFIAPI
> +ArmGetPhysicalAddressBits (
> +  VOID
> +  );
> +
>  #endif // __ARM_LIB__
> diff --git a/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S b/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
> index 1ef2f61f5979..b7173e00b039 100644
> --- a/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
> +++ b/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
> @@ -196,4 +196,21 @@ ASM_FUNC(ArmWriteSctlr)
>  3:msr   sctlr_el3, x0
>  4:ret
>  
> +ASM_FUNC(ArmGetPhysicalAddressBits)
> +  mrs   x0, id_aa64mmfr0_el1
> +  adr   x1, .LPARanges
> +  and   x0, x0, #0xf
> +  ldrb  w0, [x1, x0]
> +  ret
> +
> +//
> +// Bits 0..3 of the AA64MFR0_EL1 system register encode the size of the
> +// physical address space support on this CPU:
> +// 0 == 32 bits, 1 == 36 bits, etc etc
> +// 7 and up are reserved
> +//
> +.LPARanges:
> +  .byte 32, 36, 40, 42, 44, 48, 52,  0
> +  .byte  0,  0,  0,  0,  0,  0,  0,  0
> +
>  ASM_FUNCTION_REMOVE_IF_UNREFERENCED
> diff --git a/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S b/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S
> index f2a517671f0a..0e9f9d0453e4 100644
> --- a/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S
> +++ b/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S
> @@ -165,4 +165,12 @@ ASM_FUNC(ArmWriteCpuActlr)
>    isb
>    bx      lr
>  
> +ASM_FUNC (ArmGetPhysicalAddressBits)
> +  mrc     p15, 0, r0, c0, c1, 4   // MMFR0
> +  and     r0, r0, #0xf            // VMSA [3:0]
> +  cmp     r0, #5                  // >= 5 implies LPAE support
> +  movlt   r0, #32                 // 32 bits if no LPAE
> +  movge   r0, #40                 // 40 bits if LPAE
> +  bx      lr
> +
>  ASM_FUNCTION_REMOVE_IF_UNREFERENCED
> diff --git a/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.asm b/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.asm
> index 219140c22b13..3eb52875971d 100644
> --- a/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.asm
> +++ b/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.asm
> @@ -169,4 +169,12 @@
>    isb
>    bx      lr
>  
> + RVCT_ASM_EXPORT ArmGetPhysicalAddressBits
> +  mrc     p15, 0, r0, c0, c1, 4   ; MMFR0
> +  and     r0, r0, #0xf            ; VMSA [3:0]
> +  cmp     r0, #5                  ; >= 5 implies LPAE support
> +  movlt   r0, #32                 ; 32 bits if no LPAE
> +  movge   r0, #40                 ; 40 bits if LPAE
> +  bx      lr
> +
>    END
> 

I didn't review the assembly code, but formally, the patch looks OK to me.

Acked-by: Laszlo Ersek <lersek@redhat.com>

Re: [PATCH v3 07/16] ArmVirtPkg/XenVirtMemInfoLib: refactor reading of the PA space size

[Laszlo Ersek]

On 11/28/18 15:33, Ard Biesheuvel wrote:
> Use the new ArmLib helper to read the CPU's physical address limit
> so we can drop our own homecooked one.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  ArmVirtPkg/Library/XenVirtMemInfoLib/XenVirtMemInfoLib.inf |  6 ---
>  ArmVirtPkg/Library/XenVirtMemInfoLib/XenVirtMemInfoLib.c   | 11 +++---
>  ArmVirtPkg/Library/XenVirtMemInfoLib/AArch64/PhysAddrTop.S | 39 --------------------
>  ArmVirtPkg/Library/XenVirtMemInfoLib/Arm/PhysAddrTop.S     | 24 ------------
>  4 files changed, 5 insertions(+), 75 deletions(-)

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks
Laszlo

> 
> diff --git a/ArmVirtPkg/Library/XenVirtMemInfoLib/XenVirtMemInfoLib.inf b/ArmVirtPkg/Library/XenVirtMemInfoLib/XenVirtMemInfoLib.inf
> index cd4c805a4db9..ae107810e927 100644
> --- a/ArmVirtPkg/Library/XenVirtMemInfoLib/XenVirtMemInfoLib.inf
> +++ b/ArmVirtPkg/Library/XenVirtMemInfoLib/XenVirtMemInfoLib.inf
> @@ -24,12 +24,6 @@ [Defines]
>  [Sources]
>    XenVirtMemInfoLib.c
>  
> -[Sources.ARM]
> -  Arm/PhysAddrTop.S
> -
> -[Sources.AARCH64]
> -  AArch64/PhysAddrTop.S
> -
>  [Packages]
>    ArmPkg/ArmPkg.dec
>    ArmVirtPkg/ArmVirtPkg.dec
> diff --git a/ArmVirtPkg/Library/XenVirtMemInfoLib/XenVirtMemInfoLib.c b/ArmVirtPkg/Library/XenVirtMemInfoLib/XenVirtMemInfoLib.c
> index 88ff3167cbfd..6701dec50ea8 100644
> --- a/ArmVirtPkg/Library/XenVirtMemInfoLib/XenVirtMemInfoLib.c
> +++ b/ArmVirtPkg/Library/XenVirtMemInfoLib/XenVirtMemInfoLib.c
> @@ -18,11 +18,6 @@
>  
>  STATIC ARM_MEMORY_REGION_DESCRIPTOR  mVirtualMemoryTable[2];
>  
> -EFI_PHYSICAL_ADDRESS
> -ArmGetPhysAddrTop (
> -  VOID
> -  );
> -
>  /**
>    Return the Virtual Memory Map of your platform
>  
> @@ -42,8 +37,12 @@ ArmVirtGetMemoryMap (
>    OUT ARM_MEMORY_REGION_DESCRIPTOR   **VirtualMemoryMap
>    )
>  {
> +  EFI_PHYSICAL_ADDRESS TopOfAddressSpace;
> +
>    ASSERT (VirtualMemoryMap != NULL);
>  
> +  TopOfAddressSpace = LShiftU64 (1ULL, ArmGetPhysicalAddressBits ());
> +
>    //
>    // Map the entire physical memory space as cached. The only device
>    // we care about is the GIC, which will be stage 2 mapped as a device
> @@ -51,7 +50,7 @@ ArmVirtGetMemoryMap (
>    //
>    mVirtualMemoryTable[0].PhysicalBase = 0x0;
>    mVirtualMemoryTable[0].VirtualBase  = 0x0;
> -  mVirtualMemoryTable[0].Length       = ArmGetPhysAddrTop ();
> +  mVirtualMemoryTable[0].Length       = TopOfAddressSpace;
>    mVirtualMemoryTable[0].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK;
>  
>    mVirtualMemoryTable[1].PhysicalBase = 0x0;
> diff --git a/ArmVirtPkg/Library/XenVirtMemInfoLib/AArch64/PhysAddrTop.S b/ArmVirtPkg/Library/XenVirtMemInfoLib/AArch64/PhysAddrTop.S
> deleted file mode 100644
> index a1f6a194d59b..000000000000
> --- a/ArmVirtPkg/Library/XenVirtMemInfoLib/AArch64/PhysAddrTop.S
> +++ /dev/null
> @@ -1,39 +0,0 @@
> -#
> -#  Copyright (c) 2011-2013, ARM Limited. All rights reserved.
> -#  Copyright (c) 2016-2017, Linaro Limited. All rights reserved.
> -#
> -#  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.
> -#
> -#
> -
> -#include <AsmMacroIoLibV8.h>
> -
> -//EFI_PHYSICAL_ADDRESS
> -//GetPhysAddrTop (
> -//  VOID
> -//  );
> -ASM_FUNC(ArmGetPhysAddrTop)
> -  mrs   x0, id_aa64mmfr0_el1
> -  adr   x1, .LPARanges
> -  and   x0, x0, #7
> -  ldrb  w1, [x1, x0]
> -  mov   x0, #1
> -  lsl   x0, x0, x1
> -  ret
> -
> -//
> -// Bits 0..2 of the AA64MFR0_EL1 system register encode the size of the
> -// physical address space support on this CPU:
> -// 0 == 32 bits, 1 == 36 bits, etc etc
> -// 6 and 7 are reserved
> -//
> -.LPARanges:
> -  .byte 32, 36, 40, 42, 44, 48, -1, -1
> -
> -ASM_FUNCTION_REMOVE_IF_UNREFERENCED
> diff --git a/ArmVirtPkg/Library/XenVirtMemInfoLib/Arm/PhysAddrTop.S b/ArmVirtPkg/Library/XenVirtMemInfoLib/Arm/PhysAddrTop.S
> deleted file mode 100644
> index 9cd81529fb3d..000000000000
> --- a/ArmVirtPkg/Library/XenVirtMemInfoLib/Arm/PhysAddrTop.S
> +++ /dev/null
> @@ -1,24 +0,0 @@
> -#
> -#  Copyright (c) 2011-2013, ARM Limited. All rights reserved.
> -#  Copyright (c) 2014-2017, Linaro Limited. All rights reserved.
> -#
> -#  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.
> -#
> -#
> -
> -#include <AsmMacroIoLib.h>
> -
> -//EFI_PHYSICAL_ADDRESS
> -//GetPhysAddrTop (
> -//  VOID
> -//  );
> -ASM_FUNC(ArmGetPhysAddrTop)
> -  mov   r0, #0x00000000
> -  mov   r1, #0x10000
> -  bx    lr
> 

Re: [PATCH v3 08/16] ArmPkg/ArmMmuLib: take the CPU supported maximum PA space into account

[Laszlo Ersek]

On 11/28/18 15:33, Ard Biesheuvel wrote:
> In preparation of dropping PcdPrePiCpuMemorySize entirely, base the
> maximum size of the identity map on the capabilities of the CPU.
> Since that may exceed what is architecturally permitted when using
> 4 KB pages, take MAX_ADDRESS into account as well.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf       |  3 ---
>  ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf        |  3 ---
>  ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 11 +++++++++--
>  3 files changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf b/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
> index b9f264de8d26..246963361e45 100644
> --- a/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
> +++ b/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
> @@ -40,8 +40,5 @@ [LibraryClasses]
>    CacheMaintenanceLib
>    MemoryAllocationLib
>
> -[Pcd.AARCH64]
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
> -
>  [Pcd.ARM]
>    gArmTokenSpaceGuid.PcdNormalMemoryNonshareableOverride
> diff --git a/ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf b/ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf
> index ecf13f790734..f689c193b862 100644
> --- a/ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf
> +++ b/ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf
> @@ -35,6 +35,3 @@ [LibraryClasses]
>    ArmLib
>    CacheMaintenanceLib
>    MemoryAllocationLib
> -
> -[Pcd.AARCH64]
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize

OK

> diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> index 4b62ecb6a476..5403b8d4070e 100644
> --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> @@ -604,8 +604,15 @@ ArmConfigureMmu (
>      return EFI_INVALID_PARAMETER;
>    }
>
> -  // Cover the entire GCD memory space
> -  MaxAddress = (1UL << PcdGet8 (PcdPrePiCpuMemorySize)) - 1;
> +  //
> +  // Limit the virtual address space to what we can actually use: UEFI
> +  // mandates a 1:1 mapping, so no point in making the virtual address
> +  // space larger than the physical address space. We also have to take
> +  // into account the architectural limitations that result from UEFI's
> +  // use of 4 KB pages.
> +  //
> +  MaxAddress = MIN (LShiftU64 (1ULL, ArmGetPhysicalAddressBits ()) - 1,
> +                    MAX_ADDRESS);
>
>    // Lookup the Table Level to get the information
>    LookupAddresstoRootTable (MaxAddress, &T0SZ, &RootTableEntryCount);
>

This part of edk2 (or ArmVirtQemu) is where I *always* get lost. So let
me check the call tree.

  ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf [ArmVirtPkg/ArmVirtQemu.dsc]
    InitializeMemory()                            [ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c]
      PeiServicesInstallPeiMemory()               [...]
      // now we've got permanent PEI RAM
      MemoryPeim()                                [ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c]
        InitMmu()                                 [ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c]
          ArmVirtGetMemoryMap()                   [ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c]
          ArmConfigureMmu()                       [ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c]

In patch v3 04/16, we modify ArmVirtGetMemoryMap() so that the mapping
not cover the "Peripheral space after DRAM" (which is practically
"limitless"). Good.

In this patch, we have to modify ArmConfigureMmu() as well, because the
descriptor array that we pass it from ArmVirtGetMemoryMap() does not
contain all the information that is necessary for setting up "paging" in
general. Also considering mappings that could be added later during DXE,
such as MMIO ranges of platform drivers / devices, discontiguous RAM
ranges, and so on.

So we need a "maximum" here. The maximum that we choose doesn't
"seriously" translate to an increased memory footprint of paging
artifacts, *unlike* the size of the address space that we actually map
for access. Hence in patch v3 04/16, we are conservative, but in this
patch, when selecting the maximum, we go as high as we can -- either the
architecturally permitted limit (with 4KB page size), or, if the latter
is lower, the limit supported by the current CPU.

The GCD *could* go higher (even though we'd never attempt to map that in
UEFI), assuming the CPU is capable enough (using 64KB pages at OS
runtime).

This bends my brain. :) I'm now slowly comprehending your blurb. Indeed,
the rest of the series pertains to the GCD memory space map.

Nit: the second argument of the MIN() macro is not indented
idiomatically. With that fixed (no need to repost the series just
because of it):

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks
Laszlo

Re: [PATCH v3 09/16] ArmPkg/CpuPei: base GCD memory space size on CPU's PA range

[Laszlo Ersek]

On 11/28/18 15:33, Ard Biesheuvel wrote:
> Derive the size of the GCD memory space map directly from the CPU's
> information registers rather than from the PcdPrePiCpuMemorySize PCD,
> which will be removed.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  ArmPkg/Drivers/CpuPei/CpuPei.inf | 1 -
>  ArmPkg/Drivers/CpuPei/CpuPei.c   | 2 +-
>  2 files changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/ArmPkg/Drivers/CpuPei/CpuPei.inf b/ArmPkg/Drivers/CpuPei/CpuPei.inf
> index eafccd600983..dcea012fd8f9 100644
> --- a/ArmPkg/Drivers/CpuPei/CpuPei.inf
> +++ b/ArmPkg/Drivers/CpuPei/CpuPei.inf
> @@ -50,7 +50,6 @@ [Guids]
>    gArmMpCoreInfoGuid
>  
>  [FixedPcd]
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
>    gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize
>  
>  [Depex]
> diff --git a/ArmPkg/Drivers/CpuPei/CpuPei.c b/ArmPkg/Drivers/CpuPei/CpuPei.c
> index d54f42acfcc8..e63519ff6481 100644
> --- a/ArmPkg/Drivers/CpuPei/CpuPei.c
> +++ b/ArmPkg/Drivers/CpuPei/CpuPei.c
> @@ -73,7 +73,7 @@ InitializeCpuPeim (
>    ArmEnableBranchPrediction ();
>  
>    // Publish the CPU memory and io spaces sizes
> -  BuildCpuHob (PcdGet8 (PcdPrePiCpuMemorySize), PcdGet8 (PcdPrePiCpuIoSize));
> +  BuildCpuHob (ArmGetPhysicalAddressBits (), PcdGet8 (PcdPrePiCpuIoSize));
>  
>    // Only MP Core platform need to produce gArmMpCoreInfoPpiGuid
>    Status = PeiServicesLocatePpi (&gArmMpCoreInfoPpiGuid, 0, NULL, (VOID**)&ArmMpCoreInfoPpi);
> 

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Re: [PATCH v3 11/16] ArmVirtPkg/PrePi: base GCD memory space size on CPU's PA range

[Laszlo Ersek]

On 11/28/18 15:33, Ard Biesheuvel wrote:
> Derive the size of the GCD memory space map directly from the CPU's
> information registers rather than from the PcdPrePiCpuMemorySize PCD,
> which will be removed.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf | 1 -
>  ArmVirtPkg/PrePi/PrePi.c                            | 2 +-
>  2 files changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf b/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf
> index 1587bd92f206..034ddb41cb48 100755
> --- a/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf
> +++ b/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf
> @@ -85,7 +85,6 @@ [FixedPcd]
>  
>    gArmPlatformTokenSpaceGuid.PcdCoreCount
>  
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
>    gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize
>  
>    gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIReclaimMemory
> diff --git a/ArmVirtPkg/PrePi/PrePi.c b/ArmVirtPkg/PrePi/PrePi.c
> index f6abe2f2016b..61de6cfd4ae6 100755
> --- a/ArmVirtPkg/PrePi/PrePi.c
> +++ b/ArmVirtPkg/PrePi/PrePi.c
> @@ -80,7 +80,7 @@ PrePiMain (
>    BuildStackHob (StacksBase, StacksSize);
>  
>    //TODO: Call CpuPei as a library
> -  BuildCpuHob (PcdGet8 (PcdPrePiCpuMemorySize), PcdGet8 (PcdPrePiCpuIoSize));
> +  BuildCpuHob (ArmGetPhysicalAddressBits (), PcdGet8 (PcdPrePiCpuIoSize));
>  
>    // Set the Boot Mode
>    SetBootMode (BOOT_WITH_FULL_CONFIGURATION);
> 

Acked-by: Laszlo Ersek <lersek@redhat.com>

Re: [PATCH v3 12/16] BeagleBoardPkg/PrePi: base GCD memory space size on CPU's PA range

[Laszlo Ersek]

On 11/28/18 15:33, Ard Biesheuvel wrote:
> Derive the size of the GCD memory space map directly from the CPU's
> information registers rather than from the PcdPrePiCpuMemorySize PCD,
> which will be removed.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  BeagleBoardPkg/PrePi/PeiUniCore.inf | 1 -
>  BeagleBoardPkg/PrePi/PrePi.c        | 2 +-
>  2 files changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/BeagleBoardPkg/PrePi/PeiUniCore.inf b/BeagleBoardPkg/PrePi/PeiUniCore.inf
> index 3d72bc5b46e1..53c71d8eafc2 100644
> --- a/BeagleBoardPkg/PrePi/PeiUniCore.inf
> +++ b/BeagleBoardPkg/PrePi/PeiUniCore.inf
> @@ -86,7 +86,6 @@ [FixedPcd]
>  
>    gArmPlatformTokenSpaceGuid.PcdCoreCount
>  
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
>    gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize
>  
>    gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIReclaimMemory
> diff --git a/BeagleBoardPkg/PrePi/PrePi.c b/BeagleBoardPkg/PrePi/PrePi.c
> index 46f63f40c46e..bc9b0c80b84c 100644
> --- a/BeagleBoardPkg/PrePi/PrePi.c
> +++ b/BeagleBoardPkg/PrePi/PrePi.c
> @@ -110,7 +110,7 @@ PrePiMain (
>    BuildStackHob (StacksBase, StacksSize);
>  
>    //TODO: Call CpuPei as a library
> -  BuildCpuHob (PcdGet8 (PcdPrePiCpuMemorySize), PcdGet8 (PcdPrePiCpuIoSize));
> +  BuildCpuHob (ArmGetPhysicalAddressBits (), PcdGet8 (PcdPrePiCpuIoSize));
>  
>    // Store timer value logged at the beginning of firmware image execution
>    Performance.ResetEnd = GetTimeInNanoSecond (StartTimeStamp);
> 

Acked-by: Laszlo Ersek <lersek@redhat.com>

Re: [PATCH v3 10/16] ArmPlatformPkg/PrePi: base GCD memory space size on CPU's PA range

[Laszlo Ersek]

On 11/28/18 15:33, Ard Biesheuvel wrote:
> Derive the size of the GCD memory space map directly from the CPU's
> information registers rather than from the PcdPrePiCpuMemorySize PCD,
> which will be removed.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  ArmPlatformPkg/PrePi/PeiMPCore.inf  | 1 -
>  ArmPlatformPkg/PrePi/PeiUniCore.inf | 1 -
>  ArmPlatformPkg/PrePi/PrePi.c        | 2 +-
>  3 files changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/ArmPlatformPkg/PrePi/PeiMPCore.inf b/ArmPlatformPkg/PrePi/PeiMPCore.inf
> index 242b03175536..7e2ad6fc483d 100644
> --- a/ArmPlatformPkg/PrePi/PeiMPCore.inf
> +++ b/ArmPlatformPkg/PrePi/PeiMPCore.inf
> @@ -97,7 +97,6 @@ [FixedPcd]
>  
>    gArmPlatformTokenSpaceGuid.PcdCoreCount
>  
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
>    gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize
>  
>    gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIReclaimMemory
> diff --git a/ArmPlatformPkg/PrePi/PeiUniCore.inf b/ArmPlatformPkg/PrePi/PeiUniCore.inf
> index a45cdef4ed91..26328b7e8f67 100644
> --- a/ArmPlatformPkg/PrePi/PeiUniCore.inf
> +++ b/ArmPlatformPkg/PrePi/PeiUniCore.inf
> @@ -90,7 +90,6 @@ [FixedPcd]
>  
>    gArmPlatformTokenSpaceGuid.PcdCoreCount
>  
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
>    gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize
>  
>    gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIReclaimMemory
> diff --git a/ArmPlatformPkg/PrePi/PrePi.c b/ArmPlatformPkg/PrePi/PrePi.c
> index 02cff7ddc204..245bdded1eb3 100644
> --- a/ArmPlatformPkg/PrePi/PrePi.c
> +++ b/ArmPlatformPkg/PrePi/PrePi.c
> @@ -115,7 +115,7 @@ PrePiMain (
>    BuildStackHob (StacksBase, StacksSize);
>  
>    //TODO: Call CpuPei as a library
> -  BuildCpuHob (PcdGet8 (PcdPrePiCpuMemorySize), PcdGet8 (PcdPrePiCpuIoSize));
> +  BuildCpuHob (ArmGetPhysicalAddressBits (), PcdGet8 (PcdPrePiCpuIoSize));
>  
>    if (ArmIsMpCore ()) {
>      // Only MP Core platform need to produce gArmMpCoreInfoPpiGuid
> 

Acked-by: Laszlo Ersek <lersek@redhat.com>

Re: [PATCH v3 13/16] ArmPlatformPkg/PlatformPei: drop unused PCD references

[Laszlo Ersek]

On 11/28/18 15:33, Ard Biesheuvel wrote:
> Drop some PCD references that are not actually referenced from the
> PlatformPei code.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf | 3 ---
>  ArmPlatformPkg/PlatformPei/PlatformPeim.inf   | 3 ---
>  2 files changed, 6 deletions(-)
> 
> diff --git a/ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf b/ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf
> index 314789d0a990..23bb3f37e766 100644
> --- a/ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf
> +++ b/ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf
> @@ -46,8 +46,5 @@ [FixedPcd]
>    gArmTokenSpaceGuid.PcdFvBaseAddress
>    gArmTokenSpaceGuid.PcdFvSize
>  
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize
> -
>  [depex]
>    TRUE
> diff --git a/ArmPlatformPkg/PlatformPei/PlatformPeim.inf b/ArmPlatformPkg/PlatformPei/PlatformPeim.inf
> index 423b9ab858d1..4934baa838e1 100644
> --- a/ArmPlatformPkg/PlatformPei/PlatformPeim.inf
> +++ b/ArmPlatformPkg/PlatformPei/PlatformPeim.inf
> @@ -57,9 +57,6 @@ [FixedPcd]
>    gArmTokenSpaceGuid.PcdFvBaseAddress
>    gArmTokenSpaceGuid.PcdFvSize
>  
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize
> -
>  [Depex]
>    TRUE
>  
> 

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Re: [PATCH v3 14/16] EmbeddedPkg/PrePiLib: drop unused PCD reference

[Laszlo Ersek]

On 11/28/18 15:33, Ard Biesheuvel wrote:
> Drop the reference to gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
> which is never used.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  EmbeddedPkg/Library/PrePiLib/PrePiLib.inf | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/EmbeddedPkg/Library/PrePiLib/PrePiLib.inf b/EmbeddedPkg/Library/PrePiLib/PrePiLib.inf
> index de68405098c0..3dba884b1f31 100644
> --- a/EmbeddedPkg/Library/PrePiLib/PrePiLib.inf
> +++ b/EmbeddedPkg/Library/PrePiLib/PrePiLib.inf
> @@ -69,7 +69,6 @@ [Protocols]
>  
>  
>  [FixedPcd.common]
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
>    gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize
>  
>    gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIReclaimMemory
> 

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Re: [PATCH v3 15/16] ArmVirtPkg: drop PcdPrePiCpuMemorySize assignments from all platforms

[Laszlo Ersek]

On 11/28/18 15:33, Ard Biesheuvel wrote:
> PcdPrePiCpuMemorySize is no longer used so drop the PCD overrides
> from all platform descriptions in ArmVirtPkg.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  ArmVirtPkg/ArmVirt.dsc.inc       | 3 ---
>  ArmVirtPkg/ArmVirtQemu.dsc       | 4 ----
>  ArmVirtPkg/ArmVirtQemuKernel.dsc | 4 ----
>  3 files changed, 11 deletions(-)
> 
> diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
> index 70a0ac4d786c..fbdb5c982604 100644
> --- a/ArmVirtPkg/ArmVirt.dsc.inc
> +++ b/ArmVirtPkg/ArmVirt.dsc.inc
> @@ -388,9 +388,6 @@ [PcdsFixedAtBuild.common]
>    #
>    gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE
>  
> -[PcdsFixedAtBuild.ARM]
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|40
> -
>  [Components.common]
>    #
>    # Ramdisk support
> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
> index 885c6b14b844..a107b6bb5104 100644
> --- a/ArmVirtPkg/ArmVirtQemu.dsc
> +++ b/ArmVirtPkg/ArmVirtQemu.dsc
> @@ -143,10 +143,6 @@ [PcdsFixedAtBuild.common]
>    gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|16
>  
>  [PcdsFixedAtBuild.AARCH64]
> -  # KVM limits it IPA space to 40 bits (1 TB), so there is no need to
> -  # support anything bigger, even if the host hardware does
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|40
> -
>    # Clearing BIT0 in this PCD prevents installing a 32-bit SMBIOS entry point,
>    # if the entry point version is >= 3.0. AARCH64 OSes cannot assume the
>    # presence of the 32-bit entry point anyway (because many AARCH64 systems
> diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
> index 434d6861a56f..d8fbf14e8f4e 100644
> --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
> +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
> @@ -157,10 +157,6 @@ [PcdsFixedAtBuild.AARCH64]
>    #
>    gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|16
>  
> -  # KVM limits it IPA space to 40 bits (1 TB), so there is no need to
> -  # support anything bigger, even if the host hardware does
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|40
> -
>  [PcdsDynamicDefault.common]
>    gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|3
>  
> 

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Re: [PATCH v3 16/16] EmbeddedPkg/EmbeddedPkg.dec: drop PcdPrePiCpuMemorySize declarations

[Laszlo Ersek]

On 11/28/18 15:33, Ard Biesheuvel wrote:
> PcdPrePiCpuMemorySize is no longer used so drop the declarations from
> the package DEC file.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  EmbeddedPkg/EmbeddedPkg.dec | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/EmbeddedPkg/EmbeddedPkg.dec b/EmbeddedPkg/EmbeddedPkg.dec
> index 28a143865d0e..ff5aab07d745 100644
> --- a/EmbeddedPkg/EmbeddedPkg.dec
> +++ b/EmbeddedPkg/EmbeddedPkg.dec
> @@ -170,22 +170,18 @@ [PcdsFixedAtBuild.common]
>    gEmbeddedTokenSpaceGuid.PcdAndroidBootDevicePath|L""|VOID*|0x00000057
>  
>  [PcdsFixedAtBuild.ARM]
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|32|UINT8|0x00000010
>    gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|0|UINT8|0x00000011
>  
>    # ISP1761 USB OTG Controller
>    gEmbeddedTokenSpaceGuid.PcdIsp1761BaseAddress|0|UINT32|0x00000021
>  
>  [PcdsFixedAtBuild.AARCH64]
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|48|UINT8|0x00000010
>    gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|0|UINT8|0x00000011
>  
>  [PcdsFixedAtBuild.IA32]
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|36|UINT8|0x00000010
>    gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|16|UINT8|0x00000011
>  
>  [PcdsFixedAtBuild.X64]
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|52|UINT8|0x00000010
>    gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|16|UINT8|0x00000011
>  
>  [PcdsFixedAtBuild.common, PcdsDynamic.common]
> 

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Re: [PATCH v3 05/16] MdePkg/ProcessorBind.h AARCH64: limit MAX_ADDRESS to 48 bits

[Ard Biesheuvel]

On Wed, 28 Nov 2018 at 19:41, Laszlo Ersek <lersek@redhat.com> wrote:
>
> On 11/28/18 15:33, Ard Biesheuvel wrote:
> > AArch64 supports the use of more than 48 bits for physical and/or
> > virtual addressing, but only if the page size is set to 64 KB,
> > which is not supported by UEFI. So redefine MAX_ADDRESS to cover
> > only 48 address bits.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
> > ---
> >  MdePkg/Include/AArch64/ProcessorBind.h | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/MdePkg/Include/AArch64/ProcessorBind.h b/MdePkg/Include/AArch64/ProcessorBind.h
> > index 968c18f915ae..dad75df1c579 100644
> > --- a/MdePkg/Include/AArch64/ProcessorBind.h
> > +++ b/MdePkg/Include/AArch64/ProcessorBind.h
> > @@ -138,9 +138,9 @@ typedef INT64   INTN;
> >  #define MAX_2_BITS  0xC000000000000000ULL
> >
> >  ///
> > -/// Maximum legal AARCH64  address
> > +/// Maximum legal AARCH64  address (48 bits for 4 KB page size)
> >  ///
> > -#define MAX_ADDRESS   0xFFFFFFFFFFFFFFFFULL
> > +#define MAX_ADDRESS   0xFFFFFFFFFFFFULL
> >
> >  ///
> >  /// Maximum legal AArch64 INTN and UINTN values.
> >
>
> Hmmm.
>
> I bit the bullet and grepped the tree for MAX_ADDRESS.
>
> The amount of hits is staggering. I can't audit all of them.
>
> Generally, MAX_ADDRESS seems to be used in checks that prevent address
> wrap-around. In that regard, this change looks valid.
>
> I can't guarantee this change won't regress anything though. In the
> previous posting of this patch, I asked Liming some questions (IIRC):
>
> http://mid.mail-archive.com/6f1209fb-bb89-a70f-ba0e-3ebf2e12e459@redhat.com
>
> It would be nice to see answers. :)
>

Yep

> In addition:
>
> (a) in "BaseTools/Source/C/Include/AArch64/ProcessorBind.h", we have
> another instance of the macro definition. I suspect it should be kept in
> sync.
>

Indeed.

> (b) in "BaseTools/Source/C/Common/CommonLib.h", we have:
>
> #define MAX_UINTN MAX_ADDRESS
>
> which I think relies on (a), and hence it will be amusingly wrong after
> we synchronize (a) with MdePkg.
>
> (BTW, (b) is exactly the kind of assumption that scares me about this
> patch.)
>

That doesn't make any sense at all. What does 'native' mean in the
context of BaseTools anyway?

> We're not much past the last stable tag (edk2-stable201811), so let's
> hope there's going to be enough time to catch any regressions.
>
> With (a) and (b) investigated / fixed up, I'd be willing to A-b this.
> Cautiously :)
>

Thanks

> Anyway, this is for MdePkg, so my review is not required. (I certainly
> do not intend to *oppose* this patch.)
>
> Thanks
> Laszlo

Re: [PATCH v3 05/16] MdePkg/ProcessorBind.h AARCH64: limit MAX_ADDRESS to 48 bits

[Laszlo Ersek]

On 11/29/18 11:40, Ard Biesheuvel wrote:
> On Wed, 28 Nov 2018 at 19:41, Laszlo Ersek <lersek@redhat.com> wrote:
>>
>> On 11/28/18 15:33, Ard Biesheuvel wrote:
>>> AArch64 supports the use of more than 48 bits for physical and/or
>>> virtual addressing, but only if the page size is set to 64 KB,
>>> which is not supported by UEFI. So redefine MAX_ADDRESS to cover
>>> only 48 address bits.
>>>
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
>>> ---
>>>  MdePkg/Include/AArch64/ProcessorBind.h | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/MdePkg/Include/AArch64/ProcessorBind.h b/MdePkg/Include/AArch64/ProcessorBind.h
>>> index 968c18f915ae..dad75df1c579 100644
>>> --- a/MdePkg/Include/AArch64/ProcessorBind.h
>>> +++ b/MdePkg/Include/AArch64/ProcessorBind.h
>>> @@ -138,9 +138,9 @@ typedef INT64   INTN;
>>>  #define MAX_2_BITS  0xC000000000000000ULL
>>>
>>>  ///
>>> -/// Maximum legal AARCH64  address
>>> +/// Maximum legal AARCH64  address (48 bits for 4 KB page size)
>>>  ///
>>> -#define MAX_ADDRESS   0xFFFFFFFFFFFFFFFFULL
>>> +#define MAX_ADDRESS   0xFFFFFFFFFFFFULL
>>>
>>>  ///
>>>  /// Maximum legal AArch64 INTN and UINTN values.
>>>
>>
>> Hmmm.
>>
>> I bit the bullet and grepped the tree for MAX_ADDRESS.
>>
>> The amount of hits is staggering. I can't audit all of them.
>>
>> Generally, MAX_ADDRESS seems to be used in checks that prevent address
>> wrap-around. In that regard, this change looks valid.
>>
>> I can't guarantee this change won't regress anything though. In the
>> previous posting of this patch, I asked Liming some questions (IIRC):
>>
>> http://mid.mail-archive.com/6f1209fb-bb89-a70f-ba0e-3ebf2e12e459@redhat.com
>>
>> It would be nice to see answers. :)
>>
> 
> Yep
> 
>> In addition:
>>
>> (a) in "BaseTools/Source/C/Include/AArch64/ProcessorBind.h", we have
>> another instance of the macro definition. I suspect it should be kept in
>> sync.
>>
> 
> Indeed.
> 
>> (b) in "BaseTools/Source/C/Common/CommonLib.h", we have:
>>
>> #define MAX_UINTN MAX_ADDRESS
>>
>> which I think relies on (a), and hence it will be amusingly wrong after
>> we synchronize (a) with MdePkg.
>>
>> (BTW, (b) is exactly the kind of assumption that scares me about this
>> patch.)
>>
> 
> That doesn't make any sense at all. What does 'native' mean in the
> context of BaseTools anyway?

I can't tell whether this MAX_UINTN definition is for BaseTools itself
(i.e., "native") or for the build target arch.

"CommonLib.c" has a number of instances of MAX_UINTN... Hm, they are in
the following two functions:

- StrDecimalToUintnS() -- wrapped by StrDecimalToUintn(),
  StrToIpv4Address(), and StrToIpv6Address())

- StrHexToUintnS() -- wrapped by StrHexToUintn(), and
  StrToIpv6Address().

I tried to look at where those are called from, and the picture remains
muddled.

For example, StrHexToUintn() is called from
"BaseTools/Source/C/DevicePath/DevicePathFromText.c", functions
EisaIdFromText() and DevPathFromTextNVMe(). These functions seem to
compose binary devpath nodes from text *during the build*, likely for
the firmware-under-build to consume as-is. Hence, this use of MAX_UINTN
-- through StrHexToUintn() -- qualifies as *native* use. And for that,
the MAX_UINTN definition in "CommonLib.h" looks wrong, independently of
your patch series.

Thanks
Laszlo

Re: [PATCH v3 05/16] MdePkg/ProcessorBind.h AARCH64: limit MAX_ADDRESS to 48 bits

[Gao, Liming]

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Laszlo Ersek
> Sent: Thursday, November 29, 2018 7:34 PM
> To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Andrew Jones <drjones@redhat.com>; edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH v3 05/16] MdePkg/ProcessorBind.h AARCH64: limit MAX_ADDRESS to 48 bits
> 
> On 11/29/18 11:40, Ard Biesheuvel wrote:
> > On Wed, 28 Nov 2018 at 19:41, Laszlo Ersek <lersek@redhat.com> wrote:
> >>
> >> On 11/28/18 15:33, Ard Biesheuvel wrote:
> >>> AArch64 supports the use of more than 48 bits for physical and/or
> >>> virtual addressing, but only if the page size is set to 64 KB,
> >>> which is not supported by UEFI. So redefine MAX_ADDRESS to cover
> >>> only 48 address bits.
> >>>
> >>> Contributed-under: TianoCore Contribution Agreement 1.1
> >>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >>> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
> >>> ---
> >>>  MdePkg/Include/AArch64/ProcessorBind.h | 4 ++--
> >>>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/MdePkg/Include/AArch64/ProcessorBind.h b/MdePkg/Include/AArch64/ProcessorBind.h
> >>> index 968c18f915ae..dad75df1c579 100644
> >>> --- a/MdePkg/Include/AArch64/ProcessorBind.h
> >>> +++ b/MdePkg/Include/AArch64/ProcessorBind.h
> >>> @@ -138,9 +138,9 @@ typedef INT64   INTN;
> >>>  #define MAX_2_BITS  0xC000000000000000ULL
> >>>
> >>>  ///
> >>> -/// Maximum legal AARCH64  address
> >>> +/// Maximum legal AARCH64  address (48 bits for 4 KB page size)
> >>>  ///
> >>> -#define MAX_ADDRESS   0xFFFFFFFFFFFFFFFFULL
> >>> +#define MAX_ADDRESS   0xFFFFFFFFFFFFULL
> >>>
> >>>  ///
> >>>  /// Maximum legal AArch64 INTN and UINTN values.
> >>>
> >>
> >> Hmmm.
> >>
> >> I bit the bullet and grepped the tree for MAX_ADDRESS.
> >>
> >> The amount of hits is staggering. I can't audit all of them.
> >>
> >> Generally, MAX_ADDRESS seems to be used in checks that prevent address
> >> wrap-around. In that regard, this change looks valid.
> >>
> >> I can't guarantee this change won't regress anything though. In the
> >> previous posting of this patch, I asked Liming some questions (IIRC):
> >>
> >> http://mid.mail-archive.com/6f1209fb-bb89-a70f-ba0e-3ebf2e12e459@redhat.com
> >>
> >> It would be nice to see answers. :)
> >>
> >
> > Yep
> >
> >> In addition:
> >>
> >> (a) in "BaseTools/Source/C/Include/AArch64/ProcessorBind.h", we have
> >> another instance of the macro definition. I suspect it should be kept in
> >> sync.
> >>
> >
> > Indeed.
> >
> >> (b) in "BaseTools/Source/C/Common/CommonLib.h", we have:
> >>
> >> #define MAX_UINTN MAX_ADDRESS
> >>
> >> which I think relies on (a), and hence it will be amusingly wrong after
> >> we synchronize (a) with MdePkg.
> >>
> >> (BTW, (b) is exactly the kind of assumption that scares me about this
> >> patch.)
> >>
> >
> > That doesn't make any sense at all. What does 'native' mean in the
> > context of BaseTools anyway?
> 
> I can't tell whether this MAX_UINTN definition is for BaseTools itself
> (i.e., "native") or for the build target arch.
> 
> "CommonLib.c" has a number of instances of MAX_UINTN... Hm, they are in
> the following two functions:
> 
> - StrDecimalToUintnS() -- wrapped by StrDecimalToUintn(),
>   StrToIpv4Address(), and StrToIpv6Address())
> 
> - StrHexToUintnS() -- wrapped by StrHexToUintn(), and
>   StrToIpv6Address().
> 
> I tried to look at where those are called from, and the picture remains
> muddled.
> 
> For example, StrHexToUintn() is called from
> "BaseTools/Source/C/DevicePath/DevicePathFromText.c", functions
> EisaIdFromText() and DevPathFromTextNVMe(). These functions seem to
> compose binary devpath nodes from text *during the build*, likely for
> the firmware-under-build to consume as-is. Hence, this use of MAX_UINTN
> -- through StrHexToUintn() -- qualifies as *native* use. And for that,
> the MAX_UINTN definition in "CommonLib.h" looks wrong, independently of
> your patch series.

I agree this is an issue in BaseTools. BaseTools should generate the same result no matter on the host with the different arch. 

For this change in MdePkg, it makes sense to me. Reviewed-by: Liming Gao <liming.gao@intel.com>
> 
> Thanks
> Laszlo
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

Re: [PATCH v3 01/16] EmbeddedPkg/TemplateSec: remove unused module

[Leif Lindholm]

On Wed, Nov 28, 2018 at 03:33:42PM +0100, Ard Biesheuvel wrote:
> Remove this module: it is unused, and should not be used as an
> example going forward.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> ---
>  EmbeddedPkg/TemplateSec/TemplateSec.inf | 65 -----------------
>  EmbeddedPkg/TemplateSec/TemplateSec.c   | 76 --------------------
>  2 files changed, 141 deletions(-)
> 
> diff --git a/EmbeddedPkg/TemplateSec/TemplateSec.inf b/EmbeddedPkg/TemplateSec/TemplateSec.inf
> deleted file mode 100644
> index 3a63e59294d3..000000000000
> --- a/EmbeddedPkg/TemplateSec/TemplateSec.inf
> +++ /dev/null
> @@ -1,65 +0,0 @@
> -#/** @file
> -#
> -#    Component description file for DxeIpl module
> -#
> -#   The responsibility of this module is to load the DXE Core from a Firmware Volume. This implementation i used to load a 32-bit DXE Core.
> -#
> -#  Copyright (c) 2006 - 2008, 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.
> -#
> -#**/
> -
> -[Defines]
> -  INF_VERSION                    = 0x00010005
> -  BASE_NAME                      = TemplateSec
> -  FILE_GUID                      = 1D6F730F-5A55-4078-869B-E0A18324BDC8
> -  MODULE_TYPE                    = SEC
> -  VERSION_STRING                 = 1.0
> -
> -
> -#
> -# The following information is for reference only and not required by the build tools.
> -#
> -#  VALID_ARCHITECTURES           = IA32 X64 ARM
> -#
> -
> -[Sources.common]
> -  TemplateSec.c
> -
> -[Sources.Ia32]
> -#  Ia32/ResetVector.asm | MSFT
> -#  Ia32/ResetVector.S   | GCC
> -
> -[Sources.X64]
> -#  X64/ResetVector.asm | MSFT
> -#  X64/ResetVector.S   | GCC
> -
> -[Sources.ARM]
> -#  Arm/ResetVector.asm | RVCT
> -#  Arm/ResetVector.S   | GCC
> -
> -[Packages]
> -  MdePkg/MdePkg.dec
> -  MdeModulePkg/MdeModulePkg.dec
> -  EmbeddedPkg/EmbeddedPkg.dec
> -
> -
> -[LibraryClasses]
> -  BaseLib
> -  DebugLib
> -  BaseMemoryLib
> -  UefiDecompressLib
> -  PeCoffLib
> -  CacheMaintenanceLib
> -  PrePiLib
> -
> -[Pcd]
> -  gEmbeddedTokenSpaceGuid.PcdEmbeddedFdBaseAddress
> -  gEmbeddedTokenSpaceGuid.PcdEmbeddedFdSize
> -
> diff --git a/EmbeddedPkg/TemplateSec/TemplateSec.c b/EmbeddedPkg/TemplateSec/TemplateSec.c
> deleted file mode 100644
> index c63adbb6f90f..000000000000
> --- a/EmbeddedPkg/TemplateSec/TemplateSec.c
> +++ /dev/null
> @@ -1,76 +0,0 @@
> -/** @file
> -
> -  Copyright (c) 2008 - 2009, Apple Inc. 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.
> -
> -**/
> -
> -#include <PiPei.h>
> -
> -#include <Library/DebugLib.h>
> -#include <Library/PrePiLib.h>
> -#include <Library/PcdLib.h>
> -
> -#include <Ppi/GuidedSectionExtraction.h>
> -
> -VOID
> -_ModuleEntryPoint (
> -  VOID
> -  )
> -{
> -}
> -
> -VOID
> -CEntryPoint (
> -  VOID    *MemoryBase,
> -  UINTN   MemorySize,
> -  VOID    *StackBase,
> -  UINTN   StackSize
> -  )
> -{
> -  EFI_PHYSICAL_ADDRESS  MemoryBegin;
> -  UINT64                MemoryLength;
> -  VOID                  *HobBase;
> -
> -  //
> -  // Boot strap the C environment so the other library services will work properly.
> -  //
> -  MemoryBegin  = (EFI_PHYSICAL_ADDRESS)(UINTN)MemoryBase;
> -  MemoryLength = (UINT64)MemorySize;
> -  HobBase      = (VOID *)(UINTN)(FixedPcdGet32(PcdEmbeddedFdBaseAddress) + FixedPcdGet32(PcdEmbeddedFdSize));
> -  CreateHobList (MemoryBase, MemorySize, HobBase, StackBase);
> -
> -  MemoryBegin  = (EFI_PHYSICAL_ADDRESS)(UINTN)StackBase;
> -  MemoryLength = (UINT64)StackSize;
> -  UpdateStackHob (MemoryBegin, MemoryLength);
> -
> -  DEBUG ((DEBUG_ERROR, "CEntryPoint (%x,%x,%x,%x)\n", MemoryBase, MemorySize, StackBase, StackSize));
> -
> -  //
> -  // Add your C code stuff here....
> -  //
> -
> -
> -  //
> -  // Load the DXE Core and transfer control to it
> -  //
> -
> -  // Give the DXE Core access to our DEBUG and ASSERT infrastructure so this will work prior
> -  // to the DXE version being loaded. Thus we close the debugging gap between phases.
> -  AddDxeCoreReportStatusCodeCallback ();
> -
> -  //BuildFvHobs (PcdBfvBase, PcdBfvSize, NULL);
> -
> -  LoadDxeCoreFromFv (NULL, 0);
> -
> -  // DXE Core should always load and never return
> -  ASSERT (FALSE);
> -}
> -
> -- 
> 2.19.1
> 

Re: [PATCH v3 02/16] EmbeddedPkg/PrePiHobLib: drop CreateHobList() from library

[Leif Lindholm]

On Wed, Nov 28, 2018 at 03:33:43PM +0100, Ard Biesheuvel wrote:
> Drop the declaration and the implementation of CreateHoblist(),
> which is not used anywhere.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> ---
>  EmbeddedPkg/Include/Library/PrePiLib.h | 18 ---------
>  EmbeddedPkg/Library/PrePiHobLib/Hob.c  | 41 --------------------
>  2 files changed, 59 deletions(-)
> 
> diff --git a/EmbeddedPkg/Include/Library/PrePiLib.h b/EmbeddedPkg/Include/Library/PrePiLib.h
> index cf70fca3b619..a857308ecec2 100644
> --- a/EmbeddedPkg/Include/Library/PrePiLib.h
> +++ b/EmbeddedPkg/Include/Library/PrePiLib.h
> @@ -274,24 +274,6 @@ HobConstructor (
>    IN VOID   *EfiFreeMemoryTop
>    );
>  
> -/**
> -  Retrieves the magic value from the PE/COFF header.
> -
> -  @param  Hdr             The buffer in which to return the PE32, PE32+, or TE header.
> -
> -  @return EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC - Image is PE32
> -  @return EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC - Image is PE32+
> -
> -**/
> -VOID
> -CreateHobList (
> -  IN VOID   *MemoryBegin,
> -  IN UINTN  MemoryLength,
> -  IN VOID   *HobBase,
> -  IN VOID   *StackBase
> -  );
> -
> -
>  /**
>    This service enables PEIMs to create various types of HOBs.
>  
> diff --git a/EmbeddedPkg/Library/PrePiHobLib/Hob.c b/EmbeddedPkg/Library/PrePiHobLib/Hob.c
> index aff8ea05797b..ba16899a9184 100644
> --- a/EmbeddedPkg/Library/PrePiHobLib/Hob.c
> +++ b/EmbeddedPkg/Library/PrePiHobLib/Hob.c
> @@ -175,47 +175,6 @@ BuildResourceDescriptorHob (
>    Hob->ResourceLength    = NumberOfBytes;
>  }
>  
> -/**
> -
> -
> -**/
> -VOID
> -CreateHobList (
> -  IN VOID   *MemoryBegin,
> -  IN UINTN  MemoryLength,
> -  IN VOID   *HobBase,
> -  IN VOID   *StackBase
> -  )
> -{
> -  EFI_HOB_HANDOFF_INFO_TABLE  *Hob;
> -  EFI_RESOURCE_ATTRIBUTE_TYPE Attributes;
> -
> -  Hob = HobConstructor (MemoryBegin,MemoryLength,HobBase,StackBase);
> -  SetHobList (Hob);
> -
> -  BuildCpuHob (PcdGet8 (PcdPrePiCpuMemorySize), PcdGet8 (PcdPrePiCpuIoSize));
> -
> -  Attributes =(
> -    EFI_RESOURCE_ATTRIBUTE_PRESENT |
> -    EFI_RESOURCE_ATTRIBUTE_INITIALIZED |
> -    EFI_RESOURCE_ATTRIBUTE_TESTED |
> -    EFI_RESOURCE_ATTRIBUTE_UNCACHEABLE |
> -    EFI_RESOURCE_ATTRIBUTE_WRITE_COMBINEABLE |
> -    EFI_RESOURCE_ATTRIBUTE_WRITE_THROUGH_CACHEABLE |
> -    EFI_RESOURCE_ATTRIBUTE_WRITE_BACK_CACHEABLE
> -  );
> -
> -  BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY, Attributes, (UINTN)MemoryBegin, MemoryLength);
> -
> -  BuildStackHob ((EFI_PHYSICAL_ADDRESS)(UINTN)StackBase, ((UINTN)MemoryBegin + MemoryLength) - (UINTN)StackBase);
> -
> -  if (FeaturePcdGet (PcdPrePiProduceMemoryTypeInformationHob)) {
> -    // Optional feature that helps prevent EFI memory map fragmentation.
> -    BuildMemoryTypeInformationHob ();
> -  }
> -}
> -
> -
>  VOID
>  EFIAPI
>  BuildFvHobs (
> -- 
> 2.19.1
> 

Re: [PATCH v3 06/16] ArmPkg/ArmLib: add support for reading the max physical address space size

[Leif Lindholm]

On Wed, Nov 28, 2018 at 03:33:47PM +0100, Ard Biesheuvel wrote:
> Add a helper function that returns the maximum physical address space
> size as supported by the current CPU.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> ---
>  ArmPkg/Include/Library/ArmLib.h               |  6 ++++++
>  ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S | 17 +++++++++++++++++
>  ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S     |  8 ++++++++
>  ArmPkg/Library/ArmLib/Arm/ArmLibSupport.asm   |  8 ++++++++
>  4 files changed, 39 insertions(+)
> 
> diff --git a/ArmPkg/Include/Library/ArmLib.h b/ArmPkg/Include/Library/ArmLib.h
> index ffda50e9d767..9a804c15fdb6 100644
> --- a/ArmPkg/Include/Library/ArmLib.h
> +++ b/ArmPkg/Include/Library/ArmLib.h
> @@ -733,4 +733,10 @@ ArmWriteCntvOff (
>    UINT64   Val
>    );
>  
> +UINTN
> +EFIAPI
> +ArmGetPhysicalAddressBits (
> +  VOID
> +  );
> +
>  #endif // __ARM_LIB__
> diff --git a/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S b/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
> index 1ef2f61f5979..b7173e00b039 100644
> --- a/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
> +++ b/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
> @@ -196,4 +196,21 @@ ASM_FUNC(ArmWriteSctlr)
>  3:msr   sctlr_el3, x0
>  4:ret
>  
> +ASM_FUNC(ArmGetPhysicalAddressBits)
> +  mrs   x0, id_aa64mmfr0_el1
> +  adr   x1, .LPARanges
> +  and   x0, x0, #0xf
> +  ldrb  w0, [x1, x0]
> +  ret
> +
> +//
> +// Bits 0..3 of the AA64MFR0_EL1 system register encode the size of the
> +// physical address space support on this CPU:
> +// 0 == 32 bits, 1 == 36 bits, etc etc
> +// 7 and up are reserved
> +//
> +.LPARanges:
> +  .byte 32, 36, 40, 42, 44, 48, 52,  0
> +  .byte  0,  0,  0,  0,  0,  0,  0,  0
> +
>  ASM_FUNCTION_REMOVE_IF_UNREFERENCED
> diff --git a/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S b/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S
> index f2a517671f0a..0e9f9d0453e4 100644
> --- a/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S
> +++ b/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S
> @@ -165,4 +165,12 @@ ASM_FUNC(ArmWriteCpuActlr)
>    isb
>    bx      lr
>  
> +ASM_FUNC (ArmGetPhysicalAddressBits)
> +  mrc     p15, 0, r0, c0, c1, 4   // MMFR0
> +  and     r0, r0, #0xf            // VMSA [3:0]
> +  cmp     r0, #5                  // >= 5 implies LPAE support
> +  movlt   r0, #32                 // 32 bits if no LPAE
> +  movge   r0, #40                 // 40 bits if LPAE
> +  bx      lr
> +
>  ASM_FUNCTION_REMOVE_IF_UNREFERENCED
> diff --git a/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.asm b/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.asm
> index 219140c22b13..3eb52875971d 100644
> --- a/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.asm
> +++ b/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.asm
> @@ -169,4 +169,12 @@
>    isb
>    bx      lr
>  
> + RVCT_ASM_EXPORT ArmGetPhysicalAddressBits
> +  mrc     p15, 0, r0, c0, c1, 4   ; MMFR0
> +  and     r0, r0, #0xf            ; VMSA [3:0]
> +  cmp     r0, #5                  ; >= 5 implies LPAE support
> +  movlt   r0, #32                 ; 32 bits if no LPAE
> +  movge   r0, #40                 ; 40 bits if LPAE
> +  bx      lr
> +
>    END
> -- 
> 2.19.1
> 

Re: [PATCH v3 08/16] ArmPkg/ArmMmuLib: take the CPU supported maximum PA space into account

[Leif Lindholm]

On Wed, Nov 28, 2018 at 03:33:49PM +0100, Ard Biesheuvel wrote:
> In preparation of dropping PcdPrePiCpuMemorySize entirely, base the
> maximum size of the identity map on the capabilities of the CPU.
> Since that may exceed what is architecturally permitted when using
> 4 KB pages, take MAX_ADDRESS into account as well.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> ---
>  ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf       |  3 ---
>  ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf        |  3 ---
>  ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 11 +++++++++--
>  3 files changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf b/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
> index b9f264de8d26..246963361e45 100644
> --- a/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
> +++ b/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
> @@ -40,8 +40,5 @@ [LibraryClasses]
>    CacheMaintenanceLib
>    MemoryAllocationLib
>  
> -[Pcd.AARCH64]
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
> -
>  [Pcd.ARM]
>    gArmTokenSpaceGuid.PcdNormalMemoryNonshareableOverride
> diff --git a/ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf b/ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf
> index ecf13f790734..f689c193b862 100644
> --- a/ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf
> +++ b/ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf
> @@ -35,6 +35,3 @@ [LibraryClasses]
>    ArmLib
>    CacheMaintenanceLib
>    MemoryAllocationLib
> -
> -[Pcd.AARCH64]
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
> diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> index 4b62ecb6a476..5403b8d4070e 100644
> --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> @@ -604,8 +604,15 @@ ArmConfigureMmu (
>      return EFI_INVALID_PARAMETER;
>    }
>  
> -  // Cover the entire GCD memory space
> -  MaxAddress = (1UL << PcdGet8 (PcdPrePiCpuMemorySize)) - 1;
> +  //
> +  // Limit the virtual address space to what we can actually use: UEFI
> +  // mandates a 1:1 mapping, so no point in making the virtual address
> +  // space larger than the physical address space. We also have to take
> +  // into account the architectural limitations that result from UEFI's
> +  // use of 4 KB pages.
> +  //
> +  MaxAddress = MIN (LShiftU64 (1ULL, ArmGetPhysicalAddressBits ()) - 1,
> +                    MAX_ADDRESS);
>  
>    // Lookup the Table Level to get the information
>    LookupAddresstoRootTable (MaxAddress, &T0SZ, &RootTableEntryCount);
> -- 
> 2.19.1
> 

Re: [PATCH v3 09/16] ArmPkg/CpuPei: base GCD memory space size on CPU's PA range

[Leif Lindholm]

On Wed, Nov 28, 2018 at 03:33:50PM +0100, Ard Biesheuvel wrote:
> Derive the size of the GCD memory space map directly from the CPU's
> information registers rather than from the PcdPrePiCpuMemorySize PCD,
> which will be removed.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> ---
>  ArmPkg/Drivers/CpuPei/CpuPei.inf | 1 -
>  ArmPkg/Drivers/CpuPei/CpuPei.c   | 2 +-
>  2 files changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/ArmPkg/Drivers/CpuPei/CpuPei.inf b/ArmPkg/Drivers/CpuPei/CpuPei.inf
> index eafccd600983..dcea012fd8f9 100644
> --- a/ArmPkg/Drivers/CpuPei/CpuPei.inf
> +++ b/ArmPkg/Drivers/CpuPei/CpuPei.inf
> @@ -50,7 +50,6 @@ [Guids]
>    gArmMpCoreInfoGuid
>  
>  [FixedPcd]
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
>    gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize
>  
>  [Depex]
> diff --git a/ArmPkg/Drivers/CpuPei/CpuPei.c b/ArmPkg/Drivers/CpuPei/CpuPei.c
> index d54f42acfcc8..e63519ff6481 100644
> --- a/ArmPkg/Drivers/CpuPei/CpuPei.c
> +++ b/ArmPkg/Drivers/CpuPei/CpuPei.c
> @@ -73,7 +73,7 @@ InitializeCpuPeim (
>    ArmEnableBranchPrediction ();
>  
>    // Publish the CPU memory and io spaces sizes
> -  BuildCpuHob (PcdGet8 (PcdPrePiCpuMemorySize), PcdGet8 (PcdPrePiCpuIoSize));
> +  BuildCpuHob (ArmGetPhysicalAddressBits (), PcdGet8 (PcdPrePiCpuIoSize));
>  
>    // Only MP Core platform need to produce gArmMpCoreInfoPpiGuid
>    Status = PeiServicesLocatePpi (&gArmMpCoreInfoPpiGuid, 0, NULL, (VOID**)&ArmMpCoreInfoPpi);
> -- 
> 2.19.1
> 

Re: [PATCH v3 10/16] ArmPlatformPkg/PrePi: base GCD memory space size on CPU's PA range

[Leif Lindholm]

On Wed, Nov 28, 2018 at 03:33:51PM +0100, Ard Biesheuvel wrote:
> Derive the size of the GCD memory space map directly from the CPU's
> information registers rather than from the PcdPrePiCpuMemorySize PCD,
> which will be removed.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> ---
>  ArmPlatformPkg/PrePi/PeiMPCore.inf  | 1 -
>  ArmPlatformPkg/PrePi/PeiUniCore.inf | 1 -
>  ArmPlatformPkg/PrePi/PrePi.c        | 2 +-
>  3 files changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/ArmPlatformPkg/PrePi/PeiMPCore.inf b/ArmPlatformPkg/PrePi/PeiMPCore.inf
> index 242b03175536..7e2ad6fc483d 100644
> --- a/ArmPlatformPkg/PrePi/PeiMPCore.inf
> +++ b/ArmPlatformPkg/PrePi/PeiMPCore.inf
> @@ -97,7 +97,6 @@ [FixedPcd]
>  
>    gArmPlatformTokenSpaceGuid.PcdCoreCount
>  
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
>    gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize
>  
>    gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIReclaimMemory
> diff --git a/ArmPlatformPkg/PrePi/PeiUniCore.inf b/ArmPlatformPkg/PrePi/PeiUniCore.inf
> index a45cdef4ed91..26328b7e8f67 100644
> --- a/ArmPlatformPkg/PrePi/PeiUniCore.inf
> +++ b/ArmPlatformPkg/PrePi/PeiUniCore.inf
> @@ -90,7 +90,6 @@ [FixedPcd]
>  
>    gArmPlatformTokenSpaceGuid.PcdCoreCount
>  
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
>    gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize
>  
>    gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIReclaimMemory
> diff --git a/ArmPlatformPkg/PrePi/PrePi.c b/ArmPlatformPkg/PrePi/PrePi.c
> index 02cff7ddc204..245bdded1eb3 100644
> --- a/ArmPlatformPkg/PrePi/PrePi.c
> +++ b/ArmPlatformPkg/PrePi/PrePi.c
> @@ -115,7 +115,7 @@ PrePiMain (
>    BuildStackHob (StacksBase, StacksSize);
>  
>    //TODO: Call CpuPei as a library
> -  BuildCpuHob (PcdGet8 (PcdPrePiCpuMemorySize), PcdGet8 (PcdPrePiCpuIoSize));
> +  BuildCpuHob (ArmGetPhysicalAddressBits (), PcdGet8 (PcdPrePiCpuIoSize));
>  
>    if (ArmIsMpCore ()) {
>      // Only MP Core platform need to produce gArmMpCoreInfoPpiGuid
> -- 
> 2.19.1
> 

Re: [PATCH v3 12/16] BeagleBoardPkg/PrePi: base GCD memory space size on CPU's PA range

[Leif Lindholm]

On Wed, Nov 28, 2018 at 03:33:53PM +0100, Ard Biesheuvel wrote:
> Derive the size of the GCD memory space map directly from the CPU's
> information registers rather than from the PcdPrePiCpuMemorySize PCD,
> which will be removed.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> ---
>  BeagleBoardPkg/PrePi/PeiUniCore.inf | 1 -
>  BeagleBoardPkg/PrePi/PrePi.c        | 2 +-
>  2 files changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/BeagleBoardPkg/PrePi/PeiUniCore.inf b/BeagleBoardPkg/PrePi/PeiUniCore.inf
> index 3d72bc5b46e1..53c71d8eafc2 100644
> --- a/BeagleBoardPkg/PrePi/PeiUniCore.inf
> +++ b/BeagleBoardPkg/PrePi/PeiUniCore.inf
> @@ -86,7 +86,6 @@ [FixedPcd]
>  
>    gArmPlatformTokenSpaceGuid.PcdCoreCount
>  
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
>    gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize
>  
>    gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIReclaimMemory
> diff --git a/BeagleBoardPkg/PrePi/PrePi.c b/BeagleBoardPkg/PrePi/PrePi.c
> index 46f63f40c46e..bc9b0c80b84c 100644
> --- a/BeagleBoardPkg/PrePi/PrePi.c
> +++ b/BeagleBoardPkg/PrePi/PrePi.c
> @@ -110,7 +110,7 @@ PrePiMain (
>    BuildStackHob (StacksBase, StacksSize);
>  
>    //TODO: Call CpuPei as a library
> -  BuildCpuHob (PcdGet8 (PcdPrePiCpuMemorySize), PcdGet8 (PcdPrePiCpuIoSize));
> +  BuildCpuHob (ArmGetPhysicalAddressBits (), PcdGet8 (PcdPrePiCpuIoSize));
>  
>    // Store timer value logged at the beginning of firmware image execution
>    Performance.ResetEnd = GetTimeInNanoSecond (StartTimeStamp);
> -- 
> 2.19.1
> 

Re: [PATCH v3 13/16] ArmPlatformPkg/PlatformPei: drop unused PCD references

[Leif Lindholm]

On Wed, Nov 28, 2018 at 03:33:54PM +0100, Ard Biesheuvel wrote:
> Drop some PCD references that are not actually referenced from the
> PlatformPei code.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> ---
>  ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf | 3 ---
>  ArmPlatformPkg/PlatformPei/PlatformPeim.inf   | 3 ---
>  2 files changed, 6 deletions(-)
> 
> diff --git a/ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf b/ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf
> index 314789d0a990..23bb3f37e766 100644
> --- a/ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf
> +++ b/ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf
> @@ -46,8 +46,5 @@ [FixedPcd]
>    gArmTokenSpaceGuid.PcdFvBaseAddress
>    gArmTokenSpaceGuid.PcdFvSize
>  
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize
> -
>  [depex]
>    TRUE
> diff --git a/ArmPlatformPkg/PlatformPei/PlatformPeim.inf b/ArmPlatformPkg/PlatformPei/PlatformPeim.inf
> index 423b9ab858d1..4934baa838e1 100644
> --- a/ArmPlatformPkg/PlatformPei/PlatformPeim.inf
> +++ b/ArmPlatformPkg/PlatformPei/PlatformPeim.inf
> @@ -57,9 +57,6 @@ [FixedPcd]
>    gArmTokenSpaceGuid.PcdFvBaseAddress
>    gArmTokenSpaceGuid.PcdFvSize
>  
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize
> -
>  [Depex]
>    TRUE
>  
> -- 
> 2.19.1
> 

Re: [PATCH v3 14/16] EmbeddedPkg/PrePiLib: drop unused PCD reference

[Leif Lindholm]

On Wed, Nov 28, 2018 at 03:33:55PM +0100, Ard Biesheuvel wrote:
> Drop the reference to gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
> which is never used.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> ---
>  EmbeddedPkg/Library/PrePiLib/PrePiLib.inf | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/EmbeddedPkg/Library/PrePiLib/PrePiLib.inf b/EmbeddedPkg/Library/PrePiLib/PrePiLib.inf
> index de68405098c0..3dba884b1f31 100644
> --- a/EmbeddedPkg/Library/PrePiLib/PrePiLib.inf
> +++ b/EmbeddedPkg/Library/PrePiLib/PrePiLib.inf
> @@ -69,7 +69,6 @@ [Protocols]
>  
>  
>  [FixedPcd.common]
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
>    gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize
>  
>    gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIReclaimMemory
> -- 
> 2.19.1
> 

Re: [PATCH v3 16/16] EmbeddedPkg/EmbeddedPkg.dec: drop PcdPrePiCpuMemorySize declarations

[Leif Lindholm]

On Wed, Nov 28, 2018 at 03:33:57PM +0100, Ard Biesheuvel wrote:
> PcdPrePiCpuMemorySize is no longer used so drop the declarations from
> the package DEC file.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> ---
>  EmbeddedPkg/EmbeddedPkg.dec | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/EmbeddedPkg/EmbeddedPkg.dec b/EmbeddedPkg/EmbeddedPkg.dec
> index 28a143865d0e..ff5aab07d745 100644
> --- a/EmbeddedPkg/EmbeddedPkg.dec
> +++ b/EmbeddedPkg/EmbeddedPkg.dec
> @@ -170,22 +170,18 @@ [PcdsFixedAtBuild.common]
>    gEmbeddedTokenSpaceGuid.PcdAndroidBootDevicePath|L""|VOID*|0x00000057
>  
>  [PcdsFixedAtBuild.ARM]
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|32|UINT8|0x00000010
>    gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|0|UINT8|0x00000011
>  
>    # ISP1761 USB OTG Controller
>    gEmbeddedTokenSpaceGuid.PcdIsp1761BaseAddress|0|UINT32|0x00000021
>  
>  [PcdsFixedAtBuild.AARCH64]
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|48|UINT8|0x00000010
>    gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|0|UINT8|0x00000011
>  
>  [PcdsFixedAtBuild.IA32]
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|36|UINT8|0x00000010
>    gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|16|UINT8|0x00000011
>  
>  [PcdsFixedAtBuild.X64]
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|52|UINT8|0x00000010
>    gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|16|UINT8|0x00000011
>  
>  [PcdsFixedAtBuild.common, PcdsDynamic.common]
> -- 
> 2.19.1
> 

Re: [PATCH v3 00/16] [Arm|ArmVirt|MdePkg|Embedded]Pkg: lift 40-bit IPA space limit

[Ard Biesheuvel]

On Wed, 28 Nov 2018 at 15:34, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
> This v3 subsumes and/or supersedes
>
> [PATCH v2 00/13] ArmPkg, ArmVirtPkg: lift 40-bit IPA space limit
> [PATCH] MdePkg/ProcessorBind.h AARCH64: limit MAX_ADDRESS to 48 bits
> [PATCH v2 0/2] ArmVirtPkg: remove high peripheral space mapping
>
> The ArmVirtQemu targets currently limit the size of the IPA space to
> 40 bits because that is all what KVM supports. However, this is about
> to change, and so we need to update the code if we want to ensure that
> our UEFI firmware builds can keep running on systems that set values
> other than 40 (which could be > 40 or < 40)
>
> This series refactors how we handle the maximum size of the physical
> address space supported by the CPU in relation with the size of UEFI's
> 1:1 mapping and the size of the GCD memory space map, taking the following
> observations into account:
> - the range of the linear mapping can be tied to whatever the CPU supports
>   (as long as it doesn't exceed what the architecture permits for 4k pages)
>   since we mostly already use the maximum of 4 levels anyway, and there is
>   no memory cost involved beyond that
> - there is usually no point in mapping the entire address space, which does
>   involve a memory cost
> - the GCD memory space may be required to cover more than what UEFI can
>   address itself, since it is the based for the UEFI memory map that is
>   provided to the OS
>
> Patches #1 and #2 remove some unused code to avoid having to fix it.
>
> Patches #3 and #4 update ArmVirtQemu and ArmVirtQemuKernel to drop the high
> peripheral space mapping, and map whatever may reside there explicitly
> (currently only the ECAM space in practice, but the MMIO view of the PCI
> I/O space is mapped explicitly as well)
>
> Patch #5 was sent out before individually, and sets MAX_ADDRESS to the
> maximum value AArch64 can map in UEFI which runs with 4k pages.
>
> Patch #6 adds a helper to ArmLib to read the number of supported address
> bits and take this into account in the page table code (#8), which allows
> PcdPrePiCpuMemorySize to assume a value that exceeds the capabilities of
> the CPU.
>
> Patch #7 is mostly a cleanup patch, to switch to the new helper added in
> patch #6. No functional changes intended.
>
> Patches #9 to #12 modify building of the CPU hob (and thus the size of the
> GCD memory space) based on the CPU capabilities rather than the value of
> PcdPrePiCpuMemorySize, which is dropped in the last patch.
>
> Pacthes #13 and #14 remove some needless references to PcdPrePiCpuMemorySize
>
> Patch #15 drops the overrides of PcdPrePiCpuMemorySize from all ArmVirtPkg
> platforms.
>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Cc: Eric Auger <eric.auger@redhat.com>
> Cc: Andrew Jones <drjones@redhat.com>
> Cc: Philippe Mathieu-Daude <philmd@redhat.com>
> Cc: Julien Grall <julien.grall@linaro.org>
>
> Ard Biesheuvel (16):
>   EmbeddedPkg/TemplateSec: remove unused module
>   EmbeddedPkg/PrePiHobLib: drop CreateHobList() from library
>   ArmVirtPkg/FdtPciHostBridgeLib: map ECAM and I/O spaces in GCD memory
>     map
>   ArmVirtPkg/QemuVirtMemInfoLib: remove 1:1 mapping of top of PA range
>   MdePkg/ProcessorBind.h AARCH64: limit MAX_ADDRESS to 48 bits
>   ArmPkg/ArmLib: add support for reading the max physical address space
>     size
>   ArmVirtPkg/XenVirtMemInfoLib: refactor reading of the PA space size
>   ArmPkg/ArmMmuLib: take the CPU supported maximum PA space into account
>   ArmPkg/CpuPei: base GCD memory space size on CPU's PA range
>   ArmPlatformPkg/PrePi: base GCD memory space size on CPU's PA range
>   ArmVirtPkg/PrePi: base GCD memory space size on CPU's PA range
>   BeagleBoardPkg/PrePi: base GCD memory space size on CPU's PA range
>   ArmPlatformPkg/PlatformPei: drop unused PCD references
>   EmbeddedPkg/PrePiLib: drop unused PCD reference
>   ArmVirtPkg: drop PcdPrePiCpuMemorySize assignments from all platforms
>   EmbeddedPkg/EmbeddedPkg.dec: drop PcdPrePiCpuMemorySize declarations
>

Thanks all for the reviews.

Patches #1 .. #15 pushed as e979ea74aa14..55342094fb86

Patch #16 needs to wait until edk2-platforms is brought up to date
with the removal of PcdPrePiCpuMemorySize

Re: [PATCH v3 00/16] [Arm|ArmVirt|MdePkg|Embedded]Pkg: lift 40-bit IPA space limit

[Ard Biesheuvel]

On Thu, 29 Nov 2018 at 18:59, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
> On Wed, 28 Nov 2018 at 15:34, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> >
> > This v3 subsumes and/or supersedes
> >
> > [PATCH v2 00/13] ArmPkg, ArmVirtPkg: lift 40-bit IPA space limit
> > [PATCH] MdePkg/ProcessorBind.h AARCH64: limit MAX_ADDRESS to 48 bits
> > [PATCH v2 0/2] ArmVirtPkg: remove high peripheral space mapping
> >
> > The ArmVirtQemu targets currently limit the size of the IPA space to
> > 40 bits because that is all what KVM supports. However, this is about
> > to change, and so we need to update the code if we want to ensure that
> > our UEFI firmware builds can keep running on systems that set values
> > other than 40 (which could be > 40 or < 40)
> >
> > This series refactors how we handle the maximum size of the physical
> > address space supported by the CPU in relation with the size of UEFI's
> > 1:1 mapping and the size of the GCD memory space map, taking the following
> > observations into account:
> > - the range of the linear mapping can be tied to whatever the CPU supports
> >   (as long as it doesn't exceed what the architecture permits for 4k pages)
> >   since we mostly already use the maximum of 4 levels anyway, and there is
> >   no memory cost involved beyond that
> > - there is usually no point in mapping the entire address space, which does
> >   involve a memory cost
> > - the GCD memory space may be required to cover more than what UEFI can
> >   address itself, since it is the based for the UEFI memory map that is
> >   provided to the OS
> >
> > Patches #1 and #2 remove some unused code to avoid having to fix it.
> >
> > Patches #3 and #4 update ArmVirtQemu and ArmVirtQemuKernel to drop the high
> > peripheral space mapping, and map whatever may reside there explicitly
> > (currently only the ECAM space in practice, but the MMIO view of the PCI
> > I/O space is mapped explicitly as well)
> >
> > Patch #5 was sent out before individually, and sets MAX_ADDRESS to the
> > maximum value AArch64 can map in UEFI which runs with 4k pages.
> >
> > Patch #6 adds a helper to ArmLib to read the number of supported address
> > bits and take this into account in the page table code (#8), which allows
> > PcdPrePiCpuMemorySize to assume a value that exceeds the capabilities of
> > the CPU.
> >
> > Patch #7 is mostly a cleanup patch, to switch to the new helper added in
> > patch #6. No functional changes intended.
> >
> > Patches #9 to #12 modify building of the CPU hob (and thus the size of the
> > GCD memory space) based on the CPU capabilities rather than the value of
> > PcdPrePiCpuMemorySize, which is dropped in the last patch.
> >
> > Pacthes #13 and #14 remove some needless references to PcdPrePiCpuMemorySize
> >
> > Patch #15 drops the overrides of PcdPrePiCpuMemorySize from all ArmVirtPkg
> > platforms.
> >
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Cc: Leif Lindholm <leif.lindholm@linaro.org>
> > Cc: Eric Auger <eric.auger@redhat.com>
> > Cc: Andrew Jones <drjones@redhat.com>
> > Cc: Philippe Mathieu-Daude <philmd@redhat.com>
> > Cc: Julien Grall <julien.grall@linaro.org>
> >
> > Ard Biesheuvel (16):
> >   EmbeddedPkg/TemplateSec: remove unused module
> >   EmbeddedPkg/PrePiHobLib: drop CreateHobList() from library
> >   ArmVirtPkg/FdtPciHostBridgeLib: map ECAM and I/O spaces in GCD memory
> >     map
> >   ArmVirtPkg/QemuVirtMemInfoLib: remove 1:1 mapping of top of PA range
> >   MdePkg/ProcessorBind.h AARCH64: limit MAX_ADDRESS to 48 bits
> >   ArmPkg/ArmLib: add support for reading the max physical address space
> >     size
> >   ArmVirtPkg/XenVirtMemInfoLib: refactor reading of the PA space size
> >   ArmPkg/ArmMmuLib: take the CPU supported maximum PA space into account
> >   ArmPkg/CpuPei: base GCD memory space size on CPU's PA range
> >   ArmPlatformPkg/PrePi: base GCD memory space size on CPU's PA range
> >   ArmVirtPkg/PrePi: base GCD memory space size on CPU's PA range
> >   BeagleBoardPkg/PrePi: base GCD memory space size on CPU's PA range
> >   ArmPlatformPkg/PlatformPei: drop unused PCD references
> >   EmbeddedPkg/PrePiLib: drop unused PCD reference
> >   ArmVirtPkg: drop PcdPrePiCpuMemorySize assignments from all platforms
> >   EmbeddedPkg/EmbeddedPkg.dec: drop PcdPrePiCpuMemorySize declarations
> >
>
> Thanks all for the reviews.
>
> Patches #1 .. #15 pushed as e979ea74aa14..55342094fb86
>
> Patch #16 needs to wait until edk2-platforms is brought up to date
> with the removal of PcdPrePiCpuMemorySize

Patch #16 now merged as 55342094fb86..bcf2a9db1f8e