From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id 3CF1A74003B for ; Wed, 20 Dec 2023 07:48:44 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=EigsGfsiHjG+s5NZmRoF/UBao4k8rle8XflzIaFg0JI=; c=relaxed/simple; d=groups.io; h=Message-ID:Date:MIME-Version:User-Agent:Subject:To:Cc:References:From:In-Reply-To:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Language:Content-Type:Content-Transfer-Encoding; s=20140610; t=1703058522; v=1; b=FyXqlR3u5y00+IRDx8zZmXhtpx+3MaRGPgEb359MBFaXrLHZlmBvlpT4OQFEIU7qx0tA9pW/ 0jyypwk8QwDnP94EsfAsgRtWq6g+IM8GNlSaDpiVam8Zm3wgq4+yOlATskowA+Es2lrTiAN+Nno YSwDaZ7zRuRfeDyUX7iSUw6g= X-Received: by 127.0.0.2 with SMTP id gJpOYY7687511xQCvnzu7cHf; Tue, 19 Dec 2023 23:48:42 -0800 X-Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mx.groups.io with SMTP id smtpd.web10.17316.1703058522047716612 for ; Tue, 19 Dec 2023 23:48:42 -0800 X-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 B63501FB; Tue, 19 Dec 2023 23:49:25 -0800 (PST) X-Received: from [192.168.1.25] (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 492543F64C; Tue, 19 Dec 2023 23:48:40 -0800 (PST) Message-ID: <40d1fe0b-db99-45cd-b3ae-5e6b5472b45b@arm.com> Date: Wed, 20 Dec 2023 08:48:30 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [edk2-devel] [Resend PATCH v4 4/4] DynamicTablesPkg: AML Code generation to invoke a method To: "Attar, AbdulLateef (Abdul Lateef)" , "devel@edk2.groups.io" Cc: Sami Mujawar References: <1c56b1a9e5afe32036d950a33372849b36e10404.1702954979.git.AbdulLateef.Attar@amd.com> From: "PierreGondois" In-Reply-To: Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,pierre.gondois@arm.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: ASwXSR31ygXYmCbCwNdlIbRRx7686176AA= Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=FyXqlR3u; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=arm.com (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io Hello Abdul, On 12/20/23 05:33, Attar, AbdulLateef (Abdul Lateef) wrote: > [Public] >=20 > Hi Pierre, > Regarding the below comment >> +AmlCodeGenInvokeMethod ( >> + IN CONST CHAR8 *MethodNameString, >> + IN UINT8 NumArgs, >> + IN AML_METHOD_PARAM *Parameters OPTIONAL, >> + IN AML_NODE_HANDLE ParentNode > Maybe it would be worth also having an OUT parameter in case someone does= n't want to attach the method invocation directly (cf. other methods in the= file): > OUT AML_OBJECT_NODE_HANDLE *NewObjectNode OPTIONAL >=20 > I don=E2=80=99t think its possible because MethodInvocation is a stream o= f objects, and first object is NameString(method name) which is "Data node"= . > AML library doesn=E2=80=99t allow adding variable list arguments to the "= Data node", only "object node" is supported. Yes right, Regards, Pierre >=20 > I'll address the remaining comments and submit the v5 patch. >=20 > Thanks > AbduL >=20 > -----Original Message----- > From: Pierre Gondois > Sent: Tuesday, December 19, 2023 7:41 PM > To: Attar, AbdulLateef (Abdul Lateef) ; devel@= edk2.groups.io > Cc: Attar, AbdulLateef (Abdul Lateef) ; Sami M= ujawar > Subject: Re: [Resend PATCH v4 4/4] DynamicTablesPkg: AML Code generation = to invoke a method >=20 > Caution: This message originated from an External Source. Use proper caut= ion when opening attachments, clicking links, or responding. >=20 >=20 > Hello Abdul, > The new function looks good, there is just one small issue with the tree = layout, >=20 > Regards, > Pierre >=20 > On 12/19/23 04:06, Abdul Lateef Attar wrote: >> From: Abdul Lateef Attar >> >> Adds API to generate AML code to invoke/call another method. Also >> provides ability to pass arguments of type integer, string, ArgObj or >> LocalObj. >> >> Cc: Pierre Gondois >> Cc: Sami Mujawar >> Signed-off-by: Abdul Lateef Attar >> --- >> .../Include/Library/AmlLib/AmlLib.h | 112 +++++++++ >> .../Common/AmlLib/CodeGen/AmlCodeGen.c | 236 +++++++++++++++++= - >> 2 files changed, 347 insertions(+), 1 deletion(-) >> >> diff --git a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h >> b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h >> index eb8740692f..fb16637f02 100644 >> --- a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h >> +++ b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h >> @@ -101,6 +101,56 @@ typedef enum { >> AmlAddressRangeMax =3D 4 >> } AML_MEMORY_ATTRIBUTES_MTP; >> >> +/** Method parameter types >> + >> + Possible values are: >> + 0 - AmlMethodParamTypeInteger >> + 1 - AmlMethodParamTypeString >> + 2 - AmlMethodParamTypeArg >> + 3 - AmlMethodParamTypeLocal >> + >> + @par Reference(s) >> + - ACPI 6.5, s20.2.5 "Term Objects Encoding" >> + >> +**/ >> +typedef enum { >> + AmlMethodParamTypeInteger =3D 0, >> + AmlMethodParamTypeString =3D 1, >> + AmlMethodParamTypeArg =3D 2, >> + AmlMethodParamTypeLocal =3D 3 >> +} AML_METHOD_PARAM_TYPE; >> + >> +/** AML Method parameter data >> + holds the AML method parameter data. >> +**/ >> +typedef union { >> + UINT8 Arg; >> + UINT8 Local; >> + UINT64 Integer; >> + VOID *Buffer; >> +} AML_METHOD_PARAM_DATA; >> + >> +/** structure to hold AML method parameter types >> + Type - Type of parameter >> + Data - holds data of parameter >> + if Type is AmlMethodParamTypeInteger >> + then Data is of type Integer to hold integer value. >> + if Type is AmlMethodParamTypeString >> + then Data contains null terminated string. >> + If Type is AmlMethodParamTypeArg >> + then Data contains the Argument number, >> + 0 to 6 are supported value. >> + If Type is AmlMethodParamTypeLocal >> + then Data contains the Local variable number, >> + 0 to 7 are supported value. >> + DataSize - for future use >> +**/ >> +typedef struct { >> + AML_METHOD_PARAM_TYPE Type; >> + AML_METHOD_PARAM_DATA Data; >> + UINTN DataSize; >> +} AML_METHOD_PARAM; >> + >> /** Parse the definition block. >> >> The function parses the whole AML blob. It starts with the ACPI >> DSDT/SSDT @@ -1693,4 +1743,66 @@ AmlAddNameStringToNamedPackage ( >> IN AML_OBJECT_NODE_HANDLE NamedNode >> ); >> >> +/** AML code generation to invoke/call another method. >> + >> + This method is subset implementation of MethodInvocation defined >> + in the ACPI specification 6.5, section 20.2.5 "Term Objects >> + Encoding". >> + Added integer, string, ArgObj and LocalObj support. >> + >> + Example 1: >> + AmlCodeGenInvokeMethod ("MET0", 0, NULL, ParentNode); >> + is equivalent of the following ASL code: >> + MET0 (); >> + >> + Example 2: >> + AML_METHOD_PARAM Param[4]; >> + Param[0].Data.Integer =3D 0x100; >> + Param[0].Type =3D AmlMethodParamTypeInteger; >> + Param[1].Data.Buffer =3D "TEST"; >> + Param[1].Type =3D AmlMethodParamTypeString; >> + Param[2].Data.Arg =3D 0; >> + Param[2].Type =3D AmlMethodParamTypeArg; >> + Param[3].Data.Local =3D 2; >> + Param[3].Type =3D AmlMethodParamTypeLocal; >> + AmlCodeGenInvokeMethod ("MET0", 4, Param, ParentNode); >> + >> + is equivalent of the following ASL code: >> + MET0 (0x100, "TEST", Arg0, Local2); >> + >> + Example 3: >> + AML_METHOD_PARAM Param[2]; >> + Param[0].Data.Arg =3D 0; >> + Param[0].Type =3D AmlMethodParamTypeArg; >> + Param[1].Data.Integer =3D 0x100; >> + Param[1].Type =3D AmlMethodParamTypeInteger; >> + AmlCodeGenMethodRetNameString ("MET2", NULL, 2, TRUE, 0, ParentNode= , &MethodNode); >> + AmlCodeGenInvokeMethod ("MET3", 2, Param, MethodNode); >> + >> + is equivalent of the following ASL code: >> + Method (MET2, 2, Serialized) >> + { >> + MET3 (Arg0, 0x0100) >> + } >> + >> + @param [in] MethodNameString Method name to be called/invoked. >> + @param [in] NumArgs Number of arguments to be passed, >> + 0 to 7 are permissible values. >> + @param [in] Parameters contains the parameter data. >> + @param [in] ParentNode set ParentNode as the parent> + = of the node created. >> + >> + @retval EFI_SUCCESS Success. >> + @retval EFI_INVALID_PARAMETER Invalid parameter. >> + @retval EFI_OUT_OF_RESOURCES Failed to allocate memory. >> + **/ >> +EFI_STATUS >> +EFIAPI >> +AmlCodeGenInvokeMethod ( >> + IN CONST CHAR8 *MethodNameString, >> + IN UINT8 NumArgs, >> + IN AML_METHOD_PARAM *Parameters OPTIONAL, >> + IN AML_NODE_HANDLE ParentNode >> + ); >> + >> #endif // AML_LIB_H_ >> diff --git >> a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c >> b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c >> index a6db34fb97..688eefdcef 100644 >> --- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c >> +++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c >> @@ -2,6 +2,7 @@ >> AML Code Generation. >> >> Copyright (c) 2020 - 2022, Arm Limited. All rights reserved.
>> + Copyright (C) 2023 Advanced Micro Devices, Inc. All rights >> + reserved.
>> >> SPDX-License-Identifier: BSD-2-Clause-Patent >> **/ >> @@ -921,7 +922,7 @@ AmlCodeGenNameUnicodeString ( >> Status =3D AmlCreateDataNode ( >> EAmlNodeDataTypeRaw, >> (CONST UINT8 *)String, >> - StrSize (String), >> + (UINT32)StrSize (String), >> &DataNode >> ); >> if (EFI_ERROR (Status)) { >> @@ -3849,3 +3850,236 @@ exit_handler: >> >> return Status; >> } >> + >> +/** AML code generation to invoke/call another method. >> + >> + This method is subset implementation of MethodInvocation defined >> + in the ACPI specification 6.5, section 20.2.5 "Term Objects >> + Encoding". >> + Added integer, string, ArgObj and LocalObj support. >> + >> + Example 1: >> + AmlCodeGenInvokeMethod ("MET0", 0, NULL, ParentNode); >> + is equivalent of the following ASL code: >> + MET0 (); >> + >> + Example 2: >> + AML_METHOD_PARAM Param[4]; >> + Param[0].Data.Integer =3D 0x100; >> + Param[0].Type =3D AmlMethodParamTypeInteger; >> + Param[1].Data.Buffer =3D "TEST"; >> + Param[1].Type =3D AmlMethodParamTypeString; >> + Param[2].Data.Arg =3D 0; >> + Param[2].Type =3D AmlMethodParamTypeArg; >> + Param[3].Data.Local =3D 2; >> + Param[3].Type =3D AmlMethodParamTypeLocal; >> + AmlCodeGenInvokeMethod ("MET0", 4, Param, ParentNode); >> + >> + is equivalent of the following ASL code: >> + MET0 (0x100, "TEST", Arg0, Local2); >> + >> + Example 3: >> + AML_METHOD_PARAM Param[2]; >> + Param[0].Data.Arg =3D 0; >> + Param[0].Type =3D AmlMethodParamTypeArg; >> + Param[1].Data.Integer =3D 0x100; >> + Param[1].Type =3D AmlMethodParamTypeInteger; >> + AmlCodeGenMethodRetNameString ("MET2", NULL, 2, TRUE, 0, ParentNode= , &MethodNode); >> + AmlCodeGenInvokeMethod ("MET3", 2, Param, MethodNode); >> + >> + is equivalent of the following ASL code: >> + Method (MET2, 2, Serialized) >> + { >> + MET3 (Arg0, 0x0100) >> + } >> + >> + @param [in] MethodNameString Method name to be called/invoked. >> + @param [in] NumArgs Number of arguments to be passed, >> + 0 to 7 are permissible values. >> + @param [in] Parameters contains the parameter data. >> + @param [in] ParentNode set ParentNode as the parent >=20 > NIT: set->Set >=20 >> + of the node created. >> + >> + @retval EFI_SUCCESS Success. >> + @retval EFI_INVALID_PARAMETER Invalid parameter. >> + @retval EFI_OUT_OF_RESOURCES Failed to allocate memory. >> + **/ >> +EFI_STATUS >> +EFIAPI >> +AmlCodeGenInvokeMethod ( >> + IN CONST CHAR8 *MethodNameString, >> + IN UINT8 NumArgs, >> + IN AML_METHOD_PARAM *Parameters OPTIONAL, >> + IN AML_NODE_HANDLE ParentNode >=20 > Maybe it would be worth also having an OUT parameter in case someone does= n't want to attach the method invocation directly (cf. other methods in the= file): > OUT AML_OBJECT_NODE_HANDLE *NewObjectNode OPTIONAL >=20 >=20 >> + ) >> +{ >> + EFI_STATUS Status; >> + UINT8 Index; >> + CHAR8 *AmlNameString; >> + UINT32 AmlNameStringSize; >> + AML_DATA_NODE *DataNode; >> + AML_OBJECT_NODE *IntNode; >> + AML_NODE_HANDLE HeadNode; >> + AML_NODE_HANDLE CurrentNode; >> + AML_OBJECT_NODE *ObjectNode; >> + >> + if ((MethodNameString =3D=3D NULL) || (ParentNode =3D=3D NULL)) { >> + ASSERT (0); >> + return EFI_INVALID_PARAMETER; >> + } >> + >> + if ((NumArgs > 7) || >> + ((Parameters =3D=3D NULL) && (NumArgs > 0))) { >> + ASSERT (0); >> + return EFI_INVALID_PARAMETER; >> + } >> + >> + if (Parameters !=3D NULL) { >> + /// Validate and convert the Parameters to the stream of node >> + HeadNode =3D NULL; >> + CurrentNode =3D NULL; >> + for (Index =3D 0; Index < NumArgs; Index++) { >> + switch (Parameters[Index].Type) { >> + case AmlMethodParamTypeInteger: >> + IntNode =3D NULL; >> + Status =3D AmlCodeGenInteger (Parameters[Index].Data.Integer= , &IntNode); >> + if (EFI_ERROR (Status)) { >> + ASSERT_EFI_ERROR (Status); >> + goto exit_handler; >> + } >> + >> + CurrentNode =3D (AML_NODE_HANDLE)IntNode; >> + break; >> + case AmlMethodParamTypeString: >> + ObjectNode =3D NULL; >> + if (Parameters[Index].Data.Buffer =3D=3D NULL) { >> + ASSERT_EFI_ERROR (EFI_INVALID_PARAMETER); >> + Status =3D EFI_INVALID_PARAMETER; >> + goto exit_handler; >> + } >> + >> + Status =3D AmlCodeGenString (Parameters[Index].Data.Buffer, &= ObjectNode); >> + if (EFI_ERROR (Status)) { >> + ASSERT_EFI_ERROR (Status); >> + goto exit_handler; >> + } >> + >> + CurrentNode =3D (AML_NODE_HANDLE)ObjectNode; >> + break; >> + case AmlMethodParamTypeArg: >> + ObjectNode =3D NULL; >> + if (Parameters[Index].Data.Arg > (UINT8)(AML_ARG6 - AML_ARG0)= ) { >> + ASSERT_EFI_ERROR (EFI_INVALID_PARAMETER); >> + Status =3D EFI_INVALID_PARAMETER; >> + goto exit_handler; >> + } >> + >> + Status =3D AmlCreateObjectNode ( >> + AmlGetByteEncodingByOpCode (AML_ARG0 + Parameters[= Index].Data.Arg, 0), >> + 0, >> + &ObjectNode >> + ); >> + if (EFI_ERROR (Status)) { >> + ASSERT_EFI_ERROR (Status); >> + goto exit_handler; >> + } >> + >> + CurrentNode =3D (AML_NODE_HANDLE)ObjectNode; >> + break; >> + case AmlMethodParamTypeLocal: >> + ObjectNode =3D NULL; >> + if (Parameters[Index].Data.Local > (UINT8)(AML_LOCAL7 - AML_L= OCAL0)) { >> + ASSERT_EFI_ERROR (EFI_INVALID_PARAMETER); >> + Status =3D EFI_INVALID_PARAMETER; >> + goto exit_handler; >> + } >> + >> + Status =3D AmlCreateObjectNode ( >> + AmlGetByteEncodingByOpCode (AML_LOCAL0 + Parameter= s[Index].Data.Local, 0), >> + 0, >> + &ObjectNode >> + ); >> + if (EFI_ERROR (Status)) { >> + ASSERT_EFI_ERROR (Status); >> + goto exit_handler; >> + } >> + >> + CurrentNode =3D (AML_NODE_HANDLE)ObjectNode; >> + break; >> + default: >> + ASSERT_EFI_ERROR (EFI_INVALID_PARAMETER); >> + Status =3D EFI_INVALID_PARAMETER; >> + goto exit_handler; >> + break; >> + } >> + >> + if (HeadNode =3D=3D NULL) { >> + HeadNode =3D CurrentNode; >=20 > I think there is an issue here. The method arguments should be attached t= o the variable list of arguments of the parent, not of another argument. >=20 > For instance, the following: > Scope (_SB) { > MET0 (Arg0, Arg1, Arg2) > } >=20 > Should create a tree as: > _SB > \- (Variable list of arguments of _SB) > - MET0 > - Arg0 > - Arg1 > - Arg2 >=20 > With the present code, the created tree is: > _SB > \- (Variable list of arguments of _SB) > - MET0 > - Arg0 > \- (Variable list of arguments of Arg0) > - Arg1 > - Arg2 >=20 > When serializing the tree, the generated binary is identical, but the tre= e is incorrect. >=20 >> + } else { >> + Status =3D AmlVarListAddTail ( >> + HeadNode, >> + CurrentNode >> + ); >> + if (EFI_ERROR (Status)) { >> + ASSERT_EFI_ERROR (Status); >> + goto exit_handler; >> + } >> + } >> + } >> + } >> + >> + /// Create called/invoked method name string Status =3D >> + ConvertAslNameToAmlName (MethodNameString, &AmlNameString); if >> + (EFI_ERROR (Status)) { >> + ASSERT_EFI_ERROR (Status); >> + goto exit_handler; >> + } >> + >> + Status =3D AmlGetNameStringSize (AmlNameString, &AmlNameStringSize); >> + if (EFI_ERROR (Status)) { >> + ASSERT_EFI_ERROR (Status); >> + FreePool (AmlNameString); >> + goto exit_handler; >> + } >> + >> + DataNode =3D NULL; >> + Status =3D AmlCreateDataNode ( >> + EAmlNodeDataTypeNameString, >> + (UINT8 *)AmlNameString, >> + AmlNameStringSize, >> + &DataNode >> + ); >> + FreePool (AmlNameString); >> + if (EFI_ERROR (Status)) { >> + ASSERT_EFI_ERROR (Status); >> + goto exit_handler; >> + } >> + >> + Status =3D AmlVarListAddTail ( >> + (AML_NODE_HANDLE)ParentNode, >> + (AML_NODE_HANDLE)DataNode >> + ); >> + if (EFI_ERROR (Status)) { >=20 > I think DataNode would not be freed if we fail here. >=20 >> + ASSERT_EFI_ERROR (Status); >> + goto exit_handler; >> + } >> + >> + Status =3D AmlVarListAddTail ( >> + (AML_NODE_HANDLE)ParentNode, >> + HeadNode >> + ); >> + if (EFI_ERROR (Status)) { >> + ASSERT_EFI_ERROR (Status); >> + goto exit_handler; >> + } >> + >> + return Status; >> + >> +exit_handler: >> + if (HeadNode !=3D NULL) { >> + AmlDeleteTree (HeadNode); >> + } >> + >> + return Status; >> +} -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#112750): https://edk2.groups.io/g/devel/message/112750 Mute This Topic: https://groups.io/mt/103256864/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-