[PATCH v2 0/5] Abstract allocation of PEI accessible memory

[PATCH v2 0/5] Abstract allocation of PEI accessible memory

[Ard Biesheuvel]

At the moment, FirmwarePerformanceTableDataDxe or DxeCorePerformanceLib
are unusable on systems such as AMD Seattle, because they unconditionally
attempt to allocate memory below 4 GB, and ASSERT() if this fails. On AMD
Seattle, no 32-bit addressable DRAM exists, and so the driver will always
assert, and crash a running DEBUG build.

The reason for this is that some platforms (i.e., X64 builds consisting of
a 32-bit PEI stage and 64-bit remaining stages) require these allocations
to be below 4 GB, and for some reason, open coding this practice throughout
the code without regard for other architectures has been regarded as an
acceptable approach.

Instead, I would like to propose the approach implemented in this series:
- create an abstracted EfiAllocatePeiAccessiblePages() routine in UefiLib
  that simply allocates pages from any region on all architectures except
  X64, and allocate below 4 GB for X64
- update the various call sites with calls to this new function.

The above is implemented in patches #3 - #6. Patches #1 and #2 are fixes
for issues that I observed in ArmVirtPkg and OvmfPkg while working on
these patches.

Code can be found here:
https://github.com/ardbiesheuvel/edk2/tree/allocate-pei-v2

Changes since v1:
- add Laszlo's ack to #1 - #2
- move EfiAllocatePeiPages() to DxeServicesLib and drop Efi prefix
- update implementation to take EfiFreeMemoryTop into account when built
  for X64

Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Dandan Bi <dandan.bi@intel.com>

Ard Biesheuvel (5):
  OvmfPkg/PlatformBootManagerLib: add missing report status code call
  ArmVirtPkg/PlatformBootManagerLib: add missing report status code call
  MdePkg/DxeServicesLib: introduce AllocatePeiAccessiblePages routine
  MdeModulePkg/DxeCorePerformanceLib: use AllocatePeiAccessiblePages
  MdeModulePkg/FirmwarePerformanceDataTableDxe: use
    AllocatePeiAccessiblePages

 .../PlatformBootManagerLib.inf                |  1 +
 .../PlatformBootManagerLib/QemuKernel.c       |  4 ++
 .../DxeCorePerformanceLib.c                   | 45 ++-------------
 .../FirmwarePerformanceDxe.c                  | 51 +++--------------
 .../FirmwarePerformanceDxe.inf                |  1 +
 MdePkg/Include/Library/DxeServicesLib.h       | 23 +++++++-
 .../Library/DxeServicesLib/DxeServicesLib.c   | 55 +++++++++++++++++++
 .../Library/DxeServicesLib/DxeServicesLib.inf |  3 +
 .../PlatformBootManagerLib.inf                |  1 +
 .../PlatformBootManagerLib/QemuKernel.c       |  4 ++
 10 files changed, 104 insertions(+), 84 deletions(-)

-- 
2.17.0

[PATCH v2 1/5] OvmfPkg/PlatformBootManagerLib: add missing report status code call

[Ard Biesheuvel]

Consumers of status code reports may rely on a status code to be
reported when the ReadyToBoot event is signalled. For instance,
FirmwarePerformanceDxe will fail to install the FPDT ACPI table
in this case. So add the missing call.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
 OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | 1 +
 OvmfPkg/Library/PlatformBootManagerLib/QemuKernel.c               | 4 ++++
 2 files changed, 5 insertions(+)

diff --git a/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
index 27789b7377bc..f10b68424b91 100644
--- a/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
+++ b/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
@@ -55,6 +55,7 @@ [LibraryClasses]
   QemuFwCfgS3Lib
   LoadLinuxLib
   QemuBootOrderLib
+  ReportStatusCodeLib
   UefiLib
 
 [Pcd]
diff --git a/OvmfPkg/Library/PlatformBootManagerLib/QemuKernel.c b/OvmfPkg/Library/PlatformBootManagerLib/QemuKernel.c
index ef728dfdeb60..f20df9533fda 100644
--- a/OvmfPkg/Library/PlatformBootManagerLib/QemuKernel.c
+++ b/OvmfPkg/Library/PlatformBootManagerLib/QemuKernel.c
@@ -18,6 +18,7 @@
 #include <Library/LoadLinuxLib.h>
 #include <Library/MemoryAllocationLib.h>
 #include <Library/QemuFwCfgLib.h>
+#include <Library/ReportStatusCodeLib.h>
 #include <Library/UefiBootServicesTableLib.h>
 #include <Library/UefiLib.h>
 
@@ -149,6 +150,9 @@ TryRunningQemuKernel (
   //
   EfiSignalEventReadyToBoot();
 
+  REPORT_STATUS_CODE (EFI_PROGRESS_CODE,
+    (EFI_SOFTWARE_DXE_BS_DRIVER | EFI_SW_DXE_BS_PC_READY_TO_BOOT_EVENT));
+
   Status = LoadLinux (KernelBuf, SetupBuf);
 
 FreeAndReturn:
-- 
2.17.0

[PATCH v2 2/5] ArmVirtPkg/PlatformBootManagerLib: add missing report status code call

[Ard Biesheuvel]

Consumers of status code reports may rely on a status code to be
reported when the ReadyToBoot event is signalled. For instance,
FirmwarePerformanceDxe will fail to install the FPDT ACPI table
in this case. So add the missing call.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
 ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | 1 +
 ArmVirtPkg/Library/PlatformBootManagerLib/QemuKernel.c               | 4 ++++
 2 files changed, 5 insertions(+)

diff --git a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
index d6c1ef95dc44..0cbc82f5d27d 100644
--- a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
+++ b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
@@ -53,6 +53,7 @@ [LibraryClasses]
   PrintLib
   QemuBootOrderLib
   QemuFwCfgLib
+  ReportStatusCodeLib
   UefiBootManagerLib
   UefiBootServicesTableLib
   UefiLib
diff --git a/ArmVirtPkg/Library/PlatformBootManagerLib/QemuKernel.c b/ArmVirtPkg/Library/PlatformBootManagerLib/QemuKernel.c
index ac47d21e71c8..7b59f57eb19f 100644
--- a/ArmVirtPkg/Library/PlatformBootManagerLib/QemuKernel.c
+++ b/ArmVirtPkg/Library/PlatformBootManagerLib/QemuKernel.c
@@ -20,6 +20,7 @@
 #include <Guid/FileSystemVolumeLabelInfo.h>
 #include <Library/PrintLib.h>
 #include <Library/QemuFwCfgLib.h>
+#include <Library/ReportStatusCodeLib.h>
 #include <Protocol/DevicePath.h>
 #include <Protocol/LoadedImage.h>
 #include <Protocol/SimpleFileSystem.h>
@@ -1072,6 +1073,9 @@ TryRunningQemuKernel (
   //
   EfiSignalEventReadyToBoot();
 
+  REPORT_STATUS_CODE (EFI_PROGRESS_CODE,
+    (EFI_SOFTWARE_DXE_BS_DRIVER | EFI_SW_DXE_BS_PC_READY_TO_BOOT_EVENT));
+
   //
   // Start the image.
   //
-- 
2.17.0

[PATCH v2 3/5] MdePkg/DxeServicesLib: introduce AllocatePeiAccessiblePages routine

[Ard Biesheuvel]

Add a routine to DxeServicesLib that abstracts the allocation of memory
that should be accessible by PEI after a warm reboot. We will use it to
replace open coded implementations that limit the address to < 4 GB,
which may not be possible on non-Intel systems that have no 32-bit
addressable memory at all.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 MdePkg/Include/Library/DxeServicesLib.h          | 23 +++++++-
 MdePkg/Library/DxeServicesLib/DxeServicesLib.c   | 55 ++++++++++++++++++++
 MdePkg/Library/DxeServicesLib/DxeServicesLib.inf |  3 ++
 3 files changed, 80 insertions(+), 1 deletion(-)

diff --git a/MdePkg/Include/Library/DxeServicesLib.h b/MdePkg/Include/Library/DxeServicesLib.h
index 7c1c62236d96..20aee68af558 100644
--- a/MdePkg/Include/Library/DxeServicesLib.h
+++ b/MdePkg/Include/Library/DxeServicesLib.h
@@ -305,5 +305,26 @@ GetFileDevicePathFromAnyFv (
   OUT       EFI_DEVICE_PATH_PROTOCOL  **FvFileDevicePath
   );
 
-#endif
+/**
+  Allocates one or more 4KB pages of a given type from a memory region that is
+  accessible to PEI.
+
+  Allocates the number of 4KB pages of type 'MemoryType' and returns a
+  pointer to the allocated buffer.  The buffer returned is aligned on a 4KB
+  boundary.  If Pages is 0, then NULL is returned.  If there is not enough
+  memory remaining to satisfy the request, then NULL is returned.
 
+  @param[in]  MemoryType            The memory type to allocate
+  @param[in]  Pages                 The number of 4 KB pages to allocate.
+
+  @return A pointer to the allocated buffer or NULL if allocation fails.
+
+**/
+VOID *
+EFIAPI
+AllocatePeiAccessiblePages (
+  IN EFI_MEMORY_TYPE  MemoryType,
+  IN UINTN            Pages
+  );
+
+#endif
diff --git a/MdePkg/Library/DxeServicesLib/DxeServicesLib.c b/MdePkg/Library/DxeServicesLib/DxeServicesLib.c
index 1827c9216fbc..6719aabe5d04 100644
--- a/MdePkg/Library/DxeServicesLib/DxeServicesLib.c
+++ b/MdePkg/Library/DxeServicesLib/DxeServicesLib.c
@@ -16,6 +16,7 @@
 
 #include <PiDxe.h>
 #include <Library/DebugLib.h>
+#include <Library/HobLib.h>
 #include <Library/MemoryAllocationLib.h>
 #include <Library/UefiBootServicesTableLib.h>
 #include <Library/DevicePathLib.h>
@@ -1093,3 +1094,57 @@ Done:
 
   return Status;
 }
+
+/**
+  Allocates one or more 4KB pages of a given type from a memory region that is
+  accessible to PEI.
+
+  Allocates the number of 4KB pages of type 'MemoryType' and returns a
+  pointer to the allocated buffer.  The buffer returned is aligned on a 4KB
+  boundary.  If Pages is 0, then NULL is returned.  If there is not enough
+  memory remaining to satisfy the request, then NULL is returned.
+
+  @param[in]  MemoryType            The memory type to allocate
+  @param[in]  Pages                 The number of 4 KB pages to allocate.
+
+  @return A pointer to the allocated buffer or NULL if allocation fails.
+
+**/
+VOID *
+EFIAPI
+AllocatePeiAccessiblePages (
+  IN EFI_MEMORY_TYPE  MemoryType,
+  IN UINTN            Pages
+  )
+{
+  EFI_STATUS                  Status;
+  EFI_ALLOCATE_TYPE           AllocType;
+  EFI_PHYSICAL_ADDRESS        Memory;
+#ifdef MDE_CPU_X64
+  EFI_HOB_HANDOFF_INFO_TABLE  *PhitHob;
+#endif
+
+  if (Pages == 0) {
+    return NULL;
+  }
+
+  AllocType = AllocateAnyPages;
+#ifdef MDE_CPU_X64
+  //
+  // On X64 systems, a X64 build of DXE may be combined with a 32-bit build of
+  // PEI, and so we need to check the memory limit set by PEI, and allocate
+  // below 4 GB if the limit is set to 4 GB or lower.
+  //
+  PhitHob = (EFI_HOB_HANDOFF_INFO_TABLE *)GetHobList ();
+  if (PhitHob->EfiFreeMemoryTop <= MAX_UINT32) {
+    AllocType = AllocateMaxAddress;
+    Memory = MAX_UINT32;
+  }
+#endif
+
+  Status = gBS->AllocatePages (AllocType, MemoryType, Pages, &Memory);
+  if (EFI_ERROR (Status)) {
+    return NULL;
+  }
+  return (VOID *)(UINTN)Memory;
+}
diff --git a/MdePkg/Library/DxeServicesLib/DxeServicesLib.inf b/MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
index bd2faf2f6f2d..b0306c09872f 100644
--- a/MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
+++ b/MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
@@ -44,6 +44,9 @@ [LibraryClasses]
   UefiLib
   UefiBootServicesTableLib
 
+[LibraryClasses.common.X64]
+  HobLib
+
 [Guids]
   gEfiFileInfoGuid                              ## SOMETIMES_CONSUMES ## UNDEFINED
 
-- 
2.17.0

[PATCH v2 4/5] MdeModulePkg/DxeCorePerformanceLib: use AllocatePeiAccessiblePages

[Ard Biesheuvel]

Replace the call to and implementation of the function
FpdtAllocateReservedMemoryBelow4G() with a call to
AllocatePeiAccessiblePages, which boils down to the same on X64,
but does not crash non-X64 systems that lack memory below 4 GB.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
Note that the ZeroMem() call is dropped, but it is redundant anyway, given
that the subsequent CopyMem() call supersedes it immediately.

 MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c | 45 ++------------------
 1 file changed, 4 insertions(+), 41 deletions(-)

diff --git a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
index 71d624fc9ce9..b1f09710b65c 100644
--- a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
+++ b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
@@ -165,46 +165,6 @@ IsKnownID (
   }
 }
 
-/**
-  Allocate EfiReservedMemoryType below 4G memory address.
-
-  This function allocates EfiReservedMemoryType below 4G memory address.
-
-  @param[in]  Size   Size of memory to allocate.
-
-  @return Allocated address for output.
-
-**/
-VOID *
-FpdtAllocateReservedMemoryBelow4G (
-  IN UINTN       Size
-  )
-{
-  UINTN                 Pages;
-  EFI_PHYSICAL_ADDRESS  Address;
-  EFI_STATUS            Status;
-  VOID                  *Buffer;
-
-  Buffer  = NULL;
-  Pages   = EFI_SIZE_TO_PAGES (Size);
-  Address = 0xffffffff;
-
-  Status = gBS->AllocatePages (
-                  AllocateMaxAddress,
-                  EfiReservedMemoryType,
-                  Pages,
-                  &Address
-                  );
-  ASSERT_EFI_ERROR (Status);
-
-  if (!EFI_ERROR (Status)) {
-    Buffer = (VOID *) (UINTN) Address;
-    ZeroMem (Buffer, Size);
-  }
-
-  return Buffer;
-}
-
 /**
   Allocate buffer for Boot Performance table.
 
@@ -348,7 +308,10 @@ AllocateBootPerformanceTable (
     //
     // Fail to allocate at specified address, continue to allocate at any address.
     //
-    mAcpiBootPerformanceTable = (BOOT_PERFORMANCE_TABLE *) FpdtAllocateReservedMemoryBelow4G (BootPerformanceDataSize);
+    mAcpiBootPerformanceTable = (BOOT_PERFORMANCE_TABLE *) AllocatePeiAccessiblePages (
+                                                             EfiReservedMemoryType,
+                                                             EFI_SIZE_TO_PAGES (BootPerformanceDataSize)
+                                                             );
   }
   DEBUG ((DEBUG_INFO, "DxeCorePerformanceLib: ACPI Boot Performance Table address = 0x%x\n", mAcpiBootPerformanceTable));
 
-- 
2.17.0

[PATCH v2 5/5] MdeModulePkg/FirmwarePerformanceDataTableDxe: use AllocatePeiAccessiblePages

[Ard Biesheuvel]

Replace the call to and implementation of the function
FpdtAllocateReservedMemoryBelow4G() with a call to
AllocatePeiAccessiblePages, which boils down to the same on X64,
but does not crash non-X64 systems that lack memory below 4 GB.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
Note that the ZeroMem() call is dropped, but it is redundant anyway, given
that in both cases, the subsequent CopyMem() call supersedes it immediately.

 MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.c   | 51 ++++----------------
 MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.inf |  1 +
 2 files changed, 10 insertions(+), 42 deletions(-)

diff --git a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.c b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.c
index e719e9e482cb..ded817f37301 100644
--- a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.c
+++ b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.c
@@ -32,6 +32,7 @@
 #include <Library/UefiRuntimeServicesTableLib.h>
 #include <Library/BaseLib.h>
 #include <Library/DebugLib.h>
+#include <Library/DxeServicesLib.h>
 #include <Library/TimerLib.h>
 #include <Library/BaseMemoryLib.h>
 #include <Library/MemoryAllocationLib.h>
@@ -179,46 +180,6 @@ FpdtAcpiTableChecksum (
   Buffer[ChecksumOffset] = CalculateCheckSum8 (Buffer, Size);
 }
 
-/**
-  Allocate EfiReservedMemoryType below 4G memory address.
-
-  This function allocates EfiReservedMemoryType below 4G memory address.
-
-  @param[in]  Size   Size of memory to allocate.
-
-  @return Allocated address for output.
-
-**/
-VOID *
-FpdtAllocateReservedMemoryBelow4G (
-  IN UINTN       Size
-  )
-{
-  UINTN                 Pages;
-  EFI_PHYSICAL_ADDRESS  Address;
-  EFI_STATUS            Status;
-  VOID                  *Buffer;
-
-  Buffer  = NULL;
-  Pages   = EFI_SIZE_TO_PAGES (Size);
-  Address = 0xffffffff;
-
-  Status = gBS->AllocatePages (
-                  AllocateMaxAddress,
-                  EfiReservedMemoryType,
-                  Pages,
-                  &Address
-                  );
-  ASSERT_EFI_ERROR (Status);
-
-  if (!EFI_ERROR (Status)) {
-    Buffer = (VOID *) (UINTN) Address;
-    ZeroMem (Buffer, Size);
-  }
-
-  return Buffer;
-}
-
 /**
   Callback function upon VariableArchProtocol and LockBoxProtocol
   to allocate S3 performance table memory and save the pointer to LockBox.
@@ -287,7 +248,10 @@ FpdtAllocateS3PerformanceTableMemory (
         //
         // Fail to allocate at specified address, continue to allocate at any address.
         //
-        mAcpiS3PerformanceTable = (S3_PERFORMANCE_TABLE *) FpdtAllocateReservedMemoryBelow4G (sizeof (S3_PERFORMANCE_TABLE));
+        mAcpiS3PerformanceTable = (S3_PERFORMANCE_TABLE *) AllocatePeiAccessiblePages (
+                                                             EfiReservedMemoryType,
+                                                             EFI_SIZE_TO_PAGES (sizeof (S3_PERFORMANCE_TABLE))
+                                                             );
       }
       DEBUG ((EFI_D_INFO, "FPDT: ACPI S3 Performance Table address = 0x%x\n", mAcpiS3PerformanceTable));
       if (mAcpiS3PerformanceTable != NULL) {
@@ -368,7 +332,10 @@ InstallFirmwarePerformanceDataTable (
       //
       // Fail to allocate at specified address, continue to allocate at any address.
       //
-      mAcpiBootPerformanceTable = (BOOT_PERFORMANCE_TABLE *) FpdtAllocateReservedMemoryBelow4G (BootPerformanceDataSize);
+      mAcpiBootPerformanceTable = (BOOT_PERFORMANCE_TABLE *) AllocatePeiAccessiblePages (
+                                                               EfiReservedMemoryType,
+                                                               EFI_SIZE_TO_PAGES (BootPerformanceDataSize)
+                                                               );
     }
     DEBUG ((DEBUG_INFO, "FPDT: ACPI Boot Performance Table address = 0x%x\n", mAcpiBootPerformanceTable));
     if (mAcpiBootPerformanceTable == NULL) {
diff --git a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.inf b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.inf
index 8757bbd0aaa9..3d2dd6eb732f 100644
--- a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.inf
+++ b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.inf
@@ -44,6 +44,7 @@ [LibraryClasses]
   UefiRuntimeServicesTableLib
   BaseLib
   DebugLib
+  DxeServicesLib
   TimerLib
   BaseMemoryLib
   MemoryAllocationLib
-- 
2.17.0

Re: [PATCH v2 3/5] MdePkg/DxeServicesLib: introduce AllocatePeiAccessiblePages routine

[Laszlo Ersek]

On 05/24/18 11:09, Ard Biesheuvel wrote:
> Add a routine to DxeServicesLib that abstracts the allocation of memory
> that should be accessible by PEI after a warm reboot. We will use it to
> replace open coded implementations that limit the address to < 4 GB,
> which may not be possible on non-Intel systems that have no 32-bit
> addressable memory at all.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  MdePkg/Include/Library/DxeServicesLib.h          | 23 +++++++-
>  MdePkg/Library/DxeServicesLib/DxeServicesLib.c   | 55 ++++++++++++++++++++
>  MdePkg/Library/DxeServicesLib/DxeServicesLib.inf |  3 ++
>  3 files changed, 80 insertions(+), 1 deletion(-)
> 
> diff --git a/MdePkg/Include/Library/DxeServicesLib.h b/MdePkg/Include/Library/DxeServicesLib.h
> index 7c1c62236d96..20aee68af558 100644
> --- a/MdePkg/Include/Library/DxeServicesLib.h
> +++ b/MdePkg/Include/Library/DxeServicesLib.h
> @@ -305,5 +305,26 @@ GetFileDevicePathFromAnyFv (
>    OUT       EFI_DEVICE_PATH_PROTOCOL  **FvFileDevicePath
>    );
>  
> -#endif
> +/**
> +  Allocates one or more 4KB pages of a given type from a memory region that is
> +  accessible to PEI.
> +
> +  Allocates the number of 4KB pages of type 'MemoryType' and returns a
> +  pointer to the allocated buffer.  The buffer returned is aligned on a 4KB
> +  boundary.  If Pages is 0, then NULL is returned.  If there is not enough
> +  memory remaining to satisfy the request, then NULL is returned.
>  
> +  @param[in]  MemoryType            The memory type to allocate
> +  @param[in]  Pages                 The number of 4 KB pages to allocate.
> +
> +  @return A pointer to the allocated buffer or NULL if allocation fails.
> +
> +**/
> +VOID *
> +EFIAPI
> +AllocatePeiAccessiblePages (
> +  IN EFI_MEMORY_TYPE  MemoryType,
> +  IN UINTN            Pages
> +  );
> +
> +#endif
> diff --git a/MdePkg/Library/DxeServicesLib/DxeServicesLib.c b/MdePkg/Library/DxeServicesLib/DxeServicesLib.c
> index 1827c9216fbc..6719aabe5d04 100644
> --- a/MdePkg/Library/DxeServicesLib/DxeServicesLib.c
> +++ b/MdePkg/Library/DxeServicesLib/DxeServicesLib.c
> @@ -16,6 +16,7 @@
>  
>  #include <PiDxe.h>
>  #include <Library/DebugLib.h>
> +#include <Library/HobLib.h>
>  #include <Library/MemoryAllocationLib.h>
>  #include <Library/UefiBootServicesTableLib.h>
>  #include <Library/DevicePathLib.h>
> @@ -1093,3 +1094,57 @@ Done:
>  
>    return Status;
>  }
> +
> +/**
> +  Allocates one or more 4KB pages of a given type from a memory region that is
> +  accessible to PEI.
> +
> +  Allocates the number of 4KB pages of type 'MemoryType' and returns a
> +  pointer to the allocated buffer.  The buffer returned is aligned on a 4KB
> +  boundary.  If Pages is 0, then NULL is returned.  If there is not enough
> +  memory remaining to satisfy the request, then NULL is returned.
> +
> +  @param[in]  MemoryType            The memory type to allocate
> +  @param[in]  Pages                 The number of 4 KB pages to allocate.
> +
> +  @return A pointer to the allocated buffer or NULL if allocation fails.
> +
> +**/
> +VOID *
> +EFIAPI
> +AllocatePeiAccessiblePages (
> +  IN EFI_MEMORY_TYPE  MemoryType,
> +  IN UINTN            Pages
> +  )
> +{
> +  EFI_STATUS                  Status;
> +  EFI_ALLOCATE_TYPE           AllocType;
> +  EFI_PHYSICAL_ADDRESS        Memory;
> +#ifdef MDE_CPU_X64
> +  EFI_HOB_HANDOFF_INFO_TABLE  *PhitHob;
> +#endif
> +
> +  if (Pages == 0) {
> +    return NULL;
> +  }
> +
> +  AllocType = AllocateAnyPages;
> +#ifdef MDE_CPU_X64
> +  //
> +  // On X64 systems, a X64 build of DXE may be combined with a 32-bit build of
> +  // PEI, and so we need to check the memory limit set by PEI, and allocate
> +  // below 4 GB if the limit is set to 4 GB or lower.
> +  //
> +  PhitHob = (EFI_HOB_HANDOFF_INFO_TABLE *)GetHobList ();
> +  if (PhitHob->EfiFreeMemoryTop <= MAX_UINT32) {
> +    AllocType = AllocateMaxAddress;
> +    Memory = MAX_UINT32;
> +  }
> +#endif
> +
> +  Status = gBS->AllocatePages (AllocType, MemoryType, Pages, &Memory);
> +  if (EFI_ERROR (Status)) {
> +    return NULL;
> +  }
> +  return (VOID *)(UINTN)Memory;
> +}
> diff --git a/MdePkg/Library/DxeServicesLib/DxeServicesLib.inf b/MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
> index bd2faf2f6f2d..b0306c09872f 100644
> --- a/MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
> +++ b/MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
> @@ -44,6 +44,9 @@ [LibraryClasses]
>    UefiLib
>    UefiBootServicesTableLib
>  
> +[LibraryClasses.common.X64]
> +  HobLib
> +
>  [Guids]
>    gEfiFileInfoGuid                              ## SOMETIMES_CONSUMES ## UNDEFINED
>  
> 

Perhaps the #include <Library/HobLib.h> directive could be guarded by
#ifdef MDE_CPU_X64 as well, just for symmetry with the HobLib lib class,
but it's not too important.

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

Re: [PATCH v2 4/5] MdeModulePkg/DxeCorePerformanceLib: use AllocatePeiAccessiblePages

[Laszlo Ersek]

On 05/24/18 11:09, Ard Biesheuvel wrote:
> Replace the call to and implementation of the function
> FpdtAllocateReservedMemoryBelow4G() with a call to
> AllocatePeiAccessiblePages, which boils down to the same on X64,
> but does not crash non-X64 systems that lack memory below 4 GB.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> Note that the ZeroMem() call is dropped, but it is redundant anyway, given
> that the subsequent CopyMem() call supersedes it immediately.

I'm not sure the ZeroMem() is redundant. The allocation size -- before rounding up to full pages -- is computed like this:

  BootPerformanceDataSize = sizeof (BOOT_PERFORMANCE_TABLE) + mPerformanceLength + PcdGet32 (PcdExtFpdtBootRecordPadSize);
  if (SmmCommData != NULL && SmmBootRecordData != NULL) {
    BootPerformanceDataSize += SmmBootRecordDataSize;
  }

ZeroMem() covers all of the above, but the CopyMem() calls don't seem to cover the optional padding (from the PCD).

I'm unsure if that matters, of course.

The patch looks good to me otherwise.

Thanks
Laszlo


> 
>  MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c | 45 ++------------------
>  1 file changed, 4 insertions(+), 41 deletions(-)
> 
> diff --git a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
> index 71d624fc9ce9..b1f09710b65c 100644
> --- a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
> +++ b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
> @@ -165,46 +165,6 @@ IsKnownID (
>    }
>  }
>  
> -/**
> -  Allocate EfiReservedMemoryType below 4G memory address.
> -
> -  This function allocates EfiReservedMemoryType below 4G memory address.
> -
> -  @param[in]  Size   Size of memory to allocate.
> -
> -  @return Allocated address for output.
> -
> -**/
> -VOID *
> -FpdtAllocateReservedMemoryBelow4G (
> -  IN UINTN       Size
> -  )
> -{
> -  UINTN                 Pages;
> -  EFI_PHYSICAL_ADDRESS  Address;
> -  EFI_STATUS            Status;
> -  VOID                  *Buffer;
> -
> -  Buffer  = NULL;
> -  Pages   = EFI_SIZE_TO_PAGES (Size);
> -  Address = 0xffffffff;
> -
> -  Status = gBS->AllocatePages (
> -                  AllocateMaxAddress,
> -                  EfiReservedMemoryType,
> -                  Pages,
> -                  &Address
> -                  );
> -  ASSERT_EFI_ERROR (Status);
> -
> -  if (!EFI_ERROR (Status)) {
> -    Buffer = (VOID *) (UINTN) Address;
> -    ZeroMem (Buffer, Size);
> -  }
> -
> -  return Buffer;
> -}
> -
>  /**
>    Allocate buffer for Boot Performance table.
>  
> @@ -348,7 +308,10 @@ AllocateBootPerformanceTable (
>      //
>      // Fail to allocate at specified address, continue to allocate at any address.
>      //
> -    mAcpiBootPerformanceTable = (BOOT_PERFORMANCE_TABLE *) FpdtAllocateReservedMemoryBelow4G (BootPerformanceDataSize);
> +    mAcpiBootPerformanceTable = (BOOT_PERFORMANCE_TABLE *) AllocatePeiAccessiblePages (
> +                                                             EfiReservedMemoryType,
> +                                                             EFI_SIZE_TO_PAGES (BootPerformanceDataSize)
> +                                                             );
>    }
>    DEBUG ((DEBUG_INFO, "DxeCorePerformanceLib: ACPI Boot Performance Table address = 0x%x\n", mAcpiBootPerformanceTable));
>  
> 

Re: [PATCH v2 5/5] MdeModulePkg/FirmwarePerformanceDataTableDxe: use AllocatePeiAccessiblePages

[Laszlo Ersek]

On 05/24/18 11:09, Ard Biesheuvel wrote:
> Replace the call to and implementation of the function
> FpdtAllocateReservedMemoryBelow4G() with a call to
> AllocatePeiAccessiblePages, which boils down to the same on X64,
> but does not crash non-X64 systems that lack memory below 4 GB.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> Note that the ZeroMem() call is dropped, but it is redundant anyway, given
> that in both cases, the subsequent CopyMem() call supersedes it immediately.
> 
>  MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.c   | 51 ++++----------------
>  MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.inf |  1 +
>  2 files changed, 10 insertions(+), 42 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.c b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.c
> index e719e9e482cb..ded817f37301 100644
> --- a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.c
> +++ b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.c
> @@ -32,6 +32,7 @@
>  #include <Library/UefiRuntimeServicesTableLib.h>
>  #include <Library/BaseLib.h>
>  #include <Library/DebugLib.h>
> +#include <Library/DxeServicesLib.h>
>  #include <Library/TimerLib.h>
>  #include <Library/BaseMemoryLib.h>
>  #include <Library/MemoryAllocationLib.h>
> @@ -179,46 +180,6 @@ FpdtAcpiTableChecksum (
>    Buffer[ChecksumOffset] = CalculateCheckSum8 (Buffer, Size);
>  }
>  
> -/**
> -  Allocate EfiReservedMemoryType below 4G memory address.
> -
> -  This function allocates EfiReservedMemoryType below 4G memory address.
> -
> -  @param[in]  Size   Size of memory to allocate.
> -
> -  @return Allocated address for output.
> -
> -**/
> -VOID *
> -FpdtAllocateReservedMemoryBelow4G (
> -  IN UINTN       Size
> -  )
> -{
> -  UINTN                 Pages;
> -  EFI_PHYSICAL_ADDRESS  Address;
> -  EFI_STATUS            Status;
> -  VOID                  *Buffer;
> -
> -  Buffer  = NULL;
> -  Pages   = EFI_SIZE_TO_PAGES (Size);
> -  Address = 0xffffffff;
> -
> -  Status = gBS->AllocatePages (
> -                  AllocateMaxAddress,
> -                  EfiReservedMemoryType,
> -                  Pages,
> -                  &Address
> -                  );
> -  ASSERT_EFI_ERROR (Status);
> -
> -  if (!EFI_ERROR (Status)) {
> -    Buffer = (VOID *) (UINTN) Address;
> -    ZeroMem (Buffer, Size);
> -  }
> -
> -  return Buffer;
> -}
> -
>  /**
>    Callback function upon VariableArchProtocol and LockBoxProtocol
>    to allocate S3 performance table memory and save the pointer to LockBox.
> @@ -287,7 +248,10 @@ FpdtAllocateS3PerformanceTableMemory (
>          //
>          // Fail to allocate at specified address, continue to allocate at any address.
>          //
> -        mAcpiS3PerformanceTable = (S3_PERFORMANCE_TABLE *) FpdtAllocateReservedMemoryBelow4G (sizeof (S3_PERFORMANCE_TABLE));
> +        mAcpiS3PerformanceTable = (S3_PERFORMANCE_TABLE *) AllocatePeiAccessiblePages (
> +                                                             EfiReservedMemoryType,
> +                                                             EFI_SIZE_TO_PAGES (sizeof (S3_PERFORMANCE_TABLE))
> +                                                             );
>        }
>        DEBUG ((EFI_D_INFO, "FPDT: ACPI S3 Performance Table address = 0x%x\n", mAcpiS3PerformanceTable));
>        if (mAcpiS3PerformanceTable != NULL) {
> @@ -368,7 +332,10 @@ InstallFirmwarePerformanceDataTable (
>        //
>        // Fail to allocate at specified address, continue to allocate at any address.
>        //
> -      mAcpiBootPerformanceTable = (BOOT_PERFORMANCE_TABLE *) FpdtAllocateReservedMemoryBelow4G (BootPerformanceDataSize);
> +      mAcpiBootPerformanceTable = (BOOT_PERFORMANCE_TABLE *) AllocatePeiAccessiblePages (
> +                                                               EfiReservedMemoryType,
> +                                                               EFI_SIZE_TO_PAGES (BootPerformanceDataSize)
> +                                                               );
>      }
>      DEBUG ((DEBUG_INFO, "FPDT: ACPI Boot Performance Table address = 0x%x\n", mAcpiBootPerformanceTable));
>      if (mAcpiBootPerformanceTable == NULL) {
> diff --git a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.inf b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.inf
> index 8757bbd0aaa9..3d2dd6eb732f 100644
> --- a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.inf
> +++ b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.inf
> @@ -44,6 +44,7 @@ [LibraryClasses]
>    UefiRuntimeServicesTableLib
>    BaseLib
>    DebugLib
> +  DxeServicesLib
>    TimerLib
>    BaseMemoryLib
>    MemoryAllocationLib
> 

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

Re: [PATCH v2 4/5] MdeModulePkg/DxeCorePerformanceLib: use AllocatePeiAccessiblePages

[Ard Biesheuvel]

On 24 May 2018 at 14:49, Laszlo Ersek <lersek@redhat.com> wrote:
> On 05/24/18 11:09, Ard Biesheuvel wrote:
>> Replace the call to and implementation of the function
>> FpdtAllocateReservedMemoryBelow4G() with a call to
>> AllocatePeiAccessiblePages, which boils down to the same on X64,
>> but does not crash non-X64 systems that lack memory below 4 GB.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>> Note that the ZeroMem() call is dropped, but it is redundant anyway, given
>> that the subsequent CopyMem() call supersedes it immediately.
>
> I'm not sure the ZeroMem() is redundant. The allocation size -- before rounding up to full pages -- is computed like this:
>
>   BootPerformanceDataSize = sizeof (BOOT_PERFORMANCE_TABLE) + mPerformanceLength + PcdGet32 (PcdExtFpdtBootRecordPadSize);
>   if (SmmCommData != NULL && SmmBootRecordData != NULL) {
>     BootPerformanceDataSize += SmmBootRecordDataSize;
>   }
>
> ZeroMem() covers all of the above, but the CopyMem() calls don't seem to cover the optional padding (from the PCD).
>
> I'm unsure if that matters, of course.
>

You're quite right. I'm not sure how I missed that.

In any case, I can add back the ZeroMem () quite easily after the
single invocation of AllocatePeiAccessiblePages() that I am adding in
this file.

> The patch looks good to me otherwise.
>
> Thanks
> Laszlo
>
>
>>
>>  MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c | 45 ++------------------
>>  1 file changed, 4 insertions(+), 41 deletions(-)
>>
>> diff --git a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
>> index 71d624fc9ce9..b1f09710b65c 100644
>> --- a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
>> +++ b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
>> @@ -165,46 +165,6 @@ IsKnownID (
>>    }
>>  }
>>
>> -/**
>> -  Allocate EfiReservedMemoryType below 4G memory address.
>> -
>> -  This function allocates EfiReservedMemoryType below 4G memory address.
>> -
>> -  @param[in]  Size   Size of memory to allocate.
>> -
>> -  @return Allocated address for output.
>> -
>> -**/
>> -VOID *
>> -FpdtAllocateReservedMemoryBelow4G (
>> -  IN UINTN       Size
>> -  )
>> -{
>> -  UINTN                 Pages;
>> -  EFI_PHYSICAL_ADDRESS  Address;
>> -  EFI_STATUS            Status;
>> -  VOID                  *Buffer;
>> -
>> -  Buffer  = NULL;
>> -  Pages   = EFI_SIZE_TO_PAGES (Size);
>> -  Address = 0xffffffff;
>> -
>> -  Status = gBS->AllocatePages (
>> -                  AllocateMaxAddress,
>> -                  EfiReservedMemoryType,
>> -                  Pages,
>> -                  &Address
>> -                  );
>> -  ASSERT_EFI_ERROR (Status);
>> -
>> -  if (!EFI_ERROR (Status)) {
>> -    Buffer = (VOID *) (UINTN) Address;
>> -    ZeroMem (Buffer, Size);
>> -  }
>> -
>> -  return Buffer;
>> -}
>> -
>>  /**
>>    Allocate buffer for Boot Performance table.
>>
>> @@ -348,7 +308,10 @@ AllocateBootPerformanceTable (
>>      //
>>      // Fail to allocate at specified address, continue to allocate at any address.
>>      //
>> -    mAcpiBootPerformanceTable = (BOOT_PERFORMANCE_TABLE *) FpdtAllocateReservedMemoryBelow4G (BootPerformanceDataSize);
>> +    mAcpiBootPerformanceTable = (BOOT_PERFORMANCE_TABLE *) AllocatePeiAccessiblePages (
>> +                                                             EfiReservedMemoryType,
>> +                                                             EFI_SIZE_TO_PAGES (BootPerformanceDataSize)
>> +                                                             );
>>    }
>>    DEBUG ((DEBUG_INFO, "DxeCorePerformanceLib: ACPI Boot Performance Table address = 0x%x\n", mAcpiBootPerformanceTable));
>>
>>
>

Re: [PATCH v2 4/5] MdeModulePkg/DxeCorePerformanceLib: use AllocatePeiAccessiblePages

[Laszlo Ersek]

On 05/24/18 14:54, Ard Biesheuvel wrote:
> On 24 May 2018 at 14:49, Laszlo Ersek <lersek@redhat.com> wrote:
>> On 05/24/18 11:09, Ard Biesheuvel wrote:
>>> Replace the call to and implementation of the function
>>> FpdtAllocateReservedMemoryBelow4G() with a call to
>>> AllocatePeiAccessiblePages, which boils down to the same on X64,
>>> but does not crash non-X64 systems that lack memory below 4 GB.
>>>
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>> ---
>>> Note that the ZeroMem() call is dropped, but it is redundant anyway, given
>>> that the subsequent CopyMem() call supersedes it immediately.
>>
>> I'm not sure the ZeroMem() is redundant. The allocation size -- before rounding up to full pages -- is computed like this:
>>
>>   BootPerformanceDataSize = sizeof (BOOT_PERFORMANCE_TABLE) + mPerformanceLength + PcdGet32 (PcdExtFpdtBootRecordPadSize);
>>   if (SmmCommData != NULL && SmmBootRecordData != NULL) {
>>     BootPerformanceDataSize += SmmBootRecordDataSize;
>>   }
>>
>> ZeroMem() covers all of the above, but the CopyMem() calls don't seem to cover the optional padding (from the PCD).
>>
>> I'm unsure if that matters, of course.
>>
> 
> You're quite right. I'm not sure how I missed that.
> 
> In any case, I can add back the ZeroMem () quite easily after the
> single invocation of AllocatePeiAccessiblePages() that I am adding in
> this file.

Thanks! With that:

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

Laszlo

> 
>> The patch looks good to me otherwise.
>>
>> Thanks
>> Laszlo
>>
>>
>>>
>>>  MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c | 45 ++------------------
>>>  1 file changed, 4 insertions(+), 41 deletions(-)
>>>
>>> diff --git a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
>>> index 71d624fc9ce9..b1f09710b65c 100644
>>> --- a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
>>> +++ b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
>>> @@ -165,46 +165,6 @@ IsKnownID (
>>>    }
>>>  }
>>>
>>> -/**
>>> -  Allocate EfiReservedMemoryType below 4G memory address.
>>> -
>>> -  This function allocates EfiReservedMemoryType below 4G memory address.
>>> -
>>> -  @param[in]  Size   Size of memory to allocate.
>>> -
>>> -  @return Allocated address for output.
>>> -
>>> -**/
>>> -VOID *
>>> -FpdtAllocateReservedMemoryBelow4G (
>>> -  IN UINTN       Size
>>> -  )
>>> -{
>>> -  UINTN                 Pages;
>>> -  EFI_PHYSICAL_ADDRESS  Address;
>>> -  EFI_STATUS            Status;
>>> -  VOID                  *Buffer;
>>> -
>>> -  Buffer  = NULL;
>>> -  Pages   = EFI_SIZE_TO_PAGES (Size);
>>> -  Address = 0xffffffff;
>>> -
>>> -  Status = gBS->AllocatePages (
>>> -                  AllocateMaxAddress,
>>> -                  EfiReservedMemoryType,
>>> -                  Pages,
>>> -                  &Address
>>> -                  );
>>> -  ASSERT_EFI_ERROR (Status);
>>> -
>>> -  if (!EFI_ERROR (Status)) {
>>> -    Buffer = (VOID *) (UINTN) Address;
>>> -    ZeroMem (Buffer, Size);
>>> -  }
>>> -
>>> -  return Buffer;
>>> -}
>>> -
>>>  /**
>>>    Allocate buffer for Boot Performance table.
>>>
>>> @@ -348,7 +308,10 @@ AllocateBootPerformanceTable (
>>>      //
>>>      // Fail to allocate at specified address, continue to allocate at any address.
>>>      //
>>> -    mAcpiBootPerformanceTable = (BOOT_PERFORMANCE_TABLE *) FpdtAllocateReservedMemoryBelow4G (BootPerformanceDataSize);
>>> +    mAcpiBootPerformanceTable = (BOOT_PERFORMANCE_TABLE *) AllocatePeiAccessiblePages (
>>> +                                                             EfiReservedMemoryType,
>>> +                                                             EFI_SIZE_TO_PAGES (BootPerformanceDataSize)
>>> +                                                             );
>>>    }
>>>    DEBUG ((DEBUG_INFO, "DxeCorePerformanceLib: ACPI Boot Performance Table address = 0x%x\n", mAcpiBootPerformanceTable));
>>>
>>>
>>

Re: [PATCH v2 0/5] Abstract allocation of PEI accessible memory

[Kinney, Michael D]

Ard,

Thanks for the update.  This looks really close.

One comment is on the use of #ifdef MDE_CPU_X64
in a C file.

In general, the #ifdef for a specific type of CPU
should be limited to .h files and we should split
out CPU specific sources into their own source
files and use the INF file [Sources.<arch>] to
list the CPU specific source files.

Only other minor comment is that the summary still
mentions adding the new API to the UefiLib.

Mike


> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Thursday, May 24, 2018 2:10 AM
> To: edk2-devel@lists.01.org
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Laszlo
> Ersek <lersek@redhat.com>; Leif Lindholm
> <leif.lindholm@linaro.org>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Gao, Liming
> <liming.gao@intel.com>; Zeng, Star
> <star.zeng@intel.com>; Dong, Eric <eric.dong@intel.com>;
> Bi, Dandan <dandan.bi@intel.com>
> Subject: [PATCH v2 0/5] Abstract allocation of PEI
> accessible memory
> 
> At the moment, FirmwarePerformanceTableDataDxe or
> DxeCorePerformanceLib
> are unusable on systems such as AMD Seattle, because
> they unconditionally
> attempt to allocate memory below 4 GB, and ASSERT() if
> this fails. On AMD
> Seattle, no 32-bit addressable DRAM exists, and so the
> driver will always
> assert, and crash a running DEBUG build.
> 
> The reason for this is that some platforms (i.e., X64
> builds consisting of
> a 32-bit PEI stage and 64-bit remaining stages) require
> these allocations
> to be below 4 GB, and for some reason, open coding this
> practice throughout
> the code without regard for other architectures has been
> regarded as an
> acceptable approach.
> 
> Instead, I would like to propose the approach
> implemented in this series:
> - create an abstracted EfiAllocatePeiAccessiblePages()
> routine in UefiLib
>   that simply allocates pages from any region on all
> architectures except
>   X64, and allocate below 4 GB for X64
> - update the various call sites with calls to this new
> function.
> 
> The above is implemented in patches #3 - #6. Patches #1
> and #2 are fixes
> for issues that I observed in ArmVirtPkg and OvmfPkg
> while working on
> these patches.
> 
> Code can be found here:
> https://github.com/ardbiesheuvel/edk2/tree/allocate-pei-
> v2
> 
> Changes since v1:
> - add Laszlo's ack to #1 - #2
> - move EfiAllocatePeiPages() to DxeServicesLib and drop
> Efi prefix
> - update implementation to take EfiFreeMemoryTop into
> account when built
>   for X64
> 
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Dandan Bi <dandan.bi@intel.com>
> 
> Ard Biesheuvel (5):
>   OvmfPkg/PlatformBootManagerLib: add missing report
> status code call
>   ArmVirtPkg/PlatformBootManagerLib: add missing report
> status code call
>   MdePkg/DxeServicesLib: introduce
> AllocatePeiAccessiblePages routine
>   MdeModulePkg/DxeCorePerformanceLib: use
> AllocatePeiAccessiblePages
>   MdeModulePkg/FirmwarePerformanceDataTableDxe: use
>     AllocatePeiAccessiblePages
> 
>  .../PlatformBootManagerLib.inf                |  1 +
>  .../PlatformBootManagerLib/QemuKernel.c       |  4 ++
>  .../DxeCorePerformanceLib.c                   | 45 ++--
> -----------
>  .../FirmwarePerformanceDxe.c                  | 51 +++-
> -------------
>  .../FirmwarePerformanceDxe.inf                |  1 +
>  MdePkg/Include/Library/DxeServicesLib.h       | 23
> +++++++-
>  .../Library/DxeServicesLib/DxeServicesLib.c   | 55
> +++++++++++++++++++
>  .../Library/DxeServicesLib/DxeServicesLib.inf |  3 +
>  .../PlatformBootManagerLib.inf                |  1 +
>  .../PlatformBootManagerLib/QemuKernel.c       |  4 ++
>  10 files changed, 104 insertions(+), 84 deletions(-)
> 
> --
> 2.17.0

Re: [PATCH v2 4/5] MdeModulePkg/DxeCorePerformanceLib: use AllocatePeiAccessiblePages

[Bi, Dandan]

Hi Ard,

Thank you very much for helping fix this issue.

And for ZeroMem(), it should be added back.
Because after the memory is allocated, the optional padding (from the PCD) memory will be used directly. 
If ZeoMem() is dropped, then some contents in the padding memory will not be zero. Thus will impact the tool to parse the contents.

Thanks,
Dandan
-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard Biesheuvel
Sent: Thursday, May 24, 2018 8:54 PM
To: Laszlo Ersek <lersek@redhat.com>
Cc: Dong, Eric <eric.dong@intel.com>; edk2-devel@lists.01.org; Gao, Liming <liming.gao@intel.com>; Bi, Dandan <dandan.bi@intel.com>; Leif Lindholm <leif.lindholm@linaro.org>; Kinney, Michael D <michael.d.kinney@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: Re: [edk2] [PATCH v2 4/5] MdeModulePkg/DxeCorePerformanceLib: use AllocatePeiAccessiblePages

On 24 May 2018 at 14:49, Laszlo Ersek <lersek@redhat.com> wrote:
> On 05/24/18 11:09, Ard Biesheuvel wrote:
>> Replace the call to and implementation of the function
>> FpdtAllocateReservedMemoryBelow4G() with a call to 
>> AllocatePeiAccessiblePages, which boils down to the same on X64, but 
>> does not crash non-X64 systems that lack memory below 4 GB.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>> Note that the ZeroMem() call is dropped, but it is redundant anyway, 
>> given that the subsequent CopyMem() call supersedes it immediately.
>
> I'm not sure the ZeroMem() is redundant. The allocation size -- before rounding up to full pages -- is computed like this:
>
>   BootPerformanceDataSize = sizeof (BOOT_PERFORMANCE_TABLE) + mPerformanceLength + PcdGet32 (PcdExtFpdtBootRecordPadSize);
>   if (SmmCommData != NULL && SmmBootRecordData != NULL) {
>     BootPerformanceDataSize += SmmBootRecordDataSize;
>   }
>
> ZeroMem() covers all of the above, but the CopyMem() calls don't seem to cover the optional padding (from the PCD).
>
> I'm unsure if that matters, of course.
>

You're quite right. I'm not sure how I missed that.

In any case, I can add back the ZeroMem () quite easily after the single invocation of AllocatePeiAccessiblePages() that I am adding in this file.

> The patch looks good to me otherwise.
>
> Thanks
> Laszlo
>
>
>>
>>  MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c | 
>> 45 ++------------------
>>  1 file changed, 4 insertions(+), 41 deletions(-)
>>
>> diff --git 
>> a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c 
>> b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
>> index 71d624fc9ce9..b1f09710b65c 100644
>> --- 
>> a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
>> +++ b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLi
>> +++ b.c
>> @@ -165,46 +165,6 @@ IsKnownID (
>>    }
>>  }
>>
>> -/**
>> -  Allocate EfiReservedMemoryType below 4G memory address.
>> -
>> -  This function allocates EfiReservedMemoryType below 4G memory address.
>> -
>> -  @param[in]  Size   Size of memory to allocate.
>> -
>> -  @return Allocated address for output.
>> -
>> -**/
>> -VOID *
>> -FpdtAllocateReservedMemoryBelow4G (
>> -  IN UINTN       Size
>> -  )
>> -{
>> -  UINTN                 Pages;
>> -  EFI_PHYSICAL_ADDRESS  Address;
>> -  EFI_STATUS            Status;
>> -  VOID                  *Buffer;
>> -
>> -  Buffer  = NULL;
>> -  Pages   = EFI_SIZE_TO_PAGES (Size);
>> -  Address = 0xffffffff;
>> -
>> -  Status = gBS->AllocatePages (
>> -                  AllocateMaxAddress,
>> -                  EfiReservedMemoryType,
>> -                  Pages,
>> -                  &Address
>> -                  );
>> -  ASSERT_EFI_ERROR (Status);
>> -
>> -  if (!EFI_ERROR (Status)) {
>> -    Buffer = (VOID *) (UINTN) Address;
>> -    ZeroMem (Buffer, Size);
>> -  }
>> -
>> -  return Buffer;
>> -}
>> -
>>  /**
>>    Allocate buffer for Boot Performance table.
>>
>> @@ -348,7 +308,10 @@ AllocateBootPerformanceTable (
>>      //
>>      // Fail to allocate at specified address, continue to allocate at any address.
>>      //
>> -    mAcpiBootPerformanceTable = (BOOT_PERFORMANCE_TABLE *) FpdtAllocateReservedMemoryBelow4G (BootPerformanceDataSize);
>> +    mAcpiBootPerformanceTable = (BOOT_PERFORMANCE_TABLE *) AllocatePeiAccessiblePages (
>> +                                                             EfiReservedMemoryType,
>> +                                                             EFI_SIZE_TO_PAGES (BootPerformanceDataSize)
>> +                                                             );
>>    }
>>    DEBUG ((DEBUG_INFO, "DxeCorePerformanceLib: ACPI Boot Performance 
>> Table address = 0x%x\n", mAcpiBootPerformanceTable));
>>
>>
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [PATCH v2 4/5] MdeModulePkg/DxeCorePerformanceLib: use AllocatePeiAccessiblePages

[Zeng, Star]

mAcpiBootPerformanceTable->Header.Length could reflect the real table length, right?
When the padding (from the PCD) memory is used, mAcpiBootPerformanceTable->Header.Length will be updated accordingly.
If that is the case, it seems safe without ZeroMem.
Anyway it needs to be verified.


But, to be simple, I do not object to have ZeroMem to be equivalent with previous code.



Thanks,
Star
-----Original Message-----
From: Bi, Dandan 
Sent: Friday, May 25, 2018 10:01 AM
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Laszlo Ersek <lersek@redhat.com>
Cc: Dong, Eric <eric.dong@intel.com>; edk2-devel@lists.01.org; Gao, Liming <liming.gao@intel.com>; Leif Lindholm <leif.lindholm@linaro.org>; Kinney, Michael D <michael.d.kinney@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: RE: [edk2] [PATCH v2 4/5] MdeModulePkg/DxeCorePerformanceLib: use AllocatePeiAccessiblePages

Hi Ard,

Thank you very much for helping fix this issue.

And for ZeroMem(), it should be added back.
Because after the memory is allocated, the optional padding (from the PCD) memory will be used directly. 
If ZeoMem() is dropped, then some contents in the padding memory will not be zero. Thus will impact the tool to parse the contents.

Thanks,
Dandan
-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard Biesheuvel
Sent: Thursday, May 24, 2018 8:54 PM
To: Laszlo Ersek <lersek@redhat.com>
Cc: Dong, Eric <eric.dong@intel.com>; edk2-devel@lists.01.org; Gao, Liming <liming.gao@intel.com>; Bi, Dandan <dandan.bi@intel.com>; Leif Lindholm <leif.lindholm@linaro.org>; Kinney, Michael D <michael.d.kinney@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: Re: [edk2] [PATCH v2 4/5] MdeModulePkg/DxeCorePerformanceLib: use AllocatePeiAccessiblePages

On 24 May 2018 at 14:49, Laszlo Ersek <lersek@redhat.com> wrote:
> On 05/24/18 11:09, Ard Biesheuvel wrote:
>> Replace the call to and implementation of the function
>> FpdtAllocateReservedMemoryBelow4G() with a call to 
>> AllocatePeiAccessiblePages, which boils down to the same on X64, but 
>> does not crash non-X64 systems that lack memory below 4 GB.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>> Note that the ZeroMem() call is dropped, but it is redundant anyway, 
>> given that the subsequent CopyMem() call supersedes it immediately.
>
> I'm not sure the ZeroMem() is redundant. The allocation size -- before rounding up to full pages -- is computed like this:
>
>   BootPerformanceDataSize = sizeof (BOOT_PERFORMANCE_TABLE) + mPerformanceLength + PcdGet32 (PcdExtFpdtBootRecordPadSize);
>   if (SmmCommData != NULL && SmmBootRecordData != NULL) {
>     BootPerformanceDataSize += SmmBootRecordDataSize;
>   }
>
> ZeroMem() covers all of the above, but the CopyMem() calls don't seem to cover the optional padding (from the PCD).
>
> I'm unsure if that matters, of course.
>

You're quite right. I'm not sure how I missed that.

In any case, I can add back the ZeroMem () quite easily after the single invocation of AllocatePeiAccessiblePages() that I am adding in this file.

> The patch looks good to me otherwise.
>
> Thanks
> Laszlo
>
>
>>
>>  MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c |
>> 45 ++------------------
>>  1 file changed, 4 insertions(+), 41 deletions(-)
>>
>> diff --git
>> a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
>> b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
>> index 71d624fc9ce9..b1f09710b65c 100644
>> ---
>> a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
>> +++ b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLi
>> +++ b.c
>> @@ -165,46 +165,6 @@ IsKnownID (
>>    }
>>  }
>>
>> -/**
>> -  Allocate EfiReservedMemoryType below 4G memory address.
>> -
>> -  This function allocates EfiReservedMemoryType below 4G memory address.
>> -
>> -  @param[in]  Size   Size of memory to allocate.
>> -
>> -  @return Allocated address for output.
>> -
>> -**/
>> -VOID *
>> -FpdtAllocateReservedMemoryBelow4G (
>> -  IN UINTN       Size
>> -  )
>> -{
>> -  UINTN                 Pages;
>> -  EFI_PHYSICAL_ADDRESS  Address;
>> -  EFI_STATUS            Status;
>> -  VOID                  *Buffer;
>> -
>> -  Buffer  = NULL;
>> -  Pages   = EFI_SIZE_TO_PAGES (Size);
>> -  Address = 0xffffffff;
>> -
>> -  Status = gBS->AllocatePages (
>> -                  AllocateMaxAddress,
>> -                  EfiReservedMemoryType,
>> -                  Pages,
>> -                  &Address
>> -                  );
>> -  ASSERT_EFI_ERROR (Status);
>> -
>> -  if (!EFI_ERROR (Status)) {
>> -    Buffer = (VOID *) (UINTN) Address;
>> -    ZeroMem (Buffer, Size);
>> -  }
>> -
>> -  return Buffer;
>> -}
>> -
>>  /**
>>    Allocate buffer for Boot Performance table.
>>
>> @@ -348,7 +308,10 @@ AllocateBootPerformanceTable (
>>      //
>>      // Fail to allocate at specified address, continue to allocate at any address.
>>      //
>> -    mAcpiBootPerformanceTable = (BOOT_PERFORMANCE_TABLE *) FpdtAllocateReservedMemoryBelow4G (BootPerformanceDataSize);
>> +    mAcpiBootPerformanceTable = (BOOT_PERFORMANCE_TABLE *) AllocatePeiAccessiblePages (
>> +                                                             EfiReservedMemoryType,
>> +                                                             EFI_SIZE_TO_PAGES (BootPerformanceDataSize)
>> +                                                             );
>>    }
>>    DEBUG ((DEBUG_INFO, "DxeCorePerformanceLib: ACPI Boot Performance 
>> Table address = 0x%x\n", mAcpiBootPerformanceTable));
>>
>>
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [PATCH v2 4/5] MdeModulePkg/DxeCorePerformanceLib: use AllocatePeiAccessiblePages

[Bi, Dandan]

Hi Star,

Yes. You are right. mAcpiBootPerformanceTable->Header.Length will be updated accordingly. We can get the real table length.
Currently there are some contents in the table are not updated, just use the value in the memory.
Previously the memory value is zero.  But without ZeroMem(), the value may be non-zero. That's the difference. It may impact the code to parse the table.

Thanks,
Dandan

-----Original Message-----
From: Zeng, Star 
Sent: Friday, May 25, 2018 10:45 AM
To: Bi, Dandan <dandan.bi@intel.com>; Ard Biesheuvel <ard.biesheuvel@linaro.org>; Laszlo Ersek <lersek@redhat.com>
Cc: Dong, Eric <eric.dong@intel.com>; edk2-devel@lists.01.org; Gao, Liming <liming.gao@intel.com>; Leif Lindholm <leif.lindholm@linaro.org>; Kinney, Michael D <michael.d.kinney@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: RE: [edk2] [PATCH v2 4/5] MdeModulePkg/DxeCorePerformanceLib: use AllocatePeiAccessiblePages

mAcpiBootPerformanceTable->Header.Length could reflect the real table length, right?
When the padding (from the PCD) memory is used, mAcpiBootPerformanceTable->Header.Length will be updated accordingly.
If that is the case, it seems safe without ZeroMem.
Anyway it needs to be verified.


But, to be simple, I do not object to have ZeroMem to be equivalent with previous code.



Thanks,
Star
-----Original Message-----
From: Bi, Dandan
Sent: Friday, May 25, 2018 10:01 AM
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Laszlo Ersek <lersek@redhat.com>
Cc: Dong, Eric <eric.dong@intel.com>; edk2-devel@lists.01.org; Gao, Liming <liming.gao@intel.com>; Leif Lindholm <leif.lindholm@linaro.org>; Kinney, Michael D <michael.d.kinney@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: RE: [edk2] [PATCH v2 4/5] MdeModulePkg/DxeCorePerformanceLib: use AllocatePeiAccessiblePages

Hi Ard,

Thank you very much for helping fix this issue.

And for ZeroMem(), it should be added back.
Because after the memory is allocated, the optional padding (from the PCD) memory will be used directly. 
If ZeoMem() is dropped, then some contents in the padding memory will not be zero. Thus will impact the tool to parse the contents.

Thanks,
Dandan
-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard Biesheuvel
Sent: Thursday, May 24, 2018 8:54 PM
To: Laszlo Ersek <lersek@redhat.com>
Cc: Dong, Eric <eric.dong@intel.com>; edk2-devel@lists.01.org; Gao, Liming <liming.gao@intel.com>; Bi, Dandan <dandan.bi@intel.com>; Leif Lindholm <leif.lindholm@linaro.org>; Kinney, Michael D <michael.d.kinney@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: Re: [edk2] [PATCH v2 4/5] MdeModulePkg/DxeCorePerformanceLib: use AllocatePeiAccessiblePages

On 24 May 2018 at 14:49, Laszlo Ersek <lersek@redhat.com> wrote:
> On 05/24/18 11:09, Ard Biesheuvel wrote:
>> Replace the call to and implementation of the function
>> FpdtAllocateReservedMemoryBelow4G() with a call to 
>> AllocatePeiAccessiblePages, which boils down to the same on X64, but 
>> does not crash non-X64 systems that lack memory below 4 GB.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>> Note that the ZeroMem() call is dropped, but it is redundant anyway, 
>> given that the subsequent CopyMem() call supersedes it immediately.
>
> I'm not sure the ZeroMem() is redundant. The allocation size -- before rounding up to full pages -- is computed like this:
>
>   BootPerformanceDataSize = sizeof (BOOT_PERFORMANCE_TABLE) + mPerformanceLength + PcdGet32 (PcdExtFpdtBootRecordPadSize);
>   if (SmmCommData != NULL && SmmBootRecordData != NULL) {
>     BootPerformanceDataSize += SmmBootRecordDataSize;
>   }
>
> ZeroMem() covers all of the above, but the CopyMem() calls don't seem to cover the optional padding (from the PCD).
>
> I'm unsure if that matters, of course.
>

You're quite right. I'm not sure how I missed that.

In any case, I can add back the ZeroMem () quite easily after the single invocation of AllocatePeiAccessiblePages() that I am adding in this file.

> The patch looks good to me otherwise.
>
> Thanks
> Laszlo
>
>
>>
>>  MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c |
>> 45 ++------------------
>>  1 file changed, 4 insertions(+), 41 deletions(-)
>>
>> diff --git
>> a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
>> b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
>> index 71d624fc9ce9..b1f09710b65c 100644
>> ---
>> a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
>> +++ b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLi
>> +++ b.c
>> @@ -165,46 +165,6 @@ IsKnownID (
>>    }
>>  }
>>
>> -/**
>> -  Allocate EfiReservedMemoryType below 4G memory address.
>> -
>> -  This function allocates EfiReservedMemoryType below 4G memory address.
>> -
>> -  @param[in]  Size   Size of memory to allocate.
>> -
>> -  @return Allocated address for output.
>> -
>> -**/
>> -VOID *
>> -FpdtAllocateReservedMemoryBelow4G (
>> -  IN UINTN       Size
>> -  )
>> -{
>> -  UINTN                 Pages;
>> -  EFI_PHYSICAL_ADDRESS  Address;
>> -  EFI_STATUS            Status;
>> -  VOID                  *Buffer;
>> -
>> -  Buffer  = NULL;
>> -  Pages   = EFI_SIZE_TO_PAGES (Size);
>> -  Address = 0xffffffff;
>> -
>> -  Status = gBS->AllocatePages (
>> -                  AllocateMaxAddress,
>> -                  EfiReservedMemoryType,
>> -                  Pages,
>> -                  &Address
>> -                  );
>> -  ASSERT_EFI_ERROR (Status);
>> -
>> -  if (!EFI_ERROR (Status)) {
>> -    Buffer = (VOID *) (UINTN) Address;
>> -    ZeroMem (Buffer, Size);
>> -  }
>> -
>> -  return Buffer;
>> -}
>> -
>>  /**
>>    Allocate buffer for Boot Performance table.
>>
>> @@ -348,7 +308,10 @@ AllocateBootPerformanceTable (
>>      //
>>      // Fail to allocate at specified address, continue to allocate at any address.
>>      //
>> -    mAcpiBootPerformanceTable = (BOOT_PERFORMANCE_TABLE *) FpdtAllocateReservedMemoryBelow4G (BootPerformanceDataSize);
>> +    mAcpiBootPerformanceTable = (BOOT_PERFORMANCE_TABLE *) AllocatePeiAccessiblePages (
>> +                                                             EfiReservedMemoryType,
>> +                                                             EFI_SIZE_TO_PAGES (BootPerformanceDataSize)
>> +                                                             );
>>    }
>>    DEBUG ((DEBUG_INFO, "DxeCorePerformanceLib: ACPI Boot Performance 
>> Table address = 0x%x\n", mAcpiBootPerformanceTable));
>>
>>
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel