From: "Gao, Zhichao" <zhichao.gao@intel.com>
To: Sami Mujawar <sami.mujawar@arm.com>,
"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: "Ni, Ray" <ray.ni@intel.com>,
"Matteo.Carlini@arm.com" <Matteo.Carlini@arm.com>,
"Laura.Moretta@arm.com" <Laura.Moretta@arm.com>,
"nd@arm.com" <nd@arm.com>
Subject: Re: [PATCH v2 1/1] ShellPkg: acpiview: Add support for parsing FACS
Date: Tue, 26 Nov 2019 01:40:45 +0000 [thread overview]
Message-ID: <3CE959C139B4C44DBEA1810E3AA6F9000B87AB27@SHSMSX101.ccr.corp.intel.com> (raw)
In-Reply-To: <20191125091947.14360-1-sami.mujawar@arm.com>
Reviewed-by: Zhichao Gao <zhichao.gao@intel.com>
> -----Original Message-----
> From: Sami Mujawar [mailto:sami.mujawar@arm.com]
> Sent: Monday, November 25, 2019 5:20 PM
> To: devel@edk2.groups.io
> Cc: Sami Mujawar <sami.mujawar@arm.com>; Ni, Ray <ray.ni@intel.com>; Gao,
> Zhichao <zhichao.gao@intel.com>; Matteo.Carlini@arm.com;
> Laura.Moretta@arm.com; nd@arm.com
> Subject: [PATCH v2 1/1] ShellPkg: acpiview: Add support for parsing FACS
>
> Add support for parsing the ACPI FACS table.
>
> The FADT parser has also been updated as it links the FACS table using the
> FIRMWARE_CTRL or X_FIRMWARE_CTRL fields.
>
> Since the FACS table does not follow the standard ACPI header, the FADT parser
> extracts the FACS signature, length and version fields before invoking the FACS
> parser.
>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Zhichao Gao <zhichao.gao@intel.com>
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> ---
>
> The changes can be seen at:
> https://github.com/samimujawar/edk2/tree/656_acpiview_facs_parser_v1
>
> Notes:
> v2:
> - Fixed patch check failure in commit message, [SAMI]
> changed 'CC' to 'Cc'. No code change.
> v1:
> - Add support for parsing FACS table [SAMI]
>
> ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h | 21
> +++++
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Facs/FacsParser.c
> | 71 +++++++++++++++
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser.c
> | 90 ++++++++++++++++++--
>
> ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib
> .c | 1 +
>
> ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib
> .inf | 1 +
> 5 files changed, 179 insertions(+), 5 deletions(-)
>
> diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
> index
> 20ca358bddfa5953bfb1d1bebaebbf3079eaba01..f374f8ebfe313954c05b2a4328
> 16cf7ad3af9e32 100644
> --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
> @@ -528,6 +528,27 @@ ParseAcpiDsdt (
> IN UINT8 AcpiTableRevision
> );
>
> +/**
> + This function parses the ACPI FACS table.
> + When trace is enabled this function parses the FACS table and
> + traces the ACPI table fields.
> +
> + This function also performs validation of the ACPI table fields.
> +
> + @param [in] Trace If TRUE, trace the ACPI fields.
> + @param [in] Ptr Pointer to the start of the buffer.
> + @param [in] AcpiTableLength Length of the ACPI table.
> + @param [in] AcpiTableRevision Revision of the ACPI table.
> +**/
> +VOID
> +EFIAPI
> +ParseAcpiFacs (
> + IN BOOLEAN Trace,
> + IN UINT8* Ptr,
> + IN UINT32 AcpiTableLength,
> + IN UINT8 AcpiTableRevision
> + );
> +
> /**
> This function parses the ACPI FADT table.
> This function parses the FADT table and optionally traces the ACPI diff --git
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Facs/FacsParser.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Facs/FacsParser.c
> new file mode 100644
> index
> 0000000000000000000000000000000000000000..d6bea86bdbaa79aa35b86840
> c809394b3c7a3bf6
> --- /dev/null
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Facs/FacsPars
> +++ er.c
> @@ -0,0 +1,71 @@
> +/** @file
> + FACS table parser
> +
> + Copyright (c) 2019, ARM Limited. All rights reserved.
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> + @par Reference(s):
> + - ACPI 6.3 Specification - January 2019 **/
> +
> +#include <IndustryStandard/Acpi.h>
> +#include <Library/UefiLib.h>
> +#include "AcpiParser.h"
> +#include "AcpiTableParser.h"
> +
> +/**
> + An ACPI_PARSER array describing the ACPI FACS Table.
> +**/
> +STATIC CONST ACPI_PARSER FacsParser[] = {
> + {L"Signature", 4, 0, L"%c%c%c%c", Dump4Chars, NULL, NULL, NULL},
> + {L"Length", 4, 4, L"%d", NULL, NULL, NULL, NULL},
> + {L"Hardware Signature", 4, 8, L"0x%x", NULL, NULL, NULL, NULL},
> + {L"Firmware Waking Vector", 4, 12, L"0x%x", NULL, NULL, NULL, NULL},
> + {L"Global Lock", 4, 16, L"0x%x", NULL, NULL, NULL, NULL},
> + {L"Flags", 4, 20, L"0x%x", NULL, NULL, NULL, NULL},
> + {L"X Firmware Walking Vector", 8, 24, L"0x%lx", NULL, NULL, NULL,
> +NULL},
> + {L"Version", 1, 32, L"%d", NULL, NULL, NULL, NULL},
> + {L"Reserved", 3, 33, L"%x %x %x", Dump3Chars, NULL, NULL, NULL},
> + {L"OSPM Flags", 4, 36, L"0x%x", NULL, NULL, NULL, NULL},
> + {L"Reserved", 8, 40, L"%x %x %x %x %x %x %x %x", Dump8Chars, NULL, NULL,
> + NULL},
> + {L"Reserved", 8, 48, L"%x %x %x %x %x %x %x %x", Dump8Chars, NULL, NULL,
> + NULL},
> + {L"Reserved", 8, 56, L"%x %x %x %x %x %x %x %x", Dump8Chars, NULL, NULL,
> + NULL}
> +};
> +
> +/**
> + This function parses the ACPI FACS table.
> + When trace is enabled this function parses the FACS table and
> + traces the ACPI table fields.
> +
> + This function also performs validation of the ACPI table fields.
> +
> + @param [in] Trace If TRUE, trace the ACPI fields.
> + @param [in] Ptr Pointer to the start of the buffer.
> + @param [in] AcpiTableLength Length of the ACPI table.
> + @param [in] AcpiTableRevision Revision of the ACPI table.
> +**/
> +VOID
> +EFIAPI
> +ParseAcpiFacs (
> + IN BOOLEAN Trace,
> + IN UINT8* Ptr,
> + IN UINT32 AcpiTableLength,
> + IN UINT8 AcpiTableRevision
> + )
> +{
> + if (!Trace) {
> + return;
> + }
> +
> + ParseAcpi (
> + Trace,
> + 0,
> + "FACS",
> + Ptr,
> + AcpiTableLength,
> + PARSER_PARAMS (FacsParser)
> + );
> +}
> diff --git
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser.c
> index
> e40c9ef8ee4b3285faf8c6edf3cb6236ee367397..5b8cc174f16afb3d4feb6a51895
> 2e60c6564ee34 100644
> --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser.c
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtPars
> +++ er.c
> @@ -5,17 +5,21 @@
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> @par Reference(s):
> - - ACPI 6.2 Specification - Errata A, September 2017
> + - ACPI 6.3 Specification - January 2019
> **/
>
> #include <IndustryStandard/Acpi.h>
> #include <Library/UefiLib.h>
> #include "AcpiParser.h"
> #include "AcpiTableParser.h"
> +#include "AcpiView.h"
>
> // Local variables
> STATIC CONST UINT32* DsdtAddress;
> STATIC CONST UINT64* X_DsdtAddress;
> +STATIC CONST UINT32* Flags;
> +STATIC CONST UINT32* FirmwareCtrl;
> +STATIC CONST UINT64* X_FirmwareCtrl;
> STATIC CONST UINT8* FadtMinorRevision; STATIC
> ACPI_DESCRIPTION_HEADER_INFO AcpiHdrInfo;
>
> @@ -24,6 +28,21 @@ STATIC ACPI_DESCRIPTION_HEADER_INFO AcpiHdrInfo;
> **/
> #define HW_REDUCED_ACPI BIT20
>
> +/**
> + Offset to the FACS signature from the start of the FACS.
> +**/
> +#define FACS_SIGNATURE_OFFSET 0
> +
> +/**
> + Offset to the FACS revision from the start of the FACS.
> +**/
> +#define FACS_VERSION_OFFSET 32
> +
> +/**
> + Offset to the FACS length from the start of the FACS.
> +**/
> +#define FACS_LENGTH_OFFSET 4
> +
> /**
> Get the ACPI XSDT header info.
> **/
> @@ -113,7 +132,8 @@ ValidateFlags (
> **/
> STATIC CONST ACPI_PARSER FadtParser[] = {
> PARSE_ACPI_HEADER (&AcpiHdrInfo),
> - {L"FIRMWARE_CTRL", 4, 36, L"0x%x", NULL, NULL, ValidateFirmwareCtrl,
> NULL},
> + {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}, @@ -
> 150,13 +170,13 @@ 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, NULL, ValidateFlags, 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,
> NULL, NULL},
> - {L"X_FIRMWARE_CTRL", 8, 132, L"0x%lx", NULL, NULL,
> + {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}, @@ -
> 192,7 +212,13 @@ ParseAcpiFadt (
> IN UINT8 AcpiTableRevision
> )
> {
> - UINT8* DsdtPtr;
> + EFI_STATUS Status;
> + UINT8* DsdtPtr;
> + UINT8* FirmwareCtrlPtr;
> + UINT32 FacsSignature;
> + UINT32 FacsLength;
> + UINT8 FacsRevision;
> + PARSE_ACPI_TABLE_PROC FacsParserProc;
>
> ParseAcpi (
> Trace,
> @@ -214,6 +240,60 @@ ParseAcpiFadt (
> }
> }
>
> + // If X_FIRMWARE_CTRL is not zero then use X_FIRMWARE_CTRL and ignore
> + // FIRMWARE_CTRL, else use FIRMWARE_CTRL.
> + if ((X_FirmwareCtrl != NULL) && (*X_FirmwareCtrl != 0)) {
> + FirmwareCtrlPtr = (UINT8*)(UINTN)(*X_FirmwareCtrl); } else if
> + ((FirmwareCtrl != NULL) && (*FirmwareCtrl != 0)) {
> + FirmwareCtrlPtr = (UINT8*)(UINTN)(*FirmwareCtrl); } else {
> + FirmwareCtrlPtr = NULL;
> + // if HW_REDUCED_ACPI flag is not set, both FIRMWARE_CTRL and
> + // X_FIRMWARE_CTRL cannot be zero, and the FACS Table must be
> + // present.
> + if ((Trace) &&
> + (Flags != NULL) &&
> + ((*Flags & EFI_ACPI_6_3_HW_REDUCED_ACPI) != 0)) {
> + IncrementErrorCount ();
> + Print (L"ERROR: No FACS table found, "
> + L"both X_FIRMWARE_CTRL and FIRMWARE_CTRL are zero.\n");
> + }
> + }
> +
> + if (FirmwareCtrlPtr != NULL) {
> + // The FACS table does not have a standard ACPI table header. Therefore,
> + // the signature, length and version needs to be initially parsed.
> + // The FACS signature is 4 bytes starting at offset 0.
> + FacsSignature = *(UINT32*)(FirmwareCtrlPtr +
> + FACS_SIGNATURE_OFFSET);
> +
> + // The FACS length is 4 bytes starting at offset 4.
> + FacsLength = *(UINT32*)(FirmwareCtrlPtr + FACS_LENGTH_OFFSET);
> +
> + // The FACS version is 1 byte starting at offset 32.
> + FacsRevision = *(UINT8*)(FirmwareCtrlPtr + FACS_VERSION_OFFSET);
> +
> + Trace = ProcessTableReportOptions (
> + FacsSignature,
> + FirmwareCtrlPtr,
> + FacsLength
> + );
> +
> + Status = GetParser (FacsSignature, &FacsParserProc);
> + if (EFI_ERROR (Status)) {
> + Print (
> + L"ERROR: No registered parser found for FACS.\n"
> + );
> + return;
> + }
> +
> + FacsParserProc (
> + Trace,
> + FirmwareCtrlPtr,
> + FacsLength,
> + FacsRevision
> + );
> + }
> +
> // If X_DSDT is not zero then use X_DSDT and ignore DSDT,
> // else use DSDT.
> if (*X_DsdtAddress != 0) {
> diff --git
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommand
> Lib.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommand
> Lib.c
> index
> 3ee0f0fe421c6acdde33becca80d8efcbeda0fde..e0d5a81085525a91110d9a2527
> 9adf3376286342 100644
> ---
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommand
> Lib.c
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComm
> +++ andLib.c
> @@ -29,6 +29,7 @@ ACPI_TABLE_PARSER ParserList[] = {
> {EFI_ACPI_6_2_DEBUG_PORT_2_TABLE_SIGNATURE, ParseAcpiDbg2},
> {EFI_ACPI_6_2_DIFFERENTIATED_SYSTEM_DESCRIPTION_TABLE_SIGNATURE,
> ParseAcpiDsdt},
> + {EFI_ACPI_6_3_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE,
> + ParseAcpiFacs},
> {EFI_ACPI_6_2_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
> ParseAcpiFadt},
> {EFI_ACPI_6_2_GENERIC_TIMER_DESCRIPTION_TABLE_SIGNATURE,
> ParseAcpiGtdt},
> {EFI_ACPI_6_2_IO_REMAPPING_TABLE_SIGNATURE, ParseAcpiIort}, diff --git
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommand
> Lib.inf
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommand
> Lib.inf
> index
> 1e2fa52b00a37f73d789ba4049c531f5bc29fb5b..ea504c934aeebaa452a380e6e
> a169586e467642a 100644
> ---
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommand
> Lib.inf
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComm
> +++ andLib.inf
> @@ -31,6 +31,7 @@ [Sources.common]
> Parsers/Bgrt/BgrtParser.c
> Parsers/Dbg2/Dbg2Parser.c
> Parsers/Dsdt/DsdtParser.c
> + Parsers/Facs/FacsParser.c
> Parsers/Fadt/FadtParser.c
> Parsers/Gtdt/GtdtParser.c
> Parsers/Iort/IortParser.c
> --
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
>
prev parent reply other threads:[~2019-11-26 1:40 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-25 9:19 [PATCH v2 1/1] ShellPkg: acpiview: Add support for parsing FACS Sami Mujawar
2019-11-26 1:40 ` Gao, Zhichao [this message]
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=3CE959C139B4C44DBEA1810E3AA6F9000B87AB27@SHSMSX101.ccr.corp.intel.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