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

["Gao, Liming" <liming.gao@intel.com>]

Reviewed-by: Liming Gao <liming.gao@intel.com>

> -----Original Message-----
> From: Bi, Dandan
> Sent: Thursday, February 14, 2019 10:43 AM
> To: Bi, Dandan <dandan.bi@intel.com>; edk2-devel@lists.01.org
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Michael Turner <Michael.Turner@microsoft.com>; Max Knutsen <maknutse@microsoft.com>; Gao,
> Liming <liming.gao@intel.com>
> Subject: RE: [edk2] [patch V2] MdeModulePkg/ReportStatusCodeLib: Avoid using AllocatePool if possible
> 
> This patch is also available at
>  https://github.com/dandanbi/edk2/commit/e5c432b2d5083c0bbadbf773d460a9c22d36b267
> 
> 
> Thanks,
> Dandan
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> > Dandan Bi
> > Sent: Thursday, February 14, 2019 10:39 AM
> > To: edk2-devel@lists.01.org
> > Cc: Wu, Hao A <hao.a.wu@intel.com>; Michael Turner
> > <Michael.Turner@microsoft.com>; Max Knutsen
> > <maknutse@microsoft.com>; Gao, Liming <liming.gao@intel.com>
> > Subject: [edk2] [patch V2] MdeModulePkg/ReportStatusCodeLib: Avoid
> > using AllocatePool if possible
> >
> > From: Max Knutsen <maknutse@microsoft.com>
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1114
> >
> > V2:  simplify the code logic.
> > update
> > if (!mHaveExitedBootServices &&
> >   (StatusCodeData != (EFI_STATUS_CODE_DATA *) StatusCodeBuffer)) {
> >   gBS->FreePool (StatusCodeData);
> > }
> > to
> > if (StatusCodeData != (EFI_STATUS_CODE_DATA *) StatusCodeBuffer) {
> >   gBS->FreePool (StatusCodeData);
> > }
> >
> > 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/ReportStatusC
> > odeLib.c
> > b/MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/ReportStatusC
> > odeLib.c
> > index b73103517a..75e2f88ad2 100644
> > ---
> > a/MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/ReportStatusC
> > odeLib.c
> > +++
> > b/MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/ReportStatusC
> > od
> > +++ eLib.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 (StatusCodeData != (EFI_STATUS_CODE_DATA *) StatusCodeBuffer) {
> >      gBS->FreePool (StatusCodeData);
> >    }
> >
> >    return Status;
> >  }
> > --
> > 2.18.0.windows.1
> >
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel