Re: [edk2-devel] [PATCH v6] MdeModulePkg/Universal/StatusCodeHandler: Fix a bug about log lost

["Dandan Bi" <dandan.bi@intel.com>]

Here is the pull request https://github.com/tianocore/edk2/pull/1259



Thanks,
Dandan
> -----Original Message-----
> From: Bi, Dandan
> Sent: Tuesday, December 22, 2020 9:37 AM
> To: Tan, Ming <ming.tan@intel.com>; gaoliming@byosoft.com.cn
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> devel@edk2.groups.io
> Subject: RE: [edk2-devel] [PATCH v6]
> MdeModulePkg/Universal/StatusCodeHandler: Fix a bug about log lost
> 
> Reviewed-by: Dandan Bi <dandan.bi@intel.com> for V6, will create a pull
> request for it.
> 
> 
> Thanks,
> Dandan
> > -----Original Message-----
> > From: Tan, Ming <ming.tan@intel.com>
> > Sent: Tuesday, December 22, 2020 9:26 AM
> > To: gaoliming@byosoft.com.cn; Bi, Dandan <dandan.bi@intel.com>
> > Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A
> > <hao.a.wu@intel.com>; devel@edk2.groups.io
> > Subject: RE: [edk2-devel] [PATCH v6]
> > MdeModulePkg/Universal/StatusCodeHandler: Fix a bug about log lost
> >
> > Liming & Dandan:
> >   If there is not new concern, would you like to review and submit this patch?
> >
> >   Thank you.
> >   Tan Ming.
> >
> > -----Original Message-----
> > From: Tan, Ming
> > Sent: Tuesday, December 22, 2020 8:56 AM
> > To: gaoliming@byosoft.com.cn; Bi, Dandan <dandan.bi@intel.com>
> > Cc: devel@edk2.groups.io
> > Subject: RE: [edk2-devel] [PATCH v6]
> > MdeModulePkg/Universal/StatusCodeHandler: Fix a bug about log lost
> >
> > Liming:
> >   If remove the UnregisterSerialBootTimeHandlers(), then in
> > SerialStatusCodeWorker.c, need use an external variable
> > mRscHandlerProtocol which is defined in StatusCodeHandlerRuntimeDxe.c,
> > or call gBS->LocateProtocol to get it again.
> >   Compare with export add an external variable, add an external
> > function seems better.
> >
> >   BR/Tan Ming.
> >
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> > gaoliming
> > Sent: Tuesday, December 22, 2020 8:36 AM
> > To: devel@edk2.groups.io; Tan, Ming <ming.tan@intel.com>; Bi, Dandan
> > <dandan.bi@intel.com>
> > Subject: 回复: [edk2-devel] [PATCH v6]
> > MdeModulePkg/Universal/StatusCodeHandler: Fix a bug about log lost
> >
> > Ming:
> >   I have no concern for the updated patch.
> >
> >   I just think we can simplify the code logic and remove the function
> > UnregisterSerialBootTimeHandlers(). Unregister logic can be moved to
> > SerialStatusCodeReportWorker().
> >
> >   You can choose your preferred implementation. Reviewed-by: Liming
> > Gao <gaoliming@byosoft.com.cn>
> >
> > Thanks
> > Liming
> > > -----邮件原件-----
> > > 发件人: bounce+27952+69302+4905953+8761045@groups.io
> > > <bounce+27952+69302+4905953+8761045@groups.io> 代表 Tan, Ming
> > > 发送时间: 2020年12月21日 15:18
> > > 收件人: Bi, Dandan <dandan.bi@intel.com>; Liming Gao
> > > <gaoliming@byosoft.com.cn>
> > > 抄送: devel@edk2.groups.io
> > > 主题: Re: [edk2-devel] [PATCH v6]
> > > MdeModulePkg/Universal/StatusCodeHandler: Fix a bug about log lost
> > >
> > > Liming:
> > >   I already submit a new patch for it, is there any new concern for it?
> > >
> > >   Thank you.
> > >   Tan Ming.
> > >
> > > -----Original Message-----
> > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Tan,
> > > Ming
> > > Sent: Friday, December 18, 2020 12:51 PM
> > > To: devel@edk2.groups.io
> > > Cc: Bi, Dandan <dandan.bi@intel.com>; Liming Gao
> > > <gaoliming@byosoft.com.cn>
> > > Subject: [edk2-devel] [PATCH v6]
> > > MdeModulePkg/Universal/StatusCodeHandler: Fix a bug about log lost
> > >
> > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3126
> > >
> > > 1. If use PeiDxeDebugLibReportStatusCode as DebugLib, then some logs
> > > after
> > > ExitBootService() will be lost.
> > > 2. The root cause:
> > > 2.1 The original code will register an unregister function of
> > > gEfiEventExitBootServicesGuid, this unregister function will call
> > > EFI_RSC_HANDLER_PROTOCOL->Unregister and does not support log
> > through
> > > serial port.
> > > 2.2 And some other drivers also register call back funtions of
> > > gEfiEventExitBootServicesGuid.
> > > 2.3 Then after the unregister function is called, other call back
> > functions can't
> > > out log if them use RSC as DebugLib.
> > > 3. The DxeMain will report status code
> > EFI_SW_BS_PC_EXIT_BOOT_SERVICES
> > > after notify all the call back functions of gEfiEventExitBootServicesGuid.
> > > 4. Solution: the StatusCodeHandlerRuntimeDxe.c will not register an
> > > unregister function of gEfiEventExitBootServicesGuid, but unregister
> > > it
> > after
> > > receive the status code of EFI_SW_BS_PC_EXIT_BOOT_SERVICES.
> > >
> > > Cc: Dandan Bi <dandan.bi@intel.com>
> > > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > > Signed-off-by: Ming Tan <ming.tan@intel.com>
> > > ---
> > > V6: Change the funcname of UnregisterBootTimeHandlers to
> > > UnregisterSerialBootTimeHandlers.
> > > V5: Fix an unused var bug and compile error using GCC.
> > > V4: Fix a spell bug in code comment, change 'a' to 'an' before
> > 'unregister'.
> > > V3: Fix a spell bug in commit message, change 'a' to 'an' before
> > 'unregister'.
> > > V2: Add the REF link in commit message.
> > >
> > >  .../RuntimeDxe/SerialStatusCodeWorker.c       | 10 ++++++++++
> > >  .../RuntimeDxe/StatusCodeHandlerRuntimeDxe.c  | 19
> > > ++-----------------  .../RuntimeDxe/StatusCodeHandlerRuntimeDxe.h  |
> > > ++11
> > > +++++++++++
> > >  3 files changed, 23 insertions(+), 17 deletions(-)
> > >
> > > diff --git
> > >
> >
> a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/SerialStatusC
> > o
> > > deWorker.c
> > >
> >
> b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/SerialStatusC
> > o
> > > deWorker.c
> > > index 0b98e7ec63..348f55edba 100644
> > > ---
> > >
> >
> a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/SerialStatusC
> > o
> > > deWorker.c
> > > +++
> > >
> >
> b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/SerialStatusC
> > o
> > > +++ deWorker.c
> > > @@ -151,6 +151,16 @@ SerialStatusCodeReportWorker (
> > >    //   SerialPortWrite ((UINT8 *) Buffer, CharCount); +  //+  // If
> > register
> > > an unregister function of gEfiEventExitBootServicesGuid,+  // then
> > > some
> > log
> > > called in ExitBootServices() will be lost,+  // so unregister the
> > > handler
> > after
> > > receive the value of exit boot service.+  //+  if ((CodeType &
> > > EFI_STATUS_CODE_TYPE_MASK) == EFI_PROGRESS_CODE &&+      Value
> > > == (EFI_SOFTWARE_EFI_BOOT_SERVICE |
> > > EFI_SW_BS_PC_EXIT_BOOT_SERVICES)) {+
> > > UnregisterSerialBootTimeHandlers();+  }+   return EFI_SUCCESS; } diff
> > --git
> > >
> >
> a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeH
> > a
> > > ndlerRuntimeDxe.c
> > >
> >
> b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeH
> > a
> > > ndlerRuntimeDxe.c
> > > index a8c0fe5b71..d50335af8a 100644
> > > ---
> > >
> >
> a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeH
> > a
> > > ndlerRuntimeDxe.c
> > > +++
> > >
> >
> b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeH
> > a
> > > nd
> > > +++ lerRuntimeDxe.c
> > > @@ -10,23 +10,17 @@
> > >  #include "StatusCodeHandlerRuntimeDxe.h"  EFI_EVENT
> > > mVirtualAddressChangeEvent = NULL;-static EFI_EVENT
> > > mExitBootServicesEvent     = NULL; EFI_RSC_HANDLER_PROTOCOL
> > > *mRscHandlerProtocol       = NULL;  /**   Unregister status code
> > > callback functions only available at boot time from   report status code
> > > router when exiting boot services. -  @param  Event         Event
> > > whose notification function is being invoked.-  @param  Context
> > > Pointer to the notification function's context, which is- always
> > > zero in current implementation.- **/ VOID
> > > EFIAPI-UnregisterBootTimeHandlers (-  IN EFI_EVENT        Event,-  IN
> > > VOID             *Context+UnregisterSerialBootTimeHandlers (+
> > > VOID   ) {   if (PcdGetBool (PcdStatusCodeUseSerial)) {@@ -178,15 +172,6
> > > @@ StatusCodeHandlerRuntimeDxeEntry (
> > >      mRscHandlerProtocol->Register (RtMemoryStatusCodeReportWorker,
> > > TPL_HIGH_LEVEL);   } -  Status = gBS->CreateEventEx (-
> > > EVT_NOTIFY_SIGNAL,-                  TPL_NOTIFY,-
> > > UnregisterBootTimeHandlers,-                  NULL,-
> > > &gEfiEventExitBootServicesGuid,-
> > > &mExitBootServicesEvent-                  );-   Status =
> > > gBS->CreateEventEx (                   EVT_NOTIFY_SIGNAL,
> > > TPL_NOTIFY,diff --git
> > >
> >
> a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeH
> > a
> > > ndlerRuntimeDxe.h
> > >
> >
> b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeH
> > a
> > > ndlerRuntimeDxe.h
> > > index fd4689c2d7..a2cf2ae0b7 100644
> > > ---
> > >
> >
> a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeH
> > a
> > > ndlerRuntimeDxe.h
> > > +++
> > >
> >
> b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeH
> > a
> > > nd
> > > +++ lerRuntimeDxe.h
> > > @@ -118,4 +118,15 @@ RtMemoryStatusCodeReportWorker (
> > >    IN EFI_STATUS_CODE_DATA               *Data OPTIONAL   );
> > > +/**+  Unregister status code callback functions only available at
> > > +boot
> > time
> > > from+  report status code router when exiting boot
> > > services.++**/+VOID+EFIAPI+UnregisterSerialBootTimeHandlers (+
> > > VOID+  );+ #endif--
> > > 2.24.0.windows.2
> > >
> > >
> > >
> > > -=-=-=-=-=-=
> > > Groups.io Links: You receive all messages sent to this group.
> > > View/Reply Online (#69173):
> > > https://edk2.groups.io/g/devel/message/69173
> > > Mute This Topic: https://groups.io/mt/79055338/2557430
> > > Group Owner: devel+owner@edk2.groups.io
> > > Unsubscribe: https://edk2.groups.io/g/devel/unsub
> > > [ming.tan@intel.com] -=-=-=-=-=-=
> > >
> > >
> > >
> > >
> > >
> > >
> >
> >
> >
> >
> >
> > 
> >