public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH 0/5] Make some DebugLib instance runtime safe
@ 2019-04-10  7:03 Gao, Zhichao
  2019-04-10  7:03 ` [PATCH 1/5] MdeModulePkg: Change the SMM debug lib instance Gao, Zhichao
                   ` (5 more replies)
  0 siblings, 6 replies; 9+ messages in thread
From: Gao, Zhichao @ 2019-04-10  7:03 UTC (permalink / raw)
  To: devel
  Cc: Jiewen Yao, Chao Zhang, Jian J Wang, Ray Ni, Star Zeng,
	Liming Gao, Sean Brogan, Michael Turner, Bret Barkelew

Some pointer will be invalid after ExitBootServices, such as protocol pointer,
gST, gBS and so on. Disable the functions which used that pointer.

Remove SMM support of UefiDebugLibDebugPortProtocol, UefidebugLibConOut
and UefiDebugLibStdErr.
Before removing, should change the SMM instance in the dsc file which use
them as SMM instance. And null version is safe.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Turner <Michael.Turner@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>

Aaron Antone (3):
  MdeModulePkg/UefiDebugLibDebugPortProtocol: Make it runtime safe
  MdeModulePkg/UefidebugLibConOut: Make it runtime safe
  MdeModule/UefiDebugLibStdErr: Make it runtime safe

Zhichao Gao (2):
  MdeModulePkg: Change the SMM debug lib instance
  SignedCapsulePkg: Change the SMM debug lib instance

 MdeModulePkg/MdeModulePkg.dsc                 |   2 +-
 MdePkg/Library/UefiDebugLibConOut/DebugLib.c  | 116 +++++++-------
 .../UefiDebugLibConOut/DebugLibConstructor.c  |  72 +++++++++
 .../UefiDebugLibConOut/UefiDebugLibConOut.inf |  12 +-
 .../UefiDebugLibDebugPortProtocol/DebugLib.c  | 146 +++++++++---------
 .../DebugLibConstructor.c                     |  72 +++++++++
 .../UefiDebugLibDebugPortProtocol.inf         |  12 +-
 MdePkg/Library/UefiDebugLibStdErr/DebugLib.c  | 113 +++++++-------
 .../UefiDebugLibStdErr/DebugLibConstructor.c  |  72 +++++++++
 .../UefiDebugLibStdErr/UefiDebugLibStdErr.inf |  12 +-
 SignedCapsulePkg/SignedCapsulePkg.dsc         |   4 +-
 11 files changed, 443 insertions(+), 190 deletions(-)
 create mode 100644 MdePkg/Library/UefiDebugLibConOut/DebugLibConstructor.c
 create mode 100644 MdePkg/Library/UefiDebugLibDebugPortProtocol/DebugLibConstructor.c
 create mode 100644 MdePkg/Library/UefiDebugLibStdErr/DebugLibConstructor.c

-- 
2.21.0.windows.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/5] MdeModulePkg: Change the SMM debug lib instance
  2019-04-10  7:03 [PATCH 0/5] Make some DebugLib instance runtime safe Gao, Zhichao
@ 2019-04-10  7:03 ` Gao, Zhichao
  2019-04-11  8:44   ` [edk2-devel] " Wu, Hao A
  2019-04-10  7:03 ` [PATCH 2/5] SignedCapsulePkg: " Gao, Zhichao
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 9+ messages in thread
From: Gao, Zhichao @ 2019-04-10  7:03 UTC (permalink / raw)
  To: devel
  Cc: Jian J Wang, Ray Ni, Star Zeng, Liming Gao, Sean Brogan,
	Michael Turner, Bret Barkelew

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1416

The UefiDebugLibConOut will not support DXE_SMM_DRIVER,
change UefiDebugLibConOut to PeiDxeDebugLibReportStatusCode.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Turner <Michael.Turner@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
---
 MdeModulePkg/MdeModulePkg.dsc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MdeModulePkg/MdeModulePkg.dsc b/MdeModulePkg/MdeModulePkg.dsc
index 7ef39af847..0da28e74c2 100644
--- a/MdeModulePkg/MdeModulePkg.dsc
+++ b/MdeModulePkg/MdeModulePkg.dsc
@@ -144,7 +144,7 @@
 
 [LibraryClasses.common.DXE_SMM_DRIVER]
   HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
-  DebugLib|MdePkg/Library/UefiDebugLibConOut/UefiDebugLibConOut.inf
+  DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf
   MemoryAllocationLib|MdePkg/Library/SmmMemoryAllocationLib/SmmMemoryAllocationLib.inf
   MmServicesTableLib|MdePkg/Library/MmServicesTableLib/MmServicesTableLib.inf
   SmmServicesTableLib|MdePkg/Library/SmmServicesTableLib/SmmServicesTableLib.inf
-- 
2.21.0.windows.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/5] SignedCapsulePkg: Change the SMM debug lib instance
  2019-04-10  7:03 [PATCH 0/5] Make some DebugLib instance runtime safe Gao, Zhichao
  2019-04-10  7:03 ` [PATCH 1/5] MdeModulePkg: Change the SMM debug lib instance Gao, Zhichao
@ 2019-04-10  7:03 ` Gao, Zhichao
  2019-04-10  7:03 ` [PATCH 3/5] MdeModulePkg/UefiDebugLibDebugPortProtocol: Make it runtime safe Gao, Zhichao
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Gao, Zhichao @ 2019-04-10  7:03 UTC (permalink / raw)
  To: devel
  Cc: Jian J Wang, Ray Ni, Star Zeng, Liming Gao, Sean Brogan,
	Michael Turner, Bret Barkelew

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1416

The UefiDebugLibConOut will not support DXE_SMM_DRIVER,
change UefiDebugLibConOut to PeiDxeDebugLibReportStatusCode.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Turner <Michael.Turner@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
---
 SignedCapsulePkg/SignedCapsulePkg.dsc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/SignedCapsulePkg/SignedCapsulePkg.dsc b/SignedCapsulePkg/SignedCapsulePkg.dsc
index 862756e233..0da445503b 100644
--- a/SignedCapsulePkg/SignedCapsulePkg.dsc
+++ b/SignedCapsulePkg/SignedCapsulePkg.dsc
@@ -1,7 +1,7 @@
 ## @file
 # This package provides EDKII capsule related support.
 #
-# Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2016 - 2019, Intel Corporation. All rights reserved.<BR>
 #
 #    SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -150,7 +150,7 @@
 
 [LibraryClasses.common.DXE_SMM_DRIVER]
   HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
-  DebugLib|MdePkg/Library/UefiDebugLibConOut/UefiDebugLibConOut.inf
+  DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf
   MemoryAllocationLib|MdePkg/Library/SmmMemoryAllocationLib/SmmMemoryAllocationLib.inf
   SmmServicesTableLib|MdePkg/Library/SmmServicesTableLib/SmmServicesTableLib.inf
   SmmMemLib|MdePkg/Library/SmmMemLib/SmmMemLib.inf
-- 
2.21.0.windows.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/5] MdeModulePkg/UefiDebugLibDebugPortProtocol: Make it runtime safe
  2019-04-10  7:03 [PATCH 0/5] Make some DebugLib instance runtime safe Gao, Zhichao
  2019-04-10  7:03 ` [PATCH 1/5] MdeModulePkg: Change the SMM debug lib instance Gao, Zhichao
  2019-04-10  7:03 ` [PATCH 2/5] SignedCapsulePkg: " Gao, Zhichao
@ 2019-04-10  7:03 ` Gao, Zhichao
  2019-04-10  7:03 ` [PATCH 4/5] MdeModulePkg/UefidebugLibConOut: " Gao, Zhichao
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Gao, Zhichao @ 2019-04-10  7:03 UTC (permalink / raw)
  To: devel
  Cc: Aaron Antone, Jian J Wang, Ray Ni, Star Zeng, Liming Gao,
	Sean Brogan, Michael Turner, Bret Barkelew

From: Aaron Antone <aanton@microsoft.com>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1416

After ExitBootServices, some pointer would be invalid such as
the Protocol pointer and gBS. The function depend on those should
be prevent. So disable the related function while after
ExitBootServices.
Change the gBS to a internal one, because there will be a cycle
consume between UefiBootServicesTableLib and DebugLib due to the
library constructors.
Also remove the SMM support for this instance.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Turner <Michael.Turner@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
---
 .../UefiDebugLibDebugPortProtocol/DebugLib.c  | 146 +++++++++---------
 .../DebugLibConstructor.c                     |  72 +++++++++
 .../UefiDebugLibDebugPortProtocol.inf         |  12 +-
 3 files changed, 158 insertions(+), 72 deletions(-)
 create mode 100644 MdePkg/Library/UefiDebugLibDebugPortProtocol/DebugLibConstructor.c

diff --git a/MdePkg/Library/UefiDebugLibDebugPortProtocol/DebugLib.c b/MdePkg/Library/UefiDebugLibDebugPortProtocol/DebugLib.c
index 24bfb4f37e..cd7e2abbd3 100644
--- a/MdePkg/Library/UefiDebugLibDebugPortProtocol/DebugLib.c
+++ b/MdePkg/Library/UefiDebugLibDebugPortProtocol/DebugLib.c
@@ -9,7 +9,6 @@
 #include <Uefi.h>
 
 #include <Library/DebugLib.h>
-#include <Library/UefiBootServicesTableLib.h>
 #include <Library/PrintLib.h>
 #include <Library/PcdLib.h>
 #include <Library/BaseLib.h>
@@ -37,6 +36,9 @@ EFI_DEBUGPORT_PROTOCOL *mDebugPort = NULL;
 //
 VA_LIST     mVaListNull;
 
+extern BOOLEAN                mPostEBS;
+extern EFI_BOOT_SERVICES     *mDebugBS;
+
 /**
   Send message to DebugPort Protocol.
 
@@ -56,31 +58,33 @@ UefiDebugLibDebugPortProtocolWrite (
   UINTN      Length;
   EFI_STATUS Status;
 
-  //
-  // If mDebugPort is NULL, initialize first.
-  //
-  if (mDebugPort == NULL) {
-      Status = gBS->LocateProtocol (&gEfiDebugPortProtocolGuid, NULL, (VOID **)&mDebugPort);
-      if (EFI_ERROR (Status)) {
-          return;
-      }
+  if (!mPostEBS) {
+    //
+    // If mDebugPort is NULL, initialize first.
+    //
+    if (mDebugPort == NULL) {
+        Status = mDebugBS->LocateProtocol (&gEfiDebugPortProtocolGuid, NULL, (VOID **)&mDebugPort);
+        if (EFI_ERROR (Status)) {
+            return;
+        }
+
+        mDebugPort->Reset (mDebugPort);
+    }
 
-      mDebugPort->Reset (mDebugPort);
-  }
+    //
+    // EFI_DEBUGPORT_PROTOCOL.Write is called until all message is sent.
+    //
+    while (BufferLength > 0) {
+      Length = BufferLength;
 
-  //
-  // EFI_DEBUGPORT_PROTOCOL.Write is called until all message is sent.
-  //
-  while (BufferLength > 0) {
-    Length = BufferLength;
+      Status = mDebugPort->Write (mDebugPort, WRITE_TIMEOUT, &Length, (VOID *) Buffer);
+      if (EFI_ERROR (Status) || BufferLength < Length) {
+        break;
+      }
 
-    Status = mDebugPort->Write (mDebugPort, WRITE_TIMEOUT, &Length, (VOID *) Buffer);
-    if (EFI_ERROR (Status) || BufferLength < Length) {
-      break;
+      Buffer += Length;
+      BufferLength -= Length;
     }
-
-    Buffer += Length;
-    BufferLength -= Length;
   }
 }
 
@@ -142,31 +146,33 @@ DebugPrintMarker (
 {
   CHAR8      Buffer[MAX_DEBUG_MESSAGE_LENGTH];
 
-  //
-  // If Format is NULL, then ASSERT().
-  //
-  ASSERT (Format != NULL);
+  if (!mPostEBS) {
+    //
+    // If Format is NULL, then ASSERT().
+    //
+    ASSERT (Format != NULL);
+
+    //
+    // Check driver debug mask value and global mask
+    //
+    if ((ErrorLevel & GetDebugPrintErrorLevel ()) == 0) {
+      return;
+    }
 
-  //
-  // Check driver debug mask value and global mask
-  //
-  if ((ErrorLevel & GetDebugPrintErrorLevel ()) == 0) {
-    return;
-  }
+    //
+    // Convert the DEBUG() message to an ASCII String
+    //
+    if (BaseListMarker == NULL) {
+      AsciiVSPrint (Buffer, sizeof (Buffer), Format, VaListMarker);
+    } else {
+      AsciiBSPrint (Buffer, sizeof (Buffer), Format, BaseListMarker);
+    }
 
-  //
-  // Convert the DEBUG() message to an ASCII String
-  //
-  if (BaseListMarker == NULL) {
-    AsciiVSPrint (Buffer, sizeof (Buffer), Format, VaListMarker);
-  } else {
-    AsciiBSPrint (Buffer, sizeof (Buffer), Format, BaseListMarker);
+    //
+    // Send the print string to EFI_DEBUGPORT_PROTOCOL.Write.
+    //
+    UefiDebugLibDebugPortProtocolWrite (Buffer, AsciiStrLen (Buffer));
   }
-
-  //
-  // Send the print string to EFI_DEBUGPORT_PROTOCOL.Write.
-  //
-  UefiDebugLibDebugPortProtocolWrite (Buffer, AsciiStrLen (Buffer));
 }
 
 
@@ -259,31 +265,33 @@ DebugAssert (
 {
   CHAR8  Buffer[MAX_DEBUG_MESSAGE_LENGTH];
 
-  //
-  // Generate the ASSERT() message in ASCII format
-  //
-  AsciiSPrint (
-    Buffer,
-    sizeof (Buffer),
-    "ASSERT [%a] %a(%d): %a\n",
-    gEfiCallerBaseName,
-    FileName,
-    LineNumber,
-    Description
-    );
-
-  //
-  // Send the print string to EFI_DEBUGPORT_PROTOCOL.Write.
-  //
-  UefiDebugLibDebugPortProtocolWrite (Buffer, AsciiStrLen (Buffer));
-
-  //
-  // Generate a Breakpoint, DeadLoop, or NOP based on PCD settings
-  //
-  if ((PcdGet8(PcdDebugPropertyMask) & DEBUG_PROPERTY_ASSERT_BREAKPOINT_ENABLED) != 0) {
-    CpuBreakpoint ();
-  } else if ((PcdGet8(PcdDebugPropertyMask) & DEBUG_PROPERTY_ASSERT_DEADLOOP_ENABLED) != 0) {
-    CpuDeadLoop ();
+  if (!mPostEBS) {
+    //
+    // Generate the ASSERT() message in ASCII format
+    //
+    AsciiSPrint (
+      Buffer,
+      sizeof (Buffer),
+      "ASSERT [%a] %a(%d): %a\n",
+      gEfiCallerBaseName,
+      FileName,
+      LineNumber,
+      Description
+      );
+
+    //
+    // Send the print string to EFI_DEBUGPORT_PROTOCOL.Write.
+    //
+    UefiDebugLibDebugPortProtocolWrite (Buffer, AsciiStrLen (Buffer));
+
+    //
+    // Generate a Breakpoint, DeadLoop, or NOP based on PCD settings
+    //
+    if ((PcdGet8(PcdDebugPropertyMask) & DEBUG_PROPERTY_ASSERT_BREAKPOINT_ENABLED) != 0) {
+      CpuBreakpoint ();
+    } else if ((PcdGet8(PcdDebugPropertyMask) & DEBUG_PROPERTY_ASSERT_DEADLOOP_ENABLED) != 0) {
+      CpuDeadLoop ();
+    }
   }
 }
 
diff --git a/MdePkg/Library/UefiDebugLibDebugPortProtocol/DebugLibConstructor.c b/MdePkg/Library/UefiDebugLibDebugPortProtocol/DebugLibConstructor.c
new file mode 100644
index 0000000000..1eae917932
--- /dev/null
+++ b/MdePkg/Library/UefiDebugLibDebugPortProtocol/DebugLibConstructor.c
@@ -0,0 +1,72 @@
+/** @file
+  UEFI Dxe DebugLib constructor that prevent some debug service after ExitBootServices event,
+  because some pointer is nulled at that phase.
+
+  Copyright (c) 2018, Microsoft Corporation
+  Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.<BR>
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include <Uefi.h>
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+
+//
+// BOOLEAN value to indicate if it is at the post ExitBootServices pahse
+//
+BOOLEAN     mPostEBS = FALSE;
+
+EFI_EVENT   mExitBootServicesEvent;
+
+//
+// Pointer to SystemTable
+// This library instance may have a cycle consume with UefiBootServicesTableLib
+// because of the constructors.
+//
+EFI_BOOT_SERVICES     *mDebugBS;
+
+/**
+  This routine sets the mPostEBS for exit boot servies true
+  to prevent DebugPort protocol dereferences when the pointer is nulled
+**/
+VOID
+EFIAPI
+ExitBootServicesCallback (
+  EFI_EVENT   Event,
+  VOID*       Context
+  )
+{
+  mPostEBS = TRUE;
+  return;
+}
+
+/**
+  The constructor gets the pointers to the system table and boot services table
+
+  @param  ImageHandle     The firmware allocated handle for the EFI image.
+  @param  SystemTable     A pointer to the EFI System Table.
+
+  @retval EFI_SUCCESS     The constructor always returns EFI_SUCCESS.
+
+**/
+EFI_STATUS
+EFIAPI
+DxeDebugLibConstructor(
+  IN EFI_HANDLE                 ImageHandle,
+  IN EFI_SYSTEM_TABLE           *SystemTable
+  )
+{
+  mDebugBS = SystemTable->BootServices;
+
+  mDebugBS->CreateEventEx (
+              EVT_NOTIFY_SIGNAL,
+              TPL_NOTIFY,
+              ExitBootServicesCallback,
+              NULL,
+              &gEfiEventExitBootServicesGuid,
+              &mExitBootServicesEvent
+              );
+
+  return EFI_SUCCESS;
+}
diff --git a/MdePkg/Library/UefiDebugLibDebugPortProtocol/UefiDebugLibDebugPortProtocol.inf b/MdePkg/Library/UefiDebugLibDebugPortProtocol/UefiDebugLibDebugPortProtocol.inf
index d9f4f8a548..10a8f2a857 100644
--- a/MdePkg/Library/UefiDebugLibDebugPortProtocol/UefiDebugLibDebugPortProtocol.inf
+++ b/MdePkg/Library/UefiDebugLibDebugPortProtocol/UefiDebugLibDebugPortProtocol.inf
@@ -3,7 +3,9 @@
 #
 #  Debug Lib that sends messages to EFI_DEBUGPORT_PROTOCOL.Write.
 #
-#  Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2018, Microsoft Corporation
+#
+#  Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.<BR>
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -17,8 +19,9 @@
   FILE_GUID                      = 102287b4-6b12-4D41-91e1-ebee1f3aa614
   MODULE_TYPE                    = UEFI_DRIVER
   VERSION_STRING                 = 1.0
-  LIBRARY_CLASS                  = DebugLib|DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SMM_DRIVER UEFI_APPLICATION UEFI_DRIVER
+  LIBRARY_CLASS                  = DebugLib|DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER UEFI_APPLICATION UEFI_DRIVER
 
+  CONSTRUCTOR                    = DxeDebugLibConstructor
 
 #
 #  VALID_ARCHITECTURES           = IA32 X64 EBC
@@ -27,6 +30,7 @@
 
 [Sources]
   DebugLib.c
+  DebugLibConstructor.c
 
 
 
@@ -39,9 +43,11 @@
   BaseLib
   PcdLib
   PrintLib
-  UefiBootServicesTableLib
   DebugPrintErrorLevelLib
 
+[Guids]
+  gEfiEventExitBootServicesGuid                 ## CONSUMES
+
 [Protocols]
   gEfiDebugPortProtocolGuid                     ## CONSUMES
 
-- 
2.21.0.windows.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 4/5] MdeModulePkg/UefidebugLibConOut: Make it runtime safe
  2019-04-10  7:03 [PATCH 0/5] Make some DebugLib instance runtime safe Gao, Zhichao
                   ` (2 preceding siblings ...)
  2019-04-10  7:03 ` [PATCH 3/5] MdeModulePkg/UefiDebugLibDebugPortProtocol: Make it runtime safe Gao, Zhichao
@ 2019-04-10  7:03 ` Gao, Zhichao
  2019-04-10  7:03 ` [PATCH 5/5] MdeModule/UefiDebugLibStdErr: " Gao, Zhichao
  2019-04-11  8:39 ` [edk2-devel] [PATCH 0/5] Make some DebugLib instance " Wu, Hao A
  5 siblings, 0 replies; 9+ messages in thread
From: Gao, Zhichao @ 2019-04-10  7:03 UTC (permalink / raw)
  To: devel
  Cc: Aaron Antone, Jian J Wang, Ray Ni, Star Zeng, Liming Gao,
	Sean Brogan, Michael Turner, Bret Barkelew

From: Aaron Antone <aanton@microsoft.com>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1416

After ExitBootServices, some pointer would be invalid such as
the Protocol pointer and gBS. The function depend on those should
be prevent. So disable the related function while after
ExitBootServices.
Change the gST to a internal one, because there will be a cycle
consume between UefiBootServicesTableLib and DebugLib due to the
library constructors.
Also remove the SMM support for this instance.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Turner <Michael.Turner@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
---
 MdePkg/Library/UefiDebugLibConOut/DebugLib.c  | 116 +++++++++---------
 .../UefiDebugLibConOut/DebugLibConstructor.c  |  72 +++++++++++
 .../UefiDebugLibConOut/UefiDebugLibConOut.inf |  12 +-
 3 files changed, 142 insertions(+), 58 deletions(-)
 create mode 100644 MdePkg/Library/UefiDebugLibConOut/DebugLibConstructor.c

diff --git a/MdePkg/Library/UefiDebugLibConOut/DebugLib.c b/MdePkg/Library/UefiDebugLibConOut/DebugLib.c
index c430419c99..cf168d05cf 100644
--- a/MdePkg/Library/UefiDebugLibConOut/DebugLib.c
+++ b/MdePkg/Library/UefiDebugLibConOut/DebugLib.c
@@ -9,7 +9,6 @@
 #include <Uefi.h>
 
 #include <Library/DebugLib.h>
-#include <Library/UefiBootServicesTableLib.h>
 #include <Library/PrintLib.h>
 #include <Library/PcdLib.h>
 #include <Library/BaseLib.h>
@@ -27,6 +26,9 @@
 //
 VA_LIST     mVaListNull;
 
+extern BOOLEAN                mPostEBS;
+extern EFI_SYSTEM_TABLE       *mDebugST;
+
 /**
   Prints a debug message to the debug output device if the specified error level is enabled.
 
@@ -85,33 +87,35 @@ DebugPrintMarker (
 {
   CHAR16   Buffer[MAX_DEBUG_MESSAGE_LENGTH];
 
-  //
-  // If Format is NULL, then ASSERT().
-  //
-  ASSERT (Format != NULL);
-
-  //
-  // Check driver debug mask value and global mask
-  //
-  if ((ErrorLevel & GetDebugPrintErrorLevel ()) == 0) {
-    return;
-  }
-
-  //
-  // Convert the DEBUG() message to a Unicode String
-  //
-  if (BaseListMarker == NULL) {
-    UnicodeVSPrintAsciiFormat (Buffer, MAX_DEBUG_MESSAGE_LENGTH,  Format, VaListMarker);
-  } else {
-    UnicodeBSPrintAsciiFormat (Buffer, MAX_DEBUG_MESSAGE_LENGTH,  Format, BaseListMarker);
-  }
-
-
-  //
-  // Send the print string to the Console Output device
-  //
-  if ((gST != NULL) && (gST->ConOut != NULL)) {
-    gST->ConOut->OutputString (gST->ConOut, Buffer);
+  if (!mPostEBS) {
+    //
+    // If Format is NULL, then ASSERT().
+    //
+    ASSERT (Format != NULL);
+
+    //
+    // Check driver debug mask value and global mask
+    //
+    if ((ErrorLevel & GetDebugPrintErrorLevel ()) == 0) {
+      return;
+    }
+
+    //
+    // Convert the DEBUG() message to a Unicode String
+    //
+    if (BaseListMarker == NULL) {
+      UnicodeVSPrintAsciiFormat (Buffer, MAX_DEBUG_MESSAGE_LENGTH,  Format, VaListMarker);
+    } else {
+      UnicodeBSPrintAsciiFormat (Buffer, MAX_DEBUG_MESSAGE_LENGTH,  Format, BaseListMarker);
+    }
+
+
+    //
+    // Send the print string to the Console Output device
+    //
+    if ((mDebugST != NULL) && (mDebugST->ConOut != NULL)) {
+      mDebugST->ConOut->OutputString (mDebugST->ConOut, Buffer);
+    }
   }
 }
 
@@ -205,33 +209,35 @@ DebugAssert (
 {
   CHAR16  Buffer[MAX_DEBUG_MESSAGE_LENGTH];
 
-  //
-  // Generate the ASSERT() message in Unicode format
-  //
-  UnicodeSPrintAsciiFormat (
-    Buffer,
-    sizeof (Buffer),
-    "ASSERT [%a] %a(%d): %a\n",
-    gEfiCallerBaseName,
-    FileName,
-    LineNumber,
-    Description
-    );
-
-  //
-  // Send the print string to the Console Output device
-  //
-  if ((gST != NULL) && (gST->ConOut != NULL)) {
-    gST->ConOut->OutputString (gST->ConOut, Buffer);
-  }
-
-  //
-  // Generate a Breakpoint, DeadLoop, or NOP based on PCD settings
-  //
-  if ((PcdGet8(PcdDebugPropertyMask) & DEBUG_PROPERTY_ASSERT_BREAKPOINT_ENABLED) != 0) {
-    CpuBreakpoint ();
-  } else if ((PcdGet8(PcdDebugPropertyMask) & DEBUG_PROPERTY_ASSERT_DEADLOOP_ENABLED) != 0) {
-    CpuDeadLoop ();
+  if (!mPostEBS) {
+    //
+    // Generate the ASSERT() message in Unicode format
+    //
+    UnicodeSPrintAsciiFormat (
+      Buffer,
+      sizeof (Buffer),
+      "ASSERT [%a] %a(%d): %a\n",
+      gEfiCallerBaseName,
+      FileName,
+      LineNumber,
+      Description
+      );
+
+    //
+    // Send the print string to the Console Output device
+    //
+    if ((mDebugST != NULL) && (mDebugST->ConOut != NULL)) {
+      mDebugST->ConOut->OutputString (mDebugST->ConOut, Buffer);
+    }
+
+    //
+    // Generate a Breakpoint, DeadLoop, or NOP based on PCD settings
+    //
+    if ((PcdGet8(PcdDebugPropertyMask) & DEBUG_PROPERTY_ASSERT_BREAKPOINT_ENABLED) != 0) {
+      CpuBreakpoint ();
+    } else if ((PcdGet8(PcdDebugPropertyMask) & DEBUG_PROPERTY_ASSERT_DEADLOOP_ENABLED) != 0) {
+      CpuDeadLoop ();
+    }
   }
 }
 
diff --git a/MdePkg/Library/UefiDebugLibConOut/DebugLibConstructor.c b/MdePkg/Library/UefiDebugLibConOut/DebugLibConstructor.c
new file mode 100644
index 0000000000..48a761a71e
--- /dev/null
+++ b/MdePkg/Library/UefiDebugLibConOut/DebugLibConstructor.c
@@ -0,0 +1,72 @@
+/** @file
+  UEFI Dxe DebugLib constructor that prevent some debug service after ExitBootServices event,
+  because some pointer is nulled at that phase.
+
+  Copyright (c) 2018, Microsoft Corporation
+  Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.<BR>
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include <Uefi.h>
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+
+//
+// BOOLEAN value to indicate if it is at the post ExitBootServices pahse
+//
+BOOLEAN     mPostEBS = FALSE;
+
+EFI_EVENT   mExitBootServicesEvent;
+
+//
+// Pointer to SystemTable
+// This library instance may have a cycle consume with UefiBootServicesTableLib
+// because of the constructors.
+//
+EFI_SYSTEM_TABLE      *mDebugST;
+
+/**
+  This routine sets the mPostEBS for exit boot servies true
+  to prevent DebugPort protocol dereferences when the pointer is nulled
+**/
+VOID
+EFIAPI
+ExitBootServicesCallback (
+  EFI_EVENT   Event,
+  VOID*       Context
+  )
+{
+  mPostEBS = TRUE;
+  return;
+}
+
+/**
+  The constructor gets the pointers to the system table and boot services table
+
+  @param  ImageHandle     The firmware allocated handle for the EFI image.
+  @param  SystemTable     A pointer to the EFI System Table.
+
+  @retval EFI_SUCCESS     The constructor always returns EFI_SUCCESS.
+
+**/
+EFI_STATUS
+EFIAPI
+DxeDebugLibConstructor(
+  IN EFI_HANDLE                 ImageHandle,
+  IN EFI_SYSTEM_TABLE           *SystemTable
+  )
+{
+  mDebugST = SystemTable;
+
+  SystemTable->BootServices->CreateEventEx (
+                                EVT_NOTIFY_SIGNAL,
+                                TPL_NOTIFY,
+                                ExitBootServicesCallback,
+                                NULL,
+                                &gEfiEventExitBootServicesGuid,
+                                &mExitBootServicesEvent
+                                );
+
+  return EFI_SUCCESS;
+}
diff --git a/MdePkg/Library/UefiDebugLibConOut/UefiDebugLibConOut.inf b/MdePkg/Library/UefiDebugLibConOut/UefiDebugLibConOut.inf
index 12af4a9e98..4c279a5bf2 100644
--- a/MdePkg/Library/UefiDebugLibConOut/UefiDebugLibConOut.inf
+++ b/MdePkg/Library/UefiDebugLibConOut/UefiDebugLibConOut.inf
@@ -3,7 +3,9 @@
 #
 #  Debug Lib that sends messages to the Console Output Device in the EFI System Table.
 #
-#  Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2018, Microsoft Corporation
+#
+#  Copyright (c) 2007 - 2019, Intel Corporation. All rights reserved.<BR>
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -17,8 +19,9 @@
   FILE_GUID                      = 5cddfaf3-e9a7-4d16-bdce-1e002df475bb
   MODULE_TYPE                    = UEFI_DRIVER
   VERSION_STRING                 = 1.0
-  LIBRARY_CLASS                  = DebugLib|DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SMM_DRIVER UEFI_APPLICATION UEFI_DRIVER
+  LIBRARY_CLASS                  = DebugLib|DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER UEFI_APPLICATION UEFI_DRIVER
 
+  CONSTRUCTOR                    = DxeDebugLibConstructor
 
 #
 #  VALID_ARCHITECTURES           = IA32 X64 EBC
@@ -27,6 +30,7 @@
 
 [Sources]
   DebugLib.c
+  DebugLibConstructor.c
 
 
 
@@ -39,9 +43,11 @@
   BaseLib
   PcdLib
   PrintLib
-  UefiBootServicesTableLib
   DebugPrintErrorLevelLib
 
+[Guids]
+  gEfiEventExitBootServicesGuid                 ## CONSUMES
+
 [Pcd]
   gEfiMdePkgTokenSpaceGuid.PcdDebugClearMemoryValue        ## SOMETIMES_CONSUMES
   gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask            ## CONSUMES
-- 
2.21.0.windows.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 5/5] MdeModule/UefiDebugLibStdErr: Make it runtime safe
  2019-04-10  7:03 [PATCH 0/5] Make some DebugLib instance runtime safe Gao, Zhichao
                   ` (3 preceding siblings ...)
  2019-04-10  7:03 ` [PATCH 4/5] MdeModulePkg/UefidebugLibConOut: " Gao, Zhichao
@ 2019-04-10  7:03 ` Gao, Zhichao
  2019-04-11  8:39 ` [edk2-devel] [PATCH 0/5] Make some DebugLib instance " Wu, Hao A
  5 siblings, 0 replies; 9+ messages in thread
From: Gao, Zhichao @ 2019-04-10  7:03 UTC (permalink / raw)
  To: devel
  Cc: Aaron Antone, Jian J Wang, Ray Ni, Star Zeng, Liming Gao,
	Sean Brogan, Michael Turner, Bret Barkelew

From: Aaron Antone <aanton@microsoft.com>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1416

After ExitBootServices, some pointer would be invalid such as
the Protocol pointer and gBS. The function depend on those should
be prevent. So disable the related function while after
ExitBootServices.
Change the gST to a internal one, because there will be a cycle
consume between UefiBootServicesTableLib and DebugLib due to the
library constructors.
Also remove the SMM support for this instance.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Turner <Michael.Turner@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
---
 MdePkg/Library/UefiDebugLibStdErr/DebugLib.c  | 113 +++++++++---------
 .../UefiDebugLibStdErr/DebugLibConstructor.c  |  72 +++++++++++
 .../UefiDebugLibStdErr/UefiDebugLibStdErr.inf |  12 +-
 3 files changed, 140 insertions(+), 57 deletions(-)
 create mode 100644 MdePkg/Library/UefiDebugLibStdErr/DebugLibConstructor.c

diff --git a/MdePkg/Library/UefiDebugLibStdErr/DebugLib.c b/MdePkg/Library/UefiDebugLibStdErr/DebugLib.c
index 29f93cf3e3..40eb697e7e 100644
--- a/MdePkg/Library/UefiDebugLibStdErr/DebugLib.c
+++ b/MdePkg/Library/UefiDebugLibStdErr/DebugLib.c
@@ -10,7 +10,6 @@
 #include <Uefi.h>
 
 #include <Library/DebugLib.h>
-#include <Library/UefiBootServicesTableLib.h>
 #include <Library/PrintLib.h>
 #include <Library/PcdLib.h>
 #include <Library/BaseLib.h>
@@ -29,6 +28,8 @@
 //
 VA_LIST     mVaListNull;
 
+extern BOOLEAN                mPostEBS;
+extern EFI_SYSTEM_TABLE       *mDebugST;
 
 /**
   Prints a debug message to the debug output device if the specified error level is enabled.
@@ -88,32 +89,34 @@ DebugPrintMarker (
 {
   CHAR16   Buffer[MAX_DEBUG_MESSAGE_LENGTH];
 
-  //
-  // If Format is NULL, then ASSERT().
-  //
-  ASSERT (Format != NULL);
-
-  //
-  // Check driver debug mask value and global mask
-  //
-  if ((ErrorLevel & GetDebugPrintErrorLevel ()) == 0) {
-    return;
-  }
-
-  //
-  // Convert the DEBUG() message to a Unicode String
-  //
-  if (BaseListMarker == NULL) {
-    UnicodeVSPrintAsciiFormat (Buffer, MAX_DEBUG_MESSAGE_LENGTH, Format, VaListMarker);
-  } else {
-    UnicodeBSPrintAsciiFormat (Buffer, MAX_DEBUG_MESSAGE_LENGTH, Format, BaseListMarker);
-  }
-
-  //
-  // Send the print string to the Standard Error device
-  //
-  if ((gST != NULL) && (gST->StdErr != NULL)) {
-    gST->StdErr->OutputString (gST->StdErr, Buffer);
+  if (!mPostEBS) {
+    //
+    // If Format is NULL, then ASSERT().
+    //
+    ASSERT (Format != NULL);
+
+    //
+    // Check driver debug mask value and global mask
+    //
+    if ((ErrorLevel & GetDebugPrintErrorLevel ()) == 0) {
+      return;
+    }
+
+    //
+    // Convert the DEBUG() message to a Unicode String
+    //
+    if (BaseListMarker == NULL) {
+      UnicodeVSPrintAsciiFormat (Buffer, MAX_DEBUG_MESSAGE_LENGTH, Format, VaListMarker);
+    } else {
+      UnicodeBSPrintAsciiFormat (Buffer, MAX_DEBUG_MESSAGE_LENGTH, Format, BaseListMarker);
+    }
+
+    //
+    // Send the print string to the Standard Error device
+    //
+    if ((mDebugST != NULL) && (mDebugST->StdErr != NULL)) {
+      mDebugST->StdErr->OutputString (mDebugST->StdErr, Buffer);
+    }
   }
 }
 
@@ -207,33 +210,35 @@ DebugAssert (
 {
   CHAR16  Buffer[MAX_DEBUG_MESSAGE_LENGTH];
 
-  //
-  // Generate the ASSERT() message in Unicode format
-  //
-  UnicodeSPrintAsciiFormat (
-    Buffer,
-    sizeof (Buffer),
-    "ASSERT [%a] %a(%d): %a\n",
-    gEfiCallerBaseName,
-    FileName,
-    LineNumber,
-    Description
-    );
-
-  //
-  // Send the print string to the Standard Error device
-  //
-  if ((gST != NULL) && (gST->StdErr != NULL)) {
-    gST->StdErr->OutputString (gST->StdErr, Buffer);
-  }
-
-  //
-  // Generate a Breakpoint, DeadLoop, or NOP based on PCD settings
-  //
-  if ((PcdGet8(PcdDebugPropertyMask) & DEBUG_PROPERTY_ASSERT_BREAKPOINT_ENABLED) != 0) {
-    CpuBreakpoint ();
-  } else if ((PcdGet8(PcdDebugPropertyMask) & DEBUG_PROPERTY_ASSERT_DEADLOOP_ENABLED) != 0) {
-    CpuDeadLoop ();
+  if (!mPostEBS) {
+    //
+    // Generate the ASSERT() message in Unicode format
+    //
+    UnicodeSPrintAsciiFormat (
+      Buffer,
+      sizeof (Buffer),
+      "ASSERT [%a] %a(%d): %a\n",
+      gEfiCallerBaseName,
+      FileName,
+      LineNumber,
+      Description
+      );
+
+    //
+    // Send the print string to the Standard Error device
+    //
+    if ((mDebugST != NULL) && (mDebugST->StdErr != NULL)) {
+      mDebugST->StdErr->OutputString (mDebugST->StdErr, Buffer);
+    }
+
+    //
+    // Generate a Breakpoint, DeadLoop, or NOP based on PCD settings
+    //
+    if ((PcdGet8(PcdDebugPropertyMask) & DEBUG_PROPERTY_ASSERT_BREAKPOINT_ENABLED) != 0) {
+      CpuBreakpoint ();
+    } else if ((PcdGet8(PcdDebugPropertyMask) & DEBUG_PROPERTY_ASSERT_DEADLOOP_ENABLED) != 0) {
+      CpuDeadLoop ();
+    }
   }
 }
 
diff --git a/MdePkg/Library/UefiDebugLibStdErr/DebugLibConstructor.c b/MdePkg/Library/UefiDebugLibStdErr/DebugLibConstructor.c
new file mode 100644
index 0000000000..48a761a71e
--- /dev/null
+++ b/MdePkg/Library/UefiDebugLibStdErr/DebugLibConstructor.c
@@ -0,0 +1,72 @@
+/** @file
+  UEFI Dxe DebugLib constructor that prevent some debug service after ExitBootServices event,
+  because some pointer is nulled at that phase.
+
+  Copyright (c) 2018, Microsoft Corporation
+  Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.<BR>
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include <Uefi.h>
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+
+//
+// BOOLEAN value to indicate if it is at the post ExitBootServices pahse
+//
+BOOLEAN     mPostEBS = FALSE;
+
+EFI_EVENT   mExitBootServicesEvent;
+
+//
+// Pointer to SystemTable
+// This library instance may have a cycle consume with UefiBootServicesTableLib
+// because of the constructors.
+//
+EFI_SYSTEM_TABLE      *mDebugST;
+
+/**
+  This routine sets the mPostEBS for exit boot servies true
+  to prevent DebugPort protocol dereferences when the pointer is nulled
+**/
+VOID
+EFIAPI
+ExitBootServicesCallback (
+  EFI_EVENT   Event,
+  VOID*       Context
+  )
+{
+  mPostEBS = TRUE;
+  return;
+}
+
+/**
+  The constructor gets the pointers to the system table and boot services table
+
+  @param  ImageHandle     The firmware allocated handle for the EFI image.
+  @param  SystemTable     A pointer to the EFI System Table.
+
+  @retval EFI_SUCCESS     The constructor always returns EFI_SUCCESS.
+
+**/
+EFI_STATUS
+EFIAPI
+DxeDebugLibConstructor(
+  IN EFI_HANDLE                 ImageHandle,
+  IN EFI_SYSTEM_TABLE           *SystemTable
+  )
+{
+  mDebugST = SystemTable;
+
+  SystemTable->BootServices->CreateEventEx (
+                                EVT_NOTIFY_SIGNAL,
+                                TPL_NOTIFY,
+                                ExitBootServicesCallback,
+                                NULL,
+                                &gEfiEventExitBootServicesGuid,
+                                &mExitBootServicesEvent
+                                );
+
+  return EFI_SUCCESS;
+}
diff --git a/MdePkg/Library/UefiDebugLibStdErr/UefiDebugLibStdErr.inf b/MdePkg/Library/UefiDebugLibStdErr/UefiDebugLibStdErr.inf
index 865cc98753..deaa3427f6 100644
--- a/MdePkg/Library/UefiDebugLibStdErr/UefiDebugLibStdErr.inf
+++ b/MdePkg/Library/UefiDebugLibStdErr/UefiDebugLibStdErr.inf
@@ -3,7 +3,9 @@
 #
 #  Debug Lib that sends messages to the the Standard Error Device in the EFI System Table.
 #
-#  Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2018, Microsoft Corporation
+#
+#  Copyright (c) 2007 - 2019, Intel Corporation. All rights reserved.<BR>
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -17,8 +19,9 @@
   FILE_GUID                      = b57a1df6-ffdb-4247-a3df-3a562176751a
   MODULE_TYPE                    = UEFI_DRIVER
   VERSION_STRING                 = 1.0
-  LIBRARY_CLASS                  = DebugLib|DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SMM_DRIVER UEFI_APPLICATION UEFI_DRIVER
+  LIBRARY_CLASS                  = DebugLib|DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER UEFI_APPLICATION UEFI_DRIVER
 
+  CONSTRUCTOR                    = DxeDebugLibConstructor
 
 #
 #  VALID_ARCHITECTURES           = IA32 X64 EBC
@@ -26,6 +29,7 @@
 
 [Sources]
   DebugLib.c
+  DebugLibConstructor.c
 
 
 [Packages]
@@ -37,9 +41,11 @@
   BaseLib
   PcdLib
   PrintLib
-  UefiBootServicesTableLib
   DebugPrintErrorLevelLib
 
+[Guids]
+  gEfiEventExitBootServicesGuid                 ## CONSUMES
+
 [Pcd]
   gEfiMdePkgTokenSpaceGuid.PcdDebugClearMemoryValue   ## SOMETIMES_CONSUMES
   gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask       ## CONSUMES
-- 
2.21.0.windows.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [edk2-devel] [PATCH 0/5] Make some DebugLib instance runtime safe
  2019-04-10  7:03 [PATCH 0/5] Make some DebugLib instance runtime safe Gao, Zhichao
                   ` (4 preceding siblings ...)
  2019-04-10  7:03 ` [PATCH 5/5] MdeModule/UefiDebugLibStdErr: " Gao, Zhichao
@ 2019-04-11  8:39 ` Wu, Hao A
  5 siblings, 0 replies; 9+ messages in thread
From: Wu, Hao A @ 2019-04-11  8:39 UTC (permalink / raw)
  To: devel@edk2.groups.io, Gao, Zhichao
  Cc: Yao, Jiewen, Zhang, Chao B, Wang, Jian J, Ni, Ray, Zeng, Star,
	Gao, Liming, Sean Brogan, Michael Turner, Bret Barkelew

> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Gao, Zhichao
> Sent: Wednesday, April 10, 2019 3:03 PM
> To: devel@edk2.groups.io
> Cc: Yao, Jiewen; Zhang, Chao B; Wang, Jian J; Ni, Ray; Zeng, Star; Gao, Liming;
> Sean Brogan; Michael Turner; Bret Barkelew
> Subject: [edk2-devel] [PATCH 0/5] Make some DebugLib instance runtime
> safe
> 
> Some pointer will be invalid after ExitBootServices, such as protocol pointer,
> gST, gBS and so on. Disable the functions which used that pointer.
> 
> Remove SMM support of UefiDebugLibDebugPortProtocol,
> UefidebugLibConOut
> and UefiDebugLibStdErr.
> Before removing, should change the SMM instance in the dsc file which use
> them as SMM instance. And null version is safe.
> 
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Chao Zhang <chao.b.zhang@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Sean Brogan <sean.brogan@microsoft.com>
> Cc: Michael Turner <Michael.Turner@microsoft.com>
> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> 
> Aaron Antone (3):
>   MdeModulePkg/UefiDebugLibDebugPortProtocol: Make it runtime safe
>   MdeModulePkg/UefidebugLibConOut: Make it runtime safe
>   MdeModule/UefiDebugLibStdErr: Make it runtime safe

Seems some typos here.
Should be MdePkg instead of MdeModulePkg (MdeModule).

Best Regards,
Hao Wu

> 
> Zhichao Gao (2):
>   MdeModulePkg: Change the SMM debug lib instance
>   SignedCapsulePkg: Change the SMM debug lib instance
> 
>  MdeModulePkg/MdeModulePkg.dsc                 |   2 +-
>  MdePkg/Library/UefiDebugLibConOut/DebugLib.c  | 116 +++++++-------
>  .../UefiDebugLibConOut/DebugLibConstructor.c  |  72 +++++++++
>  .../UefiDebugLibConOut/UefiDebugLibConOut.inf |  12 +-
>  .../UefiDebugLibDebugPortProtocol/DebugLib.c  | 146 +++++++++---------
>  .../DebugLibConstructor.c                     |  72 +++++++++
>  .../UefiDebugLibDebugPortProtocol.inf         |  12 +-
>  MdePkg/Library/UefiDebugLibStdErr/DebugLib.c  | 113 +++++++-------
>  .../UefiDebugLibStdErr/DebugLibConstructor.c  |  72 +++++++++
>  .../UefiDebugLibStdErr/UefiDebugLibStdErr.inf |  12 +-
>  SignedCapsulePkg/SignedCapsulePkg.dsc         |   4 +-
>  11 files changed, 443 insertions(+), 190 deletions(-)
>  create mode 100644
> MdePkg/Library/UefiDebugLibConOut/DebugLibConstructor.c
>  create mode 100644
> MdePkg/Library/UefiDebugLibDebugPortProtocol/DebugLibConstructor.c
>  create mode 100644
> MdePkg/Library/UefiDebugLibStdErr/DebugLibConstructor.c
> 
> --
> 2.21.0.windows.1
> 
> 
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [edk2-devel] [PATCH 1/5] MdeModulePkg: Change the SMM debug lib instance
  2019-04-10  7:03 ` [PATCH 1/5] MdeModulePkg: Change the SMM debug lib instance Gao, Zhichao
@ 2019-04-11  8:44   ` Wu, Hao A
  2019-04-11  9:28     ` Gao, Zhichao
  0 siblings, 1 reply; 9+ messages in thread
From: Wu, Hao A @ 2019-04-11  8:44 UTC (permalink / raw)
  To: devel@edk2.groups.io, Gao, Zhichao
  Cc: Wang, Jian J, Ni, Ray, Zeng, Star, Gao, Liming, Sean Brogan,
	Michael Turner, Bret Barkelew

> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Gao, Zhichao
> Sent: Wednesday, April 10, 2019 3:03 PM
> To: devel@edk2.groups.io
> Cc: Wang, Jian J; Ni, Ray; Zeng, Star; Gao, Liming; Sean Brogan; Michael
> Turner; Bret Barkelew
> Subject: [edk2-devel] [PATCH 1/5] MdeModulePkg: Change the SMM debug
> lib instance
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1416
> 
> The UefiDebugLibConOut will not support DXE_SMM_DRIVER,
> change UefiDebugLibConOut to PeiDxeDebugLibReportStatusCode.

Is this a typo?
Should be 'BaseDebugLibNull' according to your change.

With this addressed,
Reviewed-by: Hao Wu <hao.a.wu@intel.com>


Best Regards,
Hao Wu

> 
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Sean Brogan <sean.brogan@microsoft.com>
> Cc: Michael Turner <Michael.Turner@microsoft.com>
> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
> ---
>  MdeModulePkg/MdeModulePkg.dsc | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/MdeModulePkg.dsc
> b/MdeModulePkg/MdeModulePkg.dsc
> index 7ef39af847..0da28e74c2 100644
> --- a/MdeModulePkg/MdeModulePkg.dsc
> +++ b/MdeModulePkg/MdeModulePkg.dsc
> @@ -144,7 +144,7 @@
> 
>  [LibraryClasses.common.DXE_SMM_DRIVER]
>    HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
> -  DebugLib|MdePkg/Library/UefiDebugLibConOut/UefiDebugLibConOut.inf
> +  DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf
> 
> MemoryAllocationLib|MdePkg/Library/SmmMemoryAllocationLib/SmmMe
> moryAllocationLib.inf
> 
> MmServicesTableLib|MdePkg/Library/MmServicesTableLib/MmServicesTabl
> eLib.inf
> 
> SmmServicesTableLib|MdePkg/Library/SmmServicesTableLib/SmmServicesT
> ableLib.inf
> --
> 2.21.0.windows.1
> 
> 
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [edk2-devel] [PATCH 1/5] MdeModulePkg: Change the SMM debug lib instance
  2019-04-11  8:44   ` [edk2-devel] " Wu, Hao A
@ 2019-04-11  9:28     ` Gao, Zhichao
  0 siblings, 0 replies; 9+ messages in thread
From: Gao, Zhichao @ 2019-04-11  9:28 UTC (permalink / raw)
  To: Wu, Hao A, devel@edk2.groups.io
  Cc: Wang, Jian J, Ni, Ray, Zeng, Star, Gao, Liming, Sean Brogan,
	Michael Turner, Bret Barkelew

Sorry to forget change the commit message. I originally want to use PeiDxeDebugLibReportStatusCode to replace the UefiDebugLibConOut.
There may be some typos in other patches in these series. And there is also a comment conflicted with Doxygen rule . I would check and fix them.

Thanks,
Zhichao

> -----Original Message-----
> From: Wu, Hao A
> Sent: Thursday, April 11, 2019 4:45 PM
> To: devel@edk2.groups.io; Gao, Zhichao <zhichao.gao@intel.com>
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Ni, Ray <ray.ni@intel.com>; Zeng,
> Star <star.zeng@intel.com>; Gao, Liming <liming.gao@intel.com>; Sean
> Brogan <sean.brogan@microsoft.com>; Michael Turner
> <Michael.Turner@microsoft.com>; Bret Barkelew
> <Bret.Barkelew@microsoft.com>
> Subject: RE: [edk2-devel] [PATCH 1/5] MdeModulePkg: Change the SMM
> debug lib instance
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> > Gao, Zhichao
> > Sent: Wednesday, April 10, 2019 3:03 PM
> > To: devel@edk2.groups.io
> > Cc: Wang, Jian J; Ni, Ray; Zeng, Star; Gao, Liming; Sean Brogan;
> > Michael Turner; Bret Barkelew
> > Subject: [edk2-devel] [PATCH 1/5] MdeModulePkg: Change the SMM
> debug
> > lib instance
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1416
> >
> > The UefiDebugLibConOut will not support DXE_SMM_DRIVER, change
> > UefiDebugLibConOut to PeiDxeDebugLibReportStatusCode.
> 
> Is this a typo?
> Should be 'BaseDebugLibNull' according to your change.
> 
> With this addressed,
> Reviewed-by: Hao Wu <hao.a.wu@intel.com>
> 
> 
> Best Regards,
> Hao Wu
> 
> >
> > Cc: Jian J Wang <jian.j.wang@intel.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Cc: Star Zeng <star.zeng@intel.com>
> > Cc: Liming Gao <liming.gao@intel.com>
> > Cc: Sean Brogan <sean.brogan@microsoft.com>
> > Cc: Michael Turner <Michael.Turner@microsoft.com>
> > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> > Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
> > ---
> >  MdeModulePkg/MdeModulePkg.dsc | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/MdeModulePkg/MdeModulePkg.dsc
> > b/MdeModulePkg/MdeModulePkg.dsc index 7ef39af847..0da28e74c2
> 100644
> > --- a/MdeModulePkg/MdeModulePkg.dsc
> > +++ b/MdeModulePkg/MdeModulePkg.dsc
> > @@ -144,7 +144,7 @@
> >
> >  [LibraryClasses.common.DXE_SMM_DRIVER]
> >    HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
> > -
> DebugLib|MdePkg/Library/UefiDebugLibConOut/UefiDebugLibConOut.inf
> > +  DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf
> >
> >
> MemoryAllocationLib|MdePkg/Library/SmmMemoryAllocationLib/SmmMe
> > moryAllocationLib.inf
> >
> >
> MmServicesTableLib|MdePkg/Library/MmServicesTableLib/MmServicesTabl
> > eLib.inf
> >
> >
> SmmServicesTableLib|MdePkg/Library/SmmServicesTableLib/SmmServicesT
> > ableLib.inf
> > --
> > 2.21.0.windows.1
> >
> >
> > 


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2019-04-11  9:29 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-04-10  7:03 [PATCH 0/5] Make some DebugLib instance runtime safe Gao, Zhichao
2019-04-10  7:03 ` [PATCH 1/5] MdeModulePkg: Change the SMM debug lib instance Gao, Zhichao
2019-04-11  8:44   ` [edk2-devel] " Wu, Hao A
2019-04-11  9:28     ` Gao, Zhichao
2019-04-10  7:03 ` [PATCH 2/5] SignedCapsulePkg: " Gao, Zhichao
2019-04-10  7:03 ` [PATCH 3/5] MdeModulePkg/UefiDebugLibDebugPortProtocol: Make it runtime safe Gao, Zhichao
2019-04-10  7:03 ` [PATCH 4/5] MdeModulePkg/UefidebugLibConOut: " Gao, Zhichao
2019-04-10  7:03 ` [PATCH 5/5] MdeModule/UefiDebugLibStdErr: " Gao, Zhichao
2019-04-11  8:39 ` [edk2-devel] [PATCH 0/5] Make some DebugLib instance " Wu, Hao A

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox