From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from EUR03-DBA-obe.outbound.protection.outlook.com (EUR03-DBA-obe.outbound.protection.outlook.com [40.107.104.88])
 by mx.groups.io with SMTP id smtpd.web09.66.1660556959977149833
 for <devel@edk2.groups.io>;
 Mon, 15 Aug 2022 02:49:20 -0700
Authentication-Results: mx.groups.io;
 dkim=fail reason="body hash did not verify" header.i=@armh.onmicrosoft.com header.s=selector2-armh-onmicrosoft-com header.b=t2bFgp2+;
 spf=pass (domain: arm.com, ip: 40.107.104.88, mailfrom: sami.mujawar@arm.com)
ARC-Seal: i=2; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=pass;
 b=kd+70P31HuNjthO0Ll/3AyaxkswAGIf7d0S8JTsUFu1Ftnn9aET7ukDrCYrnCG7iaqnAuhlngATCPGMPaxCmMWI59X+mu9pisCnS4KdPjfJZL7ildvs4WfXccCGsLpWzs3qJgLMipeLSbuhztcQRdp12HKlArU7wSNWL+ouDz0XJogSTVVfQtdta4Bv674/KGUES9JWCr9oIlo7zps1Ah4iCZq6+yp+XI9hlajamS5+ZY8z74VCYkmSO5mVmiMAdeAg8hPdrOD/ts1ZlLCHWWCRjzUjn316jVBltdHbHrQczSsToxTXhJLJ6dmm0CbSqvI62ilBtI5DlTHJeeYlDkw==
ARC-Message-Signature: i=2; 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=ma8j+2zp6E9fnSUsjv9Hm5geEThn2n1TplIgagvXSX8=;
 b=CcKn0/9xJ7w3z38MTToXeomjha0BjhLMOHevWxiZJjgnNBByRWjbeFznY1vsMEUNtSHaqht58np4JncioBUxIb9hZv9WYUELQb+4OuwdEhZADfZczGBmW7+Clu74SBK98kus4u5jAK7pnWN4z0dIYeRp/BCPnE14yCw0UeYe03u0Jcum1sBe/7oOqFrxMycc0iqZjtiHFdQ9ZfHZuyI9L3ub85dOJk+fR8U0OEqvKj/Z0yhNsAowRz5gKAHArAUmylutJKYx1yHHXFDNvGPIjRXcCJimvccz33xRPhSQsMwrh38oJyYTEhI4ZatOxLX3vQFozWSo5ABQXcEnC0y10A==
ARC-Authentication-Results: i=2; mx.microsoft.com 1; spf=pass (sender ip is
 63.35.35.123) smtp.rcpttodomain=edk2.groups.io smtp.mailfrom=arm.com;
 dmarc=pass (p=none sp=none pct=100) action=none header.from=arm.com;
 dkim=pass (signature was verified) header.d=armh.onmicrosoft.com; arc=pass (0
 oda=1 ltdi=1 spf=[1,1,smtp.mailfrom=arm.com] dkim=[1,1,header.d=arm.com]
 dmarc=[1,1,header.from=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=ma8j+2zp6E9fnSUsjv9Hm5geEThn2n1TplIgagvXSX8=;
 b=t2bFgp2+hPAS5X7EsyRtCdsV9as4dN/AR6d0E6UqBuUj3qW0YbXRonsXVUSy1zkBqt9qPTVeTq5G7FY9pZpqcyVCnGVNkXvhW3DKqr7amdcfKsYL051jy2Gg/M1ML/yhx6IMA67A5WAnuCSS9g3Ls+xep5gY2v1dWT5uqZ5xDuk=
Received: from AM6PR10CA0041.EURPRD10.PROD.OUTLOOK.COM (2603:10a6:209:80::18)
 by DB9PR08MB7676.eurprd08.prod.outlook.com (2603:10a6:10:37f::7) with
 Microsoft SMTP Server (version=TLS1_2,
 cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5525.10; Mon, 15 Aug
 2022 09:49:16 +0000
Received: from AM7EUR03FT027.eop-EUR03.prod.protection.outlook.com
 (2603:10a6:209:80:cafe::59) by AM6PR10CA0041.outlook.office365.com
 (2603:10a6:209:80::18) with Microsoft SMTP Server (version=TLS1_2,
 cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5525.10 via Frontend
 Transport; Mon, 15 Aug 2022 09:49:16 +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;
 pr=C
Received: from 64aa7808-outbound-1.mta.getcheckrecipient.com (63.35.35.123) by
 AM7EUR03FT027.mail.protection.outlook.com (100.127.140.124) with Microsoft
 SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id
 15.20.5525.11 via Frontend Transport; Mon, 15 Aug 2022 09:49:16 +0000
Received: ("Tessian outbound 73dd6a25223d:v123"); Mon, 15 Aug 2022 09:49:16 +0000
X-CheckRecipientChecked: true
X-CR-MTA-CID: f2427d781ec0e21c
X-CR-MTA-TID: 64aa7808
Received: from ab06d943c322.2
	by 64aa7808-outbound-1.mta.getcheckrecipient.com id AB19DC5F-C8DF-4EDA-8F72-A58B989974D6.1;
	Mon, 15 Aug 2022 09:49:09 +0000
Received: from EUR05-VI1-obe.outbound.protection.outlook.com
    by 64aa7808-outbound-1.mta.getcheckrecipient.com with ESMTPS id ab06d943c322.2
    (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384);
    Mon, 15 Aug 2022 09:49:09 +0000
ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none;
 b=bEeBSR+141MhrExZx/t5nxNTc2eGiM6YDJtq6AvInkBP3zVSSC/vVmpo7kkYFHUeYmdyB+3NgNfVz0aUoiT3IKeKWaGusGXtFfZogHjD9jFh+iVu2avAf5G3gieQfwmBeajeHJ4/dJYmfOHZpA+VoOY/ZwOXIzyGWajeufX58bS/vsa06r0jd2rHwyZqLQVn6r8WwY1RAP5bYtKocbV9g9k07L7ouBmzXQKRPW6EVDrci8s4tAUdZ9Lq+nT7tXT/5eN99MMZUbhMUpPl/P9BvbAW+aoehFsfOQ2VUVozjTIytyonNBsL1uWrvpBY1N03dKOjhvywF+Pyz+HUx9FR/A==
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=ma8j+2zp6E9fnSUsjv9Hm5geEThn2n1TplIgagvXSX8=;
 b=guK+98h/N8gkSVhzcdumlFotBN8bYttdd4yEkROZAxWHtbxhnvWv4iN3JcqRyjwGUCNE6qv4Um+Pt60SeRY7MCuF/Ig7nQUQlzqRTNGlAPQe5K35XVqAXmgq6KC4eQc3xI6JkyONxw8awyxahFIYiNZPrRJu3ADvkenm3sKoubx9ziKCtbtFvaSSeiGoGCTEVZJ9M/H5MgbG5JBUQ91gs/C1GdpM7A3iGWekrJCWAFe0vgVcGU+r+c5vbB/pr6kJMd3zDMoM1Jvc7SCGskOdCzi9gwM/AOQeB0OjcXfFbZTHcnmPzYPbI3rV0uwpBO3z+igRFCsFX+/PoEawNY38yw==
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=ma8j+2zp6E9fnSUsjv9Hm5geEThn2n1TplIgagvXSX8=;
 b=t2bFgp2+hPAS5X7EsyRtCdsV9as4dN/AR6d0E6UqBuUj3qW0YbXRonsXVUSy1zkBqt9qPTVeTq5G7FY9pZpqcyVCnGVNkXvhW3DKqr7amdcfKsYL051jy2Gg/M1ML/yhx6IMA67A5WAnuCSS9g3Ls+xep5gY2v1dWT5uqZ5xDuk=
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 AM5PR0802MB2580.eurprd08.prod.outlook.com (2603:10a6:203:a2::12) with
 Microsoft SMTP Server (version=TLS1_2,
 cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5525.10; Mon, 15 Aug
 2022 09:49:07 +0000
Received: from AS8PR08MB6806.eurprd08.prod.outlook.com
 ([fe80::d562:5a52:f638:7fe9]) by AS8PR08MB6806.eurprd08.prod.outlook.com
 ([fe80::d562:5a52:f638:7fe9%6]) with mapi id 15.20.5504.027; Mon, 15 Aug 2022
 09:49:07 +0000
Message-ID: <183343f3-7535-567f-0ed0-eb027e8af995@arm.com>
Date: Mon, 15 Aug 2022 10:49:05 +0100
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.11.0
Subject: Re: [edk2-devel] [PATCH] DynamicTablesPkg: Add support to build _DSD
To: Pierre Gondois <pierre.gondois@arm.com>, devel@edk2.groups.io,
 jbrasen@nvidia.com
CC: Alexei.Fedorov@arm.com, "nd@arm.com" <nd@arm.com>
References: <b535d53ecaf06aac3fa72f9c7bdef61996fafa86.1659712330.git.jbrasen@nvidia.com>
 <195c52bc-2aba-6592-c452-e1ff35e444fa@arm.com>
From: "Sami Mujawar" <sami.mujawar@arm.com>
In-Reply-To: <195c52bc-2aba-6592-c452-e1ff35e444fa@arm.com>
X-ClientProxiedBy: LO4P123CA0267.GBRP123.PROD.OUTLOOK.COM
 (2603:10a6:600:194::20) To AS8PR08MB6806.eurprd08.prod.outlook.com
 (2603:10a6:20b:39b::12)
MIME-Version: 1.0
X-MS-Office365-Filtering-Correlation-Id: f7cf8a48-62b0-4924-c3d5-08da7ea36ad2
X-MS-TrafficTypeDiagnostic: 
	AM5PR0802MB2580:EE_|AM7EUR03FT027:EE_|DB9PR08MB7676:EE_
X-LD-Processed: f34e5979-57d9-4aaa-ad4d-b122a662184d,ExtAddr
x-checkrecipientrouted: true
NoDisclaimer: true
X-MS-Exchange-SenderADCheck: 1
X-MS-Exchange-AntiSpam-Relay: 0
X-Microsoft-Antispam-Untrusted: BCL:0;
X-Microsoft-Antispam-Message-Info-Original: phA1FugMiDkVOzPTkTyP7Ahw0LVH88kdUwc8UHTIgLz0n+/p4yLvEVPaV0QqZrMBcf3A2cAqDXkfL4VPsXJJQI8L+njFtLr4DJh1B+snjtxJd/xHhK72NJqHTKvsSBEHvg+ppIscyr2eueJtgmZYxSEou8uFVu3bSN1quWLV0JmNRZ4KzfhH33MfPHRrrGkqQtKmSyIAoVoKx2L5ud7Z9yR3dLJFRAjbEFvzhqwq5f1cSrMekcNJBNDpnCWpPP9qp9Bm6sL95j+Xdy/ql6K3T51i1djJTQqcM90CNzNyj3jV6JWIG3xxSATkEOTeyiwx+gCJH0Dvn+UksFw+6P8ar0mCWgH2C8XSG/c5gl90nlD6RHhM3SOL1L+d+fetcoVcDg0siKJprPUcCX3PCuseLmwyNf6Z5R9eiLxFZgR6+N30RRy8D3a5Z9Ws5WT6hJdi3gR+nSxbq/DKJ2mahxNdcAUym7gjotuvMv0wGFX+5TaDaI0Lm7tZLtm0IeW+bjaL+s/swODKidwJoSJ6kpPwoz0nlzcPDT6mqmAZCdWxhxrEYi4V6F9x4begwy1OgQl5+jCmk/Y9qw4JT7AQvtfALnQJM/rMDxJLVaSJvwh9rOGZxx7B55Lpo4m0i9SP8dnyEyHL1m05VO6juoawWyOs42DhLXIba+MkHgbybtScplr9jLffl0zxz9FGuB66+gFZCF4Ln129FstY2CPcznpZQlXQKjwaUmfN6tb4qJSJRDT8+dRgPFAq/NKDPgJc17LDXUrbNIsjmZguMS54YLdSipeCyhrF9E7+e7VQIsSt4X/iRhVSkgBCrEdyq1GiojkmcdVnNKB+4BG7TazXYjmuZQ==
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:(13230016)(4636009)(376002)(396003)(39860400002)(366004)(346002)(136003)(6486002)(36756003)(6506007)(186003)(86362001)(31696002)(41300700001)(83380400001)(478600001)(2616005)(26005)(6512007)(53546011)(66946007)(30864003)(19627235002)(316002)(66476007)(8676002)(66556008)(4326008)(5660300002)(31686004)(8936002)(44832011)(38100700002)(2906002)(45980500001)(43740500002);DIR:OUT;SFP:1101;
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM5PR0802MB2580
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: AM7EUR03FT027.eop-EUR03.prod.protection.outlook.com
X-MS-PublicTrafficType: Email
X-MS-Office365-Filtering-Correlation-Id-Prvs: 
	726068ce-9c24-4e23-07e0-08da7ea36533
X-Microsoft-Antispam: BCL:0;
X-Microsoft-Antispam-Message-Info: 
	TNLQkeyBe3nFlt93FMD+Zgjh6iTnrkJ9uTHprrX/85IjETSKBSEYveJHDj2NvCylYGwFwN5kdytMjlWAjvZAZVDJcXsODyRIU2KielNjm7BTVktRr7aM3Ox2t76Vgpz8wTlohpGcQIYKMfB0LKxk4nFDMAV4v/ZsZrrzD2X1B/IFqXd8eKt6yQoE8gnsTd3YuLPkDdTOuNQpuy7MIHXYQtO/+S8pq+ii0Z7+opyrlfM4074o0jvDdz+leywjkhbE7rg1tN6FUmcey0Zw9LNCE20wTXgyML/Q4uFrXfwZh29Cy0w79LbvV3eE/SLoUo2LSv/AAH0/r+xD6GR5yVv0QhgGYcPYsOVQwiwoX0S0nT6UPXz93R3U/eWsvolm+MHp6c6DOWW2sw9D9c1UBt6vBN7C3XDXP8tmXIActpsuLxkPDU22wZpWlxDLBHij/QqQSyuI2XWwvTU3dQ/371nzaB2Vfl/UM0uc07Y/HIqNAUx0LWd0rxDou07oNL3a6idZyXjE4psScMe9pGLi5uk1YRulP+KHXRH93kh8/6vBh9WiLY5E+hzm4Kjvmzg436k+TUSlXDYU54TFfJ9IWvjV9vUqbx0qI9HHxPqER0YG4tNztFNSiYaOGbmPLDK3HVhPfCVSX8dQWZ81C/mqKj8Wz78rgmbmUrsvPc/P8e2v6C88l/CEjA0GFitj4snT4ntPiaoNe4gH0xHPCHxik9jKCvwuzrjVgaB1UkJ3+4dd+9AlwBnhTTkri5GKBboUQHaZAl/wmettmyLd+De9BVbPVp5R1VoMlmZn2HJaTFNAaqykBrugHuNE14RA4ZdI1YrF2QizjdbBg3tL2qoGBYDdOA==
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:(13230016)(4636009)(39860400002)(136003)(376002)(346002)(396003)(46966006)(36840700001)(40470700004)(47076005)(53546011)(316002)(82740400003)(8676002)(4326008)(83380400001)(478600001)(36756003)(31686004)(19627235002)(70206006)(6486002)(70586007)(356005)(2906002)(26005)(40480700001)(31696002)(44832011)(81166007)(30864003)(86362001)(82310400005)(40460700003)(5660300002)(8936002)(6512007)(6506007)(2616005)(336012)(41300700001)(36860700001)(186003)(43740500002);DIR:OUT;SFP:1101;
X-OriginatorOrg: arm.com
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Aug 2022 09:49:16.2531
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: f7cf8a48-62b0-4924-c3d5-08da7ea36ad2
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: 
	AM7EUR03FT027.eop-EUR03.prod.protection.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Anonymous
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB9PR08MB7676
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: quoted-printable

Hi Jeff, Pierre,

Please findmy response inline marked [SAMI].

Regards,

Sami Mujawar

On 12/08/2022 02:36 pm, Pierre Gondois wrote:
> Hi Jeff,
> Please find some inline comments below:
>
> On 8/5/22 17:14, Jeff Brasen via groups.io wrote:
>> Add APIs needed to build _DSD with different UUIDs.
>>
>> This is per ACPI specification 6.4 s6.2.5.
>>
>>
>>
>> Adds support for building data packages with format
>>
>> Package {"Name", Integer}
>>
>>
>>
>> Signed-off-by: Jeff Brasen <jbrasen@nvidia.com>
>>
>> ---
>>
>> =C2=A0 .../Include/Library/AmlLib/AmlLib.h=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 |=C2=A0 50 ++++
>>
>> =C2=A0 .../Common/AmlLib/CodeGen/AmlCodeGen.c=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0 | 236 ++++++++++++++++++
>>
>> =C2=A0 2 files changed, 286 insertions(+)
>>
>>
>>
>> diff --git a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h=20
>> b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
>>
>> index 6f214c0dfa..30cab3f6bb 100644
>>
>> --- a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
>>
>> +++ b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
>>
>> @@ -1280,6 +1280,56 @@ AmlAddLpiState (
>>
>> =C2=A0=C2=A0=C2=A0 IN=C2=A0 AML_OBJECT_NODE_HANDLE=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0 LpiNode
>>
>> =C2=A0=C2=A0=C2=A0 );
>>
>>
>> +/** AML code generation for a _DSD device data object.
>>
>> +
>>
>> +=C2=A0 AmlAddDeviceDataDescriptorPackage (Uuid, ParentNode,=20
>> NewObjectNode) is
>>
>> +=C2=A0 equivalent of the following ASL code:
>>
>> +=C2=A0=C2=A0=C2=A0 ToUUID(Uuid),
>>
>> +=C2=A0=C2=A0=C2=A0 Package () {}
>>
>> +
>>
>> +=C2=A0 @ingroup CodeGenApis
>>
>> +
>>
>> +=C2=A0 @param [in]=C2=A0 Uuid=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0 The Uuid of the descriptor to be created
>>
>> +=C2=A0 @param [in]=C2=A0 DsdNode=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0 Node of the DSD Package.
>>
>> +=C2=A0 @param [out] PackageNode=C2=A0=C2=A0=C2=A0 If success, contains =
the created=20
>> package node.
>>
>> +
>>
>> +=C2=A0 @retval EFI_SUCCESS=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0 Success.
>>
>> +=C2=A0 @retval EFI_INVALID_PARAMETER=C2=A0=C2=A0 Invalid parameter.
>>
>> +=C2=A0 @retval EFI_OUT_OF_RESOURCES=C2=A0=C2=A0=C2=A0 Failed to allocat=
e memory.
>>
>> +**/
>>
>> +EFI_STATUS
>>
>> +EFIAPI
>>
>> +AmlAddDeviceDataDescriptorPackage (
>>
>> +=C2=A0 IN=C2=A0 CONST EFI_GUID=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 *Uuid,
>>
>> +=C2=A0 IN=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 AML_OBJECT_NODE_HAN=
DLE=C2=A0 DsdNode,
>>
>> +=C2=A0 OUT=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 AML_OBJECT_NODE_HANDLE=
=C2=A0 *PackageNode
>>
>> +=C2=A0 );
>>
>> +
>>
>> +/** AML code generation to add a package with a name and value,
>>
>> + *=C2=A0 to a parent package> +
>>
>> +=C2=A0 AmlAddNameValuePackage ("Name", Value, ParentNode) is
>>
>> +=C2=A0 equivalent of the following ASL code:
>>
>> +=C2=A0=C2=A0=C2=A0 Package (2) {"Name", Value}
>>
>> +
>>
>> +=C2=A0 @ingroup CodeGenApis
>>
>> +
>>
>> +=C2=A0 @param [in]=C2=A0 Name=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0 String to place in first entry of package
>>
>> +=C2=A0 @param [in]=C2=A0 Value=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0 Integer to place in second entry of=20
>> package
>>
>> +=C2=A0 @param [in]=C2=A0 PackageNode=C2=A0=C2=A0=C2=A0 Package to add n=
ew sub package to.
>>
>> +
>>
>> +=C2=A0 @retval EFI_SUCCESS=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0 Success.
>>
>> +=C2=A0 @retval EFI_INVALID_PARAMETER=C2=A0=C2=A0 Invalid parameter.
>>
>> +=C2=A0 @retval EFI_OUT_OF_RESOURCES=C2=A0=C2=A0=C2=A0 Failed to allocat=
e memory.
>>
>> +**/
>>
>> +EFI_STATUS
>>
>> +EFIAPI
>>
>> +AmlAddNameIntegerPackage (
>>
>> +=C2=A0 IN CHAR8=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 *Name,
>>
>> +=C2=A0 IN UINT64=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 Value,
>>
>> +=C2=A0 IN AML_OBJECT_NODE_HANDLE=C2=A0 PackageNode
>>
>> +=C2=A0 );
>>
>> +
>>
>> =C2=A0 // DEPRECATED APIS
>>
>> =C2=A0 #ifndef DISABLE_NEW_DEPRECATED_INTERFACES
>>
>>
>> diff --git=20
>> a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c=20
>> b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
>>
>> index e51d2dd7f0..80e43d0e62 100644
>>
>> --- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
>>
>> +++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
>>
>> @@ -2600,3 +2600,239 @@ error_handler:
>>
>>
>> =C2=A0=C2=A0=C2=A0 return Status;
>>
>> =C2=A0 }
>>
>> +
>>
>> +/** AML code generation for a _DSD device data object.
>>
>> +
>>
>> +=C2=A0 AmlAddDeviceDataDescriptorPackage (Uuid, ParentNode,=20
>> NewObjectNode) is
>
> I think this is DsdNode and PackageNode (same for the header file).
>>
>> +=C2=A0 equivalent of the following ASL code:
>>
>> +=C2=A0=C2=A0=C2=A0 ToUUID(Uuid),
>>
>> +=C2=A0=C2=A0=C2=A0 Package () {}
>>
>> +
>>
>> +=C2=A0 @ingroup CodeGenApis
>>
>> +
>>
>> +=C2=A0 @param [in]=C2=A0 Uuid=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0 The Uuid of the descriptor to be created
>>
>> +=C2=A0 @param [in]=C2=A0 DsdNode=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0 Node of the DSD Package.
>>
>> +=C2=A0 @param [out] PackageNode=C2=A0=C2=A0=C2=A0 If success, contains =
the created=20
>> package node.
>>
>> +
>>
>> +=C2=A0 @retval EFI_SUCCESS=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0 Success.
>>
>> +=C2=A0 @retval EFI_INVALID_PARAMETER=C2=A0=C2=A0 Invalid parameter.
>>
>> +=C2=A0 @retval EFI_OUT_OF_RESOURCES=C2=A0=C2=A0=C2=A0 Failed to allocat=
e memory.
>>
>> +**/
>>
>> +EFI_STATUS
>>
>> +EFIAPI
>>
>> +AmlAddDeviceDataDescriptorPackage (
>>
>> +=C2=A0 IN=C2=A0 CONST EFI_GUID=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 *Uuid,
>>
>> +=C2=A0 IN=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 AML_OBJECT_NODE_HAN=
DLE=C2=A0 DsdNode,
>>
>> +=C2=A0 OUT=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 AML_OBJECT_NODE_HANDLE=
=C2=A0 *PackageNode
>>
>> +=C2=A0 )
>>
>> +{
>>
>> +=C2=A0 EFI_STATUS=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0 Status;
>>
>> +=C2=A0 AML_OBJECT_NODE=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 =
*UuidNode;
>>
>> +=C2=A0 AML_DATA_NODE=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0 *UuidDataNode;
>>
>> +=C2=A0 AML_OBJECT_NODE_HANDLE=C2=A0 DsdEntryList;
>>
>> +
>>
>> +=C2=A0 if ((Uuid =3D=3D NULL)=C2=A0=C2=A0=C2=A0=C2=A0 ||
>>
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 (PackageNode =3D=3D NULL) ||
>>
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 (AmlGetNodeType ((AML_NODE_HANDLE)DsdNod=
e) !=3D EAmlNodeObject) ||
>>
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 (!AmlNodeHasOpCode (DsdNode, AML_NAME_OP=
, 0))=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0 ||
>>
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 !AmlNameOpCompareName (DsdNode, "_DSD"))
>>
>> +=C2=A0 {
>>
>> +=C2=A0=C2=A0=C2=A0 ASSERT (0);
>
> This is not consistent in the file, but it could be:
> =C2=A0 ASSERT_EFI_ERROR (ASSERT_EFI_ERROR)
[SAMI] I am fine with using either ASSERTxxx macros. The only think that=20
needs to be taken care of is that the errors are handled in release=20
builds (as the ASSERTxxx macros will vanish in release builds).
>
>>
>> +=C2=A0=C2=A0=C2=A0 return EFI_INVALID_PARAMETER;
>>
>> +=C2=A0 }
>>
>> +
>>
>> +=C2=A0 // Get the Package object node of the _DSD node,
>>
>> +=C2=A0 // which is the 2nd fixed argument (i.e. index 1).
>>
>> +=C2=A0 DsdEntryList =3D (AML_OBJECT_NODE_HANDLE)AmlGetFixedArgument (
>>
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 DsdNode,
>>
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 EAmlParseIndexTerm1
>>
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 );
>>
>> +=C2=A0 if ((DsdEntryList =3D=3D=20
>> NULL)=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 ||
>>
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 (AmlGetNodeType ((AML_NODE_HANDLE)DsdEnt=
ryList) !=3D=20
>> EAmlNodeObject)=C2=A0 ||
>>
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 (!AmlNodeHasOpCode (DsdEntryList, AML_PA=
CKAGE_OP, 0)))
>>
>> +=C2=A0 {
>>
>> +=C2=A0=C2=A0=C2=A0 ASSERT (0);
>>
>> +=C2=A0=C2=A0=C2=A0 return EFI_INVALID_PARAMETER;
>>
>> +=C2=A0 }
>>
>> +
>>
>> +=C2=A0 *PackageNode =3D NULL;
>>
>> +=C2=A0 UuidNode=C2=A0=C2=A0=C2=A0=C2=A0 =3D NULL;
>
> As UuidNode is the first node allocated, we either fail and return
> (no need to free it), or succeed and the error handler should
> free it. So it is not necessary to initialize it to NULL.
> However UuidDataNode should be initialized to NULL.
>
>>
>> +
>>
>> +=C2=A0 Status =3D AmlCodeGenBuffer (NULL, 0, &UuidNode);
>>
>> +=C2=A0 if (EFI_ERROR (Status)) {
>>
>> +=C2=A0=C2=A0=C2=A0 ASSERT (0);
>>
>> +=C2=A0=C2=A0=C2=A0 goto error_handler;
>
> If it failed here then no allocation happened yet,
> so it could just be a return.
>
>>
>> +=C2=A0 }
>>
>> +
>>
>> +=C2=A0 Status =3D AmlCreateDataNode (
>>
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0 EAmlNodeDataTypeRaw,
>>
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0 (CONST UINT8 *)Uuid,
>>
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0 sizeof (EFI_GUID),
>>
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0 &UuidDataNode
>>
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0 );
>>
>> +=C2=A0 if (EFI_ERROR (Status)) {
>>
>> +=C2=A0=C2=A0=C2=A0 ASSERT (0);
>>
>> +=C2=A0=C2=A0=C2=A0 goto error_handler;
>>
>> +=C2=A0 }
>
> For the record, I tested the function and the Uuid is correctly=20
> generated.
> For Sami: would it be good to have a ToUUID() function as in ASL ?
[SAMI] Yes, that could be useful.
>
>>
>> +
>>
>> +=C2=A0 Status =3D AmlVarListAddTail (
>>
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0 (AML_NODE_HEADER *)UuidNode,
>>
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0 (AML_NODE_HEADER *)UuidDataNode
>>
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0 );
>>
>> +=C2=A0 if (EFI_ERROR (Status)) {
>>
>> +=C2=A0=C2=A0=C2=A0 ASSERT (0);
>>
>> +=C2=A0=C2=A0=C2=A0 goto error_handler;
>>
>> +=C2=A0 }
>>
>> +=C2=A0 UuidDataNode =3D NULL;
>>
>> +
>>
>> +=C2=A0 // Append to the the list of _DSD entries.
>>
>> +=C2=A0 Status =3D AmlVarListAddTail (
>>
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0 (AML_NODE_HANDLE)DsdEntryList,
>>
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0 (AML_NODE_HANDLE)UuidNode
>>
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0 );
>>
>> +=C2=A0 if (EFI_ERROR (Status)) {
>>
>> +=C2=A0=C2=A0=C2=A0 ASSERT (0);
>>
>> +=C2=A0=C2=A0=C2=A0 goto error_handler;
>>
>> +=C2=A0 }
>>
>> +
>>
>> +=C2=A0 Status =3D AmlCodeGenPackage (PackageNode);
>>
>> +=C2=A0 if (EFI_ERROR (Status)) {
>>
>> +=C2=A0=C2=A0=C2=A0 ASSERT (0);
>>
>> +=C2=A0=C2=A0=C2=A0 goto error_handler;
>>
>> +=C2=A0 }
>>
>> +
>>
>> +=C2=A0 // Append to the the list of _DSD entries.
>
> duplicated 'the' here and above
>
>>
>> +=C2=A0 Status =3D AmlVarListAddTail (
>>
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0 (AML_NODE_HANDLE)DsdEntryList,
>>
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0 (AML_NODE_HANDLE)*PackageNode
>>
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0 );
>>
>> +=C2=A0 if (EFI_ERROR (Status)) {
>>
>> +=C2=A0=C2=A0=C2=A0 ASSERT (0);
>>
>> +=C2=A0=C2=A0=C2=A0 goto error_handler;
>>
>> +=C2=A0 }
>>
>> +
>>
>> +=C2=A0 return Status;
>>
>> +
>>
>> +error_handler:
>>
>> +=C2=A0 if (UuidNode !=3D NULL) {
>>
>> +=C2=A0=C2=A0=C2=A0 //Need to check to detach as there is an error path =
that have
>>
>> +=C2=A0=C2=A0=C2=A0 //this attached to the DsdEntry
>>
>> +=C2=A0=C2=A0=C2=A0 if (AML_NODE_HAS_PARENT (UuidNode)) {
>
> Would it be possible to have a second error_handler instead ?
> Ideally AmlCodeGen.c should not be able to see the internal node
> structure.
> With the second error handler, it should look like:
>
> =C2=A0 Status =3D AmlCodeGenPackage (PackageNode);
> =C2=A0 if (EFI_ERROR (Status)) {
> =C2=A0=C2=A0=C2=A0 ASSERT (0);
> =C2=A0=C2=A0=C2=A0 goto error_handler1;
> =C2=A0 }
>
> =C2=A0 // Append to the the list of _DSD entries.
> =C2=A0 Status =3D AmlVarListAddTail (
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 =
(AML_NODE_HANDLE)DsdEntryList,
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 =
(AML_NODE_HANDLE)*PackageNode
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 =
);
> =C2=A0 if (EFI_ERROR (Status)) {
> =C2=A0=C2=A0=C2=A0 ASSERT (0);
> =C2=A0=C2=A0=C2=A0 goto error_handler1;
> =C2=A0 }
>
> =C2=A0 return Status;
>
> error_handler1:
> =C2=A0 AmlDetachNode ((AML_NODE_HANDLE)UuidNode);
>
> error_handler:
> =C2=A0 if (UuidNode !=3D NULL) {
> =C2=A0=C2=A0=C2=A0 AmlDeleteTree ((AML_NODE_HANDLE)UuidNode);
> =C2=A0 }
> ...
>
>
>>
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 AmlDetachNode ((AML_NODE_HANDLE)UuidNode=
);
>>
>> +=C2=A0=C2=A0=C2=A0 }
>>
>> +=C2=A0=C2=A0=C2=A0 AmlDeleteTree ((AML_NODE_HANDLE)UuidNode);
>>
>> +=C2=A0 }
>>
>> +
>>
>> +=C2=A0 if (*PackageNode !=3D NULL) {
>>
>> +=C2=A0=C2=A0=C2=A0 AmlDeleteTree ((AML_NODE_HANDLE)*PackageNode);
>>
>> +=C2=A0=C2=A0=C2=A0 *PackageNode =3D NULL;
>>
>> +=C2=A0 }
>>
>> +=C2=A0 if (UuidDataNode !=3D NULL) {
>>
>> +=C2=A0=C2=A0=C2=A0 AmlDeleteTree ((AML_NODE_HANDLE)UuidDataNode);
>>
>> +=C2=A0 }
>>
>> +
>>
>> +=C2=A0 return Status;
>>
>> +}
>>
>> +
>>
>> +/** AML code generation to add a package with a name and value,
>>
>> + *=C2=A0 to a parent package
>
> Is it possible to remove the '*' at the beginning of the line
> (same for the prototype) and also add a reference to
> s6.2.5 _DSD (Device Specific Data) in the two new function
> descriptions ?
>
>>
>> +
>>
>> +=C2=A0 AmlAddNameValuePackage ("Name", Value, ParentNode) is
>>
>> +=C2=A0 equivalent of the following ASL code:
>>
>> +=C2=A0=C2=A0=C2=A0 Package (2) {"Name", Value}
>>
>> +
>>
>> +=C2=A0 @ingroup CodeGenApis
>>
>> +
>>
>> +=C2=A0 @param [in]=C2=A0 Name=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0 String to place in first entry of package
>>
>> +=C2=A0 @param [in]=C2=A0 Value=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0 Integer to place in second entry of=20
>> package
>>
>> +=C2=A0 @param [in]=C2=A0 PackageNode=C2=A0=C2=A0=C2=A0 Package to add n=
ew sub package to.
>>
>> +
>>
>> +=C2=A0 @retval EFI_SUCCESS=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0 Success.
>>
>> +=C2=A0 @retval EFI_INVALID_PARAMETER=C2=A0=C2=A0 Invalid parameter.
>>
>> +=C2=A0 @retval EFI_OUT_OF_RESOURCES=C2=A0=C2=A0=C2=A0 Failed to allocat=
e memory.
>>
>> +**/
>>
>> +EFI_STATUS
>>
>> +EFIAPI
>>
>> +AmlAddNameIntegerPackage (
>
> To regroup the functions related to _DSD objects,
> would it be better to name this function:
> =C2=A0 AmlAddDsdIntegerProperty()
> and the function above:
> =C2=A0 AmlAddDsdDescriptor()
> (Sami/Jeff this is an open question)

[SAMI] Is there any other usecase where 'Package (2) {"Name", Value}'=20
may be required?

I think AmlAddNameIntegerPackage() is generic and could be useful=20
elsewhere. Would defining AmlAddDsdIntegerProperty as a macro that maps=20
to AmlAddNameIntegerPackage() be an alternative?

We could then use the macro in the DSD specific code.

[/SAMI]
>>
>> +=C2=A0 IN CHAR8=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 *Name,
>>
>> +=C2=A0 IN UINT64=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 Value,
>>
>> +=C2=A0 IN AML_OBJECT_NODE_HANDLE=C2=A0 PackageNode
>>
>> +=C2=A0 )
>>
>> +{
>>
>> +=C2=A0 EFI_STATUS=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 Status;
>>
>> +=C2=A0 AML_OBJECT_NODE=C2=A0 *NameNode;
>>
>> +=C2=A0 AML_OBJECT_NODE=C2=A0 *ValueNode;
>>
>> +=C2=A0 AML_OBJECT_NODE=C2=A0 *NewPackageNode;
>>
>> +
>>
>> +=C2=A0 if ((Name =3D=3D NULL) ||
>>
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 (AmlGetNodeType ((AML_NODE_HANDLE)Packag=
eNode) !=3D=20
>> EAmlNodeObject) ||
>>
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 (!AmlNodeHasOpCode (PackageNode, AML_PAC=
KAGE_OP, 0)))
>>
>> +=C2=A0 {
>>
>> +=C2=A0=C2=A0=C2=A0 ASSERT (0);
>>
>> +=C2=A0=C2=A0=C2=A0 return EFI_INVALID_PARAMETER;
>>
>> +=C2=A0 }
>>
>> +
>>
>> +=C2=A0 NewPackageNode =3D NULL;
>
> I think setting NewPackageNode to NULL is not necessary as:
> - if AmlCodeGenPackage() fails, we return without going to the error=20
> handler
> =C2=A0 and without trying to free it
> - if we fail later, NameNode contains a valid node which is freed in the
> =C2=A0 error handler
>
> However NameNode and ValueNode should be initialized to NULL as if
> AmlCodeGenString() fails, we go to the error handler and try to free
> non-initialized values.
>
>>
>> +=C2=A0 // The new package entry.
>>
>> +=C2=A0 Status =3D AmlCodeGenPackage (&NewPackageNode);
>>
>> +=C2=A0 if (EFI_ERROR (Status)) {
>>
>> +=C2=A0=C2=A0=C2=A0 ASSERT (0);
>
> Even though this is not consistent in the file,
> it could be ASSERT_EFI_ERROR (Status)
> (same comment for the other ASSERT).
[SAMI] I am fine with either ASSERT versions being used. But please=20
handle errors for release builds.
>
>>
>> +=C2=A0=C2=A0=C2=A0 return Status;
>>
>> +=C2=A0 }
>>
>> +
>>
>> +=C2=A0 Status =3D AmlCodeGenString (Name, &NameNode);
>>
>> +=C2=A0 if (EFI_ERROR (Status)) {
>>
>> +=C2=A0=C2=A0=C2=A0 ASSERT_EFI_ERROR (Status);
>>
>> +=C2=A0=C2=A0=C2=A0 goto error_handler;
>>
>> +=C2=A0 }
>>
>> +
>>
>> +=C2=A0 Status =3D AmlVarListAddTail (
>>
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0 (AML_NODE_HANDLE)NewPackageNode,
>>
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0 (AML_NODE_HANDLE)NameNode
>>
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0 );
>>
>> +=C2=A0 if (EFI_ERROR (Status)) {
>>
>> +=C2=A0=C2=A0=C2=A0 ASSERT (0);
>>
>> +=C2=A0=C2=A0=C2=A0 goto error_handler;
>>
>> +=C2=A0 }
>>
>> +=C2=A0 NameNode =3D NULL;
>>
>> +
>>
>> +=C2=A0 Status =3D AmlCodeGenInteger (Value, &ValueNode);
>>
>> +=C2=A0 if (EFI_ERROR (Status)) {
>>
>> +=C2=A0=C2=A0=C2=A0 ASSERT (0);
>>
>> +=C2=A0=C2=A0=C2=A0 goto error_handler;
>>
>> +=C2=A0 }
>>
>> +
>>
>> +=C2=A0 Status =3D AmlVarListAddTail (
>>
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0 (AML_NODE_HANDLE)NewPackageNode,
>>
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0 (AML_NODE_HANDLE)ValueNode
>>
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0 );
>>
>> +=C2=A0 if (EFI_ERROR (Status)) {
>>
>> +=C2=A0=C2=A0=C2=A0 ASSERT (0);
>>
>> +=C2=A0=C2=A0=C2=A0 goto error_handler;
>>
>> +=C2=A0 }
>>
>> +=C2=A0 ValueNode =3D NULL;
>>
>> +
>>
>> +=C2=A0 Status =3D AmlVarListAddTail (
>>
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0 (AML_NODE_HANDLE)PackageNode,
>>
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0 (AML_NODE_HANDLE)NewPackageNode
>>
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0 );
>>
>> +=C2=A0 if (EFI_ERROR (Status)) {
>>
>> +=C2=A0=C2=A0=C2=A0 ASSERT (0);
>>
>> +=C2=A0=C2=A0=C2=A0 goto error_handler;
>>
>> +=C2=A0 }
>>
>> +
>>
>> +=C2=A0 return Status;
>>
>> +
>>
>> +error_handler:
>>
>> +=C2=A0 if (NewPackageNode !=3D NULL) {
>>
>> +=C2=A0=C2=A0=C2=A0 AmlDeleteTree ((AML_NODE_HANDLE)NewPackageNode);
>>
>> +=C2=A0 }
>>
>> +=C2=A0 if (NameNode !=3D NULL) {
>>
>> +=C2=A0=C2=A0=C2=A0 AmlDeleteTree ((AML_NODE_HANDLE)NameNode);
>>
>> +=C2=A0 }
>>
>> +=C2=A0 if (ValueNode !=3D NULL) {
>>
>> +=C2=A0=C2=A0=C2=A0 AmlDeleteTree ((AML_NODE_HANDLE)ValueNode);
>>
>> +=C2=A0 }
>>
>> +
>>
>> +=C2=A0 return Status;
>>
>> +}
>>