[edk2-devel] [PATCH v6] MdeModulePkg/Universal/StatusCodeHandler: Fix a bug about log lost
Dandan Bi
dandan.bi at intel.com
Tue Dec 22 03:17:15 UTC 2020
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 at intel.com>; gaoliming at byosoft.com.cn
> Cc: Wang, Jian J <jian.j.wang at intel.com>; Wu, Hao A <hao.a.wu at intel.com>;
> devel at edk2.groups.io
> Subject: RE: [edk2-devel] [PATCH v6]
> MdeModulePkg/Universal/StatusCodeHandler: Fix a bug about log lost
>
> Reviewed-by: Dandan Bi <dandan.bi at intel.com> for V6, will create a pull
> request for it.
>
>
> Thanks,
> Dandan
> > -----Original Message-----
> > From: Tan, Ming <ming.tan at intel.com>
> > Sent: Tuesday, December 22, 2020 9:26 AM
> > To: gaoliming at byosoft.com.cn; Bi, Dandan <dandan.bi at intel.com>
> > Cc: Wang, Jian J <jian.j.wang at intel.com>; Wu, Hao A
> > <hao.a.wu at intel.com>; devel at 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 at byosoft.com.cn; Bi, Dandan <dandan.bi at intel.com>
> > Cc: devel at 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 at edk2.groups.io <devel at edk2.groups.io> On Behalf Of
> > gaoliming
> > Sent: Tuesday, December 22, 2020 8:36 AM
> > To: devel at edk2.groups.io; Tan, Ming <ming.tan at intel.com>; Bi, Dandan
> > <dandan.bi at 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 at byosoft.com.cn>
> >
> > Thanks
> > Liming
> > > -----邮件原件-----
> > > 发件人: bounce+27952+69302+4905953+8761045 at groups.io
> > > <bounce+27952+69302+4905953+8761045 at groups.io> 代表 Tan, Ming
> > > 发送时间: 2020年12月21日 15:18
> > > 收件人: Bi, Dandan <dandan.bi at intel.com>; Liming Gao
> > > <gaoliming at byosoft.com.cn>
> > > 抄送: devel at 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 at edk2.groups.io <devel at edk2.groups.io> On Behalf Of Tan,
> > > Ming
> > > Sent: Friday, December 18, 2020 12:51 PM
> > > To: devel at edk2.groups.io
> > > Cc: Bi, Dandan <dandan.bi at intel.com>; Liming Gao
> > > <gaoliming at 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 at intel.com>
> > > Cc: Liming Gao <gaoliming at byosoft.com.cn>
> > > Signed-off-by: Ming Tan <ming.tan at 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 at edk2.groups.io
> > > Unsubscribe: https://edk2.groups.io/g/devel/unsub
> > > [ming.tan at intel.com] -=-=-=-=-=-=
> > >
> > >
> > >
> > >
> > >
> > >
> >
> >
> >
> >
> >
> >
> >
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#69369): https://edk2.groups.io/g/devel/message/69369
Mute This Topic: https://groups.io/mt/79139927/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list