[patch] MdeModulePkg/ReportStatusCodeLib: Avoid using AllocatePool if possible

[patch] MdeModulePkg/ReportStatusCodeLib: Avoid using AllocatePool if possible

[Dandan Bi]

From: Max Knutsen <maknutse@microsoft.com>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1114

When report status code with ExtendedData data,
and the extended data can fit in the local static buffer,
there is no need to use AllocatePool to hold the ExtendedData data.

This patch is just to do the enhancement to avoid using AllocatePool.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao Wu <hao.a.wu@intel.com>
Cc: Michael Turner <Michael.Turner@microsoft.com>
Cc: Liming Gao <liming.gao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi <dandan.bi@intel.com>
---
 .../Library/DxeReportStatusCodeLib/ReportStatusCodeLib.c | 9 +++++++--
 .../RuntimeDxeReportStatusCodeLib/ReportStatusCodeLib.c  | 9 +++++++--
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/MdeModulePkg/Library/DxeReportStatusCodeLib/ReportStatusCodeLib.c b/MdeModulePkg/Library/DxeReportStatusCodeLib/ReportStatusCodeLib.c
index b28dc5c3bb..c88be5a1a3 100644
--- a/MdeModulePkg/Library/DxeReportStatusCodeLib/ReportStatusCodeLib.c
+++ b/MdeModulePkg/Library/DxeReportStatusCodeLib/ReportStatusCodeLib.c
@@ -505,13 +505,18 @@ ReportStatusCodeEx (
   //
   Tpl = gBS->RaiseTPL (TPL_HIGH_LEVEL);
   gBS->RestoreTPL (Tpl);
 
   StatusCodeData = NULL;
-  if (Tpl <= TPL_NOTIFY) {
+  if (ExtendedDataSize <= (MAX_EXTENDED_DATA_SIZE - sizeof (EFI_STATUS_CODE_DATA))) {
     //
-    // Allocate space for the Status Code Header and its buffer
+    // Use Buffer instead of allocating if possible.
+    //
+    StatusCodeData = (EFI_STATUS_CODE_DATA *)Buffer;
+  } else if (Tpl <= TPL_NOTIFY){
+    //
+    // If Buffer is not big enough, allocate space for the Status Code Header and its buffer
     //
     gBS->AllocatePool (EfiBootServicesData, sizeof (EFI_STATUS_CODE_DATA) + ExtendedDataSize, (VOID **)&StatusCodeData);
   }
 
   if (StatusCodeData == NULL) {
diff --git a/MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/ReportStatusCodeLib.c b/MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/ReportStatusCodeLib.c
index b73103517a..3380469759 100644
--- a/MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/ReportStatusCodeLib.c
+++ b/MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/ReportStatusCodeLib.c
@@ -635,11 +635,14 @@ ReportStatusCodeEx (
   if (mHaveExitedBootServices) {
     if (sizeof (EFI_STATUS_CODE_DATA) + ExtendedDataSize > MAX_EXTENDED_DATA_SIZE) {
       return EFI_OUT_OF_RESOURCES;
     }
     StatusCodeData = (EFI_STATUS_CODE_DATA *) StatusCodeBuffer;
-  } else  {
+  } else if (sizeof (EFI_STATUS_CODE_DATA) + ExtendedDataSize > MAX_EXTENDED_DATA_SIZE) {
+    //
+    // Only use AllocatePool for larger ExtendedData.
+    //
     if (gBS == NULL || gBS->AllocatePool == NULL || gBS->FreePool == NULL) {
       return EFI_UNSUPPORTED;
     }
 
     //
@@ -648,10 +651,12 @@ ReportStatusCodeEx (
     StatusCodeData = NULL;
     gBS->AllocatePool (EfiBootServicesData, sizeof (EFI_STATUS_CODE_DATA) + ExtendedDataSize, (VOID **)&StatusCodeData);
     if (StatusCodeData == NULL) {
       return EFI_OUT_OF_RESOURCES;
     }
+  } else {
+    StatusCodeData = (EFI_STATUS_CODE_DATA *) StatusCodeBuffer;
   }
 
   //
   // Fill in the extended data header
   //
@@ -678,11 +683,11 @@ ReportStatusCodeEx (
   Status = InternalReportStatusCode (Type, Value, Instance, CallerId, StatusCodeData);
 
   //
   // Free the allocated buffer
   //
-  if (!mHaveExitedBootServices) {
+  if (!mHaveExitedBootServices && (StatusCodeData != (EFI_STATUS_CODE_DATA *) StatusCodeBuffer)) {
     gBS->FreePool (StatusCodeData);
   }
 
   return Status;
 }
-- 
2.18.0.windows.1

Re: [patch] MdeModulePkg/ReportStatusCodeLib: Avoid using AllocatePool if possible

[Gao, Liming]

Dandan:
 The last change in RuntimeDxeReportStatusCodeLib can be simplified. Other change is good. 

  if (!mHaveExitedBootServices && (StatusCodeData != (EFI_STATUS_CODE_DATA *) StatusCodeBuffer)) {
    gBS->FreePool (StatusCodeData);
  }
==>
  if (StatusCodeData != (EFI_STATUS_CODE_DATA *) StatusCodeBuffer) {
    gBS->FreePool (StatusCodeData);
  }

Thanks
Liming
> -----Original Message-----
> From: Bi, Dandan
> Sent: Saturday, February 2, 2019 4:47 PM
> To: edk2-devel@lists.01.org
> Cc: Max Knutsen <maknutse@microsoft.com>; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Michael Turner
> <Michael.Turner@microsoft.com>; Gao, Liming <liming.gao@intel.com>
> Subject: [patch] MdeModulePkg/ReportStatusCodeLib: Avoid using AllocatePool if possible
> 
> From: Max Knutsen <maknutse@microsoft.com>
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1114
> 
> When report status code with ExtendedData data,
> and the extended data can fit in the local static buffer,
> there is no need to use AllocatePool to hold the ExtendedData data.
> 
> This patch is just to do the enhancement to avoid using AllocatePool.
> 
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao Wu <hao.a.wu@intel.com>
> Cc: Michael Turner <Michael.Turner@microsoft.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Dandan Bi <dandan.bi@intel.com>
> ---
>  .../Library/DxeReportStatusCodeLib/ReportStatusCodeLib.c | 9 +++++++--
>  .../RuntimeDxeReportStatusCodeLib/ReportStatusCodeLib.c  | 9 +++++++--
>  2 files changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/MdeModulePkg/Library/DxeReportStatusCodeLib/ReportStatusCodeLib.c
> b/MdeModulePkg/Library/DxeReportStatusCodeLib/ReportStatusCodeLib.c
> index b28dc5c3bb..c88be5a1a3 100644
> --- a/MdeModulePkg/Library/DxeReportStatusCodeLib/ReportStatusCodeLib.c
> +++ b/MdeModulePkg/Library/DxeReportStatusCodeLib/ReportStatusCodeLib.c
> @@ -505,13 +505,18 @@ ReportStatusCodeEx (
>    //
>    Tpl = gBS->RaiseTPL (TPL_HIGH_LEVEL);
>    gBS->RestoreTPL (Tpl);
> 
>    StatusCodeData = NULL;
> -  if (Tpl <= TPL_NOTIFY) {
> +  if (ExtendedDataSize <= (MAX_EXTENDED_DATA_SIZE - sizeof (EFI_STATUS_CODE_DATA))) {
>      //
> -    // Allocate space for the Status Code Header and its buffer
> +    // Use Buffer instead of allocating if possible.
> +    //
> +    StatusCodeData = (EFI_STATUS_CODE_DATA *)Buffer;
> +  } else if (Tpl <= TPL_NOTIFY){
> +    //
> +    // If Buffer is not big enough, allocate space for the Status Code Header and its buffer
>      //
>      gBS->AllocatePool (EfiBootServicesData, sizeof (EFI_STATUS_CODE_DATA) + ExtendedDataSize, (VOID **)&StatusCodeData);
>    }
> 
>    if (StatusCodeData == NULL) {
> diff --git a/MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/ReportStatusCodeLib.c
> b/MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/ReportStatusCodeLib.c
> index b73103517a..3380469759 100644
> --- a/MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/ReportStatusCodeLib.c
> +++ b/MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/ReportStatusCodeLib.c
> @@ -635,11 +635,14 @@ ReportStatusCodeEx (
>    if (mHaveExitedBootServices) {
>      if (sizeof (EFI_STATUS_CODE_DATA) + ExtendedDataSize > MAX_EXTENDED_DATA_SIZE) {
>        return EFI_OUT_OF_RESOURCES;
>      }
>      StatusCodeData = (EFI_STATUS_CODE_DATA *) StatusCodeBuffer;
> -  } else  {
> +  } else if (sizeof (EFI_STATUS_CODE_DATA) + ExtendedDataSize > MAX_EXTENDED_DATA_SIZE) {
> +    //
> +    // Only use AllocatePool for larger ExtendedData.
> +    //
>      if (gBS == NULL || gBS->AllocatePool == NULL || gBS->FreePool == NULL) {
>        return EFI_UNSUPPORTED;
>      }
> 
>      //
> @@ -648,10 +651,12 @@ ReportStatusCodeEx (
>      StatusCodeData = NULL;
>      gBS->AllocatePool (EfiBootServicesData, sizeof (EFI_STATUS_CODE_DATA) + ExtendedDataSize, (VOID **)&StatusCodeData);
>      if (StatusCodeData == NULL) {
>        return EFI_OUT_OF_RESOURCES;
>      }
> +  } else {
> +    StatusCodeData = (EFI_STATUS_CODE_DATA *) StatusCodeBuffer;
>    }
> 
>    //
>    // Fill in the extended data header
>    //
> @@ -678,11 +683,11 @@ ReportStatusCodeEx (
>    Status = InternalReportStatusCode (Type, Value, Instance, CallerId, StatusCodeData);
> 
>    //
>    // Free the allocated buffer
>    //
> -  if (!mHaveExitedBootServices) {
> +  if (!mHaveExitedBootServices && (StatusCodeData != (EFI_STATUS_CODE_DATA *) StatusCodeBuffer)) {
>      gBS->FreePool (StatusCodeData);
>    }
> 
>    return Status;
>  }
> --
> 2.18.0.windows.1