From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from foss.arm.com (foss.arm.com [217.140.110.172])
 by mx.groups.io with SMTP id smtpd.web08.11829.1633527223693726766
 for <devel@edk2.groups.io>;
 Wed, 06 Oct 2021 06:33:44 -0700
Authentication-Results: mx.groups.io;
 dkim=missing; spf=pass (domain: arm.com, ip: 217.140.110.172, mailfrom: pierre.gondois@arm.com)
Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14])
	by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 88AB7ED1;
	Wed,  6 Oct 2021 06:33:41 -0700 (PDT)
Received: from [192.168.1.16] (unknown [172.31.20.19])
	by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPA id 04C9D3F66F;
	Wed,  6 Oct 2021 06:33:39 -0700 (PDT)
Subject: Re: [PATCH v1 09/13] DynamicTablesPkg: AML code generation for a Method returning a NS
To: Sami Mujawar <sami.mujawar@arm.com>, devel@edk2.groups.io,
 Alexei Fedorov <Alexei.Fedorov@arm.com>
Cc: Akanksha Jain <akanksha.jain2@arm.com>,
 Alexandru Elisei <alexandru.elisei@arm.com>, nd <nd@arm.com>
References: <20210623114039.24491-1-Pierre.Gondois@arm.com>
 <20210623114039.24491-10-Pierre.Gondois@arm.com>
 <75fcbce8-4d42-3677-afa8-6857fd01be19@arm.com>
From: "PierreGondois" <pierre.gondois@arm.com>
Message-ID: <dda9aa4b-5fc9-1823-fb6c-3137fb9804a6@arm.com>
Date: Wed, 6 Oct 2021 14:33:34 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.13.0
MIME-Version: 1.0
In-Reply-To: <75fcbce8-4d42-3677-afa8-6857fd01be19@arm.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: quoted-printable

Hi Sami,

Thanks for the review, please find my answer as [Pierre]

On 10/1/21 4:23 PM, Sami Mujawar wrote:
> Hi Pierre,
>
> Please find my response 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 AmlCodeGenMethodRetNameString() to generate AML code to create
>> a Method returning a NameString (NS).
>>
>> AmlCodeGenMethodRetNameString (
>> =C2=A0=C2=A0 "MET0", "_CRS", 1, TRUE, 3, ParentNode, NewObjectNode
>> =C2=A0=C2=A0 );
>> is equivalent of the following ASL code:
>> Method(MET0, 1, Serialized, 3) {
>> =C2=A0=C2=A0 Return (_CRS)
>> }
>>
>> Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.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 53 +++++++++
>> =C2=A0 .../Common/AmlLib/CodeGen/AmlCodeGen.c=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0 | 106 ++++++++++++++++++
>> =C2=A0 2 files changed, 159 insertions(+)
>>
>> diff --git a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
>> b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
>> index 6824cf3a6c82..7740aac24470 100644
>> --- a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
>> +++ b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
>> @@ -619,6 +619,59 @@ AmlCodeGenScope (
>> =C2=A0=C2=A0=C2=A0 OUT=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 AML_OBJECT_=
NODE_HANDLE=C2=A0 * NewObjectNode=C2=A0=C2=A0 OPTIONAL
>> =C2=A0=C2=A0=C2=A0 );
>> =C2=A0 +/** AML code generation for a method returning a NameString.
>> +
>> +=C2=A0 AmlCodeGenMethodRetNameString (
>> +=C2=A0=C2=A0=C2=A0 "MET0", "_CRS", 1, TRUE, 3, ParentNode, NewObjectN=
ode
>> +=C2=A0=C2=A0=C2=A0 );
>> +=C2=A0 is equivalent of the following ASL code:
>> +=C2=A0=C2=A0=C2=A0 Method(MET0, 1, Serialized, 3) {
>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 Return (_CRS)
>> +=C2=A0=C2=A0=C2=A0 }
>> +
>> +=C2=A0 The ASL parameters "ReturnType" and "ParameterTypes" are not a=
sked
>> +=C2=A0 in this function. They are optional parameters in ASL.
>> +
>> +=C2=A0 @ingroup CodeGenApis
>> +
>> +=C2=A0 @param [in]=C2=A0 MethodNameString=C2=A0=C2=A0=C2=A0=C2=A0 The=
 new Method's name.
>> +=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 Must b=
e a NULL-terminated ASL
>> NameString
>> +=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 e.g.: =
"MET0", "_SB.MET0", etc.
>> +=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 The in=
put string is copied.
>> +=C2=A0 @param [in]=C2=A0 ReturnedNameString=C2=A0=C2=A0 The name of t=
he object returned
>> by the
>> +=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 method=
. Optional parameter, can be:
>> +=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 =
- NULL (ignored).
>> +=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 =
- A NULL-terminated ASL
>> NameString.
>> +=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 e.g.: "MET0", "_SB.MET0", etc.
>> +=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 The input string is copied.
> [SAMI] I think this should be a mandatory parameter and corresponding
> change is needed to the function code.

[Pierre]

Some methods don't have to return a specific object.
E.g.: The _SRS method (ACPI 6.4, s6.2.16 _SRS (Set Resource Settings))
which returns None as specified.
In such case, it is not necessary to return a NameString/path to a real
object.

>> +=C2=A0 @param [in]=C2=A0 NumArgs=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 Number of arguments.
>> +=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 Must b=
e 0 <=3D NumArgs <=3D 6.
>> +=C2=A0 @param [in]=C2=A0 IsSerialized=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0 TRUE is equivalent to Serialized.
>> +=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 FALSE =
is equivalent to
>> NotSerialized.
>> +=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 Defaul=
t is NotSerialized in ASL
>> spec.
>> +=C2=A0 @param [in]=C2=A0 SyncLevel=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 Synchronization level for the
>> method.
>> +=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 Must b=
e 0 <=3D SyncLevel <=3D 15.
>> +=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 Defaul=
t is 0 in ASL.
>> +=C2=A0 @param [in]=C2=A0 ParentNode=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0 If provided, set ParentNode as
>> the parent
>> +=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 of the=
 node created.
>> +=C2=A0 @param [out] NewObjectNode=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0 If success, contains the created
>> 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 alloc=
ate memory.
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +AmlCodeGenMethodRetNameString (
>> +=C2=A0 IN=C2=A0 CONST 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 * Meth=
odNameString,
>> +=C2=A0 IN=C2=A0 CONST 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 * Retu=
rnedNameString,=C2=A0 OPTIONAL
>> +=C2=A0 IN=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 UINT8=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 NumArgs,
>> +=C2=A0 IN=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 BOOLEAN=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 IsSerialized,
>> +=C2=A0 IN=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 UINT8=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 SyncLevel,
>> +=C2=A0 IN=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 AML_NODE_HANDLE=C2=
=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 ParentNode,=C2=A0=
=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 OPTIONAL
>> +=C2=A0 OUT=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 AML_OBJECT_NODE_HANDLE=
=C2=A0 * NewObjectNode=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 OPTIONAL
>> +=C2=A0 );
>> +
>
[snip]