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 | 111 ++++++++++++--------
 1 file changed, 65 insertions(+), 46 deletions(-)

diff --git a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
index b05e57e6584f..22f49a8489e7 100644
--- a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
+++ b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
@@ -355,28 +355,35 @@ 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
+                    );
+    Pointer = (UINT8 *)(UINTN)PageAddress;
+  } else {
+    Status = gBS->AllocatePool (
+                    EfiACPIReclaimMemory,
+                    TotalSize,
+                    (VOID **)&Pointer);
+  }
 
   if (EFI_ERROR (Status)) {
     return EFI_OUT_OF_RESOURCES;
   }
 
-  Pointer = (UINT8 *) (UINTN) PageAddress;
   ZeroMem (Pointer, TotalSize);
 
   AcpiTableInstance->Rsdt1 = (EFI_ACPI_DESCRIPTION_HEADER *) Pointer;
@@ -406,21 +413,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
   //
@@ -1759,29 +1771,36 @@ 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
+                    );
+    Pointer = (UINT8 *)(UINTN)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;
   ZeroMem (Pointer, TotalSize);
 
   AcpiTableInstance->Rsdt1 = (EFI_ACPI_DESCRIPTION_HEADER *) Pointer;
-- 
2.17.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#66325): https://edk2.groups.io/g/devel/message/66325
Mute This Topic: https://groups.io/mt/77553424/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On 10/16/20 17:49, Ard Biesheuvel wrote:
> 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 | 111 ++++++++++++--------
>  1 file changed, 65 insertions(+), 46 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
> index b05e57e6584f..22f49a8489e7 100644
> --- a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
> +++ b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
> @@ -355,28 +355,35 @@ 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.

Hmmm. I first thought that the last two lines of the commend should not
be re-instated. But I was wrong. Because supporting ACPI 1.0b causes us
to allocate even areas pointed-to by XSDT entries under 4GB.

> +    //
> +    PageAddress = 0xFFFFFFFF;
> +    Status = gBS->AllocatePages (
> +                    mAcpiTableAllocType,
> +                    EfiACPIReclaimMemory,
> +                    EFI_SIZE_TO_PAGES (TotalSize),
> +                    &PageAddress
> +                    );
> +    Pointer = (UINT8 *)(UINTN)PageAddress;

(1) Same comment as before; we shouldn't convert PageAddress to a
pointer unless the allocation succeeds.

> +  } else {
> +    Status = gBS->AllocatePool (
> +                    EfiACPIReclaimMemory,
> +                    TotalSize,
> +                    (VOID **)&Pointer);
> +  }
>  
>    if (EFI_ERROR (Status)) {
>      return EFI_OUT_OF_RESOURCES;
>    }
>  
> -  Pointer = (UINT8 *) (UINTN) PageAddress;
>    ZeroMem (Pointer, TotalSize);
>  
>    AcpiTableInstance->Rsdt1 = (EFI_ACPI_DESCRIPTION_HEADER *) Pointer;
> @@ -406,21 +413,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
>    //
> @@ -1759,29 +1771,36 @@ 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
> +                    );
> +    Pointer = (UINT8 *)(UINTN)PageAddress;

(2) Same as (1).

Looks reasonable otherwise.

Thanks
Laszlo


> +  } 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;
>    ZeroMem (Pointer, TotalSize);
>  
>    AcpiTableInstance->Rsdt1 = (EFI_ACPI_DESCRIPTION_HEADER *) Pointer;
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#66401): https://edk2.groups.io/g/devel/message/66401
Mute This Topic: https://groups.io/mt/77553424/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On 10/19/20 22:06, Laszlo Ersek wrote:
> On 10/16/20 17:49, Ard Biesheuvel wrote:
>> 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 | 111 ++++++++++++--------
>>  1 file changed, 65 insertions(+), 46 deletions(-)
>>
>> diff --git a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
>> index b05e57e6584f..22f49a8489e7 100644
>> --- a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
>> +++ b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
>> @@ -355,28 +355,35 @@ 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.
> 
> Hmmm. I first thought that the last two lines of the commend should not
> be re-instated. But I was wrong. Because supporting ACPI 1.0b causes us
> to allocate even areas pointed-to by XSDT entries under 4GB.
> 
>> +    //
>> +    PageAddress = 0xFFFFFFFF;
>> +    Status = gBS->AllocatePages (
>> +                    mAcpiTableAllocType,
>> +                    EfiACPIReclaimMemory,
>> +                    EFI_SIZE_TO_PAGES (TotalSize),
>> +                    &PageAddress
>> +                    );
>> +    Pointer = (UINT8 *)(UINTN)PageAddress;
> 
> (1) Same comment as before; we shouldn't convert PageAddress to a
> pointer unless the allocation succeeds.
> 
>> +  } else {
>> +    Status = gBS->AllocatePool (
>> +                    EfiACPIReclaimMemory,
>> +                    TotalSize,
>> +                    (VOID **)&Pointer);
>> +  }
>>  
>>    if (EFI_ERROR (Status)) {
>>      return EFI_OUT_OF_RESOURCES;
>>    }
>>  
>> -  Pointer = (UINT8 *) (UINTN) PageAddress;
>>    ZeroMem (Pointer, TotalSize);
>>  
>>    AcpiTableInstance->Rsdt1 = (EFI_ACPI_DESCRIPTION_HEADER *) Pointer;
>> @@ -406,21 +413,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
>>    //
>> @@ -1759,29 +1771,36 @@ 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
>> +                    );
>> +    Pointer = (UINT8 *)(UINTN)PageAddress;
> 
> (2) Same as (1).
> 
> Looks reasonable otherwise.

(3) Style: for both gBS->AllocatePool() calls added in this patch,
please either break the closing paren to a new line, or use the
"condensed" style (which you do use elsewhere in these patches).

Thanks!
Laszlo


> 
>> +  } 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;
>>    ZeroMem (Pointer, TotalSize);
>>  
>>    AcpiTableInstance->Rsdt1 = (EFI_ACPI_DESCRIPTION_HEADER *) Pointer;
>>
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#66402): https://edk2.groups.io/g/devel/message/66402
Mute This Topic: https://groups.io/mt/77553424/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-