From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from EUR05-DB8-obe.outbound.protection.outlook.com (EUR05-DB8-obe.outbound.protection.outlook.com [40.107.20.52]) by mx.groups.io with SMTP id smtpd.web10.9525.1633101204613304917 for ; Fri, 01 Oct 2021 08:13:25 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@armh.onmicrosoft.com header.s=selector2-armh-onmicrosoft-com header.b=ZUmVvSL/; spf=pass (domain: arm.com, ip: 40.107.20.52, 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=KEs/vILNv8psuNlUWG3FlesjRmL2Bpu4UnvWHLG6GeU=; b=ZUmVvSL/DC75+38qVhI9eM1K31hirtzVBKHNjTn/lstA9l14qKOP+c6js9O+7zQLKP0Chv00ck7XgpUKSE5QwM2vKdaL/1eNu2It3u5BU1Yf5+04if6Ho5MW0CYeujSOmMNd1PQE26qlgUrR97Vfyt6OTL0fHx9znWS49YGYGHg= Received: from AM6P193CA0107.EURP193.PROD.OUTLOOK.COM (2603:10a6:209:88::48) by DB6PR08MB2888.eurprd08.prod.outlook.com (2603:10a6:6:25::21) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4566.15; Fri, 1 Oct 2021 15:13:21 +0000 Received: from VE1EUR03FT044.eop-EUR03.prod.protection.outlook.com (2603:10a6:209:88:cafe::8c) by AM6P193CA0107.outlook.office365.com (2603:10a6:209:88::48) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4566.14 via Frontend Transport; Fri, 1 Oct 2021 15:13:21 +0000 X-MS-Exchange-Authentication-Results: spf=pass (sender IP is 63.33.187.114) smtp.mailfrom=arm.com; edk2.groups.io; dkim=pass (signature was verified) header.d=armh.onmicrosoft.com;edk2.groups.io; dmarc=pass action=none header.from=arm.com; Received-SPF: Pass (protection.outlook.com: domain of arm.com designates 63.33.187.114 as permitted sender) receiver=protection.outlook.com; client-ip=63.33.187.114; helo=64aa7808-outbound-2.mta.getcheckrecipient.com; Received: from 64aa7808-outbound-2.mta.getcheckrecipient.com (63.33.187.114) by VE1EUR03FT044.mail.protection.outlook.com (10.152.19.106) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4566.14 via Frontend Transport; Fri, 1 Oct 2021 15:13:20 +0000 Received: ("Tessian outbound c9f4ff96a6ad:v103"); Fri, 01 Oct 2021 15:13:20 +0000 X-CheckRecipientChecked: true X-CR-MTA-CID: 3fb007438bfe56df X-CR-MTA-TID: 64aa7808 Received: from 91c6d7db4aeb.2 by 64aa7808-outbound-1.mta.getcheckrecipient.com id 8E09B72C-BABA-4B4B-9D10-CA02F69DF9B3.1; Fri, 01 Oct 2021 15:13:10 +0000 Received: from EUR02-HE1-obe.outbound.protection.outlook.com by 64aa7808-outbound-1.mta.getcheckrecipient.com with ESMTPS id 91c6d7db4aeb.2 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384); Fri, 01 Oct 2021 15:13:10 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Axejhg0XsuHssVLB1eXPk6WUHN+Ln1EAwpV2jTaQu/JdAjONCtW9hrhQgbfqE2UTa10rG9MgeKiPpZFVEqxZFPUspZtuabZglcstKGcr9pA8+FUJwAuUM1ONc5+wwgYbGgaTT0dg9wizLln4EbHmlQ3ShMtvwZZFV0pXbmRzPZDrCyVc+I6j0laKsxytlUGevCSsLgshaPt+zmg0sJstQbJIK3AMaAtECuyD9/PXKwNzy7r2CPwx4QLxHUPUtmvnbPbk0x6PVHUSx38mhWOBh454fTX6MYXrBPFolqWXz3j5o8Re/pt4wZaPjMw8W4kfmikSejCwz4/dDN6XaXu8Gg== 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=KEs/vILNv8psuNlUWG3FlesjRmL2Bpu4UnvWHLG6GeU=; b=f6N9PAFaUhrEX2PThyWaWIkt5hXa4Jv9yMphvt9BthiPiqILCW0y7WwOxSSITwlWwwJ/lMx4iX1kUfgWGlOfysO8c1qUsvTsyaoapRsUXl2wCQ6+Opt5mLxRyGNyFpyyROACfLjrF9bTzIxR8/5BARJ055n/SEIyaBT5jrzfvgubboc7jfaoFbYmrmwLgN2XZTKfq+OJeDVfLX9VzvOhTNsfiwdM0zr5tXuDNk+V9L1LWYRGxyTCpXrC94hRNSdYnaLpTpmBDpT7tYzDbN3qOSdE3nuI36JpYZYBdA6njBQO38b2UopcNVcZ7aaqT40vT8osTHAI6GKiD1tHQ3allg== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; 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=KEs/vILNv8psuNlUWG3FlesjRmL2Bpu4UnvWHLG6GeU=; b=ZUmVvSL/DC75+38qVhI9eM1K31hirtzVBKHNjTn/lstA9l14qKOP+c6js9O+7zQLKP0Chv00ck7XgpUKSE5QwM2vKdaL/1eNu2It3u5BU1Yf5+04if6Ho5MW0CYeujSOmMNd1PQE26qlgUrR97Vfyt6OTL0fHx9znWS49YGYGHg= Authentication-Results-Original: arm.com; dkim=none (message not signed) header.d=none;arm.com; dmarc=none action=none header.from=arm.com; Received: from AS8PR08MB6806.eurprd08.prod.outlook.com (2603:10a6:20b:39b::12) by AS8PR08MB5944.eurprd08.prod.outlook.com (2603:10a6:20b:297::11) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4566.19; Fri, 1 Oct 2021 15:13:08 +0000 Received: from AS8PR08MB6806.eurprd08.prod.outlook.com ([fe80::c8a5:672a:9ff2:e554]) by AS8PR08MB6806.eurprd08.prod.outlook.com ([fe80::c8a5:672a:9ff2:e554%6]) with mapi id 15.20.4566.019; Fri, 1 Oct 2021 15:13:07 +0000 Subject: Re: [PATCH v1 08/13] DynamicTablesPkg: AML code generation to Return a NameString To: Pierre.Gondois@arm.com, devel@edk2.groups.io, Alexei Fedorov Cc: Akanksha Jain , Alexandru Elisei , nd References: <20210623114039.24491-1-Pierre.Gondois@arm.com> <20210623114039.24491-9-Pierre.Gondois@arm.com> From: "Sami Mujawar" Message-ID: Date: Fri, 1 Oct 2021 16:13:07 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.0.1 In-Reply-To: <20210623114039.24491-9-Pierre.Gondois@arm.com> X-ClientProxiedBy: LO4P123CA0098.GBRP123.PROD.OUTLOOK.COM (2603:10a6:600:191::13) To AS8PR08MB6806.eurprd08.prod.outlook.com (2603:10a6:20b:39b::12) MIME-Version: 1.0 Received: from [10.1.196.43] (217.140.106.52) by LO4P123CA0098.GBRP123.PROD.OUTLOOK.COM (2603:10a6:600:191::13) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4566.14 via Frontend Transport; Fri, 1 Oct 2021 15:13:06 +0000 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: d48f5e8c-4fb7-4d6a-c065-08d984ee0168 X-MS-TrafficTypeDiagnostic: AS8PR08MB5944:|DB6PR08MB2888: X-MS-Exchange-Transport-Forked: True X-Microsoft-Antispam-PRVS: x-checkrecipientrouted: true NoDisclaimer: true X-MS-Oob-TLC-OOBClassifiers: OLM:7219;OLM:7219; X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam-Untrusted: BCL:0; X-Microsoft-Antispam-Message-Info-Original: 1xEHJv1ahCV2T7jfNLBUHSn38ShJkdWPuyZRpzmMcy65lKITtN7OEH5Jk9b4ZHLNKRmvFZYSN4zXy7tCwsTjs7UDaC6X8GY6ChZl1r3FWXgd4KwGd2KxGlvc7Ldb59KqWLushYTdl0/9boHuf4kpmE/f1Aaas+WoDgi16OQ1wr5HGccTelVSWf0uuzm/JKUfKd71BCsXeUtNgVL21C66ZSITB9/InM+f5pmooCIophYaSF2yzbnM0i/5XP72oF+z0ondxeU8IVasEMLwcXSrxfpZtF4XO1Si6XHjad75ba0kz7k2zZjDqCw6LEALoHIQmccZJj0vQFa2oGOdcE5Nuxd6XJfIE0qpIwJ+A/3mq3nMSyLMPfqKID0qTMOHrbrnyEvaJeG/xr9ge1vMdjaBy/Ky5x5dcWr97j3B+AVR55fjAxRmHhp+ELox1yCZ15cWt92QNc8h2Xw3ekrrs35i3Z9Sh3O1nW6GogYfBypUnQaIXFqFjp7sZZmdYQFL7vtHS0J0MjgFgCZcnfRVOVTu1z0j1umJnkzwZ6llXT3lzWZXJVDb7OdlOytTCpL68RFqvVVeCaG8NaJHtphuuOfObypoghpTzau3qrSMbGahyEPwnKcRT8exyKnEw09aiSpHzuIapqI9rxOyx57kEUCQwYaRoegdiu3nzFjba2estxfA5cIIJrqYkAEgjJd0eWpj0Ai4kW2pIjT25cZbBbr73p/whKqcHqbIxV0ncCRXMC6nY7gAOcVUXlQyHooWrWikvC9wZ+s5jHAQ1PuIc3oq9w== 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)(31696002)(83380400001)(37006003)(30864003)(36756003)(956004)(66946007)(8936002)(66476007)(86362001)(2616005)(66556008)(508600001)(6486002)(186003)(8676002)(6862004)(31686004)(26005)(16576012)(52116002)(53546011)(54906003)(5660300002)(33964004)(316002)(6636002)(2906002)(38350700002)(38100700002)(44832011)(4326008)(43740500002)(45980500001);DIR:OUT;SFP:1101; X-MS-Exchange-Transport-CrossTenantHeadersStamped: AS8PR08MB5944 Original-Authentication-Results: arm.com; dkim=none (message not signed) header.d=none;arm.com; dmarc=none action=none header.from=arm.com; Return-Path: Sami.Mujawar@arm.com X-EOPAttributedMessage: 0 X-MS-Exchange-Transport-CrossTenantHeadersStripped: VE1EUR03FT044.eop-EUR03.prod.protection.outlook.com X-MS-Office365-Filtering-Correlation-Id-Prvs: abfd1fd0-94bb-4192-aba5-08d984edf910 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: MEiq7RQbe6qx5cDvXQymUIY7ByBJVlSdXF8Ggz98OvYO1BTGce0DesR/gXBKvUCXFsaGA+APhnw5CcHKB0EIh4AZuBJeZuehdxQe9vfvM6y7A5vXVCDEhioLDeNF4CGXUHYieffLqS7C0RZc2/dctMjuu69EKfAYZPgvkZkhjk/h0zDEGuVfCXr7v7Y7oUFpiVUwImietZcaceN/G0yHNlMvStriLFRy8dur+E99mjieAgiYJsi7iG8P5Un2uFdjcCQJVnys/nt/QEP/uHwQBluCJm7Mjqyvgacq+QjBGqIcmvTBnX/q1tEyU1/O+M/fh6bt23DhBvMYcT4z9nPPGa8zOW5yA7KFdDfpZtzOo0KOv8L2mUT5lswjrvBrF1vVRR36OPIGuosb1t43CZ95aFdhtqPeTnKcXihWENYau3knJasUK62iYKm2VpWARUOiVJCr4uNbJpZnPdmzjIovTEWcOxJgCppfcGCGcjKPIM+NOp48bc5BYt7gdBeMHcmiLy7omFifR3Yuoe6ZfcBvWDz4YL30wfSN5G6o3qWHAgrQNlwTxtevY3odc5k0weZHgI22VGEqVn+aHdnqlbtGoEegsUhXpn3N+EMY2a0P9w7ENvJ1nTCzbbqClc/5qGybE+EaE8h3SJY+vLI7BDDPbTjQSL2PtGnbz67Dbaij6MB9RMVfjSIPZrbfohJLWx+iDRiBuYVdyeBXQVzrYKOw64rFUIZxZyef4imN3yBWh5g= X-Forefront-Antispam-Report: CIP:63.33.187.114;CTRY:IE;LANG:en;SCL:1;SRV:;IPV:CAL;SFV:NSPM;H:64aa7808-outbound-2.mta.getcheckrecipient.com;PTR:ec2-63-33-187-114.eu-west-1.compute.amazonaws.com;CAT:NONE;SFS:(4636009)(46966006)(36840700001)(186003)(508600001)(2616005)(5660300002)(36860700001)(33964004)(44832011)(31696002)(316002)(47076005)(336012)(16576012)(956004)(30864003)(53546011)(6486002)(54906003)(37006003)(8936002)(82310400003)(70586007)(2906002)(6636002)(4326008)(86362001)(356005)(26005)(81166007)(83380400001)(8676002)(36756003)(6862004)(31686004)(70206006)(43740500002);DIR:OUT;SFP:1101; X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 01 Oct 2021 15:13:20.8901 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: d48f5e8c-4fb7-4d6a-c065-08d984ee0168 X-MS-Exchange-CrossTenant-Id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=f34e5979-57d9-4aaa-ad4d-b122a662184d;Ip=[63.33.187.114];Helo=[64aa7808-outbound-2.mta.getcheckrecipient.com] X-MS-Exchange-CrossTenant-AuthSource: VE1EUR03FT044.eop-EUR03.prod.protection.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB6PR08MB2888 Content-Type: multipart/alternative; boundary="------------03AFF6FF25EE369BC5195D8C" Content-Language: en-GB --------------03AFF6FF25EE369BC5195D8C Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Hi Pierre, Please find my feedback inline marked [SAMI]. Regards, Sami Mujawar On 23/06/2021 12:40 PM, Pierre.Gondois@arm.com wrote: > From: Pierre Gondois > > Add AmlCodeGenReturnNameString() to generate AML code for a > Return object node, returning the object as a NameString. > > AmlCodeGenReturn ("NAM1", ParentNode, NewObjectNode) is > equivalent of the following ASL code: > Return(NAM1) > > Signed-off-by: Pierre Gondois > --- > .../Common/AmlLib/CodeGen/AmlCodeGen.c | 178 ++++++++++++++++++ > 1 file changed, 178 insertions(+) > > diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c > index 32665f7f8d8f..75dadbaf4ac3 100644 > --- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c > +++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c > @@ -1137,3 +1137,181 @@ error_handler1: > } > return Status; > } > + > +/** AML code generation for a Return object node. > + > + AmlCodeGenReturn (ReturnNode, ParentNode, NewObjectNode) is > + equivalent of the following ASL code: > + Return([Content of the ReturnNode]) > + > + The ACPI 6.3 specification, s20.2.5.3 "Type 1 Opcodes Encoding" states: > + DefReturn := ReturnOp ArgObject > + ReturnOp := 0xA4 > + ArgObject := TermArg => DataRefObject > + > + Thus, the ReturnNode must be evaluated as a DataRefObject. It can > + be a NameString referencing an object. As this CodeGen Api doesn't > + do semantic checking, it is strongly advised to check the AML bytecode > + generated by this function against an ASL compiler. > + > + The ReturnNode must be generated inside a Method body scope. > + > + @param [in] ReturnNode The object returned by the Return ASL statement. > + This node is deleted if an error occurs. > + @param [in] ParentNode If provided, set ParentNode as the parent > + of the node created. [SAMI] Can we mention here that the ParentNode must be a MethodOp Node, please? Also is it possible to add a check to that effect. > + @param [out] NewObjectNode If success, contains the created node. > + > + @retval EFI_SUCCESS Success. > + @retval EFI_INVALID_PARAMETER Invalid parameter. > + @retval EFI_OUT_OF_RESOURCES Failed to allocate memory. > +**/ > +STATIC > +EFI_STATUS > +EFIAPI > +AmlCodeGenReturn ( > + IN AML_NODE_HEADER * ReturnNode, > + IN AML_NODE_HEADER * ParentNode, OPTIONAL > + OUT AML_OBJECT_NODE ** NewObjectNode OPTIONAL > + ) > +{ > + EFI_STATUS Status; > + AML_OBJECT_NODE * ObjectNode; > + > + if ((ReturnNode == NULL) || > + ((ParentNode == NULL) && (NewObjectNode == NULL))) { > + ASSERT (0); > + return EFI_INVALID_PARAMETER; > + } > + > + ObjectNode = NULL; > + > + Status = AmlCreateObjectNode ( > + AmlGetByteEncodingByOpCode (AML_RETURN_OP, 0), > + 0, > + &ObjectNode > + ); > + if (EFI_ERROR (Status)) { > + ASSERT (0); > + goto error_handler; > + } > + > + Status = AmlSetFixedArgument ( > + ObjectNode, > + EAmlParseIndexTerm0, > + (AML_NODE_HEADER*)ReturnNode > + ); > + if (EFI_ERROR (Status)) { > + ASSERT (0); > + goto error_handler; > + } > + > + ReturnNode = NULL; > + > + Status = LinkNode ( > + ObjectNode, > + ParentNode, > + NewObjectNode > + ); > + if (EFI_ERROR (Status)) { > + ASSERT (0); > + goto error_handler; > + } > + > + return Status; > + > +error_handler: > + AmlDeleteTree (ReturnNode); [SAMI] Add a check to see if ReturnNode is NULL before deleting. > + if (ObjectNode != NULL) { > + AmlDeleteTree ((AML_NODE_HEADER*)ObjectNode); > + } > + return Status; > +} > + > +/** AML code generation for a Return object node, > + returning the object as an input NameString. > + > + AmlCodeGenReturn ("NAM1", ParentNode, NewObjectNode) is > + equivalent of the following ASL code: > + Return(NAM1) > + > + The ACPI 6.3 specification, s20.2.5.3 "Type 1 Opcodes Encoding" states: > + DefReturn := ReturnOp ArgObject > + ReturnOp := 0xA4 > + ArgObject := TermArg => DataRefObject > + > + Thus, the ReturnNode must be evaluated as a DataRefObject. It can > + be a NameString referencing an object. As this CodeGen Api doesn't > + do semantic checking, it is strongly advised to check the AML bytecode > + generated by this function against an ASL compiler. > + > + The ReturnNode must be generated inside a Method body scope. > + > + @param [in] NameString The object referenced by this NameString > + is returned by the Return ASL statement. > + Must be a NULL-terminated ASL NameString > + e.g.: "NAM1", "_SB.NAM1", etc. > + The input string is copied. > + @param [in] ParentNode If provided, set ParentNode as the parent > + of the node created. [SAMI] Same comment as above. > + @param [out] NewObjectNode If success, contains the created node. > + > + @retval EFI_SUCCESS Success. > + @retval EFI_INVALID_PARAMETER Invalid parameter. > + @retval EFI_OUT_OF_RESOURCES Failed to allocate memory. > +**/ > +STATIC > +EFI_STATUS > +EFIAPI > +AmlCodeGenReturnNameString ( > + IN CONST CHAR8 * NameString, > + IN AML_NODE_HEADER * ParentNode, OPTIONAL > + OUT AML_OBJECT_NODE ** NewObjectNode OPTIONAL > + ) > +{ > + EFI_STATUS Status; > + AML_DATA_NODE * DataNode; > + CHAR8 * AmlNameString; > + UINT32 AmlNameStringSize; > + > + DataNode = NULL; > + > + Status = ConvertAslNameToAmlName (NameString, &AmlNameString); > + if (EFI_ERROR (Status)) { > + ASSERT (0); > + return Status; > + } > + > + Status = AmlGetNameStringSize (AmlNameString, &AmlNameStringSize); > + if (EFI_ERROR (Status)) { > + ASSERT (0); > + goto exit_handler; > + } > + > + Status = AmlCreateDataNode ( > + EAmlNodeDataTypeNameString, > + (UINT8*)AmlNameString, > + AmlNameStringSize, > + &DataNode > + ); > + if (EFI_ERROR (Status)) { > + ASSERT (0); > + goto exit_handler; > + } > + > + // AmlCodeGenReturn() deletes DataNode if error. > + Status = AmlCodeGenReturn ( > + (AML_NODE_HEADER*)DataNode, > + ParentNode, > + NewObjectNode > + ); > + if (EFI_ERROR (Status)) { > + ASSERT (0); > + } [SAMI] Can we replace this withASSERT_EFI_ERROR()? > + > +exit_handler: > + if (AmlNameString != NULL) { > + FreePool (AmlNameString); > + } > + return Status; > +} --------------03AFF6FF25EE369BC5195D8C Content-Type: text/html; charset=utf-8 Content-Transfer-Encoding: 7bit

Hi Pierre,

Please find my feedback inline marked [SAMI].

Regards,

Sami Mujawar


On 23/06/2021 12:40 PM, Pierre.Gondois@arm.com wrote:
From: Pierre Gondois <Pierre.Gondois@arm.com>

Add AmlCodeGenReturnNameString() to generate AML code for a
Return object node, returning the object as a NameString.

AmlCodeGenReturn ("NAM1", ParentNode, NewObjectNode) is
equivalent of the following ASL code:
  Return(NAM1)

Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
---
 .../Common/AmlLib/CodeGen/AmlCodeGen.c        | 178 ++++++++++++++++++
 1 file changed, 178 insertions(+)

diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
index 32665f7f8d8f..75dadbaf4ac3 100644
--- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
+++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
@@ -1137,3 +1137,181 @@ error_handler1:
   }
   return Status;
 }
+
+/** AML code generation for a Return object node.
+
+  AmlCodeGenReturn (ReturnNode, ParentNode, NewObjectNode) is
+  equivalent of the following ASL code:
+    Return([Content of the ReturnNode])
+
+  The ACPI 6.3 specification, s20.2.5.3 "Type 1 Opcodes Encoding" states:
+    DefReturn := ReturnOp ArgObject
+    ReturnOp := 0xA4
+    ArgObject := TermArg => DataRefObject
+
+  Thus, the ReturnNode must be evaluated as a DataRefObject. It can
+  be a NameString referencing an object. As this CodeGen Api doesn't
+  do semantic checking, it is strongly advised to check the AML bytecode
+  generated by this function against an ASL compiler.
+
+  The ReturnNode must be generated inside a Method body scope.
+
+  @param [in]  ReturnNode     The object returned by the Return ASL statement.
+                              This node is deleted if an error occurs.
+  @param [in]  ParentNode     If provided, set ParentNode as the parent
+                              of the node created.
[SAMI] Can we mention here that the ParentNode must be a MethodOp Node, please? Also is it possible to add a check to that effect.
+  @param [out] NewObjectNode  If success, contains the created node.
+
+  @retval EFI_SUCCESS             Success.
+  @retval EFI_INVALID_PARAMETER   Invalid parameter.
+  @retval EFI_OUT_OF_RESOURCES    Failed to allocate memory.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+AmlCodeGenReturn (
+  IN  AML_NODE_HEADER     * ReturnNode,
+  IN  AML_NODE_HEADER     * ParentNode,     OPTIONAL
+  OUT AML_OBJECT_NODE    ** NewObjectNode   OPTIONAL
+  )
+{
+  EFI_STATUS        Status;
+  AML_OBJECT_NODE * ObjectNode;
+
+  if ((ReturnNode == NULL)  ||
+      ((ParentNode == NULL) && (NewObjectNode == NULL))) {
+    ASSERT (0);
+    return EFI_INVALID_PARAMETER;
+  }
+
+  ObjectNode = NULL;
+
+  Status = AmlCreateObjectNode (
+             AmlGetByteEncodingByOpCode (AML_RETURN_OP, 0),
+             0,
+             &ObjectNode
+             );
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    goto error_handler;
+  }
+
+  Status = AmlSetFixedArgument (
+             ObjectNode,
+             EAmlParseIndexTerm0,
+             (AML_NODE_HEADER*)ReturnNode
+             );
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    goto error_handler;
+  }
+
+  ReturnNode = NULL;
+
+  Status = LinkNode (
+             ObjectNode,
+             ParentNode,
+             NewObjectNode
+             );
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    goto error_handler;
+  }
+
+  return Status;
+
+error_handler:
+  AmlDeleteTree (ReturnNode);
[SAMI] Add a check to see if ReturnNode is NULL before deleting.
+  if (ObjectNode != NULL) {
+    AmlDeleteTree ((AML_NODE_HEADER*)ObjectNode);
+  }
+  return Status;
+}
+
+/** AML code generation for a Return object node,
+    returning the object as an input NameString.
+
+  AmlCodeGenReturn ("NAM1", ParentNode, NewObjectNode) is
+  equivalent of the following ASL code:
+    Return(NAM1)
+
+  The ACPI 6.3 specification, s20.2.5.3 "Type 1 Opcodes Encoding" states:
+    DefReturn := ReturnOp ArgObject
+    ReturnOp := 0xA4
+    ArgObject := TermArg => DataRefObject
+
+  Thus, the ReturnNode must be evaluated as a DataRefObject. It can
+  be a NameString referencing an object. As this CodeGen Api doesn't
+  do semantic checking, it is strongly advised to check the AML bytecode
+  generated by this function against an ASL compiler.
+
+  The ReturnNode must be generated inside a Method body scope.
+
+  @param [in]  NameString     The object referenced by this NameString
+                              is returned by the Return ASL statement.
+                              Must be a NULL-terminated ASL NameString
+                              e.g.: "NAM1", "_SB.NAM1", etc.
+                              The input string is copied.
+  @param [in]  ParentNode     If provided, set ParentNode as the parent
+                              of the node created.
[SAMI] Same comment as above. 
+  @param [out] NewObjectNode  If success, contains the created node.
+
+  @retval EFI_SUCCESS             Success.
+  @retval EFI_INVALID_PARAMETER   Invalid parameter.
+  @retval EFI_OUT_OF_RESOURCES    Failed to allocate memory.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+AmlCodeGenReturnNameString (
+  IN  CONST CHAR8               * NameString,
+  IN        AML_NODE_HEADER     * ParentNode,     OPTIONAL
+  OUT       AML_OBJECT_NODE    ** NewObjectNode   OPTIONAL
+  )
+{
+  EFI_STATUS          Status;
+  AML_DATA_NODE     * DataNode;
+  CHAR8             * AmlNameString;
+  UINT32              AmlNameStringSize;
+
+  DataNode = NULL;
+
+  Status = ConvertAslNameToAmlName (NameString, &AmlNameString);
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    return Status;
+  }
+
+  Status = AmlGetNameStringSize (AmlNameString, &AmlNameStringSize);
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    goto exit_handler;
+  }
+
+  Status = AmlCreateDataNode (
+             EAmlNodeDataTypeNameString,
+             (UINT8*)AmlNameString,
+             AmlNameStringSize,
+             &DataNode
+             );
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    goto exit_handler;
+  }
+
+  // AmlCodeGenReturn() deletes DataNode if error.
+  Status = AmlCodeGenReturn (
+             (AML_NODE_HEADER*)DataNode,
+             ParentNode,
+             NewObjectNode
+             );
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+  }
[SAMI] Can we replace this with ASSERT_EFI_ERROR()? 
+
+exit_handler:
+  if (AmlNameString != NULL) {
+    FreePool (AmlNameString);
+  }
+  return Status;
+}

--------------03AFF6FF25EE369BC5195D8C--