From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from EUR02-VE1-obe.outbound.protection.outlook.com (EUR02-VE1-obe.outbound.protection.outlook.com [40.107.2.68]) by mx.groups.io with SMTP id smtpd.web08.233.1642610814112976732 for ; Wed, 19 Jan 2022 08:46:55 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@armh.onmicrosoft.com header.s=selector2-armh-onmicrosoft-com header.b=wTcARbcH; spf=pass (domain: arm.com, ip: 40.107.2.68, mailfrom: sami.mujawar@arm.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector2-armh-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=H4nJzZIpyKRodclHxXuPmT9JTwu5KWrwNqPIqjtAtg4=; b=wTcARbcHieDxY3YyOd2kQSYfY3kgZIxdMSULXhdKFh+yL+3GpVJeqpig+z5OoXuIpAPs5qiCBcC4tXWYgXGBjJLlu5BRd4IzjOkqHgF81fKYQUrCu8Fj1uktDMe3gfyjn8t4xtbKP5p7j/Ly1GSeue9WWqF1m3zGo7FDuIa1q/I= Received: from AS8PR05CA0005.eurprd05.prod.outlook.com (2603:10a6:20b:311::10) by DB9PR08MB6875.eurprd08.prod.outlook.com (2603:10a6:10:2aa::18) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4888.10; Wed, 19 Jan 2022 16:46:50 +0000 Received: from VE1EUR03FT055.eop-EUR03.prod.protection.outlook.com (2603:10a6:20b:311:cafe::cc) by AS8PR05CA0005.outlook.office365.com (2603:10a6:20b:311::10) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4909.8 via Frontend Transport; Wed, 19 Jan 2022 16:46:50 +0000 X-MS-Exchange-Authentication-Results: spf=pass (sender IP is 63.35.35.123) smtp.mailfrom=arm.com; dkim=pass (signature was verified) header.d=armh.onmicrosoft.com;dmarc=pass action=none header.from=arm.com; Received-SPF: Pass (protection.outlook.com: domain of arm.com designates 63.35.35.123 as permitted sender) receiver=protection.outlook.com; client-ip=63.35.35.123; helo=64aa7808-outbound-1.mta.getcheckrecipient.com; Received: from 64aa7808-outbound-1.mta.getcheckrecipient.com (63.35.35.123) by VE1EUR03FT055.mail.protection.outlook.com (10.152.19.158) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4888.9 via Frontend Transport; Wed, 19 Jan 2022 16:46:50 +0000 Received: ("Tessian outbound 741ca6c82739:v113"); Wed, 19 Jan 2022 16:46:49 +0000 X-CheckRecipientChecked: true X-CR-MTA-CID: ba9cffac22f213d5 X-CR-MTA-TID: 64aa7808 Received: from 7aa11e342d0d.1 by 64aa7808-outbound-1.mta.getcheckrecipient.com id 91DE9838-61CA-4F52-8CA9-4746BC639D7A.1; Wed, 19 Jan 2022 16:46:41 +0000 Received: from EUR03-VE1-obe.outbound.protection.outlook.com by 64aa7808-outbound-1.mta.getcheckrecipient.com with ESMTPS id 7aa11e342d0d.1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384); Wed, 19 Jan 2022 16:46:41 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=awxH4XxWs2n7prYBPfLJ1QW26uFXRR4apTxxeqSjPnn202AUFFuemtMr9ZuQYizvrc4sybRywSMQADgl9OC2oplJKYQpBkWrDmQaqXKMl1FFjJrSv4ixgeabeEPoveHlEazeZRAqW7Ol04bcjlHZHirSsrflkmz/elHdtqQ3dFIU78F+RrAnS/Sngy2H97mGVOaffZj6N/8bnwp8c3fKUq/qAAj3uqt7yPqeYWsAT++MYnZnjw2uxw4R9Ee2GjnF/vBX8gam9cbr66fjQA03arqI19DbH3h5kr92WedtqoqK3QrSULk9KuMuDpgk/0pjAsSXdzPnU06OWqXvU726fw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=H4nJzZIpyKRodclHxXuPmT9JTwu5KWrwNqPIqjtAtg4=; b=JvclYljef9v3fa2qVP1ZbD9XGC+5x5OXFp0MhR+RZVRG260pGftiIU4Aynnbqoyv/ekm5zlducqlzWodqcSQ6uaxlfJJ/895mOqDftohDWOT7MNFXtvvrXSmywWtMO0j2SLHktT70jIkHeSke2zGgUTbcUGG4WmGJGswRPFPJaEe382LDHdZST0pkEx/BcmlJHCb33OKCl7ivhwQiUcFmQpXUDwMqhdQGexV0a+MJnfUgY8dUB/SHmrqnwUqR4rke8IE5gt/bGM1zulm2SyrJfIfsTz6Jlogg39Bq1iQTlAFYhl2VEz77QuEWqB/nBiwH97WTgR9ezTvZMcPBKiAbA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector2-armh-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=H4nJzZIpyKRodclHxXuPmT9JTwu5KWrwNqPIqjtAtg4=; b=wTcARbcHieDxY3YyOd2kQSYfY3kgZIxdMSULXhdKFh+yL+3GpVJeqpig+z5OoXuIpAPs5qiCBcC4tXWYgXGBjJLlu5BRd4IzjOkqHgF81fKYQUrCu8Fj1uktDMe3gfyjn8t4xtbKP5p7j/Ly1GSeue9WWqF1m3zGo7FDuIa1q/I= Authentication-Results-Original: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com; Received: from AS8PR08MB6806.eurprd08.prod.outlook.com (2603:10a6:20b:39b::12) by VI1PR08MB3326.eurprd08.prod.outlook.com (2603:10a6:803:49::32) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4888.12; Wed, 19 Jan 2022 16:46:37 +0000 Received: from AS8PR08MB6806.eurprd08.prod.outlook.com ([fe80::34b6:4579:9e43:6b95]) by AS8PR08MB6806.eurprd08.prod.outlook.com ([fe80::34b6:4579:9e43:6b95%5]) with mapi id 15.20.4888.014; Wed, 19 Jan 2022 16:46:37 +0000 Subject: Re: [PATCH v4 1/2] ShellPkg/AcpiView: Adds ACPI_PARSER bitfield parser From: "Sami Mujawar" To: Abdul Lateef Attar , devel@edk2.groups.io Cc: Ray Ni , Zhichao Gao , nd References: <20211219144437.3721-1-abdattar@amd.com> <20211219144437.3721-2-abdattar@amd.com> <7dac633c-4bae-460d-0998-7da08b459023@arm.com> Message-ID: Date: Wed, 19 Jan 2022 16:46:46 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.0.1 In-Reply-To: <7dac633c-4bae-460d-0998-7da08b459023@arm.com> X-ClientProxiedBy: LNXP265CA0021.GBRP265.PROD.OUTLOOK.COM (2603:10a6:600:5e::33) To AS8PR08MB6806.eurprd08.prod.outlook.com (2603:10a6:20b:39b::12) MIME-Version: 1.0 X-MS-Office365-Filtering-Correlation-Id: 94218061-d753-48c8-385b-08d9db6b4a34 X-MS-TrafficTypeDiagnostic: VI1PR08MB3326:EE_|VE1EUR03FT055:EE_|DB9PR08MB6875:EE_ X-Microsoft-Antispam-PRVS: x-checkrecipientrouted: true NoDisclaimer: true X-MS-Oob-TLC-OOBClassifiers: OLM:10000;OLM:10000; X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam-Untrusted: BCL:0; X-Microsoft-Antispam-Message-Info-Original: qrVOOfGv6UtIJsRpHMeCOFe7s8mntO2EQPhyqzEzORkBDQznBa6WSF3GpyG2k1I8zoV7OjWf0PeY+7EeQDWORRQjjT76eeSQ6Vbm1fU5VDpmoxchAyTEd2VwxKdhC6FyO8cEdEamwn80YdjNy1Geo+RFypBJtc0g03JEj/gtV8xqRiPUzC7+IWq/QInP4CEVCWyxQCxutq+fPxb2eWvswlleL75n36NehEmAjGRH6rKrl3AmhA58vw0LyB+lsyYaUZaN8fmbUTnXzTA8aOi92hLUxpHrtVbwVGY7+LaN3t0V1Vzl/WkgkV1dL5SrQ+fiteiw2mhw0kJVdSnAdBbHn5q1Q0mAXwqZKo8DkEo3OPEiiXFbVlJxCAQK3094wLo0EB48H7fbSB+SIweVN72WfofoBIVVd5I3inza65V8tViAud+lAdi+1sj2QXODGMj8v68eIr8M4RvW+0dzsUyKJoA43in1RlweQ5XuuQv5AVTp/lp3TCmBam9ED1sIIeU1LjdSTMHdC2tomI7FC/qq2ZhMu1c5GzVr08uYgQZgVqEOH4x2XQEw+W0jXmyrpxyK0Jz2bXJtlR+wJiFt8Bx66XoARNTbnPioEkm79+RhHVIdw2tM3GVE+2gl9KvJtD7caqvtUCHbD33f3rTh5uvSlOzTSrcYlmBNwXRBMmJEoUaaShgYSjZWAxz9wt/cSAbqYQ8goz+xse3VLaPIDhluUdnENknWp4YtImA+gZopXQ1Mz8nzs+HK9cyUBYVjdGl9ef6qEJXYzXU/D81hoFr8vw== X-Forefront-Antispam-Report-Untrusted: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:AS8PR08MB6806.eurprd08.prod.outlook.com;PTR:;CAT:NONE;SFS:(4636009)(366004)(66476007)(38100700002)(316002)(66556008)(31686004)(2616005)(30864003)(6486002)(6666004)(52116002)(6506007)(5660300002)(66946007)(36756003)(38350700002)(6512007)(53546011)(186003)(2906002)(31696002)(4326008)(54906003)(86362001)(33964004)(26005)(508600001)(83380400001)(8676002)(8936002)(44832011)(45980500001)(43740500002);DIR:OUT;SFP:1101; X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR08MB3326 Original-Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com; Return-Path: Sami.Mujawar@arm.com X-EOPAttributedMessage: 0 X-MS-Exchange-Transport-CrossTenantHeadersStripped: VE1EUR03FT055.eop-EUR03.prod.protection.outlook.com X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id-Prvs: 90c48da8-8c99-4fc6-6315-08d9db6b42b8 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: 7GcyTmC52hajbv6gCkGufOKSurTUpsI8df4WvJubYejNvfDDU8MZiNPBfH32qhlv0f/B8tYH29vo2JXVQRjSwwozn2Ri5PSP5bkC3g1/g/pM1rRvXUWDLmR/ULYvm5ugCZqm3sXBnKZguYpcwHchOFRqqtHSraRiVt012CnDtNJrpUMDAtpMxvsYXQEoO8yRWzXmMT/uzhjG/PCyd8usga56Rb4rPdX5wziI4uqNsflMiTxXGENPhbSrTmfY6pBtabGfyhbwK0OlDp2KV7nm/URdvSasIa+QCh0ykEusYR5/v4e44PwYIMksEA77j0Vp8htbN0EWSP8TzQ+LBRugXP2ZL6RYXM0vOjgrjhM9FGR84T9+OPuB4s+jHePbcp9NBmoiSGEVVaqmwAP9XUl4jB7F5UBf1BmW7EQLhMIBKfteZpMVcgxRun0cBWZfxU6fZkqGNja+1LGzc9Zinbj3Yu77GgpoGVQt3h7p+Z1EsaAxAW8Enap573+6SxepTMK16C3HByTyNGnrvpeN9KzQZ+uXe7yGJA4ER22uUhWvf6aqDAI6oGT82iK5+l/NAAr5BX4rYwjgTkoqwNsY0LVW7MiVkL9j60DGncxGVtkU78ueLVLmh/godEjajd+t6GTrjWSxBoaUGvP7glc5+hJ7JvJO+AZ7uDnuvO3QSs0q0QaeG93IkacajMq3LioYUvl1VT5LDUMvguDk1hESLMhBlY3Wc2RT1fDu06zDT+Va+Io= X-Forefront-Antispam-Report: CIP:63.35.35.123;CTRY:IE;LANG:en;SCL:1;SRV:;IPV:CAL;SFV:NSPM;H:64aa7808-outbound-1.mta.getcheckrecipient.com;PTR:ec2-63-35-35-123.eu-west-1.compute.amazonaws.com;CAT:NONE;SFS:(4636009)(36840700001)(46966006)(31696002)(82310400004)(33964004)(36756003)(81166007)(8936002)(53546011)(54906003)(336012)(47076005)(6506007)(86362001)(6666004)(356005)(44832011)(31686004)(316002)(2616005)(508600001)(83380400001)(186003)(26005)(36860700001)(70586007)(30864003)(6512007)(70206006)(4326008)(5660300002)(8676002)(2906002)(6486002)(43740500002);DIR:OUT;SFP:1101; X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 19 Jan 2022 16:46:50.1149 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: 94218061-d753-48c8-385b-08d9db6b4a34 X-MS-Exchange-CrossTenant-Id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=f34e5979-57d9-4aaa-ad4d-b122a662184d;Ip=[63.35.35.123];Helo=[64aa7808-outbound-1.mta.getcheckrecipient.com] X-MS-Exchange-CrossTenant-AuthSource: VE1EUR03FT055.eop-EUR03.prod.protection.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB9PR08MB6875 Content-Type: multipart/alternative; boundary="------------56E3F2099E89C1CA2EB353D6" Content-Language: en-GB --------------56E3F2099E89C1CA2EB353D6 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Hi Abdul, Please also create a bugzilla ticket and add a reference to it in the commit message. Regards, Sami Mujawar On 19/01/2022 04:44 PM, Sami Mujawar wrote: > > Hi Abdul, > > Thank you for providing a patch to add this feature to Acpiview. > > I have some minor feedback marked inline as [SAMI]. > > Regards, > > Sami Mujawar > > > On 19/12/2021 02:44 PM, Abdul Lateef Attar wrote: >> Adds ParseAcpiBitFields() which is based on >> ParseAcpi() and capable of parsing the bit fields. >> Supports parsing of UINT8, UINT16, UINT32 and UINT64 byte data. >> >> Cc: Ray Ni >> Cc: Zhichao Gao >> Cc: Sami Mujawar >> Signed-off-by: Abdul Lateef Attar >> --- >> ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h | 45 +++++ >> ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c | 185 ++++++++++++++++++++ >> 2 files changed, 230 insertions(+) >> >> diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h >> index 5c916a4720b8..83266e8ec2d3 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 >> >> **/ >> >> >> >> @@ -251,6 +252,11 @@ typedef VOID (EFIAPI *FNPTR_FIELD_VALIDATOR)(UINT8 *Ptr, VOID *Context); >> the field data. If the field is more complex and requires additional >> >> processing for formatting and representation a print formatter function >> >> can be specified in 'PrintFormatter'. >> >> + >> >> + ParseAcpiBitFields() uses AcpiParser structure to parse the bit fields. >> >> + It considers Length as a number of bits that need to be parsed. >> >> + Also, the Offset field will be considered as starting offset of the bitfield. >> >> + >> >> The PrintFormatter function may choose to use the format string >> >> specified by 'Format' or use its own internal format string. >> >> >> >> @@ -264,10 +270,12 @@ typedef struct AcpiParser { >> >> >> /// The length of the field. >> >> /// (Byte Length column from ACPI table spec) >> >> + /// Length(in bits) of the bitfield if used with ParseAcpiBitFields(). >> >> UINT32 Length; >> >> >> >> /// The offset of the field from the start of the table. >> >> /// (Byte Offset column from ACPI table spec) >> >> + /// The Bit offset of the field if used with ParseAcpiBitFields(). >> >> UINT32 Offset; >> >> >> >> /// Optional Print() style format string for tracing the data. If not >> >> @@ -364,6 +372,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 cb193a5ea449..94ee26f42ab9 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 >> >> #include >> >> #include >> >> +#include >> >> #include "AcpiParser.h" >> >> #include "AcpiView.h" >> >> #include "AcpiViewConfig.h" >> >> @@ -752,3 +754,186 @@ 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 zero or exceeding the 64 bit 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; > [SAMI] Is there a use-case where the bitfield length will be zero? I > think any unused bits would be represented as Reserved. Considering > this, Parser[Index].Length == 0 would mean an incorrect an incorrect > description inACPI_PARSER, right? > If so, the default case for the switch statement below would print > "ERROR: %a: CANNOT PARSE THIS FIELD, Field Length". Can you check > this, please? > [/SAMI] >> + } >> >> + >> >> + 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 + 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; > [SAMI] ACPI_PARSER.ItemPtr is used to get a reference to the field > data in the original ACPI table data. > In the current case, Parser[Index].ItemPtr is being set to a local > variable (i.e. Data). I don't think this is what we want. I think it > would be better to not supportACPI_PARSER.ItemPtr for BitFields. > I would recommend adding a comment to clarify that ItemPtr is not > supported for BitFields in this function, as well as the documentation > for ACPI_PARSER structure. > [/SAMI] >> + } >> >> + >> >> + Offset += Parser[Index].Length; >> >> + } // for >> >> + >> >> + // Decrement the Indent >> >> + gIndent -= Indent; >> >> + return Offset; >> >> +} >> > --------------56E3F2099E89C1CA2EB353D6 Content-Type: text/html; charset=utf-8 Content-Transfer-Encoding: 8bit

Hi Abdul,

Please also create a bugzilla ticket and add a reference to it in the commit message.

Regards,

Sami Mujawar


On 19/01/2022 04:44 PM, Sami Mujawar wrote:

Hi Abdul,

Thank you for providing a patch to add this feature to Acpiview.

I have some minor feedback marked inline as [SAMI].

Regards,

Sami Mujawar


On 19/12/2021 02:44 PM, Abdul Lateef Attar wrote:
Adds ParseAcpiBitFields() which is based on
ParseAcpi() and capable of parsing the bit fields.
Supports parsing of UINT8, UINT16, UINT32 and UINT64 byte data.

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 |  45 +++++
 ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c | 185 ++++++++++++++++++++
 2 files changed, 230 insertions(+)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
index 5c916a4720b8..83266e8ec2d3 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

 **/

 

@@ -251,6 +252,11 @@ typedef VOID (EFIAPI *FNPTR_FIELD_VALIDATOR)(UINT8 *Ptr, VOID *Context);
   the field data. If the field is more complex and requires additional

   processing for formatting and representation a print formatter function

   can be specified in 'PrintFormatter'.

+

+  ParseAcpiBitFields() uses AcpiParser structure to parse the bit fields.

+  It considers Length as a number of bits that need to be parsed.

+  Also, the Offset field will be considered as starting offset of the bitfield.

+

   The PrintFormatter function may choose to use the format string

   specified by 'Format' or use its own internal format string.

 

@@ -264,10 +270,12 @@ typedef struct AcpiParser {
 

   /// The length of the field.

   /// (Byte Length column from ACPI table spec)

+  /// Length(in bits) of the bitfield if used with ParseAcpiBitFields().

   UINT32                   Length;

 

   /// The offset of the field from the start of the table.

   /// (Byte Offset column from ACPI table spec)

+  /// The Bit offset of the field if used with ParseAcpiBitFields().

   UINT32                   Offset;

 

   /// Optional Print() style format string for tracing the data. If not

@@ -364,6 +372,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 cb193a5ea449..94ee26f42ab9 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"

@@ -752,3 +754,186 @@ 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 zero or exceeding the 64 bit 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;
[SAMI] Is there a use-case where the bitfield length will be zero? I think any unused bits would be represented as Reserved. Considering this, Parser[Index].Length == 0 would mean an incorrect an incorrect description in ACPI_PARSER, right?
If so, the default case for the switch statement below would print "ERROR: %a: CANNOT PARSE THIS FIELD, Field Length". Can you check this, please?
[/SAMI]
+    }

+

+    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 + 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;
[SAMI]  ACPI_PARSER.ItemPtr is used to get a reference to the field data in the original ACPI table data.
In the current case, Parser[Index].ItemPtr is being set to a local variable (i.e. Data). I don't think this is what we want. I think it would be better to not support ACPI_PARSER.ItemPtr for BitFields.
I would recommend adding a comment to clarify that ItemPtr is not supported for BitFields in this function, as well as the documentation for ACPI_PARSER structure.
[/SAMI]
+    }

+

+    Offset += Parser[Index].Length;

+  } // for

+

+  // Decrement the Indent

+  gIndent -= Indent;

+  return Offset;

+}


--------------56E3F2099E89C1CA2EB353D6--