Re: [patch] MdeModulePkg/HiiDB: Minimize memory allocation times after ReadyToBoot

["Dong, Eric" <eric.dong@intel.com>]

Reviewed-by: Eric Dong <eric.dong@intel.com>

> -----Original Message-----
> From: Bi, Dandan
> Sent: Thursday, April 25, 2019 11:37 AM
> To: devel@edk2.groups.io
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> <liming.gao@intel.com>; Dong, Eric <eric.dong@intel.com>
> Subject: [patch] MdeModulePkg/HiiDB: Minimize memory allocation times
> after ReadyToBoot
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1597
> 
> Currently RTData are allocated at/after ReadyToBoot to store the
> contents in HiiDatabase and the HII configurations for OS runtime
> utilization.
> Some platforms may meet S4 resume issue since the allocation after
> ReadyToBoot cause memory map change.
> Now this patch to do some overallocation to minimize the number
> of memory allocations after ReadyToBoot and also add warning
> message when do allocation after ReadyToBoot.
> 
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Signed-off-by: Dandan Bi <dandan.bi@intel.com>
> ---
>  .../Universal/HiiDatabaseDxe/Database.c       | 24 +++++++++++++------
>  1 file changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/Database.c
> b/MdeModulePkg/Universal/HiiDatabaseDxe/Database.c
> index 6da0e30c98..d3791ca68b 100644
> --- a/MdeModulePkg/Universal/HiiDatabaseDxe/Database.c
> +++ b/MdeModulePkg/Universal/HiiDatabaseDxe/Database.c
> @@ -1,9 +1,9 @@
>  /** @file
>  Implementation for EFI_HII_DATABASE_PROTOCOL.
> 
> -Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2007 - 2019, Intel Corporation. All rights reserved.<BR>
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> 
> 
> @@ -3361,18 +3361,23 @@ HiiGetConfigRespInfo(
>    Status = HiiConfigRoutingExportConfig(&Private-
> >ConfigRouting,&ConfigAltResp);
> 
>    if (!EFI_ERROR (Status)){
>      ConfigSize = StrSize(ConfigAltResp);
>      if (ConfigSize > gConfigRespSize){
> -      gConfigRespSize = ConfigSize;
> +      //
> +      // Do 25% overallocation to minimize the number of memory allocations
> after ReadyToBoot.
> +      // Since lots of allocation after ReadyToBoot may change memory map
> and cause S4 resume issue.
> +      //
> +      gConfigRespSize = ConfigSize + (ConfigSize >> 2);
>        if (gRTConfigRespBuffer != NULL){
>          FreePool(gRTConfigRespBuffer);
> +        DEBUG ((DEBUG_WARN, "[HiiDatabase]: Memory allocation is required
> after ReadyToBoot, which may change memory map and cause S4 resume
> issue.\n"));
>        }
> -      gRTConfigRespBuffer =
> (EFI_STRING)AllocateRuntimeZeroPool(ConfigSize);
> +      gRTConfigRespBuffer = (EFI_STRING) AllocateRuntimeZeroPool
> (gConfigRespSize);
>        if (gRTConfigRespBuffer == NULL){
>          FreePool(ConfigAltResp);
> -        DEBUG ((DEBUG_ERROR, "Not enough memory resource to get the
> ConfigResp string.\n"));
> +        DEBUG ((DEBUG_ERROR, "[HiiDatabase]: No enough memory resource
> to store the ConfigResp string.\n"));
>          return EFI_OUT_OF_RESOURCES;
>        }
>      } else {
>        ZeroMem(gRTConfigRespBuffer,gConfigRespSize);
>      }
> @@ -3412,17 +3417,22 @@ HiiGetDatabaseInfo(
>    Status = HiiExportPackageLists(This, NULL, &DatabaseInfoSize,
> DatabaseInfo);
> 
>    ASSERT(Status == EFI_BUFFER_TOO_SMALL);
> 
>    if(DatabaseInfoSize > gDatabaseInfoSize ) {
> -    gDatabaseInfoSize = DatabaseInfoSize;
> +    //
> +    // Do 25% overallocation to minimize the number of memory allocations
> after ReadyToBoot.
> +    // Since lots of allocation after ReadyToBoot may change memory map
> and cause S4 resume issue.
> +    //
> +    gDatabaseInfoSize = DatabaseInfoSize + (DatabaseInfoSize >> 2);
>      if (gRTDatabaseInfoBuffer != NULL){
>        FreePool(gRTDatabaseInfoBuffer);
> +      DEBUG ((DEBUG_WARN, "[HiiDatabase]: Memory allocation is required
> after ReadyToBoot, which may change memory map and cause S4 resume
> issue.\n"));
>      }
> -    gRTDatabaseInfoBuffer = AllocateRuntimeZeroPool(DatabaseInfoSize);
> +    gRTDatabaseInfoBuffer = AllocateRuntimeZeroPool (gDatabaseInfoSize);
>      if (gRTDatabaseInfoBuffer == NULL){
> -      DEBUG ((DEBUG_ERROR, "Not enough memory resource to get the
> HiiDatabase info.\n"));
> +      DEBUG ((DEBUG_ERROR, "[HiiDatabase]: No enough memory resource
> to store the HiiDatabase info.\n"));
>        return EFI_OUT_OF_RESOURCES;
>      }
>    } else {
>      ZeroMem(gRTDatabaseInfoBuffer,gDatabaseInfoSize);
>    }
> --
> 2.18.0.windows.1