public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Ard Biesheuvel" <ard.biesheuvel@arm.com>
To: devel@edk2.groups.io
Cc: 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>
Subject: [PATCH v2 1/3] MdeModulePkg/AcpiTableDxe: use pool allocations when possible
Date: Wed, 28 Oct 2020 20:42:24 +0100	[thread overview]
Message-ID: <20201028194226.21556-2-ard.biesheuvel@arm.com> (raw)
In-Reply-To: <20201028194226.21556-1-ard.biesheuvel@arm.com>

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


  reply	other threads:[~2020-10-28 19:42 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-28 19:42 [PATCH v2 0/3] MdeModulePkg: use pool allocations for ACPI tables Ard Biesheuvel
2020-10-28 19:42 ` Ard Biesheuvel [this message]
2020-10-28 19:42 ` [PATCH v2 2/3] MdeModulePkg/AcpiTableDxe: use pool allocation for RSDT/XSDT if possible Ard Biesheuvel
2020-10-28 19:42 ` [PATCH v2 3/3] MdeModulePkg/AcpiTableDxe: use pool allocation for RSDP " Ard Biesheuvel
2020-10-30  1:01 ` 回复: [edk2-devel] [PATCH v2 0/3] MdeModulePkg: use pool allocations for ACPI tables gaoliming
2020-10-30 14:51   ` Ard Biesheuvel
2020-11-02 17:27     ` Laszlo Ersek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201028194226.21556-2-ard.biesheuvel@arm.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox