[edk2-devel] [PATCH v4 2/4] DynamicTablesPkg: Add support for simple method invocation.

Jeff Brasen via groups.io posted 4 patches 5 months, 1 week ago
[edk2-devel] [PATCH v4 2/4] DynamicTablesPkg: Add support for simple method invocation.
Posted by Jeff Brasen via groups.io 5 months, 1 week ago
Add support to add Return objects via AML that pass a single integer

argument to the named method.



Signed-off-by: Jeff Brasen <jbrasen@nvidia.com>

---

 .../Include/Library/AmlLib/AmlLib.h           |  54 ++++

 .../Common/AmlLib/CodeGen/AmlCodeGen.c        | 244 ++++++++++++++++++

 2 files changed, 298 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..ea519d1aa8 100644

--- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c

+++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c

@@ -1881,6 +1881,138 @@ 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;

+  ObjectNode = 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);

+    goto exit_handler;

+  }

+

+  // AmlCodeGenReturn() deletes DataNode if error.

+  Status = AmlCodeGenReturn (

+             (AML_NODE_HEADER *)DataNode,

+             ParentNode,

+             &ObjectNode

+             );

+

+  // DataNode is either deleted or added to ObjectNode, set to NULL so we don't

+  // delete it again

+  DataNode = NULL;

+  if (EFI_ERROR (Status)) {

+    ASSERT (0);

+    goto exit_handler;

+  }

+

+  Status = AmlVarListAddTail (

+             (AML_NODE_HANDLE)ObjectNode,

+             (AML_NODE_HANDLE)IntNode

+             );

+  if (EFI_ERROR (Status)) {

+    ASSERT (0);

+    goto exit_handler;

+  }

+

+  if (NewObjectNode != 0) {

+    *NewObjectNode = ObjectNode;

+  }

+

+exit_handler:

+  if (AmlNameString != NULL) {

+    FreePool (AmlNameString);

+  }

+

+  if (IntNode != NULL) {

+    AmlDeleteTree ((AML_NODE_HANDLE)IntNode);

+  }

+

+  if (DataNode != NULL) {

+    AmlDeleteTree ((AML_NODE_HANDLE)DataNode);

+  }

+

+  if (ObjectNode != NULL) {

+    AmlDeleteTree ((AML_NODE_HANDLE)ObjectNode);

+  }

+

+  return Status;

+}

+

 /** AML code generation for a method returning a NameString.

 

   AmlCodeGenMethodRetNameString (

@@ -1989,6 +2121,118 @@ error_handler:

   return Status;

 }

 

+/** 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

+  )

+{

+  EFI_STATUS              Status;

+  AML_OBJECT_NODE_HANDLE  MethodNode;

+

+  if ((MethodNameString == NULL)  ||

+      ((ParentNode == NULL) && (NewObjectNode == NULL)))

+  {

+    ASSERT (0);

+    return EFI_INVALID_PARAMETER;

+  }

+

+  // Create a Method named MethodNameString.

+  Status = AmlCodeGenMethod (

+             MethodNameString,

+             NumArgs,

+             IsSerialized,

+             SyncLevel,

+             NULL,

+             &MethodNode

+             );

+  if (EFI_ERROR (Status)) {

+    ASSERT (0);

+    return Status;

+  }

+

+  // Return ReturnedNameString if provided.

+  if (ReturnedNameString != NULL) {

+    Status = AmlCodeGenReturnNameStringIntegerArgument (

+               ReturnedNameString,

+               IntegerArgument,

+               (AML_NODE_HANDLE)MethodNode,

+               NULL

+               );

+    if (EFI_ERROR (Status)) {

+      ASSERT (0);

+      goto error_handler;

+    }

+  }

+

+  Status = LinkNode (

+             MethodNode,

+             ParentNode,

+             NewObjectNode

+             );

+  if (EFI_ERROR (Status)) {

+    ASSERT (0);

+    goto error_handler;

+  }

+

+  return Status;

+

+error_handler:

+  if (MethodNode != NULL) {

+    AmlDeleteTree ((AML_NODE_HANDLE)MethodNode);

+  }

+

+  return Status;

+}

+

 /** AML code generation for a method returning an Integer.

 

   AmlCodeGenMethodRetInteger (

-- 

2.25.1





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108801): https://edk2.groups.io/g/devel/message/108801
Mute This Topic: https://groups.io/mt/101436335/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v4 2/4] DynamicTablesPkg: Add support for simple method invocation.
Posted by Sami Mujawar 5 months ago
Hi Jeff,

Thank you for this patch.

Please see my response inline marked [SAMI].

Regards,

Sami Mujawar

On 18/09/2023 04:46 pm, 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<jbrasen@nvidia.com>
>
> ---
>
>   .../Include/Library/AmlLib/AmlLib.h           |  54 ++++
>
>   .../Common/AmlLib/CodeGen/AmlCodeGen.c        | 244 ++++++++++++++++++
>
>   2 files changed, 298 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..ea519d1aa8 100644
>
> --- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
>
> +++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
>
> @@ -1881,6 +1881,138 @@ 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;
>
> +  ObjectNode = 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);
>
> +    goto exit_handler;
>
> +  }
>
> +
>
> +  // AmlCodeGenReturn() deletes DataNode if error.
>
> +  Status = AmlCodeGenReturn (
>
> +             (AML_NODE_HEADER *)DataNode,
>
> +             ParentNode,
>
> +             &ObjectNode
>
> +             );
>
> +
>
> +  // DataNode is either deleted or added to ObjectNode, set to NULL so we don't
>
> +  // delete it again
>
> +  DataNode = NULL;
>
> +  if (EFI_ERROR (Status)) {
>
> +    ASSERT (0);
>
> +    goto exit_handler;
>
> +  }
>
> +
>
> +  Status = AmlVarListAddTail (
>
> +             (AML_NODE_HANDLE)ObjectNode,
>
> +             (AML_NODE_HANDLE)IntNode
>
> +             );
>
> +  if (EFI_ERROR (Status)) {
>
> +    ASSERT (0);
>
> +    goto exit_handler;
>
> +  }
>
> +
>
> +  if (NewObjectNode != 0) {
>
> +    *NewObjectNode = ObjectNode;
>
> +  }
>
> +
>
> +exit_handler:
>
> +  if (AmlNameString != NULL) {
>
> +    FreePool (AmlNameString);
>
> +  }
>
> +
>
> +  if (IntNode != NULL) {
>
> +    AmlDeleteTree ((AML_NODE_HANDLE)IntNode);
>
> +  }
>
> +
>
> +  if (DataNode != NULL) {
>
> +    AmlDeleteTree ((AML_NODE_HANDLE)DataNode);
>
> +  }
>
> +
>
> +  if (ObjectNode != NULL) {
>
> +    AmlDeleteTree ((AML_NODE_HANDLE)ObjectNode);
>
> +  }
>
> +
>
> +  return Status;
>
> +}

[SAMI] I think the error handling in the above function can be 
simplified as below:

...

STATIC
EFI_STATUS
EFIAPI
AmlCodeGenReturnNameStringIntegerArgument (
INCONSTCHAR8            *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;
   ObjectNode = NULL;
   Status = ConvertAslNameToAmlName (NameString, &AmlNameString);
if(EFI_ERROR (Status)) {
ASSERT(0);
returnStatus;
   }
   Status = AmlGetNameStringSize (AmlNameString, &AmlNameStringSize);
if(EFI_ERROR (Status)) {
ASSERT(0);
gotoexit_handler;
   }
   Status = AmlCodeGenInteger (Integer, &IntNode);
if(EFI_ERROR (Status)) {
ASSERT(0);
gotoexit_handler;
   }
   Status = AmlCreateDataNode (
EAmlNodeDataTypeNameString,
              (UINT8 *)AmlNameString,
              AmlNameStringSize,
              &DataNode
              );
if(EFI_ERROR (Status)) {
ASSERT(0);
gotoexit_handler1;
   }
// AmlCodeGenReturn() deletes DataNode if error.
   Status = AmlCodeGenReturn (
              (AML_NODE_HEADER *)DataNode,
              ParentNode,
              &ObjectNode
              );
if(EFI_ERROR (Status)) {
ASSERT(0);
gotoexit_handler1;
   }
   Status = AmlVarListAddTail (
              (AML_NODE_HANDLE)ObjectNode,
              (AML_NODE_HANDLE)IntNode
              );
if(EFI_ERROR (Status)) {
// ObjectNode is already attached to ParentNode in AmlCodeGenReturn(),
// so no need to free it here, it will be deleted when deleting the
// ParentNode tree
ASSERT(0);
gotoexit_handler1;
   }
if(NewObjectNode != 0) {
     *NewObjectNode = ObjectNode;
   }
gotoexit_handler;
exit_handler1:
if(IntNode != NULL) {
     AmlDeleteTree ((AML_NODE_HANDLE)IntNode);
   }
exit_handler:
if(AmlNameString != NULL) {
     FreePool (AmlNameString);
   }
returnStatus;
}


Please let me know if you agree with this, and I will make this change 
before merging.

[/SAMI]

> +
>
>   /** AML code generation for a method returning a NameString.
>
>   
>
>     AmlCodeGenMethodRetNameString (
>
> @@ -1989,6 +2121,118 @@ error_handler:
>
>     return Status;
>
>   }
>
>   
>
> +/** 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
>
> +  )
>
> +{
>
> +  EFI_STATUS              Status;
>
> +  AML_OBJECT_NODE_HANDLE  MethodNode;
>
> +
>
> +  if ((MethodNameString == NULL)  ||
>
> +      ((ParentNode == NULL) && (NewObjectNode == NULL)))
>
> +  {
>
> +    ASSERT (0);
>
> +    return EFI_INVALID_PARAMETER;
>
> +  }
>
> +
>
> +  // Create a Method named MethodNameString.
>
> +  Status = AmlCodeGenMethod (
>
> +             MethodNameString,
>
> +             NumArgs,
>
> +             IsSerialized,
>
> +             SyncLevel,
>
> +             NULL,
>
> +             &MethodNode
>
> +             );
>
> +  if (EFI_ERROR (Status)) {
>
> +    ASSERT (0);
>
> +    return Status;
>
> +  }
>
> +
>
> +  // Return ReturnedNameString if provided.
>
> +  if (ReturnedNameString != NULL) {
>
> +    Status = AmlCodeGenReturnNameStringIntegerArgument (
>
> +               ReturnedNameString,
>
> +               IntegerArgument,
>
> +               (AML_NODE_HANDLE)MethodNode,
>
> +               NULL
>
> +               );
>
> +    if (EFI_ERROR (Status)) {
>
> +      ASSERT (0);
>
> +      goto error_handler;
>
> +    }
>
> +  }
>
> +
>
> +  Status = LinkNode (
>
> +             MethodNode,
>
> +             ParentNode,
>
> +             NewObjectNode
>
> +             );
>
> +  if (EFI_ERROR (Status)) {
>
> +    ASSERT (0);
>
> +    goto error_handler;
>
> +  }
>
> +
>
> +  return Status;
>
> +
>
> +error_handler:
>
> +  if (MethodNode != NULL) {
>
> +    AmlDeleteTree ((AML_NODE_HANDLE)MethodNode);
>
> +  }
>
> +
>
> +  return Status;
>
> +}
>
> +
>
>   /** AML code generation for a method returning an Integer.
>
>   
>
>     AmlCodeGenMethodRetInteger (
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108955): https://edk2.groups.io/g/devel/message/108955
Mute This Topic: https://groups.io/mt/101436335/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v4 2/4] DynamicTablesPkg: Add support for simple method invocation.
Posted by Jeff Brasen via groups.io 5 months ago
Only thing I see is if AmlCodeGenInteger fails we don’t delete DataNode right?

From: Sami Mujawar <sami.mujawar@arm.com>
Sent: Thursday, September 21, 2023 10:49 AM
To: Jeff Brasen <jbrasen@nvidia.com>; devel@edk2.groups.io
Cc: pierre.gondois@arm.com; Swatisri Kantamsetti <swatisrik@nvidia.com>; Ashish Singhal <ashishsingha@nvidia.com>; nd@arm.com
Subject: Re: [PATCH v4 2/4] DynamicTablesPkg: Add support for simple method invocation.

External email: Use caution opening links or attachments


Hi Jeff,

Thank you for this patch.

Please see my response inline marked [SAMI].

Regards,

Sami Mujawar
On 18/09/2023 04:46 pm, 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 <jbrasen@nvidia.com><mailto:jbrasen@nvidia.com>



---



 .../Include/Library/AmlLib/AmlLib.h           |  54 ++++



 .../Common/AmlLib/CodeGen/AmlCodeGen.c        | 244 ++++++++++++++++++



 2 files changed, 298 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..ea519d1aa8 100644



--- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c



+++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c



@@ -1881,6 +1881,138 @@ 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;



+  ObjectNode = 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);



+    goto exit_handler;



+  }



+



+  // AmlCodeGenReturn() deletes DataNode if error.



+  Status = AmlCodeGenReturn (



+             (AML_NODE_HEADER *)DataNode,



+             ParentNode,



+             &ObjectNode



+             );



+



+  // DataNode is either deleted or added to ObjectNode, set to NULL so we don't



+  // delete it again



+  DataNode = NULL;



+  if (EFI_ERROR (Status)) {



+    ASSERT (0);



+    goto exit_handler;



+  }



+



+  Status = AmlVarListAddTail (



+             (AML_NODE_HANDLE)ObjectNode,



+             (AML_NODE_HANDLE)IntNode



+             );



+  if (EFI_ERROR (Status)) {



+    ASSERT (0);



+    goto exit_handler;



+  }



+



+  if (NewObjectNode != 0) {



+    *NewObjectNode = ObjectNode;



+  }



+



+exit_handler:



+  if (AmlNameString != NULL) {



+    FreePool (AmlNameString);



+  }



+



+  if (IntNode != NULL) {



+    AmlDeleteTree ((AML_NODE_HANDLE)IntNode);



+  }



+



+  if (DataNode != NULL) {



+    AmlDeleteTree ((AML_NODE_HANDLE)DataNode);



+  }



+



+  if (ObjectNode != NULL) {



+    AmlDeleteTree ((AML_NODE_HANDLE)ObjectNode);



+  }



+



+  return Status;



+}

[SAMI] I think the error handling in the above function can be simplified as below:

...

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;

  ObjectNode = 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 = AmlCodeGenInteger (Integer, &IntNode);

  if (EFI_ERROR (Status)) {

    ASSERT (0);

    goto exit_handler;

  }

  Status = AmlCreateDataNode (

             EAmlNodeDataTypeNameString,

             (UINT8 *)AmlNameString,

             AmlNameStringSize,

             &DataNode

             );

  if (EFI_ERROR (Status)) {

    ASSERT (0);

    goto exit_handler1;

  }

  // AmlCodeGenReturn() deletes DataNode if error.

  Status = AmlCodeGenReturn (

             (AML_NODE_HEADER *)DataNode,

             ParentNode,

             &ObjectNode

             );

  if (EFI_ERROR (Status)) {

    ASSERT (0);

    goto exit_handler1;

  }

  Status = AmlVarListAddTail (

             (AML_NODE_HANDLE)ObjectNode,

             (AML_NODE_HANDLE)IntNode

             );

  if (EFI_ERROR (Status)) {

    // ObjectNode is already attached to ParentNode in AmlCodeGenReturn(),

    // so no need to free it here, it will be deleted when deleting the

    // ParentNode tree

    ASSERT (0);

    goto exit_handler1;

  }

  if (NewObjectNode !=
0) {

    *NewObjectNode = ObjectNode;

  }

  goto exit_handler;

exit_handler1:

  if (IntNode !=
NULL) {

    AmlDeleteTree ((AML_NODE_HANDLE)IntNode);

  }

exit_handler:

  if (AmlNameString !=
NULL) {

    FreePool (AmlNameString);

  }

  return Status;

}




Please let me know if you agree with this, and I will make this change before merging.

[/SAMI]



+



 /** AML code generation for a method returning a NameString.







   AmlCodeGenMethodRetNameString (



@@ -1989,6 +2121,118 @@ error_handler:



   return Status;



 }







+/** 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



+  )



+{



+  EFI_STATUS              Status;



+  AML_OBJECT_NODE_HANDLE  MethodNode;



+



+  if ((MethodNameString == NULL)  ||



+      ((ParentNode == NULL) && (NewObjectNode == NULL)))



+  {



+    ASSERT (0);



+    return EFI_INVALID_PARAMETER;



+  }



+



+  // Create a Method named MethodNameString.



+  Status = AmlCodeGenMethod (



+             MethodNameString,



+             NumArgs,



+             IsSerialized,



+             SyncLevel,



+             NULL,



+             &MethodNode



+             );



+  if (EFI_ERROR (Status)) {



+    ASSERT (0);



+    return Status;



+  }



+



+  // Return ReturnedNameString if provided.



+  if (ReturnedNameString != NULL) {



+    Status = AmlCodeGenReturnNameStringIntegerArgument (



+               ReturnedNameString,



+               IntegerArgument,



+               (AML_NODE_HANDLE)MethodNode,



+               NULL



+               );



+    if (EFI_ERROR (Status)) {



+      ASSERT (0);



+      goto error_handler;



+    }



+  }



+



+  Status = LinkNode (



+             MethodNode,



+             ParentNode,



+             NewObjectNode



+             );



+  if (EFI_ERROR (Status)) {



+    ASSERT (0);



+    goto error_handler;



+  }



+



+  return Status;



+



+error_handler:



+  if (MethodNode != NULL) {



+    AmlDeleteTree ((AML_NODE_HANDLE)MethodNode);



+  }



+



+  return Status;



+}



+



 /** AML code generation for a method returning an Integer.







   AmlCodeGenMethodRetInteger (




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108961): https://edk2.groups.io/g/devel/message/108961
Mute This Topic: https://groups.io/mt/101436335/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v4 2/4] DynamicTablesPkg: Add support for simple method invocation.
Posted by Sami Mujawar 5 months ago
Hi Jeff,

Yes, I recorded the integer node and data node creation.

Regards,

Sami Mujawar


________________________________
From: Jeff Brasen <jbrasen@nvidia.com>
Sent: 21 September 2023 18:38
To: Sami Mujawar <Sami.Mujawar@arm.com>; devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Pierre Gondois <Pierre.Gondois@arm.com>; Swatisri Kantamsetti <swatisrik@nvidia.com>; Ashish Singhal <ashishsingha@nvidia.com>; nd <nd@arm.com>
Subject: RE: [PATCH v4 2/4] DynamicTablesPkg: Add support for simple method invocation.


Only thing I see is if AmlCodeGenInteger fails we don’t delete DataNode right?



From: Sami Mujawar <sami.mujawar@arm.com>
Sent: Thursday, September 21, 2023 10:49 AM
To: Jeff Brasen <jbrasen@nvidia.com>; devel@edk2.groups.io
Cc: pierre.gondois@arm.com; Swatisri Kantamsetti <swatisrik@nvidia.com>; Ashish Singhal <ashishsingha@nvidia.com>; nd@arm.com
Subject: Re: [PATCH v4 2/4] DynamicTablesPkg: Add support for simple method invocation.



External email: Use caution opening links or attachments



Hi Jeff,

Thank you for this patch.

Please see my response inline marked [SAMI].

Regards,

Sami Mujawar

On 18/09/2023 04:46 pm, 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 <jbrasen@nvidia.com><mailto:jbrasen@nvidia.com>



---



 .../Include/Library/AmlLib/AmlLib.h           |  54 ++++



 .../Common/AmlLib/CodeGen/AmlCodeGen.c        | 244 ++++++++++++++++++



 2 files changed, 298 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..ea519d1aa8 100644



--- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c



+++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c



@@ -1881,6 +1881,138 @@ 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;



+  ObjectNode = 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);



+    goto exit_handler;



+  }



+



+  // AmlCodeGenReturn() deletes DataNode if error.



+  Status = AmlCodeGenReturn (



+             (AML_NODE_HEADER *)DataNode,



+             ParentNode,



+             &ObjectNode



+             );



+



+  // DataNode is either deleted or added to ObjectNode, set to NULL so we don't



+  // delete it again



+  DataNode = NULL;



+  if (EFI_ERROR (Status)) {



+    ASSERT (0);



+    goto exit_handler;



+  }



+



+  Status = AmlVarListAddTail (



+             (AML_NODE_HANDLE)ObjectNode,



+             (AML_NODE_HANDLE)IntNode



+             );



+  if (EFI_ERROR (Status)) {



+    ASSERT (0);



+    goto exit_handler;



+  }



+



+  if (NewObjectNode != 0) {



+    *NewObjectNode = ObjectNode;



+  }



+



+exit_handler:



+  if (AmlNameString != NULL) {



+    FreePool (AmlNameString);



+  }



+



+  if (IntNode != NULL) {



+    AmlDeleteTree ((AML_NODE_HANDLE)IntNode);



+  }



+



+  if (DataNode != NULL) {



+    AmlDeleteTree ((AML_NODE_HANDLE)DataNode);



+  }



+



+  if (ObjectNode != NULL) {



+    AmlDeleteTree ((AML_NODE_HANDLE)ObjectNode);



+  }



+



+  return Status;



+}

[SAMI] I think the error handling in the above function can be simplified as below:

...



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;



  ObjectNode = 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 = AmlCodeGenInteger (Integer, &IntNode);



  if (EFI_ERROR (Status)) {



    ASSERT (0);



    goto exit_handler;



  }



  Status = AmlCreateDataNode (



             EAmlNodeDataTypeNameString,



             (UINT8 *)AmlNameString,



             AmlNameStringSize,



             &DataNode



             );



  if (EFI_ERROR (Status)) {



    ASSERT (0);



    goto exit_handler1;



  }



  // AmlCodeGenReturn() deletes DataNode if error.



  Status = AmlCodeGenReturn (



             (AML_NODE_HEADER *)DataNode,



             ParentNode,



             &ObjectNode



             );



  if (EFI_ERROR (Status)) {



    ASSERT (0);



    goto exit_handler1;



  }



  Status = AmlVarListAddTail (



             (AML_NODE_HANDLE)ObjectNode,



             (AML_NODE_HANDLE)IntNode



             );



  if (EFI_ERROR (Status)) {



    // ObjectNode is already attached to ParentNode in AmlCodeGenReturn(),



    // so no need to free it here, it will be deleted when deleting the



    // ParentNode tree



    ASSERT (0);



    goto exit_handler1;



  }



  if (NewObjectNode !=

0) {



    *NewObjectNode = ObjectNode;



  }



  goto exit_handler;



exit_handler1:



  if (IntNode !=

NULL) {



    AmlDeleteTree ((AML_NODE_HANDLE)IntNode);



  }



exit_handler:



  if (AmlNameString !=

NULL) {



    FreePool (AmlNameString);



  }



  return Status;



}





Please let me know if you agree with this, and I will make this change before merging.

[/SAMI]



+



 /** AML code generation for a method returning a NameString.







   AmlCodeGenMethodRetNameString (



@@ -1989,6 +2121,118 @@ error_handler:



   return Status;



 }







+/** 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



+  )



+{



+  EFI_STATUS              Status;



+  AML_OBJECT_NODE_HANDLE  MethodNode;



+



+  if ((MethodNameString == NULL)  ||



+      ((ParentNode == NULL) && (NewObjectNode == NULL)))



+  {



+    ASSERT (0);



+    return EFI_INVALID_PARAMETER;



+  }



+



+  // Create a Method named MethodNameString.



+  Status = AmlCodeGenMethod (



+             MethodNameString,



+             NumArgs,



+             IsSerialized,



+             SyncLevel,



+             NULL,



+             &MethodNode



+             );



+  if (EFI_ERROR (Status)) {



+    ASSERT (0);



+    return Status;



+  }



+



+  // Return ReturnedNameString if provided.



+  if (ReturnedNameString != NULL) {



+    Status = AmlCodeGenReturnNameStringIntegerArgument (



+               ReturnedNameString,



+               IntegerArgument,



+               (AML_NODE_HANDLE)MethodNode,



+               NULL



+               );



+    if (EFI_ERROR (Status)) {



+      ASSERT (0);



+      goto error_handler;



+    }



+  }



+



+  Status = LinkNode (



+             MethodNode,



+             ParentNode,



+             NewObjectNode



+             );



+  if (EFI_ERROR (Status)) {



+    ASSERT (0);



+    goto error_handler;



+  }



+



+  return Status;



+



+error_handler:



+  if (MethodNode != NULL) {



+    AmlDeleteTree ((AML_NODE_HANDLE)MethodNode);



+  }



+



+  return Status;



+}



+



 /** AML code generation for a method returning an Integer.







   AmlCodeGenMethodRetInteger (




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108968): https://edk2.groups.io/g/devel/message/108968
Mute This Topic: https://groups.io/mt/101436335/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v4 2/4] DynamicTablesPkg: Add support for simple method invocation.
Posted by Jeff Brasen via groups.io 5 months ago
I see you swapped the order of the functions, that looks good and avoids special handling for that case. That looks good to me.

Thanks,

Jeff
From: Sami Mujawar <Sami.Mujawar@arm.com>
Sent: Thursday, September 21, 2023 2:07 PM
To: Jeff Brasen <jbrasen@nvidia.com>; devel@edk2.groups.io
Cc: Pierre Gondois <Pierre.Gondois@arm.com>; Swatisri Kantamsetti <swatisrik@nvidia.com>; Ashish Singhal <ashishsingha@nvidia.com>; nd <nd@arm.com>
Subject: Re: [PATCH v4 2/4] DynamicTablesPkg: Add support for simple method invocation.

External email: Use caution opening links or attachments

Hi Jeff,

Yes, I recorded the integer node and data node creation.

Regards,

Sami Mujawar


________________________________
From: Jeff Brasen <jbrasen@nvidia.com<mailto:jbrasen@nvidia.com>>
Sent: 21 September 2023 18:38
To: Sami Mujawar <Sami.Mujawar@arm.com<mailto:Sami.Mujawar@arm.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>
Cc: Pierre Gondois <Pierre.Gondois@arm.com<mailto:Pierre.Gondois@arm.com>>; Swatisri Kantamsetti <swatisrik@nvidia.com<mailto:swatisrik@nvidia.com>>; Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; nd <nd@arm.com<mailto:nd@arm.com>>
Subject: RE: [PATCH v4 2/4] DynamicTablesPkg: Add support for simple method invocation.


Only thing I see is if AmlCodeGenInteger fails we don't delete DataNode right?



From: Sami Mujawar <sami.mujawar@arm.com<mailto:sami.mujawar@arm.com>>
Sent: Thursday, September 21, 2023 10:49 AM
To: Jeff Brasen <jbrasen@nvidia.com<mailto:jbrasen@nvidia.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Cc: pierre.gondois@arm.com<mailto:pierre.gondois@arm.com>; Swatisri Kantamsetti <swatisrik@nvidia.com<mailto:swatisrik@nvidia.com>>; Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; nd@arm.com<mailto:nd@arm.com>
Subject: Re: [PATCH v4 2/4] DynamicTablesPkg: Add support for simple method invocation.



External email: Use caution opening links or attachments



Hi Jeff,

Thank you for this patch.

Please see my response inline marked [SAMI].

Regards,

Sami Mujawar

On 18/09/2023 04:46 pm, 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 <jbrasen@nvidia.com><mailto:jbrasen@nvidia.com>



---



 .../Include/Library/AmlLib/AmlLib.h           |  54 ++++



 .../Common/AmlLib/CodeGen/AmlCodeGen.c        | 244 ++++++++++++++++++



 2 files changed, 298 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..ea519d1aa8 100644



--- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c



+++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c



@@ -1881,6 +1881,138 @@ 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;



+  ObjectNode = 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);



+    goto exit_handler;



+  }



+



+  // AmlCodeGenReturn() deletes DataNode if error.



+  Status = AmlCodeGenReturn (



+             (AML_NODE_HEADER *)DataNode,



+             ParentNode,



+             &ObjectNode



+             );



+



+  // DataNode is either deleted or added to ObjectNode, set to NULL so we don't



+  // delete it again



+  DataNode = NULL;



+  if (EFI_ERROR (Status)) {



+    ASSERT (0);



+    goto exit_handler;



+  }



+



+  Status = AmlVarListAddTail (



+             (AML_NODE_HANDLE)ObjectNode,



+             (AML_NODE_HANDLE)IntNode



+             );



+  if (EFI_ERROR (Status)) {



+    ASSERT (0);



+    goto exit_handler;



+  }



+



+  if (NewObjectNode != 0) {



+    *NewObjectNode = ObjectNode;



+  }



+



+exit_handler:



+  if (AmlNameString != NULL) {



+    FreePool (AmlNameString);



+  }



+



+  if (IntNode != NULL) {



+    AmlDeleteTree ((AML_NODE_HANDLE)IntNode);



+  }



+



+  if (DataNode != NULL) {



+    AmlDeleteTree ((AML_NODE_HANDLE)DataNode);



+  }



+



+  if (ObjectNode != NULL) {



+    AmlDeleteTree ((AML_NODE_HANDLE)ObjectNode);



+  }



+



+  return Status;



+}

[SAMI] I think the error handling in the above function can be simplified as below:

...



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;



  ObjectNode = 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 = AmlCodeGenInteger (Integer, &IntNode);



  if (EFI_ERROR (Status)) {



    ASSERT (0);



    goto exit_handler;



  }



  Status = AmlCreateDataNode (



             EAmlNodeDataTypeNameString,



             (UINT8 *)AmlNameString,



             AmlNameStringSize,



             &DataNode



             );



  if (EFI_ERROR (Status)) {



    ASSERT (0);



    goto exit_handler1;



  }



  // AmlCodeGenReturn() deletes DataNode if error.



  Status = AmlCodeGenReturn (



             (AML_NODE_HEADER *)DataNode,



             ParentNode,



             &ObjectNode



             );



  if (EFI_ERROR (Status)) {



    ASSERT (0);



    goto exit_handler1;



  }



  Status = AmlVarListAddTail (



             (AML_NODE_HANDLE)ObjectNode,



             (AML_NODE_HANDLE)IntNode



             );



  if (EFI_ERROR (Status)) {



    // ObjectNode is already attached to ParentNode in AmlCodeGenReturn(),



    // so no need to free it here, it will be deleted when deleting the



    // ParentNode tree



    ASSERT (0);



    goto exit_handler1;



  }



  if (NewObjectNode !=

0) {



    *NewObjectNode = ObjectNode;



  }



  goto exit_handler;



exit_handler1:



  if (IntNode !=

NULL) {



    AmlDeleteTree ((AML_NODE_HANDLE)IntNode);



  }



exit_handler:



  if (AmlNameString !=

NULL) {



    FreePool (AmlNameString);



  }



  return Status;



}





Please let me know if you agree with this, and I will make this change before merging.

[/SAMI]



+



 /** AML code generation for a method returning a NameString.







   AmlCodeGenMethodRetNameString (



@@ -1989,6 +2121,118 @@ error_handler:



   return Status;



 }







+/** 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



+  )



+{



+  EFI_STATUS              Status;



+  AML_OBJECT_NODE_HANDLE  MethodNode;



+



+  if ((MethodNameString == NULL)  ||



+      ((ParentNode == NULL) && (NewObjectNode == NULL)))



+  {



+    ASSERT (0);



+    return EFI_INVALID_PARAMETER;



+  }



+



+  // Create a Method named MethodNameString.



+  Status = AmlCodeGenMethod (



+             MethodNameString,



+             NumArgs,



+             IsSerialized,



+             SyncLevel,



+             NULL,



+             &MethodNode



+             );



+  if (EFI_ERROR (Status)) {



+    ASSERT (0);



+    return Status;



+  }



+



+  // Return ReturnedNameString if provided.



+  if (ReturnedNameString != NULL) {



+    Status = AmlCodeGenReturnNameStringIntegerArgument (



+               ReturnedNameString,



+               IntegerArgument,



+               (AML_NODE_HANDLE)MethodNode,



+               NULL



+               );



+    if (EFI_ERROR (Status)) {



+      ASSERT (0);



+      goto error_handler;



+    }



+  }



+



+  Status = LinkNode (



+             MethodNode,



+             ParentNode,



+             NewObjectNode



+             );



+  if (EFI_ERROR (Status)) {



+    ASSERT (0);



+    goto error_handler;



+  }



+



+  return Status;



+



+error_handler:



+  if (MethodNode != NULL) {



+    AmlDeleteTree ((AML_NODE_HANDLE)MethodNode);



+  }



+



+  return Status;



+}



+



 /** AML code generation for a method returning an Integer.







   AmlCodeGenMethodRetInteger (




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108972): https://edk2.groups.io/g/devel/message/108972
Mute This Topic: https://groups.io/mt/101436335/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v4 2/4] DynamicTablesPkg: Add support for simple method invocation.
Posted by Attar, AbdulLateef (Abdul Lateef) via groups.io 5 months ago
[AMD Official Use Only - General]

Hi Jeff, Sami,
                I'm not reviewer, just providing the opinion.
How about making it generic(generic to integer argument)  instead of single integer argument.
    IN        UINT64                  IntegerArgument
    IN         UINT64                   *IntegerArgumentArray.
Create the list(by making use of AmlVarListAddTail) of data object depends on IntergerArgument value.

Also, I think if data in data node is AML_ARG0(0x68), AML_ARG1...., then it will considered as Arg0, Arg1, ....etc.

Thanks
AbduL


From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Jeff Brasen via groups.io
Sent: Friday, September 22, 2023 2:19 AM
To: Sami Mujawar <Sami.Mujawar@arm.com>; devel@edk2.groups.io
Cc: Pierre Gondois <Pierre.Gondois@arm.com>; Swatisri Kantamsetti <swatisrik@nvidia.com>; Ashish Singhal <ashishsingha@nvidia.com>; nd <nd@arm.com>
Subject: Re: [edk2-devel] [PATCH v4 2/4] DynamicTablesPkg: Add support for simple method invocation.

Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.

I see you swapped the order of the functions, that looks good and avoids special handling for that case. That looks good to me.

Thanks,

Jeff
From: Sami Mujawar <Sami.Mujawar@arm.com<mailto:Sami.Mujawar@arm.com>>
Sent: Thursday, September 21, 2023 2:07 PM
To: Jeff Brasen <jbrasen@nvidia.com<mailto:jbrasen@nvidia.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Cc: Pierre Gondois <Pierre.Gondois@arm.com<mailto:Pierre.Gondois@arm.com>>; Swatisri Kantamsetti <swatisrik@nvidia.com<mailto:swatisrik@nvidia.com>>; Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; nd <nd@arm.com<mailto:nd@arm.com>>
Subject: Re: [PATCH v4 2/4] DynamicTablesPkg: Add support for simple method invocation.

External email: Use caution opening links or attachments

Hi Jeff,

Yes, I recorded the integer node and data node creation.

Regards,

Sami Mujawar


________________________________
From: Jeff Brasen <jbrasen@nvidia.com<mailto:jbrasen@nvidia.com>>
Sent: 21 September 2023 18:38
To: Sami Mujawar <Sami.Mujawar@arm.com<mailto:Sami.Mujawar@arm.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>
Cc: Pierre Gondois <Pierre.Gondois@arm.com<mailto:Pierre.Gondois@arm.com>>; Swatisri Kantamsetti <swatisrik@nvidia.com<mailto:swatisrik@nvidia.com>>; Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; nd <nd@arm.com<mailto:nd@arm.com>>
Subject: RE: [PATCH v4 2/4] DynamicTablesPkg: Add support for simple method invocation.


Only thing I see is if AmlCodeGenInteger fails we don't delete DataNode right?



From: Sami Mujawar <sami.mujawar@arm.com<mailto:sami.mujawar@arm.com>>
Sent: Thursday, September 21, 2023 10:49 AM
To: Jeff Brasen <jbrasen@nvidia.com<mailto:jbrasen@nvidia.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Cc: pierre.gondois@arm.com<mailto:pierre.gondois@arm.com>; Swatisri Kantamsetti <swatisrik@nvidia.com<mailto:swatisrik@nvidia.com>>; Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; nd@arm.com<mailto:nd@arm.com>
Subject: Re: [PATCH v4 2/4] DynamicTablesPkg: Add support for simple method invocation.



External email: Use caution opening links or attachments



Hi Jeff,

Thank you for this patch.

Please see my response inline marked [SAMI].

Regards,

Sami Mujawar

On 18/09/2023 04:46 pm, 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 <jbrasen@nvidia.com><mailto:jbrasen@nvidia.com>



---



 .../Include/Library/AmlLib/AmlLib.h           |  54 ++++



 .../Common/AmlLib/CodeGen/AmlCodeGen.c        | 244 ++++++++++++++++++



 2 files changed, 298 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..ea519d1aa8 100644



--- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c



+++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c



@@ -1881,6 +1881,138 @@ 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;



+  ObjectNode = 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);



+    goto exit_handler;



+  }



+



+  // AmlCodeGenReturn() deletes DataNode if error.



+  Status = AmlCodeGenReturn (



+             (AML_NODE_HEADER *)DataNode,



+             ParentNode,



+             &ObjectNode



+             );



+



+  // DataNode is either deleted or added to ObjectNode, set to NULL so we don't



+  // delete it again



+  DataNode = NULL;



+  if (EFI_ERROR (Status)) {



+    ASSERT (0);



+    goto exit_handler;



+  }



+



+  Status = AmlVarListAddTail (



+             (AML_NODE_HANDLE)ObjectNode,



+             (AML_NODE_HANDLE)IntNode



+             );



+  if (EFI_ERROR (Status)) {



+    ASSERT (0);



+    goto exit_handler;



+  }



+



+  if (NewObjectNode != 0) {



+    *NewObjectNode = ObjectNode;



+  }



+



+exit_handler:



+  if (AmlNameString != NULL) {



+    FreePool (AmlNameString);



+  }



+



+  if (IntNode != NULL) {



+    AmlDeleteTree ((AML_NODE_HANDLE)IntNode);



+  }



+



+  if (DataNode != NULL) {



+    AmlDeleteTree ((AML_NODE_HANDLE)DataNode);



+  }



+



+  if (ObjectNode != NULL) {



+    AmlDeleteTree ((AML_NODE_HANDLE)ObjectNode);



+  }



+



+  return Status;



+}

[SAMI] I think the error handling in the above function can be simplified as below:

...



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;



  ObjectNode = 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 = AmlCodeGenInteger (Integer, &IntNode);



  if (EFI_ERROR (Status)) {



    ASSERT (0);



    goto exit_handler;



  }



  Status = AmlCreateDataNode (



             EAmlNodeDataTypeNameString,



             (UINT8 *)AmlNameString,



             AmlNameStringSize,



             &DataNode



             );



  if (EFI_ERROR (Status)) {



    ASSERT (0);



    goto exit_handler1;



  }



  // AmlCodeGenReturn() deletes DataNode if error.



  Status = AmlCodeGenReturn (



             (AML_NODE_HEADER *)DataNode,



             ParentNode,



             &ObjectNode



             );



  if (EFI_ERROR (Status)) {



    ASSERT (0);



    goto exit_handler1;



  }



  Status = AmlVarListAddTail (



             (AML_NODE_HANDLE)ObjectNode,



             (AML_NODE_HANDLE)IntNode



             );



  if (EFI_ERROR (Status)) {



    // ObjectNode is already attached to ParentNode in AmlCodeGenReturn(),



    // so no need to free it here, it will be deleted when deleting the



    // ParentNode tree



    ASSERT (0);



    goto exit_handler1;



  }



  if (NewObjectNode !=

0) {



    *NewObjectNode = ObjectNode;



  }



  goto exit_handler;



exit_handler1:



  if (IntNode !=

NULL) {



    AmlDeleteTree ((AML_NODE_HANDLE)IntNode);



  }



exit_handler:



  if (AmlNameString !=

NULL) {



    FreePool (AmlNameString);



  }



  return Status;



}





Please let me know if you agree with this, and I will make this change before merging.

[/SAMI]



+



 /** AML code generation for a method returning a NameString.







   AmlCodeGenMethodRetNameString (



@@ -1989,6 +2121,118 @@ error_handler:



   return Status;



 }







+/** 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



+  )



+{



+  EFI_STATUS              Status;



+  AML_OBJECT_NODE_HANDLE  MethodNode;



+



+  if ((MethodNameString == NULL)  ||



+      ((ParentNode == NULL) && (NewObjectNode == NULL)))



+  {



+    ASSERT (0);



+    return EFI_INVALID_PARAMETER;



+  }



+



+  // Create a Method named MethodNameString.



+  Status = AmlCodeGenMethod (



+             MethodNameString,



+             NumArgs,



+             IsSerialized,



+             SyncLevel,



+             NULL,



+             &MethodNode



+             );



+  if (EFI_ERROR (Status)) {



+    ASSERT (0);



+    return Status;



+  }



+



+  // Return ReturnedNameString if provided.



+  if (ReturnedNameString != NULL) {



+    Status = AmlCodeGenReturnNameStringIntegerArgument (



+               ReturnedNameString,



+               IntegerArgument,



+               (AML_NODE_HANDLE)MethodNode,



+               NULL



+               );



+    if (EFI_ERROR (Status)) {



+      ASSERT (0);



+      goto error_handler;



+    }



+  }



+



+  Status = LinkNode (



+             MethodNode,



+             ParentNode,



+             NewObjectNode



+             );



+  if (EFI_ERROR (Status)) {



+    ASSERT (0);



+    goto error_handler;



+  }



+



+  return Status;



+



+error_handler:



+  if (MethodNode != NULL) {



+    AmlDeleteTree ((AML_NODE_HANDLE)MethodNode);



+  }



+



+  return Status;



+}



+



 /** AML code generation for a method returning an Integer.







   AmlCodeGenMethodRetInteger (





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109024): https://edk2.groups.io/g/devel/message/109024
Mute This Topic: https://groups.io/mt/101436335/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v4 2/4] DynamicTablesPkg: Add support for simple method invocation.
Posted by PierreGondois 5 months ago
Hello Abdul,

On 9/23/23 06:22, Attar, AbdulLateef (Abdul Lateef) wrote:
> [AMD Official Use Only - General]
> 
> 
> Hi Jeff, Sami,
> 
>                  I’m not reviewer, just providing the opinion.
> 
> How about making it generic(generic to integer argument)  instead of single integer argument.
> 
>      IN        UINT64                  IntegerArgument
> 
>      IN         UINT64 **IntegerArgumentArray.*
> 
> Create the list(by making use of AmlVarListAddTail) of data object depends on IntergerArgument value.
> 
> Also, I think if data in data node is AML_ARG0(0x68), AML_ARG1…., then it will considered as Arg0, Arg1, ….etc.

Just to be sure, you would like the method generated by AmlCodeGenMethodRetNameStringIntegerArgument()
to take multiple integer argument ? This would allow to generate a function taking N input arguments like:

Method(MET0, N, Serialized, 3) {
   Return (MET1 (1, 2, ..., N))
}

It is true AmlCodeGenMethodRetNameStringIntegerArgument() takes a NumArgs parameter,
but currently NumArgs can only equal 1.

Is there a specific use case that you are looking to solve with this solution ?

Regards,
Pierre

> 
> Thanks
> 
> AbduL
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109123): https://edk2.groups.io/g/devel/message/109123
Mute This Topic: https://groups.io/mt/101436335/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-