[AMD Official Use Only - General]
Hi Sami,
Thanks for the in-depth review and making the changes.
Regarding below comment.
[SAMI] This does not seem to be related to this patch. Were you getting any
issues when building? If so, can you submit a separate patch, please?
If you agree, I will drop the above line from this patch and merge.
[/SAMI]
[Abdul] With VS2019 I am seeing compilation error, I'll submit a separate patch
for this.
Thanks
AbduL
-----Original Message-----
From: Sami Mujawar <sami.muja...@arm.com>
Sent: Thursday, December 21, 2023 7:26 PM
To: Attar, AbdulLateef (Abdul Lateef) <abdullateef.at...@amd.com>;
devel@edk2.groups.io
Cc: Attar, AbdulLateef (Abdul Lateef) <abdullateef.at...@amd.com>; Pierre
Gondois <pierre.gond...@arm.com>; n...@arm.com
Subject: Re: [Resend PATCH v5 4/4] DynamicTablesPkg: AML Code generation to
invoke a method
Caution: This message originated from an External Source. Use proper caution
when opening attachments, clicking links, or responding.
Hi Abdul,
I have some minor feedback maked inline as [SAMI].
I think other than the typecast change for the value returned by StrSize(), the
remainder of the patch looks ok to me (i.e. with the minor changes I suggested).
Please let me know if you agree with my suggestions and I will make the changes
and get this patch merged.
Regards,
Sami Mujawar
On 20/12/2023 09:38 am, Abdul Lateef Attar wrote:
> From: Abdul Lateef Attar <abdullateef.at...@amd.com>
>
> 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 <pierre.gond...@arm.com>
> Cc: Sami Mujawar <sami.muja...@arm.com>
> Signed-off-by: Abdul Lateef Attar <abdullateef.at...@amd.com>
> ---
> .../Include/Library/AmlLib/AmlLib.h | 112 +++++++++
> .../Common/AmlLib/CodeGen/AmlCodeGen.c | 235 +++++++++++++++++-
> 2 files changed, 346 insertions(+), 1 deletion(-)
>
> diff --git a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
> b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
> index eb8740692f..043ec3d842 100644
> --- a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
> +++ b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
> @@ -101,6 +101,56 @@ typedef enum {
> AmlAddressRangeMax = 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 = 0,
> + AmlMethodParamTypeString = 1,
> + AmlMethodParamTypeArg = 2,
> + AmlMethodParamTypeLocal = 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 a 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 to the following ASL code:
> + MET0 ();
> +
> + Example 2:
> + AML_METHOD_PARAM Param[4];
> + Param[0].Data.Integer = 0x100;
> + Param[0].Type = AmlMethodParamTypeInteger;
> + Param[1].Data.Buffer = "TEST";
> + Param[1].Type = AmlMethodParamTypeString;
> + Param[2].Data.Arg = 0;
> + Param[2].Type = AmlMethodParamTypeArg;
> + Param[3].Data.Local = 2;
> + Param[3].Type = AmlMethodParamTypeLocal;
> + AmlCodeGenInvokeMethod ("MET0", 4, Param, ParentNode);
> +
> + is equivalent to the following ASL code:
> + MET0 (0x100, "TEST", Arg0, Local2);
> +
> + Example 3:
> + AML_METHOD_PARAM Param[2];
> + Param[0].Data.Arg = 0;
> + Param[0].Type = AmlMethodParamTypeArg;
> + Param[1].Data.Integer = 0x100;
> + Param[1].Type = AmlMethodParamTypeInteger;
> + AmlCodeGenMethodRetNameString ("MET2", NULL, 2, TRUE, 0, ParentNode,
> &MethodNode);
> + AmlCodeGenInvokeMethod ("MET3", 2, Param, MethodNode);
> +
> + is equivalent to the following ASL code:
> + Method (MET2, 2, Serialized)
> + {
> + MET3 (Arg0, 0x0100)
> + }
> +
> + @param [in] MethodNameString The method name to be called or 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 The parent node to which the method
> invocation
> + nodes are attached.
> +
> + @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..ddec9b67ed 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.<BR>
> + Copyright (C) 2023 Advanced Micro Devices, Inc. All rights
> + reserved.<BR>
>
> SPDX-License-Identifier: BSD-2-Clause-Patent
> **/
> @@ -921,7 +922,7 @@ AmlCodeGenNameUnicodeString (
> Status = AmlCreateDataNode (
> EAmlNodeDataTypeRaw,
> (CONST UINT8 *)String,
> - StrSize (String),
> + (UINT32)StrSize (String),
[SAMI] This does not seem to be related to this patch. Were you getting any
issues when building? If so, can you submit a separate patch, please?
If you agree, I will drop the above line from this patch and merge.
[/SAMI]
> &DataNode
> );
> if (EFI_ERROR (Status)) {
> @@ -3849,3 +3850,235 @@ exit_handler:
>
> return Status;
> }
> +
> +/** AML code generation to invoke/call another method.
> +
> + This method is a 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 to the following ASL code:
> + MET0 ();
> +
> + Example 2:
> + AML_METHOD_PARAM Param[4];
> + Param[0].Data.Integer = 0x100;
> + Param[0].Type = AmlMethodParamTypeInteger;
> + Param[1].Data.Buffer = "TEST";
> + Param[1].Type = AmlMethodParamTypeString;
> + Param[2].Data.Arg = 0;
> + Param[2].Type = AmlMethodParamTypeArg;
> + Param[3].Data.Local = 2;
> + Param[3].Type = AmlMethodParamTypeLocal;
> + AmlCodeGenInvokeMethod ("MET0", 4, Param, ParentNode);
> +
> + is equivalent to the following ASL code:
> + MET0 (0x100, "TEST", Arg0, Local2);
> +
> + Example 3:
> + AML_METHOD_PARAM Param[2];
> + Param[0].Data.Arg = 0;
> + Param[0].Type = AmlMethodParamTypeArg;
> + Param[1].Data.Integer = 0x100;
> + Param[1].Type = AmlMethodParamTypeInteger;
> + AmlCodeGenMethodRetNameString ("MET2", NULL, 2, TRUE, 0, ParentNode,
> &MethodNode);
> + AmlCodeGenInvokeMethod ("MET3", 2, Param, MethodNode);
> +
> + is equivalent to the following ASL code:
> + Method (MET2, 2, Serialized)
> + {
> + MET3 (Arg0, 0x0100)
> + }
> +
> + @param [in] MethodNameString The method name to be called or 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 The parent node to which the method
> invocation
> + nodes are attached.
> +
> + @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
> + )
> +{
> + EFI_STATUS Status;
> + UINT8 Index;
> + CHAR8 *AmlNameString;
> + UINT32 AmlNameStringSize;
> + AML_DATA_NODE *DataNode;
> + AML_OBJECT_NODE *ObjectNode;
> + AML_NODE_HANDLE *NodeStream;
> +
> + if ((MethodNameString == NULL) || (ParentNode == NULL)) {
> + ASSERT (0);
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + if ((NumArgs > 7) ||
> + ((Parameters == NULL) && (NumArgs > 0))) {
> + ASSERT (0);
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + /// Allocate space to store methodname, object, data node pointers
> + NodeStream = AllocateZeroPool (sizeof (AML_NODE_HANDLE) * (NumArgs +
> + 1)); if (NodeStream == NULL) {
> + ASSERT (0);
> + return EFI_OUT_OF_RESOURCES;
> + }
> +
> + /// Create a called or invoked method name string.
> + Status = ConvertAslNameToAmlName (MethodNameString,
> + &AmlNameString); if (EFI_ERROR (Status)) {
> + ASSERT_EFI_ERROR (Status);
> + goto exit_handler;
> + }
> +
> + Status = AmlGetNameStringSize (AmlNameString, &AmlNameStringSize);
> + if (EFI_ERROR (Status)) {
> + ASSERT_EFI_ERROR (Status);
> + FreePool (AmlNameString);
> + goto exit_handler;
> + }
> +
> + DataNode = NULL;
> + Status = AmlCreateDataNode (
> + EAmlNodeDataTypeNameString,
> + (UINT8 *)AmlNameString,
> + AmlNameStringSize,
> + &DataNode
> + );
> + FreePool (AmlNameString);
> + if (EFI_ERROR (Status)) {
> + ASSERT_EFI_ERROR (Status);
> + goto exit_handler;
> + }
> +
> + NodeStream[0] = (AML_NODE_HANDLE)DataNode;
> +
> + if (Parameters != NULL) {
> + /// Validate and convert the Parameters to the stream of nodes.
> + for (Index = 0; Index < NumArgs; Index++) {
> + switch (Parameters[Index].Type) {
> + case AmlMethodParamTypeInteger:
> + ObjectNode = NULL;
[SAMI] I think the above line can be moved right after the 'for'
statement. If you agree I can fix this before merging.
> + Status = AmlCodeGenInteger (Parameters[Index].Data.Integer,
> &ObjectNode);
> + if (EFI_ERROR (Status)) {
> + ASSERT_EFI_ERROR (Status);
> + goto exit_handler;
> + }
> +
> + NodeStream[Index+1] = (AML_NODE_HANDLE)ObjectNode;
[SAMI] Minor, a space is required before and after '+'. I will fix that before
merging.
> + break;
> + case AmlMethodParamTypeString:
> + ObjectNode = NULL;
> + if (Parameters[Index].Data.Buffer == NULL) {
> + ASSERT_EFI_ERROR (EFI_INVALID_PARAMETER);
> + Status = EFI_INVALID_PARAMETER;
> + goto exit_handler;
> + }
> +
> + Status = AmlCodeGenString (Parameters[Index].Data.Buffer,
> + &ObjectNode);
[SAMI] Minor, I prefer to keep the code within 80 characters. I will fix that
before merging.
> + if (EFI_ERROR (Status)) {
> + ASSERT_EFI_ERROR (Status);
> + goto exit_handler;
> + }
> +
> + NodeStream[Index+1] = (AML_NODE_HANDLE)ObjectNode;
[SAMI] I believe the above line can be moved after the switch block. I can do
that if you agree.
> + break;
> + case AmlMethodParamTypeArg:
> + ObjectNode = NULL;
> + if (Parameters[Index].Data.Arg > (UINT8)(AML_ARG6 - AML_ARG0)) {
> + ASSERT_EFI_ERROR (EFI_INVALID_PARAMETER);
> + Status = EFI_INVALID_PARAMETER;
> + goto exit_handler;
> + }
> +
> + Status = AmlCreateObjectNode (
> + AmlGetByteEncodingByOpCode (AML_ARG0 +
> Parameters[Index].Data.Arg, 0),
> + 0,
> + &ObjectNode
> + );
> + if (EFI_ERROR (Status)) {
> + ASSERT_EFI_ERROR (Status);
> + goto exit_handler;
> + }
> +
> + NodeStream[Index+1] = (AML_NODE_HANDLE)ObjectNode;
> + break;
> + case AmlMethodParamTypeLocal:
> + ObjectNode = NULL;
> + if (Parameters[Index].Data.Local > (UINT8)(AML_LOCAL7 -
> AML_LOCAL0)) {
> + ASSERT_EFI_ERROR (EFI_INVALID_PARAMETER);
> + Status = EFI_INVALID_PARAMETER;
> + goto exit_handler;
> + }
> +
> + Status = AmlCreateObjectNode (
> + AmlGetByteEncodingByOpCode (AML_LOCAL0 +
> Parameters[Index].Data.Local, 0),
> + 0,
> + &ObjectNode
> + );
> + if (EFI_ERROR (Status)) {
> + ASSERT_EFI_ERROR (Status);
> + goto exit_handler;
> + }
> +
> + NodeStream[Index+1] = (AML_NODE_HANDLE)ObjectNode;
> + break;
> + default:
> + ASSERT_EFI_ERROR (EFI_INVALID_PARAMETER);
> + Status = EFI_INVALID_PARAMETER;
> + goto exit_handler;
> + break;
> + }
> + }
> + }
> +
> + /// Index <= NumArgs, because an additional method name was added.
> + for (Index = 0; Index <= NumArgs; Index++) {
> + Status = AmlVarListAddTail (
> + (AML_NODE_HANDLE)ParentNode,
> + (AML_NODE_HANDLE)NodeStream[Index]
> + );
> + if (EFI_ERROR (Status)) {
> + ASSERT_EFI_ERROR (Status);
> + goto exit_handler_detach;
> + }
> + }
> +
> + FreePool (NodeStream);
> + return Status;
> +
> +exit_handler_detach:
> + /// The index contains the last successful node attached.
> + for ( ; Index > 0; Index--) {
> + /// Index contains the node number that is failed for
> AmlVarListAddTail().
> + /// Hence, start detaching from the last successful
> + AmlDetachNode (NodeStream[Index-1]);
> + }
> +
> +exit_handler:
> + /// Index <= NumArgs, because an additional method name was added.
> + for (Index = 0; Index <= NumArgs; Index++) {
> + if (NodeStream[Index] != 0) {
> + AmlDeleteTree (NodeStream[Index]);
> + }
> + }
> +
> + FreePool (NodeStream);
> + return Status;
> +}
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112834): https://edk2.groups.io/g/devel/message/112834
Mute This Topic: https://groups.io/mt/103278521/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
