public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v3 0/5] Abstract allocation of PEI accessible memory
@ 2018-05-28 14:40 Ard Biesheuvel
  2018-05-28 14:40 ` [PATCH v3 1/5] OvmfPkg/PlatformBootManagerLib: add missing report status code call Ard Biesheuvel
                   ` (5 more replies)
  0 siblings, 6 replies; 19+ messages in thread
From: Ard Biesheuvel @ 2018-05-28 14:40 UTC (permalink / raw)
  To: edk2-devel; +Cc: 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 AllocatePeiAccessiblePages() routine in DxeServicesLib
  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-v3

Changes since v2:
- move AllocatePeiAccessiblePages() to a separate .c file, and create a special
  X64 version (#3)
- add back ZeroMem() call to #4
- add Laszlo's ack to #4 - #5 (but not to #3 since it has been updated)

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

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                   | 48 ++-----------
 .../FirmwarePerformanceDxe.c                  | 51 +++-----------
 .../FirmwarePerformanceDxe.inf                |  1 +
 MdePkg/Include/Library/DxeServicesLib.h       | 23 ++++++-
 MdePkg/Library/DxeServicesLib/Allocate.c      | 54 +++++++++++++++
 .../Library/DxeServicesLib/DxeServicesLib.inf | 11 ++-
 MdePkg/Library/DxeServicesLib/X64/Allocate.c  | 69 +++++++++++++++++++
 .../PlatformBootManagerLib.inf                |  1 +
 .../PlatformBootManagerLib/QemuKernel.c       |  4 ++
 11 files changed, 182 insertions(+), 85 deletions(-)
 create mode 100644 MdePkg/Library/DxeServicesLib/Allocate.c
 create mode 100644 MdePkg/Library/DxeServicesLib/X64/Allocate.c

-- 
2.17.0



^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v3 1/5] OvmfPkg/PlatformBootManagerLib: add missing report status code call
  2018-05-28 14:40 [PATCH v3 0/5] Abstract allocation of PEI accessible memory Ard Biesheuvel
@ 2018-05-28 14:40 ` Ard Biesheuvel
  2018-05-28 14:40 ` [PATCH v3 2/5] ArmVirtPkg/PlatformBootManagerLib: " Ard Biesheuvel
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 19+ messages in thread
From: Ard Biesheuvel @ 2018-05-28 14:40 UTC (permalink / raw)
  To: edk2-devel; +Cc: 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 4b72c44bcf0a..d355d0440efd 100644
--- a/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
+++ b/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
@@ -56,6 +56,7 @@ [LibraryClasses]
   QemuFwCfgS3Lib
   LoadLinuxLib
   QemuBootOrderLib
+  ReportStatusCodeLib
   UefiLib
   Tcg2PhysicalPresenceLib
 
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



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v3 2/5] ArmVirtPkg/PlatformBootManagerLib: add missing report status code call
  2018-05-28 14:40 [PATCH v3 0/5] Abstract allocation of PEI accessible memory Ard Biesheuvel
  2018-05-28 14:40 ` [PATCH v3 1/5] OvmfPkg/PlatformBootManagerLib: add missing report status code call Ard Biesheuvel
@ 2018-05-28 14:40 ` Ard Biesheuvel
  2018-05-28 14:40 ` [PATCH v3 3/5] MdePkg/DxeServicesLib: introduce AllocatePeiAccessiblePages routine Ard Biesheuvel
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 19+ messages in thread
From: Ard Biesheuvel @ 2018-05-28 14:40 UTC (permalink / raw)
  To: edk2-devel; +Cc: 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



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v3 3/5] MdePkg/DxeServicesLib: introduce AllocatePeiAccessiblePages routine
  2018-05-28 14:40 [PATCH v3 0/5] Abstract allocation of PEI accessible memory Ard Biesheuvel
  2018-05-28 14:40 ` [PATCH v3 1/5] OvmfPkg/PlatformBootManagerLib: add missing report status code call Ard Biesheuvel
  2018-05-28 14:40 ` [PATCH v3 2/5] ArmVirtPkg/PlatformBootManagerLib: " Ard Biesheuvel
@ 2018-05-28 14:40 ` Ard Biesheuvel
  2018-05-28 14:59   ` Laszlo Ersek
  2018-05-29  1:37   ` Zeng, Star
  2018-05-28 14:40 ` [PATCH v3 4/5] MdeModulePkg/DxeCorePerformanceLib: use AllocatePeiAccessiblePages Ard Biesheuvel
                   ` (2 subsequent siblings)
  5 siblings, 2 replies; 19+ messages in thread
From: Ard Biesheuvel @ 2018-05-28 14:40 UTC (permalink / raw)
  To: edk2-devel; +Cc: 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/Allocate.c         | 54 +++++++++++++++
 MdePkg/Library/DxeServicesLib/DxeServicesLib.inf | 11 +++-
 MdePkg/Library/DxeServicesLib/X64/Allocate.c     | 69 ++++++++++++++++++++
 4 files changed, 155 insertions(+), 2 deletions(-)

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/Allocate.c b/MdePkg/Library/DxeServicesLib/Allocate.c
new file mode 100644
index 000000000000..4d118f766d49
--- /dev/null
+++ b/MdePkg/Library/DxeServicesLib/Allocate.c
@@ -0,0 +1,54 @@
+/** @file
+  DxeServicesLib memory allocation routines
+
+  Copyright (c) 2018, Linaro, Ltd. 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 <PiDxe.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/DxeServicesLib.h>
+
+/**
+  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_PHYSICAL_ADDRESS        Memory;
+
+  if (Pages == 0) {
+    return NULL;
+  }
+
+  Status = gBS->AllocatePages (AllocateAnyPages, 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..50ae24f8ee22 100644
--- a/MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
+++ b/MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
@@ -27,12 +27,18 @@ [Defines]
   LIBRARY_CLASS                  = DxeServicesLib|DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SAL_DRIVER DXE_SMM_DRIVER SMM_CORE UEFI_APPLICATION UEFI_DRIVER
 
 #
-#  VALID_ARCHITECTURES           = IA32 X64 IPF EBC
+#  VALID_ARCHITECTURES           = IA32 X64 IPF EBC ARM AARCH64
 #
 
 [Sources]
   DxeServicesLib.c
 
+[Sources.IA32, Sources.IPF, Sources.EBC, Sources.ARM, Sources.AARCH64]
+  Allocate.c
+
+[Sources.X64]
+  X64/Allocate.c
+
 [Packages]
   MdePkg/MdePkg.dec
 
@@ -44,6 +50,9 @@ [LibraryClasses]
   UefiLib
   UefiBootServicesTableLib
 
+[LibraryClasses.X64]
+  HobLib
+
 [Guids]
   gEfiFileInfoGuid                              ## SOMETIMES_CONSUMES ## UNDEFINED
 
diff --git a/MdePkg/Library/DxeServicesLib/X64/Allocate.c b/MdePkg/Library/DxeServicesLib/X64/Allocate.c
new file mode 100644
index 000000000000..b6d34ba20881
--- /dev/null
+++ b/MdePkg/Library/DxeServicesLib/X64/Allocate.c
@@ -0,0 +1,69 @@
+/** @file
+  DxeServicesLib memory allocation routines
+
+  Copyright (c) 2018, Linaro, Ltd. 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 <PiDxe.h>
+#include <Library/HobLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/DxeServicesLib.h>
+
+/**
+  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;
+  EFI_HOB_HANDOFF_INFO_TABLE  *PhitHob;
+
+  if (Pages == 0) {
+    return NULL;
+  }
+
+  AllocType = AllocateAnyPages;
+  //
+  // 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;
+  }
+
+  Status = gBS->AllocatePages (AllocType, MemoryType, Pages, &Memory);
+  if (EFI_ERROR (Status)) {
+    return NULL;
+  }
+  return (VOID *)(UINTN)Memory;
+}
-- 
2.17.0



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v3 4/5] MdeModulePkg/DxeCorePerformanceLib: use AllocatePeiAccessiblePages
  2018-05-28 14:40 [PATCH v3 0/5] Abstract allocation of PEI accessible memory Ard Biesheuvel
                   ` (2 preceding siblings ...)
  2018-05-28 14:40 ` [PATCH v3 3/5] MdePkg/DxeServicesLib: introduce AllocatePeiAccessiblePages routine Ard Biesheuvel
@ 2018-05-28 14:40 ` Ard Biesheuvel
  2018-05-29  1:33   ` Zeng, Star
  2018-05-28 14:40 ` [PATCH v3 5/5] MdeModulePkg/FirmwarePerformanceDataTableDxe: " Ard Biesheuvel
  2018-05-29  8:49 ` [PATCH v3 0/5] Abstract allocation of PEI accessible memory Ard Biesheuvel
  5 siblings, 1 reply; 19+ messages in thread
From: Ard Biesheuvel @ 2018-05-28 14:40 UTC (permalink / raw)
  To: edk2-devel; +Cc: 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>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
 MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c | 48 +++-----------------
 1 file changed, 7 insertions(+), 41 deletions(-)

diff --git a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
index 71d624fc9ce9..68b29ac5a9e2 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,13 @@ 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)
+                                                             );
+    if (mAcpiBootPerformanceTable != NULL) {
+      ZeroMem (mAcpiBootPerformanceTable, BootPerformanceDataSize);
+    }
   }
   DEBUG ((DEBUG_INFO, "DxeCorePerformanceLib: ACPI Boot Performance Table address = 0x%x\n", mAcpiBootPerformanceTable));
 
-- 
2.17.0



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v3 5/5] MdeModulePkg/FirmwarePerformanceDataTableDxe: use AllocatePeiAccessiblePages
  2018-05-28 14:40 [PATCH v3 0/5] Abstract allocation of PEI accessible memory Ard Biesheuvel
                   ` (3 preceding siblings ...)
  2018-05-28 14:40 ` [PATCH v3 4/5] MdeModulePkg/DxeCorePerformanceLib: use AllocatePeiAccessiblePages Ard Biesheuvel
@ 2018-05-28 14:40 ` Ard Biesheuvel
  2018-05-29  1:32   ` Zeng, Star
  2018-05-29  8:49 ` [PATCH v3 0/5] Abstract allocation of PEI accessible memory Ard Biesheuvel
  5 siblings, 1 reply; 19+ messages in thread
From: Ard Biesheuvel @ 2018-05-28 14:40 UTC (permalink / raw)
  To: edk2-devel; +Cc: 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>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
 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



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 3/5] MdePkg/DxeServicesLib: introduce AllocatePeiAccessiblePages routine
  2018-05-28 14:40 ` [PATCH v3 3/5] MdePkg/DxeServicesLib: introduce AllocatePeiAccessiblePages routine Ard Biesheuvel
@ 2018-05-28 14:59   ` Laszlo Ersek
  2018-05-29  3:34     ` Gao, Liming
  2018-05-29  1:37   ` Zeng, Star
  1 sibling, 1 reply; 19+ messages in thread
From: Laszlo Ersek @ 2018-05-28 14:59 UTC (permalink / raw)
  To: Ard Biesheuvel, edk2-devel

On 05/28/18 16:40, 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/Allocate.c         | 54 +++++++++++++++
>  MdePkg/Library/DxeServicesLib/DxeServicesLib.inf | 11 +++-
>  MdePkg/Library/DxeServicesLib/X64/Allocate.c     | 69 ++++++++++++++++++++
>  4 files changed, 155 insertions(+), 2 deletions(-)

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

Thanks,
Laszlo

> 
> 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/Allocate.c b/MdePkg/Library/DxeServicesLib/Allocate.c
> new file mode 100644
> index 000000000000..4d118f766d49
> --- /dev/null
> +++ b/MdePkg/Library/DxeServicesLib/Allocate.c
> @@ -0,0 +1,54 @@
> +/** @file
> +  DxeServicesLib memory allocation routines
> +
> +  Copyright (c) 2018, Linaro, Ltd. 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 <PiDxe.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/DxeServicesLib.h>
> +
> +/**
> +  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_PHYSICAL_ADDRESS        Memory;
> +
> +  if (Pages == 0) {
> +    return NULL;
> +  }
> +
> +  Status = gBS->AllocatePages (AllocateAnyPages, 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..50ae24f8ee22 100644
> --- a/MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
> +++ b/MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
> @@ -27,12 +27,18 @@ [Defines]
>    LIBRARY_CLASS                  = DxeServicesLib|DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SAL_DRIVER DXE_SMM_DRIVER SMM_CORE UEFI_APPLICATION UEFI_DRIVER
>  
>  #
> -#  VALID_ARCHITECTURES           = IA32 X64 IPF EBC
> +#  VALID_ARCHITECTURES           = IA32 X64 IPF EBC ARM AARCH64
>  #
>  
>  [Sources]
>    DxeServicesLib.c
>  
> +[Sources.IA32, Sources.IPF, Sources.EBC, Sources.ARM, Sources.AARCH64]
> +  Allocate.c
> +
> +[Sources.X64]
> +  X64/Allocate.c
> +
>  [Packages]
>    MdePkg/MdePkg.dec
>  
> @@ -44,6 +50,9 @@ [LibraryClasses]
>    UefiLib
>    UefiBootServicesTableLib
>  
> +[LibraryClasses.X64]
> +  HobLib
> +
>  [Guids]
>    gEfiFileInfoGuid                              ## SOMETIMES_CONSUMES ## UNDEFINED
>  
> diff --git a/MdePkg/Library/DxeServicesLib/X64/Allocate.c b/MdePkg/Library/DxeServicesLib/X64/Allocate.c
> new file mode 100644
> index 000000000000..b6d34ba20881
> --- /dev/null
> +++ b/MdePkg/Library/DxeServicesLib/X64/Allocate.c
> @@ -0,0 +1,69 @@
> +/** @file
> +  DxeServicesLib memory allocation routines
> +
> +  Copyright (c) 2018, Linaro, Ltd. 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 <PiDxe.h>
> +#include <Library/HobLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/DxeServicesLib.h>
> +
> +/**
> +  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;
> +  EFI_HOB_HANDOFF_INFO_TABLE  *PhitHob;
> +
> +  if (Pages == 0) {
> +    return NULL;
> +  }
> +
> +  AllocType = AllocateAnyPages;
> +  //
> +  // 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;
> +  }
> +
> +  Status = gBS->AllocatePages (AllocType, MemoryType, Pages, &Memory);
> +  if (EFI_ERROR (Status)) {
> +    return NULL;
> +  }
> +  return (VOID *)(UINTN)Memory;
> +}
> 



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 5/5] MdeModulePkg/FirmwarePerformanceDataTableDxe: use AllocatePeiAccessiblePages
  2018-05-28 14:40 ` [PATCH v3 5/5] MdeModulePkg/FirmwarePerformanceDataTableDxe: " Ard Biesheuvel
@ 2018-05-29  1:32   ` Zeng, Star
  0 siblings, 0 replies; 19+ messages in thread
From: Zeng, Star @ 2018-05-29  1:32 UTC (permalink / raw)
  To: Ard Biesheuvel, edk2-devel@lists.01.org; +Cc: Zeng, Star

Reviewed-by: Star Zeng <star.zeng@intel.com>

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard Biesheuvel
Sent: Monday, May 28, 2018 10:40 PM
To: edk2-devel@lists.01.org
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Subject: [edk2] [PATCH v3 5/5] MdeModulePkg/FirmwarePerformanceDataTableDxe: use AllocatePeiAccessiblePages

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>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
 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/Firmwa
+++ rePerformanceDxe.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/Firmwa
+++ rePerformanceDxe.inf
@@ -44,6 +44,7 @@ [LibraryClasses]
   UefiRuntimeServicesTableLib
   BaseLib
   DebugLib
+  DxeServicesLib
   TimerLib
   BaseMemoryLib
   MemoryAllocationLib
--
2.17.0

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 4/5] MdeModulePkg/DxeCorePerformanceLib: use AllocatePeiAccessiblePages
  2018-05-28 14:40 ` [PATCH v3 4/5] MdeModulePkg/DxeCorePerformanceLib: use AllocatePeiAccessiblePages Ard Biesheuvel
@ 2018-05-29  1:33   ` Zeng, Star
  0 siblings, 0 replies; 19+ messages in thread
From: Zeng, Star @ 2018-05-29  1:33 UTC (permalink / raw)
  To: Ard Biesheuvel, edk2-devel@lists.01.org; +Cc: Zeng, Star

Reviewed-by: Star Zeng <star.zeng@intel.com>

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard Biesheuvel
Sent: Monday, May 28, 2018 10:40 PM
To: edk2-devel@lists.01.org
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Subject: [edk2] [PATCH v3 4/5] MdeModulePkg/DxeCorePerformanceLib: use AllocatePeiAccessiblePages

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>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
 MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c | 48 +++-----------------
 1 file changed, 7 insertions(+), 41 deletions(-)

diff --git a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
index 71d624fc9ce9..68b29ac5a9e2 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,13 @@ 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)
+                                                             );
+    if (mAcpiBootPerformanceTable != NULL) {
+      ZeroMem (mAcpiBootPerformanceTable, BootPerformanceDataSize);
+    }
   }
   DEBUG ((DEBUG_INFO, "DxeCorePerformanceLib: ACPI Boot Performance Table address = 0x%x\n", mAcpiBootPerformanceTable));
 
--
2.17.0

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 3/5] MdePkg/DxeServicesLib: introduce AllocatePeiAccessiblePages routine
  2018-05-28 14:40 ` [PATCH v3 3/5] MdePkg/DxeServicesLib: introduce AllocatePeiAccessiblePages routine Ard Biesheuvel
  2018-05-28 14:59   ` Laszlo Ersek
@ 2018-05-29  1:37   ` Zeng, Star
  2018-05-29 15:18     ` Yao, Jiewen
  1 sibling, 1 reply; 19+ messages in thread
From: Zeng, Star @ 2018-05-29  1:37 UTC (permalink / raw)
  To: Ard Biesheuvel, edk2-devel@lists.01.org; +Cc: Zeng, Star

I think " be accessible by PEI after a warm reboot " should be " be accessible by PEI after resuming from S3 ".
You can update it when pushing without need to send a new patch if other has no comment to the code part.


Thanks,
Star
-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard Biesheuvel
Sent: Monday, May 28, 2018 10:40 PM
To: edk2-devel@lists.01.org
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Subject: [edk2] [PATCH v3 3/5] MdePkg/DxeServicesLib: introduce AllocatePeiAccessiblePages routine

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/Allocate.c         | 54 +++++++++++++++
 MdePkg/Library/DxeServicesLib/DxeServicesLib.inf | 11 +++-
 MdePkg/Library/DxeServicesLib/X64/Allocate.c     | 69 ++++++++++++++++++++
 4 files changed, 155 insertions(+), 2 deletions(-)

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/Allocate.c b/MdePkg/Library/DxeServicesLib/Allocate.c
new file mode 100644
index 000000000000..4d118f766d49
--- /dev/null
+++ b/MdePkg/Library/DxeServicesLib/Allocate.c
@@ -0,0 +1,54 @@
+/** @file
+  DxeServicesLib memory allocation routines
+
+  Copyright (c) 2018, Linaro, Ltd. 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 <PiDxe.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/DxeServicesLib.h>
+
+/**
+  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_PHYSICAL_ADDRESS        Memory;
+
+  if (Pages == 0) {
+    return NULL;
+  }
+
+  Status = gBS->AllocatePages (AllocateAnyPages, 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..50ae24f8ee22 100644
--- a/MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
+++ b/MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
@@ -27,12 +27,18 @@ [Defines]
   LIBRARY_CLASS                  = DxeServicesLib|DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SAL_DRIVER DXE_SMM_DRIVER SMM_CORE UEFI_APPLICATION UEFI_DRIVER
 
 #
-#  VALID_ARCHITECTURES           = IA32 X64 IPF EBC
+#  VALID_ARCHITECTURES           = IA32 X64 IPF EBC ARM AARCH64
 #
 
 [Sources]
   DxeServicesLib.c
 
+[Sources.IA32, Sources.IPF, Sources.EBC, Sources.ARM, Sources.AARCH64]
+  Allocate.c
+
+[Sources.X64]
+  X64/Allocate.c
+
 [Packages]
   MdePkg/MdePkg.dec
 
@@ -44,6 +50,9 @@ [LibraryClasses]
   UefiLib
   UefiBootServicesTableLib
 
+[LibraryClasses.X64]
+  HobLib
+
 [Guids]
   gEfiFileInfoGuid                              ## SOMETIMES_CONSUMES ## UNDEFINED
 
diff --git a/MdePkg/Library/DxeServicesLib/X64/Allocate.c b/MdePkg/Library/DxeServicesLib/X64/Allocate.c
new file mode 100644
index 000000000000..b6d34ba20881
--- /dev/null
+++ b/MdePkg/Library/DxeServicesLib/X64/Allocate.c
@@ -0,0 +1,69 @@
+/** @file
+  DxeServicesLib memory allocation routines
+
+  Copyright (c) 2018, Linaro, Ltd. 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 <PiDxe.h>
+#include <Library/HobLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/DxeServicesLib.h>
+
+/**
+  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;
+  EFI_HOB_HANDOFF_INFO_TABLE  *PhitHob;
+
+  if (Pages == 0) {
+    return NULL;
+  }
+
+  AllocType = AllocateAnyPages;
+  //
+  // 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;
+  }
+
+  Status = gBS->AllocatePages (AllocType, MemoryType, Pages, &Memory);
+  if (EFI_ERROR (Status)) {
+    return NULL;
+  }
+  return (VOID *)(UINTN)Memory;
+}
-- 
2.17.0

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 3/5] MdePkg/DxeServicesLib: introduce AllocatePeiAccessiblePages routine
  2018-05-28 14:59   ` Laszlo Ersek
@ 2018-05-29  3:34     ` Gao, Liming
  0 siblings, 0 replies; 19+ messages in thread
From: Gao, Liming @ 2018-05-29  3:34 UTC (permalink / raw)
  To: Laszlo Ersek, Ard Biesheuvel, edk2-devel@lists.01.org

The patch is good to me.

Reviewed-by: Liming Gao <liming.gao@intel.com>

>-----Original Message-----
>From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
>Laszlo Ersek
>Sent: Monday, May 28, 2018 11:00 PM
>To: Ard Biesheuvel <ard.biesheuvel@linaro.org>; edk2-devel@lists.01.org
>Subject: Re: [edk2] [PATCH v3 3/5] MdePkg/DxeServicesLib: introduce
>AllocatePeiAccessiblePages routine
>
>On 05/28/18 16:40, 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/Allocate.c         | 54 +++++++++++++++
>>  MdePkg/Library/DxeServicesLib/DxeServicesLib.inf | 11 +++-
>>  MdePkg/Library/DxeServicesLib/X64/Allocate.c     | 69
>++++++++++++++++++++
>>  4 files changed, 155 insertions(+), 2 deletions(-)
>
>Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>
>Thanks,
>Laszlo
>
>>
>> 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/Allocate.c
>b/MdePkg/Library/DxeServicesLib/Allocate.c
>> new file mode 100644
>> index 000000000000..4d118f766d49
>> --- /dev/null
>> +++ b/MdePkg/Library/DxeServicesLib/Allocate.c
>> @@ -0,0 +1,54 @@
>> +/** @file
>> +  DxeServicesLib memory allocation routines
>> +
>> +  Copyright (c) 2018, Linaro, Ltd. 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 <PiDxe.h>
>> +#include <Library/UefiBootServicesTableLib.h>
>> +#include <Library/DxeServicesLib.h>
>> +
>> +/**
>> +  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_PHYSICAL_ADDRESS        Memory;
>> +
>> +  if (Pages == 0) {
>> +    return NULL;
>> +  }
>> +
>> +  Status = gBS->AllocatePages (AllocateAnyPages, 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..50ae24f8ee22 100644
>> --- a/MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
>> +++ b/MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
>> @@ -27,12 +27,18 @@ [Defines]
>>    LIBRARY_CLASS                  = DxeServicesLib|DXE_CORE DXE_DRIVER
>DXE_RUNTIME_DRIVER DXE_SAL_DRIVER DXE_SMM_DRIVER SMM_CORE
>UEFI_APPLICATION UEFI_DRIVER
>>
>>  #
>> -#  VALID_ARCHITECTURES           = IA32 X64 IPF EBC
>> +#  VALID_ARCHITECTURES           = IA32 X64 IPF EBC ARM AARCH64
>>  #
>>
>>  [Sources]
>>    DxeServicesLib.c
>>
>> +[Sources.IA32, Sources.IPF, Sources.EBC, Sources.ARM, Sources.AARCH64]
>> +  Allocate.c
>> +
>> +[Sources.X64]
>> +  X64/Allocate.c
>> +
>>  [Packages]
>>    MdePkg/MdePkg.dec
>>
>> @@ -44,6 +50,9 @@ [LibraryClasses]
>>    UefiLib
>>    UefiBootServicesTableLib
>>
>> +[LibraryClasses.X64]
>> +  HobLib
>> +
>>  [Guids]
>>    gEfiFileInfoGuid                              ## SOMETIMES_CONSUMES ## UNDEFINED
>>
>> diff --git a/MdePkg/Library/DxeServicesLib/X64/Allocate.c
>b/MdePkg/Library/DxeServicesLib/X64/Allocate.c
>> new file mode 100644
>> index 000000000000..b6d34ba20881
>> --- /dev/null
>> +++ b/MdePkg/Library/DxeServicesLib/X64/Allocate.c
>> @@ -0,0 +1,69 @@
>> +/** @file
>> +  DxeServicesLib memory allocation routines
>> +
>> +  Copyright (c) 2018, Linaro, Ltd. 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 <PiDxe.h>
>> +#include <Library/HobLib.h>
>> +#include <Library/UefiBootServicesTableLib.h>
>> +#include <Library/DxeServicesLib.h>
>> +
>> +/**
>> +  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;
>> +  EFI_HOB_HANDOFF_INFO_TABLE  *PhitHob;
>> +
>> +  if (Pages == 0) {
>> +    return NULL;
>> +  }
>> +
>> +  AllocType = AllocateAnyPages;
>> +  //
>> +  // 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;
>> +  }
>> +
>> +  Status = gBS->AllocatePages (AllocType, MemoryType, Pages, &Memory);
>> +  if (EFI_ERROR (Status)) {
>> +    return NULL;
>> +  }
>> +  return (VOID *)(UINTN)Memory;
>> +}
>>
>
>_______________________________________________
>edk2-devel mailing list
>edk2-devel@lists.01.org
>https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 0/5] Abstract allocation of PEI accessible memory
  2018-05-28 14:40 [PATCH v3 0/5] Abstract allocation of PEI accessible memory Ard Biesheuvel
                   ` (4 preceding siblings ...)
  2018-05-28 14:40 ` [PATCH v3 5/5] MdeModulePkg/FirmwarePerformanceDataTableDxe: " Ard Biesheuvel
@ 2018-05-29  8:49 ` Ard Biesheuvel
  5 siblings, 0 replies; 19+ messages in thread
From: Ard Biesheuvel @ 2018-05-29  8:49 UTC (permalink / raw)
  To: edk2-devel@lists.01.org; +Cc: Ard Biesheuvel

On 28 May 2018 at 16:40, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> 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 AllocatePeiAccessiblePages() routine in DxeServicesLib
>   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-v3
>
> Changes since v2:
> - move AllocatePeiAccessiblePages() to a separate .c file, and create a special
>   X64 version (#3)
> - add back ZeroMem() call to #4
> - add Laszlo's ack to #4 - #5 (but not to #3 since it has been updated)
>
> 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
>
> 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
>

Pushed as 2d0c6692eee4..65e984cd8ad8

Thanks all.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 3/5] MdePkg/DxeServicesLib: introduce AllocatePeiAccessiblePages routine
  2018-05-29  1:37   ` Zeng, Star
@ 2018-05-29 15:18     ` Yao, Jiewen
  2018-05-29 15:31       ` Kinney, Michael D
  0 siblings, 1 reply; 19+ messages in thread
From: Yao, Jiewen @ 2018-05-29 15:18 UTC (permalink / raw)
  To: Zeng, Star, Ard Biesheuvel, edk2-devel@lists.01.org; +Cc: Zeng, Star

Maybe " be accessible by PEI after a warm reboot or S3" ?

We may want to consider Capsule Update case.


> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Zeng,
> Star
> Sent: Monday, May 28, 2018 6:37 PM
> To: Ard Biesheuvel <ard.biesheuvel@linaro.org>; edk2-devel@lists.01.org
> Cc: Zeng, Star <star.zeng@intel.com>
> Subject: Re: [edk2] [PATCH v3 3/5] MdePkg/DxeServicesLib: introduce
> AllocatePeiAccessiblePages routine
> 
> I think " be accessible by PEI after a warm reboot " should be " be accessible by
> PEI after resuming from S3 ".
> You can update it when pushing without need to send a new patch if other has
> no comment to the code part.
> 
> 
> Thanks,
> Star
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard
> Biesheuvel
> Sent: Monday, May 28, 2018 10:40 PM
> To: edk2-devel@lists.01.org
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Subject: [edk2] [PATCH v3 3/5] MdePkg/DxeServicesLib: introduce
> AllocatePeiAccessiblePages routine
> 
> 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/Allocate.c         | 54 +++++++++++++++
>  MdePkg/Library/DxeServicesLib/DxeServicesLib.inf | 11 +++-
>  MdePkg/Library/DxeServicesLib/X64/Allocate.c     | 69
> ++++++++++++++++++++
>  4 files changed, 155 insertions(+), 2 deletions(-)
> 
> 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/Allocate.c
> b/MdePkg/Library/DxeServicesLib/Allocate.c
> new file mode 100644
> index 000000000000..4d118f766d49
> --- /dev/null
> +++ b/MdePkg/Library/DxeServicesLib/Allocate.c
> @@ -0,0 +1,54 @@
> +/** @file
> +  DxeServicesLib memory allocation routines
> +
> +  Copyright (c) 2018, Linaro, Ltd. 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 <PiDxe.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/DxeServicesLib.h>
> +
> +/**
> +  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_PHYSICAL_ADDRESS        Memory;
> +
> +  if (Pages == 0) {
> +    return NULL;
> +  }
> +
> +  Status = gBS->AllocatePages (AllocateAnyPages, 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..50ae24f8ee22 100644
> --- a/MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
> +++ b/MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
> @@ -27,12 +27,18 @@ [Defines]
>    LIBRARY_CLASS                  = DxeServicesLib|DXE_CORE
> DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SAL_DRIVER DXE_SMM_DRIVER
> SMM_CORE UEFI_APPLICATION UEFI_DRIVER
> 
>  #
> -#  VALID_ARCHITECTURES           = IA32 X64 IPF EBC
> +#  VALID_ARCHITECTURES           = IA32 X64 IPF EBC ARM AARCH64
>  #
> 
>  [Sources]
>    DxeServicesLib.c
> 
> +[Sources.IA32, Sources.IPF, Sources.EBC, Sources.ARM, Sources.AARCH64]
> +  Allocate.c
> +
> +[Sources.X64]
> +  X64/Allocate.c
> +
>  [Packages]
>    MdePkg/MdePkg.dec
> 
> @@ -44,6 +50,9 @@ [LibraryClasses]
>    UefiLib
>    UefiBootServicesTableLib
> 
> +[LibraryClasses.X64]
> +  HobLib
> +
>  [Guids]
>    gEfiFileInfoGuid                              ##
> SOMETIMES_CONSUMES ## UNDEFINED
> 
> diff --git a/MdePkg/Library/DxeServicesLib/X64/Allocate.c
> b/MdePkg/Library/DxeServicesLib/X64/Allocate.c
> new file mode 100644
> index 000000000000..b6d34ba20881
> --- /dev/null
> +++ b/MdePkg/Library/DxeServicesLib/X64/Allocate.c
> @@ -0,0 +1,69 @@
> +/** @file
> +  DxeServicesLib memory allocation routines
> +
> +  Copyright (c) 2018, Linaro, Ltd. 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 <PiDxe.h>
> +#include <Library/HobLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/DxeServicesLib.h>
> +
> +/**
> +  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;
> +  EFI_HOB_HANDOFF_INFO_TABLE  *PhitHob;
> +
> +  if (Pages == 0) {
> +    return NULL;
> +  }
> +
> +  AllocType = AllocateAnyPages;
> +  //
> +  // 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;
> +  }
> +
> +  Status = gBS->AllocatePages (AllocType, MemoryType, Pages, &Memory);
> +  if (EFI_ERROR (Status)) {
> +    return NULL;
> +  }
> +  return (VOID *)(UINTN)Memory;
> +}
> --
> 2.17.0
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 3/5] MdePkg/DxeServicesLib: introduce AllocatePeiAccessiblePages routine
  2018-05-29 15:18     ` Yao, Jiewen
@ 2018-05-29 15:31       ` Kinney, Michael D
  2018-05-29 15:36         ` Yao, Jiewen
  0 siblings, 1 reply; 19+ messages in thread
From: Kinney, Michael D @ 2018-05-29 15:31 UTC (permalink / raw)
  To: Yao, Jiewen, Zeng, Star, Ard Biesheuvel, edk2-devel@lists.01.org,
	Kinney, Michael D
  Cc: Zeng, Star

Jiewen,

I do not think this service applies to the capsule
use cases.

A capsule sent at OS runtime can be placed anywhere
in memory by the OS.  The OS does not know what
memory PEI can easily access or not.

The main PEI requirement is to not corrupt capsules
in memory.

Mike

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-
> bounces@lists.01.org] On Behalf Of Yao, Jiewen
> Sent: Tuesday, May 29, 2018 8:19 AM
> To: Zeng, Star <star.zeng@intel.com>; Ard Biesheuvel
> <ard.biesheuvel@linaro.org>; edk2-devel@lists.01.org
> Cc: Zeng, Star <star.zeng@intel.com>
> Subject: Re: [edk2] [PATCH v3 3/5]
> MdePkg/DxeServicesLib: introduce
> AllocatePeiAccessiblePages routine
> 
> Maybe " be accessible by PEI after a warm reboot or S3"
> ?
> 
> We may want to consider Capsule Update case.
> 
> 
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-
> bounces@lists.01.org] On Behalf Of Zeng,
> > Star
> > Sent: Monday, May 28, 2018 6:37 PM
> > To: Ard Biesheuvel <ard.biesheuvel@linaro.org>; edk2-
> devel@lists.01.org
> > Cc: Zeng, Star <star.zeng@intel.com>
> > Subject: Re: [edk2] [PATCH v3 3/5]
> MdePkg/DxeServicesLib: introduce
> > AllocatePeiAccessiblePages routine
> >
> > I think " be accessible by PEI after a warm reboot "
> should be " be accessible by
> > PEI after resuming from S3 ".
> > You can update it when pushing without need to send a
> new patch if other has
> > no comment to the code part.
> >
> >
> > Thanks,
> > Star
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-
> bounces@lists.01.org] On Behalf Of Ard
> > Biesheuvel
> > Sent: Monday, May 28, 2018 10:40 PM
> > To: edk2-devel@lists.01.org
> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > Subject: [edk2] [PATCH v3 3/5] MdePkg/DxeServicesLib:
> introduce
> > AllocatePeiAccessiblePages routine
> >
> > 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/Allocate.c         | 54
> +++++++++++++++
> >  MdePkg/Library/DxeServicesLib/DxeServicesLib.inf | 11
> +++-
> >  MdePkg/Library/DxeServicesLib/X64/Allocate.c     | 69
> > ++++++++++++++++++++
> >  4 files changed, 155 insertions(+), 2 deletions(-)
> >
> > 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/Allocate.c
> > b/MdePkg/Library/DxeServicesLib/Allocate.c
> > new file mode 100644
> > index 000000000000..4d118f766d49
> > --- /dev/null
> > +++ b/MdePkg/Library/DxeServicesLib/Allocate.c
> > @@ -0,0 +1,54 @@
> > +/** @file
> > +  DxeServicesLib memory allocation routines
> > +
> > +  Copyright (c) 2018, Linaro, Ltd. 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 <PiDxe.h>
> > +#include <Library/UefiBootServicesTableLib.h>
> > +#include <Library/DxeServicesLib.h>
> > +
> > +/**
> > +  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_PHYSICAL_ADDRESS        Memory;
> > +
> > +  if (Pages == 0) {
> > +    return NULL;
> > +  }
> > +
> > +  Status = gBS->AllocatePages (AllocateAnyPages,
> 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..50ae24f8ee22 100644
> > --- a/MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
> > +++ b/MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
> > @@ -27,12 +27,18 @@ [Defines]
> >    LIBRARY_CLASS                  =
> DxeServicesLib|DXE_CORE
> > DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SAL_DRIVER
> DXE_SMM_DRIVER
> > SMM_CORE UEFI_APPLICATION UEFI_DRIVER
> >
> >  #
> > -#  VALID_ARCHITECTURES           = IA32 X64 IPF EBC
> > +#  VALID_ARCHITECTURES           = IA32 X64 IPF EBC
> ARM AARCH64
> >  #
> >
> >  [Sources]
> >    DxeServicesLib.c
> >
> > +[Sources.IA32, Sources.IPF, Sources.EBC, Sources.ARM,
> Sources.AARCH64]
> > +  Allocate.c
> > +
> > +[Sources.X64]
> > +  X64/Allocate.c
> > +
> >  [Packages]
> >    MdePkg/MdePkg.dec
> >
> > @@ -44,6 +50,9 @@ [LibraryClasses]
> >    UefiLib
> >    UefiBootServicesTableLib
> >
> > +[LibraryClasses.X64]
> > +  HobLib
> > +
> >  [Guids]
> >    gEfiFileInfoGuid                              ##
> > SOMETIMES_CONSUMES ## UNDEFINED
> >
> > diff --git
> a/MdePkg/Library/DxeServicesLib/X64/Allocate.c
> > b/MdePkg/Library/DxeServicesLib/X64/Allocate.c
> > new file mode 100644
> > index 000000000000..b6d34ba20881
> > --- /dev/null
> > +++ b/MdePkg/Library/DxeServicesLib/X64/Allocate.c
> > @@ -0,0 +1,69 @@
> > +/** @file
> > +  DxeServicesLib memory allocation routines
> > +
> > +  Copyright (c) 2018, Linaro, Ltd. 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 <PiDxe.h>
> > +#include <Library/HobLib.h>
> > +#include <Library/UefiBootServicesTableLib.h>
> > +#include <Library/DxeServicesLib.h>
> > +
> > +/**
> > +  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;
> > +  EFI_HOB_HANDOFF_INFO_TABLE  *PhitHob;
> > +
> > +  if (Pages == 0) {
> > +    return NULL;
> > +  }
> > +
> > +  AllocType = AllocateAnyPages;
> > +  //
> > +  // 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;
> > +  }
> > +
> > +  Status = gBS->AllocatePages (AllocType, MemoryType,
> Pages, &Memory);
> > +  if (EFI_ERROR (Status)) {
> > +    return NULL;
> > +  }
> > +  return (VOID *)(UINTN)Memory;
> > +}
> > --
> > 2.17.0
> >
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 3/5] MdePkg/DxeServicesLib: introduce AllocatePeiAccessiblePages routine
  2018-05-29 15:31       ` Kinney, Michael D
@ 2018-05-29 15:36         ` Yao, Jiewen
  2018-05-29 16:09           ` Kinney, Michael D
  0 siblings, 1 reply; 19+ messages in thread
From: Yao, Jiewen @ 2018-05-29 15:36 UTC (permalink / raw)
  To: Kinney, Michael D, Zeng, Star, Ard Biesheuvel,
	edk2-devel@lists.01.org
  Cc: Zeng, Star, Yao, Jiewen

Mike
Please refer to https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Universal/CapsuleRuntimeDxe/X64/SaveLongModeContext.c

It uses AllocateReservedMemoryBelow4G() for the context accessed in PEI.

Thank you
Yao Jiewen


> -----Original Message-----
> From: Kinney, Michael D
> Sent: Tuesday, May 29, 2018 8:31 AM
> To: Yao, Jiewen <jiewen.yao@intel.com>; Zeng, Star <star.zeng@intel.com>; Ard
> Biesheuvel <ard.biesheuvel@linaro.org>; edk2-devel@lists.01.org; Kinney,
> Michael D <michael.d.kinney@intel.com>
> Cc: Zeng, Star <star.zeng@intel.com>
> Subject: RE: [edk2] [PATCH v3 3/5] MdePkg/DxeServicesLib: introduce
> AllocatePeiAccessiblePages routine
> 
> Jiewen,
> 
> I do not think this service applies to the capsule
> use cases.
> 
> A capsule sent at OS runtime can be placed anywhere
> in memory by the OS.  The OS does not know what
> memory PEI can easily access or not.
> 
> The main PEI requirement is to not corrupt capsules
> in memory.
> 
> Mike
> 
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-
> > bounces@lists.01.org] On Behalf Of Yao, Jiewen
> > Sent: Tuesday, May 29, 2018 8:19 AM
> > To: Zeng, Star <star.zeng@intel.com>; Ard Biesheuvel
> > <ard.biesheuvel@linaro.org>; edk2-devel@lists.01.org
> > Cc: Zeng, Star <star.zeng@intel.com>
> > Subject: Re: [edk2] [PATCH v3 3/5]
> > MdePkg/DxeServicesLib: introduce
> > AllocatePeiAccessiblePages routine
> >
> > Maybe " be accessible by PEI after a warm reboot or S3"
> > ?
> >
> > We may want to consider Capsule Update case.
> >
> >
> > > -----Original Message-----
> > > From: edk2-devel [mailto:edk2-devel-
> > bounces@lists.01.org] On Behalf Of Zeng,
> > > Star
> > > Sent: Monday, May 28, 2018 6:37 PM
> > > To: Ard Biesheuvel <ard.biesheuvel@linaro.org>; edk2-
> > devel@lists.01.org
> > > Cc: Zeng, Star <star.zeng@intel.com>
> > > Subject: Re: [edk2] [PATCH v3 3/5]
> > MdePkg/DxeServicesLib: introduce
> > > AllocatePeiAccessiblePages routine
> > >
> > > I think " be accessible by PEI after a warm reboot "
> > should be " be accessible by
> > > PEI after resuming from S3 ".
> > > You can update it when pushing without need to send a
> > new patch if other has
> > > no comment to the code part.
> > >
> > >
> > > Thanks,
> > > Star
> > > -----Original Message-----
> > > From: edk2-devel [mailto:edk2-devel-
> > bounces@lists.01.org] On Behalf Of Ard
> > > Biesheuvel
> > > Sent: Monday, May 28, 2018 10:40 PM
> > > To: edk2-devel@lists.01.org
> > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > > Subject: [edk2] [PATCH v3 3/5] MdePkg/DxeServicesLib:
> > introduce
> > > AllocatePeiAccessiblePages routine
> > >
> > > 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/Allocate.c         | 54
> > +++++++++++++++
> > >  MdePkg/Library/DxeServicesLib/DxeServicesLib.inf | 11
> > +++-
> > >  MdePkg/Library/DxeServicesLib/X64/Allocate.c     | 69
> > > ++++++++++++++++++++
> > >  4 files changed, 155 insertions(+), 2 deletions(-)
> > >
> > > 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/Allocate.c
> > > b/MdePkg/Library/DxeServicesLib/Allocate.c
> > > new file mode 100644
> > > index 000000000000..4d118f766d49
> > > --- /dev/null
> > > +++ b/MdePkg/Library/DxeServicesLib/Allocate.c
> > > @@ -0,0 +1,54 @@
> > > +/** @file
> > > +  DxeServicesLib memory allocation routines
> > > +
> > > +  Copyright (c) 2018, Linaro, Ltd. 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 <PiDxe.h>
> > > +#include <Library/UefiBootServicesTableLib.h>
> > > +#include <Library/DxeServicesLib.h>
> > > +
> > > +/**
> > > +  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_PHYSICAL_ADDRESS        Memory;
> > > +
> > > +  if (Pages == 0) {
> > > +    return NULL;
> > > +  }
> > > +
> > > +  Status = gBS->AllocatePages (AllocateAnyPages,
> > 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..50ae24f8ee22 100644
> > > --- a/MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
> > > +++ b/MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
> > > @@ -27,12 +27,18 @@ [Defines]
> > >    LIBRARY_CLASS                  =
> > DxeServicesLib|DXE_CORE
> > > DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SAL_DRIVER
> > DXE_SMM_DRIVER
> > > SMM_CORE UEFI_APPLICATION UEFI_DRIVER
> > >
> > >  #
> > > -#  VALID_ARCHITECTURES           = IA32 X64 IPF EBC
> > > +#  VALID_ARCHITECTURES           = IA32 X64 IPF EBC
> > ARM AARCH64
> > >  #
> > >
> > >  [Sources]
> > >    DxeServicesLib.c
> > >
> > > +[Sources.IA32, Sources.IPF, Sources.EBC, Sources.ARM,
> > Sources.AARCH64]
> > > +  Allocate.c
> > > +
> > > +[Sources.X64]
> > > +  X64/Allocate.c
> > > +
> > >  [Packages]
> > >    MdePkg/MdePkg.dec
> > >
> > > @@ -44,6 +50,9 @@ [LibraryClasses]
> > >    UefiLib
> > >    UefiBootServicesTableLib
> > >
> > > +[LibraryClasses.X64]
> > > +  HobLib
> > > +
> > >  [Guids]
> > >    gEfiFileInfoGuid                              ##
> > > SOMETIMES_CONSUMES ## UNDEFINED
> > >
> > > diff --git
> > a/MdePkg/Library/DxeServicesLib/X64/Allocate.c
> > > b/MdePkg/Library/DxeServicesLib/X64/Allocate.c
> > > new file mode 100644
> > > index 000000000000..b6d34ba20881
> > > --- /dev/null
> > > +++ b/MdePkg/Library/DxeServicesLib/X64/Allocate.c
> > > @@ -0,0 +1,69 @@
> > > +/** @file
> > > +  DxeServicesLib memory allocation routines
> > > +
> > > +  Copyright (c) 2018, Linaro, Ltd. 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 <PiDxe.h>
> > > +#include <Library/HobLib.h>
> > > +#include <Library/UefiBootServicesTableLib.h>
> > > +#include <Library/DxeServicesLib.h>
> > > +
> > > +/**
> > > +  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;
> > > +  EFI_HOB_HANDOFF_INFO_TABLE  *PhitHob;
> > > +
> > > +  if (Pages == 0) {
> > > +    return NULL;
> > > +  }
> > > +
> > > +  AllocType = AllocateAnyPages;
> > > +  //
> > > +  // 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;
> > > +  }
> > > +
> > > +  Status = gBS->AllocatePages (AllocType, MemoryType,
> > Pages, &Memory);
> > > +  if (EFI_ERROR (Status)) {
> > > +    return NULL;
> > > +  }
> > > +  return (VOID *)(UINTN)Memory;
> > > +}
> > > --
> > > 2.17.0
> > >
> > > _______________________________________________
> > > edk2-devel mailing list
> > > edk2-devel@lists.01.org
> > > https://lists.01.org/mailman/listinfo/edk2-devel
> > > _______________________________________________
> > > edk2-devel mailing list
> > > edk2-devel@lists.01.org
> > > https://lists.01.org/mailman/listinfo/edk2-devel
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 3/5] MdePkg/DxeServicesLib: introduce AllocatePeiAccessiblePages routine
  2018-05-29 15:36         ` Yao, Jiewen
@ 2018-05-29 16:09           ` Kinney, Michael D
  2018-05-29 16:19             ` Ard Biesheuvel
  2018-05-30  0:58             ` Zeng, Star
  0 siblings, 2 replies; 19+ messages in thread
From: Kinney, Michael D @ 2018-05-29 16:09 UTC (permalink / raw)
  To: Yao, Jiewen, Zeng, Star, Ard Biesheuvel, edk2-devel@lists.01.org,
	Kinney, Michael D
  Cc: Zeng, Star

Jiewen,

I see what you mean.  It is not the submitting of
capsules you are referring to.  It is the processing
if capsules in the PEI phase that depends on some
things being setup in DXE phase and DXE phase needs
to know if PEI is in IA32 mode.

So I agree that the commit message could add capsule
processing to the list of features that can use this
new service.

The logic in that file is using PcdDxeIplSwitchToLongMode.
That PCD is TRUE when PEI is IA32 and DXE is X64.  But it
is ignored when  PEI and DXE are both X64 and could be 
TRUE or FALSE.  Is there a logic issue here with using that
PCD when PEI and DXE are both X64?

Thanks,

Mike

> -----Original Message-----
> From: Yao, Jiewen
> Sent: Tuesday, May 29, 2018 8:36 AM
> To: Kinney, Michael D <michael.d.kinney@intel.com>;
> Zeng, Star <star.zeng@intel.com>; Ard Biesheuvel
> <ard.biesheuvel@linaro.org>; edk2-devel@lists.01.org
> Cc: Zeng, Star <star.zeng@intel.com>; Yao, Jiewen
> <jiewen.yao@intel.com>
> Subject: RE: [edk2] [PATCH v3 3/5]
> MdePkg/DxeServicesLib: introduce
> AllocatePeiAccessiblePages routine
> 
> Mike
> Please refer to
> https://github.com/tianocore/edk2/blob/master/MdeModuleP
> kg/Universal/CapsuleRuntimeDxe/X64/SaveLongModeContext.c
> 
> It uses AllocateReservedMemoryBelow4G() for the context
> accessed in PEI.
> 
> Thank you
> Yao Jiewen
> 
> 
> > -----Original Message-----
> > From: Kinney, Michael D
> > Sent: Tuesday, May 29, 2018 8:31 AM
> > To: Yao, Jiewen <jiewen.yao@intel.com>; Zeng, Star
> <star.zeng@intel.com>; Ard
> > Biesheuvel <ard.biesheuvel@linaro.org>; edk2-
> devel@lists.01.org; Kinney,
> > Michael D <michael.d.kinney@intel.com>
> > Cc: Zeng, Star <star.zeng@intel.com>
> > Subject: RE: [edk2] [PATCH v3 3/5]
> MdePkg/DxeServicesLib: introduce
> > AllocatePeiAccessiblePages routine
> >
> > Jiewen,
> >
> > I do not think this service applies to the capsule
> > use cases.
> >
> > A capsule sent at OS runtime can be placed anywhere
> > in memory by the OS.  The OS does not know what
> > memory PEI can easily access or not.
> >
> > The main PEI requirement is to not corrupt capsules
> > in memory.
> >
> > Mike
> >
> > > -----Original Message-----
> > > From: edk2-devel [mailto:edk2-devel-
> > > bounces@lists.01.org] On Behalf Of Yao, Jiewen
> > > Sent: Tuesday, May 29, 2018 8:19 AM
> > > To: Zeng, Star <star.zeng@intel.com>; Ard Biesheuvel
> > > <ard.biesheuvel@linaro.org>; edk2-devel@lists.01.org
> > > Cc: Zeng, Star <star.zeng@intel.com>
> > > Subject: Re: [edk2] [PATCH v3 3/5]
> > > MdePkg/DxeServicesLib: introduce
> > > AllocatePeiAccessiblePages routine
> > >
> > > Maybe " be accessible by PEI after a warm reboot or
> S3"
> > > ?
> > >
> > > We may want to consider Capsule Update case.
> > >
> > >
> > > > -----Original Message-----
> > > > From: edk2-devel [mailto:edk2-devel-
> > > bounces@lists.01.org] On Behalf Of Zeng,
> > > > Star
> > > > Sent: Monday, May 28, 2018 6:37 PM
> > > > To: Ard Biesheuvel <ard.biesheuvel@linaro.org>;
> edk2-
> > > devel@lists.01.org
> > > > Cc: Zeng, Star <star.zeng@intel.com>
> > > > Subject: Re: [edk2] [PATCH v3 3/5]
> > > MdePkg/DxeServicesLib: introduce
> > > > AllocatePeiAccessiblePages routine
> > > >
> > > > I think " be accessible by PEI after a warm reboot
> "
> > > should be " be accessible by
> > > > PEI after resuming from S3 ".
> > > > You can update it when pushing without need to
> send a
> > > new patch if other has
> > > > no comment to the code part.
> > > >
> > > >
> > > > Thanks,
> > > > Star
> > > > -----Original Message-----
> > > > From: edk2-devel [mailto:edk2-devel-
> > > bounces@lists.01.org] On Behalf Of Ard
> > > > Biesheuvel
> > > > Sent: Monday, May 28, 2018 10:40 PM
> > > > To: edk2-devel@lists.01.org
> > > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > > > Subject: [edk2] [PATCH v3 3/5]
> MdePkg/DxeServicesLib:
> > > introduce
> > > > AllocatePeiAccessiblePages routine
> > > >
> > > > 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/Allocate.c
> | 54
> > > +++++++++++++++
> > > >  MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
> | 11
> > > +++-
> > > >  MdePkg/Library/DxeServicesLib/X64/Allocate.c
> | 69
> > > > ++++++++++++++++++++
> > > >  4 files changed, 155 insertions(+), 2 deletions(-
> )
> > > >
> > > > 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/Allocate.c
> > > > b/MdePkg/Library/DxeServicesLib/Allocate.c
> > > > new file mode 100644
> > > > index 000000000000..4d118f766d49
> > > > --- /dev/null
> > > > +++ b/MdePkg/Library/DxeServicesLib/Allocate.c
> > > > @@ -0,0 +1,54 @@
> > > > +/** @file
> > > > +  DxeServicesLib memory allocation routines
> > > > +
> > > > +  Copyright (c) 2018, Linaro, Ltd. 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 <PiDxe.h>
> > > > +#include <Library/UefiBootServicesTableLib.h>
> > > > +#include <Library/DxeServicesLib.h>
> > > > +
> > > > +/**
> > > > +  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_PHYSICAL_ADDRESS        Memory;
> > > > +
> > > > +  if (Pages == 0) {
> > > > +    return NULL;
> > > > +  }
> > > > +
> > > > +  Status = gBS->AllocatePages (AllocateAnyPages,
> > > 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..50ae24f8ee22 100644
> > > > ---
> a/MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
> > > > +++
> b/MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
> > > > @@ -27,12 +27,18 @@ [Defines]
> > > >    LIBRARY_CLASS                  =
> > > DxeServicesLib|DXE_CORE
> > > > DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SAL_DRIVER
> > > DXE_SMM_DRIVER
> > > > SMM_CORE UEFI_APPLICATION UEFI_DRIVER
> > > >
> > > >  #
> > > > -#  VALID_ARCHITECTURES           = IA32 X64 IPF
> EBC
> > > > +#  VALID_ARCHITECTURES           = IA32 X64 IPF
> EBC
> > > ARM AARCH64
> > > >  #
> > > >
> > > >  [Sources]
> > > >    DxeServicesLib.c
> > > >
> > > > +[Sources.IA32, Sources.IPF, Sources.EBC,
> Sources.ARM,
> > > Sources.AARCH64]
> > > > +  Allocate.c
> > > > +
> > > > +[Sources.X64]
> > > > +  X64/Allocate.c
> > > > +
> > > >  [Packages]
> > > >    MdePkg/MdePkg.dec
> > > >
> > > > @@ -44,6 +50,9 @@ [LibraryClasses]
> > > >    UefiLib
> > > >    UefiBootServicesTableLib
> > > >
> > > > +[LibraryClasses.X64]
> > > > +  HobLib
> > > > +
> > > >  [Guids]
> > > >    gEfiFileInfoGuid
> ##
> > > > SOMETIMES_CONSUMES ## UNDEFINED
> > > >
> > > > diff --git
> > > a/MdePkg/Library/DxeServicesLib/X64/Allocate.c
> > > > b/MdePkg/Library/DxeServicesLib/X64/Allocate.c
> > > > new file mode 100644
> > > > index 000000000000..b6d34ba20881
> > > > --- /dev/null
> > > > +++ b/MdePkg/Library/DxeServicesLib/X64/Allocate.c
> > > > @@ -0,0 +1,69 @@
> > > > +/** @file
> > > > +  DxeServicesLib memory allocation routines
> > > > +
> > > > +  Copyright (c) 2018, Linaro, Ltd. 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 <PiDxe.h>
> > > > +#include <Library/HobLib.h>
> > > > +#include <Library/UefiBootServicesTableLib.h>
> > > > +#include <Library/DxeServicesLib.h>
> > > > +
> > > > +/**
> > > > +  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;
> > > > +  EFI_HOB_HANDOFF_INFO_TABLE  *PhitHob;
> > > > +
> > > > +  if (Pages == 0) {
> > > > +    return NULL;
> > > > +  }
> > > > +
> > > > +  AllocType = AllocateAnyPages;
> > > > +  //
> > > > +  // 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;
> > > > +  }
> > > > +
> > > > +  Status = gBS->AllocatePages (AllocType,
> MemoryType,
> > > Pages, &Memory);
> > > > +  if (EFI_ERROR (Status)) {
> > > > +    return NULL;
> > > > +  }
> > > > +  return (VOID *)(UINTN)Memory;
> > > > +}
> > > > --
> > > > 2.17.0
> > > >
> > > > _______________________________________________
> > > > edk2-devel mailing list
> > > > edk2-devel@lists.01.org
> > > > https://lists.01.org/mailman/listinfo/edk2-devel
> > > > _______________________________________________
> > > > edk2-devel mailing list
> > > > edk2-devel@lists.01.org
> > > > https://lists.01.org/mailman/listinfo/edk2-devel
> > > _______________________________________________
> > > edk2-devel mailing list
> > > edk2-devel@lists.01.org
> > > https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 3/5] MdePkg/DxeServicesLib: introduce AllocatePeiAccessiblePages routine
  2018-05-29 16:09           ` Kinney, Michael D
@ 2018-05-29 16:19             ` Ard Biesheuvel
  2018-05-29 16:41               ` Yao, Jiewen
  2018-05-30  0:58             ` Zeng, Star
  1 sibling, 1 reply; 19+ messages in thread
From: Ard Biesheuvel @ 2018-05-29 16:19 UTC (permalink / raw)
  To: Kinney, Michael D; +Cc: Yao, Jiewen, Zeng, Star, edk2-devel@lists.01.org

On 29 May 2018 at 18:09, Kinney, Michael D <michael.d.kinney@intel.com> wrote:
> Jiewen,
>
> I see what you mean.  It is not the submitting of
> capsules you are referring to.  It is the processing
> if capsules in the PEI phase that depends on some
> things being setup in DXE phase and DXE phase needs
> to know if PEI is in IA32 mode.
>
> So I agree that the commit message could add capsule
> processing to the list of features that can use this
> new service.
>
> The logic in that file is using PcdDxeIplSwitchToLongMode.
> That PCD is TRUE when PEI is IA32 and DXE is X64.  But it
> is ignored when  PEI and DXE are both X64 and could be
> TRUE or FALSE.  Is there a logic issue here with using that
> PCD when PEI and DXE are both X64?
>

Hi all,

The reason I disregarded this particular case in my series is that
this code is already specific to X64. However, given the discussion
you are having, I guess the logic of looking at EfiFreeMemoryTop [at
runtime] combined with the various [build time] PCD values could help
to refine this case as well.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 3/5] MdePkg/DxeServicesLib: introduce AllocatePeiAccessiblePages routine
  2018-05-29 16:19             ` Ard Biesheuvel
@ 2018-05-29 16:41               ` Yao, Jiewen
  0 siblings, 0 replies; 19+ messages in thread
From: Yao, Jiewen @ 2018-05-29 16:41 UTC (permalink / raw)
  To: Ard Biesheuvel, Kinney, Michael D
  Cc: edk2-devel@lists.01.org, Zeng, Star, Yao, Jiewen

Thanks.
I guess we need it even for X64 PEI/X64 DXE case, because a platform may only allocates 4G even in X64 PEI.

BTW, I don't mind if you want to use a separate patch to handle capsule case.
Current API is good enough and I do not want to block the check-in. :-)

I like this simple solution.
Patch series: Reviewed-by: Jiewen.Yao@intel.com

Thank you
Yao Jiewen

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard
> Biesheuvel
> Sent: Tuesday, May 29, 2018 9:20 AM
> To: Kinney, Michael D <michael.d.kinney@intel.com>
> Cc: edk2-devel@lists.01.org; Yao, Jiewen <jiewen.yao@intel.com>; Zeng, Star
> <star.zeng@intel.com>
> Subject: Re: [edk2] [PATCH v3 3/5] MdePkg/DxeServicesLib: introduce
> AllocatePeiAccessiblePages routine
> 
> On 29 May 2018 at 18:09, Kinney, Michael D <michael.d.kinney@intel.com>
> wrote:
> > Jiewen,
> >
> > I see what you mean.  It is not the submitting of
> > capsules you are referring to.  It is the processing
> > if capsules in the PEI phase that depends on some
> > things being setup in DXE phase and DXE phase needs
> > to know if PEI is in IA32 mode.
> >
> > So I agree that the commit message could add capsule
> > processing to the list of features that can use this
> > new service.
> >
> > The logic in that file is using PcdDxeIplSwitchToLongMode.
> > That PCD is TRUE when PEI is IA32 and DXE is X64.  But it
> > is ignored when  PEI and DXE are both X64 and could be
> > TRUE or FALSE.  Is there a logic issue here with using that
> > PCD when PEI and DXE are both X64?
> >
> 
> Hi all,
> 
> The reason I disregarded this particular case in my series is that
> this code is already specific to X64. However, given the discussion
> you are having, I guess the logic of looking at EfiFreeMemoryTop [at
> runtime] combined with the various [build time] PCD values could help
> to refine this case as well.
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 3/5] MdePkg/DxeServicesLib: introduce AllocatePeiAccessiblePages routine
  2018-05-29 16:09           ` Kinney, Michael D
  2018-05-29 16:19             ` Ard Biesheuvel
@ 2018-05-30  0:58             ` Zeng, Star
  1 sibling, 0 replies; 19+ messages in thread
From: Zeng, Star @ 2018-05-30  0:58 UTC (permalink / raw)
  To: Kinney, Michael D, Yao, Jiewen, Ard Biesheuvel,
	edk2-devel@lists.01.org
  Cc: Zeng, Star

We can consider capsule update path is a special S3 path. :)
Both capsule update path and S3 path have the attribute that the memory content at previous boot are reserved.


Thanks,
Star
-----Original Message-----
From: Kinney, Michael D 
Sent: Wednesday, May 30, 2018 12:09 AM
To: Yao, Jiewen <jiewen.yao@intel.com>; Zeng, Star <star.zeng@intel.com>; Ard Biesheuvel <ard.biesheuvel@linaro.org>; edk2-devel@lists.01.org; Kinney, Michael D <michael.d.kinney@intel.com>
Cc: Zeng, Star <star.zeng@intel.com>
Subject: RE: [edk2] [PATCH v3 3/5] MdePkg/DxeServicesLib: introduce AllocatePeiAccessiblePages routine

Jiewen,

I see what you mean.  It is not the submitting of capsules you are referring to.  It is the processing if capsules in the PEI phase that depends on some things being setup in DXE phase and DXE phase needs to know if PEI is in IA32 mode.

So I agree that the commit message could add capsule processing to the list of features that can use this new service.

The logic in that file is using PcdDxeIplSwitchToLongMode.
That PCD is TRUE when PEI is IA32 and DXE is X64.  But it is ignored when  PEI and DXE are both X64 and could be TRUE or FALSE.  Is there a logic issue here with using that PCD when PEI and DXE are both X64?

Thanks,

Mike

> -----Original Message-----
> From: Yao, Jiewen
> Sent: Tuesday, May 29, 2018 8:36 AM
> To: Kinney, Michael D <michael.d.kinney@intel.com>; Zeng, Star 
> <star.zeng@intel.com>; Ard Biesheuvel <ard.biesheuvel@linaro.org>; 
> edk2-devel@lists.01.org
> Cc: Zeng, Star <star.zeng@intel.com>; Yao, Jiewen 
> <jiewen.yao@intel.com>
> Subject: RE: [edk2] [PATCH v3 3/5]
> MdePkg/DxeServicesLib: introduce
> AllocatePeiAccessiblePages routine
> 
> Mike
> Please refer to
> https://github.com/tianocore/edk2/blob/master/MdeModuleP
> kg/Universal/CapsuleRuntimeDxe/X64/SaveLongModeContext.c
> 
> It uses AllocateReservedMemoryBelow4G() for the context accessed in 
> PEI.
> 
> Thank you
> Yao Jiewen
> 
> 
> > -----Original Message-----
> > From: Kinney, Michael D
> > Sent: Tuesday, May 29, 2018 8:31 AM
> > To: Yao, Jiewen <jiewen.yao@intel.com>; Zeng, Star
> <star.zeng@intel.com>; Ard
> > Biesheuvel <ard.biesheuvel@linaro.org>; edk2-
> devel@lists.01.org; Kinney,
> > Michael D <michael.d.kinney@intel.com>
> > Cc: Zeng, Star <star.zeng@intel.com>
> > Subject: RE: [edk2] [PATCH v3 3/5]
> MdePkg/DxeServicesLib: introduce
> > AllocatePeiAccessiblePages routine
> >
> > Jiewen,
> >
> > I do not think this service applies to the capsule use cases.
> >
> > A capsule sent at OS runtime can be placed anywhere in memory by the 
> > OS.  The OS does not know what memory PEI can easily access or not.
> >
> > The main PEI requirement is to not corrupt capsules in memory.
> >
> > Mike
> >
> > > -----Original Message-----
> > > From: edk2-devel [mailto:edk2-devel- bounces@lists.01.org] On 
> > > Behalf Of Yao, Jiewen
> > > Sent: Tuesday, May 29, 2018 8:19 AM
> > > To: Zeng, Star <star.zeng@intel.com>; Ard Biesheuvel 
> > > <ard.biesheuvel@linaro.org>; edk2-devel@lists.01.org
> > > Cc: Zeng, Star <star.zeng@intel.com>
> > > Subject: Re: [edk2] [PATCH v3 3/5]
> > > MdePkg/DxeServicesLib: introduce
> > > AllocatePeiAccessiblePages routine
> > >
> > > Maybe " be accessible by PEI after a warm reboot or
> S3"
> > > ?
> > >
> > > We may want to consider Capsule Update case.
> > >
> > >
> > > > -----Original Message-----
> > > > From: edk2-devel [mailto:edk2-devel-
> > > bounces@lists.01.org] On Behalf Of Zeng,
> > > > Star
> > > > Sent: Monday, May 28, 2018 6:37 PM
> > > > To: Ard Biesheuvel <ard.biesheuvel@linaro.org>;
> edk2-
> > > devel@lists.01.org
> > > > Cc: Zeng, Star <star.zeng@intel.com>
> > > > Subject: Re: [edk2] [PATCH v3 3/5]
> > > MdePkg/DxeServicesLib: introduce
> > > > AllocatePeiAccessiblePages routine
> > > >
> > > > I think " be accessible by PEI after a warm reboot
> "
> > > should be " be accessible by
> > > > PEI after resuming from S3 ".
> > > > You can update it when pushing without need to
> send a
> > > new patch if other has
> > > > no comment to the code part.
> > > >
> > > >
> > > > Thanks,
> > > > Star
> > > > -----Original Message-----
> > > > From: edk2-devel [mailto:edk2-devel-
> > > bounces@lists.01.org] On Behalf Of Ard
> > > > Biesheuvel
> > > > Sent: Monday, May 28, 2018 10:40 PM
> > > > To: edk2-devel@lists.01.org
> > > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > > > Subject: [edk2] [PATCH v3 3/5]
> MdePkg/DxeServicesLib:
> > > introduce
> > > > AllocatePeiAccessiblePages routine
> > > >
> > > > 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/Allocate.c
> | 54
> > > +++++++++++++++
> > > >  MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
> | 11
> > > +++-
> > > >  MdePkg/Library/DxeServicesLib/X64/Allocate.c
> | 69
> > > > ++++++++++++++++++++
> > > >  4 files changed, 155 insertions(+), 2 deletions(-
> )
> > > >
> > > > 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/Allocate.c
> > > > b/MdePkg/Library/DxeServicesLib/Allocate.c
> > > > new file mode 100644
> > > > index 000000000000..4d118f766d49
> > > > --- /dev/null
> > > > +++ b/MdePkg/Library/DxeServicesLib/Allocate.c
> > > > @@ -0,0 +1,54 @@
> > > > +/** @file
> > > > +  DxeServicesLib memory allocation routines
> > > > +
> > > > +  Copyright (c) 2018, Linaro, Ltd. 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 <PiDxe.h>
> > > > +#include <Library/UefiBootServicesTableLib.h>
> > > > +#include <Library/DxeServicesLib.h>
> > > > +
> > > > +/**
> > > > +  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_PHYSICAL_ADDRESS        Memory;
> > > > +
> > > > +  if (Pages == 0) {
> > > > +    return NULL;
> > > > +  }
> > > > +
> > > > +  Status = gBS->AllocatePages (AllocateAnyPages,
> > > 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..50ae24f8ee22 100644
> > > > ---
> a/MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
> > > > +++
> b/MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
> > > > @@ -27,12 +27,18 @@ [Defines]
> > > >    LIBRARY_CLASS                  =
> > > DxeServicesLib|DXE_CORE
> > > > DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SAL_DRIVER
> > > DXE_SMM_DRIVER
> > > > SMM_CORE UEFI_APPLICATION UEFI_DRIVER
> > > >
> > > >  #
> > > > -#  VALID_ARCHITECTURES           = IA32 X64 IPF
> EBC
> > > > +#  VALID_ARCHITECTURES           = IA32 X64 IPF
> EBC
> > > ARM AARCH64
> > > >  #
> > > >
> > > >  [Sources]
> > > >    DxeServicesLib.c
> > > >
> > > > +[Sources.IA32, Sources.IPF, Sources.EBC,
> Sources.ARM,
> > > Sources.AARCH64]
> > > > +  Allocate.c
> > > > +
> > > > +[Sources.X64]
> > > > +  X64/Allocate.c
> > > > +
> > > >  [Packages]
> > > >    MdePkg/MdePkg.dec
> > > >
> > > > @@ -44,6 +50,9 @@ [LibraryClasses]
> > > >    UefiLib
> > > >    UefiBootServicesTableLib
> > > >
> > > > +[LibraryClasses.X64]
> > > > +  HobLib
> > > > +
> > > >  [Guids]
> > > >    gEfiFileInfoGuid
> ##
> > > > SOMETIMES_CONSUMES ## UNDEFINED
> > > >
> > > > diff --git
> > > a/MdePkg/Library/DxeServicesLib/X64/Allocate.c
> > > > b/MdePkg/Library/DxeServicesLib/X64/Allocate.c
> > > > new file mode 100644
> > > > index 000000000000..b6d34ba20881
> > > > --- /dev/null
> > > > +++ b/MdePkg/Library/DxeServicesLib/X64/Allocate.c
> > > > @@ -0,0 +1,69 @@
> > > > +/** @file
> > > > +  DxeServicesLib memory allocation routines
> > > > +
> > > > +  Copyright (c) 2018, Linaro, Ltd. 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 <PiDxe.h>
> > > > +#include <Library/HobLib.h>
> > > > +#include <Library/UefiBootServicesTableLib.h>
> > > > +#include <Library/DxeServicesLib.h>
> > > > +
> > > > +/**
> > > > +  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;
> > > > +  EFI_HOB_HANDOFF_INFO_TABLE  *PhitHob;
> > > > +
> > > > +  if (Pages == 0) {
> > > > +    return NULL;
> > > > +  }
> > > > +
> > > > +  AllocType = AllocateAnyPages;  //  // 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;
> > > > +  }
> > > > +
> > > > +  Status = gBS->AllocatePages (AllocType,
> MemoryType,
> > > Pages, &Memory);
> > > > +  if (EFI_ERROR (Status)) {
> > > > +    return NULL;
> > > > +  }
> > > > +  return (VOID *)(UINTN)Memory; }
> > > > --
> > > > 2.17.0
> > > >
> > > > _______________________________________________
> > > > edk2-devel mailing list
> > > > edk2-devel@lists.01.org
> > > > https://lists.01.org/mailman/listinfo/edk2-devel
> > > > _______________________________________________
> > > > edk2-devel mailing list
> > > > edk2-devel@lists.01.org
> > > > https://lists.01.org/mailman/listinfo/edk2-devel
> > > _______________________________________________
> > > edk2-devel mailing list
> > > edk2-devel@lists.01.org
> > > https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2018-05-30  0:58 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-05-28 14:40 [PATCH v3 0/5] Abstract allocation of PEI accessible memory Ard Biesheuvel
2018-05-28 14:40 ` [PATCH v3 1/5] OvmfPkg/PlatformBootManagerLib: add missing report status code call Ard Biesheuvel
2018-05-28 14:40 ` [PATCH v3 2/5] ArmVirtPkg/PlatformBootManagerLib: " Ard Biesheuvel
2018-05-28 14:40 ` [PATCH v3 3/5] MdePkg/DxeServicesLib: introduce AllocatePeiAccessiblePages routine Ard Biesheuvel
2018-05-28 14:59   ` Laszlo Ersek
2018-05-29  3:34     ` Gao, Liming
2018-05-29  1:37   ` Zeng, Star
2018-05-29 15:18     ` Yao, Jiewen
2018-05-29 15:31       ` Kinney, Michael D
2018-05-29 15:36         ` Yao, Jiewen
2018-05-29 16:09           ` Kinney, Michael D
2018-05-29 16:19             ` Ard Biesheuvel
2018-05-29 16:41               ` Yao, Jiewen
2018-05-30  0:58             ` Zeng, Star
2018-05-28 14:40 ` [PATCH v3 4/5] MdeModulePkg/DxeCorePerformanceLib: use AllocatePeiAccessiblePages Ard Biesheuvel
2018-05-29  1:33   ` Zeng, Star
2018-05-28 14:40 ` [PATCH v3 5/5] MdeModulePkg/FirmwarePerformanceDataTableDxe: " Ard Biesheuvel
2018-05-29  1:32   ` Zeng, Star
2018-05-29  8:49 ` [PATCH v3 0/5] Abstract allocation of PEI accessible memory Ard Biesheuvel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox