public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Sami Mujawar" <sami.mujawar@arm.com>
To: Abdul Lateef Attar <abdattar@amd.com>, devel@edk2.groups.io
Cc: Ray Ni <ray.ni@intel.com>, Zhichao Gao <zhichao.gao@intel.com>,
	nd <nd@arm.com>
Subject: Re: [PATCH v4 2/2] ShellPkg/AcpiView: PrintFormatter for FADT Flags field
Date: Wed, 19 Jan 2022 16:44:36 +0000	[thread overview]
Message-ID: <8aedd459-d594-3a5a-a6f6-5fd125639639@arm.com> (raw)
In-Reply-To: <20211219144437.3721-3-abdattar@amd.com>

Hi Abdul,

Thank you for this patch.

These changes look good to me.

Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>

Regards,

Sami Mujawar


On 19/12/2021 02:44 PM, Abdul Lateef Attar wrote:
> Adds PrintFormatter function to the FADT flags field.
> Prints indivisual flag name along with flag value.
>
> 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/Parsers/Fadt/FadtParser.c | 167 +++++++++++++-------
>   1 file changed, 112 insertions(+), 55 deletions(-)
>
> diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser.c
> index f8fbb4bcb8e9..5ff02f3d38a1 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):
>
> @@ -130,69 +131,125 @@ 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.
>
>   **/
>
>   STATIC CONST ACPI_PARSER  FadtParser[] = {
>
>     PARSE_ACPI_HEADER (&AcpiHdrInfo),
>
> -  { L"FIRMWARE_CTRL",              4,   36,  L"0x%x",  NULL,    (VOID **)&FirmwareCtrl,
>
> +  { L"FIRMWARE_CTRL",              4,   36,  L"0x%x",  NULL,          (VOID **)&FirmwareCtrl,
>
>       ValidateFirmwareCtrl,          NULL },
>
> -  { L"DSDT",                       4,   40,  L"0x%x",  NULL,    (VOID **)&DsdtAddress,      NULL,           NULL },
>
> -  { L"Reserved",                   1,   44,  L"%x",    NULL,    NULL,                       NULL,           NULL },
>
> -  { L"Preferred_PM_Profile",       1,   45,  L"0x%x",  NULL,    NULL,                       NULL,           NULL },
>
> -  { L"SCI_INT",                    2,   46,  L"0x%x",  NULL,    NULL,                       NULL,           NULL },
>
> -  { L"SMI_CMD",                    4,   48,  L"0x%x",  NULL,    NULL,                       NULL,           NULL },
>
> -  { L"ACPI_ENABLE",                1,   52,  L"0x%x",  NULL,    NULL,                       NULL,           NULL },
>
> -  { L"ACPI_DISABLE",               1,   53,  L"0x%x",  NULL,    NULL,                       NULL,           NULL },
>
> -  { L"S4BIOS_REQ",                 1,   54,  L"0x%x",  NULL,    NULL,                       NULL,           NULL },
>
> -  { L"PSTATE_CNT",                 1,   55,  L"0x%x",  NULL,    NULL,                       NULL,           NULL },
>
> -  { L"PM1a_EVT_BLK",               4,   56,  L"0x%x",  NULL,    NULL,                       NULL,           NULL },
>
> -  { L"PM1b_EVT_BLK",               4,   60,  L"0x%x",  NULL,    NULL,                       NULL,           NULL },
>
> -  { L"PM1a_CNT_BLK",               4,   64,  L"0x%x",  NULL,    NULL,                       NULL,           NULL },
>
> -  { L"PM1b_CNT_BLK",               4,   68,  L"0x%x",  NULL,    NULL,                       NULL,           NULL },
>
> -  { L"PM2_CNT_BLK",                4,   72,  L"0x%x",  NULL,    NULL,                       NULL,           NULL },
>
> -  { L"PM_TMR_BLK",                 4,   76,  L"0x%x",  NULL,    NULL,                       NULL,           NULL },
>
> -  { L"GPE0_BLK",                   4,   80,  L"0x%x",  NULL,    NULL,                       NULL,           NULL },
>
> -  { L"GPE1_BLK",                   4,   84,  L"0x%x",  NULL,    NULL,                       NULL,           NULL },
>
> -  { L"PM1_EVT_LEN",                1,   88,  L"0x%x",  NULL,    NULL,                       NULL,           NULL },
>
> -  { L"PM1_CNT_LEN",                1,   89,  L"0x%x",  NULL,    NULL,                       NULL,           NULL },
>
> -  { L"PM2_CNT_LEN",                1,   90,  L"0x%x",  NULL,    NULL,                       NULL,           NULL },
>
> -  { L"PM_TMR_LEN",                 1,   91,  L"0x%x",  NULL,    NULL,                       NULL,           NULL },
>
> -  { L"GPE0_BLK_LEN",               1,   92,  L"0x%x",  NULL,    NULL,                       NULL,           NULL },
>
> -  { L"GPE1_BLK_LEN",               1,   93,  L"0x%x",  NULL,    NULL,                       NULL,           NULL },
>
> -  { L"GPE1_BASE",                  1,   94,  L"0x%x",  NULL,    NULL,                       NULL,           NULL },
>
> -  { L"CST_CNT",                    1,   95,  L"0x%x",  NULL,    NULL,                       NULL,           NULL },
>
> -  { L"P_LVL2_LAT",                 2,   96,  L"0x%x",  NULL,    NULL,                       NULL,           NULL },
>
> -  { L"P_LVL3_LAT",                 2,   98,  L"0x%x",  NULL,    NULL,                       NULL,           NULL },
>
> -  { L"FLUSH_SIZE",                 2,   100, L"0x%x",  NULL,    NULL,                       NULL,           NULL },
>
> -  { L"FLUSH_STRIDE",               2,   102, L"0x%x",  NULL,    NULL,                       NULL,           NULL },
>
> -  { L"DUTY_OFFSET",                1,   104, L"0x%x",  NULL,    NULL,                       NULL,           NULL },
>
> -  { L"DUTY_WIDTH",                 1,   105, L"0x%x",  NULL,    NULL,                       NULL,           NULL },
>
> -  { L"DAY_ALRM",                   1,   106, L"0x%x",  NULL,    NULL,                       NULL,           NULL },
>
> -  { L"MON_ALRM",                   1,   107, L"0x%x",  NULL,    NULL,                       NULL,           NULL },
>
> -  { 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"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 },
>
> -  { L"FADT Minor Version",         1,   131, L"0x%x",  NULL,    (VOID **)&FadtMinorRevision,
>
> +  { L"DSDT",                       4,   40,  L"0x%x",  NULL,          (VOID **)&DsdtAddress,      NULL,           NULL },
>
> +  { L"Reserved",                   1,   44,  L"%x",    NULL,          NULL,                       NULL,           NULL },
>
> +  { L"Preferred_PM_Profile",       1,   45,  L"0x%x",  NULL,          NULL,                       NULL,           NULL },
>
> +  { L"SCI_INT",                    2,   46,  L"0x%x",  NULL,          NULL,                       NULL,           NULL },
>
> +  { L"SMI_CMD",                    4,   48,  L"0x%x",  NULL,          NULL,                       NULL,           NULL },
>
> +  { L"ACPI_ENABLE",                1,   52,  L"0x%x",  NULL,          NULL,                       NULL,           NULL },
>
> +  { L"ACPI_DISABLE",               1,   53,  L"0x%x",  NULL,          NULL,                       NULL,           NULL },
>
> +  { L"S4BIOS_REQ",                 1,   54,  L"0x%x",  NULL,          NULL,                       NULL,           NULL },
>
> +  { L"PSTATE_CNT",                 1,   55,  L"0x%x",  NULL,          NULL,                       NULL,           NULL },
>
> +  { L"PM1a_EVT_BLK",               4,   56,  L"0x%x",  NULL,          NULL,                       NULL,           NULL },
>
> +  { L"PM1b_EVT_BLK",               4,   60,  L"0x%x",  NULL,          NULL,                       NULL,           NULL },
>
> +  { L"PM1a_CNT_BLK",               4,   64,  L"0x%x",  NULL,          NULL,                       NULL,           NULL },
>
> +  { L"PM1b_CNT_BLK",               4,   68,  L"0x%x",  NULL,          NULL,                       NULL,           NULL },
>
> +  { L"PM2_CNT_BLK",                4,   72,  L"0x%x",  NULL,          NULL,                       NULL,           NULL },
>
> +  { L"PM_TMR_BLK",                 4,   76,  L"0x%x",  NULL,          NULL,                       NULL,           NULL },
>
> +  { L"GPE0_BLK",                   4,   80,  L"0x%x",  NULL,          NULL,                       NULL,           NULL },
>
> +  { L"GPE1_BLK",                   4,   84,  L"0x%x",  NULL,          NULL,                       NULL,           NULL },
>
> +  { L"PM1_EVT_LEN",                1,   88,  L"0x%x",  NULL,          NULL,                       NULL,           NULL },
>
> +  { L"PM1_CNT_LEN",                1,   89,  L"0x%x",  NULL,          NULL,                       NULL,           NULL },
>
> +  { L"PM2_CNT_LEN",                1,   90,  L"0x%x",  NULL,          NULL,                       NULL,           NULL },
>
> +  { L"PM_TMR_LEN",                 1,   91,  L"0x%x",  NULL,          NULL,                       NULL,           NULL },
>
> +  { L"GPE0_BLK_LEN",               1,   92,  L"0x%x",  NULL,          NULL,                       NULL,           NULL },
>
> +  { L"GPE1_BLK_LEN",               1,   93,  L"0x%x",  NULL,          NULL,                       NULL,           NULL },
>
> +  { L"GPE1_BASE",                  1,   94,  L"0x%x",  NULL,          NULL,                       NULL,           NULL },
>
> +  { L"CST_CNT",                    1,   95,  L"0x%x",  NULL,          NULL,                       NULL,           NULL },
>
> +  { L"P_LVL2_LAT",                 2,   96,  L"0x%x",  NULL,          NULL,                       NULL,           NULL },
>
> +  { L"P_LVL3_LAT",                 2,   98,  L"0x%x",  NULL,          NULL,                       NULL,           NULL },
>
> +  { L"FLUSH_SIZE",                 2,   100, L"0x%x",  NULL,          NULL,                       NULL,           NULL },
>
> +  { L"FLUSH_STRIDE",               2,   102, L"0x%x",  NULL,          NULL,                       NULL,           NULL },
>
> +  { L"DUTY_OFFSET",                1,   104, L"0x%x",  NULL,          NULL,                       NULL,           NULL },
>
> +  { L"DUTY_WIDTH",                 1,   105, L"0x%x",  NULL,          NULL,                       NULL,           NULL },
>
> +  { L"DAY_ALRM",                   1,   106, L"0x%x",  NULL,          NULL,                       NULL,           NULL },
>
> +  { L"MON_ALRM",                   1,   107, L"0x%x",  NULL,          NULL,                       NULL,           NULL },
>
> +  { 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, 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 },
>
> +  { L"FADT Minor Version",         1,   131, L"0x%x",  NULL,          (VOID **)&FadtMinorRevision,
>
>       NULL,                          NULL },
>
> -  { L"X_FIRMWARE_CTRL",            8,   132, L"0x%lx", NULL,    (VOID **)&X_FirmwareCtrl,
>
> +  { L"X_FIRMWARE_CTRL",            8,   132, L"0x%lx", NULL,          (VOID **)&X_FirmwareCtrl,
>
>       ValidateXFirmwareCtrl,         NULL },
>
> -  { L"X_DSDT",                     8,   140, L"0x%lx", NULL,    (VOID **)&X_DsdtAddress,    NULL,           NULL },
>
> -  { L"X_PM1a_EVT_BLK",             12,  148, NULL,     DumpGas, NULL,                       NULL,           NULL },
>
> -  { L"X_PM1b_EVT_BLK",             12,  160, NULL,     DumpGas, NULL,                       NULL,           NULL },
>
> -  { L"X_PM1a_CNT_BLK",             12,  172, NULL,     DumpGas, NULL,                       NULL,           NULL },
>
> -  { L"X_PM1b_CNT_BLK",             12,  184, NULL,     DumpGas, NULL,                       NULL,           NULL },
>
> -  { L"X_PM2_CNT_BLK",              12,  196, NULL,     DumpGas, NULL,                       NULL,           NULL },
>
> -  { L"X_PM_TMR_BLK",               12,  208, NULL,     DumpGas, NULL,                       NULL,           NULL },
>
> -  { L"X_GPE0_BLK",                 12,  220, NULL,     DumpGas, NULL,                       NULL,           NULL },
>
> -  { L"X_GPE1_BLK",                 12,  232, NULL,     DumpGas, NULL,                       NULL,           NULL },
>
> -  { L"SLEEP_CONTROL_REG",          12,  244, NULL,     DumpGas, NULL,                       NULL,           NULL },
>
> -  { L"SLEEP_STATUS_REG",           12,  256, NULL,     DumpGas, NULL,                       NULL,           NULL },
>
> -  { L"Hypervisor VendorIdentity",  8,   268, L"%lx",   NULL,    NULL,                       NULL,           NULL }
>
> +  { L"X_DSDT",                     8,   140, L"0x%lx", NULL,          (VOID **)&X_DsdtAddress,    NULL,           NULL },
>
> +  { L"X_PM1a_EVT_BLK",             12,  148, NULL,     DumpGas,       NULL,                       NULL,           NULL },
>
> +  { L"X_PM1b_EVT_BLK",             12,  160, NULL,     DumpGas,       NULL,                       NULL,           NULL },
>
> +  { L"X_PM1a_CNT_BLK",             12,  172, NULL,     DumpGas,       NULL,                       NULL,           NULL },
>
> +  { L"X_PM1b_CNT_BLK",             12,  184, NULL,     DumpGas,       NULL,                       NULL,           NULL },
>
> +  { L"X_PM2_CNT_BLK",              12,  196, NULL,     DumpGas,       NULL,                       NULL,           NULL },
>
> +  { L"X_PM_TMR_BLK",               12,  208, NULL,     DumpGas,       NULL,                       NULL,           NULL },
>
> +  { L"X_GPE0_BLK",                 12,  220, NULL,     DumpGas,       NULL,                       NULL,           NULL },
>
> +  { L"X_GPE1_BLK",                 12,  232, NULL,     DumpGas,       NULL,                       NULL,           NULL },
>
> +  { L"SLEEP_CONTROL_REG",          12,  244, NULL,     DumpGas,       NULL,                       NULL,           NULL },
>
> +  { L"SLEEP_STATUS_REG",           12,  256, NULL,     DumpGas,       NULL,                       NULL,           NULL },
>
> +  { L"Hypervisor VendorIdentity",  8,   268, L"%lx",   NULL,          NULL,                       NULL,           NULL }
>
>   };
>
>   
>
>   /**
>


  reply	other threads:[~2022-01-19 16:44 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20211219144437.3721-1-abdattar@amd.com>
2021-12-19 14:44 ` [PATCH v4 1/2] ShellPkg/AcpiView: Adds ACPI_PARSER bitfield parser Abdul Lateef Attar
2022-01-19 16:44   ` Sami Mujawar
2022-01-19 16:46     ` Sami Mujawar
2022-01-20  3:24       ` [edk2-devel] " Attar, AbdulLateef (Abdul Lateef)
2022-01-20  8:36         ` Gao, Zhichao
2022-01-20 12:31           ` Sami Mujawar
2022-01-21  7:29             ` Gao, Zhichao
2021-12-19 14:44 ` [PATCH v4 2/2] ShellPkg/AcpiView: PrintFormatter for FADT Flags field Abdul Lateef Attar
2022-01-19 16:44   ` Sami Mujawar [this message]
2022-01-20  8:39     ` [edk2-devel] " Gao, Zhichao
     [not found] ` <16C22F2D003754C4.24651@groups.io>
2022-01-07  2:58   ` [edk2-devel] [PATCH v4 1/2] ShellPkg/AcpiView: Adds ACPI_PARSER bitfield parser Attar, AbdulLateef (Abdul Lateef)
2022-01-19  3:39     ` Attar, AbdulLateef (Abdul Lateef)

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=8aedd459-d594-3a5a-a6f6-5fd125639639@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