[PATCH v2 0/3] MdeModulePkg: use pool allocations for ACPI tables

[PATCH v2 0/3] MdeModulePkg: use pool allocations for ACPI tables

[Ard Biesheuvel]

Currently, the AcpiTableDxe memory allocator uses page based allocations,
for which the only reason seems to be that it permits the use of a memory
limit, which is necessary for ACPI 1.0 tables that need to reside in the
first 4 GB of memory.

That requirement does not exist on AArch64, and since page based allocations
are rounded up to 64 KB multiples, this wastes some memory in a way that
can easily be avoided. So let's use the existing 'mAcpiTableAllocType'
policy variable, and switch to pool allocations if it is set to
'AllocateAnyPages'

Changes since v1:
- incorporate Laszlo's review feedback
  . update description of EFI_ACPI_TABLE_LIST struct
  . avoid EFI_PHYSICAL_ADDRESS to VOID* conversion in case AllocatePages()
    returns an error
  . put closing ) on a line by itself when using the non-condensed format
    for passing function call arguments.
  . fix error in comment text

Example output from Linux booting on ArmVirtQemu:

Before:
 ACPI: RSDP 0x0000000078510000 000024 (v02 BOCHS )
 ACPI: XSDT 0x0000000078500000 00004C (v01 BOCHS  BXPCFACP 00000001      01000013)
 ACPI: FACP 0x00000000784C0000 00010C (v05 BOCHS  BXPCFACP 00000001 BXPC 00000001)
 ACPI: DSDT 0x00000000784D0000 0014BB (v02 BOCHS  BXPCDSDT 00000001 BXPC 00000001)
 ACPI: APIC 0x00000000784B0000 0000A8 (v03 BOCHS  BXPCAPIC 00000001 BXPC 00000001)
 ACPI: GTDT 0x00000000784A0000 000060 (v02 BOCHS  BXPCGTDT 00000001 BXPC 00000001)
 ACPI: MCFG 0x0000000078490000 00003C (v01 BOCHS  BXPCMCFG 00000001 BXPC 00000001)
 ACPI: SPCR 0x0000000078480000 000050 (v02 BOCHS  BXPCSPCR 00000001 BXPC 00000001)

After:
 ACPI: RSDP 0x000000007C030018 000024 (v02 BOCHS )
 ACPI: XSDT 0x000000007C03FE98 00004C (v01 BOCHS  BXPCFACP 00000001      01000013)
 ACPI: FACP 0x000000007C03FA98 00010C (v05 BOCHS  BXPCFACP 00000001 BXPC 00000001)
 ACPI: DSDT 0x000000007C037518 0014BB (v02 BOCHS  BXPCDSDT 00000001 BXPC 00000001)
 ACPI: APIC 0x000000007C03FC18 0000A8 (v03 BOCHS  BXPCAPIC 00000001 BXPC 00000001)
 ACPI: GTDT 0x000000007C03FD18 000060 (v02 BOCHS  BXPCGTDT 00000001 BXPC 00000001)
 ACPI: MCFG 0x000000007C03FE18 00003C (v01 BOCHS  BXPCMCFG 00000001 BXPC 00000001)
 ACPI: SPCR 0x000000007C03FF98 000050 (v02 BOCHS  BXPCSPCR 00000001 BXPC 00000001)

Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Leif Lindholm <leif@nuviainc.com>

Ard Biesheuvel (3):
  MdeModulePkg/AcpiTableDxe: use pool allocations when possible
  MdeModulePkg/AcpiTableDxe: use pool allocation for RSDT/XSDT if
    possible
  MdeModulePkg/AcpiTableDxe: use pool allocation for RSDP if possible

 .../Universal/Acpi/AcpiTableDxe/AcpiTable.h   |  11 +-
 .../Universal/Acpi/AcpiTableDxe/AcpiSdt.c     |   4 +-
 .../Acpi/AcpiTableDxe/AcpiTableProtocol.c     | 230 ++++++++++++------
 3 files changed, 162 insertions(+), 83 deletions(-)

-- 
2.17.1

[PATCH v2 1/3] MdeModulePkg/AcpiTableDxe: use pool allocations when possible

[Ard Biesheuvel]

On AArch64 systems, page based allocations for memory types that are
relevant to the OS are rounded up to 64 KB multiples. This wastes
some space in the ACPI table memory allocator, since it uses page
based allocations in order to be able to place the ACPI tables low
in memory.

Since the latter requirement does not exist on AArch64, switch to pool
allocations for all ACPI tables except the root tables if the active
allocation policy permits them to be anywhere in memory. The root
tables will be handled in a subsequent patch.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
---
 MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTable.h         | 11 ++-
 MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiSdt.c           |  4 +-
 MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c | 79 ++++++++++++++------
 3 files changed, 66 insertions(+), 28 deletions(-)

diff --git a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTable.h b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTable.h
index 425a462fd92b..9d7cf7ccfc76 100644
--- a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTable.h
+++ b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTable.h
@@ -55,18 +55,21 @@
 //  Link is the linked list data.
 //  Version is the versions of the ACPI tables that this table belongs in.
 //  Table is a pointer to the table.
-//  PageAddress is the address of the pages allocated for the table.
-//  NumberOfPages is the number of pages allocated at PageAddress.
+//  TableSize is the size of the table
 //  Handle is used to identify a particular table.
+//  PoolAllocation carries the allocation type:
+//    FALSE: Table points to EFI_SIZE_TO_PAGES(TableSize) pages allocated using
+//           gBS->AllocatePages ()
+//    TRUE:  Table points to TableSize bytes allocated using gBS->AllocatePool ()
 //
 typedef struct {
   UINT32                  Signature;
   LIST_ENTRY              Link;
   EFI_ACPI_TABLE_VERSION  Version;
   EFI_ACPI_COMMON_HEADER  *Table;
-  EFI_PHYSICAL_ADDRESS    PageAddress;
-  UINTN                   NumberOfPages;
+  UINTN                   TableSize;
   UINTN                   Handle;
+  BOOLEAN                 PoolAllocation;
 } EFI_ACPI_TABLE_LIST;
 
 //
diff --git a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiSdt.c b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiSdt.c
index b1cba20c8ed7..14ced68e645f 100644
--- a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiSdt.c
+++ b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiSdt.c
@@ -68,8 +68,8 @@ FindTableByBuffer (
 
   while (CurrentLink != StartLink) {
     CurrentTableList = EFI_ACPI_TABLE_LIST_FROM_LINK (CurrentLink);
-    if (((UINTN)CurrentTableList->PageAddress <= (UINTN)Buffer) &&
-        ((UINTN)CurrentTableList->PageAddress + EFI_PAGES_TO_SIZE(CurrentTableList->NumberOfPages) > (UINTN)Buffer)) {
+    if (((UINTN)CurrentTableList->Table <= (UINTN)Buffer) &&
+        ((UINTN)CurrentTableList->Table + CurrentTableList->TableSize > (UINTN)Buffer)) {
       //
       // Good! Found Table.
       //
diff --git a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
index ad7baf8205b4..e85a98aee6bf 100644
--- a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
+++ b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
@@ -428,6 +428,26 @@ ReallocateAcpiTableBuffer (
   return EFI_SUCCESS;
 }
 
+/**
+  Free the memory associated with the provided EFI_ACPI_TABLE_LIST instance
+
+  @param  TableEntry                EFI_ACPI_TABLE_LIST instance pointer
+
+**/
+STATIC
+VOID
+FreeTableMemory (
+  EFI_ACPI_TABLE_LIST   *TableEntry
+  )
+{
+  if (TableEntry->PoolAllocation) {
+    gBS->FreePool (TableEntry->Table);
+  } else {
+    gBS->FreePages ((EFI_PHYSICAL_ADDRESS)(UINTN)TableEntry->Table,
+           EFI_SIZE_TO_PAGES (TableEntry->TableSize));
+  }
+}
+
 /**
   This function adds an ACPI table to the table list.  It will detect FACS and
   allocate the correct type of memory and properly align the table.
@@ -454,14 +474,15 @@ AddTableToList (
   OUT UINTN                               *Handle
   )
 {
-  EFI_STATUS          Status;
-  EFI_ACPI_TABLE_LIST *CurrentTableList;
-  UINT32              CurrentTableSignature;
-  UINT32              CurrentTableSize;
-  UINT32              *CurrentRsdtEntry;
-  VOID                *CurrentXsdtEntry;
-  UINT64              Buffer64;
-  BOOLEAN             AddToRsdt;
+  EFI_STATUS            Status;
+  EFI_ACPI_TABLE_LIST   *CurrentTableList;
+  UINT32                CurrentTableSignature;
+  UINT32                CurrentTableSize;
+  UINT32                *CurrentRsdtEntry;
+  VOID                  *CurrentXsdtEntry;
+  EFI_PHYSICAL_ADDRESS  AllocPhysAddress;
+  UINT64                Buffer64;
+  BOOLEAN               AddToRsdt;
 
   //
   // Check for invalid input parameters
@@ -496,8 +517,9 @@ AddTableToList (
   // There is no architectural reason these should be below 4GB, it is purely
   // for convenience of implementation that we force memory below 4GB.
   //
-  CurrentTableList->PageAddress   = 0xFFFFFFFF;
-  CurrentTableList->NumberOfPages = EFI_SIZE_TO_PAGES (CurrentTableSize);
+  AllocPhysAddress                  = 0xFFFFFFFF;
+  CurrentTableList->TableSize       = CurrentTableSize;
+  CurrentTableList->PoolAllocation  = FALSE;
 
   //
   // Allocation memory type depends on the type of the table
@@ -518,9 +540,21 @@ AddTableToList (
     Status = gBS->AllocatePages (
                     AllocateMaxAddress,
                     EfiACPIMemoryNVS,
-                    CurrentTableList->NumberOfPages,
-                    &CurrentTableList->PageAddress
+                    EFI_SIZE_TO_PAGES (CurrentTableList->TableSize),
+                    &AllocPhysAddress
                     );
+  } else if (mAcpiTableAllocType == AllocateAnyPages) {
+    //
+    // If there is no allocation limit, there is also no need to use page
+    // based allocations for ACPI tables, which may be wasteful on platforms
+    // such as AArch64 that allocate multiples of 64 KB
+    //
+    Status = gBS->AllocatePool (
+                    EfiACPIReclaimMemory,
+                    CurrentTableList->TableSize,
+                    (VOID **)&CurrentTableList->Table
+                    );
+    CurrentTableList->PoolAllocation = TRUE;
   } else {
     //
     // All other tables are ACPI reclaim memory, no alignment requirements.
@@ -528,9 +562,10 @@ AddTableToList (
     Status = gBS->AllocatePages (
                     mAcpiTableAllocType,
                     EfiACPIReclaimMemory,
-                    CurrentTableList->NumberOfPages,
-                    &CurrentTableList->PageAddress
+                    EFI_SIZE_TO_PAGES (CurrentTableList->TableSize),
+                    &AllocPhysAddress
                     );
+    CurrentTableList->Table = (EFI_ACPI_COMMON_HEADER *)(UINTN)AllocPhysAddress;
   }
   //
   // Check return value from memory alloc.
@@ -539,10 +574,10 @@ AddTableToList (
     gBS->FreePool (CurrentTableList);
     return EFI_OUT_OF_RESOURCES;
   }
-  //
-  // Update the table pointer with the allocated memory start
-  //
-  CurrentTableList->Table = (EFI_ACPI_COMMON_HEADER *) (UINTN) CurrentTableList->PageAddress;
+
+  if (!CurrentTableList->PoolAllocation) {
+    CurrentTableList->Table = (EFI_ACPI_COMMON_HEADER *)(UINTN)AllocPhysAddress;
+  }
 
   //
   // Initialize the table contents
@@ -575,7 +610,7 @@ AddTableToList (
     if (((Version & EFI_ACPI_TABLE_VERSION_1_0B) != 0 && AcpiTableInstance->Fadt1 != NULL) ||
         ((Version & ACPI_TABLE_VERSION_GTE_2_0)  != 0 && AcpiTableInstance->Fadt3 != NULL)
         ) {
-      gBS->FreePages (CurrentTableList->PageAddress, CurrentTableList->NumberOfPages);
+      FreeTableMemory (CurrentTableList);
       gBS->FreePool (CurrentTableList);
       return EFI_ACCESS_DENIED;
     }
@@ -729,7 +764,7 @@ AddTableToList (
     if (((Version & EFI_ACPI_TABLE_VERSION_1_0B) != 0 && AcpiTableInstance->Facs1 != NULL) ||
         ((Version & ACPI_TABLE_VERSION_GTE_2_0)  != 0 && AcpiTableInstance->Facs3 != NULL)
         ) {
-      gBS->FreePages (CurrentTableList->PageAddress, CurrentTableList->NumberOfPages);
+      FreeTableMemory (CurrentTableList);
       gBS->FreePool (CurrentTableList);
       return EFI_ACCESS_DENIED;
     }
@@ -813,7 +848,7 @@ AddTableToList (
     if (((Version & EFI_ACPI_TABLE_VERSION_1_0B) != 0 && AcpiTableInstance->Dsdt1 != NULL) ||
         ((Version & ACPI_TABLE_VERSION_GTE_2_0)  != 0 && AcpiTableInstance->Dsdt3 != NULL)
         ) {
-      gBS->FreePages (CurrentTableList->PageAddress, CurrentTableList->NumberOfPages);
+      FreeTableMemory (CurrentTableList);
       gBS->FreePool (CurrentTableList);
       return EFI_ACCESS_DENIED;
     }
@@ -1449,7 +1484,7 @@ DeleteTable (
     //
     // Free the Table
     //
-    gBS->FreePages (Table->PageAddress, Table->NumberOfPages);
+    FreeTableMemory (Table);
     RemoveEntryList (&(Table->Link));
     gBS->FreePool (Table);
   }
-- 
2.17.1

[PATCH v2 2/3] MdeModulePkg/AcpiTableDxe: use pool allocation for RSDT/XSDT if possible

[Ard Biesheuvel]

If no memory allocation limit is in effect for ACPI tables, prefer
pool allocations over page allocations, to avoid wasting memory on
systems where page based allocations are rounded up to 64 KB, such
as AArch64.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
---
 MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c | 118 ++++++++++++--------
 1 file changed, 72 insertions(+), 46 deletions(-)

diff --git a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
index e85a98aee6bf..3793291a9668 100644
--- a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
+++ b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
@@ -355,28 +355,39 @@ ReallocateAcpiTableBuffer (
                  NewMaxTableNumber * sizeof (UINT32);
   }
 
-  //
-  // Allocate memory in the lower 32 bit of address range for
-  // compatibility with ACPI 1.0 OS.
-  //
-  // This is done because ACPI 1.0 pointers are 32 bit values.
-  // ACPI 2.0 OS and all 64 bit OS must use the 64 bit ACPI table addresses.
-  // There is no architectural reason these should be below 4GB, it is purely
-  // for convenience of implementation that we force memory below 4GB.
-  //
-  PageAddress = 0xFFFFFFFF;
-  Status = gBS->AllocatePages (
-                  mAcpiTableAllocType,
-                  EfiACPIReclaimMemory,
-                  EFI_SIZE_TO_PAGES (TotalSize),
-                  &PageAddress
-                  );
+  if (mAcpiTableAllocType != AllocateAnyPages) {
+    //
+    // Allocate memory in the lower 32 bit of address range for
+    // compatibility with ACPI 1.0 OS.
+    //
+    // This is done because ACPI 1.0 pointers are 32 bit values.
+    // ACPI 2.0 OS and all 64 bit OS must use the 64 bit ACPI table addresses.
+    // There is no architectural reason these should be below 4GB, it is purely
+    // for convenience of implementation that we force memory below 4GB.
+    //
+    PageAddress = 0xFFFFFFFF;
+    Status = gBS->AllocatePages (
+                    mAcpiTableAllocType,
+                    EfiACPIReclaimMemory,
+                    EFI_SIZE_TO_PAGES (TotalSize),
+                    &PageAddress
+                    );
+  } else {
+    Status = gBS->AllocatePool (
+                    EfiACPIReclaimMemory,
+                    TotalSize,
+                    (VOID **)&Pointer
+                    );
+  }
 
   if (EFI_ERROR (Status)) {
     return EFI_OUT_OF_RESOURCES;
   }
 
-  Pointer = (UINT8 *) (UINTN) PageAddress;
+  if (mAcpiTableAllocType != AllocateAnyPages) {
+    Pointer = (UINT8 *)(UINTN)PageAddress;
+  }
+
   ZeroMem (Pointer, TotalSize);
 
   AcpiTableInstance->Rsdt1 = (EFI_ACPI_DESCRIPTION_HEADER *) Pointer;
@@ -406,21 +417,26 @@ ReallocateAcpiTableBuffer (
   }
   CopyMem (AcpiTableInstance->Xsdt, TempPrivateData.Xsdt, (sizeof (EFI_ACPI_DESCRIPTION_HEADER) + mEfiAcpiMaxNumTables * sizeof (UINT64)));
 
-  //
-  // Calculate orignal ACPI table buffer size
-  //
-  TotalSize = sizeof (EFI_ACPI_DESCRIPTION_HEADER) +         // for ACPI 2.0/3.0 XSDT
-              mEfiAcpiMaxNumTables * sizeof (UINT64);
+  if (mAcpiTableAllocType != AllocateAnyPages) {
+    //
+    // Calculate orignal ACPI table buffer size
+    //
+    TotalSize = sizeof (EFI_ACPI_DESCRIPTION_HEADER) +         // for ACPI 2.0/3.0 XSDT
+                mEfiAcpiMaxNumTables * sizeof (UINT64);
 
-  if ((PcdGet32 (PcdAcpiExposedTableVersions) & EFI_ACPI_TABLE_VERSION_1_0B) != 0) {
-    TotalSize += sizeof (EFI_ACPI_DESCRIPTION_HEADER) +         // for ACPI 1.0 RSDT
-                 mEfiAcpiMaxNumTables * sizeof (UINT32) +
-                 sizeof (EFI_ACPI_DESCRIPTION_HEADER) +         // for ACPI 2.0/3.0 RSDT
-                 mEfiAcpiMaxNumTables * sizeof (UINT32);
+    if ((PcdGet32 (PcdAcpiExposedTableVersions) & EFI_ACPI_TABLE_VERSION_1_0B) != 0) {
+      TotalSize += sizeof (EFI_ACPI_DESCRIPTION_HEADER) +         // for ACPI 1.0 RSDT
+                   mEfiAcpiMaxNumTables * sizeof (UINT32) +
+                   sizeof (EFI_ACPI_DESCRIPTION_HEADER) +         // for ACPI 2.0/3.0 RSDT
+                   mEfiAcpiMaxNumTables * sizeof (UINT32);
+    }
+
+    gBS->FreePages ((EFI_PHYSICAL_ADDRESS)(UINTN)TempPrivateData.Rsdt1,
+           EFI_SIZE_TO_PAGES (TotalSize));
+  } else {
+    gBS->FreePool (TempPrivateData.Rsdt1);
   }
 
-  gBS->FreePages ((EFI_PHYSICAL_ADDRESS)(UINTN)TempPrivateData.Rsdt1, EFI_SIZE_TO_PAGES (TotalSize));
-
   //
   // Update the Max ACPI table number
   //
@@ -1763,29 +1779,39 @@ AcpiTableAcpiTableConstructor (
                  mEfiAcpiMaxNumTables * sizeof (UINT32);
   }
 
-  //
-  // Allocate memory in the lower 32 bit of address range for
-  // compatibility with ACPI 1.0 OS.
-  //
-  // This is done because ACPI 1.0 pointers are 32 bit values.
-  // ACPI 2.0 OS and all 64 bit OS must use the 64 bit ACPI table addresses.
-  // There is no architectural reason these should be below 4GB, it is purely
-  // for convenience of implementation that we force memory below 4GB.
-  //
-  PageAddress = 0xFFFFFFFF;
-  Status = gBS->AllocatePages (
-                  mAcpiTableAllocType,
-                  EfiACPIReclaimMemory,
-                  EFI_SIZE_TO_PAGES (TotalSize),
-                  &PageAddress
-                  );
+  if (mAcpiTableAllocType != AllocateAnyPages) {
+    //
+    // Allocate memory in the lower 32 bit of address range for
+    // compatibility with ACPI 1.0 OS.
+    //
+    // This is done because ACPI 1.0 pointers are 32 bit values.
+    // ACPI 2.0 OS and all 64 bit OS must use the 64 bit ACPI table addresses.
+    // There is no architectural reason these should be below 4GB, it is purely
+    // for convenience of implementation that we force memory below 4GB.
+    //
+    PageAddress = 0xFFFFFFFF;
+    Status = gBS->AllocatePages (
+                    mAcpiTableAllocType,
+                    EfiACPIReclaimMemory,
+                    EFI_SIZE_TO_PAGES (TotalSize),
+                    &PageAddress
+                    );
+  } else {
+    Status = gBS->AllocatePool (
+                    EfiACPIReclaimMemory,
+                    TotalSize,
+                    (VOID **)&Pointer
+                    );
+  }
 
   if (EFI_ERROR (Status)) {
     gBS->FreePages ((EFI_PHYSICAL_ADDRESS)(UINTN)AcpiTableInstance->Rsdp1, EFI_SIZE_TO_PAGES (RsdpTableSize));
     return EFI_OUT_OF_RESOURCES;
   }
 
-  Pointer = (UINT8 *) (UINTN) PageAddress;
+  if (mAcpiTableAllocType != AllocateAnyPages) {
+    Pointer = (UINT8 *)(UINTN)PageAddress;
+  }
   ZeroMem (Pointer, TotalSize);
 
   AcpiTableInstance->Rsdt1 = (EFI_ACPI_DESCRIPTION_HEADER *) Pointer;
-- 
2.17.1

[PATCH v2 3/3] MdeModulePkg/AcpiTableDxe: use pool allocation for RSDP if possible

[Ard Biesheuvel]

Use a pool allocation for the RSDP ACPI root pointer structure if no
memory limit is in effect that forces us to use page based allocation,
which may be wasteful if they get rounded up to 64 KB as is the case
on AArch64.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
---
 MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c | 33 ++++++++++++++------
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
index 3793291a9668..edd7f9afd4fc 100644
--- a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
+++ b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
@@ -1745,19 +1745,29 @@ AcpiTableAcpiTableConstructor (
     RsdpTableSize += sizeof (EFI_ACPI_1_0_ROOT_SYSTEM_DESCRIPTION_POINTER);
   }
 
-  PageAddress = 0xFFFFFFFF;
-  Status = gBS->AllocatePages (
-                  mAcpiTableAllocType,
-                  EfiACPIReclaimMemory,
-                  EFI_SIZE_TO_PAGES (RsdpTableSize),
-                  &PageAddress
-                  );
+  if (mAcpiTableAllocType != AllocateAnyPages) {
+    PageAddress = 0xFFFFFFFF;
+    Status = gBS->AllocatePages (
+                    mAcpiTableAllocType,
+                    EfiACPIReclaimMemory,
+                    EFI_SIZE_TO_PAGES (RsdpTableSize),
+                    &PageAddress
+                    );
+  } else {
+    Status = gBS->AllocatePool (
+                    EfiACPIReclaimMemory,
+                    RsdpTableSize,
+                    (VOID **)&Pointer
+                    );
+  }
 
   if (EFI_ERROR (Status)) {
     return EFI_OUT_OF_RESOURCES;
   }
 
-  Pointer = (UINT8 *) (UINTN) PageAddress;
+  if (mAcpiTableAllocType != AllocateAnyPages) {
+    Pointer = (UINT8 *)(UINTN)PageAddress;
+  }
   ZeroMem (Pointer, RsdpTableSize);
 
   AcpiTableInstance->Rsdp1 = (EFI_ACPI_1_0_ROOT_SYSTEM_DESCRIPTION_POINTER *) Pointer;
@@ -1805,7 +1815,12 @@ AcpiTableAcpiTableConstructor (
   }
 
   if (EFI_ERROR (Status)) {
-    gBS->FreePages ((EFI_PHYSICAL_ADDRESS)(UINTN)AcpiTableInstance->Rsdp1, EFI_SIZE_TO_PAGES (RsdpTableSize));
+    if (mAcpiTableAllocType != AllocateAnyPages) {
+      gBS->FreePages ((EFI_PHYSICAL_ADDRESS)(UINTN)AcpiTableInstance->Rsdp1,
+             EFI_SIZE_TO_PAGES (RsdpTableSize));
+    } else {
+      gBS->FreePool (AcpiTableInstance->Rsdp1);
+    }
     return EFI_OUT_OF_RESOURCES;
   }
 
-- 
2.17.1

回复: [edk2-devel] [PATCH v2 0/3] MdeModulePkg: use pool allocations for ACPI tables

[gaoliming]

Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>

> -----邮件原件-----
> 发件人: bounce+27952+66710+4905953+8761045@groups.io
> <bounce+27952+66710+4905953+8761045@groups.io> 代表 Ard
> Biesheuvel
> 发送时间: 2020年10月29日 3:42
> 收件人: devel@edk2.groups.io
> 抄送: Ard Biesheuvel <ard.biesheuvel@arm.com>; Dandan Bi
> <dandan.bi@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Jian J
> Wang <jian.j.wang@intel.com>; Hao A Wu <hao.a.wu@intel.com>; Sami
> Mujawar <sami.mujawar@arm.com>; Laszlo Ersek <lersek@redhat.com>;
> Leif Lindholm <leif@nuviainc.com>
> 主题: [edk2-devel] [PATCH v2 0/3] MdeModulePkg: use pool allocations for
> ACPI tables
> 
> Currently, the AcpiTableDxe memory allocator uses page based allocations,
> for which the only reason seems to be that it permits the use of a memory
> limit, which is necessary for ACPI 1.0 tables that need to reside in the
> first 4 GB of memory.
> 
> That requirement does not exist on AArch64, and since page based
allocations
> are rounded up to 64 KB multiples, this wastes some memory in a way that
> can easily be avoided. So let's use the existing 'mAcpiTableAllocType'
> policy variable, and switch to pool allocations if it is set to
> 'AllocateAnyPages'
> 
> Changes since v1:
> - incorporate Laszlo's review feedback
>   . update description of EFI_ACPI_TABLE_LIST struct
>   . avoid EFI_PHYSICAL_ADDRESS to VOID* conversion in case
> AllocatePages()
>     returns an error
>   . put closing ) on a line by itself when using the non-condensed format
>     for passing function call arguments.
>   . fix error in comment text
> 
> Example output from Linux booting on ArmVirtQemu:
> 
> Before:
>  ACPI: RSDP 0x0000000078510000 000024 (v02 BOCHS )
>  ACPI: XSDT 0x0000000078500000 00004C (v01 BOCHS  BXPCFACP
> 00000001      01000013)
>  ACPI: FACP 0x00000000784C0000 00010C (v05 BOCHS  BXPCFACP
> 00000001 BXPC 00000001)
>  ACPI: DSDT 0x00000000784D0000 0014BB (v02 BOCHS  BXPCDSDT
> 00000001 BXPC 00000001)
>  ACPI: APIC 0x00000000784B0000 0000A8 (v03 BOCHS  BXPCAPIC
> 00000001 BXPC 00000001)
>  ACPI: GTDT 0x00000000784A0000 000060 (v02 BOCHS  BXPCGTDT
> 00000001 BXPC 00000001)
>  ACPI: MCFG 0x0000000078490000 00003C (v01 BOCHS  BXPCMCFG
> 00000001 BXPC 00000001)
>  ACPI: SPCR 0x0000000078480000 000050 (v02 BOCHS  BXPCSPCR
> 00000001 BXPC 00000001)
> 
> After:
>  ACPI: RSDP 0x000000007C030018 000024 (v02 BOCHS )
>  ACPI: XSDT 0x000000007C03FE98 00004C (v01 BOCHS  BXPCFACP
> 00000001      01000013)
>  ACPI: FACP 0x000000007C03FA98 00010C (v05 BOCHS  BXPCFACP
> 00000001 BXPC 00000001)
>  ACPI: DSDT 0x000000007C037518 0014BB (v02 BOCHS  BXPCDSDT
> 00000001 BXPC 00000001)
>  ACPI: APIC 0x000000007C03FC18 0000A8 (v03 BOCHS  BXPCAPIC
> 00000001 BXPC 00000001)
>  ACPI: GTDT 0x000000007C03FD18 000060 (v02 BOCHS  BXPCGTDT
> 00000001 BXPC 00000001)
>  ACPI: MCFG 0x000000007C03FE18 00003C (v01 BOCHS  BXPCMCFG
> 00000001 BXPC 00000001)
>  ACPI: SPCR 0x000000007C03FF98 000050 (v02 BOCHS  BXPCSPCR
> 00000001 BXPC 00000001)
> 
> Cc: Dandan Bi <dandan.bi@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Sami Mujawar <sami.mujawar@arm.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Leif Lindholm <leif@nuviainc.com>
> 
> Ard Biesheuvel (3):
>   MdeModulePkg/AcpiTableDxe: use pool allocations when possible
>   MdeModulePkg/AcpiTableDxe: use pool allocation for RSDT/XSDT if
>     possible
>   MdeModulePkg/AcpiTableDxe: use pool allocation for RSDP if possible
> 
>  .../Universal/Acpi/AcpiTableDxe/AcpiTable.h   |  11 +-
>  .../Universal/Acpi/AcpiTableDxe/AcpiSdt.c     |   4 +-
>  .../Acpi/AcpiTableDxe/AcpiTableProtocol.c     | 230 ++++++++++++------
>  3 files changed, 162 insertions(+), 83 deletions(-)
> 
> --
> 2.17.1
> 
> 
> 
> 
> 

Re: 回复: [edk2-devel] [PATCH v2 0/3] MdeModulePkg: use pool allocations for ACPI tables

[Ard Biesheuvel]

On 10/30/20 2:01 AM, gaoliming wrote:
> Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
> 

Merged #1062 into master.

Thanks all,

>> -----邮件原件-----
>> 发件人: bounce+27952+66710+4905953+8761045@groups.io
>> <bounce+27952+66710+4905953+8761045@groups.io> 代表 Ard
>> Biesheuvel
>> 发送时间: 2020年10月29日 3:42
>> 收件人: devel@edk2.groups.io
>> 抄送: Ard Biesheuvel <ard.biesheuvel@arm.com>; Dandan Bi
>> <dandan.bi@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Jian J
>> Wang <jian.j.wang@intel.com>; Hao A Wu <hao.a.wu@intel.com>; Sami
>> Mujawar <sami.mujawar@arm.com>; Laszlo Ersek <lersek@redhat.com>;
>> Leif Lindholm <leif@nuviainc.com>
>> 主题: [edk2-devel] [PATCH v2 0/3] MdeModulePkg: use pool allocations for
>> ACPI tables
>>
>> Currently, the AcpiTableDxe memory allocator uses page based allocations,
>> for which the only reason seems to be that it permits the use of a memory
>> limit, which is necessary for ACPI 1.0 tables that need to reside in the
>> first 4 GB of memory.
>>
>> That requirement does not exist on AArch64, and since page based
> allocations
>> are rounded up to 64 KB multiples, this wastes some memory in a way that
>> can easily be avoided. So let's use the existing 'mAcpiTableAllocType'
>> policy variable, and switch to pool allocations if it is set to
>> 'AllocateAnyPages'
>>
>> Changes since v1:
>> - incorporate Laszlo's review feedback
>>    . update description of EFI_ACPI_TABLE_LIST struct
>>    . avoid EFI_PHYSICAL_ADDRESS to VOID* conversion in case
>> AllocatePages()
>>      returns an error
>>    . put closing ) on a line by itself when using the non-condensed format
>>      for passing function call arguments.
>>    . fix error in comment text
>>
>> Example output from Linux booting on ArmVirtQemu:
>>
>> Before:
>>   ACPI: RSDP 0x0000000078510000 000024 (v02 BOCHS )
>>   ACPI: XSDT 0x0000000078500000 00004C (v01 BOCHS  BXPCFACP
>> 00000001      01000013)
>>   ACPI: FACP 0x00000000784C0000 00010C (v05 BOCHS  BXPCFACP
>> 00000001 BXPC 00000001)
>>   ACPI: DSDT 0x00000000784D0000 0014BB (v02 BOCHS  BXPCDSDT
>> 00000001 BXPC 00000001)
>>   ACPI: APIC 0x00000000784B0000 0000A8 (v03 BOCHS  BXPCAPIC
>> 00000001 BXPC 00000001)
>>   ACPI: GTDT 0x00000000784A0000 000060 (v02 BOCHS  BXPCGTDT
>> 00000001 BXPC 00000001)
>>   ACPI: MCFG 0x0000000078490000 00003C (v01 BOCHS  BXPCMCFG
>> 00000001 BXPC 00000001)
>>   ACPI: SPCR 0x0000000078480000 000050 (v02 BOCHS  BXPCSPCR
>> 00000001 BXPC 00000001)
>>
>> After:
>>   ACPI: RSDP 0x000000007C030018 000024 (v02 BOCHS )
>>   ACPI: XSDT 0x000000007C03FE98 00004C (v01 BOCHS  BXPCFACP
>> 00000001      01000013)
>>   ACPI: FACP 0x000000007C03FA98 00010C (v05 BOCHS  BXPCFACP
>> 00000001 BXPC 00000001)
>>   ACPI: DSDT 0x000000007C037518 0014BB (v02 BOCHS  BXPCDSDT
>> 00000001 BXPC 00000001)
>>   ACPI: APIC 0x000000007C03FC18 0000A8 (v03 BOCHS  BXPCAPIC
>> 00000001 BXPC 00000001)
>>   ACPI: GTDT 0x000000007C03FD18 000060 (v02 BOCHS  BXPCGTDT
>> 00000001 BXPC 00000001)
>>   ACPI: MCFG 0x000000007C03FE18 00003C (v01 BOCHS  BXPCMCFG
>> 00000001 BXPC 00000001)
>>   ACPI: SPCR 0x000000007C03FF98 000050 (v02 BOCHS  BXPCSPCR
>> 00000001 BXPC 00000001)
>>
>> Cc: Dandan Bi <dandan.bi@intel.com>
>> Cc: Liming Gao <gaoliming@byosoft.com.cn>
>> Cc: Jian J Wang <jian.j.wang@intel.com>
>> Cc: Hao A Wu <hao.a.wu@intel.com>
>> Cc: Sami Mujawar <sami.mujawar@arm.com>
>> Cc: Laszlo Ersek <lersek@redhat.com>
>> Cc: Leif Lindholm <leif@nuviainc.com>
>>
>> Ard Biesheuvel (3):
>>    MdeModulePkg/AcpiTableDxe: use pool allocations when possible
>>    MdeModulePkg/AcpiTableDxe: use pool allocation for RSDT/XSDT if
>>      possible
>>    MdeModulePkg/AcpiTableDxe: use pool allocation for RSDP if possible
>>
>>   .../Universal/Acpi/AcpiTableDxe/AcpiTable.h   |  11 +-
>>   .../Universal/Acpi/AcpiTableDxe/AcpiSdt.c     |   4 +-
>>   .../Acpi/AcpiTableDxe/AcpiTableProtocol.c     | 230 ++++++++++++------
>>   3 files changed, 162 insertions(+), 83 deletions(-)
>>
>> --
>> 2.17.1
>>
>>
>>
>> 
>>
> 
> 
> 

Re: 回复: [edk2-devel] [PATCH v2 0/3] MdeModulePkg: use pool allocations for ACPI tables

[Laszlo Ersek]

On 10/30/20 15:51, Ard Biesheuvel wrote:
> On 10/30/20 2:01 AM, gaoliming wrote:
>> Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
>>
> 
> Merged #1062 into master.

Sorry for not reacting to this v2 series, I've been away. (I hope that
my automated out-of-office response worked and let you know.) I'm glad
that the patches are now merged.

Thanks
Laszlo