See below.
> -----Original Message-----
> From: Tomas Pilar <tomas.pi...@arm.com>
> Sent: Monday, June 29, 2020 11:20 PM
> To: devel@edk2.groups.io
> Cc: sami.muja...@arm.com; n...@arm.com; Ni, Ray <ray...@intel.com>; Gao,
> Zhichao <zhichao....@intel.com>
> Subject: [PATCH 4/8] ShellPkg/AcpiView: Create a logging facility
>
> Extract error and warning logging into separate methods. Fold highlighting and
> other output properties into the logging methods.
>
> Cc: Ray Ni <ray...@intel.com>
> Cc: Zhichao Gao <zhichao....@intel.com>
> Signed-off-by: Tomas Pilar <tomas.pi...@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 ...
> +)
Please align the ')'.
> +{
> + 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,
> + ...)
Align the ')' in the next line.
> +{
> + 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,
> + ...
> + );
Align the parameter Format with other parameters.
> +
> +/**
> + 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;
> +}
For this two inline functions:
Refer to CCS_2_1 Section 5.1.9: Do not use the in-line assembler.
Refer to CCS_2_1 Section 5.3.7: include file shall not generate the code or
define data variables.
Thanks,
Zhichao
> +
> +// 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/UefiShellAcpiViewCommandLi
> b.inf
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLi
> b.inf
> index 91459f9ec632..e0586cbccec2 100644
> ---
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLi
> b.inf
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComm
> +++ andLib.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 (#62332): https://edk2.groups.io/g/devel/message/62332
Mute This Topic: https://groups.io/mt/75193749/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
