public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH edk2-platforms v2 0/2] Create UART DebugLib implementation for runtime drivers
@ 2018-02-22 16:28 Ard Biesheuvel
  2018-02-22 16:28 ` [PATCH edk2-platforms v2 1/2] MdePkg: introduce DxeRuntimeDebugLibSerialPort Ard Biesheuvel
  2018-02-22 16:28 ` [PATCH edk2-platforms v2 2/2] ArmVirtPkg: switch to DXE runtime version of DebugLib where appropriate Ard Biesheuvel
  0 siblings, 2 replies; 8+ messages in thread
From: Ard Biesheuvel @ 2018-02-22 16:28 UTC (permalink / raw)
  To: edk2-devel
  Cc: leif.lindholm, lersek, liming.gao, michael.d.kinney, afish,
	star.zeng, ruiyu.ni, Ard Biesheuvel

Commit 4bf95a9f361e ("MdeModulePkg/ResetSystemRuntimeDxe: Add more debug
message") broke the DEBUG build for systems using a MMIO mapped UART for
DEBUG output. In other words, it broke the build for all ARM and AARCH64
systems, given that port I/O does not exist on those architectures.

Instead of patching it up locally, let's fix this issue once and for all,
by creating a UART DebugLib implementation for DXE_RUNTIME_DRIVER modules
that does the right thing by default.

v2:
- incorporate Laszlo's feedback into patch #1
- add Laszlo's R-b to patch #2
- dropped patch #3 that blacklists the original BASE implementation for
  DXE_RUNTIME_DRIVER modules

Note that I retained the deadloop/breakpoint code for ASSERT()s at runtime.
I think it is perfectly reasonable behavior for a DEBUG build at runtime,
even if other cores may be up and running as well: the purpose of these
facilities is to allow a debugger to attach to the CPU to figure out what
has happened, and both deadloops and breakpoints can achieve that just
fine even at runtime.

Ard Biesheuvel (2):
  MdePkg: introduce DxeRuntimeDebugLibSerialPort
  ArmVirtPkg: switch to DXE runtime version of DebugLib where
    appropriate

 ArmVirtPkg/ArmVirt.dsc.inc                                                   |   3 +
 MdePkg/Library/DxeRuntimeDebugLibSerialPort/DebugLib.c                       | 373 ++++++++++++++++++++
 MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.inf |  56 +++
 MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.uni |  21 ++
 4 files changed, 453 insertions(+)
 create mode 100644 MdePkg/Library/DxeRuntimeDebugLibSerialPort/DebugLib.c
 create mode 100644 MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.inf
 create mode 100644 MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.uni

-- 
2.11.0



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

* [PATCH edk2-platforms v2 1/2] MdePkg: introduce DxeRuntimeDebugLibSerialPort
  2018-02-22 16:28 [PATCH edk2-platforms v2 0/2] Create UART DebugLib implementation for runtime drivers Ard Biesheuvel
@ 2018-02-22 16:28 ` Ard Biesheuvel
  2018-02-22 17:45   ` Ard Biesheuvel
                     ` (2 more replies)
  2018-02-22 16:28 ` [PATCH edk2-platforms v2 2/2] ArmVirtPkg: switch to DXE runtime version of DebugLib where appropriate Ard Biesheuvel
  1 sibling, 3 replies; 8+ messages in thread
From: Ard Biesheuvel @ 2018-02-22 16:28 UTC (permalink / raw)
  To: edk2-devel
  Cc: leif.lindholm, lersek, liming.gao, michael.d.kinney, afish,
	star.zeng, ruiyu.ni, Ard Biesheuvel

Introduce a variant of BaseDebugLibSerialPort that behaves correctly with
regards to the use of the serial port after ExitBootServices(). Also, it
caches PCD values in the constructor so that no calls into PcdLib are
made at runtime.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 MdePkg/Library/DxeRuntimeDebugLibSerialPort/DebugLib.c                       | 373 ++++++++++++++++++++
 MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.inf |  56 +++
 MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.uni |  21 ++
 3 files changed, 450 insertions(+)

diff --git a/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DebugLib.c b/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DebugLib.c
new file mode 100644
index 000000000000..8579469029a1
--- /dev/null
+++ b/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DebugLib.c
@@ -0,0 +1,373 @@
+/** @file
+  DXE runtime Debug library instance based on Serial Port library.
+  It takes care not to call into SerialPortLib or PcdLib after
+  ExitBootServices() has been called, to prevent touching hardware that is
+  no longer owned by the firmware, or invoking protocols that are not safe
+  for runtime (such as gEfiPcdProtocolGuid)
+
+  Copyright (c) 2006 - 2011, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2018, Linaro, Ltd. All rights reserved.<BR>
+
+  This program and the accompanying materials
+  are licensed and made available under the terms and conditions of the BSD License
+  which accompanies this distribution.  The full text of the license may be found at
+  http://opensource.org/licenses/bsd-license.php.
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include <Base.h>
+#include <Library/DebugLib.h>
+#include <Library/DebugPrintErrorLevelLib.h>
+#include <Library/BaseLib.h>
+#include <Library/PrintLib.h>
+#include <Library/PcdLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/SerialPortLib.h>
+
+STATIC EFI_EVENT      mEfiExitBootServicesEvent;
+STATIC BOOLEAN        mEfiAtRuntime;
+
+STATIC UINT8          mDebugClearMemoryValue;
+STATIC UINT8          mDebugPropertyMask;
+STATIC UINT32         mFixedDebugPrintErrorLevel;
+
+//
+// Define the maximum debug and assert message length that this library supports
+//
+#define MAX_DEBUG_MESSAGE_LENGTH  0x100
+
+/**
+  Set AtRuntime flag as TRUE after ExitBootServices.
+
+  @param[in]  Event   The Event that is being processed.
+  @param[in]  Context The Event Context.
+
+**/
+STATIC
+VOID
+EFIAPI
+ExitBootServicesEvent (
+  IN EFI_EVENT        Event,
+  IN VOID             *Context
+  )
+{
+  mEfiAtRuntime = TRUE;
+}
+
+/**
+  The constructor function to initialize the Serial Port library and
+  register a callback for the ExitBootServices event.
+
+  @param[in]  ImageHandle   The firmware allocated handle for the EFI image.
+  @param[in]  SystemTable   A pointer to the EFI System Table.
+
+  @retval EFI_SUCCESS   The operation completed successfully.
+  @retval other         Either the serial port failed to initialize or the
+                        ExitBootServices event callback registration failed.
+**/
+EFI_STATUS
+EFIAPI
+DxeRuntimeDebugLibSerialPortConstructor (
+  IN EFI_HANDLE        ImageHandle,
+  IN EFI_SYSTEM_TABLE  *SystemTable
+  )
+{
+  EFI_STATUS    Status;
+
+  mDebugClearMemoryValue      = PcdGet8 (PcdDebugClearMemoryValue);
+  mDebugPropertyMask          = PcdGet8 (PcdDebugPropertyMask);
+  mFixedDebugPrintErrorLevel  = PcdGet32 (PcdFixedDebugPrintErrorLevel);
+
+  Status = SerialPortInitialize ();
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  return SystemTable->BootServices->CreateEventEx (EVT_NOTIFY_SIGNAL,
+                                      TPL_NOTIFY, ExitBootServicesEvent, NULL,
+                                      &gEfiEventExitBootServicesGuid,
+                                      &mEfiExitBootServicesEvent);
+}
+
+/**
+  If a runtime driver exits with an error, it must call this routine
+  to free the allocated resource before the exiting.
+
+  @param[in]  ImageHandle   The firmware allocated handle for the EFI image.
+  @param[in]  SystemTable   A pointer to the EFI System Table.
+
+  @retval     EFI_SUCCESS       The Runtime Driver Lib shutdown successfully.
+  @retval     EFI_UNSUPPORTED   Runtime Driver lib was not initialized.
+**/
+EFI_STATUS
+EFIAPI
+DxeRuntimeDebugLibSerialPortDestructor (
+  IN EFI_HANDLE        ImageHandle,
+  IN EFI_SYSTEM_TABLE  *SystemTable
+  )
+{
+  return SystemTable->BootServices->CloseEvent (mEfiExitBootServicesEvent);
+}
+
+/**
+  Prints a debug message to the debug output device if the specified error level
+  is enabled.
+
+  If any bit in ErrorLevel is also set in DebugPrintErrorLevelLib function
+  GetDebugPrintErrorLevel (), then print the message specified by Format and the
+  associated variable argument list to the debug output device.
+
+  If Format is NULL, then ASSERT().
+
+  @param  ErrorLevel  The error level of the debug message.
+  @param  Format      Format string for the debug message to print.
+  @param  ...         Variable argument list whose contents are accessed
+                      based on the format string specified by Format.
+
+**/
+VOID
+EFIAPI
+DebugPrint (
+  IN  UINTN        ErrorLevel,
+  IN  CONST CHAR8  *Format,
+  ...
+  )
+{
+  CHAR8    Buffer[MAX_DEBUG_MESSAGE_LENGTH];
+  VA_LIST  Marker;
+
+  if (mEfiAtRuntime) {
+    return;
+  }
+
+  ASSERT (Format != NULL);
+
+  //
+  // Check driver debug mask value and global mask
+  //
+  if ((ErrorLevel & GetDebugPrintErrorLevel ()) == 0) {
+    return;
+  }
+
+  //
+  // Convert the DEBUG() message to an ASCII String
+  //
+  VA_START (Marker, Format);
+  AsciiVSPrint (Buffer, sizeof (Buffer), Format, Marker);
+  VA_END (Marker);
+
+  //
+  // Send the print string to a Serial Port
+  //
+  SerialPortWrite ((UINT8 *)Buffer, AsciiStrLen (Buffer));
+}
+
+
+/**
+  Prints an assert message containing a filename, line number, and description.
+  This may be followed by a breakpoint or a dead loop.
+
+  Print a message of the form "ASSERT <FileName>(<LineNumber>): <Description>\n"
+  to the debug output device.  If DEBUG_PROPERTY_ASSERT_BREAKPOINT_ENABLED bit
+  of PcdDebugProperyMask is set then CpuBreakpoint() is called. Otherwise, if
+  DEBUG_PROPERTY_ASSERT_DEADLOOP_ENABLED bit of PcdDebugProperyMask is set then
+  CpuDeadLoop() is called.  If neither of these bits are set, then this function
+  returns immediately after the message is printed to the debug output device.
+  DebugAssert() must actively prevent recursion.  If DebugAssert() is called
+  while processing another DebugAssert(), then DebugAssert() must return
+  immediately.
+
+  If FileName is NULL, then a <FileName> string of "(NULL) Filename" is printed.
+  If Description is NULL, then a <Description> string of "(NULL) Description" is
+  printed.
+
+  @param  FileName     The pointer to the name of the source file that generated
+                       the assert condition.
+  @param  LineNumber   The line number in the source file that generated the
+                       assert condition
+  @param  Description  The pointer to the description of the assert condition.
+
+**/
+VOID
+EFIAPI
+DebugAssert (
+  IN CONST CHAR8  *FileName,
+  IN UINTN        LineNumber,
+  IN CONST CHAR8  *Description
+  )
+{
+  CHAR8  Buffer[MAX_DEBUG_MESSAGE_LENGTH];
+
+  if (!mEfiAtRuntime) {
+    //
+    // 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 the Console Output device
+    //
+    SerialPortWrite ((UINT8 *)Buffer, AsciiStrLen (Buffer));
+  }
+
+  //
+  // Generate a Breakpoint, DeadLoop, or NOP based on PCD settings
+  //
+  if ((mDebugPropertyMask & DEBUG_PROPERTY_ASSERT_BREAKPOINT_ENABLED) != 0) {
+    CpuBreakpoint ();
+  } else if ((mDebugPropertyMask &
+              DEBUG_PROPERTY_ASSERT_DEADLOOP_ENABLED) != 0) {
+    CpuDeadLoop ();
+  }
+}
+
+
+/**
+  Fills a target buffer with PcdDebugClearMemoryValue, and returns the target
+  buffer.
+
+  This function fills Length bytes of Buffer with the value specified by
+  PcdDebugClearMemoryValue, and returns Buffer.
+
+  If Buffer is NULL, then ASSERT().
+  If Length is greater than (MAX_ADDRESS - Buffer + 1), then ASSERT().
+
+  @param   Buffer  The pointer to the target buffer to be filled with
+                   PcdDebugClearMemoryValue.
+  @param   Length  The number of bytes in Buffer to fill with
+                   PcdDebugClearMemoryValue.
+
+  @return  Buffer  The pointer to the target buffer filled with
+                   PcdDebugClearMemoryValue.
+
+**/
+VOID *
+EFIAPI
+DebugClearMemory (
+  OUT VOID  *Buffer,
+  IN UINTN  Length
+  )
+{
+  if (!mEfiAtRuntime) {
+    ASSERT (Buffer != NULL);
+    ASSERT (Length <= (MAX_ADDRESS - (UINTN)Buffer + 1));
+  }
+
+  return SetMem (Buffer, Length, mDebugClearMemoryValue);
+}
+
+
+/**
+  Returns TRUE if ASSERT() macros are enabled.
+
+  This function returns TRUE if the DEBUG_PROPERTY_DEBUG_ASSERT_ENABLED bit of
+  PcdDebugProperyMask is set.  Otherwise FALSE is returned.
+
+  @retval  TRUE    The DEBUG_PROPERTY_DEBUG_ASSERT_ENABLED bit of
+                   PcdDebugProperyMask is set.
+  @retval  FALSE   The DEBUG_PROPERTY_DEBUG_ASSERT_ENABLED bit of
+                   PcdDebugProperyMask is clear.
+
+**/
+BOOLEAN
+EFIAPI
+DebugAssertEnabled (
+  VOID
+  )
+{
+  return (BOOLEAN)((mDebugPropertyMask &
+                    DEBUG_PROPERTY_DEBUG_ASSERT_ENABLED) != 0);
+}
+
+
+/**
+  Returns TRUE if DEBUG() macros are enabled.
+
+  This function returns TRUE if the DEBUG_PROPERTY_DEBUG_PRINT_ENABLED bit of
+  PcdDebugProperyMask is set.  Otherwise FALSE is returned.
+
+  @retval  TRUE    The DEBUG_PROPERTY_DEBUG_PRINT_ENABLED bit of
+                   PcdDebugProperyMask is set.
+  @retval  FALSE   The DEBUG_PROPERTY_DEBUG_PRINT_ENABLED bit of
+                   PcdDebugProperyMask is clear.
+
+**/
+BOOLEAN
+EFIAPI
+DebugPrintEnabled (
+  VOID
+  )
+{
+  return (BOOLEAN)((mDebugPropertyMask &
+                    DEBUG_PROPERTY_DEBUG_PRINT_ENABLED) != 0);
+}
+
+
+/**
+  Returns TRUE if DEBUG_CODE() macros are enabled.
+
+  This function returns TRUE if the DEBUG_PROPERTY_DEBUG_CODE_ENABLED bit of
+  PcdDebugProperyMask is set.  Otherwise FALSE is returned.
+
+  @retval  TRUE    The DEBUG_PROPERTY_DEBUG_CODE_ENABLED bit of
+                   PcdDebugProperyMask is set.
+  @retval  FALSE   The DEBUG_PROPERTY_DEBUG_CODE_ENABLED bit of
+                   PcdDebugProperyMask is clear.
+
+**/
+BOOLEAN
+EFIAPI
+DebugCodeEnabled (
+  VOID
+  )
+{
+  return (BOOLEAN)((mDebugPropertyMask &
+                    DEBUG_PROPERTY_DEBUG_CODE_ENABLED) != 0);
+}
+
+
+/**
+  Returns TRUE if DEBUG_CLEAR_MEMORY() macro is enabled.
+
+  This function returns TRUE if the DEBUG_PROPERTY_CLEAR_MEMORY_ENABLED bit of
+  PcdDebugProperyMask is set.  Otherwise FALSE is returned.
+
+  @retval  TRUE    The DEBUG_PROPERTY_CLEAR_MEMORY_ENABLED bit of
+                   PcdDebugProperyMask is set.
+  @retval  FALSE   The DEBUG_PROPERTY_CLEAR_MEMORY_ENABLED bit of
+                   PcdDebugProperyMask is clear.
+
+**/
+BOOLEAN
+EFIAPI
+DebugClearMemoryEnabled (
+  VOID
+  )
+{
+  return (BOOLEAN)((mDebugPropertyMask &
+                    DEBUG_PROPERTY_CLEAR_MEMORY_ENABLED) != 0);
+}
+
+/**
+  Returns TRUE if any one of the bit is set both in ErrorLevel and
+  PcdFixedDebugPrintErrorLevel.
+
+  This function compares the bit mask of ErrorLevel and
+  PcdFixedDebugPrintErrorLevel.
+
+  @retval  TRUE    Current ErrorLevel is supported.
+  @retval  FALSE   Current ErrorLevel is not supported.
+
+**/
+BOOLEAN
+EFIAPI
+DebugPrintLevelEnabled (
+  IN  CONST UINTN        ErrorLevel
+  )
+{
+  return (BOOLEAN)((ErrorLevel & mFixedDebugPrintErrorLevel) != 0);
+}
diff --git a/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.inf b/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.inf
new file mode 100644
index 000000000000..d5e9d3f8dee7
--- /dev/null
+++ b/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.inf
@@ -0,0 +1,56 @@
+## @file
+#  DXE runtime Debug library instance based on Serial Port library.
+#  It takes care not to call into SerialPortLib or PcdLib after
+#  ExitBootServices() has been called, to prevent touching hardware that is
+#  no longer owned by the firmware, or invoking protocols that are not safe
+#  for runtime (such as gEfiPcdProtocolGuid)
+#
+#  Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2018, Linaro, Ltd. All rights reserved.<BR>
+#
+#  This program and the accompanying materials
+#  are licensed and made available under the terms and conditions of the BSD License
+#  which accompanies this distribution. The full text of the license may be found at
+#  http://opensource.org/licenses/bsd-license.php.
+#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+#
+#
+##
+
+[Defines]
+  INF_VERSION                    = 0x0001001A
+  BASE_NAME                      = DxeRuntimeDebugLibSerialPort
+  MODULE_UNI_FILE                = DxeRuntimeDebugLibSerialPort.uni
+  FILE_GUID                      = 9D914E2F-7CCB-41DB-8E74-9AFF8F3BBFBF
+  MODULE_TYPE                    = DXE_RUNTIME_DRIVER
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = DebugLib|DXE_RUNTIME_DRIVER
+  CONSTRUCTOR                    = DxeRuntimeDebugLibSerialPortConstructor
+  DESTRUCTOR                     = DxeRuntimeDebugLibSerialPortDestructor
+
+#
+#  VALID_ARCHITECTURES           = AARCH64 ARM IA32 X64 IPF EBC
+#
+
+[Sources]
+  DebugLib.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+
+[LibraryClasses]
+  BaseLib
+  BaseMemoryLib
+  DebugPrintErrorLevelLib
+  PcdLib
+  PrintLib
+  SerialPortLib
+
+[Guids]
+  gEfiEventExitBootServicesGuid                         ## CONSUMES ## Event
+
+[Pcd]
+  gEfiMdePkgTokenSpaceGuid.PcdDebugClearMemoryValue     ## CONSUMES
+  gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask         ## CONSUMES
+  gEfiMdePkgTokenSpaceGuid.PcdFixedDebugPrintErrorLevel ## CONSUMES
diff --git a/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.uni b/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.uni
new file mode 100644
index 000000000000..cd65515c4177
--- /dev/null
+++ b/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.uni
@@ -0,0 +1,21 @@
+// /** @file
+// Instance of Debug Library based on Serial Port Library.
+//
+// It uses Print Library to produce formatted output strings to seiral port device.
+//
+// Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.<BR>
+//
+// This program and the accompanying materials
+// are licensed and made available under the terms and conditions of the BSD License
+// which accompanies this distribution. The full text of the license may be found at
+// http://opensource.org/licenses/bsd-license.php.
+// THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+// WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+//
+// **/
+
+
+#string STR_MODULE_ABSTRACT             #language en-US "Instance of Debug Library based on Serial Port Library"
+
+#string STR_MODULE_DESCRIPTION          #language en-US "It uses Print Library to produce formatted output strings to a serial port device."
+
-- 
2.11.0



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

* [PATCH edk2-platforms v2 2/2] ArmVirtPkg: switch to DXE runtime version of DebugLib where appropriate
  2018-02-22 16:28 [PATCH edk2-platforms v2 0/2] Create UART DebugLib implementation for runtime drivers Ard Biesheuvel
  2018-02-22 16:28 ` [PATCH edk2-platforms v2 1/2] MdePkg: introduce DxeRuntimeDebugLibSerialPort Ard Biesheuvel
@ 2018-02-22 16:28 ` Ard Biesheuvel
  1 sibling, 0 replies; 8+ messages in thread
From: Ard Biesheuvel @ 2018-02-22 16:28 UTC (permalink / raw)
  To: edk2-devel
  Cc: leif.lindholm, lersek, liming.gao, michael.d.kinney, afish,
	star.zeng, ruiyu.ni, Ard Biesheuvel

Switch all users of ArmVirt.dsc.inc to the new DebugLib implementation
that was created especially for DXE_RUNTIME_DRIVER modules, ensuring
that DEBUG() calls do not touch the UART at runtime.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
 ArmVirtPkg/ArmVirt.dsc.inc | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
index 0cb48f08e9bf..cde514958da2 100644
--- a/ArmVirtPkg/ArmVirt.dsc.inc
+++ b/ArmVirtPkg/ArmVirt.dsc.inc
@@ -231,6 +231,9 @@ [LibraryClasses.common.UEFI_DRIVER]
 [LibraryClasses.common.DXE_RUNTIME_DRIVER]
   MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
   CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf
+!if $(TARGET) != RELEASE
+  DebugLib|MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.inf
+!endif
 
 !if $(SECURE_BOOT_ENABLE) == TRUE
   BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
-- 
2.11.0



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

* Re: [PATCH edk2-platforms v2 1/2] MdePkg: introduce DxeRuntimeDebugLibSerialPort
  2018-02-22 16:28 ` [PATCH edk2-platforms v2 1/2] MdePkg: introduce DxeRuntimeDebugLibSerialPort Ard Biesheuvel
@ 2018-02-22 17:45   ` Ard Biesheuvel
  2018-02-22 17:46   ` Kinney, Michael D
  2018-02-22 18:15   ` Laszlo Ersek
  2 siblings, 0 replies; 8+ messages in thread
From: Ard Biesheuvel @ 2018-02-22 17:45 UTC (permalink / raw)
  To: edk2-devel@lists.01.org
  Cc: Leif Lindholm, Laszlo Ersek, Gao, Liming, Kinney, Michael D,
	afish, Zeng, Star, Ruiyu Ni, Ard Biesheuvel

On 22 February 2018 at 16:28, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> Introduce a variant of BaseDebugLibSerialPort that behaves correctly with
> regards to the use of the serial port after ExitBootServices(). Also, it
> caches PCD values in the constructor so that no calls into PcdLib are
> made at runtime.
>

Actually (and perhaps this is what Mike was trying to point out to
me), the PCDs in question only exist as fixed or patchable in the
first place, so caching them is pointless. I will respin with that
part removed.


> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  MdePkg/Library/DxeRuntimeDebugLibSerialPort/DebugLib.c                       | 373 ++++++++++++++++++++
>  MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.inf |  56 +++
>  MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.uni |  21 ++
>  3 files changed, 450 insertions(+)
>
> diff --git a/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DebugLib.c b/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DebugLib.c
> new file mode 100644
> index 000000000000..8579469029a1
> --- /dev/null
> +++ b/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DebugLib.c
> @@ -0,0 +1,373 @@
> +/** @file
> +  DXE runtime Debug library instance based on Serial Port library.
> +  It takes care not to call into SerialPortLib or PcdLib after
> +  ExitBootServices() has been called, to prevent touching hardware that is
> +  no longer owned by the firmware, or invoking protocols that are not safe
> +  for runtime (such as gEfiPcdProtocolGuid)
> +
> +  Copyright (c) 2006 - 2011, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2018, Linaro, Ltd. All rights reserved.<BR>
> +
> +  This program and the accompanying materials
> +  are licensed and made available under the terms and conditions of the BSD License
> +  which accompanies this distribution.  The full text of the license may be found at
> +  http://opensource.org/licenses/bsd-license.php.
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +
> +**/
> +
> +#include <Base.h>
> +#include <Library/DebugLib.h>
> +#include <Library/DebugPrintErrorLevelLib.h>
> +#include <Library/BaseLib.h>
> +#include <Library/PrintLib.h>
> +#include <Library/PcdLib.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/SerialPortLib.h>
> +
> +STATIC EFI_EVENT      mEfiExitBootServicesEvent;
> +STATIC BOOLEAN        mEfiAtRuntime;
> +
> +STATIC UINT8          mDebugClearMemoryValue;
> +STATIC UINT8          mDebugPropertyMask;
> +STATIC UINT32         mFixedDebugPrintErrorLevel;
> +
> +//
> +// Define the maximum debug and assert message length that this library supports
> +//
> +#define MAX_DEBUG_MESSAGE_LENGTH  0x100
> +
> +/**
> +  Set AtRuntime flag as TRUE after ExitBootServices.
> +
> +  @param[in]  Event   The Event that is being processed.
> +  @param[in]  Context The Event Context.
> +
> +**/
> +STATIC
> +VOID
> +EFIAPI
> +ExitBootServicesEvent (
> +  IN EFI_EVENT        Event,
> +  IN VOID             *Context
> +  )
> +{
> +  mEfiAtRuntime = TRUE;
> +}
> +
> +/**
> +  The constructor function to initialize the Serial Port library and
> +  register a callback for the ExitBootServices event.
> +
> +  @param[in]  ImageHandle   The firmware allocated handle for the EFI image.
> +  @param[in]  SystemTable   A pointer to the EFI System Table.
> +
> +  @retval EFI_SUCCESS   The operation completed successfully.
> +  @retval other         Either the serial port failed to initialize or the
> +                        ExitBootServices event callback registration failed.
> +**/
> +EFI_STATUS
> +EFIAPI
> +DxeRuntimeDebugLibSerialPortConstructor (
> +  IN EFI_HANDLE        ImageHandle,
> +  IN EFI_SYSTEM_TABLE  *SystemTable
> +  )
> +{
> +  EFI_STATUS    Status;
> +
> +  mDebugClearMemoryValue      = PcdGet8 (PcdDebugClearMemoryValue);
> +  mDebugPropertyMask          = PcdGet8 (PcdDebugPropertyMask);
> +  mFixedDebugPrintErrorLevel  = PcdGet32 (PcdFixedDebugPrintErrorLevel);
> +
> +  Status = SerialPortInitialize ();
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  return SystemTable->BootServices->CreateEventEx (EVT_NOTIFY_SIGNAL,
> +                                      TPL_NOTIFY, ExitBootServicesEvent, NULL,
> +                                      &gEfiEventExitBootServicesGuid,
> +                                      &mEfiExitBootServicesEvent);
> +}
> +
> +/**
> +  If a runtime driver exits with an error, it must call this routine
> +  to free the allocated resource before the exiting.
> +
> +  @param[in]  ImageHandle   The firmware allocated handle for the EFI image.
> +  @param[in]  SystemTable   A pointer to the EFI System Table.
> +
> +  @retval     EFI_SUCCESS       The Runtime Driver Lib shutdown successfully.
> +  @retval     EFI_UNSUPPORTED   Runtime Driver lib was not initialized.
> +**/
> +EFI_STATUS
> +EFIAPI
> +DxeRuntimeDebugLibSerialPortDestructor (
> +  IN EFI_HANDLE        ImageHandle,
> +  IN EFI_SYSTEM_TABLE  *SystemTable
> +  )
> +{
> +  return SystemTable->BootServices->CloseEvent (mEfiExitBootServicesEvent);
> +}
> +
> +/**
> +  Prints a debug message to the debug output device if the specified error level
> +  is enabled.
> +
> +  If any bit in ErrorLevel is also set in DebugPrintErrorLevelLib function
> +  GetDebugPrintErrorLevel (), then print the message specified by Format and the
> +  associated variable argument list to the debug output device.
> +
> +  If Format is NULL, then ASSERT().
> +
> +  @param  ErrorLevel  The error level of the debug message.
> +  @param  Format      Format string for the debug message to print.
> +  @param  ...         Variable argument list whose contents are accessed
> +                      based on the format string specified by Format.
> +
> +**/
> +VOID
> +EFIAPI
> +DebugPrint (
> +  IN  UINTN        ErrorLevel,
> +  IN  CONST CHAR8  *Format,
> +  ...
> +  )
> +{
> +  CHAR8    Buffer[MAX_DEBUG_MESSAGE_LENGTH];
> +  VA_LIST  Marker;
> +
> +  if (mEfiAtRuntime) {
> +    return;
> +  }
> +
> +  ASSERT (Format != NULL);
> +
> +  //
> +  // Check driver debug mask value and global mask
> +  //
> +  if ((ErrorLevel & GetDebugPrintErrorLevel ()) == 0) {
> +    return;
> +  }
> +
> +  //
> +  // Convert the DEBUG() message to an ASCII String
> +  //
> +  VA_START (Marker, Format);
> +  AsciiVSPrint (Buffer, sizeof (Buffer), Format, Marker);
> +  VA_END (Marker);
> +
> +  //
> +  // Send the print string to a Serial Port
> +  //
> +  SerialPortWrite ((UINT8 *)Buffer, AsciiStrLen (Buffer));
> +}
> +
> +
> +/**
> +  Prints an assert message containing a filename, line number, and description.
> +  This may be followed by a breakpoint or a dead loop.
> +
> +  Print a message of the form "ASSERT <FileName>(<LineNumber>): <Description>\n"
> +  to the debug output device.  If DEBUG_PROPERTY_ASSERT_BREAKPOINT_ENABLED bit
> +  of PcdDebugProperyMask is set then CpuBreakpoint() is called. Otherwise, if
> +  DEBUG_PROPERTY_ASSERT_DEADLOOP_ENABLED bit of PcdDebugProperyMask is set then
> +  CpuDeadLoop() is called.  If neither of these bits are set, then this function
> +  returns immediately after the message is printed to the debug output device.
> +  DebugAssert() must actively prevent recursion.  If DebugAssert() is called
> +  while processing another DebugAssert(), then DebugAssert() must return
> +  immediately.
> +
> +  If FileName is NULL, then a <FileName> string of "(NULL) Filename" is printed.
> +  If Description is NULL, then a <Description> string of "(NULL) Description" is
> +  printed.
> +
> +  @param  FileName     The pointer to the name of the source file that generated
> +                       the assert condition.
> +  @param  LineNumber   The line number in the source file that generated the
> +                       assert condition
> +  @param  Description  The pointer to the description of the assert condition.
> +
> +**/
> +VOID
> +EFIAPI
> +DebugAssert (
> +  IN CONST CHAR8  *FileName,
> +  IN UINTN        LineNumber,
> +  IN CONST CHAR8  *Description
> +  )
> +{
> +  CHAR8  Buffer[MAX_DEBUG_MESSAGE_LENGTH];
> +
> +  if (!mEfiAtRuntime) {
> +    //
> +    // 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 the Console Output device
> +    //
> +    SerialPortWrite ((UINT8 *)Buffer, AsciiStrLen (Buffer));
> +  }
> +
> +  //
> +  // Generate a Breakpoint, DeadLoop, or NOP based on PCD settings
> +  //
> +  if ((mDebugPropertyMask & DEBUG_PROPERTY_ASSERT_BREAKPOINT_ENABLED) != 0) {
> +    CpuBreakpoint ();
> +  } else if ((mDebugPropertyMask &
> +              DEBUG_PROPERTY_ASSERT_DEADLOOP_ENABLED) != 0) {
> +    CpuDeadLoop ();
> +  }
> +}
> +
> +
> +/**
> +  Fills a target buffer with PcdDebugClearMemoryValue, and returns the target
> +  buffer.
> +
> +  This function fills Length bytes of Buffer with the value specified by
> +  PcdDebugClearMemoryValue, and returns Buffer.
> +
> +  If Buffer is NULL, then ASSERT().
> +  If Length is greater than (MAX_ADDRESS - Buffer + 1), then ASSERT().
> +
> +  @param   Buffer  The pointer to the target buffer to be filled with
> +                   PcdDebugClearMemoryValue.
> +  @param   Length  The number of bytes in Buffer to fill with
> +                   PcdDebugClearMemoryValue.
> +
> +  @return  Buffer  The pointer to the target buffer filled with
> +                   PcdDebugClearMemoryValue.
> +
> +**/
> +VOID *
> +EFIAPI
> +DebugClearMemory (
> +  OUT VOID  *Buffer,
> +  IN UINTN  Length
> +  )
> +{
> +  if (!mEfiAtRuntime) {
> +    ASSERT (Buffer != NULL);
> +    ASSERT (Length <= (MAX_ADDRESS - (UINTN)Buffer + 1));
> +  }
> +
> +  return SetMem (Buffer, Length, mDebugClearMemoryValue);
> +}
> +
> +
> +/**
> +  Returns TRUE if ASSERT() macros are enabled.
> +
> +  This function returns TRUE if the DEBUG_PROPERTY_DEBUG_ASSERT_ENABLED bit of
> +  PcdDebugProperyMask is set.  Otherwise FALSE is returned.
> +
> +  @retval  TRUE    The DEBUG_PROPERTY_DEBUG_ASSERT_ENABLED bit of
> +                   PcdDebugProperyMask is set.
> +  @retval  FALSE   The DEBUG_PROPERTY_DEBUG_ASSERT_ENABLED bit of
> +                   PcdDebugProperyMask is clear.
> +
> +**/
> +BOOLEAN
> +EFIAPI
> +DebugAssertEnabled (
> +  VOID
> +  )
> +{
> +  return (BOOLEAN)((mDebugPropertyMask &
> +                    DEBUG_PROPERTY_DEBUG_ASSERT_ENABLED) != 0);
> +}
> +
> +
> +/**
> +  Returns TRUE if DEBUG() macros are enabled.
> +
> +  This function returns TRUE if the DEBUG_PROPERTY_DEBUG_PRINT_ENABLED bit of
> +  PcdDebugProperyMask is set.  Otherwise FALSE is returned.
> +
> +  @retval  TRUE    The DEBUG_PROPERTY_DEBUG_PRINT_ENABLED bit of
> +                   PcdDebugProperyMask is set.
> +  @retval  FALSE   The DEBUG_PROPERTY_DEBUG_PRINT_ENABLED bit of
> +                   PcdDebugProperyMask is clear.
> +
> +**/
> +BOOLEAN
> +EFIAPI
> +DebugPrintEnabled (
> +  VOID
> +  )
> +{
> +  return (BOOLEAN)((mDebugPropertyMask &
> +                    DEBUG_PROPERTY_DEBUG_PRINT_ENABLED) != 0);
> +}
> +
> +
> +/**
> +  Returns TRUE if DEBUG_CODE() macros are enabled.
> +
> +  This function returns TRUE if the DEBUG_PROPERTY_DEBUG_CODE_ENABLED bit of
> +  PcdDebugProperyMask is set.  Otherwise FALSE is returned.
> +
> +  @retval  TRUE    The DEBUG_PROPERTY_DEBUG_CODE_ENABLED bit of
> +                   PcdDebugProperyMask is set.
> +  @retval  FALSE   The DEBUG_PROPERTY_DEBUG_CODE_ENABLED bit of
> +                   PcdDebugProperyMask is clear.
> +
> +**/
> +BOOLEAN
> +EFIAPI
> +DebugCodeEnabled (
> +  VOID
> +  )
> +{
> +  return (BOOLEAN)((mDebugPropertyMask &
> +                    DEBUG_PROPERTY_DEBUG_CODE_ENABLED) != 0);
> +}
> +
> +
> +/**
> +  Returns TRUE if DEBUG_CLEAR_MEMORY() macro is enabled.
> +
> +  This function returns TRUE if the DEBUG_PROPERTY_CLEAR_MEMORY_ENABLED bit of
> +  PcdDebugProperyMask is set.  Otherwise FALSE is returned.
> +
> +  @retval  TRUE    The DEBUG_PROPERTY_CLEAR_MEMORY_ENABLED bit of
> +                   PcdDebugProperyMask is set.
> +  @retval  FALSE   The DEBUG_PROPERTY_CLEAR_MEMORY_ENABLED bit of
> +                   PcdDebugProperyMask is clear.
> +
> +**/
> +BOOLEAN
> +EFIAPI
> +DebugClearMemoryEnabled (
> +  VOID
> +  )
> +{
> +  return (BOOLEAN)((mDebugPropertyMask &
> +                    DEBUG_PROPERTY_CLEAR_MEMORY_ENABLED) != 0);
> +}
> +
> +/**
> +  Returns TRUE if any one of the bit is set both in ErrorLevel and
> +  PcdFixedDebugPrintErrorLevel.
> +
> +  This function compares the bit mask of ErrorLevel and
> +  PcdFixedDebugPrintErrorLevel.
> +
> +  @retval  TRUE    Current ErrorLevel is supported.
> +  @retval  FALSE   Current ErrorLevel is not supported.
> +
> +**/
> +BOOLEAN
> +EFIAPI
> +DebugPrintLevelEnabled (
> +  IN  CONST UINTN        ErrorLevel
> +  )
> +{
> +  return (BOOLEAN)((ErrorLevel & mFixedDebugPrintErrorLevel) != 0);
> +}
> diff --git a/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.inf b/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.inf
> new file mode 100644
> index 000000000000..d5e9d3f8dee7
> --- /dev/null
> +++ b/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.inf
> @@ -0,0 +1,56 @@
> +## @file
> +#  DXE runtime Debug library instance based on Serial Port library.
> +#  It takes care not to call into SerialPortLib or PcdLib after
> +#  ExitBootServices() has been called, to prevent touching hardware that is
> +#  no longer owned by the firmware, or invoking protocols that are not safe
> +#  for runtime (such as gEfiPcdProtocolGuid)
> +#
> +#  Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.<BR>
> +#  Copyright (c) 2018, Linaro, Ltd. All rights reserved.<BR>
> +#
> +#  This program and the accompanying materials
> +#  are licensed and made available under the terms and conditions of the BSD License
> +#  which accompanies this distribution. The full text of the license may be found at
> +#  http://opensource.org/licenses/bsd-license.php.
> +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +#
> +#
> +##
> +
> +[Defines]
> +  INF_VERSION                    = 0x0001001A
> +  BASE_NAME                      = DxeRuntimeDebugLibSerialPort
> +  MODULE_UNI_FILE                = DxeRuntimeDebugLibSerialPort.uni
> +  FILE_GUID                      = 9D914E2F-7CCB-41DB-8E74-9AFF8F3BBFBF
> +  MODULE_TYPE                    = DXE_RUNTIME_DRIVER
> +  VERSION_STRING                 = 1.0
> +  LIBRARY_CLASS                  = DebugLib|DXE_RUNTIME_DRIVER
> +  CONSTRUCTOR                    = DxeRuntimeDebugLibSerialPortConstructor
> +  DESTRUCTOR                     = DxeRuntimeDebugLibSerialPortDestructor
> +
> +#
> +#  VALID_ARCHITECTURES           = AARCH64 ARM IA32 X64 IPF EBC
> +#
> +
> +[Sources]
> +  DebugLib.c
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +
> +[LibraryClasses]
> +  BaseLib
> +  BaseMemoryLib
> +  DebugPrintErrorLevelLib
> +  PcdLib
> +  PrintLib
> +  SerialPortLib
> +
> +[Guids]
> +  gEfiEventExitBootServicesGuid                         ## CONSUMES ## Event
> +
> +[Pcd]
> +  gEfiMdePkgTokenSpaceGuid.PcdDebugClearMemoryValue     ## CONSUMES
> +  gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask         ## CONSUMES
> +  gEfiMdePkgTokenSpaceGuid.PcdFixedDebugPrintErrorLevel ## CONSUMES
> diff --git a/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.uni b/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.uni
> new file mode 100644
> index 000000000000..cd65515c4177
> --- /dev/null
> +++ b/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.uni
> @@ -0,0 +1,21 @@
> +// /** @file
> +// Instance of Debug Library based on Serial Port Library.
> +//
> +// It uses Print Library to produce formatted output strings to seiral port device.
> +//
> +// Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.<BR>
> +//
> +// This program and the accompanying materials
> +// are licensed and made available under the terms and conditions of the BSD License
> +// which accompanies this distribution. The full text of the license may be found at
> +// http://opensource.org/licenses/bsd-license.php.
> +// THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +// WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +//
> +// **/
> +
> +
> +#string STR_MODULE_ABSTRACT             #language en-US "Instance of Debug Library based on Serial Port Library"
> +
> +#string STR_MODULE_DESCRIPTION          #language en-US "It uses Print Library to produce formatted output strings to a serial port device."
> +
> --
> 2.11.0
>


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

* Re: [PATCH edk2-platforms v2 1/2] MdePkg: introduce DxeRuntimeDebugLibSerialPort
  2018-02-22 16:28 ` [PATCH edk2-platforms v2 1/2] MdePkg: introduce DxeRuntimeDebugLibSerialPort Ard Biesheuvel
  2018-02-22 17:45   ` Ard Biesheuvel
@ 2018-02-22 17:46   ` Kinney, Michael D
  2018-02-22 17:50     ` Ard Biesheuvel
  2018-02-22 18:15   ` Laszlo Ersek
  2 siblings, 1 reply; 8+ messages in thread
From: Kinney, Michael D @ 2018-02-22 17:46 UTC (permalink / raw)
  To: Ard Biesheuvel, edk2-devel@lists.01.org, Kinney, Michael D
  Cc: leif.lindholm@linaro.org, lersek@redhat.com, Gao, Liming,
	afish@apple.com, Zeng, Star, Ni, Ruiyu

Ard,

The check for the runtime flag seems inconsistent.

Since the issue being addressed is use of SerialPortLib,
can't the mEfiAtRuntime flag be use around SerialPortLib
calls only.

Mike

> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Thursday, February 22, 2018 8:28 AM
> To: edk2-devel@lists.01.org
> Cc: leif.lindholm@linaro.org; lersek@redhat.com; Gao,
> Liming <liming.gao@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; afish@apple.com; Zeng,
> Star <star.zeng@intel.com>; Ni, Ruiyu
> <ruiyu.ni@intel.com>; Ard Biesheuvel
> <ard.biesheuvel@linaro.org>
> Subject: [PATCH edk2-platforms v2 1/2] MdePkg: introduce
> DxeRuntimeDebugLibSerialPort
> 
> Introduce a variant of BaseDebugLibSerialPort that
> behaves correctly with
> regards to the use of the serial port after
> ExitBootServices(). Also, it
> caches PCD values in the constructor so that no calls
> into PcdLib are
> made at runtime.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel
> <ard.biesheuvel@linaro.org>
> ---
>  MdePkg/Library/DxeRuntimeDebugLibSerialPort/DebugLib.c
> | 373 ++++++++++++++++++++
> 
> MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDe
> bugLibSerialPort.inf |  56 +++
> 
> MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDe
> bugLibSerialPort.uni |  21 ++
>  3 files changed, 450 insertions(+)
> 
> diff --git
> a/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DebugLib.c
> b/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DebugLib.c
> new file mode 100644
> index 000000000000..8579469029a1
> --- /dev/null
> +++
> b/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DebugLib.c
> @@ -0,0 +1,373 @@
> +/** @file
> +  DXE runtime Debug library instance based on Serial
> Port library.
> +  It takes care not to call into SerialPortLib or
> PcdLib after
> +  ExitBootServices() has been called, to prevent
> touching hardware that is
> +  no longer owned by the firmware, or invoking
> protocols that are not safe
> +  for runtime (such as gEfiPcdProtocolGuid)
> +
> +  Copyright (c) 2006 - 2011, Intel Corporation. All
> rights reserved.<BR>
> +  Copyright (c) 2018, Linaro, Ltd. All rights
> reserved.<BR>
> +
> +  This program and the accompanying materials
> +  are licensed and made available under the terms and
> conditions of the BSD License
> +  which accompanies this distribution.  The full text
> of the license may be found at
> +  http://opensource.org/licenses/bsd-license.php.
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON
> AN "AS IS" BASIS,
> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND,
> EITHER EXPRESS OR IMPLIED.
> +
> +**/
> +
> +#include <Base.h>
> +#include <Library/DebugLib.h>
> +#include <Library/DebugPrintErrorLevelLib.h>
> +#include <Library/BaseLib.h>
> +#include <Library/PrintLib.h>
> +#include <Library/PcdLib.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/SerialPortLib.h>
> +
> +STATIC EFI_EVENT      mEfiExitBootServicesEvent;
> +STATIC BOOLEAN        mEfiAtRuntime;
> +
> +STATIC UINT8          mDebugClearMemoryValue;
> +STATIC UINT8          mDebugPropertyMask;
> +STATIC UINT32         mFixedDebugPrintErrorLevel;
> +
> +//
> +// Define the maximum debug and assert message length
> that this library supports
> +//
> +#define MAX_DEBUG_MESSAGE_LENGTH  0x100
> +
> +/**
> +  Set AtRuntime flag as TRUE after ExitBootServices.
> +
> +  @param[in]  Event   The Event that is being
> processed.
> +  @param[in]  Context The Event Context.
> +
> +**/
> +STATIC
> +VOID
> +EFIAPI
> +ExitBootServicesEvent (
> +  IN EFI_EVENT        Event,
> +  IN VOID             *Context
> +  )
> +{
> +  mEfiAtRuntime = TRUE;
> +}
> +
> +/**
> +  The constructor function to initialize the Serial
> Port library and
> +  register a callback for the ExitBootServices event.
> +
> +  @param[in]  ImageHandle   The firmware allocated
> handle for the EFI image.
> +  @param[in]  SystemTable   A pointer to the EFI System
> Table.
> +
> +  @retval EFI_SUCCESS   The operation completed
> successfully.
> +  @retval other         Either the serial port failed
> to initialize or the
> +                        ExitBootServices event callback
> registration failed.
> +**/
> +EFI_STATUS
> +EFIAPI
> +DxeRuntimeDebugLibSerialPortConstructor (
> +  IN EFI_HANDLE        ImageHandle,
> +  IN EFI_SYSTEM_TABLE  *SystemTable
> +  )
> +{
> +  EFI_STATUS    Status;
> +
> +  mDebugClearMemoryValue      = PcdGet8
> (PcdDebugClearMemoryValue);
> +  mDebugPropertyMask          = PcdGet8
> (PcdDebugPropertyMask);
> +  mFixedDebugPrintErrorLevel  = PcdGet32
> (PcdFixedDebugPrintErrorLevel);
> +
> +  Status = SerialPortInitialize ();
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  return SystemTable->BootServices->CreateEventEx
> (EVT_NOTIFY_SIGNAL,
> +                                      TPL_NOTIFY,
> ExitBootServicesEvent, NULL,
> +
> &gEfiEventExitBootServicesGuid,
> +
> &mEfiExitBootServicesEvent);
> +}
> +
> +/**
> +  If a runtime driver exits with an error, it must call
> this routine
> +  to free the allocated resource before the exiting.
> +
> +  @param[in]  ImageHandle   The firmware allocated
> handle for the EFI image.
> +  @param[in]  SystemTable   A pointer to the EFI System
> Table.
> +
> +  @retval     EFI_SUCCESS       The Runtime Driver Lib
> shutdown successfully.
> +  @retval     EFI_UNSUPPORTED   Runtime Driver lib was
> not initialized.
> +**/
> +EFI_STATUS
> +EFIAPI
> +DxeRuntimeDebugLibSerialPortDestructor (
> +  IN EFI_HANDLE        ImageHandle,
> +  IN EFI_SYSTEM_TABLE  *SystemTable
> +  )
> +{
> +  return SystemTable->BootServices->CloseEvent
> (mEfiExitBootServicesEvent);
> +}
> +
> +/**
> +  Prints a debug message to the debug output device if
> the specified error level
> +  is enabled.
> +
> +  If any bit in ErrorLevel is also set in
> DebugPrintErrorLevelLib function
> +  GetDebugPrintErrorLevel (), then print the message
> specified by Format and the
> +  associated variable argument list to the debug output
> device.
> +
> +  If Format is NULL, then ASSERT().
> +
> +  @param  ErrorLevel  The error level of the debug
> message.
> +  @param  Format      Format string for the debug
> message to print.
> +  @param  ...         Variable argument list whose
> contents are accessed
> +                      based on the format string
> specified by Format.
> +
> +**/
> +VOID
> +EFIAPI
> +DebugPrint (
> +  IN  UINTN        ErrorLevel,
> +  IN  CONST CHAR8  *Format,
> +  ...
> +  )
> +{
> +  CHAR8    Buffer[MAX_DEBUG_MESSAGE_LENGTH];
> +  VA_LIST  Marker;
> +
> +  if (mEfiAtRuntime) {
> +    return;
> +  }
> +
> +  ASSERT (Format != NULL);
> +
> +  //
> +  // Check driver debug mask value and global mask
> +  //
> +  if ((ErrorLevel & GetDebugPrintErrorLevel ()) == 0) {
> +    return;
> +  }
> +
> +  //
> +  // Convert the DEBUG() message to an ASCII String
> +  //
> +  VA_START (Marker, Format);
> +  AsciiVSPrint (Buffer, sizeof (Buffer), Format,
> Marker);
> +  VA_END (Marker);
> +
> +  //
> +  // Send the print string to a Serial Port
> +  //
> +  SerialPortWrite ((UINT8 *)Buffer, AsciiStrLen
> (Buffer));
> +}
> +
> +
> +/**
> +  Prints an assert message containing a filename, line
> number, and description.
> +  This may be followed by a breakpoint or a dead loop.
> +
> +  Print a message of the form "ASSERT
> <FileName>(<LineNumber>): <Description>\n"
> +  to the debug output device.  If
> DEBUG_PROPERTY_ASSERT_BREAKPOINT_ENABLED bit
> +  of PcdDebugProperyMask is set then CpuBreakpoint() is
> called. Otherwise, if
> +  DEBUG_PROPERTY_ASSERT_DEADLOOP_ENABLED bit of
> PcdDebugProperyMask is set then
> +  CpuDeadLoop() is called.  If neither of these bits
> are set, then this function
> +  returns immediately after the message is printed to
> the debug output device.
> +  DebugAssert() must actively prevent recursion.  If
> DebugAssert() is called
> +  while processing another DebugAssert(), then
> DebugAssert() must return
> +  immediately.
> +
> +  If FileName is NULL, then a <FileName> string of
> "(NULL) Filename" is printed.
> +  If Description is NULL, then a <Description> string
> of "(NULL) Description" is
> +  printed.
> +
> +  @param  FileName     The pointer to the name of the
> source file that generated
> +                       the assert condition.
> +  @param  LineNumber   The line number in the source
> file that generated the
> +                       assert condition
> +  @param  Description  The pointer to the description
> of the assert condition.
> +
> +**/
> +VOID
> +EFIAPI
> +DebugAssert (
> +  IN CONST CHAR8  *FileName,
> +  IN UINTN        LineNumber,
> +  IN CONST CHAR8  *Description
> +  )
> +{
> +  CHAR8  Buffer[MAX_DEBUG_MESSAGE_LENGTH];
> +
> +  if (!mEfiAtRuntime) {
> +    //
> +    // 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 the Console Output
> device
> +    //
> +    SerialPortWrite ((UINT8 *)Buffer, AsciiStrLen
> (Buffer));
> +  }
> +
> +  //
> +  // Generate a Breakpoint, DeadLoop, or NOP based on
> PCD settings
> +  //
> +  if ((mDebugPropertyMask &
> DEBUG_PROPERTY_ASSERT_BREAKPOINT_ENABLED) != 0) {
> +    CpuBreakpoint ();
> +  } else if ((mDebugPropertyMask &
> +              DEBUG_PROPERTY_ASSERT_DEADLOOP_ENABLED)
> != 0) {
> +    CpuDeadLoop ();
> +  }
> +}
> +
> +
> +/**
> +  Fills a target buffer with PcdDebugClearMemoryValue,
> and returns the target
> +  buffer.
> +
> +  This function fills Length bytes of Buffer with the
> value specified by
> +  PcdDebugClearMemoryValue, and returns Buffer.
> +
> +  If Buffer is NULL, then ASSERT().
> +  If Length is greater than (MAX_ADDRESS - Buffer + 1),
> then ASSERT().
> +
> +  @param   Buffer  The pointer to the target buffer to
> be filled with
> +                   PcdDebugClearMemoryValue.
> +  @param   Length  The number of bytes in Buffer to
> fill with
> +                   PcdDebugClearMemoryValue.
> +
> +  @return  Buffer  The pointer to the target buffer
> filled with
> +                   PcdDebugClearMemoryValue.
> +
> +**/
> +VOID *
> +EFIAPI
> +DebugClearMemory (
> +  OUT VOID  *Buffer,
> +  IN UINTN  Length
> +  )
> +{
> +  if (!mEfiAtRuntime) {
> +    ASSERT (Buffer != NULL);
> +    ASSERT (Length <= (MAX_ADDRESS - (UINTN)Buffer +
> 1));
> +  }
> +
> +  return SetMem (Buffer, Length,
> mDebugClearMemoryValue);
> +}
> +
> +
> +/**
> +  Returns TRUE if ASSERT() macros are enabled.
> +
> +  This function returns TRUE if the
> DEBUG_PROPERTY_DEBUG_ASSERT_ENABLED bit of
> +  PcdDebugProperyMask is set.  Otherwise FALSE is
> returned.
> +
> +  @retval  TRUE    The
> DEBUG_PROPERTY_DEBUG_ASSERT_ENABLED bit of
> +                   PcdDebugProperyMask is set.
> +  @retval  FALSE   The
> DEBUG_PROPERTY_DEBUG_ASSERT_ENABLED bit of
> +                   PcdDebugProperyMask is clear.
> +
> +**/
> +BOOLEAN
> +EFIAPI
> +DebugAssertEnabled (
> +  VOID
> +  )
> +{
> +  return (BOOLEAN)((mDebugPropertyMask &
> +
> DEBUG_PROPERTY_DEBUG_ASSERT_ENABLED) != 0);
> +}
> +
> +
> +/**
> +  Returns TRUE if DEBUG() macros are enabled.
> +
> +  This function returns TRUE if the
> DEBUG_PROPERTY_DEBUG_PRINT_ENABLED bit of
> +  PcdDebugProperyMask is set.  Otherwise FALSE is
> returned.
> +
> +  @retval  TRUE    The
> DEBUG_PROPERTY_DEBUG_PRINT_ENABLED bit of
> +                   PcdDebugProperyMask is set.
> +  @retval  FALSE   The
> DEBUG_PROPERTY_DEBUG_PRINT_ENABLED bit of
> +                   PcdDebugProperyMask is clear.
> +
> +**/
> +BOOLEAN
> +EFIAPI
> +DebugPrintEnabled (
> +  VOID
> +  )
> +{
> +  return (BOOLEAN)((mDebugPropertyMask &
> +                    DEBUG_PROPERTY_DEBUG_PRINT_ENABLED)
> != 0);
> +}
> +
> +
> +/**
> +  Returns TRUE if DEBUG_CODE() macros are enabled.
> +
> +  This function returns TRUE if the
> DEBUG_PROPERTY_DEBUG_CODE_ENABLED bit of
> +  PcdDebugProperyMask is set.  Otherwise FALSE is
> returned.
> +
> +  @retval  TRUE    The
> DEBUG_PROPERTY_DEBUG_CODE_ENABLED bit of
> +                   PcdDebugProperyMask is set.
> +  @retval  FALSE   The
> DEBUG_PROPERTY_DEBUG_CODE_ENABLED bit of
> +                   PcdDebugProperyMask is clear.
> +
> +**/
> +BOOLEAN
> +EFIAPI
> +DebugCodeEnabled (
> +  VOID
> +  )
> +{
> +  return (BOOLEAN)((mDebugPropertyMask &
> +                    DEBUG_PROPERTY_DEBUG_CODE_ENABLED)
> != 0);
> +}
> +
> +
> +/**
> +  Returns TRUE if DEBUG_CLEAR_MEMORY() macro is
> enabled.
> +
> +  This function returns TRUE if the
> DEBUG_PROPERTY_CLEAR_MEMORY_ENABLED bit of
> +  PcdDebugProperyMask is set.  Otherwise FALSE is
> returned.
> +
> +  @retval  TRUE    The
> DEBUG_PROPERTY_CLEAR_MEMORY_ENABLED bit of
> +                   PcdDebugProperyMask is set.
> +  @retval  FALSE   The
> DEBUG_PROPERTY_CLEAR_MEMORY_ENABLED bit of
> +                   PcdDebugProperyMask is clear.
> +
> +**/
> +BOOLEAN
> +EFIAPI
> +DebugClearMemoryEnabled (
> +  VOID
> +  )
> +{
> +  return (BOOLEAN)((mDebugPropertyMask &
> +
> DEBUG_PROPERTY_CLEAR_MEMORY_ENABLED) != 0);
> +}
> +
> +/**
> +  Returns TRUE if any one of the bit is set both in
> ErrorLevel and
> +  PcdFixedDebugPrintErrorLevel.
> +
> +  This function compares the bit mask of ErrorLevel and
> +  PcdFixedDebugPrintErrorLevel.
> +
> +  @retval  TRUE    Current ErrorLevel is supported.
> +  @retval  FALSE   Current ErrorLevel is not supported.
> +
> +**/
> +BOOLEAN
> +EFIAPI
> +DebugPrintLevelEnabled (
> +  IN  CONST UINTN        ErrorLevel
> +  )
> +{
> +  return (BOOLEAN)((ErrorLevel &
> mFixedDebugPrintErrorLevel) != 0);
> +}
> diff --git
> a/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntime
> DebugLibSerialPort.inf
> b/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntime
> DebugLibSerialPort.inf
> new file mode 100644
> index 000000000000..d5e9d3f8dee7
> --- /dev/null
> +++
> b/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntime
> DebugLibSerialPort.inf
> @@ -0,0 +1,56 @@
> +## @file
> +#  DXE runtime Debug library instance based on Serial
> Port library.
> +#  It takes care not to call into SerialPortLib or
> PcdLib after
> +#  ExitBootServices() has been called, to prevent
> touching hardware that is
> +#  no longer owned by the firmware, or invoking
> protocols that are not safe
> +#  for runtime (such as gEfiPcdProtocolGuid)
> +#
> +#  Copyright (c) 2006 - 2015, Intel Corporation. All
> rights reserved.<BR>
> +#  Copyright (c) 2018, Linaro, Ltd. All rights
> reserved.<BR>
> +#
> +#  This program and the accompanying materials
> +#  are licensed and made available under the terms and
> conditions of the BSD License
> +#  which accompanies this distribution. The full text
> of the license may be found at
> +#  http://opensource.org/licenses/bsd-license.php.
> +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON
> AN "AS IS" BASIS,
> +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND,
> EITHER EXPRESS OR IMPLIED.
> +#
> +#
> +##
> +
> +[Defines]
> +  INF_VERSION                    = 0x0001001A
> +  BASE_NAME                      =
> DxeRuntimeDebugLibSerialPort
> +  MODULE_UNI_FILE                =
> DxeRuntimeDebugLibSerialPort.uni
> +  FILE_GUID                      = 9D914E2F-7CCB-41DB-
> 8E74-9AFF8F3BBFBF
> +  MODULE_TYPE                    = DXE_RUNTIME_DRIVER
> +  VERSION_STRING                 = 1.0
> +  LIBRARY_CLASS                  =
> DebugLib|DXE_RUNTIME_DRIVER
> +  CONSTRUCTOR                    =
> DxeRuntimeDebugLibSerialPortConstructor
> +  DESTRUCTOR                     =
> DxeRuntimeDebugLibSerialPortDestructor
> +
> +#
> +#  VALID_ARCHITECTURES           = AARCH64 ARM IA32 X64
> IPF EBC
> +#
> +
> +[Sources]
> +  DebugLib.c
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +
> +[LibraryClasses]
> +  BaseLib
> +  BaseMemoryLib
> +  DebugPrintErrorLevelLib
> +  PcdLib
> +  PrintLib
> +  SerialPortLib
> +
> +[Guids]
> +  gEfiEventExitBootServicesGuid
> ## CONSUMES ## Event
> +
> +[Pcd]
> +  gEfiMdePkgTokenSpaceGuid.PcdDebugClearMemoryValue
> ## CONSUMES
> +  gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask
> ## CONSUMES
> +  gEfiMdePkgTokenSpaceGuid.PcdFixedDebugPrintErrorLevel
> ## CONSUMES
> diff --git
> a/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntime
> DebugLibSerialPort.uni
> b/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntime
> DebugLibSerialPort.uni
> new file mode 100644
> index 000000000000..cd65515c4177
> --- /dev/null
> +++
> b/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntime
> DebugLibSerialPort.uni
> @@ -0,0 +1,21 @@
> +// /** @file
> +// Instance of Debug Library based on Serial Port
> Library.
> +//
> +// It uses Print Library to produce formatted output
> strings to seiral port device.
> +//
> +// Copyright (c) 2006 - 2014, Intel Corporation. All
> rights reserved.<BR>
> +//
> +// This program and the accompanying materials
> +// are licensed and made available under the terms and
> conditions of the BSD License
> +// which accompanies this distribution. The full text
> of the license may be found at
> +// http://opensource.org/licenses/bsd-license.php.
> +// THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON
> AN "AS IS" BASIS,
> +// WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND,
> EITHER EXPRESS OR IMPLIED.
> +//
> +// **/
> +
> +
> +#string STR_MODULE_ABSTRACT             #language en-US
> "Instance of Debug Library based on Serial Port Library"
> +
> +#string STR_MODULE_DESCRIPTION          #language en-US
> "It uses Print Library to produce formatted output
> strings to a serial port device."
> +
> --
> 2.11.0



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

* Re: [PATCH edk2-platforms v2 1/2] MdePkg: introduce DxeRuntimeDebugLibSerialPort
  2018-02-22 17:46   ` Kinney, Michael D
@ 2018-02-22 17:50     ` Ard Biesheuvel
  2018-02-22 18:26       ` Laszlo Ersek
  0 siblings, 1 reply; 8+ messages in thread
From: Ard Biesheuvel @ 2018-02-22 17:50 UTC (permalink / raw)
  To: Kinney, Michael D
  Cc: edk2-devel@lists.01.org, leif.lindholm@linaro.org,
	lersek@redhat.com, Gao, Liming, afish@apple.com, Zeng, Star,
	Ni, Ruiyu

On 22 February 2018 at 17:46, Kinney, Michael D
<michael.d.kinney@intel.com> wrote:
> Ard,
>
> The check for the runtime flag seems inconsistent.
>
> Since the issue being addressed is use of SerialPortLib,
> can't the mEfiAtRuntime flag be use around SerialPortLib
> calls only.
>

I take it you mean the checks around ASSERT()s? I agree that those are
redundant, since the check will occur in the ASSERT() implementation
as well.

Will fix.


>> -----Original Message-----
>> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
>> Sent: Thursday, February 22, 2018 8:28 AM
>> To: edk2-devel@lists.01.org
>> Cc: leif.lindholm@linaro.org; lersek@redhat.com; Gao,
>> Liming <liming.gao@intel.com>; Kinney, Michael D
>> <michael.d.kinney@intel.com>; afish@apple.com; Zeng,
>> Star <star.zeng@intel.com>; Ni, Ruiyu
>> <ruiyu.ni@intel.com>; Ard Biesheuvel
>> <ard.biesheuvel@linaro.org>
>> Subject: [PATCH edk2-platforms v2 1/2] MdePkg: introduce
>> DxeRuntimeDebugLibSerialPort
>>
>> Introduce a variant of BaseDebugLibSerialPort that
>> behaves correctly with
>> regards to the use of the serial port after
>> ExitBootServices(). Also, it
>> caches PCD values in the constructor so that no calls
>> into PcdLib are
>> made at runtime.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel
>> <ard.biesheuvel@linaro.org>
>> ---
>>  MdePkg/Library/DxeRuntimeDebugLibSerialPort/DebugLib.c
>> | 373 ++++++++++++++++++++
>>
>> MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDe
>> bugLibSerialPort.inf |  56 +++
>>
>> MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDe
>> bugLibSerialPort.uni |  21 ++
>>  3 files changed, 450 insertions(+)
>>
>> diff --git
>> a/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DebugLib.c
>> b/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DebugLib.c
>> new file mode 100644
>> index 000000000000..8579469029a1
>> --- /dev/null
>> +++
>> b/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DebugLib.c
>> @@ -0,0 +1,373 @@
>> +/** @file
>> +  DXE runtime Debug library instance based on Serial
>> Port library.
>> +  It takes care not to call into SerialPortLib or
>> PcdLib after
>> +  ExitBootServices() has been called, to prevent
>> touching hardware that is
>> +  no longer owned by the firmware, or invoking
>> protocols that are not safe
>> +  for runtime (such as gEfiPcdProtocolGuid)
>> +
>> +  Copyright (c) 2006 - 2011, Intel Corporation. All
>> rights reserved.<BR>
>> +  Copyright (c) 2018, Linaro, Ltd. All rights
>> reserved.<BR>
>> +
>> +  This program and the accompanying materials
>> +  are licensed and made available under the terms and
>> conditions of the BSD License
>> +  which accompanies this distribution.  The full text
>> of the license may be found at
>> +  http://opensource.org/licenses/bsd-license.php.
>> +
>> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON
>> AN "AS IS" BASIS,
>> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND,
>> EITHER EXPRESS OR IMPLIED.
>> +
>> +**/
>> +
>> +#include <Base.h>
>> +#include <Library/DebugLib.h>
>> +#include <Library/DebugPrintErrorLevelLib.h>
>> +#include <Library/BaseLib.h>
>> +#include <Library/PrintLib.h>
>> +#include <Library/PcdLib.h>
>> +#include <Library/BaseMemoryLib.h>
>> +#include <Library/SerialPortLib.h>
>> +
>> +STATIC EFI_EVENT      mEfiExitBootServicesEvent;
>> +STATIC BOOLEAN        mEfiAtRuntime;
>> +
>> +STATIC UINT8          mDebugClearMemoryValue;
>> +STATIC UINT8          mDebugPropertyMask;
>> +STATIC UINT32         mFixedDebugPrintErrorLevel;
>> +
>> +//
>> +// Define the maximum debug and assert message length
>> that this library supports
>> +//
>> +#define MAX_DEBUG_MESSAGE_LENGTH  0x100
>> +
>> +/**
>> +  Set AtRuntime flag as TRUE after ExitBootServices.
>> +
>> +  @param[in]  Event   The Event that is being
>> processed.
>> +  @param[in]  Context The Event Context.
>> +
>> +**/
>> +STATIC
>> +VOID
>> +EFIAPI
>> +ExitBootServicesEvent (
>> +  IN EFI_EVENT        Event,
>> +  IN VOID             *Context
>> +  )
>> +{
>> +  mEfiAtRuntime = TRUE;
>> +}
>> +
>> +/**
>> +  The constructor function to initialize the Serial
>> Port library and
>> +  register a callback for the ExitBootServices event.
>> +
>> +  @param[in]  ImageHandle   The firmware allocated
>> handle for the EFI image.
>> +  @param[in]  SystemTable   A pointer to the EFI System
>> Table.
>> +
>> +  @retval EFI_SUCCESS   The operation completed
>> successfully.
>> +  @retval other         Either the serial port failed
>> to initialize or the
>> +                        ExitBootServices event callback
>> registration failed.
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +DxeRuntimeDebugLibSerialPortConstructor (
>> +  IN EFI_HANDLE        ImageHandle,
>> +  IN EFI_SYSTEM_TABLE  *SystemTable
>> +  )
>> +{
>> +  EFI_STATUS    Status;
>> +
>> +  mDebugClearMemoryValue      = PcdGet8
>> (PcdDebugClearMemoryValue);
>> +  mDebugPropertyMask          = PcdGet8
>> (PcdDebugPropertyMask);
>> +  mFixedDebugPrintErrorLevel  = PcdGet32
>> (PcdFixedDebugPrintErrorLevel);
>> +
>> +  Status = SerialPortInitialize ();
>> +  if (EFI_ERROR (Status)) {
>> +    return Status;
>> +  }
>> +
>> +  return SystemTable->BootServices->CreateEventEx
>> (EVT_NOTIFY_SIGNAL,
>> +                                      TPL_NOTIFY,
>> ExitBootServicesEvent, NULL,
>> +
>> &gEfiEventExitBootServicesGuid,
>> +
>> &mEfiExitBootServicesEvent);
>> +}
>> +
>> +/**
>> +  If a runtime driver exits with an error, it must call
>> this routine
>> +  to free the allocated resource before the exiting.
>> +
>> +  @param[in]  ImageHandle   The firmware allocated
>> handle for the EFI image.
>> +  @param[in]  SystemTable   A pointer to the EFI System
>> Table.
>> +
>> +  @retval     EFI_SUCCESS       The Runtime Driver Lib
>> shutdown successfully.
>> +  @retval     EFI_UNSUPPORTED   Runtime Driver lib was
>> not initialized.
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +DxeRuntimeDebugLibSerialPortDestructor (
>> +  IN EFI_HANDLE        ImageHandle,
>> +  IN EFI_SYSTEM_TABLE  *SystemTable
>> +  )
>> +{
>> +  return SystemTable->BootServices->CloseEvent
>> (mEfiExitBootServicesEvent);
>> +}
>> +
>> +/**
>> +  Prints a debug message to the debug output device if
>> the specified error level
>> +  is enabled.
>> +
>> +  If any bit in ErrorLevel is also set in
>> DebugPrintErrorLevelLib function
>> +  GetDebugPrintErrorLevel (), then print the message
>> specified by Format and the
>> +  associated variable argument list to the debug output
>> device.
>> +
>> +  If Format is NULL, then ASSERT().
>> +
>> +  @param  ErrorLevel  The error level of the debug
>> message.
>> +  @param  Format      Format string for the debug
>> message to print.
>> +  @param  ...         Variable argument list whose
>> contents are accessed
>> +                      based on the format string
>> specified by Format.
>> +
>> +**/
>> +VOID
>> +EFIAPI
>> +DebugPrint (
>> +  IN  UINTN        ErrorLevel,
>> +  IN  CONST CHAR8  *Format,
>> +  ...
>> +  )
>> +{
>> +  CHAR8    Buffer[MAX_DEBUG_MESSAGE_LENGTH];
>> +  VA_LIST  Marker;
>> +
>> +  if (mEfiAtRuntime) {
>> +    return;
>> +  }
>> +
>> +  ASSERT (Format != NULL);
>> +
>> +  //
>> +  // Check driver debug mask value and global mask
>> +  //
>> +  if ((ErrorLevel & GetDebugPrintErrorLevel ()) == 0) {
>> +    return;
>> +  }
>> +
>> +  //
>> +  // Convert the DEBUG() message to an ASCII String
>> +  //
>> +  VA_START (Marker, Format);
>> +  AsciiVSPrint (Buffer, sizeof (Buffer), Format,
>> Marker);
>> +  VA_END (Marker);
>> +
>> +  //
>> +  // Send the print string to a Serial Port
>> +  //
>> +  SerialPortWrite ((UINT8 *)Buffer, AsciiStrLen
>> (Buffer));
>> +}
>> +
>> +
>> +/**
>> +  Prints an assert message containing a filename, line
>> number, and description.
>> +  This may be followed by a breakpoint or a dead loop.
>> +
>> +  Print a message of the form "ASSERT
>> <FileName>(<LineNumber>): <Description>\n"
>> +  to the debug output device.  If
>> DEBUG_PROPERTY_ASSERT_BREAKPOINT_ENABLED bit
>> +  of PcdDebugProperyMask is set then CpuBreakpoint() is
>> called. Otherwise, if
>> +  DEBUG_PROPERTY_ASSERT_DEADLOOP_ENABLED bit of
>> PcdDebugProperyMask is set then
>> +  CpuDeadLoop() is called.  If neither of these bits
>> are set, then this function
>> +  returns immediately after the message is printed to
>> the debug output device.
>> +  DebugAssert() must actively prevent recursion.  If
>> DebugAssert() is called
>> +  while processing another DebugAssert(), then
>> DebugAssert() must return
>> +  immediately.
>> +
>> +  If FileName is NULL, then a <FileName> string of
>> "(NULL) Filename" is printed.
>> +  If Description is NULL, then a <Description> string
>> of "(NULL) Description" is
>> +  printed.
>> +
>> +  @param  FileName     The pointer to the name of the
>> source file that generated
>> +                       the assert condition.
>> +  @param  LineNumber   The line number in the source
>> file that generated the
>> +                       assert condition
>> +  @param  Description  The pointer to the description
>> of the assert condition.
>> +
>> +**/
>> +VOID
>> +EFIAPI
>> +DebugAssert (
>> +  IN CONST CHAR8  *FileName,
>> +  IN UINTN        LineNumber,
>> +  IN CONST CHAR8  *Description
>> +  )
>> +{
>> +  CHAR8  Buffer[MAX_DEBUG_MESSAGE_LENGTH];
>> +
>> +  if (!mEfiAtRuntime) {
>> +    //
>> +    // 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 the Console Output
>> device
>> +    //
>> +    SerialPortWrite ((UINT8 *)Buffer, AsciiStrLen
>> (Buffer));
>> +  }
>> +
>> +  //
>> +  // Generate a Breakpoint, DeadLoop, or NOP based on
>> PCD settings
>> +  //
>> +  if ((mDebugPropertyMask &
>> DEBUG_PROPERTY_ASSERT_BREAKPOINT_ENABLED) != 0) {
>> +    CpuBreakpoint ();
>> +  } else if ((mDebugPropertyMask &
>> +              DEBUG_PROPERTY_ASSERT_DEADLOOP_ENABLED)
>> != 0) {
>> +    CpuDeadLoop ();
>> +  }
>> +}
>> +
>> +
>> +/**
>> +  Fills a target buffer with PcdDebugClearMemoryValue,
>> and returns the target
>> +  buffer.
>> +
>> +  This function fills Length bytes of Buffer with the
>> value specified by
>> +  PcdDebugClearMemoryValue, and returns Buffer.
>> +
>> +  If Buffer is NULL, then ASSERT().
>> +  If Length is greater than (MAX_ADDRESS - Buffer + 1),
>> then ASSERT().
>> +
>> +  @param   Buffer  The pointer to the target buffer to
>> be filled with
>> +                   PcdDebugClearMemoryValue.
>> +  @param   Length  The number of bytes in Buffer to
>> fill with
>> +                   PcdDebugClearMemoryValue.
>> +
>> +  @return  Buffer  The pointer to the target buffer
>> filled with
>> +                   PcdDebugClearMemoryValue.
>> +
>> +**/
>> +VOID *
>> +EFIAPI
>> +DebugClearMemory (
>> +  OUT VOID  *Buffer,
>> +  IN UINTN  Length
>> +  )
>> +{
>> +  if (!mEfiAtRuntime) {
>> +    ASSERT (Buffer != NULL);
>> +    ASSERT (Length <= (MAX_ADDRESS - (UINTN)Buffer +
>> 1));
>> +  }
>> +
>> +  return SetMem (Buffer, Length,
>> mDebugClearMemoryValue);
>> +}
>> +
>> +
>> +/**
>> +  Returns TRUE if ASSERT() macros are enabled.
>> +
>> +  This function returns TRUE if the
>> DEBUG_PROPERTY_DEBUG_ASSERT_ENABLED bit of
>> +  PcdDebugProperyMask is set.  Otherwise FALSE is
>> returned.
>> +
>> +  @retval  TRUE    The
>> DEBUG_PROPERTY_DEBUG_ASSERT_ENABLED bit of
>> +                   PcdDebugProperyMask is set.
>> +  @retval  FALSE   The
>> DEBUG_PROPERTY_DEBUG_ASSERT_ENABLED bit of
>> +                   PcdDebugProperyMask is clear.
>> +
>> +**/
>> +BOOLEAN
>> +EFIAPI
>> +DebugAssertEnabled (
>> +  VOID
>> +  )
>> +{
>> +  return (BOOLEAN)((mDebugPropertyMask &
>> +
>> DEBUG_PROPERTY_DEBUG_ASSERT_ENABLED) != 0);
>> +}
>> +
>> +
>> +/**
>> +  Returns TRUE if DEBUG() macros are enabled.
>> +
>> +  This function returns TRUE if the
>> DEBUG_PROPERTY_DEBUG_PRINT_ENABLED bit of
>> +  PcdDebugProperyMask is set.  Otherwise FALSE is
>> returned.
>> +
>> +  @retval  TRUE    The
>> DEBUG_PROPERTY_DEBUG_PRINT_ENABLED bit of
>> +                   PcdDebugProperyMask is set.
>> +  @retval  FALSE   The
>> DEBUG_PROPERTY_DEBUG_PRINT_ENABLED bit of
>> +                   PcdDebugProperyMask is clear.
>> +
>> +**/
>> +BOOLEAN
>> +EFIAPI
>> +DebugPrintEnabled (
>> +  VOID
>> +  )
>> +{
>> +  return (BOOLEAN)((mDebugPropertyMask &
>> +                    DEBUG_PROPERTY_DEBUG_PRINT_ENABLED)
>> != 0);
>> +}
>> +
>> +
>> +/**
>> +  Returns TRUE if DEBUG_CODE() macros are enabled.
>> +
>> +  This function returns TRUE if the
>> DEBUG_PROPERTY_DEBUG_CODE_ENABLED bit of
>> +  PcdDebugProperyMask is set.  Otherwise FALSE is
>> returned.
>> +
>> +  @retval  TRUE    The
>> DEBUG_PROPERTY_DEBUG_CODE_ENABLED bit of
>> +                   PcdDebugProperyMask is set.
>> +  @retval  FALSE   The
>> DEBUG_PROPERTY_DEBUG_CODE_ENABLED bit of
>> +                   PcdDebugProperyMask is clear.
>> +
>> +**/
>> +BOOLEAN
>> +EFIAPI
>> +DebugCodeEnabled (
>> +  VOID
>> +  )
>> +{
>> +  return (BOOLEAN)((mDebugPropertyMask &
>> +                    DEBUG_PROPERTY_DEBUG_CODE_ENABLED)
>> != 0);
>> +}
>> +
>> +
>> +/**
>> +  Returns TRUE if DEBUG_CLEAR_MEMORY() macro is
>> enabled.
>> +
>> +  This function returns TRUE if the
>> DEBUG_PROPERTY_CLEAR_MEMORY_ENABLED bit of
>> +  PcdDebugProperyMask is set.  Otherwise FALSE is
>> returned.
>> +
>> +  @retval  TRUE    The
>> DEBUG_PROPERTY_CLEAR_MEMORY_ENABLED bit of
>> +                   PcdDebugProperyMask is set.
>> +  @retval  FALSE   The
>> DEBUG_PROPERTY_CLEAR_MEMORY_ENABLED bit of
>> +                   PcdDebugProperyMask is clear.
>> +
>> +**/
>> +BOOLEAN
>> +EFIAPI
>> +DebugClearMemoryEnabled (
>> +  VOID
>> +  )
>> +{
>> +  return (BOOLEAN)((mDebugPropertyMask &
>> +
>> DEBUG_PROPERTY_CLEAR_MEMORY_ENABLED) != 0);
>> +}
>> +
>> +/**
>> +  Returns TRUE if any one of the bit is set both in
>> ErrorLevel and
>> +  PcdFixedDebugPrintErrorLevel.
>> +
>> +  This function compares the bit mask of ErrorLevel and
>> +  PcdFixedDebugPrintErrorLevel.
>> +
>> +  @retval  TRUE    Current ErrorLevel is supported.
>> +  @retval  FALSE   Current ErrorLevel is not supported.
>> +
>> +**/
>> +BOOLEAN
>> +EFIAPI
>> +DebugPrintLevelEnabled (
>> +  IN  CONST UINTN        ErrorLevel
>> +  )
>> +{
>> +  return (BOOLEAN)((ErrorLevel &
>> mFixedDebugPrintErrorLevel) != 0);
>> +}
>> diff --git
>> a/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntime
>> DebugLibSerialPort.inf
>> b/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntime
>> DebugLibSerialPort.inf
>> new file mode 100644
>> index 000000000000..d5e9d3f8dee7
>> --- /dev/null
>> +++
>> b/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntime
>> DebugLibSerialPort.inf
>> @@ -0,0 +1,56 @@
>> +## @file
>> +#  DXE runtime Debug library instance based on Serial
>> Port library.
>> +#  It takes care not to call into SerialPortLib or
>> PcdLib after
>> +#  ExitBootServices() has been called, to prevent
>> touching hardware that is
>> +#  no longer owned by the firmware, or invoking
>> protocols that are not safe
>> +#  for runtime (such as gEfiPcdProtocolGuid)
>> +#
>> +#  Copyright (c) 2006 - 2015, Intel Corporation. All
>> rights reserved.<BR>
>> +#  Copyright (c) 2018, Linaro, Ltd. All rights
>> reserved.<BR>
>> +#
>> +#  This program and the accompanying materials
>> +#  are licensed and made available under the terms and
>> conditions of the BSD License
>> +#  which accompanies this distribution. The full text
>> of the license may be found at
>> +#  http://opensource.org/licenses/bsd-license.php.
>> +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON
>> AN "AS IS" BASIS,
>> +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND,
>> EITHER EXPRESS OR IMPLIED.
>> +#
>> +#
>> +##
>> +
>> +[Defines]
>> +  INF_VERSION                    = 0x0001001A
>> +  BASE_NAME                      =
>> DxeRuntimeDebugLibSerialPort
>> +  MODULE_UNI_FILE                =
>> DxeRuntimeDebugLibSerialPort.uni
>> +  FILE_GUID                      = 9D914E2F-7CCB-41DB-
>> 8E74-9AFF8F3BBFBF
>> +  MODULE_TYPE                    = DXE_RUNTIME_DRIVER
>> +  VERSION_STRING                 = 1.0
>> +  LIBRARY_CLASS                  =
>> DebugLib|DXE_RUNTIME_DRIVER
>> +  CONSTRUCTOR                    =
>> DxeRuntimeDebugLibSerialPortConstructor
>> +  DESTRUCTOR                     =
>> DxeRuntimeDebugLibSerialPortDestructor
>> +
>> +#
>> +#  VALID_ARCHITECTURES           = AARCH64 ARM IA32 X64
>> IPF EBC
>> +#
>> +
>> +[Sources]
>> +  DebugLib.c
>> +
>> +[Packages]
>> +  MdePkg/MdePkg.dec
>> +
>> +[LibraryClasses]
>> +  BaseLib
>> +  BaseMemoryLib
>> +  DebugPrintErrorLevelLib
>> +  PcdLib
>> +  PrintLib
>> +  SerialPortLib
>> +
>> +[Guids]
>> +  gEfiEventExitBootServicesGuid
>> ## CONSUMES ## Event
>> +
>> +[Pcd]
>> +  gEfiMdePkgTokenSpaceGuid.PcdDebugClearMemoryValue
>> ## CONSUMES
>> +  gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask
>> ## CONSUMES
>> +  gEfiMdePkgTokenSpaceGuid.PcdFixedDebugPrintErrorLevel
>> ## CONSUMES
>> diff --git
>> a/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntime
>> DebugLibSerialPort.uni
>> b/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntime
>> DebugLibSerialPort.uni
>> new file mode 100644
>> index 000000000000..cd65515c4177
>> --- /dev/null
>> +++
>> b/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntime
>> DebugLibSerialPort.uni
>> @@ -0,0 +1,21 @@
>> +// /** @file
>> +// Instance of Debug Library based on Serial Port
>> Library.
>> +//
>> +// It uses Print Library to produce formatted output
>> strings to seiral port device.
>> +//
>> +// Copyright (c) 2006 - 2014, Intel Corporation. All
>> rights reserved.<BR>
>> +//
>> +// This program and the accompanying materials
>> +// are licensed and made available under the terms and
>> conditions of the BSD License
>> +// which accompanies this distribution. The full text
>> of the license may be found at
>> +// http://opensource.org/licenses/bsd-license.php.
>> +// THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON
>> AN "AS IS" BASIS,
>> +// WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND,
>> EITHER EXPRESS OR IMPLIED.
>> +//
>> +// **/
>> +
>> +
>> +#string STR_MODULE_ABSTRACT             #language en-US
>> "Instance of Debug Library based on Serial Port Library"
>> +
>> +#string STR_MODULE_DESCRIPTION          #language en-US
>> "It uses Print Library to produce formatted output
>> strings to a serial port device."
>> +
>> --
>> 2.11.0
>


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

* Re: [PATCH edk2-platforms v2 1/2] MdePkg: introduce DxeRuntimeDebugLibSerialPort
  2018-02-22 16:28 ` [PATCH edk2-platforms v2 1/2] MdePkg: introduce DxeRuntimeDebugLibSerialPort Ard Biesheuvel
  2018-02-22 17:45   ` Ard Biesheuvel
  2018-02-22 17:46   ` Kinney, Michael D
@ 2018-02-22 18:15   ` Laszlo Ersek
  2 siblings, 0 replies; 8+ messages in thread
From: Laszlo Ersek @ 2018-02-22 18:15 UTC (permalink / raw)
  To: Ard Biesheuvel, edk2-devel
  Cc: ruiyu.ni, afish, liming.gao, leif.lindholm, michael.d.kinney,
	star.zeng

On 02/22/18 17:28, Ard Biesheuvel wrote:
> Introduce a variant of BaseDebugLibSerialPort that behaves correctly with
> regards to the use of the serial port after ExitBootServices(). Also, it
> caches PCD values in the constructor so that no calls into PcdLib are
> made at runtime.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  MdePkg/Library/DxeRuntimeDebugLibSerialPort/DebugLib.c                       | 373 ++++++++++++++++++++
>  MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.inf |  56 +++
>  MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.uni |  21 ++
>  3 files changed, 450 insertions(+)

> +/**
> +  If a runtime driver exits with an error, it must call this routine
> +  to free the allocated resource before the exiting.
> +
> +  @param[in]  ImageHandle   The firmware allocated handle for the EFI image.
> +  @param[in]  SystemTable   A pointer to the EFI System Table.
> +
> +  @retval     EFI_SUCCESS       The Runtime Driver Lib shutdown successfully.
> +  @retval     EFI_UNSUPPORTED   Runtime Driver lib was not initialized.
> +**/
> +EFI_STATUS
> +EFIAPI
> +DxeRuntimeDebugLibSerialPortDestructor (
> +  IN EFI_HANDLE        ImageHandle,
> +  IN EFI_SYSTEM_TABLE  *SystemTable
> +  )
> +{
> +  return SystemTable->BootServices->CloseEvent (mEfiExitBootServicesEvent);
> +}

The leading comments still refer to "Runtime Driver Lib", but that's
harmless.

The UNI file (STR_ contents and @file comment) have not been updated
with the special purpose of this library, but I can live with that.

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks!
LAszlo


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

* Re: [PATCH edk2-platforms v2 1/2] MdePkg: introduce DxeRuntimeDebugLibSerialPort
  2018-02-22 17:50     ` Ard Biesheuvel
@ 2018-02-22 18:26       ` Laszlo Ersek
  0 siblings, 0 replies; 8+ messages in thread
From: Laszlo Ersek @ 2018-02-22 18:26 UTC (permalink / raw)
  To: Ard Biesheuvel, Kinney, Michael D
  Cc: Ni, Ruiyu, edk2-devel@lists.01.org, afish@apple.com, Gao, Liming,
	leif.lindholm@linaro.org, Zeng, Star

On 02/22/18 18:50, Ard Biesheuvel wrote:
> On 22 February 2018 at 17:46, Kinney, Michael D
> <michael.d.kinney@intel.com> wrote:
>> Ard,
>>
>> The check for the runtime flag seems inconsistent.
>>
>> Since the issue being addressed is use of SerialPortLib,
>> can't the mEfiAtRuntime flag be use around SerialPortLib
>> calls only.
>>
> 
> I take it you mean the checks around ASSERT()s? I agree that those are
> redundant, since the check will occur in the ASSERT() implementation
> as well.
> 
> Will fix.

I noticed that as well, comparing the C file against its counterpart in
BaseDebugLibSerialPort. I thought it was harmless: DebugClearMemory()
should only be called via DEBUG_CLEAR_MEMORY(), and that only occurs
from MdeModulePkg/Core/Dxe/Mem, typically when memory (pool/pages) is
freed, or the memmap is changed similarly (IIUC when entries are
"converted"). None of those should happen at runtime; so I didn't think
we'd get a call to DebugClearMemory() with mEfiAtRuntime==TRUE.

Anyway I agree it's cleaner to fix that up.

Thanks
Laszlo


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

end of thread, other threads:[~2018-02-22 18:20 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-02-22 16:28 [PATCH edk2-platforms v2 0/2] Create UART DebugLib implementation for runtime drivers Ard Biesheuvel
2018-02-22 16:28 ` [PATCH edk2-platforms v2 1/2] MdePkg: introduce DxeRuntimeDebugLibSerialPort Ard Biesheuvel
2018-02-22 17:45   ` Ard Biesheuvel
2018-02-22 17:46   ` Kinney, Michael D
2018-02-22 17:50     ` Ard Biesheuvel
2018-02-22 18:26       ` Laszlo Ersek
2018-02-22 18:15   ` Laszlo Ersek
2018-02-22 16:28 ` [PATCH edk2-platforms v2 2/2] ArmVirtPkg: switch to DXE runtime version of DebugLib where appropriate Ard Biesheuvel

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