[edk2-devel] [Resend PATCH v4 0/4] DynamicTablesPkg: Adds WordIO and method invocation ability

[edk2-devel] [Resend PATCH v4 0/4] DynamicTablesPkg: Adds WordIO and method invocation ability

[Abdul Lateef Attar via groups.io]

From: Abdul Lateef Attar <AbdulLateef.Attar@amd.com>

PR: https://github.com/tianocore/edk2/pull/5157

Resending patch with CI failure fix and added example.

V4: delta changes
  Address Pierre's review comments.
  Merged patch 4 & patch 5.
  Also added parameter logic suggested by Pierre.
V3: delta changes
  CI fix and error handling for 
    DynamicTablesPkg: Adds API to generate a method with ArgN.
  Added new patch "DynamicTablesPkg: Adds wrapper API AmlCodeGenMethodInvokeMethodArgn"
V2:
  Added new API to generate a method which
  invokes another method with argument.
V1:
Code changes to
Add API to generate word I/O ranges.
Correct the parameter to generate word bus number.
Corrects the function pointer typedef

Cc: Pierre Gondois <pierre.gondois@arm.com>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Cc: Abdul Lateef Attar <AbdulLateef.Attar@amd.com>

Abdul Lateef Attar (4):
  DynamicTablesPkg: AML Code generation for word I/O ranges
  DynamicTablesPkg: Corrects AmlCodeGenRdWordBusNumber parameters
  DynamicTablesPkg: Corrects function pointer typedef of
    AML_PARSE_FUNCTION
  DynamicTablesPkg: AML Code generation to invoke a method

 .../Include/Library/AmlLib/AmlLib.h           | 187 +++++++++++++-
 .../Common/AmlLib/CodeGen/AmlCodeGen.c        | 236 +++++++++++++++++-
 .../AmlLib/CodeGen/AmlResourceDataCodeGen.c   |  98 +++++++-
 .../Library/Common/AmlLib/Parser/AmlParser.c  |   3 +-
 4 files changed, 511 insertions(+), 13 deletions(-)

-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112668): https://edk2.groups.io/g/devel/message/112668
Mute This Topic: https://groups.io/mt/103256858/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [Resend PATCH v4 1/4] DynamicTablesPkg: AML Code generation for word I/O ranges

[Abdul Lateef Attar via groups.io]

From: Abdul Lateef Attar <AbdulLateef.Attar@amd.com>

Add helper functions to generate AML resource data
for word I/O.

Cc: Pierre Gondois <pierre.gondois@arm.com>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Signed-off-by: Abdul Lateef Attar <AbdulLateef.Attar@amd.com>
Reviewed-by: Pierre Gondois <pierre.gondois@arm.com>
---
 .../Include/Library/AmlLib/AmlLib.h           | 65 ++++++++++++++
 .../AmlLib/CodeGen/AmlResourceDataCodeGen.c   | 88 +++++++++++++++++++
 2 files changed, 153 insertions(+)

diff --git a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
index 71e8539b30..5e340b94ce 100644
--- a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
+++ b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
@@ -2,6 +2,7 @@
   AML Lib.
 
   Copyright (c) 2019 - 2021, Arm Limited. All rights reserved.<BR>
+  Copyright (C) 2023 Advanced Micro Devices, Inc. All rights reserved.<BR>
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 **/
@@ -724,6 +725,70 @@ AmlCodeGenRdWordBusNumber (
   OUT       AML_DATA_NODE_HANDLE    *NewRdNode  OPTIONAL
   );
 
+/** Code generation for the "WordIO ()" ASL function.
+
+  The Resource Data effectively created is a Word Address Space Resource
+  Data. Cf ACPI 6.5:
+   - s6.4.3.5.3 "Word Address Space Descriptor".
+
+  The created resource data node can be:
+   - appended to the list of resource data elements of the NameOpNode.
+     In such case NameOpNode must be defined by a the "Name ()" ASL statement
+     and initially contain a "ResourceTemplate ()".
+   - returned through the NewRdNode parameter.
+
+  @param [in]  IsResourceConsumer   ResourceUsage parameter.
+  @param [in]  IsMinFixed           Minimum address is fixed.
+  @param [in]  IsMaxFixed           Maximum address is fixed.
+  @param [in]  IsPosDecode          Decode parameter
+  @param [in]  IsaRanges            Possible values are:
+                                     0-Reserved
+                                     1-NonISAOnly
+                                     2-ISAOnly
+                                     3-EntireRange
+  @param [in]  AddressGranularity   Address granularity.
+  @param [in]  AddressMinimum       Minimum address.
+  @param [in]  AddressMaximum       Maximum address.
+  @param [in]  AddressTranslation   Address translation.
+  @param [in]  RangeLength          Range length.
+  @param [in]  ResourceSourceIndex  Resource Source index.
+                                    Not supported. Must be 0.
+  @param [in]  ResourceSource       Resource Source.
+                                    Not supported. Must be NULL.
+  @param [in]  IsDenseTranslation   TranslationDensity parameter.
+  @param [in]  IsTypeStatic         TranslationType parameter.
+  @param [in]  NameOpNode           NameOp object node defining a named object.
+                                    If provided, append the new resource data
+                                    node to the list of resource data elements
+                                    of this node.
+  @param [out] NewRdNode            If provided and success,
+                                    contain the created node.
+
+  @retval EFI_SUCCESS             The function completed successfully.
+  @retval EFI_INVALID_PARAMETER   Invalid parameter.
+  @retval EFI_OUT_OF_RESOURCES    Could not allocate memory.
+**/
+EFI_STATUS
+EFIAPI
+AmlCodeGenRdWordIo (
+  IN        BOOLEAN IsResourceConsumer,
+  IN        BOOLEAN IsMinFixed,
+  IN        BOOLEAN IsMaxFixed,
+  IN        BOOLEAN IsPosDecode,
+  IN        UINT8 IsaRanges,
+  IN        UINT16 AddressGranularity,
+  IN        UINT16 AddressMinimum,
+  IN        UINT16 AddressMaximum,
+  IN        UINT16 AddressTranslation,
+  IN        UINT16 RangeLength,
+  IN        UINT8 ResourceSourceIndex,
+  IN  CONST CHAR8 *ResourceSource,
+  IN        BOOLEAN IsDenseTranslation,
+  IN        BOOLEAN IsTypeStatic,
+  IN        AML_OBJECT_NODE_HANDLE NameOpNode, OPTIONAL
+  OUT       AML_DATA_NODE_HANDLE    *NewRdNode  OPTIONAL
+  );
+
 /** Code generation for the "QWordIO ()" ASL function.
 
   The Resource Data effectively created is a QWord Address Space Resource
diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.c b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.c
index 0bc1c1d119..60fe69ba6d 100644
--- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.c
+++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.c
@@ -2,6 +2,7 @@
   AML Resource Data Code Generation.
 
   Copyright (c) 2020 - 2021, Arm Limited. All rights reserved.<BR>
+  Copyright (C) 2023 Advanced Micro Devices, Inc. All rights reserved.<BR>
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -878,6 +879,93 @@ AmlCodeGenRdWordBusNumber (
            );
 }
 
+/** Code generation for the "WordIO ()" ASL function.
+
+  The Resource Data effectively created is a Word Address Space Resource
+  Data. Cf ACPI 6.5:
+   - s6.4.3.5.3 "Word Address Space Descriptor".
+
+  The created resource data node can be:
+   - appended to the list of resource data elements of the NameOpNode.
+     In such case NameOpNode must be defined by a the "Name ()" ASL statement
+     and initially contain a "ResourceTemplate ()".
+   - returned through the NewRdNode parameter.
+
+  @param [in]  IsResourceConsumer   ResourceUsage parameter.
+  @param [in]  IsMinFixed           Minimum address is fixed.
+  @param [in]  IsMaxFixed           Maximum address is fixed.
+  @param [in]  IsPosDecode          Decode parameter
+  @param [in]  IsaRanges            Possible values are:
+                                     0-Reserved
+                                     1-NonISAOnly
+                                     2-ISAOnly
+                                     3-EntireRange
+  @param [in]  AddressGranularity   Address granularity.
+  @param [in]  AddressMinimum       Minimum address.
+  @param [in]  AddressMaximum       Maximum address.
+  @param [in]  AddressTranslation   Address translation.
+  @param [in]  RangeLength          Range length.
+  @param [in]  ResourceSourceIndex  Resource Source index.
+                                    Not supported. Must be 0.
+  @param [in]  ResourceSource       Resource Source.
+                                    Not supported. Must be NULL.
+  @param [in]  IsDenseTranslation   TranslationDensity parameter.
+  @param [in]  IsTypeStatic         TranslationType parameter.
+  @param [in]  NameOpNode           NameOp object node defining a named object.
+                                    If provided, append the new resource data
+                                    node to the list of resource data elements
+                                    of this node.
+  @param [out] NewRdNode            If provided and success,
+                                    contain the created node.
+
+  @retval EFI_SUCCESS             The function completed successfully.
+  @retval EFI_INVALID_PARAMETER   Invalid parameter.
+  @retval EFI_OUT_OF_RESOURCES    Could not allocate memory.
+**/
+EFI_STATUS
+EFIAPI
+AmlCodeGenRdWordIo (
+  IN        BOOLEAN IsResourceConsumer,
+  IN        BOOLEAN IsMinFixed,
+  IN        BOOLEAN IsMaxFixed,
+  IN        BOOLEAN IsPosDecode,
+  IN        UINT8 IsaRanges,
+  IN        UINT16 AddressGranularity,
+  IN        UINT16 AddressMinimum,
+  IN        UINT16 AddressMaximum,
+  IN        UINT16 AddressTranslation,
+  IN        UINT16 RangeLength,
+  IN        UINT8 ResourceSourceIndex,
+  IN  CONST CHAR8 *ResourceSource,
+  IN        BOOLEAN IsDenseTranslation,
+  IN        BOOLEAN IsTypeStatic,
+  IN        AML_OBJECT_NODE_HANDLE NameOpNode, OPTIONAL
+  OUT       AML_DATA_NODE_HANDLE    *NewRdNode  OPTIONAL
+  )
+{
+  return AmlCodeGenRdWordSpace (
+           ACPI_ADDRESS_SPACE_TYPE_IO,
+           IsResourceConsumer,
+           IsPosDecode,
+           IsMinFixed,
+           IsMaxFixed,
+           RdIoRangeSpecificFlags (
+             IsaRanges,
+             IsDenseTranslation,
+             IsTypeStatic
+             ),
+           AddressGranularity,
+           AddressMinimum,
+           AddressMaximum,
+           AddressTranslation,
+           RangeLength,
+           ResourceSourceIndex,
+           ResourceSource,
+           NameOpNode,
+           NewRdNode
+           );
+}
+
 /** Code generation for the "QWordSpace ()" ASL function.
 
   The Resource Data effectively created is a QWord Address Space Resource
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112669): https://edk2.groups.io/g/devel/message/112669
Mute This Topic: https://groups.io/mt/103256859/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [Resend PATCH v4 2/4] DynamicTablesPkg: Corrects AmlCodeGenRdWordBusNumber parameters

[Abdul Lateef Attar via groups.io]

From: Abdul Lateef Attar <AbdulLateef.Attar@amd.com>

Corrects the parameters of AmlCodeGenRdWordBusNumber() to
UINT16 to generate word bus number.

Cc: Pierre Gondois <pierre.gondois@arm.com>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Signed-off-by: Abdul Lateef Attar <AbdulLateef.Attar@amd.com>
Reviewed-by: Pierre Gondois <pierre.gondois@arm.com>
---
 DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h       | 10 +++++-----
 .../Common/AmlLib/CodeGen/AmlResourceDataCodeGen.c     | 10 +++++-----
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
index 5e340b94ce..eb8740692f 100644
--- a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
+++ b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
@@ -714,11 +714,11 @@ AmlCodeGenRdWordBusNumber (
   IN        BOOLEAN IsMinFixed,
   IN        BOOLEAN IsMaxFixed,
   IN        BOOLEAN IsPosDecode,
-  IN        UINT32 AddressGranularity,
-  IN        UINT32 AddressMinimum,
-  IN        UINT32 AddressMaximum,
-  IN        UINT32 AddressTranslation,
-  IN        UINT32 RangeLength,
+  IN        UINT16 AddressGranularity,
+  IN        UINT16 AddressMinimum,
+  IN        UINT16 AddressMaximum,
+  IN        UINT16 AddressTranslation,
+  IN        UINT16 RangeLength,
   IN        UINT8 ResourceSourceIndex,
   IN  CONST CHAR8 *ResourceSource,
   IN        AML_OBJECT_NODE_HANDLE NameOpNode, OPTIONAL
diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.c b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.c
index 60fe69ba6d..46243f981c 100644
--- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.c
+++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.c
@@ -848,11 +848,11 @@ AmlCodeGenRdWordBusNumber (
   IN        BOOLEAN IsMinFixed,
   IN        BOOLEAN IsMaxFixed,
   IN        BOOLEAN IsPosDecode,
-  IN        UINT32 AddressGranularity,
-  IN        UINT32 AddressMinimum,
-  IN        UINT32 AddressMaximum,
-  IN        UINT32 AddressTranslation,
-  IN        UINT32 RangeLength,
+  IN        UINT16 AddressGranularity,
+  IN        UINT16 AddressMinimum,
+  IN        UINT16 AddressMaximum,
+  IN        UINT16 AddressTranslation,
+  IN        UINT16 RangeLength,
   IN        UINT8 ResourceSourceIndex,
   IN  CONST CHAR8 *ResourceSource,
   IN        AML_OBJECT_NODE_HANDLE NameOpNode, OPTIONAL
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112671): https://edk2.groups.io/g/devel/message/112671
Mute This Topic: https://groups.io/mt/103256861/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [Resend PATCH v4 3/4] DynamicTablesPkg: Corrects function pointer typedef of AML_PARSE_FUNCTION

[Abdul Lateef Attar via groups.io]

From: Abdul Lateef Attar <AbdulLateef.Attar@amd.com>

Corrects the function pointer typedef AML_PARSE_FUNCTION,
otherwise some compiler like VS2019 gives error.

Cc: Pierre Gondois <pierre.gondois@arm.com>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Signed-off-by: Abdul Lateef Attar <AbdulLateef.Attar@amd.com>
Reviewed-by: Pierre Gondois <pierre.gondois@arm.com>
---
 DynamicTablesPkg/Library/Common/AmlLib/Parser/AmlParser.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/DynamicTablesPkg/Library/Common/AmlLib/Parser/AmlParser.c b/DynamicTablesPkg/Library/Common/AmlLib/Parser/AmlParser.c
index 8e584d4930..d3a51a94c7 100644
--- a/DynamicTablesPkg/Library/Common/AmlLib/Parser/AmlParser.c
+++ b/DynamicTablesPkg/Library/Common/AmlLib/Parser/AmlParser.c
@@ -100,8 +100,7 @@ AmlParseStream (
 **/
 typedef
 EFI_STATUS
-EFIAPI
-(*AML_PARSE_FUNCTION) (
+(EFIAPI *AML_PARSE_FUNCTION)(
   IN      CONST AML_NODE_HEADER   *Node,
   IN            AML_PARSE_FORMAT  ExpectedFormat,
   IN  OUT       AML_STREAM        *FStream,
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112670): https://edk2.groups.io/g/devel/message/112670
Mute This Topic: https://groups.io/mt/103256860/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [Resend PATCH v4 4/4] DynamicTablesPkg: AML Code generation to invoke a method

[Abdul Lateef Attar via groups.io]

From: Abdul Lateef Attar <AbdulLateef.Attar@amd.com>

Adds API to generate AML code to invoke/call another
method. Also provides ability to pass arguments of
type integer, string, ArgObj or LocalObj.

Cc: Pierre Gondois <pierre.gondois@arm.com>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Signed-off-by: Abdul Lateef Attar <AbdulLateef.Attar@amd.com>
---
 .../Include/Library/AmlLib/AmlLib.h           | 112 +++++++++
 .../Common/AmlLib/CodeGen/AmlCodeGen.c        | 236 +++++++++++++++++-
 2 files changed, 347 insertions(+), 1 deletion(-)

diff --git a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
index eb8740692f..fb16637f02 100644
--- a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
+++ b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
@@ -101,6 +101,56 @@ typedef enum {
   AmlAddressRangeMax      = 4
 } AML_MEMORY_ATTRIBUTES_MTP;
 
+/** Method parameter types
+
+  Possible values are:
+    0 - AmlMethodParamTypeInteger
+    1 - AmlMethodParamTypeString
+    2 - AmlMethodParamTypeArg
+    3 - AmlMethodParamTypeLocal
+
+  @par Reference(s)
+  - ACPI 6.5, s20.2.5 "Term Objects Encoding"
+
+**/
+typedef enum {
+  AmlMethodParamTypeInteger = 0,
+  AmlMethodParamTypeString  = 1,
+  AmlMethodParamTypeArg     = 2,
+  AmlMethodParamTypeLocal   = 3
+} AML_METHOD_PARAM_TYPE;
+
+/** AML Method parameter data
+  holds the AML method parameter data.
+**/
+typedef union {
+  UINT8     Arg;
+  UINT8     Local;
+  UINT64    Integer;
+  VOID      *Buffer;
+} AML_METHOD_PARAM_DATA;
+
+/** structure to hold AML method parameter types
+  Type  -   Type of parameter
+  Data  -   holds data of parameter
+            if Type is AmlMethodParamTypeInteger
+              then Data is of type Integer to hold integer value.
+            if Type is AmlMethodParamTypeString
+              then Data contains null terminated string.
+            If Type is AmlMethodParamTypeArg
+              then Data contains the Argument number,
+              0 to 6 are supported value.
+            If Type is AmlMethodParamTypeLocal
+              then Data contains the Local variable number,
+              0 to 7 are supported value.
+  DataSize - for future use
+**/
+typedef struct {
+  AML_METHOD_PARAM_TYPE    Type;
+  AML_METHOD_PARAM_DATA    Data;
+  UINTN                    DataSize;
+} AML_METHOD_PARAM;
+
 /** Parse the definition block.
 
   The function parses the whole AML blob. It starts with the ACPI DSDT/SSDT
@@ -1693,4 +1743,66 @@ AmlAddNameStringToNamedPackage (
   IN AML_OBJECT_NODE_HANDLE  NamedNode
   );
 
+/** AML code generation to invoke/call another method.
+
+  This method is subset implementation of MethodInvocation
+  defined in the ACPI specification 6.5,
+  section 20.2.5 "Term Objects Encoding".
+  Added integer, string, ArgObj and LocalObj support.
+
+  Example 1:
+    AmlCodeGenInvokeMethod ("MET0", 0, NULL, ParentNode);
+    is equivalent of the following ASL code:
+      MET0 ();
+
+  Example 2:
+    AML_METHOD_PARAM  Param[4];
+    Param[0].Data.Integer = 0x100;
+    Param[0].Type = AmlMethodParamTypeInteger;
+    Param[1].Data.Buffer = "TEST";
+    Param[1].Type = AmlMethodParamTypeString;
+    Param[2].Data.Arg = 0;
+    Param[2].Type = AmlMethodParamTypeArg;
+    Param[3].Data.Local = 2;
+    Param[3].Type = AmlMethodParamTypeLocal;
+    AmlCodeGenInvokeMethod ("MET0", 4, Param, ParentNode);
+
+    is equivalent of the following ASL code:
+      MET0 (0x100, "TEST", Arg0, Local2);
+
+  Example 3:
+    AML_METHOD_PARAM  Param[2];
+    Param[0].Data.Arg = 0;
+    Param[0].Type = AmlMethodParamTypeArg;
+    Param[1].Data.Integer = 0x100;
+    Param[1].Type = AmlMethodParamTypeInteger;
+    AmlCodeGenMethodRetNameString ("MET2", NULL, 2, TRUE, 0, ParentNode, &MethodNode);
+    AmlCodeGenInvokeMethod ("MET3", 2, Param, MethodNode);
+
+    is equivalent of the following ASL code:
+    Method (MET2, 2, Serialized)
+    {
+      MET3 (Arg0, 0x0100)
+    }
+
+  @param [in] MethodNameString  Method name to be called/invoked.
+  @param [in] NumArgs           Number of arguments to be passed,
+                                0 to 7 are permissible values.
+  @param [in] Parameters        contains the parameter data.
+  @param [in] ParentNode        set ParentNode as the parent
+                                of the node created.
+
+  @retval EFI_SUCCESS             Success.
+  @retval EFI_INVALID_PARAMETER   Invalid parameter.
+  @retval EFI_OUT_OF_RESOURCES    Failed to allocate memory.
+ **/
+EFI_STATUS
+EFIAPI
+AmlCodeGenInvokeMethod (
+  IN  CONST CHAR8             *MethodNameString,
+  IN        UINT8             NumArgs,
+  IN        AML_METHOD_PARAM  *Parameters   OPTIONAL,
+  IN        AML_NODE_HANDLE   ParentNode
+  );
+
 #endif // AML_LIB_H_
diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
index a6db34fb97..688eefdcef 100644
--- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
+++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
@@ -2,6 +2,7 @@
   AML Code Generation.
 
   Copyright (c) 2020 - 2022, Arm Limited. All rights reserved.<BR>
+  Copyright (C) 2023 Advanced Micro Devices, Inc. All rights reserved.<BR>
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 **/
@@ -921,7 +922,7 @@ AmlCodeGenNameUnicodeString (
   Status = AmlCreateDataNode (
              EAmlNodeDataTypeRaw,
              (CONST UINT8 *)String,
-             StrSize (String),
+             (UINT32)StrSize (String),
              &DataNode
              );
   if (EFI_ERROR (Status)) {
@@ -3849,3 +3850,236 @@ exit_handler:
 
   return Status;
 }
+
+/** AML code generation to invoke/call another method.
+
+  This method is subset implementation of MethodInvocation
+  defined in the ACPI specification 6.5,
+  section 20.2.5 "Term Objects Encoding".
+  Added integer, string, ArgObj and LocalObj support.
+
+  Example 1:
+    AmlCodeGenInvokeMethod ("MET0", 0, NULL, ParentNode);
+    is equivalent of the following ASL code:
+      MET0 ();
+
+  Example 2:
+    AML_METHOD_PARAM  Param[4];
+    Param[0].Data.Integer = 0x100;
+    Param[0].Type = AmlMethodParamTypeInteger;
+    Param[1].Data.Buffer = "TEST";
+    Param[1].Type = AmlMethodParamTypeString;
+    Param[2].Data.Arg = 0;
+    Param[2].Type = AmlMethodParamTypeArg;
+    Param[3].Data.Local = 2;
+    Param[3].Type = AmlMethodParamTypeLocal;
+    AmlCodeGenInvokeMethod ("MET0", 4, Param, ParentNode);
+
+    is equivalent of the following ASL code:
+      MET0 (0x100, "TEST", Arg0, Local2);
+
+  Example 3:
+    AML_METHOD_PARAM  Param[2];
+    Param[0].Data.Arg = 0;
+    Param[0].Type = AmlMethodParamTypeArg;
+    Param[1].Data.Integer = 0x100;
+    Param[1].Type = AmlMethodParamTypeInteger;
+    AmlCodeGenMethodRetNameString ("MET2", NULL, 2, TRUE, 0, ParentNode, &MethodNode);
+    AmlCodeGenInvokeMethod ("MET3", 2, Param, MethodNode);
+
+    is equivalent of the following ASL code:
+    Method (MET2, 2, Serialized)
+    {
+      MET3 (Arg0, 0x0100)
+    }
+
+  @param [in] MethodNameString  Method name to be called/invoked.
+  @param [in] NumArgs           Number of arguments to be passed,
+                                0 to 7 are permissible values.
+  @param [in] Parameters        contains the parameter data.
+  @param [in] ParentNode        set ParentNode as the parent
+                                of the node created.
+
+  @retval EFI_SUCCESS             Success.
+  @retval EFI_INVALID_PARAMETER   Invalid parameter.
+  @retval EFI_OUT_OF_RESOURCES    Failed to allocate memory.
+ **/
+EFI_STATUS
+EFIAPI
+AmlCodeGenInvokeMethod (
+  IN  CONST CHAR8             *MethodNameString,
+  IN        UINT8             NumArgs,
+  IN        AML_METHOD_PARAM  *Parameters   OPTIONAL,
+  IN        AML_NODE_HANDLE   ParentNode
+  )
+{
+  EFI_STATUS       Status;
+  UINT8            Index;
+  CHAR8            *AmlNameString;
+  UINT32           AmlNameStringSize;
+  AML_DATA_NODE    *DataNode;
+  AML_OBJECT_NODE  *IntNode;
+  AML_NODE_HANDLE  HeadNode;
+  AML_NODE_HANDLE  CurrentNode;
+  AML_OBJECT_NODE  *ObjectNode;
+
+  if ((MethodNameString == NULL) || (ParentNode == NULL)) {
+    ASSERT (0);
+    return EFI_INVALID_PARAMETER;
+  }
+
+  if ((NumArgs > 7) ||
+      ((Parameters == NULL) && (NumArgs > 0)))
+  {
+    ASSERT (0);
+    return EFI_INVALID_PARAMETER;
+  }
+
+  if (Parameters != NULL) {
+    /// Validate and convert the Parameters to the stream of node
+    HeadNode    = NULL;
+    CurrentNode = NULL;
+    for (Index = 0; Index < NumArgs; Index++) {
+      switch (Parameters[Index].Type) {
+        case AmlMethodParamTypeInteger:
+          IntNode = NULL;
+          Status  = AmlCodeGenInteger (Parameters[Index].Data.Integer, &IntNode);
+          if (EFI_ERROR (Status)) {
+            ASSERT_EFI_ERROR (Status);
+            goto exit_handler;
+          }
+
+          CurrentNode = (AML_NODE_HANDLE)IntNode;
+          break;
+        case AmlMethodParamTypeString:
+          ObjectNode = NULL;
+          if (Parameters[Index].Data.Buffer == NULL) {
+            ASSERT_EFI_ERROR (EFI_INVALID_PARAMETER);
+            Status = EFI_INVALID_PARAMETER;
+            goto exit_handler;
+          }
+
+          Status = AmlCodeGenString (Parameters[Index].Data.Buffer, &ObjectNode);
+          if (EFI_ERROR (Status)) {
+            ASSERT_EFI_ERROR (Status);
+            goto exit_handler;
+          }
+
+          CurrentNode = (AML_NODE_HANDLE)ObjectNode;
+          break;
+        case AmlMethodParamTypeArg:
+          ObjectNode = NULL;
+          if (Parameters[Index].Data.Arg > (UINT8)(AML_ARG6 - AML_ARG0)) {
+            ASSERT_EFI_ERROR (EFI_INVALID_PARAMETER);
+            Status = EFI_INVALID_PARAMETER;
+            goto exit_handler;
+          }
+
+          Status = AmlCreateObjectNode (
+                     AmlGetByteEncodingByOpCode (AML_ARG0 + Parameters[Index].Data.Arg, 0),
+                     0,
+                     &ObjectNode
+                     );
+          if (EFI_ERROR (Status)) {
+            ASSERT_EFI_ERROR (Status);
+            goto exit_handler;
+          }
+
+          CurrentNode = (AML_NODE_HANDLE)ObjectNode;
+          break;
+        case AmlMethodParamTypeLocal:
+          ObjectNode = NULL;
+          if (Parameters[Index].Data.Local > (UINT8)(AML_LOCAL7 - AML_LOCAL0)) {
+            ASSERT_EFI_ERROR (EFI_INVALID_PARAMETER);
+            Status = EFI_INVALID_PARAMETER;
+            goto exit_handler;
+          }
+
+          Status = AmlCreateObjectNode (
+                     AmlGetByteEncodingByOpCode (AML_LOCAL0 + Parameters[Index].Data.Local, 0),
+                     0,
+                     &ObjectNode
+                     );
+          if (EFI_ERROR (Status)) {
+            ASSERT_EFI_ERROR (Status);
+            goto exit_handler;
+          }
+
+          CurrentNode = (AML_NODE_HANDLE)ObjectNode;
+          break;
+        default:
+          ASSERT_EFI_ERROR (EFI_INVALID_PARAMETER);
+          Status = EFI_INVALID_PARAMETER;
+          goto exit_handler;
+          break;
+      }
+
+      if (HeadNode == NULL) {
+        HeadNode = CurrentNode;
+      } else {
+        Status = AmlVarListAddTail (
+                   HeadNode,
+                   CurrentNode
+                   );
+        if (EFI_ERROR (Status)) {
+          ASSERT_EFI_ERROR (Status);
+          goto exit_handler;
+        }
+      }
+    }
+  }
+
+  /// Create called/invoked method name string
+  Status = ConvertAslNameToAmlName (MethodNameString, &AmlNameString);
+  if (EFI_ERROR (Status)) {
+    ASSERT_EFI_ERROR (Status);
+    goto exit_handler;
+  }
+
+  Status = AmlGetNameStringSize (AmlNameString, &AmlNameStringSize);
+  if (EFI_ERROR (Status)) {
+    ASSERT_EFI_ERROR (Status);
+    FreePool (AmlNameString);
+    goto exit_handler;
+  }
+
+  DataNode = NULL;
+  Status   = AmlCreateDataNode (
+               EAmlNodeDataTypeNameString,
+               (UINT8 *)AmlNameString,
+               AmlNameStringSize,
+               &DataNode
+               );
+  FreePool (AmlNameString);
+  if (EFI_ERROR (Status)) {
+    ASSERT_EFI_ERROR (Status);
+    goto exit_handler;
+  }
+
+  Status = AmlVarListAddTail (
+             (AML_NODE_HANDLE)ParentNode,
+             (AML_NODE_HANDLE)DataNode
+             );
+  if (EFI_ERROR (Status)) {
+    ASSERT_EFI_ERROR (Status);
+    goto exit_handler;
+  }
+
+  Status = AmlVarListAddTail (
+             (AML_NODE_HANDLE)ParentNode,
+             HeadNode
+             );
+  if (EFI_ERROR (Status)) {
+    ASSERT_EFI_ERROR (Status);
+    goto exit_handler;
+  }
+
+  return Status;
+
+exit_handler:
+  if (HeadNode != NULL) {
+    AmlDeleteTree (HeadNode);
+  }
+
+  return Status;
+}
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112672): https://edk2.groups.io/g/devel/message/112672
Mute This Topic: https://groups.io/mt/103256864/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [Resend PATCH v4 4/4] DynamicTablesPkg: AML Code generation to invoke a method

[PierreGondois]

Hello Abdul,
The new function looks good, there is just one small issue with
the tree layout,

Regards,
Pierre

On 12/19/23 04:06, Abdul Lateef Attar wrote:
> From: Abdul Lateef Attar <AbdulLateef.Attar@amd.com>
> 
> Adds API to generate AML code to invoke/call another
> method. Also provides ability to pass arguments of
> type integer, string, ArgObj or LocalObj.
> 
> Cc: Pierre Gondois <pierre.gondois@arm.com>
> Cc: Sami Mujawar <sami.mujawar@arm.com>
> Signed-off-by: Abdul Lateef Attar <AbdulLateef.Attar@amd.com>
> ---
>   .../Include/Library/AmlLib/AmlLib.h           | 112 +++++++++
>   .../Common/AmlLib/CodeGen/AmlCodeGen.c        | 236 +++++++++++++++++-
>   2 files changed, 347 insertions(+), 1 deletion(-)
> 
> diff --git a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
> index eb8740692f..fb16637f02 100644
> --- a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
> +++ b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
> @@ -101,6 +101,56 @@ typedef enum {
>     AmlAddressRangeMax      = 4
>   } AML_MEMORY_ATTRIBUTES_MTP;
>   
> +/** Method parameter types
> +
> +  Possible values are:
> +    0 - AmlMethodParamTypeInteger
> +    1 - AmlMethodParamTypeString
> +    2 - AmlMethodParamTypeArg
> +    3 - AmlMethodParamTypeLocal
> +
> +  @par Reference(s)
> +  - ACPI 6.5, s20.2.5 "Term Objects Encoding"
> +
> +**/
> +typedef enum {
> +  AmlMethodParamTypeInteger = 0,
> +  AmlMethodParamTypeString  = 1,
> +  AmlMethodParamTypeArg     = 2,
> +  AmlMethodParamTypeLocal   = 3
> +} AML_METHOD_PARAM_TYPE;
> +
> +/** AML Method parameter data
> +  holds the AML method parameter data.
> +**/
> +typedef union {
> +  UINT8     Arg;
> +  UINT8     Local;
> +  UINT64    Integer;
> +  VOID      *Buffer;
> +} AML_METHOD_PARAM_DATA;
> +
> +/** structure to hold AML method parameter types
> +  Type  -   Type of parameter
> +  Data  -   holds data of parameter
> +            if Type is AmlMethodParamTypeInteger
> +              then Data is of type Integer to hold integer value.
> +            if Type is AmlMethodParamTypeString
> +              then Data contains null terminated string.
> +            If Type is AmlMethodParamTypeArg
> +              then Data contains the Argument number,
> +              0 to 6 are supported value.
> +            If Type is AmlMethodParamTypeLocal
> +              then Data contains the Local variable number,
> +              0 to 7 are supported value.
> +  DataSize - for future use
> +**/
> +typedef struct {
> +  AML_METHOD_PARAM_TYPE    Type;
> +  AML_METHOD_PARAM_DATA    Data;
> +  UINTN                    DataSize;
> +} AML_METHOD_PARAM;
> +
>   /** Parse the definition block.
>   
>     The function parses the whole AML blob. It starts with the ACPI DSDT/SSDT
> @@ -1693,4 +1743,66 @@ AmlAddNameStringToNamedPackage (
>     IN AML_OBJECT_NODE_HANDLE  NamedNode
>     );
>   
> +/** AML code generation to invoke/call another method.
> +
> +  This method is subset implementation of MethodInvocation
> +  defined in the ACPI specification 6.5,
> +  section 20.2.5 "Term Objects Encoding".
> +  Added integer, string, ArgObj and LocalObj support.
> +
> +  Example 1:
> +    AmlCodeGenInvokeMethod ("MET0", 0, NULL, ParentNode);
> +    is equivalent of the following ASL code:
> +      MET0 ();
> +
> +  Example 2:
> +    AML_METHOD_PARAM  Param[4];
> +    Param[0].Data.Integer = 0x100;
> +    Param[0].Type = AmlMethodParamTypeInteger;
> +    Param[1].Data.Buffer = "TEST";
> +    Param[1].Type = AmlMethodParamTypeString;
> +    Param[2].Data.Arg = 0;
> +    Param[2].Type = AmlMethodParamTypeArg;
> +    Param[3].Data.Local = 2;
> +    Param[3].Type = AmlMethodParamTypeLocal;
> +    AmlCodeGenInvokeMethod ("MET0", 4, Param, ParentNode);
> +
> +    is equivalent of the following ASL code:
> +      MET0 (0x100, "TEST", Arg0, Local2);
> +
> +  Example 3:
> +    AML_METHOD_PARAM  Param[2];
> +    Param[0].Data.Arg = 0;
> +    Param[0].Type = AmlMethodParamTypeArg;
> +    Param[1].Data.Integer = 0x100;
> +    Param[1].Type = AmlMethodParamTypeInteger;
> +    AmlCodeGenMethodRetNameString ("MET2", NULL, 2, TRUE, 0, ParentNode, &MethodNode);
> +    AmlCodeGenInvokeMethod ("MET3", 2, Param, MethodNode);
> +
> +    is equivalent of the following ASL code:
> +    Method (MET2, 2, Serialized)
> +    {
> +      MET3 (Arg0, 0x0100)
> +    }
> +
> +  @param [in] MethodNameString  Method name to be called/invoked.
> +  @param [in] NumArgs           Number of arguments to be passed,
> +                                0 to 7 are permissible values.
> +  @param [in] Parameters        contains the parameter data.
> +  @param [in] ParentNode        set ParentNode as the parent> +                                of the node created.
> +
> +  @retval EFI_SUCCESS             Success.
> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
> +  @retval EFI_OUT_OF_RESOURCES    Failed to allocate memory.
> + **/
> +EFI_STATUS
> +EFIAPI
> +AmlCodeGenInvokeMethod (
> +  IN  CONST CHAR8             *MethodNameString,
> +  IN        UINT8             NumArgs,
> +  IN        AML_METHOD_PARAM  *Parameters   OPTIONAL,
> +  IN        AML_NODE_HANDLE   ParentNode
> +  );
> +
>   #endif // AML_LIB_H_
> diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
> index a6db34fb97..688eefdcef 100644
> --- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
> +++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
> @@ -2,6 +2,7 @@
>     AML Code Generation.
>   
>     Copyright (c) 2020 - 2022, Arm Limited. All rights reserved.<BR>
> +  Copyright (C) 2023 Advanced Micro Devices, Inc. All rights reserved.<BR>
>   
>     SPDX-License-Identifier: BSD-2-Clause-Patent
>   **/
> @@ -921,7 +922,7 @@ AmlCodeGenNameUnicodeString (
>     Status = AmlCreateDataNode (
>                EAmlNodeDataTypeRaw,
>                (CONST UINT8 *)String,
> -             StrSize (String),
> +             (UINT32)StrSize (String),
>                &DataNode
>                );
>     if (EFI_ERROR (Status)) {
> @@ -3849,3 +3850,236 @@ exit_handler:
>   
>     return Status;
>   }
> +
> +/** AML code generation to invoke/call another method.
> +
> +  This method is subset implementation of MethodInvocation
> +  defined in the ACPI specification 6.5,
> +  section 20.2.5 "Term Objects Encoding".
> +  Added integer, string, ArgObj and LocalObj support.
> +
> +  Example 1:
> +    AmlCodeGenInvokeMethod ("MET0", 0, NULL, ParentNode);
> +    is equivalent of the following ASL code:
> +      MET0 ();
> +
> +  Example 2:
> +    AML_METHOD_PARAM  Param[4];
> +    Param[0].Data.Integer = 0x100;
> +    Param[0].Type = AmlMethodParamTypeInteger;
> +    Param[1].Data.Buffer = "TEST";
> +    Param[1].Type = AmlMethodParamTypeString;
> +    Param[2].Data.Arg = 0;
> +    Param[2].Type = AmlMethodParamTypeArg;
> +    Param[3].Data.Local = 2;
> +    Param[3].Type = AmlMethodParamTypeLocal;
> +    AmlCodeGenInvokeMethod ("MET0", 4, Param, ParentNode);
> +
> +    is equivalent of the following ASL code:
> +      MET0 (0x100, "TEST", Arg0, Local2);
> +
> +  Example 3:
> +    AML_METHOD_PARAM  Param[2];
> +    Param[0].Data.Arg = 0;
> +    Param[0].Type = AmlMethodParamTypeArg;
> +    Param[1].Data.Integer = 0x100;
> +    Param[1].Type = AmlMethodParamTypeInteger;
> +    AmlCodeGenMethodRetNameString ("MET2", NULL, 2, TRUE, 0, ParentNode, &MethodNode);
> +    AmlCodeGenInvokeMethod ("MET3", 2, Param, MethodNode);
> +
> +    is equivalent of the following ASL code:
> +    Method (MET2, 2, Serialized)
> +    {
> +      MET3 (Arg0, 0x0100)
> +    }
> +
> +  @param [in] MethodNameString  Method name to be called/invoked.
> +  @param [in] NumArgs           Number of arguments to be passed,
> +                                0 to 7 are permissible values.
> +  @param [in] Parameters        contains the parameter data.
> +  @param [in] ParentNode        set ParentNode as the parent

NIT: set->Set

> +                                of the node created.
> +
> +  @retval EFI_SUCCESS             Success.
> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
> +  @retval EFI_OUT_OF_RESOURCES    Failed to allocate memory.
> + **/
> +EFI_STATUS
> +EFIAPI
> +AmlCodeGenInvokeMethod (
> +  IN  CONST CHAR8             *MethodNameString,
> +  IN        UINT8             NumArgs,
> +  IN        AML_METHOD_PARAM  *Parameters   OPTIONAL,
> +  IN        AML_NODE_HANDLE   ParentNode

Maybe it would be worth also having an OUT parameter in case
someone doesn't want to attach the method invocation directly
(cf. other methods in the file):
   OUT AML_OBJECT_NODE_HANDLE  *NewObjectNode   OPTIONAL


> +  )
> +{
> +  EFI_STATUS       Status;
> +  UINT8            Index;
> +  CHAR8            *AmlNameString;
> +  UINT32           AmlNameStringSize;
> +  AML_DATA_NODE    *DataNode;
> +  AML_OBJECT_NODE  *IntNode;
> +  AML_NODE_HANDLE  HeadNode;
> +  AML_NODE_HANDLE  CurrentNode;
> +  AML_OBJECT_NODE  *ObjectNode;
> +
> +  if ((MethodNameString == NULL) || (ParentNode == NULL)) {
> +    ASSERT (0);
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  if ((NumArgs > 7) ||
> +      ((Parameters == NULL) && (NumArgs > 0)))
> +  {
> +    ASSERT (0);
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  if (Parameters != NULL) {
> +    /// Validate and convert the Parameters to the stream of node
> +    HeadNode    = NULL;
> +    CurrentNode = NULL;
> +    for (Index = 0; Index < NumArgs; Index++) {
> +      switch (Parameters[Index].Type) {
> +        case AmlMethodParamTypeInteger:
> +          IntNode = NULL;
> +          Status  = AmlCodeGenInteger (Parameters[Index].Data.Integer, &IntNode);
> +          if (EFI_ERROR (Status)) {
> +            ASSERT_EFI_ERROR (Status);
> +            goto exit_handler;
> +          }
> +
> +          CurrentNode = (AML_NODE_HANDLE)IntNode;
> +          break;
> +        case AmlMethodParamTypeString:
> +          ObjectNode = NULL;
> +          if (Parameters[Index].Data.Buffer == NULL) {
> +            ASSERT_EFI_ERROR (EFI_INVALID_PARAMETER);
> +            Status = EFI_INVALID_PARAMETER;
> +            goto exit_handler;
> +          }
> +
> +          Status = AmlCodeGenString (Parameters[Index].Data.Buffer, &ObjectNode);
> +          if (EFI_ERROR (Status)) {
> +            ASSERT_EFI_ERROR (Status);
> +            goto exit_handler;
> +          }
> +
> +          CurrentNode = (AML_NODE_HANDLE)ObjectNode;
> +          break;
> +        case AmlMethodParamTypeArg:
> +          ObjectNode = NULL;
> +          if (Parameters[Index].Data.Arg > (UINT8)(AML_ARG6 - AML_ARG0)) {
> +            ASSERT_EFI_ERROR (EFI_INVALID_PARAMETER);
> +            Status = EFI_INVALID_PARAMETER;
> +            goto exit_handler;
> +          }
> +
> +          Status = AmlCreateObjectNode (
> +                     AmlGetByteEncodingByOpCode (AML_ARG0 + Parameters[Index].Data.Arg, 0),
> +                     0,
> +                     &ObjectNode
> +                     );
> +          if (EFI_ERROR (Status)) {
> +            ASSERT_EFI_ERROR (Status);
> +            goto exit_handler;
> +          }
> +
> +          CurrentNode = (AML_NODE_HANDLE)ObjectNode;
> +          break;
> +        case AmlMethodParamTypeLocal:
> +          ObjectNode = NULL;
> +          if (Parameters[Index].Data.Local > (UINT8)(AML_LOCAL7 - AML_LOCAL0)) {
> +            ASSERT_EFI_ERROR (EFI_INVALID_PARAMETER);
> +            Status = EFI_INVALID_PARAMETER;
> +            goto exit_handler;
> +          }
> +
> +          Status = AmlCreateObjectNode (
> +                     AmlGetByteEncodingByOpCode (AML_LOCAL0 + Parameters[Index].Data.Local, 0),
> +                     0,
> +                     &ObjectNode
> +                     );
> +          if (EFI_ERROR (Status)) {
> +            ASSERT_EFI_ERROR (Status);
> +            goto exit_handler;
> +          }
> +
> +          CurrentNode = (AML_NODE_HANDLE)ObjectNode;
> +          break;
> +        default:
> +          ASSERT_EFI_ERROR (EFI_INVALID_PARAMETER);
> +          Status = EFI_INVALID_PARAMETER;
> +          goto exit_handler;
> +          break;
> +      }
> +
> +      if (HeadNode == NULL) {
> +        HeadNode = CurrentNode;

I think there is an issue here. The method arguments should be attached
to the variable list of arguments of the parent, not of another argument.

For instance, the following:
Scope (_SB) {
   MET0 (Arg0, Arg1, Arg2)
}

Should create a tree as:
_SB
\- (Variable list of arguments of _SB)
   - MET0
   - Arg0
   - Arg1
   - Arg2

With the present code, the created tree is:
_SB
\- (Variable list of arguments of _SB)
   - MET0
   - Arg0
   \- (Variable list of arguments of Arg0)
     - Arg1
     - Arg2

When serializing the tree, the generated binary is identical, but
the tree is incorrect.

> +      } else {
> +        Status = AmlVarListAddTail (
> +                   HeadNode,
> +                   CurrentNode
> +                   );
> +        if (EFI_ERROR (Status)) {
> +          ASSERT_EFI_ERROR (Status);
> +          goto exit_handler;
> +        }
> +      }
> +    }
> +  }
> +
> +  /// Create called/invoked method name string
> +  Status = ConvertAslNameToAmlName (MethodNameString, &AmlNameString);
> +  if (EFI_ERROR (Status)) {
> +    ASSERT_EFI_ERROR (Status);
> +    goto exit_handler;
> +  }
> +
> +  Status = AmlGetNameStringSize (AmlNameString, &AmlNameStringSize);
> +  if (EFI_ERROR (Status)) {
> +    ASSERT_EFI_ERROR (Status);
> +    FreePool (AmlNameString);
> +    goto exit_handler;
> +  }
> +
> +  DataNode = NULL;
> +  Status   = AmlCreateDataNode (
> +               EAmlNodeDataTypeNameString,
> +               (UINT8 *)AmlNameString,
> +               AmlNameStringSize,
> +               &DataNode
> +               );
> +  FreePool (AmlNameString);
> +  if (EFI_ERROR (Status)) {
> +    ASSERT_EFI_ERROR (Status);
> +    goto exit_handler;
> +  }
> +
> +  Status = AmlVarListAddTail (
> +             (AML_NODE_HANDLE)ParentNode,
> +             (AML_NODE_HANDLE)DataNode
> +             );
> +  if (EFI_ERROR (Status)) {

I think DataNode would not be freed if we fail here.

> +    ASSERT_EFI_ERROR (Status);
> +    goto exit_handler;
> +  }
> +
> +  Status = AmlVarListAddTail (
> +             (AML_NODE_HANDLE)ParentNode,
> +             HeadNode
> +             );
> +  if (EFI_ERROR (Status)) {
> +    ASSERT_EFI_ERROR (Status);
> +    goto exit_handler;
> +  }
> +
> +  return Status;
> +
> +exit_handler:
> +  if (HeadNode != NULL) {
> +    AmlDeleteTree (HeadNode);
> +  }
> +
> +  return Status;
> +}


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112732): https://edk2.groups.io/g/devel/message/112732
Mute This Topic: https://groups.io/mt/103256864/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [Resend PATCH v4 4/4] DynamicTablesPkg: AML Code generation to invoke a method

[Abdul Lateef Attar via groups.io]

[Public]

Hi Pierre,
        Regarding the below comment
> +AmlCodeGenInvokeMethod (
> +  IN  CONST CHAR8             *MethodNameString,
> +  IN        UINT8             NumArgs,
> +  IN        AML_METHOD_PARAM  *Parameters   OPTIONAL,
> +  IN        AML_NODE_HANDLE   ParentNode
Maybe it would be worth also having an OUT parameter in case someone doesn't want to attach the method invocation directly (cf. other methods in the file):
   OUT AML_OBJECT_NODE_HANDLE  *NewObjectNode   OPTIONAL

I don’t think its possible because MethodInvocation is a stream of objects, and first object is NameString(method name) which is "Data node".
AML library doesn’t allow adding variable list arguments to the "Data node", only "object node" is supported.

I'll address the remaining comments and submit the v5 patch.

Thanks
AbduL

-----Original Message-----
From: Pierre Gondois <pierre.gondois@arm.com>
Sent: Tuesday, December 19, 2023 7:41 PM
To: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>; devel@edk2.groups.io
Cc: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>; Sami Mujawar <sami.mujawar@arm.com>
Subject: Re: [Resend PATCH v4 4/4] DynamicTablesPkg: AML Code generation to invoke a method

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


Hello Abdul,
The new function looks good, there is just one small issue with the tree layout,

Regards,
Pierre

On 12/19/23 04:06, Abdul Lateef Attar wrote:
> From: Abdul Lateef Attar <AbdulLateef.Attar@amd.com>
>
> Adds API to generate AML code to invoke/call another method. Also
> provides ability to pass arguments of type integer, string, ArgObj or
> LocalObj.
>
> Cc: Pierre Gondois <pierre.gondois@arm.com>
> Cc: Sami Mujawar <sami.mujawar@arm.com>
> Signed-off-by: Abdul Lateef Attar <AbdulLateef.Attar@amd.com>
> ---
>   .../Include/Library/AmlLib/AmlLib.h           | 112 +++++++++
>   .../Common/AmlLib/CodeGen/AmlCodeGen.c        | 236 +++++++++++++++++-
>   2 files changed, 347 insertions(+), 1 deletion(-)
>
> diff --git a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
> b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
> index eb8740692f..fb16637f02 100644
> --- a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
> +++ b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
> @@ -101,6 +101,56 @@ typedef enum {
>     AmlAddressRangeMax      = 4
>   } AML_MEMORY_ATTRIBUTES_MTP;
>
> +/** Method parameter types
> +
> +  Possible values are:
> +    0 - AmlMethodParamTypeInteger
> +    1 - AmlMethodParamTypeString
> +    2 - AmlMethodParamTypeArg
> +    3 - AmlMethodParamTypeLocal
> +
> +  @par Reference(s)
> +  - ACPI 6.5, s20.2.5 "Term Objects Encoding"
> +
> +**/
> +typedef enum {
> +  AmlMethodParamTypeInteger = 0,
> +  AmlMethodParamTypeString  = 1,
> +  AmlMethodParamTypeArg     = 2,
> +  AmlMethodParamTypeLocal   = 3
> +} AML_METHOD_PARAM_TYPE;
> +
> +/** AML Method parameter data
> +  holds the AML method parameter data.
> +**/
> +typedef union {
> +  UINT8     Arg;
> +  UINT8     Local;
> +  UINT64    Integer;
> +  VOID      *Buffer;
> +} AML_METHOD_PARAM_DATA;
> +
> +/** structure to hold AML method parameter types
> +  Type  -   Type of parameter
> +  Data  -   holds data of parameter
> +            if Type is AmlMethodParamTypeInteger
> +              then Data is of type Integer to hold integer value.
> +            if Type is AmlMethodParamTypeString
> +              then Data contains null terminated string.
> +            If Type is AmlMethodParamTypeArg
> +              then Data contains the Argument number,
> +              0 to 6 are supported value.
> +            If Type is AmlMethodParamTypeLocal
> +              then Data contains the Local variable number,
> +              0 to 7 are supported value.
> +  DataSize - for future use
> +**/
> +typedef struct {
> +  AML_METHOD_PARAM_TYPE    Type;
> +  AML_METHOD_PARAM_DATA    Data;
> +  UINTN                    DataSize;
> +} AML_METHOD_PARAM;
> +
>   /** Parse the definition block.
>
>     The function parses the whole AML blob. It starts with the ACPI
> DSDT/SSDT @@ -1693,4 +1743,66 @@ AmlAddNameStringToNamedPackage (
>     IN AML_OBJECT_NODE_HANDLE  NamedNode
>     );
>
> +/** AML code generation to invoke/call another method.
> +
> +  This method is subset implementation of MethodInvocation  defined
> + in the ACPI specification 6.5,  section 20.2.5 "Term Objects
> + Encoding".
> +  Added integer, string, ArgObj and LocalObj support.
> +
> +  Example 1:
> +    AmlCodeGenInvokeMethod ("MET0", 0, NULL, ParentNode);
> +    is equivalent of the following ASL code:
> +      MET0 ();
> +
> +  Example 2:
> +    AML_METHOD_PARAM  Param[4];
> +    Param[0].Data.Integer = 0x100;
> +    Param[0].Type = AmlMethodParamTypeInteger;
> +    Param[1].Data.Buffer = "TEST";
> +    Param[1].Type = AmlMethodParamTypeString;
> +    Param[2].Data.Arg = 0;
> +    Param[2].Type = AmlMethodParamTypeArg;
> +    Param[3].Data.Local = 2;
> +    Param[3].Type = AmlMethodParamTypeLocal;
> +    AmlCodeGenInvokeMethod ("MET0", 4, Param, ParentNode);
> +
> +    is equivalent of the following ASL code:
> +      MET0 (0x100, "TEST", Arg0, Local2);
> +
> +  Example 3:
> +    AML_METHOD_PARAM  Param[2];
> +    Param[0].Data.Arg = 0;
> +    Param[0].Type = AmlMethodParamTypeArg;
> +    Param[1].Data.Integer = 0x100;
> +    Param[1].Type = AmlMethodParamTypeInteger;
> +    AmlCodeGenMethodRetNameString ("MET2", NULL, 2, TRUE, 0, ParentNode, &MethodNode);
> +    AmlCodeGenInvokeMethod ("MET3", 2, Param, MethodNode);
> +
> +    is equivalent of the following ASL code:
> +    Method (MET2, 2, Serialized)
> +    {
> +      MET3 (Arg0, 0x0100)
> +    }
> +
> +  @param [in] MethodNameString  Method name to be called/invoked.
> +  @param [in] NumArgs           Number of arguments to be passed,
> +                                0 to 7 are permissible values.
> +  @param [in] Parameters        contains the parameter data.
> +  @param [in] ParentNode        set ParentNode as the parent> +                                of the node created.
> +
> +  @retval EFI_SUCCESS             Success.
> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
> +  @retval EFI_OUT_OF_RESOURCES    Failed to allocate memory.
> + **/
> +EFI_STATUS
> +EFIAPI
> +AmlCodeGenInvokeMethod (
> +  IN  CONST CHAR8             *MethodNameString,
> +  IN        UINT8             NumArgs,
> +  IN        AML_METHOD_PARAM  *Parameters   OPTIONAL,
> +  IN        AML_NODE_HANDLE   ParentNode
> +  );
> +
>   #endif // AML_LIB_H_
> diff --git
> a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
> b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
> index a6db34fb97..688eefdcef 100644
> --- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
> +++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
> @@ -2,6 +2,7 @@
>     AML Code Generation.
>
>     Copyright (c) 2020 - 2022, Arm Limited. All rights reserved.<BR>
> +  Copyright (C) 2023 Advanced Micro Devices, Inc. All rights
> + reserved.<BR>
>
>     SPDX-License-Identifier: BSD-2-Clause-Patent
>   **/
> @@ -921,7 +922,7 @@ AmlCodeGenNameUnicodeString (
>     Status = AmlCreateDataNode (
>                EAmlNodeDataTypeRaw,
>                (CONST UINT8 *)String,
> -             StrSize (String),
> +             (UINT32)StrSize (String),
>                &DataNode
>                );
>     if (EFI_ERROR (Status)) {
> @@ -3849,3 +3850,236 @@ exit_handler:
>
>     return Status;
>   }
> +
> +/** AML code generation to invoke/call another method.
> +
> +  This method is subset implementation of MethodInvocation  defined
> + in the ACPI specification 6.5,  section 20.2.5 "Term Objects
> + Encoding".
> +  Added integer, string, ArgObj and LocalObj support.
> +
> +  Example 1:
> +    AmlCodeGenInvokeMethod ("MET0", 0, NULL, ParentNode);
> +    is equivalent of the following ASL code:
> +      MET0 ();
> +
> +  Example 2:
> +    AML_METHOD_PARAM  Param[4];
> +    Param[0].Data.Integer = 0x100;
> +    Param[0].Type = AmlMethodParamTypeInteger;
> +    Param[1].Data.Buffer = "TEST";
> +    Param[1].Type = AmlMethodParamTypeString;
> +    Param[2].Data.Arg = 0;
> +    Param[2].Type = AmlMethodParamTypeArg;
> +    Param[3].Data.Local = 2;
> +    Param[3].Type = AmlMethodParamTypeLocal;
> +    AmlCodeGenInvokeMethod ("MET0", 4, Param, ParentNode);
> +
> +    is equivalent of the following ASL code:
> +      MET0 (0x100, "TEST", Arg0, Local2);
> +
> +  Example 3:
> +    AML_METHOD_PARAM  Param[2];
> +    Param[0].Data.Arg = 0;
> +    Param[0].Type = AmlMethodParamTypeArg;
> +    Param[1].Data.Integer = 0x100;
> +    Param[1].Type = AmlMethodParamTypeInteger;
> +    AmlCodeGenMethodRetNameString ("MET2", NULL, 2, TRUE, 0, ParentNode, &MethodNode);
> +    AmlCodeGenInvokeMethod ("MET3", 2, Param, MethodNode);
> +
> +    is equivalent of the following ASL code:
> +    Method (MET2, 2, Serialized)
> +    {
> +      MET3 (Arg0, 0x0100)
> +    }
> +
> +  @param [in] MethodNameString  Method name to be called/invoked.
> +  @param [in] NumArgs           Number of arguments to be passed,
> +                                0 to 7 are permissible values.
> +  @param [in] Parameters        contains the parameter data.
> +  @param [in] ParentNode        set ParentNode as the parent

NIT: set->Set

> +                                of the node created.
> +
> +  @retval EFI_SUCCESS             Success.
> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
> +  @retval EFI_OUT_OF_RESOURCES    Failed to allocate memory.
> + **/
> +EFI_STATUS
> +EFIAPI
> +AmlCodeGenInvokeMethod (
> +  IN  CONST CHAR8             *MethodNameString,
> +  IN        UINT8             NumArgs,
> +  IN        AML_METHOD_PARAM  *Parameters   OPTIONAL,
> +  IN        AML_NODE_HANDLE   ParentNode

Maybe it would be worth also having an OUT parameter in case someone doesn't want to attach the method invocation directly (cf. other methods in the file):
   OUT AML_OBJECT_NODE_HANDLE  *NewObjectNode   OPTIONAL


> +  )
> +{
> +  EFI_STATUS       Status;
> +  UINT8            Index;
> +  CHAR8            *AmlNameString;
> +  UINT32           AmlNameStringSize;
> +  AML_DATA_NODE    *DataNode;
> +  AML_OBJECT_NODE  *IntNode;
> +  AML_NODE_HANDLE  HeadNode;
> +  AML_NODE_HANDLE  CurrentNode;
> +  AML_OBJECT_NODE  *ObjectNode;
> +
> +  if ((MethodNameString == NULL) || (ParentNode == NULL)) {
> +    ASSERT (0);
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  if ((NumArgs > 7) ||
> +      ((Parameters == NULL) && (NumArgs > 0)))  {
> +    ASSERT (0);
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  if (Parameters != NULL) {
> +    /// Validate and convert the Parameters to the stream of node
> +    HeadNode    = NULL;
> +    CurrentNode = NULL;
> +    for (Index = 0; Index < NumArgs; Index++) {
> +      switch (Parameters[Index].Type) {
> +        case AmlMethodParamTypeInteger:
> +          IntNode = NULL;
> +          Status  = AmlCodeGenInteger (Parameters[Index].Data.Integer, &IntNode);
> +          if (EFI_ERROR (Status)) {
> +            ASSERT_EFI_ERROR (Status);
> +            goto exit_handler;
> +          }
> +
> +          CurrentNode = (AML_NODE_HANDLE)IntNode;
> +          break;
> +        case AmlMethodParamTypeString:
> +          ObjectNode = NULL;
> +          if (Parameters[Index].Data.Buffer == NULL) {
> +            ASSERT_EFI_ERROR (EFI_INVALID_PARAMETER);
> +            Status = EFI_INVALID_PARAMETER;
> +            goto exit_handler;
> +          }
> +
> +          Status = AmlCodeGenString (Parameters[Index].Data.Buffer, &ObjectNode);
> +          if (EFI_ERROR (Status)) {
> +            ASSERT_EFI_ERROR (Status);
> +            goto exit_handler;
> +          }
> +
> +          CurrentNode = (AML_NODE_HANDLE)ObjectNode;
> +          break;
> +        case AmlMethodParamTypeArg:
> +          ObjectNode = NULL;
> +          if (Parameters[Index].Data.Arg > (UINT8)(AML_ARG6 - AML_ARG0)) {
> +            ASSERT_EFI_ERROR (EFI_INVALID_PARAMETER);
> +            Status = EFI_INVALID_PARAMETER;
> +            goto exit_handler;
> +          }
> +
> +          Status = AmlCreateObjectNode (
> +                     AmlGetByteEncodingByOpCode (AML_ARG0 + Parameters[Index].Data.Arg, 0),
> +                     0,
> +                     &ObjectNode
> +                     );
> +          if (EFI_ERROR (Status)) {
> +            ASSERT_EFI_ERROR (Status);
> +            goto exit_handler;
> +          }
> +
> +          CurrentNode = (AML_NODE_HANDLE)ObjectNode;
> +          break;
> +        case AmlMethodParamTypeLocal:
> +          ObjectNode = NULL;
> +          if (Parameters[Index].Data.Local > (UINT8)(AML_LOCAL7 - AML_LOCAL0)) {
> +            ASSERT_EFI_ERROR (EFI_INVALID_PARAMETER);
> +            Status = EFI_INVALID_PARAMETER;
> +            goto exit_handler;
> +          }
> +
> +          Status = AmlCreateObjectNode (
> +                     AmlGetByteEncodingByOpCode (AML_LOCAL0 + Parameters[Index].Data.Local, 0),
> +                     0,
> +                     &ObjectNode
> +                     );
> +          if (EFI_ERROR (Status)) {
> +            ASSERT_EFI_ERROR (Status);
> +            goto exit_handler;
> +          }
> +
> +          CurrentNode = (AML_NODE_HANDLE)ObjectNode;
> +          break;
> +        default:
> +          ASSERT_EFI_ERROR (EFI_INVALID_PARAMETER);
> +          Status = EFI_INVALID_PARAMETER;
> +          goto exit_handler;
> +          break;
> +      }
> +
> +      if (HeadNode == NULL) {
> +        HeadNode = CurrentNode;

I think there is an issue here. The method arguments should be attached to the variable list of arguments of the parent, not of another argument.

For instance, the following:
Scope (_SB) {
   MET0 (Arg0, Arg1, Arg2)
}

Should create a tree as:
_SB
\- (Variable list of arguments of _SB)
   - MET0
   - Arg0
   - Arg1
   - Arg2

With the present code, the created tree is:
_SB
\- (Variable list of arguments of _SB)
   - MET0
   - Arg0
   \- (Variable list of arguments of Arg0)
     - Arg1
     - Arg2

When serializing the tree, the generated binary is identical, but the tree is incorrect.

> +      } else {
> +        Status = AmlVarListAddTail (
> +                   HeadNode,
> +                   CurrentNode
> +                   );
> +        if (EFI_ERROR (Status)) {
> +          ASSERT_EFI_ERROR (Status);
> +          goto exit_handler;
> +        }
> +      }
> +    }
> +  }
> +
> +  /// Create called/invoked method name string  Status =
> + ConvertAslNameToAmlName (MethodNameString, &AmlNameString);  if
> + (EFI_ERROR (Status)) {
> +    ASSERT_EFI_ERROR (Status);
> +    goto exit_handler;
> +  }
> +
> +  Status = AmlGetNameStringSize (AmlNameString, &AmlNameStringSize);
> + if (EFI_ERROR (Status)) {
> +    ASSERT_EFI_ERROR (Status);
> +    FreePool (AmlNameString);
> +    goto exit_handler;
> +  }
> +
> +  DataNode = NULL;
> +  Status   = AmlCreateDataNode (
> +               EAmlNodeDataTypeNameString,
> +               (UINT8 *)AmlNameString,
> +               AmlNameStringSize,
> +               &DataNode
> +               );
> +  FreePool (AmlNameString);
> +  if (EFI_ERROR (Status)) {
> +    ASSERT_EFI_ERROR (Status);
> +    goto exit_handler;
> +  }
> +
> +  Status = AmlVarListAddTail (
> +             (AML_NODE_HANDLE)ParentNode,
> +             (AML_NODE_HANDLE)DataNode
> +             );
> +  if (EFI_ERROR (Status)) {

I think DataNode would not be freed if we fail here.

> +    ASSERT_EFI_ERROR (Status);
> +    goto exit_handler;
> +  }
> +
> +  Status = AmlVarListAddTail (
> +             (AML_NODE_HANDLE)ParentNode,
> +             HeadNode
> +             );
> +  if (EFI_ERROR (Status)) {
> +    ASSERT_EFI_ERROR (Status);
> +    goto exit_handler;
> +  }
> +
> +  return Status;
> +
> +exit_handler:
> +  if (HeadNode != NULL) {
> +    AmlDeleteTree (HeadNode);
> +  }
> +
> +  return Status;
> +}


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112742): https://edk2.groups.io/g/devel/message/112742
Mute This Topic: https://groups.io/mt/103256864/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [Resend PATCH v4 4/4] DynamicTablesPkg: AML Code generation to invoke a method

[PierreGondois]

Hello Abdul,

On 12/20/23 05:33, Attar, AbdulLateef (Abdul Lateef) wrote:
> [Public]
> 
> Hi Pierre,
>          Regarding the below comment
>> +AmlCodeGenInvokeMethod (
>> +  IN  CONST CHAR8             *MethodNameString,
>> +  IN        UINT8             NumArgs,
>> +  IN        AML_METHOD_PARAM  *Parameters   OPTIONAL,
>> +  IN        AML_NODE_HANDLE   ParentNode
> Maybe it would be worth also having an OUT parameter in case someone doesn't want to attach the method invocation directly (cf. other methods in the file):
>     OUT AML_OBJECT_NODE_HANDLE  *NewObjectNode   OPTIONAL
> 
> I don’t think its possible because MethodInvocation is a stream of objects, and first object is NameString(method name) which is "Data node".
> AML library doesn’t allow adding variable list arguments to the "Data node", only "object node" is supported.

Yes right,

Regards,
Pierre

> 
> I'll address the remaining comments and submit the v5 patch.
> 
> Thanks
> AbduL
> 
> -----Original Message-----
> From: Pierre Gondois <pierre.gondois@arm.com>
> Sent: Tuesday, December 19, 2023 7:41 PM
> To: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>; devel@edk2.groups.io
> Cc: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>; Sami Mujawar <sami.mujawar@arm.com>
> Subject: Re: [Resend PATCH v4 4/4] DynamicTablesPkg: AML Code generation to invoke a method
> 
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> 
> 
> Hello Abdul,
> The new function looks good, there is just one small issue with the tree layout,
> 
> Regards,
> Pierre
> 
> On 12/19/23 04:06, Abdul Lateef Attar wrote:
>> From: Abdul Lateef Attar <AbdulLateef.Attar@amd.com>
>>
>> Adds API to generate AML code to invoke/call another method. Also
>> provides ability to pass arguments of type integer, string, ArgObj or
>> LocalObj.
>>
>> Cc: Pierre Gondois <pierre.gondois@arm.com>
>> Cc: Sami Mujawar <sami.mujawar@arm.com>
>> Signed-off-by: Abdul Lateef Attar <AbdulLateef.Attar@amd.com>
>> ---
>>    .../Include/Library/AmlLib/AmlLib.h           | 112 +++++++++
>>    .../Common/AmlLib/CodeGen/AmlCodeGen.c        | 236 +++++++++++++++++-
>>    2 files changed, 347 insertions(+), 1 deletion(-)
>>
>> diff --git a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
>> b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
>> index eb8740692f..fb16637f02 100644
>> --- a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
>> +++ b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
>> @@ -101,6 +101,56 @@ typedef enum {
>>      AmlAddressRangeMax      = 4
>>    } AML_MEMORY_ATTRIBUTES_MTP;
>>
>> +/** Method parameter types
>> +
>> +  Possible values are:
>> +    0 - AmlMethodParamTypeInteger
>> +    1 - AmlMethodParamTypeString
>> +    2 - AmlMethodParamTypeArg
>> +    3 - AmlMethodParamTypeLocal
>> +
>> +  @par Reference(s)
>> +  - ACPI 6.5, s20.2.5 "Term Objects Encoding"
>> +
>> +**/
>> +typedef enum {
>> +  AmlMethodParamTypeInteger = 0,
>> +  AmlMethodParamTypeString  = 1,
>> +  AmlMethodParamTypeArg     = 2,
>> +  AmlMethodParamTypeLocal   = 3
>> +} AML_METHOD_PARAM_TYPE;
>> +
>> +/** AML Method parameter data
>> +  holds the AML method parameter data.
>> +**/
>> +typedef union {
>> +  UINT8     Arg;
>> +  UINT8     Local;
>> +  UINT64    Integer;
>> +  VOID      *Buffer;
>> +} AML_METHOD_PARAM_DATA;
>> +
>> +/** structure to hold AML method parameter types
>> +  Type  -   Type of parameter
>> +  Data  -   holds data of parameter
>> +            if Type is AmlMethodParamTypeInteger
>> +              then Data is of type Integer to hold integer value.
>> +            if Type is AmlMethodParamTypeString
>> +              then Data contains null terminated string.
>> +            If Type is AmlMethodParamTypeArg
>> +              then Data contains the Argument number,
>> +              0 to 6 are supported value.
>> +            If Type is AmlMethodParamTypeLocal
>> +              then Data contains the Local variable number,
>> +              0 to 7 are supported value.
>> +  DataSize - for future use
>> +**/
>> +typedef struct {
>> +  AML_METHOD_PARAM_TYPE    Type;
>> +  AML_METHOD_PARAM_DATA    Data;
>> +  UINTN                    DataSize;
>> +} AML_METHOD_PARAM;
>> +
>>    /** Parse the definition block.
>>
>>      The function parses the whole AML blob. It starts with the ACPI
>> DSDT/SSDT @@ -1693,4 +1743,66 @@ AmlAddNameStringToNamedPackage (
>>      IN AML_OBJECT_NODE_HANDLE  NamedNode
>>      );
>>
>> +/** AML code generation to invoke/call another method.
>> +
>> +  This method is subset implementation of MethodInvocation  defined
>> + in the ACPI specification 6.5,  section 20.2.5 "Term Objects
>> + Encoding".
>> +  Added integer, string, ArgObj and LocalObj support.
>> +
>> +  Example 1:
>> +    AmlCodeGenInvokeMethod ("MET0", 0, NULL, ParentNode);
>> +    is equivalent of the following ASL code:
>> +      MET0 ();
>> +
>> +  Example 2:
>> +    AML_METHOD_PARAM  Param[4];
>> +    Param[0].Data.Integer = 0x100;
>> +    Param[0].Type = AmlMethodParamTypeInteger;
>> +    Param[1].Data.Buffer = "TEST";
>> +    Param[1].Type = AmlMethodParamTypeString;
>> +    Param[2].Data.Arg = 0;
>> +    Param[2].Type = AmlMethodParamTypeArg;
>> +    Param[3].Data.Local = 2;
>> +    Param[3].Type = AmlMethodParamTypeLocal;
>> +    AmlCodeGenInvokeMethod ("MET0", 4, Param, ParentNode);
>> +
>> +    is equivalent of the following ASL code:
>> +      MET0 (0x100, "TEST", Arg0, Local2);
>> +
>> +  Example 3:
>> +    AML_METHOD_PARAM  Param[2];
>> +    Param[0].Data.Arg = 0;
>> +    Param[0].Type = AmlMethodParamTypeArg;
>> +    Param[1].Data.Integer = 0x100;
>> +    Param[1].Type = AmlMethodParamTypeInteger;
>> +    AmlCodeGenMethodRetNameString ("MET2", NULL, 2, TRUE, 0, ParentNode, &MethodNode);
>> +    AmlCodeGenInvokeMethod ("MET3", 2, Param, MethodNode);
>> +
>> +    is equivalent of the following ASL code:
>> +    Method (MET2, 2, Serialized)
>> +    {
>> +      MET3 (Arg0, 0x0100)
>> +    }
>> +
>> +  @param [in] MethodNameString  Method name to be called/invoked.
>> +  @param [in] NumArgs           Number of arguments to be passed,
>> +                                0 to 7 are permissible values.
>> +  @param [in] Parameters        contains the parameter data.
>> +  @param [in] ParentNode        set ParentNode as the parent> +                                of the node created.
>> +
>> +  @retval EFI_SUCCESS             Success.
>> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
>> +  @retval EFI_OUT_OF_RESOURCES    Failed to allocate memory.
>> + **/
>> +EFI_STATUS
>> +EFIAPI
>> +AmlCodeGenInvokeMethod (
>> +  IN  CONST CHAR8             *MethodNameString,
>> +  IN        UINT8             NumArgs,
>> +  IN        AML_METHOD_PARAM  *Parameters   OPTIONAL,
>> +  IN        AML_NODE_HANDLE   ParentNode
>> +  );
>> +
>>    #endif // AML_LIB_H_
>> diff --git
>> a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
>> b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
>> index a6db34fb97..688eefdcef 100644
>> --- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
>> +++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
>> @@ -2,6 +2,7 @@
>>      AML Code Generation.
>>
>>      Copyright (c) 2020 - 2022, Arm Limited. All rights reserved.<BR>
>> +  Copyright (C) 2023 Advanced Micro Devices, Inc. All rights
>> + reserved.<BR>
>>
>>      SPDX-License-Identifier: BSD-2-Clause-Patent
>>    **/
>> @@ -921,7 +922,7 @@ AmlCodeGenNameUnicodeString (
>>      Status = AmlCreateDataNode (
>>                 EAmlNodeDataTypeRaw,
>>                 (CONST UINT8 *)String,
>> -             StrSize (String),
>> +             (UINT32)StrSize (String),
>>                 &DataNode
>>                 );
>>      if (EFI_ERROR (Status)) {
>> @@ -3849,3 +3850,236 @@ exit_handler:
>>
>>      return Status;
>>    }
>> +
>> +/** AML code generation to invoke/call another method.
>> +
>> +  This method is subset implementation of MethodInvocation  defined
>> + in the ACPI specification 6.5,  section 20.2.5 "Term Objects
>> + Encoding".
>> +  Added integer, string, ArgObj and LocalObj support.
>> +
>> +  Example 1:
>> +    AmlCodeGenInvokeMethod ("MET0", 0, NULL, ParentNode);
>> +    is equivalent of the following ASL code:
>> +      MET0 ();
>> +
>> +  Example 2:
>> +    AML_METHOD_PARAM  Param[4];
>> +    Param[0].Data.Integer = 0x100;
>> +    Param[0].Type = AmlMethodParamTypeInteger;
>> +    Param[1].Data.Buffer = "TEST";
>> +    Param[1].Type = AmlMethodParamTypeString;
>> +    Param[2].Data.Arg = 0;
>> +    Param[2].Type = AmlMethodParamTypeArg;
>> +    Param[3].Data.Local = 2;
>> +    Param[3].Type = AmlMethodParamTypeLocal;
>> +    AmlCodeGenInvokeMethod ("MET0", 4, Param, ParentNode);
>> +
>> +    is equivalent of the following ASL code:
>> +      MET0 (0x100, "TEST", Arg0, Local2);
>> +
>> +  Example 3:
>> +    AML_METHOD_PARAM  Param[2];
>> +    Param[0].Data.Arg = 0;
>> +    Param[0].Type = AmlMethodParamTypeArg;
>> +    Param[1].Data.Integer = 0x100;
>> +    Param[1].Type = AmlMethodParamTypeInteger;
>> +    AmlCodeGenMethodRetNameString ("MET2", NULL, 2, TRUE, 0, ParentNode, &MethodNode);
>> +    AmlCodeGenInvokeMethod ("MET3", 2, Param, MethodNode);
>> +
>> +    is equivalent of the following ASL code:
>> +    Method (MET2, 2, Serialized)
>> +    {
>> +      MET3 (Arg0, 0x0100)
>> +    }
>> +
>> +  @param [in] MethodNameString  Method name to be called/invoked.
>> +  @param [in] NumArgs           Number of arguments to be passed,
>> +                                0 to 7 are permissible values.
>> +  @param [in] Parameters        contains the parameter data.
>> +  @param [in] ParentNode        set ParentNode as the parent
> 
> NIT: set->Set
> 
>> +                                of the node created.
>> +
>> +  @retval EFI_SUCCESS             Success.
>> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
>> +  @retval EFI_OUT_OF_RESOURCES    Failed to allocate memory.
>> + **/
>> +EFI_STATUS
>> +EFIAPI
>> +AmlCodeGenInvokeMethod (
>> +  IN  CONST CHAR8             *MethodNameString,
>> +  IN        UINT8             NumArgs,
>> +  IN        AML_METHOD_PARAM  *Parameters   OPTIONAL,
>> +  IN        AML_NODE_HANDLE   ParentNode
> 
> Maybe it would be worth also having an OUT parameter in case someone doesn't want to attach the method invocation directly (cf. other methods in the file):
>     OUT AML_OBJECT_NODE_HANDLE  *NewObjectNode   OPTIONAL
> 
> 
>> +  )
>> +{
>> +  EFI_STATUS       Status;
>> +  UINT8            Index;
>> +  CHAR8            *AmlNameString;
>> +  UINT32           AmlNameStringSize;
>> +  AML_DATA_NODE    *DataNode;
>> +  AML_OBJECT_NODE  *IntNode;
>> +  AML_NODE_HANDLE  HeadNode;
>> +  AML_NODE_HANDLE  CurrentNode;
>> +  AML_OBJECT_NODE  *ObjectNode;
>> +
>> +  if ((MethodNameString == NULL) || (ParentNode == NULL)) {
>> +    ASSERT (0);
>> +    return EFI_INVALID_PARAMETER;
>> +  }
>> +
>> +  if ((NumArgs > 7) ||
>> +      ((Parameters == NULL) && (NumArgs > 0)))  {
>> +    ASSERT (0);
>> +    return EFI_INVALID_PARAMETER;
>> +  }
>> +
>> +  if (Parameters != NULL) {
>> +    /// Validate and convert the Parameters to the stream of node
>> +    HeadNode    = NULL;
>> +    CurrentNode = NULL;
>> +    for (Index = 0; Index < NumArgs; Index++) {
>> +      switch (Parameters[Index].Type) {
>> +        case AmlMethodParamTypeInteger:
>> +          IntNode = NULL;
>> +          Status  = AmlCodeGenInteger (Parameters[Index].Data.Integer, &IntNode);
>> +          if (EFI_ERROR (Status)) {
>> +            ASSERT_EFI_ERROR (Status);
>> +            goto exit_handler;
>> +          }
>> +
>> +          CurrentNode = (AML_NODE_HANDLE)IntNode;
>> +          break;
>> +        case AmlMethodParamTypeString:
>> +          ObjectNode = NULL;
>> +          if (Parameters[Index].Data.Buffer == NULL) {
>> +            ASSERT_EFI_ERROR (EFI_INVALID_PARAMETER);
>> +            Status = EFI_INVALID_PARAMETER;
>> +            goto exit_handler;
>> +          }
>> +
>> +          Status = AmlCodeGenString (Parameters[Index].Data.Buffer, &ObjectNode);
>> +          if (EFI_ERROR (Status)) {
>> +            ASSERT_EFI_ERROR (Status);
>> +            goto exit_handler;
>> +          }
>> +
>> +          CurrentNode = (AML_NODE_HANDLE)ObjectNode;
>> +          break;
>> +        case AmlMethodParamTypeArg:
>> +          ObjectNode = NULL;
>> +          if (Parameters[Index].Data.Arg > (UINT8)(AML_ARG6 - AML_ARG0)) {
>> +            ASSERT_EFI_ERROR (EFI_INVALID_PARAMETER);
>> +            Status = EFI_INVALID_PARAMETER;
>> +            goto exit_handler;
>> +          }
>> +
>> +          Status = AmlCreateObjectNode (
>> +                     AmlGetByteEncodingByOpCode (AML_ARG0 + Parameters[Index].Data.Arg, 0),
>> +                     0,
>> +                     &ObjectNode
>> +                     );
>> +          if (EFI_ERROR (Status)) {
>> +            ASSERT_EFI_ERROR (Status);
>> +            goto exit_handler;
>> +          }
>> +
>> +          CurrentNode = (AML_NODE_HANDLE)ObjectNode;
>> +          break;
>> +        case AmlMethodParamTypeLocal:
>> +          ObjectNode = NULL;
>> +          if (Parameters[Index].Data.Local > (UINT8)(AML_LOCAL7 - AML_LOCAL0)) {
>> +            ASSERT_EFI_ERROR (EFI_INVALID_PARAMETER);
>> +            Status = EFI_INVALID_PARAMETER;
>> +            goto exit_handler;
>> +          }
>> +
>> +          Status = AmlCreateObjectNode (
>> +                     AmlGetByteEncodingByOpCode (AML_LOCAL0 + Parameters[Index].Data.Local, 0),
>> +                     0,
>> +                     &ObjectNode
>> +                     );
>> +          if (EFI_ERROR (Status)) {
>> +            ASSERT_EFI_ERROR (Status);
>> +            goto exit_handler;
>> +          }
>> +
>> +          CurrentNode = (AML_NODE_HANDLE)ObjectNode;
>> +          break;
>> +        default:
>> +          ASSERT_EFI_ERROR (EFI_INVALID_PARAMETER);
>> +          Status = EFI_INVALID_PARAMETER;
>> +          goto exit_handler;
>> +          break;
>> +      }
>> +
>> +      if (HeadNode == NULL) {
>> +        HeadNode = CurrentNode;
> 
> I think there is an issue here. The method arguments should be attached to the variable list of arguments of the parent, not of another argument.
> 
> For instance, the following:
> Scope (_SB) {
>     MET0 (Arg0, Arg1, Arg2)
> }
> 
> Should create a tree as:
> _SB
> \- (Variable list of arguments of _SB)
>     - MET0
>     - Arg0
>     - Arg1
>     - Arg2
> 
> With the present code, the created tree is:
> _SB
> \- (Variable list of arguments of _SB)
>     - MET0
>     - Arg0
>     \- (Variable list of arguments of Arg0)
>       - Arg1
>       - Arg2
> 
> When serializing the tree, the generated binary is identical, but the tree is incorrect.
> 
>> +      } else {
>> +        Status = AmlVarListAddTail (
>> +                   HeadNode,
>> +                   CurrentNode
>> +                   );
>> +        if (EFI_ERROR (Status)) {
>> +          ASSERT_EFI_ERROR (Status);
>> +          goto exit_handler;
>> +        }
>> +      }
>> +    }
>> +  }
>> +
>> +  /// Create called/invoked method name string  Status =
>> + ConvertAslNameToAmlName (MethodNameString, &AmlNameString);  if
>> + (EFI_ERROR (Status)) {
>> +    ASSERT_EFI_ERROR (Status);
>> +    goto exit_handler;
>> +  }
>> +
>> +  Status = AmlGetNameStringSize (AmlNameString, &AmlNameStringSize);
>> + if (EFI_ERROR (Status)) {
>> +    ASSERT_EFI_ERROR (Status);
>> +    FreePool (AmlNameString);
>> +    goto exit_handler;
>> +  }
>> +
>> +  DataNode = NULL;
>> +  Status   = AmlCreateDataNode (
>> +               EAmlNodeDataTypeNameString,
>> +               (UINT8 *)AmlNameString,
>> +               AmlNameStringSize,
>> +               &DataNode
>> +               );
>> +  FreePool (AmlNameString);
>> +  if (EFI_ERROR (Status)) {
>> +    ASSERT_EFI_ERROR (Status);
>> +    goto exit_handler;
>> +  }
>> +
>> +  Status = AmlVarListAddTail (
>> +             (AML_NODE_HANDLE)ParentNode,
>> +             (AML_NODE_HANDLE)DataNode
>> +             );
>> +  if (EFI_ERROR (Status)) {
> 
> I think DataNode would not be freed if we fail here.
> 
>> +    ASSERT_EFI_ERROR (Status);
>> +    goto exit_handler;
>> +  }
>> +
>> +  Status = AmlVarListAddTail (
>> +             (AML_NODE_HANDLE)ParentNode,
>> +             HeadNode
>> +             );
>> +  if (EFI_ERROR (Status)) {
>> +    ASSERT_EFI_ERROR (Status);
>> +    goto exit_handler;
>> +  }
>> +
>> +  return Status;
>> +
>> +exit_handler:
>> +  if (HeadNode != NULL) {
>> +    AmlDeleteTree (HeadNode);
>> +  }
>> +
>> +  return Status;
>> +}


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#112750): https://edk2.groups.io/g/devel/message/112750
Mute This Topic: https://groups.io/mt/103256864/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-