From: "Tomas Pilar (tpilar)" <Tomas.Pilar@arm.com>
To: <devel@edk2.groups.io>
Cc: <Sami.Mujawar@arm.com>, <nd@arm.com>, Ray Ni <ray.ni@intel.com>,
"Zhichao Gao" <zhichao.gao@intel.com>
Subject: [PATCH v2 4/8] ShellPkg/AcpiView: Create a logging facility
Date: Sun, 12 Jul 2020 11:32:11 +0100 [thread overview]
Message-ID: <20200712103215.855-5-Tomas.Pilar@arm.com> (raw)
In-Reply-To: <20200712103215.855-1-Tomas.Pilar@arm.com>
Extract error and warning logging into separate methods. Fold
highlighting and other output properties into the logging methods.
Change-Id: I46bba2afc6fe8d7bc0c92ec7054f2af2d2254441
Cc: Ray Ni <ray.ni@intel.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
Signed-off-by: Tomas Pilar <tomas.pilar@arm.com>
---
.../UefiShellAcpiViewCommandLib/AcpiParser.c | 5 +-
.../UefiShellAcpiViewCommandLib/AcpiViewLog.c | 338 ++++++++++++++++++
.../UefiShellAcpiViewCommandLib/AcpiViewLog.h | 192 ++++++++++
.../UefiShellAcpiViewCommandLib.inf | 2 +
4 files changed, 533 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..7ec276ac7528
--- /dev/null
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiViewLog.c
@@ -0,0 +1,338 @@
+/** @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>
+#include <Library/PrintLib.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);
+}
+
+/**
+ 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
+**/
+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);
+}
+
+/**
+ 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
+**/
+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;
+}
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiViewLog.h b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiViewLog.h
new file mode 100644
index 000000000000..ce276ef7add2
--- /dev/null
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiViewLog.h
@@ -0,0 +1,192 @@
+/** @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_
+
+/**
+ 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
+**/
+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
+ );
+
+// 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
+**/
+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
+ );
+
+// 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
+// Constraint internally cast to BOOLEAN using !!(Constraint)
+#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
next prev parent reply other threads:[~2020-07-12 10:32 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-12 10:32 [PATCH v2 0/8] ShellPkg/AcpiView: Refactor Error Logging Tomas Pilar (tpilar)
2020-07-12 10:32 ` [PATCH v2 1/8] ShellPkg/AcpiView: Extract configuration struct Tomas Pilar (tpilar)
2020-07-12 10:32 ` [PATCH v2 2/8] ShellPkg/AcpiView: Declutter error counters Tomas Pilar (tpilar)
2020-07-12 10:32 ` [PATCH v2 3/8] ShellPkg/AcpiView: Modify error message Tomas Pilar (tpilar)
2020-07-12 10:32 ` Tomas Pilar (tpilar) [this message]
2020-07-12 10:32 ` [PATCH v2 5/8] ShellPkg/AcpiView: Refactor PrintFieldName Tomas Pilar (tpilar)
2020-07-12 10:32 ` [PATCH v2 6/8] ShellPkg/AcpiView: Refactor dump helpers Tomas Pilar (tpilar)
2020-07-12 10:32 ` [PATCH v2 7/8] ShellPkg/AcpiView: Refactor AcpiView Tomas Pilar (tpilar)
2020-07-12 10:32 ` [PATCH v2 8/8] ShellPkg/AcpiView: Refactor table parsers Tomas Pilar (tpilar)
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=20200712103215.855-5-Tomas.Pilar@arm.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