* Re: [edk2-devel] [PATCH v6] MdeModulePkg/Universal/StatusCodeHandler: Fix a bug about log lost [not found] <1651B662B221A676.25767@groups.io> @ 2020-12-21 7:17 ` Tan, Ming 2020-12-22 0:35 ` 回复: " gaoliming 0 siblings, 1 reply; 8+ messages in thread From: Tan, Ming @ 2020-12-21 7:17 UTC (permalink / raw) To: Bi, Dandan, Liming Gao; +Cc: devel@edk2.groups.io 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/SerialStatusCodeWorker.c b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/SerialStatusCodeWorker.c index 0b98e7ec63..348f55edba 100644 --- a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/SerialStatusCodeWorker.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/StatusCodeHandlerRuntimeDxe.c b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHandlerRuntimeDxe.c index a8c0fe5b71..d50335af8a 100644 --- a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHandlerRuntimeDxe.c +++ b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHand +++ 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/StatusCodeHandlerRuntimeDxe.h b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHandlerRuntimeDxe.h index fd4689c2d7..a2cf2ae0b7 100644 --- a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHandlerRuntimeDxe.h +++ b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHand +++ 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] -=-=-=-=-=-= ^ permalink raw reply [flat|nested] 8+ messages in thread
* 回复: [edk2-devel] [PATCH v6] MdeModulePkg/Universal/StatusCodeHandler: Fix a bug about log lost 2020-12-21 7:17 ` [edk2-devel] [PATCH v6] MdeModulePkg/Universal/StatusCodeHandler: Fix a bug about log lost Tan, Ming @ 2020-12-22 0:35 ` gaoliming 2020-12-22 0:56 ` Tan, Ming 0 siblings, 1 reply; 8+ messages in thread From: gaoliming @ 2020-12-22 0:35 UTC (permalink / raw) To: devel, ming.tan, 'Bi, Dandan' 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/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@edk2.groups.io > Unsubscribe: https://edk2.groups.io/g/devel/unsub [ming.tan@intel.com] > -=-=-=-=-=-= > > > > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [PATCH v6] MdeModulePkg/Universal/StatusCodeHandler: Fix a bug about log lost 2020-12-22 0:35 ` 回复: " gaoliming @ 2020-12-22 0:56 ` Tan, Ming 2020-12-22 1:26 ` Tan, Ming 2020-12-22 1:40 ` 回复: " gaoliming 0 siblings, 2 replies; 8+ messages in thread From: Tan, Ming @ 2020-12-22 0:56 UTC (permalink / raw) To: gaoliming@byosoft.com.cn, Bi, Dandan; +Cc: devel@edk2.groups.io 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/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@edk2.groups.io > Unsubscribe: https://edk2.groups.io/g/devel/unsub [ming.tan@intel.com] > -=-=-=-=-=-= > > > > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [PATCH v6] MdeModulePkg/Universal/StatusCodeHandler: Fix a bug about log lost 2020-12-22 0:56 ` Tan, Ming @ 2020-12-22 1:26 ` Tan, Ming 2020-12-22 1:36 ` Dandan Bi 2020-12-22 1:40 ` 回复: " gaoliming 1 sibling, 1 reply; 8+ messages in thread From: Tan, Ming @ 2020-12-22 1:26 UTC (permalink / raw) To: gaoliming@byosoft.com.cn, Bi, Dandan Cc: Wang, Jian J, Wu, Hao A, devel@edk2.groups.io 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/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@edk2.groups.io > Unsubscribe: https://edk2.groups.io/g/devel/unsub [ming.tan@intel.com] > -=-=-=-=-=-= > > > > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [PATCH v6] MdeModulePkg/Universal/StatusCodeHandler: Fix a bug about log lost 2020-12-22 1:26 ` Tan, Ming @ 2020-12-22 1:36 ` Dandan Bi 2020-12-22 3:17 ` Dandan Bi 0 siblings, 1 reply; 8+ messages in thread From: Dandan Bi @ 2020-12-22 1:36 UTC (permalink / raw) To: Tan, Ming, gaoliming@byosoft.com.cn Cc: Wang, Jian J, Wu, Hao A, devel@edk2.groups.io 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] > > -=-=-=-=-=-= > > > > > > > > > > > > > > > > > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [PATCH v6] MdeModulePkg/Universal/StatusCodeHandler: Fix a bug about log lost 2020-12-22 1:36 ` Dandan Bi @ 2020-12-22 3:17 ` Dandan Bi 2021-01-07 14:19 ` Laszlo Ersek 0 siblings, 1 reply; 8+ messages in thread From: Dandan Bi @ 2020-12-22 3:17 UTC (permalink / raw) To: Tan, Ming, gaoliming@byosoft.com.cn Cc: Wang, Jian J, Wu, Hao A, devel@edk2.groups.io 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] -=-=-=-=-=-= > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [PATCH v6] MdeModulePkg/Universal/StatusCodeHandler: Fix a bug about log lost 2020-12-22 3:17 ` Dandan Bi @ 2021-01-07 14:19 ` Laszlo Ersek 0 siblings, 0 replies; 8+ messages in thread From: Laszlo Ersek @ 2021-01-07 14:19 UTC (permalink / raw) To: devel, dandan.bi, Tan, Ming, gaoliming@byosoft.com.cn Cc: Wang, Jian J, Wu, Hao A On 12/22/20 04:17, Dandan Bi wrote: > Here is the pull request https://github.com/tianocore/edk2/pull/1259 "PR can not be merged due to a PatchCheck failure. Please resolve and resubmit" I'm closing the PR for now. Thanks, Laszlo ^ permalink raw reply [flat|nested] 8+ messages in thread
* 回复: [edk2-devel] [PATCH v6] MdeModulePkg/Universal/StatusCodeHandler: Fix a bug about log lost 2020-12-22 0:56 ` Tan, Ming 2020-12-22 1:26 ` Tan, Ming @ 2020-12-22 1:40 ` gaoliming 1 sibling, 0 replies; 8+ messages in thread From: gaoliming @ 2020-12-22 1:40 UTC (permalink / raw) To: 'Tan, Ming', 'Bi, Dandan'; +Cc: devel Ming: Just need to declare extern mRscHandlerProtocol in StatusCodeHandlerRuntimeDxe.h. It has been located in module entry point. So, I think it is simple. Thanks Liming > -----邮件原件----- > 发件人: Tan, Ming <ming.tan@intel.com> > 发送时间: 2020年12月22日 8:56 > 收件人: gaoliming@byosoft.com.cn; Bi, Dandan <dandan.bi@intel.com> > 抄送: devel@edk2.groups.io > 主题: 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/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@edk2.groups.io > > Unsubscribe: https://edk2.groups.io/g/devel/unsub [ming.tan@intel.com] > > -=-=-=-=-=-= > > > > > > > > > > > > > > > > > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-01-07 14:20 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1651B662B221A676.25767@groups.io>
2020-12-21 7:17 ` [edk2-devel] [PATCH v6] MdeModulePkg/Universal/StatusCodeHandler: Fix a bug about log lost Tan, Ming
2020-12-22 0:35 ` 回复: " gaoliming
2020-12-22 0:56 ` Tan, Ming
2020-12-22 1:26 ` Tan, Ming
2020-12-22 1:36 ` Dandan Bi
2020-12-22 3:17 ` Dandan Bi
2021-01-07 14:19 ` Laszlo Ersek
2020-12-22 1:40 ` 回复: " gaoliming
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox