1. Patchew
  2. EDK2
  3. View series

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

Tan, Ming posted 1 patch 3 years, 4 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/edk2 tags/patchew/20201217135859.1677-1-ming.tan@intel.com
There is a newer version of this series
.../RuntimeDxe/SerialStatusCodeWorker.c         | 10 ++++++++++
.../RuntimeDxe/StatusCodeHandlerRuntimeDxe.c    | 17 +----------------
.../RuntimeDxe/StatusCodeHandlerRuntimeDxe.h    | 11 +++++++++++
3 files changed, 22 insertions(+), 16 deletions(-)
[edk2-devel] [PATCH v5] MdeModulePkg/Universal/StatusCodeHandler: Fix a bug about log lost
Posted by Tan, Ming 3 years, 4 months ago 
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>
---
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    | 17 +----------------
 .../RuntimeDxe/StatusCodeHandlerRuntimeDxe.h    | 11 +++++++++++
 3 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/SerialStatusCodeWorker.c b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/SerialStatusCodeWorker.c
index 0b98e7ec63..f7ab71e32a 100644
--- a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/SerialStatusCodeWorker.c
+++ b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/SerialStatusCodeWorker.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)) {
+    UnregisterBootTimeHandlers();
+  }
+
   return EFI_SUCCESS;
 }
 
diff --git a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHandlerRuntimeDxe.c b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHandlerRuntimeDxe.c
index a8c0fe5b71..2e11c1f4d2 100644
--- a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHandlerRuntimeDxe.c
+++ b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHandlerRuntimeDxe.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
+  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..f1789dc538 100644
--- a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHandlerRuntimeDxe.h
+++ b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHandlerRuntimeDxe.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
+UnregisterBootTimeHandlers (
+  VOID
+  );
+
 #endif
-- 
2.24.0.windows.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#69130): https://edk2.groups.io/g/devel/message/69130
Mute This Topic: https://groups.io/mt/79036460/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
回复: [edk2-devel] [PATCH v5] MdeModulePkg/Universal/StatusCodeHandler: Fix a bug about log lost
Posted by gaoliming 3 years, 4 months ago 
Ming:
  This change uses the accurate point to exit boot service. It is a good
enhancement. 

  For the patch, I have one comment that SerialStatusCodeReportWorker()
should call mRscHandlerProtocol unregister itself. 
  If there are more than one Status code handlers to be unregistered at the
exit boot service, each handler should unregister itself. 

Thanks
Liming
> -----邮件原件-----
> 发件人: bounce+27952+69130+4905953+8761045@groups.io
> <bounce+27952+69130+4905953+8761045@groups.io> 代表 Tan, Ming
> 发送时间: 2020年12月17日 21:59
> 收件人: devel@edk2.groups.io
> 抄送: Dandan Bi <dandan.bi@intel.com>; Liming Gao
> <gaoliming@byosoft.com.cn>
> 主题: [edk2-devel] [PATCH v5] 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>
> ---
> 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    | 17 +----------------
>  .../RuntimeDxe/StatusCodeHandlerRuntimeDxe.h    | 11 +++++++++++
>  3 files changed, 22 insertions(+), 16 deletions(-)
> 
> diff --git
> a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/SerialStatusCo
> deWorker.c
> b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/SerialStatusCo
> deWorker.c
> index 0b98e7ec63..f7ab71e32a 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)) {
> 
> +    UnregisterBootTimeHandlers();
> 
> +  }
> 
> +
> 
>    return EFI_SUCCESS;
> 
>  }
> 
> 
> 
> diff --git
> a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHa
> ndlerRuntimeDxe.c
> b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHa
> ndlerRuntimeDxe.c
> index a8c0fe5b71..2e11c1f4d2 100644
> ---
> a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHa
> ndlerRuntimeDxe.c
> +++
> b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHa
> ndlerRuntimeDxe.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
> 
> +  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..f1789dc538 100644
> ---
> a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHa
> ndlerRuntimeDxe.h
> +++
> b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHa
> ndlerRuntimeDxe.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
> 
> +UnregisterBootTimeHandlers (
> 
> +  VOID
> 
> +  );
> 
> +
> 
>  #endif
> 
> --
> 2.24.0.windows.2
> 
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#69130): https://edk2.groups.io/g/devel/message/69130
> Mute This Topic: https://groups.io/mt/79036460/4905953
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub
> [gaoliming@byosoft.com.cn]
> -=-=-=-=-=-=
> 





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#69160): https://edk2.groups.io/g/devel/message/69160
Mute This Topic: https://groups.io/mt/79052112/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v5] MdeModulePkg/Universal/StatusCodeHandler: Fix a bug about log lost
Posted by Tan, Ming 3 years, 4 months ago 
Liming:
  Yes, in another private driver about status code, there are several handlers need to be unregistered, so I create several unregister functions to be called.
  And currently the UnregisterBootTimeHandlers function only has one handler need to be unregistered, so I call it directly. If we want to highlight it, I can change the function name to UnregisterSerialBootTimeHandlers().

  BR/Tan Ming.

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of gaoliming
Sent: Friday, December 18, 2020 9:10 AM
To: devel@edk2.groups.io; Tan, Ming <ming.tan@intel.com>
Cc: Bi, Dandan <dandan.bi@intel.com>
Subject: 回复: [edk2-devel] [PATCH v5] MdeModulePkg/Universal/StatusCodeHandler: Fix a bug about log lost

Ming:
  This change uses the accurate point to exit boot service. It is a good enhancement. 

  For the patch, I have one comment that SerialStatusCodeReportWorker() should call mRscHandlerProtocol unregister itself. 
  If there are more than one Status code handlers to be unregistered at the exit boot service, each handler should unregister itself. 

Thanks
Liming
> -----邮件原件-----
> 发件人: bounce+27952+69130+4905953+8761045@groups.io
> <bounce+27952+69130+4905953+8761045@groups.io> 代表 Tan, Ming
> 发送时间: 2020年12月17日 21:59
> 收件人: devel@edk2.groups.io
> 抄送: Dandan Bi <dandan.bi@intel.com>; Liming Gao 
> <gaoliming@byosoft.com.cn>
> 主题: [edk2-devel] [PATCH v5] 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>
> ---
> 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    | 17 +----------------
>  .../RuntimeDxe/StatusCodeHandlerRuntimeDxe.h    | 11 +++++++++++
>  3 files changed, 22 insertions(+), 16 deletions(-)
> 
> diff --git
> a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/SerialStatusCo
> deWorker.c
> b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/SerialStatusCo
> deWorker.c
> index 0b98e7ec63..f7ab71e32a 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)) {
> 
> +    UnregisterBootTimeHandlers();
> 
> +  }
> 
> +
> 
>    return EFI_SUCCESS;
> 
>  }
> 
> 
> 
> diff --git
> a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHa
> ndlerRuntimeDxe.c
> b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHa
> ndlerRuntimeDxe.c
> index a8c0fe5b71..2e11c1f4d2 100644
> ---
> a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHa
> ndlerRuntimeDxe.c
> +++
> b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHa
> ndlerRuntimeDxe.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
> 
> +  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..f1789dc538 100644
> ---
> a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHa
> ndlerRuntimeDxe.h
> +++
> b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHa
> ndlerRuntimeDxe.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
> 
> +UnregisterBootTimeHandlers (
> 
> +  VOID
> 
> +  );
> 
> +
> 
>  #endif
> 
> --
> 2.24.0.windows.2
> 
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#69130): 
> https://edk2.groups.io/g/devel/message/69130
> Mute This Topic: https://groups.io/mt/79036460/4905953
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub
> [gaoliming@byosoft.com.cn]
> -=-=-=-=-=-=
> 










-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#69161): https://edk2.groups.io/g/devel/message/69161
Mute This Topic: https://groups.io/mt/79052389/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
回复: [edk2-devel] [PATCH v5] MdeModulePkg/Universal/StatusCodeHandler: Fix a bug about log lost
Posted by gaoliming 3 years, 4 months ago 
Ming:
  In fact, UnregisterBootTimeHandlers is not required any more. Each ReportWorker() should call mRscHandlerProtocol to unregister itself. 

  If UnregisterBootTimeHandlers is still kept, it may be wrongly used for the future reportworker extension. 

Thanks
Liming
> -----邮件原件-----
> 发件人: Tan, Ming <ming.tan@intel.com>
> 发送时间: 2020年12月18日 9:21
> 收件人: devel@edk2.groups.io; gaoliming@byosoft.com.cn
> 抄送: Bi, Dandan <dandan.bi@intel.com>
> 主题: RE: [edk2-devel] [PATCH v5]
> MdeModulePkg/Universal/StatusCodeHandler: Fix a bug about log lost
> 
> Liming:
>   Yes, in another private driver about status code, there are several handlers
> need to be unregistered, so I create several unregister functions to be called.
>   And currently the UnregisterBootTimeHandlers function only has one
> handler need to be unregistered, so I call it directly. If we want to highlight it, I
> can change the function name to UnregisterSerialBootTimeHandlers().
> 
>   BR/Tan Ming.
> 
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of gaoliming
> Sent: Friday, December 18, 2020 9:10 AM
> To: devel@edk2.groups.io; Tan, Ming <ming.tan@intel.com>
> Cc: Bi, Dandan <dandan.bi@intel.com>
> Subject: 回复: [edk2-devel] [PATCH v5]
> MdeModulePkg/Universal/StatusCodeHandler: Fix a bug about log lost
> 
> Ming:
>   This change uses the accurate point to exit boot service. It is a good
> enhancement.
> 
>   For the patch, I have one comment that SerialStatusCodeReportWorker()
> should call mRscHandlerProtocol unregister itself.
>   If there are more than one Status code handlers to be unregistered at the
> exit boot service, each handler should unregister itself.
> 
> Thanks
> Liming
> > -----邮件原件-----
> > 发件人: bounce+27952+69130+4905953+8761045@groups.io
> > <bounce+27952+69130+4905953+8761045@groups.io> 代表 Tan, Ming
> > 发送时间: 2020年12月17日 21:59
> > 收件人: devel@edk2.groups.io
> > 抄送: Dandan Bi <dandan.bi@intel.com>; Liming Gao
> > <gaoliming@byosoft.com.cn>
> > 主题: [edk2-devel] [PATCH v5]
> 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>
> > ---
> > 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    | 17 +----------------
> >  .../RuntimeDxe/StatusCodeHandlerRuntimeDxe.h    | 11 +++++++++++
> >  3 files changed, 22 insertions(+), 16 deletions(-)
> >
> > diff --git
> >
> a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/SerialStatusCo
> > deWorker.c
> >
> b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/SerialStatusCo
> > deWorker.c
> > index 0b98e7ec63..f7ab71e32a 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)) {
> >
> > +    UnregisterBootTimeHandlers();
> >
> > +  }
> >
> > +
> >
> >    return EFI_SUCCESS;
> >
> >  }
> >
> >
> >
> > diff --git
> >
> a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHa
> > ndlerRuntimeDxe.c
> >
> b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHa
> > ndlerRuntimeDxe.c
> > index a8c0fe5b71..2e11c1f4d2 100644
> > ---
> >
> a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHa
> > ndlerRuntimeDxe.c
> > +++
> >
> b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHa
> > ndlerRuntimeDxe.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
> >
> > +  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..f1789dc538 100644
> > ---
> >
> a/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHa
> > ndlerRuntimeDxe.h
> > +++
> >
> b/MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHa
> > ndlerRuntimeDxe.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
> >
> > +UnregisterBootTimeHandlers (
> >
> > +  VOID
> >
> > +  );
> >
> > +
> >
> >  #endif
> >
> > --
> > 2.24.0.windows.2
> >
> >
> >
> > -=-=-=-=-=-=
> > Groups.io Links: You receive all messages sent to this group.
> > View/Reply Online (#69130):
> > https://edk2.groups.io/g/devel/message/69130
> > Mute This Topic: https://groups.io/mt/79036460/4905953
> > Group Owner: devel+owner@edk2.groups.io
> > Unsubscribe: https://edk2.groups.io/g/devel/unsub
> > [gaoliming@byosoft.com.cn]
> > -=-=-=-=-=-=
> >
> 
> 
> 
> 
> 
> 
> 





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#69275): https://edk2.groups.io/g/devel/message/69275
Mute This Topic: https://groups.io/mt/79116748/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.