public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v2 0/1] Adds PrintFormatter function to the FADT flags field.
@ 2021-11-17 12:29 Abdul Lateef Attar
  2021-11-17 12:29 ` [PATCH v2 1/1] ShellPkg/AcpiView: PrintFormatter for FADT Flags field Abdul Lateef Attar
  0 siblings, 1 reply; 3+ messages in thread
From: Abdul Lateef Attar @ 2021-11-17 12:29 UTC (permalink / raw)
  To: devel; +Cc: Ray Ni, Zhichao Gao, Sami Mujawar

Hi Zhichao, Sami,
	Thank you very much for quick review comments.
I had made changes according to review comments and added
ParseAcpiBitFields() to parse the BitFields.

REF: https://github.com/abdattar/edk2/tree/FadtFlagsParser

Cc: Ray Ni <ray.ni@intel.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Abdul Lateef Attar (1):
  ShellPkg/AcpiView: PrintFormatter for FADT Flags field

 ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h              |  38 +++++
 ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c              | 177 ++++++++++++++++++++
 ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser.c |  59 ++++++-
 3 files changed, 273 insertions(+), 1 deletion(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH v2 1/1] ShellPkg/AcpiView: PrintFormatter for FADT Flags field
  2021-11-17 12:29 [PATCH v2 0/1] Adds PrintFormatter function to the FADT flags field Abdul Lateef Attar
@ 2021-11-17 12:29 ` Abdul Lateef Attar
  2021-11-18 10:44   ` Sami Mujawar
  0 siblings, 1 reply; 3+ messages in thread
From: Abdul Lateef Attar @ 2021-11-17 12:29 UTC (permalink / raw)
  To: devel; +Cc: Ray Ni, Zhichao Gao, Sami Mujawar

Adds PrintFormatter function to the FADT flags field.
Prints indivisual flag name along with flag value.
Adds new parser function ParseAcpiBitFields() to handle
bit fields.

Cc: Ray Ni <ray.ni@intel.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Signed-off-by: Abdul Lateef Attar <abdattar@amd.com>
---
 ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h              |  38 +++++
 ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c              | 177 ++++++++++++++++++++
 ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser.c |  59 ++++++-
 3 files changed, 273 insertions(+), 1 deletion(-)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
index 5e34a70c8bae..444a977aced8 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
@@ -2,6 +2,7 @@
   Header file for ACPI parser
 
   Copyright (c) 2016 - 2020, Arm Limited. All rights reserved.
+  Copyright (c) 2021, AMD Incorporated. All rights reserved.
   SPDX-License-Identifier: BSD-2-Clause-Patent
 **/
 
@@ -365,6 +366,43 @@ ParseAcpi (
   IN UINT32             ParserItems
   );
 
+/**
+  This function is used to parse an ACPI table bitfield buffer.
+
+  The ACPI table buffer is parsed using the ACPI table parser information
+  specified by a pointer to an array of ACPI_PARSER elements. This parser
+  function iterates through each item on the ACPI_PARSER array and logs the ACPI table bitfields.
+
+  This function can optionally be used to parse ACPI tables and fetch specific
+  field values. The ItemPtr member of the ACPI_PARSER structure (where used)
+  is updated by this parser function to point to the selected field data
+  (e.g. useful for variable length nested fields).
+
+  @param [in] Trace        Trace the ACPI fields TRUE else only parse the
+                           table.
+  @param [in] Indent       Number of spaces to indent the output.
+  @param [in] AsciiName    Optional pointer to an ASCII string that describes
+                           the table being parsed.
+  @param [in] Ptr          Pointer to the start of the buffer.
+  @param [in] Length       Length of the buffer pointed by Ptr.
+  @param [in] Parser       Pointer to an array of ACPI_PARSER structure that
+                           describes the table being parsed.
+  @param [in] ParserItems  Number of items in the ACPI_PARSER array.
+
+  @retval Number of bits parsed.
+**/
+UINT32
+EFIAPI
+ParseAcpiBitFields (
+  IN BOOLEAN            Trace,
+  IN UINT32             Indent,
+  IN CONST CHAR8*       AsciiName OPTIONAL,
+  IN UINT8*             Ptr,
+  IN UINT32             Length,
+  IN CONST ACPI_PARSER* Parser,
+  IN UINT32             ParserItems
+);
+
 /**
    This is a helper macro to pass parameters to the Parser functions.
 
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
index 74056e72c359..33a9e3d1e97a 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
@@ -2,12 +2,14 @@
   ACPI parser
 
   Copyright (c) 2016 - 2021, Arm Limited. All rights reserved.
+  Copyright (c) 2021, AMD Incorporated. All rights reserved.
   SPDX-License-Identifier: BSD-2-Clause-Patent
 **/
 
 #include <Uefi.h>
 #include <Library/UefiLib.h>
 #include <Library/UefiBootServicesTableLib.h>
+#include <Library/BaseMemoryLib.h>
 #include "AcpiParser.h"
 #include "AcpiView.h"
 #include "AcpiViewConfig.h"
@@ -738,3 +740,178 @@ ParseAcpiHeader (
 
   return BytesParsed;
 }
+
+/**
+  This function is used to parse an ACPI table bitfield buffer.
+
+  The ACPI table buffer is parsed using the ACPI table parser information
+  specified by a pointer to an array of ACPI_PARSER elements. This parser
+  function iterates through each item on the ACPI_PARSER array and logs the ACPI table bitfields.
+
+  This function can optionally be used to parse ACPI tables and fetch specific
+  field values. The ItemPtr member of the ACPI_PARSER structure (where used)
+  is updated by this parser function to point to the selected field data
+  (e.g. useful for variable length nested fields).
+
+  @param [in] Trace        Trace the ACPI fields TRUE else only parse the
+                           table.
+  @param [in] Indent       Number of spaces to indent the output.
+  @param [in] AsciiName    Optional pointer to an ASCII string that describes
+                           the table being parsed.
+  @param [in] Ptr          Pointer to the start of the buffer.
+  @param [in] Length       Length in bytes of the buffer pointed by Ptr.
+  @param [in] Parser       Pointer to an array of ACPI_PARSER structure that
+                           describes the table being parsed.
+  @param [in] ParserItems  Number of items in the ACPI_PARSER array.
+
+  @retval Number of bits parsed.
+**/
+UINT32
+EFIAPI
+ParseAcpiBitFields (
+  IN BOOLEAN            Trace,
+  IN UINT32             Indent,
+  IN CONST CHAR8*       AsciiName OPTIONAL,
+  IN UINT8*             Ptr,
+  IN UINT32             Length,
+  IN CONST ACPI_PARSER* Parser,
+  IN UINT32             ParserItems
+)
+{
+  UINT32  Index;
+  UINT32  Offset;
+  BOOLEAN HighLight;
+  UINTN   OriginalAttribute;
+
+  UINT64 Data;
+  UINT64 BitsData;
+
+  if (Length <= 0 || Length > 8) {
+    IncrementErrorCount ();
+    Print (
+        L"\nERROR: Bitfield Length(%d) is exceeding the 64bits limit \n",
+        Length
+        );
+    return 0;
+  }
+  //
+  // set local variables to suppress incorrect compiler/analyzer warnings
+  //
+  OriginalAttribute = 0;
+  Offset = 0;
+
+  // Increment the Indent
+  gIndent += Indent;
+
+  CopyMem ((VOID *)&BitsData, (VOID *)Ptr, Length);
+  if (Trace && (AsciiName != NULL)){
+    HighLight = GetColourHighlighting ();
+
+    if (HighLight) {
+      OriginalAttribute = gST->ConOut->Mode->Attribute;
+      gST->ConOut->SetAttribute (
+                     gST->ConOut,
+                     EFI_TEXT_ATTR(EFI_YELLOW,
+                       ((OriginalAttribute&(BIT4|BIT5|BIT6))>>4))
+                     );
+    }
+    Print (
+      L"%*a%-*a :\n",
+      gIndent,
+      "",
+      (OUTPUT_FIELD_COLUMN_WIDTH - gIndent),
+      AsciiName
+      );
+    if (HighLight) {
+      gST->ConOut->SetAttribute (gST->ConOut, OriginalAttribute);
+    }
+  }
+
+  for (Index = 0; Index < ParserItems; Index++) {
+    if ((Offset + Parser[Index].Length) > (Length * 8)) {
+      // For fields outside the buffer length provided, reset any pointers
+      // which were supposed to be updated by this function call
+      if (Parser[Index].ItemPtr != NULL) {
+        *Parser[Index].ItemPtr = NULL;
+      }
+
+      // We don't parse past the end of the max length specified
+      continue;
+    }
+
+    if (Parser[Index].Length == 0) {
+      // don't parse the bitfield whose length is zero
+      continue;
+    }
+
+    if (GetConsistencyChecking () &&
+        (Offset != Parser[Index].Offset)) {
+      IncrementErrorCount ();
+      Print (
+        L"\nERROR: %a: Offset Mismatch for %s\n"
+          L"CurrentOffset = %d FieldOffset = %d\n",
+        AsciiName,
+        Parser[Index].NameStr,
+        Offset,
+        Parser[Index].Offset
+        );
+    }
+
+    // extract Bitfield data for the current item
+    Data = (BitsData >> Parser[Index].Offset) & ~(~0ULL << Parser[Index].Length);
+
+    if (Trace) {
+      // if there is a Formatter function let the function handle
+      // the printing else if a Format is specified in the table use
+      // the Format for printing
+      PrintFieldName (2, Parser[Index].NameStr);
+      if (Parser[Index].PrintFormatter != NULL) {
+        Parser[Index].PrintFormatter (Parser[Index].Format, (UINT8 *)&Data);
+      } else if (Parser[Index].Format != NULL) {
+        // convert bit length to byte length
+        switch ((Parser[Index].Length / 8) + (Parser[Index].Length % 8)) {
+          // print the data depends on byte size
+          case 1:
+            DumpUint8 (Parser[Index].Format, (UINT8 *)&Data);
+            break;
+          case 2:
+            DumpUint16 (Parser[Index].Format, (UINT8 *)&Data);
+            break;
+          case 3:
+          case 4:
+            DumpUint32 (Parser[Index].Format, (UINT8 *)&Data);
+            break;
+          case 5:
+          case 6:
+          case 7:
+          case 8:
+            DumpUint64 (Parser[Index].Format, (UINT8 *)&Data);
+            break;
+          default:
+            Print (
+              L"\nERROR: %a: CANNOT PARSE THIS FIELD, Field Length = %d\n",
+              AsciiName,
+              Parser[Index].Length
+              );
+        } // switch
+      }
+      // Validating only makes sense if we are tracing
+      // the parsed table entries, to report by table name.
+      if (GetConsistencyChecking () &&
+          (Parser[Index].FieldValidator != NULL)) {
+        Parser[Index].FieldValidator ((UINT8 *)&Data, Parser[Index].Context);
+      }
+      Print (L"\n");
+    } // if (Trace)
+
+    if (Parser[Index].ItemPtr != NULL) {
+      *Parser[Index].ItemPtr = (VOID*)(UINT8 *)&Data;
+    }
+
+    Offset += Parser[Index].Length;
+  } // for
+
+  // Decrement the Indent
+  gIndent -= Indent;
+  return Offset;
+}
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser.c
index d86718bab67d..3b59864d2c7e 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser.c
@@ -2,6 +2,7 @@
   FADT table parser
 
   Copyright (c) 2016 - 2020, ARM Limited. All rights reserved.
+  Copyright (c) 2021, AMD Incorporated. All rights reserved.
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
   @par Reference(s):
@@ -127,6 +128,62 @@ ValidateFlags (
 #endif
 }
 
+STATIC CONST ACPI_PARSER FadtFlagParser [] = {
+  {L"WBINVD", 1, 0, L"%d", NULL, NULL, NULL, NULL},
+  {L"WBINVD_FLUSH", 1, 1, L"%d", NULL, NULL, NULL, NULL},
+  {L"PROC_C1", 1, 2, L"%d", NULL, NULL, NULL, NULL},
+  {L"P_LVL2_UP", 1, 3, L"%d", NULL, NULL, NULL, NULL},
+  {L"PWR_BUTTON", 1, 4, L"%d", NULL, NULL, NULL, NULL},
+  {L"SLP_BUTTON", 1, 5, L"%d", NULL, NULL, NULL, NULL},
+  {L"FIX_RTC", 1, 6, L"%d", NULL, NULL, NULL, NULL},
+  {L"RTC_S4", 1, 7, L"%d", NULL, NULL, NULL, NULL},
+  {L"TMR_VAL_EXT", 1, 8, L"%d", NULL, NULL, NULL, NULL},
+  {L"DCK_CAP", 1, 9, L"%d", NULL, NULL, NULL, NULL},
+  {L"RESET_REG_SUP", 1, 10, L"%d", NULL, NULL, NULL, NULL},
+  {L"SEALED_CASE", 1, 11, L"%d", NULL, NULL, NULL, NULL},
+  {L"HEADLESS", 1, 12, L"%d", NULL, NULL, NULL, NULL},
+  {L"CPU_SW_SLP", 1, 13, L"%d", NULL, NULL, NULL, NULL},
+  {L"PCI_EXP_WAK", 1, 14, L"%d", NULL, NULL, NULL, NULL},
+  {L"USE_PLATFORM_CLOCK", 1, 15, L"%d", NULL, NULL, NULL, NULL},
+  {L"S4_RTC_STS_VALID", 1, 16, L"%d", NULL, NULL, NULL, NULL},
+  {L"REMOTE_POWER_ON_CAPABLE", 1, 17, L"%d", NULL, NULL, NULL, NULL},
+  {L"FORCE_APIC_CLUSTER_MODEL", 1, 18, L"%d", NULL, NULL, NULL, NULL},
+  {L"FORCE_APIC_PHYSICAL_DESTINATION_MODE", 1, 19, L"%d", NULL, NULL, NULL, NULL},
+  {L"HW_REDUCED_ACPI", 1, 20, L"%d", NULL, NULL, NULL, NULL},
+  {L"LOW_POWER_S0_IDLE_CAPABLE", 1, 21, L"%d", NULL, NULL, NULL, NULL},
+  {L"Reserved", 10, 22, L"%d", NULL, NULL, NULL, NULL}
+};
+
+/**
+  This function traces FADT Flags fields.
+  If no format string is specified the Format must be NULL.
+
+  @param [in] Format  Optional format string for tracing the data.
+  @param [in] Ptr     Pointer to the start of the buffer.
+**/
+VOID
+EFIAPI
+DumpFadtFlags (
+  IN CONST CHAR16* Format OPTIONAL,
+  IN UINT8*        Ptr
+  )
+{
+  if (Format != NULL) {
+    Print (Format, *(UINT32*)Ptr);
+    return;
+  }
+
+  Print (L"0x%X\n", *(UINT32*)Ptr);
+  ParseAcpiBitFields (
+    TRUE,
+    2,
+    NULL,
+    Ptr,
+    4,
+    PARSER_PARAMS (FadtFlagParser)
+    );
+}
+
 /**
   An ACPI_PARSER array describing the ACPI FADT Table.
 **/
@@ -170,7 +227,7 @@ STATIC CONST ACPI_PARSER FadtParser[] = {
   {L"CENTURY", 1, 108, L"0x%x", NULL, NULL, NULL, NULL},
   {L"IAPC_BOOT_ARCH", 2, 109, L"0x%x", NULL, NULL, NULL, NULL},
   {L"Reserved", 1, 111, L"0x%x", NULL, NULL, NULL, NULL},
-  {L"Flags", 4, 112, L"0x%x", NULL, (VOID**)&Flags, ValidateFlags, NULL},
+  {L"Flags", 4, 112, NULL, DumpFadtFlags, (VOID**)&Flags, ValidateFlags, NULL},
   {L"RESET_REG", 12, 116, NULL, DumpGas, NULL, NULL, NULL},
   {L"RESET_VALUE", 1, 128, L"0x%x", NULL, NULL, NULL, NULL},
   {L"ARM_BOOT_ARCH", 2, 129, L"0x%x", NULL, NULL, NULL, NULL},
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2 1/1] ShellPkg/AcpiView: PrintFormatter for FADT Flags field
  2021-11-17 12:29 ` [PATCH v2 1/1] ShellPkg/AcpiView: PrintFormatter for FADT Flags field Abdul Lateef Attar
@ 2021-11-18 10:44   ` Sami Mujawar
  0 siblings, 0 replies; 3+ messages in thread
From: Sami Mujawar @ 2021-11-18 10:44 UTC (permalink / raw)
  To: Abdul Lateef Attar, devel; +Cc: Ray Ni, Zhichao Gao, nd

Hi Abdul,

Thank you for implementing the suggestion.

Pleas find my feedback inline marked [SAMI].

Regards,

Sami Mujawar


On 17/11/2021 12:29 PM, Abdul Lateef Attar wrote:
> Adds PrintFormatter function to the FADT flags field.
> Prints indivisual flag name along with flag value.
> Adds new parser function ParseAcpiBitFields() to handle
> bit fields.
>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Zhichao Gao <zhichao.gao@intel.com>
> Cc: Sami Mujawar <sami.mujawar@arm.com>
> Signed-off-by: Abdul Lateef Attar <abdattar@amd.com>
> ---
>   ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h              |  38 +++++
>   ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c              | 177 ++++++++++++++++++++
>   ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser.c |  59 ++++++-
>   3 files changed, 273 insertions(+), 1 deletion(-)
>
> diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
> index 5e34a70c8bae..444a977aced8 100644
> --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
> @@ -2,6 +2,7 @@
>     Header file for ACPI parser
>
>   
>
>     Copyright (c) 2016 - 2020, Arm Limited. All rights reserved.
>
> +  Copyright (c) 2021, AMD Incorporated. All rights reserved.
>
>     SPDX-License-Identifier: BSD-2-Clause-Patent
>
>   **/
>
>   
>
> @@ -365,6 +366,43 @@ ParseAcpi (
>     IN UINT32             ParserItems
>
>     );
>
>   
>
> +/**
>
> +  This function is used to parse an ACPI table bitfield buffer.
>
> +
>
> +  The ACPI table buffer is parsed using the ACPI table parser information
>
> +  specified by a pointer to an array of ACPI_PARSER elements. This parser
>
> +  function iterates through each item on the ACPI_PARSER array and logs the ACPI table bitfields.
>
> +
>
> +  This function can optionally be used to parse ACPI tables and fetch specific
>
> +  field values. The ItemPtr member of the ACPI_PARSER structure (where used)
>
> +  is updated by this parser function to point to the selected field data
>
> +  (e.g. useful for variable length nested fields).
>
> +
>
> +  @param [in] Trace        Trace the ACPI fields TRUE else only parse the
>
> +                           table.
>
> +  @param [in] Indent       Number of spaces to indent the output.
>
> +  @param [in] AsciiName    Optional pointer to an ASCII string that describes
>
> +                           the table being parsed.
>
> +  @param [in] Ptr          Pointer to the start of the buffer.
>
> +  @param [in] Length       Length of the buffer pointed by Ptr.
>
> +  @param [in] Parser       Pointer to an array of ACPI_PARSER structure that
>
> +                           describes the table being parsed.
>
> +  @param [in] ParserItems  Number of items in the ACPI_PARSER array.
>
> +
>
> +  @retval Number of bits parsed.
>
> +**/
>
> +UINT32
>
> +EFIAPI
>
> +ParseAcpiBitFields (
>
> +  IN BOOLEAN            Trace,
>
> +  IN UINT32             Indent,
>
> +  IN CONST CHAR8*       AsciiName OPTIONAL,
>
> +  IN UINT8*             Ptr,
>
> +  IN UINT32             Length,
>
> +  IN CONST ACPI_PARSER* Parser,
>
> +  IN UINT32             ParserItems
>
> +);
>
> +
>
>   /**
>
>      This is a helper macro to pass parameters to the Parser functions.
>
>   
>
> diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
> index 74056e72c359..33a9e3d1e97a 100644
> --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
> @@ -2,12 +2,14 @@
>     ACPI parser
>
>   
>
>     Copyright (c) 2016 - 2021, Arm Limited. All rights reserved.
>
> +  Copyright (c) 2021, AMD Incorporated. All rights reserved.
>
>     SPDX-License-Identifier: BSD-2-Clause-Patent
>
>   **/
>
>   
>
>   #include <Uefi.h>
>
>   #include <Library/UefiLib.h>
>
>   #include <Library/UefiBootServicesTableLib.h>
>
> +#include <Library/BaseMemoryLib.h>
>
>   #include "AcpiParser.h"
>
>   #include "AcpiView.h"
>
>   #include "AcpiViewConfig.h"
>
> @@ -738,3 +740,178 @@ ParseAcpiHeader (
>   
>
>     return BytesParsed;
>
>   }
>
> +
>
> +/**
>
> +  This function is used to parse an ACPI table bitfield buffer.
>
> +
>
> +  The ACPI table buffer is parsed using the ACPI table parser information
>
> +  specified by a pointer to an array of ACPI_PARSER elements. This parser
>
> +  function iterates through each item on the ACPI_PARSER array and logs the ACPI table bitfields.
>
> +
>
> +  This function can optionally be used to parse ACPI tables and fetch specific
>
> +  field values. The ItemPtr member of the ACPI_PARSER structure (where used)
>
> +  is updated by this parser function to point to the selected field data
>
> +  (e.g. useful for variable length nested fields).
>
> +
>
> +  @param [in] Trace        Trace the ACPI fields TRUE else only parse the
>
> +                           table.
>
> +  @param [in] Indent       Number of spaces to indent the output.
>
> +  @param [in] AsciiName    Optional pointer to an ASCII string that describes
>
> +                           the table being parsed.
>
> +  @param [in] Ptr          Pointer to the start of the buffer.
>
> +  @param [in] Length       Length in bytes of the buffer pointed by Ptr.
>
> +  @param [in] Parser       Pointer to an array of ACPI_PARSER structure that
>
> +                           describes the table being parsed.
>
> +  @param [in] ParserItems  Number of items in the ACPI_PARSER array.
>
> +
>
> +  @retval Number of bits parsed.
>
> +**/
>
> +UINT32
>
> +EFIAPI
>
> +ParseAcpiBitFields (
>
> +  IN BOOLEAN            Trace,
>
> +  IN UINT32             Indent,
>
> +  IN CONST CHAR8*       AsciiName OPTIONAL,
>
> +  IN UINT8*             Ptr,
>
> +  IN UINT32             Length,
>
> +  IN CONST ACPI_PARSER* Parser,
>
> +  IN UINT32             ParserItems
>
> +)
>
> +{
>
> +  UINT32  Index;
>
> +  UINT32  Offset;
>
> +  BOOLEAN HighLight;
>
> +  UINTN   OriginalAttribute;
>
> +
>
> +  UINT64 Data;
>
> +  UINT64 BitsData;
>
> +
>
> +  if (Length <= 0 || Length > 8) {
[SAMI] Minor, since Lenght is UINT32 this check can be changed to 'if 
(Length == 0 || Length > 8) {'. Similarly the error log message below 
can be modified to say that the length is zero or exceeding the 64 bits 
limit.
>
> +    IncrementErrorCount ();
>
> +    Print (
>
> +        L"\nERROR: Bitfield Length(%d) is exceeding the 64bits limit \n",
>
> +        Length
>
> +        );
>
> +    return 0;
>
> +  }
>
> +  //
>
> +  // set local variables to suppress incorrect compiler/analyzer warnings
>
> +  //
>
> +  OriginalAttribute = 0;
>
> +  Offset = 0;
>
> +
>
> +  // Increment the Indent
>
> +  gIndent += Indent;
>
> +
>
> +  CopyMem ((VOID *)&BitsData, (VOID *)Ptr, Length);
>
> +  if (Trace && (AsciiName != NULL)){
>
> +    HighLight = GetColourHighlighting ();
>
> +
>
> +    if (HighLight) {
>
> +      OriginalAttribute = gST->ConOut->Mode->Attribute;
>
> +      gST->ConOut->SetAttribute (
>
> +                     gST->ConOut,
>
> +                     EFI_TEXT_ATTR(EFI_YELLOW,
>
> +                       ((OriginalAttribute&(BIT4|BIT5|BIT6))>>4))
>
> +                     );
>
> +    }
>
> +    Print (
>
> +      L"%*a%-*a :\n",
>
> +      gIndent,
>
> +      "",
>
> +      (OUTPUT_FIELD_COLUMN_WIDTH - gIndent),
>
> +      AsciiName
>
> +      );
>
> +    if (HighLight) {
>
> +      gST->ConOut->SetAttribute (gST->ConOut, OriginalAttribute);
>
> +    }
>
> +  }
>
> +
>
> +  for (Index = 0; Index < ParserItems; Index++) {
>
> +    if ((Offset + Parser[Index].Length) > (Length * 8)) {
>
> +      // For fields outside the buffer length provided, reset any pointers
>
> +      // which were supposed to be updated by this function call
>
> +      if (Parser[Index].ItemPtr != NULL) {
>
> +        *Parser[Index].ItemPtr = NULL;
>
> +      }
>
> +
>
> +      // We don't parse past the end of the max length specified
>
> +      continue;
>
> +    }
>
> +
>
> +    if (Parser[Index].Length == 0) {
>
> +      // don't parse the bitfield whose length is zero
>
> +      continue;
>
> +    }
>
> +
>
> +    if (GetConsistencyChecking () &&
>
> +        (Offset != Parser[Index].Offset)) {
>
> +      IncrementErrorCount ();
>
> +      Print (
>
> +        L"\nERROR: %a: Offset Mismatch for %s\n"
>
> +          L"CurrentOffset = %d FieldOffset = %d\n",
>
> +        AsciiName,
>
> +        Parser[Index].NameStr,
>
> +        Offset,
>
> +        Parser[Index].Offset
>
> +        );
>
> +    }
>
> +
>
> +    // extract Bitfield data for the current item
>
> +    Data = (BitsData >> Parser[Index].Offset) & ~(~0ULL << Parser[Index].Length);
[SAMI] Is it possible to update the documentation for ACPI_PARSER 
structure fields to relect that the Offset and Length fields have a 
modified meaning when used with ParseAcpiBitFields(), please?
>
> +
>
> +    if (Trace) {
>
> +      // if there is a Formatter function let the function handle
>
> +      // the printing else if a Format is specified in the table use
>
> +      // the Format for printing
>
> +      PrintFieldName (2, Parser[Index].NameStr);
>
> +      if (Parser[Index].PrintFormatter != NULL) {
>
> +        Parser[Index].PrintFormatter (Parser[Index].Format, (UINT8 *)&Data);
>
> +      } else if (Parser[Index].Format != NULL) {
>
> +        // convert bit length to byte length
>
> +        switch ((Parser[Index].Length / 8) + (Parser[Index].Length % 8)) {
[SAMI] Should this be 'switch ((Parser[Index].Length + 7) >> 3) {'?
>
> +          // print the data depends on byte size
>
> +          case 1:
>
> +            DumpUint8 (Parser[Index].Format, (UINT8 *)&Data);
>
> +            break;
>
> +          case 2:
>
> +            DumpUint16 (Parser[Index].Format, (UINT8 *)&Data);
>
> +            break;
>
> +          case 3:
>
> +          case 4:
>
> +            DumpUint32 (Parser[Index].Format, (UINT8 *)&Data);
>
> +            break;
>
> +          case 5:
>
> +          case 6:
>
> +          case 7:
>
> +          case 8:
>
> +            DumpUint64 (Parser[Index].Format, (UINT8 *)&Data);
>
> +            break;
>
> +          default:
>
> +            Print (
>
> +              L"\nERROR: %a: CANNOT PARSE THIS FIELD, Field Length = %d\n",
>
> +              AsciiName,
>
> +              Parser[Index].Length
>
> +              );
>
> +        } // switch
>
> +      }
>
> +      // Validating only makes sense if we are tracing
>
> +      // the parsed table entries, to report by table name.
>
> +      if (GetConsistencyChecking () &&
>
> +          (Parser[Index].FieldValidator != NULL)) {
>
> +        Parser[Index].FieldValidator ((UINT8 *)&Data, Parser[Index].Context);
>
> +      }
>
> +      Print (L"\n");
>
> +    } // if (Trace)
>
> +
>
> +    if (Parser[Index].ItemPtr != NULL) {
>
> +      *Parser[Index].ItemPtr = (VOID*)(UINT8 *)&Data;
>
> +    }
>
> +
>
> +    Offset += Parser[Index].Length;
>
> +  } // for
>
> +
>
> +  // Decrement the Indent
>
> +  gIndent -= Indent;
>
> +  return Offset;
>
> +}
>
> diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser.c
> index d86718bab67d..3b59864d2c7e 100644
> --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser.c
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser.c
> @@ -2,6 +2,7 @@
>     FADT table parser
[SAMI] I think the FADT change can be a separate patch.
>   
>
>     Copyright (c) 2016 - 2020, ARM Limited. All rights reserved.
>
> +  Copyright (c) 2021, AMD Incorporated. All rights reserved.
>
>     SPDX-License-Identifier: BSD-2-Clause-Patent
>
>   
>
>     @par Reference(s):
>
> @@ -127,6 +128,62 @@ ValidateFlags (
>   #endif
>
>   }
>
>   
>
> +STATIC CONST ACPI_PARSER FadtFlagParser [] = {
>
> +  {L"WBINVD", 1, 0, L"%d", NULL, NULL, NULL, NULL},
>
> +  {L"WBINVD_FLUSH", 1, 1, L"%d", NULL, NULL, NULL, NULL},
>
> +  {L"PROC_C1", 1, 2, L"%d", NULL, NULL, NULL, NULL},
>
> +  {L"P_LVL2_UP", 1, 3, L"%d", NULL, NULL, NULL, NULL},
>
> +  {L"PWR_BUTTON", 1, 4, L"%d", NULL, NULL, NULL, NULL},
>
> +  {L"SLP_BUTTON", 1, 5, L"%d", NULL, NULL, NULL, NULL},
>
> +  {L"FIX_RTC", 1, 6, L"%d", NULL, NULL, NULL, NULL},
>
> +  {L"RTC_S4", 1, 7, L"%d", NULL, NULL, NULL, NULL},
>
> +  {L"TMR_VAL_EXT", 1, 8, L"%d", NULL, NULL, NULL, NULL},
>
> +  {L"DCK_CAP", 1, 9, L"%d", NULL, NULL, NULL, NULL},
>
> +  {L"RESET_REG_SUP", 1, 10, L"%d", NULL, NULL, NULL, NULL},
>
> +  {L"SEALED_CASE", 1, 11, L"%d", NULL, NULL, NULL, NULL},
>
> +  {L"HEADLESS", 1, 12, L"%d", NULL, NULL, NULL, NULL},
>
> +  {L"CPU_SW_SLP", 1, 13, L"%d", NULL, NULL, NULL, NULL},
>
> +  {L"PCI_EXP_WAK", 1, 14, L"%d", NULL, NULL, NULL, NULL},
>
> +  {L"USE_PLATFORM_CLOCK", 1, 15, L"%d", NULL, NULL, NULL, NULL},
>
> +  {L"S4_RTC_STS_VALID", 1, 16, L"%d", NULL, NULL, NULL, NULL},
>
> +  {L"REMOTE_POWER_ON_CAPABLE", 1, 17, L"%d", NULL, NULL, NULL, NULL},
>
> +  {L"FORCE_APIC_CLUSTER_MODEL", 1, 18, L"%d", NULL, NULL, NULL, NULL},
>
> +  {L"FORCE_APIC_PHYSICAL_DESTINATION_MODE", 1, 19, L"%d", NULL, NULL, NULL, NULL},
>
> +  {L"HW_REDUCED_ACPI", 1, 20, L"%d", NULL, NULL, NULL, NULL},
>
> +  {L"LOW_POWER_S0_IDLE_CAPABLE", 1, 21, L"%d", NULL, NULL, NULL, NULL},
>
> +  {L"Reserved", 10, 22, L"%d", NULL, NULL, NULL, NULL}
>
> +};
>
> +
>
> +/**
>
> +  This function traces FADT Flags fields.
>
> +  If no format string is specified the Format must be NULL.
>
> +
>
> +  @param [in] Format  Optional format string for tracing the data.
>
> +  @param [in] Ptr     Pointer to the start of the buffer.
>
> +**/
>
> +VOID
>
> +EFIAPI
>
> +DumpFadtFlags (
>
> +  IN CONST CHAR16* Format OPTIONAL,
>
> +  IN UINT8*        Ptr
>
> +  )
>
> +{
>
> +  if (Format != NULL) {
>
> +    Print (Format, *(UINT32*)Ptr);
>
> +    return;
>
> +  }
>
> +
>
> +  Print (L"0x%X\n", *(UINT32*)Ptr);
>
> +  ParseAcpiBitFields (
>
> +    TRUE,
>
> +    2,
>
> +    NULL,
>
> +    Ptr,
>
> +    4,
>
> +    PARSER_PARAMS (FadtFlagParser)
>
> +    );
>
> +}
>
> +
>
>   /**
>
>     An ACPI_PARSER array describing the ACPI FADT Table.
>
>   **/
>
> @@ -170,7 +227,7 @@ STATIC CONST ACPI_PARSER FadtParser[] = {
>     {L"CENTURY", 1, 108, L"0x%x", NULL, NULL, NULL, NULL},
>
>     {L"IAPC_BOOT_ARCH", 2, 109, L"0x%x", NULL, NULL, NULL, NULL},
>
>     {L"Reserved", 1, 111, L"0x%x", NULL, NULL, NULL, NULL},
>
> -  {L"Flags", 4, 112, L"0x%x", NULL, (VOID**)&Flags, ValidateFlags, NULL},
>
> +  {L"Flags", 4, 112, NULL, DumpFadtFlags, (VOID**)&Flags, ValidateFlags, NULL},
>
>     {L"RESET_REG", 12, 116, NULL, DumpGas, NULL, NULL, NULL},
>
>     {L"RESET_VALUE", 1, 128, L"0x%x", NULL, NULL, NULL, NULL},
>
>     {L"ARM_BOOT_ARCH", 2, 129, L"0x%x", NULL, NULL, NULL, NULL},
>


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-11-18 10:44 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-11-17 12:29 [PATCH v2 0/1] Adds PrintFormatter function to the FADT flags field Abdul Lateef Attar
2021-11-17 12:29 ` [PATCH v2 1/1] ShellPkg/AcpiView: PrintFormatter for FADT Flags field Abdul Lateef Attar
2021-11-18 10:44   ` Sami Mujawar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox