Re: [PATCH v3 1/2] MdePkg: introduce DxeRuntimeDebugLibSerialPort

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On 22 February 2018 at 19:40, Kinney, Michael D
<michael.d.kinney@intel.com> wrote:
>
> Ard,
>
> In DebugAssert(), if you have deadloop or BP enabled for
> the ASSERT(), then it would be good to have the message
> available in the local variable Buffer.
>
> Perhaps only the call to SerialPortWrite() should be
> filtered.
>

Good point!


>
>> -----Original Message-----
>> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
>> Sent: Thursday, February 22, 2018 10:15 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 v3 1/2] MdePkg: introduce
>> DxeRuntimeDebugLibSerialPort
>>
>> Introduce a variant of BaseDebugLibSerialPort that
>> behaves correctly with
>> regards to the use of the serial port after
>> ExitBootServices(). At boot
>> time, all DEBUG() prints and ASSERT() invocations are
>> executed normally.
>> At runtime, DEBUG() prints are dropped entirely, and
>> ASSERT()s omit the
>> serial output as well, and only perform the configured
>> post-ASSERT()
>> action, i.e., issue a CPU breakpoint or enter a
>> deadloop.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel
>> <ard.biesheuvel@linaro.org>
>> ---
>>  MdePkg/Library/DxeRuntimeDebugLibSerialPort/DebugLib.c
>> | 346 ++++++++++++++++++++
>>
>> MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDe
>> bugLibSerialPort.inf |  55 ++++
>>
>> MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDe
>> bugLibSerialPort.uni |  21 ++
>>  3 files changed, 422 insertions(+)
>>
>> diff --git
>> a/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DebugLib.c
>> b/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DebugLib.c
>> new file mode 100644
>> index 000000000000..a987159d8fc8
>> --- /dev/null
>> +++
>> b/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DebugLib.c
>> @@ -0,0 +1,346 @@
>> +/** @file
>> +  DXE runtime Debug library instance based on Serial
>> Port library.
>> +  It takes care not to call into SerialPortLib after
>> ExitBootServices() has
>> +  been called, to prevent touching hardware that is no
>> longer owned by the
>> +  firmware.
>> +
>> +  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;
>> +
>> +//
>> +// 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;
>> +
>> +  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;
>> +  }
>> +
>> +  //
>> +  // If Format is NULL, then ASSERT().
>> +  //
>> +  ASSERT (Format != NULL);
>> +
>> +  //
>> +  // Check driver debug mask value and global mask
>> +  //
>> +  if ((ErrorLevel & GetDebugPrintErrorLevel ()) == 0) {
>> +    return;
>> +  }
>> +
>> +  //
>> +  // Convert the DEBUG() message to 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 ((PcdGet8(PcdDebugPropertyMask) &
>> DEBUG_PROPERTY_ASSERT_BREAKPOINT_ENABLED) != 0) {
>> +    CpuBreakpoint ();
>> +  } else if ((PcdGet8(PcdDebugPropertyMask) &
>> 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 zeros PcdDebugClearMemoryValue.
>> +
>> +  @return  Buffer  The pointer to the target buffer
>> filled with PcdDebugClearMemoryValue.
>> +
>> +**/
>> +VOID *
>> +EFIAPI
>> +DebugClearMemory (
>> +  OUT VOID  *Buffer,
>> +  IN UINTN  Length
>> +  )
>> +{
>> +  //
>> +  // If Buffer is NULL, then ASSERT().
>> +  //
>> +  ASSERT (Buffer != NULL);
>> +
>> +  //
>> +  // SetMem() checks for the the ASSERT() condition on
>> Length and returns Buffer
>> +  //
>> +  return SetMem (Buffer, Length,
>> PcdGet8(PcdDebugClearMemoryValue));
>> +}
>> +
>> +
>> +/**
>> +  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) ((PcdGet8(PcdDebugPropertyMask) &
>> 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) ((PcdGet8(PcdDebugPropertyMask) &
>> 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) ((PcdGet8(PcdDebugPropertyMask) &
>> 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) ((PcdGet8(PcdDebugPropertyMask) &
>> 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 &
>> PcdGet32(PcdFixedDebugPrintErrorLevel)) != 0);
>> +}
>> diff --git
>> a/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntime
>> DebugLibSerialPort.inf
>> b/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntime
>> DebugLibSerialPort.inf
>> new file mode 100644
>> index 000000000000..4b5157ca093c
>> --- /dev/null
>> +++
>> b/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntime
>> DebugLibSerialPort.inf
>> @@ -0,0 +1,55 @@
>> +## @file
>> +#  DXE runtime Debug library instance based on Serial
>> Port library.
>> +#  It takes care not to call into SerialPortLib after
>> ExitBootServices() has
>> +#  been called, to prevent touching hardware that is no
>> longer owned by the
>> +#  firmware.
>> +#
>> +#  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
>