public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Gao, Zhichao" <zhichao.gao@intel.com>
To: devel@edk2.groups.io
Cc: Aaron Antone <aanton@microsoft.com>,
	Michael D Kinney <michael.d.kinney@intel.com>,
	Liming Gao <liming.gao@intel.com>,
	Sean Brogan <sean.brogan@microsoft.com>,
	Michael Turner <Michael.Turner@microsoft.com>,
	Bret Barkelew <Bret.Barkelew@microsoft.com>
Subject: [PATCH V2 3/5] MdePkg/UefiDebugLibDebugPortProtocol: Make it runtime safe
Date: Fri, 12 Apr 2019 08:42:29 +0800	[thread overview]
Message-ID: <20190412004231.17280-4-zhichao.gao@intel.com> (raw)
In-Reply-To: <20190412004231.17280-1-zhichao.gao@intel.com>

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: Michael D Kinney <michael.d.kinney@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                     |  77 +++++++++
 .../UefiDebugLibDebugPortProtocol.inf         |  12 +-
 3 files changed, 163 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..ed2cb70c21
--- /dev/null
+++ b/MdePkg/Library/UefiDebugLibDebugPortProtocol/DebugLibConstructor.c
@@ -0,0 +1,77 @@
+/** @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.
+
+  @param  Event        Event whose notification function is being invoked.
+  @param  Context      Pointer to the notification function's context.
+
+**/
+VOID
+EFIAPI
+ExitBootServicesCallback (
+  EFI_EVENT   Event,
+  VOID*       Context
+  )
+{
+  mPostEBS = TRUE;
+  return;
+}
+
+/**
+  The constructor gets the pointers to boot services table.
+  And create a event to indicate it is after ExitBootServices.
+
+  @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


  parent reply	other threads:[~2019-04-12  0:42 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-12  0:42 [PATCH V2 0/5] Make some DebugLib instance runtime safe Gao, Zhichao
2019-04-12  0:42 ` [PATCH V2 1/5] MdeModulePkg: Change the SMM debug lib instance Gao, Zhichao
2019-04-12  0:42 ` [PATCH V2 2/5] SignedCapsulePkg: " Gao, Zhichao
2019-04-12  0:42 ` Gao, Zhichao [this message]
2019-04-12  0:42 ` [PATCH V2 4/5] MdePkg/UefidebugLibConOut: Make it runtime safe Gao, Zhichao
2019-04-12  0:42 ` [PATCH V2 5/5] MdePkg/UefiDebugLibStdErr: " Gao, Zhichao
2019-04-17  1:24 ` [PATCH V2 0/5] Make some DebugLib instance " Liming Gao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190412004231.17280-4-zhichao.gao@intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox