[edk2-devel] [PATCH 4/8] ShellPkg/AcpiView: Create a logging facility

Tomas Pilar (tpilar) Tomas.Pilar at arm.com
Mon Jun 29 15:20:04 UTC 2020


Extract error and warning logging into separate methods. Fold
highlighting and other output properties into the logging methods.

Cc: Ray Ni <ray.ni at intel.com>
Cc: Zhichao Gao <zhichao.gao at intel.com>
Signed-off-by: Tomas Pilar <tomas.pilar at arm.com>
---
 .../UefiShellAcpiViewCommandLib/AcpiParser.c  |   5 +-
 .../UefiShellAcpiViewCommandLib/AcpiViewLog.c | 230 +++++++++++++++++
 .../UefiShellAcpiViewCommandLib/AcpiViewLog.h | 233 ++++++++++++++++++
 .../UefiShellAcpiViewCommandLib.inf           |   2 +
 4 files changed, 466 insertions(+), 4 deletions(-)
 create mode 100644 ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiViewLog.c
 create mode 100644 ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiViewLog.h

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
index 7017fa93efae..b88594cf3865 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
@@ -11,13 +11,10 @@
 #include "AcpiParser.h"
 #include "AcpiView.h"
 #include "AcpiViewConfig.h"
+#include "AcpiViewLog.h"
 
 STATIC UINT32   gIndent;
 
-// Publicly accessible error and warning counters.
-UINT32   mTableErrorCount;
-UINT32   mTableWarningCount;
-
 STATIC ACPI_DESCRIPTION_HEADER_INFO AcpiHdrInfo;
 
 /**
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiViewLog.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiViewLog.c
new file mode 100644
index 000000000000..9b9aaa855fdc
--- /dev/null
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiViewLog.c
@@ -0,0 +1,230 @@
+/** @file
+  'acpiview' logging and output facility
+
+  Copyright (c) 2020, ARM Limited. All rights reserved.<BR>
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include "AcpiViewLog.h"
+#include "AcpiViewConfig.h"
+#include "AcpiParser.h"
+#include <Library/UefiBootServicesTableLib.h>
+
+static CHAR16 mOutputBuffer [MAX_OUTPUT_SIZE] = { 0 };
+
+// String descriptions of error types
+static const CHAR16* mErrorTypeDesc [ACPI_ERROR_MAX] = {
+  L"Not an error",        ///< ACPI_ERROR_NONE
+  L"Generic",             ///< ACPI_ERROR_GENERIC
+  L"Checksum",            ///< ACPI_ERROR_CSUM
+  L"Parsing",             ///< ACPI_ERROR_PARSE
+  L"Length",              ///< ACPI_ERROR_LENGTH
+  L"Value",               ///< ACPI_ERROR_VALUE
+  L"Cross-check",         ///< ACPI_ERROR_CROSS
+};
+
+// Publicly accessible error and warning counters.
+UINT32   mTableErrorCount;
+UINT32   mTableWarningCount;
+
+/**
+  Change the attributes of the standard output console
+  to change the colour of the text according to the given
+  severity of a log message.
+
+  @param[in] Severity          The severity of the log message that is being
+                               annotated with changed colour text.
+  @param[in] OriginalAttribute The current attributes of ConOut that will be modified.
+**/
+static
+VOID
+EFIAPI
+ApplyColor (
+  IN ACPI_LOG_SEVERITY Severity,
+  IN UINTN             OriginalAttribute
+  )
+{
+  if (!mConfig.ColourHighlighting) {
+    return;
+  }
+
+  // Strip the foreground colour
+  UINTN NewAttribute = OriginalAttribute & 0xF0;
+
+  // Add specific foreground colour based on severity
+  switch (Severity) {
+  case ACPI_DEBUG:
+    NewAttribute |= EFI_DARKGRAY;
+    break;
+  case ACPI_HIGHLIGHT:
+    NewAttribute |= EFI_LIGHTBLUE;
+    break;
+  case ACPI_GOOD:
+    NewAttribute |= EFI_GREEN;
+    break;
+  case ACPI_ITEM:
+  case ACPI_WARN:
+    NewAttribute |= EFI_YELLOW;
+    break;
+  case ACPI_BAD:
+  case ACPI_ERROR:
+  case ACPI_FATAL:
+    NewAttribute |= EFI_RED;
+    break;
+  case ACPI_INFO:
+  default:
+    NewAttribute |= OriginalAttribute;
+    break;
+  }
+
+  gST->ConOut->SetAttribute (gST->ConOut, NewAttribute);
+}
+
+/**
+  Restore ConOut text attributes.
+
+  @param[in] OriginalAttribute The attribute set that will be restored.
+**/
+static
+VOID
+EFIAPI
+RestoreColor(
+  IN UINTN OriginalAttribute
+  )
+{
+  gST->ConOut->SetAttribute (gST->ConOut, OriginalAttribute);
+}
+
+/**
+  Formats and prints an ascii string to screen.
+
+  @param[in] Format String that will be formatted and printed.
+  @param[in] Marker The marker for variable parameters to be formatted.
+**/
+static
+VOID
+EFIAPI
+AcpiViewVSOutput (
+  IN const CHAR16  *Format,
+  IN VA_LIST Marker
+  )
+{
+  UnicodeVSPrint (mOutputBuffer, sizeof(mOutputBuffer), Format, Marker);
+  gST->ConOut->OutputString(gST->ConOut, mOutputBuffer);
+}
+
+/**
+  Formats and prints and ascii string to screen.
+
+  @param[in] Format String that will be formatted and printed.
+  @param[in] ...    A variable number of parameters that will be formatted.
+**/
+VOID
+EFIAPI
+AcpiViewOutput (
+  IN const CHAR16 *Format,
+  IN ...
+)
+{
+  VA_LIST Marker;
+  VA_START (Marker, Format);
+
+  AcpiViewVSOutput (Format, Marker);
+
+  VA_END (Marker);
+}
+
+
+/**
+  Prints the base file name given a full file path.
+
+  @param[in] FullName Fully qualified file path
+**/
+VOID
+EFIAPI
+PrintFileName (
+  IN const CHAR8* FullName
+  )
+{
+  const CHAR8* Cursor;
+  UINTN Size;
+
+  Cursor = FullName;
+  Size = 0;
+
+  // Find the end point of the string.
+  while (*Cursor && Cursor < FullName + MAX_OUTPUT_SIZE)
+    Cursor++;
+
+  // Find the rightmost path separator.
+  while (*Cursor != '\\' && *Cursor != '/' && Cursor > FullName) {
+    Cursor--;
+    Size++;
+  }
+
+  // Print base file name.
+  AcpiViewOutput (L"%.*a", Size - 1, Cursor + 1);
+}
+
+/**
+  AcpiView output and logging function. Will log the event to
+  configured output (currently screen) and annotate with colour
+  and extra metadata.
+
+  @param[in] FileName      The full filename of the source file where this
+                           event occured.
+  @param[in] FunctionName  The name of the function where this event occured.
+  @param[in] LineNumber    The line number in the source code where this event
+                           occured.
+  @param[in] Severity      The severity of the event that occured.
+  @param[in] Format        The format of the string describing the event.
+  @param[in] ...           The variable number of parameters that will format the
+                           string supplied in Format.
+**/
+VOID
+EFIAPI
+AcpiViewLog (
+  IN const CHAR8       *FileName,
+  IN const CHAR8       *FunctionName,
+  IN UINTN             LineNumber,
+  IN ACPI_ERROR_TYPE   Error,
+  IN ACPI_LOG_SEVERITY Severity,
+  IN const CHAR16      *Format,
+  ...)
+{
+  VA_LIST         Marker;
+  UINTN           OriginalAttribute;
+
+  OriginalAttribute = gST->ConOut->Mode->Attribute;
+  ApplyColor (Severity, OriginalAttribute);
+  VA_START (Marker, Format);
+
+  switch (Severity) {
+  case ACPI_FATAL:
+    AcpiViewOutput (L"FATAL ");
+    break;
+  case ACPI_ERROR:
+    AcpiViewOutput (L"ERROR[%s] ", mErrorTypeDesc[Error]);
+    mTableErrorCount++;
+    break;
+  case ACPI_WARN:
+    AcpiViewOutput (L"WARN ");
+    mTableWarningCount++;
+    break;
+  default:
+    break;
+  }
+
+  if (Severity >= ACPI_WARN) {
+    AcpiViewOutput (L"(");
+    PrintFileName (FileName);
+    AcpiViewOutput (L":%d) ", LineNumber);
+  }
+
+  AcpiViewVSOutput (Format, Marker);
+  AcpiViewOutput (L"\n");
+
+  VA_END (Marker);
+  RestoreColor (OriginalAttribute);
+}
+
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiViewLog.h b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiViewLog.h
new file mode 100644
index 000000000000..77049cd8eec2
--- /dev/null
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiViewLog.h
@@ -0,0 +1,233 @@
+/** @file
+  Header file for 'acpiview' logging and output facility
+
+  Copyright (c) 2020, ARM Limited. All rights reserved.<BR>
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#ifndef ACPI_VIEW_LOG_H_
+#define ACPI_VIEW_LOG_H_
+
+#include <Library/PrintLib.h>
+
+/*
+  Categories of errors that can be logged by AcpiView.
+*/
+typedef enum {
+  ACPI_ERROR_NONE,        ///< Not an error
+  ACPI_ERROR_GENERIC,     ///< An unspecified error
+  ACPI_ERROR_CSUM,        ///< A checksum was invalid
+  ACPI_ERROR_PARSE,       ///< Failed to parse item
+  ACPI_ERROR_LENGTH,      ///< Size of a thing is incorrect
+  ACPI_ERROR_VALUE,       ///< The value of a field was incorrect
+  ACPI_ERROR_CROSS,       ///< A constraint on multiple items was violated
+  ACPI_ERROR_MAX
+} ACPI_ERROR_TYPE;
+
+/*
+  Different severities of events that can be logged.
+*/
+typedef enum {
+  ACPI_DEBUG,       ///< Will not be shown unless specified on command line
+  ACPI_INFO,        ///< Standard output
+  ACPI_GOOD,        ///< Unspecified good outcome, green colour
+  ACPI_BAD,         ///< Unspecified bad outcome, red colour
+  ACPI_ITEM,        ///< Used when describing multiple items
+  ACPI_HIGHLIGHT,   ///< A new context or section has been entered
+  ACPI_WARN,        ///< An unusual event happened
+  ACPI_ERROR,       ///< Acpi table is not conformant
+  ACPI_FATAL        ///< This will abort program execution.
+} ACPI_LOG_SEVERITY;
+
+// Publicly accessible error and warning counters.
+extern UINT32   mTableErrorCount;
+extern UINT32   mTableWarningCount;
+
+/**
+  AcpiView output and logging function. Will log the event to
+  configured output (currently screen) and annotate with colour
+  and extra metadata.
+
+  @param[in] FileName      The full filename of the source file where this
+                           event occured.
+  @param[in] FunctionName  The name of the function where this event occured.
+  @param[in] LineNumber    The line number in the source code where this event
+                           occured.
+  @param[in] Severity      The severity of the event that occured.
+  @param[in] Error         The type of the erorr reported. May be ACPI_ERROR_NONE if the event
+                           is not an error.
+  @param[in] Format        The format of the string describing the event.
+  @param[in] ...           The variable number of parameters that will format the
+                           string supplied in Format.
+**/
+VOID
+EFIAPI
+AcpiViewLog (
+  IN const CHAR8       *FileName,
+  IN const CHAR8       *FunctionName,
+  IN UINTN             LineNumber,
+  IN ACPI_ERROR_TYPE   Error,
+  IN ACPI_LOG_SEVERITY Severity,
+  IN const CHAR16            *Format,
+  ...
+  );
+
+/**
+  Formats and prints and ascii string to screen.
+
+  @param[in] Format String that will be formatted and printed.
+  @param[in] ...    A variable number of parameters that will be formatted.
+**/
+VOID
+EFIAPI
+AcpiViewOutput (
+  IN const CHAR16 *Format,
+  IN ...
+  );
+
+/**
+  Check that a buffer has not been overrun by a member. Can be invoked
+  using the BufferOverrun macro that fills in local source metadata
+  (first three parameters) for logging purposes.
+
+  @param[in] FileName        Source file where this invocation is made
+  @param[in] FunctionName    Name of the local symbol
+  @param[in] LineNumber      Source line number of the local call
+  @param[in] ItemDescription User friendly name for the member being checked
+  @param[in] Position        Memory address of the member
+  @param[in] Length          Length of the member
+  @param[in] Buffer          Memory address of the buffer where member resides
+  @param[in] BufferSize      Size of the buffer where member resides
+
+  @retval TRUE               Buffer was overrun
+  @retval FALSE              Buffer is safe
+**/
+static
+inline
+BOOLEAN
+EFIAPI
+MemberIntegrityInternal (
+  IN const CHAR8  *FileName,
+  IN const CHAR8  *FunctionName,
+  IN UINTN        LineNumber,
+  IN const CHAR8 *ItemDescription,
+  IN UINTN        Offset,
+  IN UINTN        Length,
+  IN VOID         *Buffer,
+  IN UINTN        BufferSize)
+{
+  if (Length == 0) {
+    AcpiViewLog (
+      FileName,
+      FunctionName,
+      LineNumber,
+      ACPI_ERROR_LENGTH,
+      ACPI_ERROR,
+      L"%a at %p in buffer %p+%x has zero size!",
+      ItemDescription,
+      (UINT8 *)Buffer + Offset,
+      Buffer,
+      BufferSize);
+    return TRUE;
+  }
+
+  if (Offset + Length > BufferSize) {
+    AcpiViewLog (
+      FileName,
+      FunctionName,
+      LineNumber,
+      ACPI_ERROR_LENGTH,
+      ACPI_ERROR,
+      L"%a %p+%x overruns buffer %p+%x",
+      ItemDescription,
+      (UINT8 *) Buffer + Offset,
+      Length,
+      Buffer,
+      BufferSize);
+  }
+
+  return (Offset + Length > BufferSize);
+}
+
+// Determine if a member located at Offset into a Buffer lies entirely
+// within the BufferSize given the member size is Length
+// Evaluates to TRUE and logs error if buffer is overrun or if Length is zero
+#define AssertMemberIntegrity(Offset, Length, Buffer, BufferSize) \
+  MemberIntegrityInternal (__FILE__, __func__, __LINE__, #Length, Offset, Length, Buffer, BufferSize)
+
+
+/**
+  Checks that a boolean constraint evaluates correctly. Can be invoked
+  using the CheckConstraint macro that fills in the source code metadata.
+
+  @param[in] FileName        Source file where this invocation is made
+  @param[in] FunctionName    Name of the local symbol
+  @param[in] LineNumber      Source line number of the local call
+  @param[in] ConstraintText  The Source code of the constraint
+  @param[in] Specification   The specification that imposes the constraint
+  @param[in] Constraint      The actual constraint
+  @
+
+  @retval TRUE               Constraint is violated
+  @retval FALSE              Constraint is not violated
+**/
+static
+inline
+BOOLEAN
+EFIAPI
+CheckConstraintInternal (
+  IN const CHAR8       *FileName,
+  IN const CHAR8       *FunctionName,
+  IN UINTN             LineNumber,
+  IN const CHAR8       *ConstraintText,
+  IN const CHAR16      *Specification,
+  IN BOOLEAN           Constraint,
+  IN ACPI_LOG_SEVERITY Severity
+)
+{
+  if (!Constraint) {
+    AcpiViewLog (
+      FileName,
+      FunctionName,
+      LineNumber,
+      Severity == ACPI_ERROR ? ACPI_ERROR_VALUE : ACPI_ERROR_NONE,
+      Severity,
+      L"(%a) Constraint was violated: %a",
+      Specification,
+      ConstraintText);
+  }
+
+  // Return TRUE if constraint was violated
+  return !Constraint;
+}
+
+// Evaluates to TRUE and logs error if a constraint is violated
+// Constraint internally cast to BOOLEAN using !!(Constraint)
+#define AssertConstraint(Specification, Constraint) \
+  CheckConstraintInternal (                           \
+    __FILE__, __func__, __LINE__, #Constraint, Specification, !!(Constraint), ACPI_ERROR)
+
+// Evaluates to TRUE and logs error if a constraint is violated
+#define WarnConstraint(Specification, Constraint) \
+  CheckConstraintInternal (                           \
+    __FILE__, __func__, __LINE__, #Constraint, Specification, Constraint, ACPI_WARN)
+
+
+// Maximum string size that can be printed
+#define MAX_OUTPUT_SIZE 256
+
+#define _AcpiLog(...) AcpiViewLog(__FILE__, __func__, __LINE__, __VA_ARGS__)
+
+// Generic Loging macro, needs severity and formatted string
+#define AcpiLog(Severity, ...) _AcpiLog(ACPI_ERROR_NONE, Severity, __VA_ARGS__)
+
+// Log undecorated text, needs formatted string
+#define AcpiInfo(...) _AcpiLog(ACPI_ERROR_NONE, ACPI_INFO, __VA_ARGS__)
+
+// Log error and increment counter, needs error type and formatted string
+#define AcpiError(Error, ...) _AcpiLog(Error, ACPI_ERROR, __VA_ARGS__)
+
+// Log a FATAL error, needs formatted string
+#define AcpiFatal(...) _AcpiLog(ACPI_ERROR_GENERIC, ACPI_FATAL, __VA_ARGS__)
+
+#endif // ACPI_VIEW_LOG_H_
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.inf b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.inf
index 91459f9ec632..e0586cbccec2 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.inf
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.inf
@@ -27,6 +27,8 @@ [Sources.common]
   AcpiView.h
   AcpiViewConfig.c
   AcpiViewConfig.h
+  AcpiViewLog.h
+  AcpiViewLog.c
   Parsers/Bgrt/BgrtParser.c
   Parsers/Dbg2/Dbg2Parser.c
   Parsers/Dsdt/DsdtParser.c
-- 
2.24.1.windows.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61809): https://edk2.groups.io/g/devel/message/61809
Mute This Topic: https://groups.io/mt/75193749/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-





More information about the edk2-devel-archive mailing list