回复: [edk2-devel] [PATCH v6] MdeModulePkg/Universal/StatusCodeHandler: Fix a bug about log lost
gaoliming
gaoliming at byosoft.com.cn
Tue Dec 22 00:35:32 UTC 2020
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/SerialStatusCo
> deWorker.c
> b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/SerialStatusCo
> deWorker.c
> index 0b98e7ec63..348f55edba 100644
> ---
> a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/SerialStatusCo
> deWorker.c
> +++
> b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/SerialStatusCo
> +++ 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/StatusCodeHa
> ndlerRuntimeDxe.c
> b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHa
> ndlerRuntimeDxe.c
> index a8c0fe5b71..d50335af8a 100644
> ---
> a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHa
> ndlerRuntimeDxe.c
> +++
> b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHa
> 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/StatusCodeHa
> ndlerRuntimeDxe.h
> b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHa
> ndlerRuntimeDxe.h
> index fd4689c2d7..a2cf2ae0b7 100644
> ---
> a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHa
> ndlerRuntimeDxe.h
> +++
> b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHa
> 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 (#69349): https://edk2.groups.io/g/devel/message/69349
Mute This Topic: https://groups.io/mt/79139635/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