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.web10.23594.1688377365335355278 for ; Mon, 03 Jul 2023 02:42:45 -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 B276E1FB; Mon, 3 Jul 2023 02:43:27 -0700 (PDT) Received: from [10.34.100.101] (e126645.nice.arm.com [10.34.100.101]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 2BC8C3F762; Mon, 3 Jul 2023 02:42:44 -0700 (PDT) Message-ID: Date: Mon, 3 Jul 2023 11:42:40 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.11.0 From: "PierreGondois" Subject: Re: [PATCH 2/4] DynamicTablesPkg: Add support for simple method invocation. To: Jeff Brasen , devel@edk2.groups.io Cc: Sami.Mujawar@arm.com, Alexei.Fedorov@arm.com, Swatisri Kantamsetti , Ashish Singhal References: In-Reply-To: Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Hello Jeff, On 6/30/23 01:01, Jeff Brasen wrote: > Add support to add Return objects via AML that pass a single integer > argument to the named method. > > Signed-off-by: Jeff Brasen > Reviewed-by: Swatisri Kantamsetti > Reviewed-by: Ashish Singhal > --- > .../Include/Library/AmlLib/AmlLib.h | 54 +++++ > .../Common/AmlLib/CodeGen/AmlCodeGen.c | 224 ++++++++++++++++++ > 2 files changed, 278 insertions(+) > > diff --git a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h > index d201ae9499..b82c7a3ce8 100644 > --- a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h > +++ b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h > @@ -1194,6 +1194,60 @@ AmlCodeGenMethodRetInteger ( > OUT AML_OBJECT_NODE_HANDLE *NewObjectNode OPTIONAL > ); > > +/** AML code generation for a method returning a NameString that takes an > + integer argument. > + > + AmlCodeGenMethodRetNameStringIntegerArgument ( > + "MET0", "MET1", 1, TRUE, 3, 5, ParentNode, NewObjectNode > + ); > + is equivalent of the following ASL code: > + Method(MET0, 1, Serialized, 3) { > + Return (MET1 (5)) > + } > + > + The ASL parameters "ReturnType" and "ParameterTypes" are not asked > + in this function. They are optional parameters in ASL. > + > + @param [in] MethodNameString The new Method's name. > + Must be a NULL-terminated ASL NameString > + e.g.: "MET0", "_SB.MET0", etc. > + The input string is copied. > + @param [in] ReturnedNameString The name of the object returned by the > + method. Optional parameter, can be: > + - NULL (ignored). > + - A NULL-terminated ASL NameString. > + e.g.: "MET0", "_SB.MET0", etc. > + The input string is copied. > + @param [in] NumArgs Number of arguments. > + Must be 0 <= NumArgs <= 6. > + @param [in] IsSerialized TRUE is equivalent to Serialized. > + FALSE is equivalent to NotSerialized. > + Default is NotSerialized in ASL spec. > + @param [in] SyncLevel Synchronization level for the method. > + Must be 0 <= SyncLevel <= 15. > + Default is 0 in ASL. > + @param [in] IntegerArgument Argument to pass to the NameString. > + @param [in] ParentNode If provided, set ParentNode as the parent > + of the node created. > + @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. > +**/ > +EFI_STATUS > +EFIAPI > +AmlCodeGenMethodRetNameStringIntegerArgument ( > + IN CONST CHAR8 *MethodNameString, > + IN CONST CHAR8 *ReturnedNameString OPTIONAL, > + IN UINT8 NumArgs, > + IN BOOLEAN IsSerialized, > + IN UINT8 SyncLevel, > + IN UINT64 IntegerArgument, > + IN AML_NODE_HANDLE ParentNode OPTIONAL, > + OUT AML_OBJECT_NODE_HANDLE *NewObjectNode OPTIONAL > + ); > + > /** Create a _LPI name. > > AmlCreateLpiNode ("_LPI", 0, 1, ParentNode, &LpiNode) is > diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c > index 88537b7e2d..2b95839e4c 100644 > --- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c > +++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c > @@ -1881,6 +1881,118 @@ AmlCodeGenReturnInteger ( > return Status; > } > > +/** AML code generation for a Return object node, > + returning the object as an input NameString with a integer argument. > + > + AmlCodeGenReturn ("NAM1", 6, ParentNode, NewObjectNode) is > + equivalent of the following ASL code: > + Return(NAM1 (6)) > + > + 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] Integer Argument to pass to the NameString > + @param [in] ParentNode If provided, set ParentNode as the parent > + of the node created. > + Must be a MethodOp node. > + @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 > +AmlCodeGenReturnNameStringIntegerArgument ( > + IN CONST CHAR8 *NameString, > + IN UINT64 Integer, > + IN AML_NODE_HEADER *ParentNode OPTIONAL, > + OUT AML_OBJECT_NODE **NewObjectNode OPTIONAL > + ) > +{ > + EFI_STATUS Status; > + AML_DATA_NODE *DataNode; > + AML_OBJECT_NODE *IntNode; > + CHAR8 *AmlNameString; > + UINT32 AmlNameStringSize; > + AML_OBJECT_NODE *ObjectNode; > + > + DataNode = NULL; > + IntNode = 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; > + } > + > + Status = AmlCodeGenInteger (Integer, &IntNode); > + if (EFI_ERROR (Status)) { > + ASSERT (0); I think AmlNameString still need to be freed, so it should be + goto exit_handler; > + return Status; > + } > + > + // AmlCodeGenReturn() deletes DataNode if error. > + Status = AmlCodeGenReturn ( > + (AML_NODE_HEADER *)DataNode, > + ParentNode, > + &ObjectNode > + ); > + ASSERT_EFI_ERROR (Status); > + > + Status = AmlVarListAddTail ( > + (AML_NODE_HANDLE)ObjectNode, > + (AML_NODE_HANDLE)IntNode > + ); > + if (EFI_ERROR (Status)) { > + ASSERT (0); similar remark here > + return Status; > + } > + > + if (NewObjectNode != 0) { > + *NewObjectNode = ObjectNode; > + } > + > +exit_handler: > + if (AmlNameString != NULL) { > + FreePool (AmlNameString); > + } > + > + return Status; > +} > +