Re: [edk2-devel] [PATCH v2 1/1] MdeModulePkg/AcpiTableDxe: PCD switch to avoid using ACPI reclaim memory

["Aaron Li" <aaron.li@intel.com>]

Hi Liming,

Could you please help review and merge it? Thanks.

Best,
Aaron

-----Original Message-----
From: Liu, Zhiguang <zhiguang.liu@intel.com> 
Sent: Monday, April 1, 2024 10:06 AM
To: Li, Aaron <aaron.li@intel.com>; devel@edk2.groups.io
Cc: Bi, Dandan <dandan.bi@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Liu, Yun Y <yun.y.liu@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Ni, Ray <ray.ni@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: RE: [PATCH v2 1/1] MdeModulePkg/AcpiTableDxe: PCD switch to avoid using ACPI reclaim memory

Reviewed-by: Zhiguang Liu <zhiguang.liu@intel.com>

> -----Original Message-----
> From: Li, Aaron <aaron.li@intel.com>
> Sent: Tuesday, March 26, 2024 3:58 PM
> To: devel@edk2.groups.io
> Cc: Liu, Zhiguang <zhiguang.liu@intel.com>; Bi, Dandan
> <dandan.bi@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Liu, Yun Y
> <yun.y.liu@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Ni, Ray
> <ray.ni@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: [PATCH v2 1/1] MdeModulePkg/AcpiTableDxe: PCD switch to avoid
> using ACPI reclaim memory
> 
> UEFI spec defined ACPI Tables at boot time can be contained in memory of
> type EfiACPIReclaimMemory or EfiAcpiMemoryNVS, although InstallAcpiTable
> with AcpiTableProtocol will only allocate memory with type
> EfiACPIReclaimMemory (Except FACS).
> 
> This patch provides an optional method controlled by PCD to avoid using
> EfiACPIReclaimMemory, by setting the PCD PcdNoACPIReclaimMemory to
> TRUE, all ACPI allocated memory will use EfiAcpiMemoryNVS instead.
> 
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> Cc: Dandan Bi <dandan.bi@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Liu Yun <yun.y.liu@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Signed-off-by: Aaron Li <aaron.li@intel.com>
> ---
>  MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c | 37
> +++++++++++++++-----
>  MdeModulePkg/MdeModulePkg.dec                                |  7 ++++
>  MdeModulePkg/MdeModulePkg.uni                                |  8 +++++
>  MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf    |  1 +
>  4 files changed, 45 insertions(+), 8 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
> b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
> index e09bc9b704f5..080768033cfa 100644
> --- a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
> +++ b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
> @@ -340,6 +340,7 @@ ReallocateAcpiTableBuffer (
>    EFI_ACPI_TABLE_INSTANCE  TempPrivateData;
>    EFI_STATUS               Status;
>    UINT64                   CurrentData;
> +  EFI_MEMORY_TYPE          AcpiAllocateMemoryType;
> 
>    CopyMem (&TempPrivateData, AcpiTableInstance, sizeof
> (EFI_ACPI_TABLE_INSTANCE));
>    //
> @@ -359,6 +360,12 @@ ReallocateAcpiTableBuffer (
>                   NewMaxTableNumber * sizeof (UINT32);
>    }
> 
> +  if (PcdGetBool (PcdNoACPIReclaimMemory)) {
> +    AcpiAllocateMemoryType = EfiACPIMemoryNVS;  } else {
> +    AcpiAllocateMemoryType = EfiACPIReclaimMemory;  }
> +
>    if (mAcpiTableAllocType != AllocateAnyPages) {
>      //
>      // Allocate memory in the lower 32 bit of address range for @@ -372,13
> +379,13 @@ ReallocateAcpiTableBuffer (
>      PageAddress = 0xFFFFFFFF;
>      Status      = gBS->AllocatePages (
>                           mAcpiTableAllocType,
> -                         EfiACPIReclaimMemory,
> +                         AcpiAllocateMemoryType,
>                           EFI_SIZE_TO_PAGES (TotalSize),
>                           &PageAddress
>                           );
>    } else {
>      Status = gBS->AllocatePool (
> -                    EfiACPIReclaimMemory,
> +                    AcpiAllocateMemoryType,
>                      TotalSize,
>                      (VOID **)&Pointer
>                      );
> @@ -512,6 +519,7 @@ AddTableToList (
>    EFI_PHYSICAL_ADDRESS  AllocPhysAddress;
>    UINT64                Buffer64;
>    BOOLEAN               AddToRsdt;
> +  EFI_MEMORY_TYPE       AcpiAllocateMemoryType;
> 
>    //
>    // Check for invalid input parameters @@ -550,6 +558,12 @@
> AddTableToList (
>    CurrentTableList->TableSize      = CurrentTableSize;
>    CurrentTableList->PoolAllocation = FALSE;
> 
> +  if (PcdGetBool (PcdNoACPIReclaimMemory)) {
> +    AcpiAllocateMemoryType = EfiACPIMemoryNVS;  } else {
> +    AcpiAllocateMemoryType = EfiACPIReclaimMemory;  }
> +
>    //
>    // Allocation memory type depends on the type of the table
>    //
> @@ -585,7 +599,7 @@ AddTableToList (
>      // such as AArch64 that allocate multiples of 64 KB
>      //
>      Status = gBS->AllocatePool (
> -                    EfiACPIReclaimMemory,
> +                    AcpiAllocateMemoryType,
>                      CurrentTableList->TableSize,
>                      (VOID **)&CurrentTableList->Table
>                      );
> @@ -596,7 +610,7 @@ AddTableToList (
>      //
>      Status = gBS->AllocatePages (
>                      mAcpiTableAllocType,
> -                    EfiACPIReclaimMemory,
> +                    AcpiAllocateMemoryType,
>                      EFI_SIZE_TO_PAGES (CurrentTableList->TableSize),
>                      &AllocPhysAddress
>                      );
> @@ -1944,6 +1958,7 @@ AcpiTableAcpiTableConstructor (
>    UINTN                 RsdpTableSize;
>    UINT8                 *Pointer;
>    EFI_PHYSICAL_ADDRESS  PageAddress;
> +  EFI_MEMORY_TYPE       AcpiAllocateMemoryType;
> 
>    //
>    // Check for invalid input parameters @@ -1978,17 +1993,23 @@
> AcpiTableAcpiTableConstructor (
>      RsdpTableSize += sizeof
> (EFI_ACPI_1_0_ROOT_SYSTEM_DESCRIPTION_POINTER);
>    }
> 
> +  if (PcdGetBool (PcdNoACPIReclaimMemory)) {
> +    AcpiAllocateMemoryType = EfiACPIMemoryNVS;  } else {
> +    AcpiAllocateMemoryType = EfiACPIReclaimMemory;  }
> +
>    if (mAcpiTableAllocType != AllocateAnyPages) {
>      PageAddress = 0xFFFFFFFF;
>      Status      = gBS->AllocatePages (
>                           mAcpiTableAllocType,
> -                         EfiACPIReclaimMemory,
> +                         AcpiAllocateMemoryType,
>                           EFI_SIZE_TO_PAGES (RsdpTableSize),
>                           &PageAddress
>                           );
>    } else {
>      Status = gBS->AllocatePool (
> -                    EfiACPIReclaimMemory,
> +                    AcpiAllocateMemoryType,
>                      RsdpTableSize,
>                      (VOID **)&Pointer
>                      );
> @@ -2037,13 +2058,13 @@ AcpiTableAcpiTableConstructor (
>      PageAddress = 0xFFFFFFFF;
>      Status      = gBS->AllocatePages (
>                           mAcpiTableAllocType,
> -                         EfiACPIReclaimMemory,
> +                         AcpiAllocateMemoryType,
>                           EFI_SIZE_TO_PAGES (TotalSize),
>                           &PageAddress
>                           );
>    } else {
>      Status = gBS->AllocatePool (
> -                    EfiACPIReclaimMemory,
> +                    AcpiAllocateMemoryType,
>                      TotalSize,
>                      (VOID **)&Pointer
>                      );
> diff --git a/MdeModulePkg/MdeModulePkg.dec
> b/MdeModulePkg/MdeModulePkg.dec index a82dedc070df..a91058e5b5df
> 100644
> --- a/MdeModulePkg/MdeModulePkg.dec
> +++ b/MdeModulePkg/MdeModulePkg.dec
> @@ -1533,6 +1533,13 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
>    # @Prompt Exposed ACPI table versions.
> 
> gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiExposedTableVersions|0x3E|UIN
> T32|0x0001004c
> 
> +  ## Indicates whether ACPI Reclaim memory is not available  # Default
> + is FALSE that means ACPI Reclaim memory is available  # If it is set
> + to TRUE that means ACPI Reclaim memory is not available  # For example
> + ACPI Table protocol will use ACPI NVS memory instead of ACPI Reclaim
> + memory  # @Prompt ACPI Reclaim memory is not available.
> +
> +
> gEfiMdeModulePkgTokenSpaceGuid.PcdNoACPIReclaimMemory|FALSE|BOOL
> EAN|0x
> + 0001008b
> +
>    ## This PCD defines the MAX repair count.
>    #  The default value is 0 that means infinite.
>    # @Prompt MAX repair count
> diff --git a/MdeModulePkg/MdeModulePkg.uni
> b/MdeModulePkg/MdeModulePkg.uni index
> a17d34d60b21..6079285e3f8b 100644
> --- a/MdeModulePkg/MdeModulePkg.uni
> +++ b/MdeModulePkg/MdeModulePkg.uni
> @@ -955,6 +955,14 @@
>                                                                                               "BIT 4 -
> EFI_ACPI_TABLE_VERSION_4_0.<BR>\n"
>                                                                                               "BIT 5 -
> EFI_ACPI_TABLE_VERSION_5_0.<BR>"
> 
> +#string
> STR_gEfiMdeModulePkgTokenSpaceGuid_PcdNoACPIReclaimMemory_PROM
> PT  #language en-US "ACPI Reclaim memory is not available."
> +
> +#string
> STR_gEfiMdeModulePkgTokenSpaceGuid_PcdNoACPIReclaimMemory_HELP
> #language en-US "Indicates whether ACPI Reclaim memory is not available\n"
> +                                                                                     "Default is FALSE that means ACPI
> Reclaim memory is available\n"
> +                                                                                     "If it is set to TRUE that means
> ACPI Reclaim memory is not available\n"
> +                                                                                     "For example ACPI Table protocol
> will use ACPI NVS memory instead of ACPI Reclaim memory"
> +
> +
>  #string
> STR_gEfiMdeModulePkgTokenSpaceGuid_PcdHiiOsRuntimeSupport_PROMPT
> #language en-US "Enable export HII data and configuration to be used in OS
> runtime."
> 
>  #string
> STR_gEfiMdeModulePkgTokenSpaceGuid_PcdHiiOsRuntimeSupport_HELP
> #language en-US "Indicates if HII data and configuration has been
> exported.<BR><BR>\n"
> diff --git a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf
> b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf
> index 86dea43e27e4..be498a56cff0 100644
> --- a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf
> +++ b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf
> @@ -68,6 +68,7 @@ [Pcd]
>    gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorId        ##
> CONSUMES
>    gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorRevision  ##
> CONSUMES
>    gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiExposedTableVersions    ##
> CONSUMES
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdNoACPIReclaimMemory         ##
> CONSUMES
> 
>  [Protocols]
>    gEfiAcpiTableProtocolGuid                     ## PRODUCES
> --
> 2.44.0.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117274): https://edk2.groups.io/g/devel/message/117274
Mute This Topic: https://groups.io/mt/105154551/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-