public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Kinney, Michael D" <michael.d.kinney@intel.com>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>,
	"Kinney, Michael D" <michael.d.kinney@intel.com>
Cc: "Ni, Ruiyu" <ruiyu.ni@intel.com>,
	"afish@apple.com" <afish@apple.com>,
	"leif.lindholm@linaro.org" <leif.lindholm@linaro.org>,
	"Gao, Liming" <liming.gao@intel.com>,
	"lersek@redhat.com" <lersek@redhat.com>,
	"Zeng, Star" <star.zeng@intel.com>
Subject: Re: [PATCH 1/3] MdePkg: introduce DxeRuntimeDebugLibSerialPort
Date: Tue, 20 Feb 2018 19:22:59 +0000	[thread overview]
Message-ID: <E92EE9817A31E24EB0585FDF735412F5B8969D35@ORSMSX113.amr.corp.intel.com> (raw)
In-Reply-To: <20180220110524.9050-2-ard.biesheuvel@linaro.org>

Ard,

Both FixedAtBuild and PatchableInModule are
safe at runtime.  Should not limit to only 
FixedAtBuild.

It is also possible to use Dynamic and DynamicEx
from a runtime driver as long as the PCD values
are retrieved before ExitBootServices().

Mike

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-
> bounces@lists.01.org] On Behalf Of Ard Biesheuvel
> Sent: Tuesday, February 20, 2018 3:05 AM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Ard Biesheuvel
> <ard.biesheuvel@linaro.org>; afish@apple.com;
> leif.lindholm@linaro.org; Gao, Liming
> <liming.gao@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; lersek@redhat.com; Zeng,
> Star <star.zeng@intel.com>
> Subject: [edk2] [PATCH 1/3] MdePkg: introduce
> DxeRuntimeDebugLibSerialPort
> 
> Introduce a variant of BaseDebugLibSerialPort that
> behaves correctly wrt
> to use of the serial port after ExitBootServices().
> Also, it uses fixed
> PCDs for all the parameterized values 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
> | 342 ++++++++++++++++++++
> 
> MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDe
> bugLibSerialPort.inf |  46 +++
> 
> MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDe
> bugLibSerialPort.uni |  21 ++
>  3 files changed, 409 insertions(+)
> 
> diff --git
> a/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DebugLib.c
> b/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DebugLib.c
> new file mode 100644
> index 000000000000..d18267d91322
> --- /dev/null
> +++
> b/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DebugLib.c
> @@ -0,0 +1,342 @@
> +/** @file
> +  DXE runtime Debug library instance based on Serial
> Port library.
> +  It uses PrintLib to send debug messages to serial
> port device.
> +
> +  NOTE: If the Serial Port library enables hardware
> flow control, then a call
> +  to DebugPrint() or DebugAssert() may hang if writes
> to the serial port are
> +  being blocked.  This may occur if a key(s) are
> pressed in a terminal emulator
> +  used to monitor the DEBUG() and ASSERT() messages.
> +
> +  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
> +RuntimeLibExitBootServicesEvent (
> +  IN EFI_EVENT        Event,
> +  IN VOID             *Context
> +  )
> +{
> +  mEfiAtRuntime = TRUE;
> +}
> +
> +/**
> +  The constructor function initialize the Serial Port
> Library
> +
> +  @retval EFI_SUCCESS   The constructor always returns
> RETURN_SUCCESS.
> +
> +**/
> +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,
> +
> RuntimeLibExitBootServicesEvent,
> +                                      NULL,
> +
> &gEfiEventExitBootServicesGuid,
> +
> &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;
> +  }
> +
> +  //
> +  // 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 ((FixedPcdGet8 (PcdDebugPropertyMask) &
> +       DEBUG_PROPERTY_ASSERT_BREAKPOINT_ENABLED) != 0)
> {
> +    CpuBreakpoint ();
> +  } else if ((FixedPcdGet8 (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
> +                   PcdDebugClearMemoryValue.
> +
> +  @return  Buffer  The pointer to the target buffer
> filled with
> +                   PcdDebugClearMemoryValue.
> +
> +**/
> +VOID *
> +EFIAPI
> +DebugClearMemory (
> +  OUT VOID  *Buffer,
> +  IN UINTN  Length
> +  )
> +{
> +  //
> +  // SetMem() checks for the the ASSERT() condition on
> Length and returns Buffer
> +  //
> +  return SetMem (Buffer, Length, FixedPcdGet8
> (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 (FixedPcdGet8 (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 (FixedPcdGet8 (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 (FixedPcdGet8 (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 (FixedPcdGet8 (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 (ErrorLevel & FixedPcdGet32
> (PcdFixedDebugPrintErrorLevel)) != 0;
> +}
> diff --git
> a/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntime
> DebugLibSerialPort.inf
> b/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntime
> DebugLibSerialPort.inf
> new file mode 100644
> index 000000000000..9f300f4f1b12
> --- /dev/null
> +++
> b/MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntime
> DebugLibSerialPort.inf
> @@ -0,0 +1,46 @@
> +## @file
> +#
> +#  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                    = 0x00010005
> +  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
> +  CONSTRUCTOR                    =
> DxeRuntimeDebugLibSerialPortConstructor
> +
> +[Sources]
> +  DebugLib.c
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +
> +[LibraryClasses]
> +  BaseLib
> +  BaseMemoryLib
> +  DebugPrintErrorLevelLib
> +  PcdLib
> +  PrintLib
> +  SerialPortLib
> +
> +[Guids]
> +  gEfiEventExitBootServicesGuid
> +
> +[FixedPcd]
> +  gEfiMdePkgTokenSpaceGuid.PcdDebugClearMemoryValue
> ## SOMETIMES_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
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


  parent reply	other threads:[~2018-02-20 19:17 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-20 11:05 [PATCH 0/3] Create UART DebugLib implementation for runtime drivers Ard Biesheuvel
2018-02-20 11:05 ` [PATCH 1/3] MdePkg: introduce DxeRuntimeDebugLibSerialPort Ard Biesheuvel
2018-02-20 14:16   ` Leif Lindholm
2018-02-20 16:20     ` Andrew Fish
2018-02-20 17:08       ` Leif Lindholm
2018-02-20 16:35   ` Laszlo Ersek
2018-02-20 19:22   ` Kinney, Michael D [this message]
2018-02-22 15:21     ` Ard Biesheuvel
2018-02-20 11:05 ` [PATCH 2/3] ArmVirtPkg: switch to DXE runtime version of DebugLib where appropriate Ard Biesheuvel
2018-02-20 16:44   ` Laszlo Ersek
2018-02-20 11:05 ` [PATCH 3/3] MdePkg/BaseDebugLibSerialPort: blacklist for use by DXE runtime drivers Ard Biesheuvel
2018-02-20 14:22   ` Leif Lindholm
2018-02-20 16:59     ` Laszlo Ersek
2018-02-20 18:02       ` Kinney, Michael D
2018-02-21 10:27         ` Laszlo Ersek

Reply instructions:

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

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

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

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

  git send-email \
    --in-reply-to=E92EE9817A31E24EB0585FDF735412F5B8969D35@ORSMSX113.amr.corp.intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

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

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