public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [edk2-devel] [PATCH v4 0/4] Add support for generating ACPI ThermalZones
@ 2023-09-18 15:46 Jeff Brasen via groups.io
  2023-09-18 15:46 ` [edk2-devel] [PATCH v4 1/4] DynamicTablesPkg: Add ThermalZone CodeGen function Jeff Brasen via groups.io
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Jeff Brasen via groups.io @ 2023-09-18 15:46 UTC (permalink / raw)
  To: devel
  Cc: Alexei.Fedorov, pierre.gondois, Sami.Mujawar, swatisrik,
	ashishsingha, Jeff Brasen

Add APIs needed to create thermal zones dynamically.
Does not add a generator for this as creating the TMP method generically may
be difficult.

Change log:

v4 - Fixed an additional error handling path
v3 - Fixed a couple error handling paths
v2 - renamed NameString function and added goto dones in a couple error cases

Jeff Brasen (4):
  DynamicTablesPkg: Add ThermalZone CodeGen function
  DynamicTablesPkg: Add support for simple method invocation.
  DynamicTablesPkg: Add support to add Strings to package
  DynamicTablesPkg: Add Aml NameUnicodeString API

 .../Include/Library/AmlLib/AmlLib.h           | 130 +++++
 .../Common/AmlLib/CodeGen/AmlCodeGen.c        | 534 ++++++++++++++++++
 2 files changed, 664 insertions(+)

-- 
2.25.1



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



^ permalink raw reply	[flat|nested] 13+ messages in thread

* [edk2-devel] [PATCH v4 1/4] DynamicTablesPkg: Add ThermalZone CodeGen function
  2023-09-18 15:46 [edk2-devel] [PATCH v4 0/4] Add support for generating ACPI ThermalZones Jeff Brasen via groups.io
@ 2023-09-18 15:46 ` Jeff Brasen via groups.io
  2023-09-18 15:46 ` [edk2-devel] [PATCH v4 2/4] DynamicTablesPkg: Add support for simple method invocation Jeff Brasen via groups.io
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 13+ messages in thread
From: Jeff Brasen via groups.io @ 2023-09-18 15:46 UTC (permalink / raw)
  To: devel
  Cc: Alexei.Fedorov, pierre.gondois, Sami.Mujawar, swatisrik,
	ashishsingha, Jeff Brasen

Add API to generate a ThermalZone object to AmlLib.



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

---

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

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

 2 files changed, 144 insertions(+)



diff --git a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h

index 9210c50915..d201ae9499 100644

--- a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h

+++ b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h

@@ -1038,6 +1038,34 @@ AmlCodeGenDevice (

   OUT       AML_OBJECT_NODE_HANDLE  *NewObjectNode   OPTIONAL

   );

 

+/** AML code generation for a ThermalZone object node.

+

+  AmlCodeGenThermalZone ("TZ00", ParentNode, NewObjectNode) is

+  equivalent of the following ASL code:

+    ThermalZone(TZ00) {}

+

+  @ingroup CodeGenApis

+

+  @param  [in] NameString     The new ThermalZone's name.

+                              Must be a NULL-terminated ASL NameString

+                              e.g.: "DEV0", "DV15.DEV0", etc.

+                              The input string is copied.

+  @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

+AmlCodeGenThermalZone (

+  IN  CONST CHAR8                   *NameString,

+  IN        AML_NODE_HANDLE         ParentNode      OPTIONAL,

+  OUT       AML_OBJECT_NODE_HANDLE  *NewObjectNode   OPTIONAL

+  );

+

 /** AML code generation for a Scope object node.

 

   AmlCodeGenScope ("_SB", ParentNode, NewObjectNode) is

diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c

index 0b223379fa..88537b7e2d 100644

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

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

@@ -1218,6 +1218,122 @@ error_handler1:

   return Status;

 }

 

+/** AML code generation for a ThermalZone object node.

+

+  AmlCodeGenThermalZone ("TZ00", ParentNode, NewObjectNode) is

+  equivalent of the following ASL code:

+    ThermalZone(TZ00) {}

+

+  @ingroup CodeGenApis

+

+  @param  [in] NameString     The new ThermalZone's name.

+                              Must be a NULL-terminated ASL NameString

+                              e.g.: "DEV0", "DV15.DEV0", etc.

+                              The input string is copied.

+  @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

+AmlCodeGenThermalZone (

+  IN  CONST CHAR8                   *NameString,

+  IN        AML_NODE_HANDLE         ParentNode      OPTIONAL,

+  OUT       AML_OBJECT_NODE_HANDLE  *NewObjectNode   OPTIONAL

+  )

+{

+  EFI_STATUS       Status;

+  AML_OBJECT_NODE  *ObjectNode;

+  AML_DATA_NODE    *DataNode;

+  CHAR8            *AmlNameString;

+  UINT32           AmlNameStringSize;

+

+  if ((NameString == NULL)  ||

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

+  {

+    ASSERT (0);

+    return EFI_INVALID_PARAMETER;

+  }

+

+  ObjectNode    = NULL;

+  DataNode      = NULL;

+  AmlNameString = NULL;

+

+  Status = ConvertAslNameToAmlName (NameString, &AmlNameString);

+  if (EFI_ERROR (Status)) {

+    ASSERT (0);

+    return Status;

+  }

+

+  Status = AmlGetNameStringSize (AmlNameString, &AmlNameStringSize);

+  if (EFI_ERROR (Status)) {

+    ASSERT (0);

+    goto error_handler1;

+  }

+

+  Status = AmlCreateObjectNode (

+             AmlGetByteEncodingByOpCode (AML_EXT_OP, AML_EXT_THERMAL_ZONE_OP),

+             AmlNameStringSize + AmlComputePkgLengthWidth (AmlNameStringSize),

+             &ObjectNode

+             );

+  if (EFI_ERROR (Status)) {

+    ASSERT (0);

+    goto error_handler1;

+  }

+

+  Status = AmlCreateDataNode (

+             EAmlNodeDataTypeNameString,

+             (UINT8 *)AmlNameString,

+             AmlNameStringSize,

+             &DataNode

+             );

+  if (EFI_ERROR (Status)) {

+    ASSERT (0);

+    goto error_handler2;

+  }

+

+  Status = AmlSetFixedArgument (

+             ObjectNode,

+             EAmlParseIndexTerm0,

+             (AML_NODE_HEADER *)DataNode

+             );

+  if (EFI_ERROR (Status)) {

+    ASSERT (0);

+    AmlDeleteTree ((AML_NODE_HEADER *)DataNode);

+    goto error_handler2;

+  }

+

+  Status = LinkNode (

+             ObjectNode,

+             ParentNode,

+             NewObjectNode

+             );

+  if (EFI_ERROR (Status)) {

+    ASSERT (0);

+    goto error_handler2;

+  }

+

+  // Free AmlNameString before returning as it is copied

+  // in the call to AmlCreateDataNode().

+  goto error_handler1;

+

+error_handler2:

+  if (ObjectNode != NULL) {

+    AmlDeleteTree ((AML_NODE_HEADER *)ObjectNode);

+  }

+

+error_handler1:

+  if (AmlNameString != NULL) {

+    FreePool (AmlNameString);

+  }

+

+  return Status;

+}

+

 /** AML code generation for a Scope object node.

 

   AmlCodeGenScope ("_SB", ParentNode, NewObjectNode) is

-- 

2.25.1





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



^ permalink raw reply	[flat|nested] 13+ messages in thread

* [edk2-devel] [PATCH v4 2/4] DynamicTablesPkg: Add support for simple method invocation.
  2023-09-18 15:46 [edk2-devel] [PATCH v4 0/4] Add support for generating ACPI ThermalZones Jeff Brasen via groups.io
  2023-09-18 15:46 ` [edk2-devel] [PATCH v4 1/4] DynamicTablesPkg: Add ThermalZone CodeGen function Jeff Brasen via groups.io
@ 2023-09-18 15:46 ` Jeff Brasen via groups.io
  2023-09-21 16:49   ` Sami Mujawar
  2023-09-18 15:46 ` [edk2-devel] [PATCH v4 3/4] DynamicTablesPkg: Add support to add Strings to package Jeff Brasen via groups.io
  2023-09-18 15:47 ` [edk2-devel] [PATCH v4 4/4] DynamicTablesPkg: Add Aml NameUnicodeString API Jeff Brasen via groups.io
  3 siblings, 1 reply; 13+ messages in thread
From: Jeff Brasen via groups.io @ 2023-09-18 15:46 UTC (permalink / raw)
  To: devel
  Cc: Alexei.Fedorov, pierre.gondois, Sami.Mujawar, swatisrik,
	ashishsingha, Jeff Brasen

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/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 13+ messages in thread

* [edk2-devel] [PATCH v4 3/4] DynamicTablesPkg: Add support to add Strings to package
  2023-09-18 15:46 [edk2-devel] [PATCH v4 0/4] Add support for generating ACPI ThermalZones Jeff Brasen via groups.io
  2023-09-18 15:46 ` [edk2-devel] [PATCH v4 1/4] DynamicTablesPkg: Add ThermalZone CodeGen function Jeff Brasen via groups.io
  2023-09-18 15:46 ` [edk2-devel] [PATCH v4 2/4] DynamicTablesPkg: Add support for simple method invocation Jeff Brasen via groups.io
@ 2023-09-18 15:46 ` Jeff Brasen via groups.io
  2023-09-21 16:49   ` Sami Mujawar
  2023-09-18 15:47 ` [edk2-devel] [PATCH v4 4/4] DynamicTablesPkg: Add Aml NameUnicodeString API Jeff Brasen via groups.io
  3 siblings, 1 reply; 13+ messages in thread
From: Jeff Brasen via groups.io @ 2023-09-18 15:46 UTC (permalink / raw)
  To: devel
  Cc: Alexei.Fedorov, pierre.gondois, Sami.Mujawar, swatisrik,
	ashishsingha, Jeff Brasen

Add API to add a String to a package created with NamedPackage API.



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

---

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

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

 2 files changed, 105 insertions(+)



diff --git a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h

index b82c7a3ce8..f4a4908753 100644

--- a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h

+++ b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h

@@ -1472,4 +1472,21 @@ AmlCreateCpcNode (

   OUT AML_OBJECT_NODE_HANDLE  *NewCpcNode   OPTIONAL

   );

 

+/** AML code generation to add a NameString to the package in a named node.

+

+

+  @param [in]  NameString     NameString to add

+  @param [in]  NamedNode      Node to add the string to the included package.

+

+  @retval EFI_SUCCESS             Success.

+  @retval EFI_INVALID_PARAMETER   Invalid parameter.

+  @retval EFI_OUT_OF_RESOURCES    Failed to allocate memory.

+**/

+EFI_STATUS

+EFIAPI

+AmlAddNameStringToNamedPackage (

+  IN CHAR8                   *NameString,

+  IN AML_OBJECT_NODE_HANDLE  NamedNode

+  );

+

 #endif // AML_LIB_H_

diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c

index ea519d1aa8..2afd405750 100644

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

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

@@ -3685,3 +3685,91 @@ error_handler:

   AmlDeleteTree ((AML_NODE_HANDLE)CpcNode);

   return Status;

 }

+

+/** AML code generation to add a NameString to the package in a named node.

+

+

+  @param [in]  NameString     NameString to add

+  @param [in]  NamedNode      Node to add the string to the included package.

+

+  @retval EFI_SUCCESS             Success.

+  @retval EFI_INVALID_PARAMETER   Invalid parameter.

+  @retval EFI_OUT_OF_RESOURCES    Failed to allocate memory.

+**/

+EFI_STATUS

+EFIAPI

+AmlAddNameStringToNamedPackage (

+  IN CHAR8                   *NameString,

+  IN AML_OBJECT_NODE_HANDLE  NamedNode

+  )

+{

+  EFI_STATUS              Status;

+  AML_DATA_NODE           *DataNode;

+  CHAR8                   *AmlNameString;

+  UINT32                  AmlNameStringSize;

+  AML_OBJECT_NODE_HANDLE  PackageNode;

+

+  DataNode = NULL;

+

+  if ((NamedNode == NULL)                                              ||

+      (AmlGetNodeType ((AML_NODE_HANDLE)NamedNode) != EAmlNodeObject)  ||

+      (!AmlNodeHasOpCode (NamedNode, AML_NAME_OP, 0)))

+  {

+    ASSERT (0);

+    return EFI_INVALID_PARAMETER;

+  }

+

+  PackageNode = (AML_OBJECT_NODE_HANDLE)AmlGetFixedArgument (

+                                          NamedNode,

+                                          EAmlParseIndexTerm1

+                                          );

+  if ((PackageNode == NULL)                                             ||

+      (AmlGetNodeType ((AML_NODE_HANDLE)PackageNode) != EAmlNodeObject) ||

+      (!AmlNodeHasOpCode (PackageNode, AML_PACKAGE_OP, 0)))

+  {

+    ASSERT (0);

+    return EFI_INVALID_PARAMETER;

+  }

+

+  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 = AmlVarListAddTail (

+             (AML_NODE_HANDLE)PackageNode,

+             (AML_NODE_HANDLE)DataNode

+             );

+  ASSERT_EFI_ERROR (Status);

+

+exit_handler:

+  if (AmlNameString != NULL) {

+    FreePool (AmlNameString);

+  }

+

+  if (EFI_ERROR (Status)) {

+    if (DataNode != NULL) {

+      AmlDeleteTree ((AML_NODE_HANDLE)DataNode);

+    }

+  }

+

+  return Status;

+}

-- 

2.25.1





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



^ permalink raw reply	[flat|nested] 13+ messages in thread

* [edk2-devel] [PATCH v4 4/4] DynamicTablesPkg: Add Aml NameUnicodeString API
  2023-09-18 15:46 [edk2-devel] [PATCH v4 0/4] Add support for generating ACPI ThermalZones Jeff Brasen via groups.io
                   ` (2 preceding siblings ...)
  2023-09-18 15:46 ` [edk2-devel] [PATCH v4 3/4] DynamicTablesPkg: Add support to add Strings to package Jeff Brasen via groups.io
@ 2023-09-18 15:47 ` Jeff Brasen via groups.io
  3 siblings, 0 replies; 13+ messages in thread
From: Jeff Brasen via groups.io @ 2023-09-18 15:47 UTC (permalink / raw)
  To: devel
  Cc: Alexei.Fedorov, pierre.gondois, Sami.Mujawar, swatisrik,
	ashishsingha, Jeff Brasen

Add API to generate a Name that contains a Unicode string buffer.



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

---

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

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

 2 files changed, 117 insertions(+)



diff --git a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h

index f4a4908753..e0dc1340ab 100644

--- a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h

+++ b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h

@@ -958,6 +958,37 @@ AmlCodeGenNameResourceTemplate (

   OUT       AML_OBJECT_NODE_HANDLE  *NewObjectNode   OPTIONAL

   );

 

+/** AML code generation for a Name object node, containing a String.

+

+  AmlCodeGenNameUnicodeString ("_STR", L"String", ParentNode, NewObjectNode) is

+  equivalent of the following ASL code:

+    Name(_STR, Unicode ("String"))

+

+  @ingroup CodeGenApis

+

+  @param  [in] NameString     The new variable name.

+                              Must be a NULL-terminated ASL NameString

+                              e.g.: "DEV0", "DV15.DEV0", etc.

+                              The input string is copied.

+  @param [in]  String         NULL terminated Unicode String to associate 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

+AmlCodeGenNameUnicodeString (

+  IN  CONST CHAR8                   *NameString,

+  IN        CHAR16                  *String,

+  IN        AML_NODE_HANDLE         ParentNode      OPTIONAL,

+  OUT       AML_OBJECT_NODE_HANDLE  *NewObjectNode   OPTIONAL

+  );

+

 /** Add a _PRT entry.

 

   AmlCodeGenPrtEntry (0x0FFFF, 0, "LNKA", 0, PrtNameNode) is

diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c

index 2afd405750..51eece5958 100644

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

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

@@ -869,6 +869,92 @@ AmlCodeGenNameResourceTemplate (

   return Status;

 }

 

+/** AML code generation for a Name object node, containing a String.

+

+ AmlCodeGenNameUnicodeString ("_STR", L"String", ParentNode, NewObjectNode) is

+ equivalent of the following ASL code:

+   Name(_STR, Unicode ("String"))

+

+ @ingroup CodeGenApis

+

+ @param  [in] NameString     The new variable name.

+                             Must be a NULL-terminated ASL NameString

+                             e.g.: "DEV0", "DV15.DEV0", etc.

+                             The input string is copied.

+ @param [in]  String         NULL terminated Unicode String to associate 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

+AmlCodeGenNameUnicodeString (

+  IN  CONST CHAR8                   *NameString,

+  IN        CHAR16                  *String,

+  IN        AML_NODE_HANDLE         ParentNode      OPTIONAL,

+  OUT       AML_OBJECT_NODE_HANDLE  *NewObjectNode   OPTIONAL

+  )

+{

+  EFI_STATUS       Status;

+  AML_OBJECT_NODE  *ObjectNode;

+  AML_DATA_NODE    *DataNode;

+

+  if ((NameString == NULL)  ||

+      (String == NULL)      ||

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

+  {

+    ASSERT (0);

+    return EFI_INVALID_PARAMETER;

+  }

+

+  Status = AmlCodeGenBuffer (NULL, 0, &ObjectNode);

+  if (EFI_ERROR (Status)) {

+    ASSERT_EFI_ERROR (Status);

+    return Status;

+  }

+

+  Status = AmlCreateDataNode (

+             EAmlNodeDataTypeRaw,

+             (CONST UINT8 *)String,

+             StrSize (String),

+             &DataNode

+             );

+  if (EFI_ERROR (Status)) {

+    ASSERT_EFI_ERROR (Status);

+    AmlDeleteTree ((AML_NODE_HEADER *)ObjectNode);

+    return Status;

+  }

+

+  Status = AmlVarListAddTail (

+             (AML_NODE_HEADER *)ObjectNode,

+             (AML_NODE_HEADER *)DataNode

+             );

+  if (EFI_ERROR (Status)) {

+    ASSERT_EFI_ERROR (Status);

+    AmlDeleteTree ((AML_NODE_HEADER *)ObjectNode);

+    AmlDeleteTree ((AML_NODE_HANDLE)DataNode);

+    return Status;

+  }

+

+  Status = AmlCodeGenName (

+             NameString,

+             ObjectNode,

+             ParentNode,

+             NewObjectNode

+             );

+  if (EFI_ERROR (Status)) {

+    ASSERT (0);

+    AmlDeleteTree ((AML_NODE_HEADER *)ObjectNode);

+  }

+

+  return Status;

+}

+

 /** Add a _PRT entry.

 

   AmlCodeGenPrtEntry (0x0FFFF, 0, "LNKA", 0, PrtNameNode) is

-- 

2.25.1





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



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [edk2-devel] [PATCH v4 2/4] DynamicTablesPkg: Add support for simple method invocation.
  2023-09-18 15:46 ` [edk2-devel] [PATCH v4 2/4] DynamicTablesPkg: Add support for simple method invocation Jeff Brasen via groups.io
@ 2023-09-21 16:49   ` Sami Mujawar
  2023-09-21 17:38     ` Jeff Brasen via groups.io
  0 siblings, 1 reply; 13+ messages in thread
From: Sami Mujawar @ 2023-09-21 16:49 UTC (permalink / raw)
  To: Jeff Brasen, devel; +Cc: pierre.gondois, swatisrik, ashishsingha, nd@arm.com

[-- Attachment #1: Type: text/plain, Size: 16103 bytes --]

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/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 25488 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [edk2-devel] [PATCH v4 3/4] DynamicTablesPkg: Add support to add Strings to package
  2023-09-18 15:46 ` [edk2-devel] [PATCH v4 3/4] DynamicTablesPkg: Add support to add Strings to package Jeff Brasen via groups.io
@ 2023-09-21 16:49   ` Sami Mujawar
  2023-09-21 16:52     ` Jeff Brasen via groups.io
  0 siblings, 1 reply; 13+ messages in thread
From: Sami Mujawar @ 2023-09-21 16:49 UTC (permalink / raw)
  To: Jeff Brasen, devel; +Cc: pierre.gondois, swatisrik, ashishsingha, nd@arm.com

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 API to add a String to a package created with NamedPackage API.
>
>
>
> Signed-off-by: Jeff Brasen <jbrasen@nvidia.com>
>
> ---
>
>   .../Include/Library/AmlLib/AmlLib.h           | 17 ++++
>
>   .../Common/AmlLib/CodeGen/AmlCodeGen.c        | 88 +++++++++++++++++++
>
>   2 files changed, 105 insertions(+)
>
>
>
> diff --git a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
>
> index b82c7a3ce8..f4a4908753 100644
>
> --- a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
>
> +++ b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
>
> @@ -1472,4 +1472,21 @@ AmlCreateCpcNode (
>
>     OUT AML_OBJECT_NODE_HANDLE  *NewCpcNode   OPTIONAL
>
>     );
>
>   
>
> +/** AML code generation to add a NameString to the package in a named node.
>
> +
>
> +
>
> +  @param [in]  NameString     NameString to add
>
> +  @param [in]  NamedNode      Node to add the string to the included package.
>
> +
>
> +  @retval EFI_SUCCESS             Success.
>
> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
>
> +  @retval EFI_OUT_OF_RESOURCES    Failed to allocate memory.
>
> +**/
>
> +EFI_STATUS
>
> +EFIAPI
>
> +AmlAddNameStringToNamedPackage (
>
> +  IN CHAR8                   *NameString,
>
> +  IN AML_OBJECT_NODE_HANDLE  NamedNode
>
> +  );
>
> +
>
>   #endif // AML_LIB_H_
>
> diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
>
> index ea519d1aa8..2afd405750 100644
>
> --- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
>
> +++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
>
> @@ -3685,3 +3685,91 @@ error_handler:
>
>     AmlDeleteTree ((AML_NODE_HANDLE)CpcNode);
>
>     return Status;
>
>   }
>
> +
>
> +/** AML code generation to add a NameString to the package in a named node.
>
> +
>
> +
>
> +  @param [in]  NameString     NameString to add
>
> +  @param [in]  NamedNode      Node to add the string to the included package.
>
> +
>
> +  @retval EFI_SUCCESS             Success.
>
> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
>
> +  @retval EFI_OUT_OF_RESOURCES    Failed to allocate memory.
>
> +**/
>
> +EFI_STATUS
>
> +EFIAPI
>
> +AmlAddNameStringToNamedPackage (
>
> +  IN CHAR8                   *NameString,
>
> +  IN AML_OBJECT_NODE_HANDLE  NamedNode
>
> +  )
>
> +{
>
> +  EFI_STATUS              Status;
>
> +  AML_DATA_NODE           *DataNode;
>
> +  CHAR8                   *AmlNameString;
>
> +  UINT32                  AmlNameStringSize;
>
> +  AML_OBJECT_NODE_HANDLE  PackageNode;
>
> +
>
> +  DataNode = NULL;
>
> +
>
> +  if ((NamedNode == NULL)                                              ||
>
> +      (AmlGetNodeType ((AML_NODE_HANDLE)NamedNode) != EAmlNodeObject)  ||
>
> +      (!AmlNodeHasOpCode (NamedNode, AML_NAME_OP, 0)))
>
> +  {
>
> +    ASSERT (0);
>
> +    return EFI_INVALID_PARAMETER;
>
> +  }
>
> +
>
> +  PackageNode = (AML_OBJECT_NODE_HANDLE)AmlGetFixedArgument (
>
> +                                          NamedNode,
>
> +                                          EAmlParseIndexTerm1
>
> +                                          );
>
> +  if ((PackageNode == NULL)                                             ||
>
> +      (AmlGetNodeType ((AML_NODE_HANDLE)PackageNode) != EAmlNodeObject) ||
>
> +      (!AmlNodeHasOpCode (PackageNode, AML_PACKAGE_OP, 0)))
>
> +  {
>
> +    ASSERT (0);
>
> +    return EFI_INVALID_PARAMETER;
>
> +  }
>
> +
>
> +  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 = AmlVarListAddTail (
>
> +             (AML_NODE_HANDLE)PackageNode,
>
> +             (AML_NODE_HANDLE)DataNode
>
> +             );
>
> +  ASSERT_EFI_ERROR (Status);
>
> +
>
> +exit_handler:
>
> +  if (AmlNameString != NULL) {
>
> +    FreePool (AmlNameString);
>
> +  }
>
> +
[SAMI] I think the error handling code below can be moved after 
AmlVarListAddTail() above and ASSERT_EFI_ERROR() can me removed. If you 
agree, I will make this change before merging the series.
>
> +  if (EFI_ERROR (Status)) {
>
> +    if (DataNode != NULL) {
>
> +      AmlDeleteTree ((AML_NODE_HANDLE)DataNode);
>
> +    }
>
> +  }
>
> +
>
> +  return Status;
>
> +}
>


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



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [edk2-devel] [PATCH v4 3/4] DynamicTablesPkg: Add support to add Strings to package
  2023-09-21 16:49   ` Sami Mujawar
@ 2023-09-21 16:52     ` Jeff Brasen via groups.io
  0 siblings, 0 replies; 13+ messages in thread
From: Jeff Brasen via groups.io @ 2023-09-21 16:52 UTC (permalink / raw)
  To: Sami Mujawar, devel@edk2.groups.io
  Cc: pierre.gondois@arm.com, Swatisri Kantamsetti, Ashish Singhal,
	nd@arm.com

Hi Sami,

  That sounds good.

Thanks,
Jeff

> -----Original Message-----
> From: Sami Mujawar <sami.mujawar@arm.com>
> Sent: Thursday, September 21, 2023 10:50 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 3/4] DynamicTablesPkg: Add support to add Strings to
> package
> 
> 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 API to add a String to a package created with NamedPackage API.
> >
> >
> >
> > Signed-off-by: Jeff Brasen <jbrasen@nvidia.com>
> >
> > ---
> >
> >   .../Include/Library/AmlLib/AmlLib.h           | 17 ++++
> >
> >   .../Common/AmlLib/CodeGen/AmlCodeGen.c        | 88
> +++++++++++++++++++
> >
> >   2 files changed, 105 insertions(+)
> >
> >
> >
> > diff --git a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
> > b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
> >
> > index b82c7a3ce8..f4a4908753 100644
> >
> > --- a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
> >
> > +++ b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
> >
> > @@ -1472,4 +1472,21 @@ AmlCreateCpcNode (
> >
> >     OUT AML_OBJECT_NODE_HANDLE  *NewCpcNode   OPTIONAL
> >
> >     );
> >
> >
> >
> > +/** AML code generation to add a NameString to the package in a named
> node.
> >
> > +
> >
> > +
> >
> > +  @param [in]  NameString     NameString to add
> >
> > +  @param [in]  NamedNode      Node to add the string to the included
> package.
> >
> > +
> >
> > +  @retval EFI_SUCCESS             Success.
> >
> > +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
> >
> > +  @retval EFI_OUT_OF_RESOURCES    Failed to allocate memory.
> >
> > +**/
> >
> > +EFI_STATUS
> >
> > +EFIAPI
> >
> > +AmlAddNameStringToNamedPackage (
> >
> > +  IN CHAR8                   *NameString,
> >
> > +  IN AML_OBJECT_NODE_HANDLE  NamedNode
> >
> > +  );
> >
> > +
> >
> >   #endif // AML_LIB_H_
> >
> > diff --git
> > a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
> > b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
> >
> > index ea519d1aa8..2afd405750 100644
> >
> > ---
> a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
> >
> > +++
> b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
> >
> > @@ -3685,3 +3685,91 @@ error_handler:
> >
> >     AmlDeleteTree ((AML_NODE_HANDLE)CpcNode);
> >
> >     return Status;
> >
> >   }
> >
> > +
> >
> > +/** AML code generation to add a NameString to the package in a named
> node.
> >
> > +
> >
> > +
> >
> > +  @param [in]  NameString     NameString to add
> >
> > +  @param [in]  NamedNode      Node to add the string to the included
> package.
> >
> > +
> >
> > +  @retval EFI_SUCCESS             Success.
> >
> > +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
> >
> > +  @retval EFI_OUT_OF_RESOURCES    Failed to allocate memory.
> >
> > +**/
> >
> > +EFI_STATUS
> >
> > +EFIAPI
> >
> > +AmlAddNameStringToNamedPackage (
> >
> > +  IN CHAR8                   *NameString,
> >
> > +  IN AML_OBJECT_NODE_HANDLE  NamedNode
> >
> > +  )
> >
> > +{
> >
> > +  EFI_STATUS              Status;
> >
> > +  AML_DATA_NODE           *DataNode;
> >
> > +  CHAR8                   *AmlNameString;
> >
> > +  UINT32                  AmlNameStringSize;
> >
> > +  AML_OBJECT_NODE_HANDLE  PackageNode;
> >
> > +
> >
> > +  DataNode = NULL;
> >
> > +
> >
> > +  if ((NamedNode == NULL)                                              ||
> >
> > +      (AmlGetNodeType ((AML_NODE_HANDLE)NamedNode) !=
> EAmlNodeObject)
> > + ||
> >
> > +      (!AmlNodeHasOpCode (NamedNode, AML_NAME_OP, 0)))
> >
> > +  {
> >
> > +    ASSERT (0);
> >
> > +    return EFI_INVALID_PARAMETER;
> >
> > +  }
> >
> > +
> >
> > +  PackageNode = (AML_OBJECT_NODE_HANDLE)AmlGetFixedArgument (
> >
> > +                                          NamedNode,
> >
> > +                                          EAmlParseIndexTerm1
> >
> > +                                          );
> >
> > +  if ((PackageNode == NULL)                                             ||
> >
> > +      (AmlGetNodeType ((AML_NODE_HANDLE)PackageNode) !=
> > + EAmlNodeObject) ||
> >
> > +      (!AmlNodeHasOpCode (PackageNode, AML_PACKAGE_OP, 0)))
> >
> > +  {
> >
> > +    ASSERT (0);
> >
> > +    return EFI_INVALID_PARAMETER;
> >
> > +  }
> >
> > +
> >
> > +  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 = AmlVarListAddTail (
> >
> > +             (AML_NODE_HANDLE)PackageNode,
> >
> > +             (AML_NODE_HANDLE)DataNode
> >
> > +             );
> >
> > +  ASSERT_EFI_ERROR (Status);
> >
> > +
> >
> > +exit_handler:
> >
> > +  if (AmlNameString != NULL) {
> >
> > +    FreePool (AmlNameString);
> >
> > +  }
> >
> > +
> [SAMI] I think the error handling code below can be moved after
> AmlVarListAddTail() above and ASSERT_EFI_ERROR() can me removed. If you
> agree, I will make this change before merging the series.
> >
[Jeff] That sounds good
> > +  if (EFI_ERROR (Status)) {
> >
> > +    if (DataNode != NULL) {
> >
> > +      AmlDeleteTree ((AML_NODE_HANDLE)DataNode);
> >
> > +    }
> >
> > +  }
> >
> > +
> >
> > +  return Status;
> >
> > +}
> >


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



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [edk2-devel] [PATCH v4 2/4] DynamicTablesPkg: Add support for simple method invocation.
  2023-09-21 16:49   ` Sami Mujawar
@ 2023-09-21 17:38     ` Jeff Brasen via groups.io
  2023-09-21 20:07       ` Sami Mujawar
  0 siblings, 1 reply; 13+ messages in thread
From: Jeff Brasen via groups.io @ 2023-09-21 17:38 UTC (permalink / raw)
  To: Sami Mujawar, devel@edk2.groups.io
  Cc: pierre.gondois@arm.com, Swatisri Kantamsetti, Ashish Singhal,
	nd@arm.com

[-- Attachment #1: Type: text/plain, Size: 17836 bytes --]

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/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 83606 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [edk2-devel] [PATCH v4 2/4] DynamicTablesPkg: Add support for simple method invocation.
  2023-09-21 17:38     ` Jeff Brasen via groups.io
@ 2023-09-21 20:07       ` Sami Mujawar
  2023-09-21 20:48         ` Jeff Brasen via groups.io
  0 siblings, 1 reply; 13+ messages in thread
From: Sami Mujawar @ 2023-09-21 20:07 UTC (permalink / raw)
  To: Jeff Brasen (jbrasen@nvidia.com), devel@edk2.groups.io
  Cc: Pierre Gondois, Swatisri Kantamsetti, Ashish Singhal, nd

[-- Attachment #1: Type: text/plain, Size: 18716 bytes --]

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/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 136668 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [edk2-devel] [PATCH v4 2/4] DynamicTablesPkg: Add support for simple method invocation.
  2023-09-21 20:07       ` Sami Mujawar
@ 2023-09-21 20:48         ` Jeff Brasen via groups.io
  2023-09-23  4:22           ` Attar, AbdulLateef (Abdul Lateef) via groups.io
  0 siblings, 1 reply; 13+ messages in thread
From: Jeff Brasen via groups.io @ 2023-09-21 20:48 UTC (permalink / raw)
  To: Sami Mujawar, devel@edk2.groups.io
  Cc: Pierre Gondois, Swatisri Kantamsetti, Ashish Singhal, nd

[-- Attachment #1: Type: text/plain, Size: 19732 bytes --]

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/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 89666 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [edk2-devel] [PATCH v4 2/4] DynamicTablesPkg: Add support for simple method invocation.
  2023-09-21 20:48         ` Jeff Brasen via groups.io
@ 2023-09-23  4:22           ` Attar, AbdulLateef (Abdul Lateef) via groups.io
  2023-09-27 15:07             ` PierreGondois
  0 siblings, 1 reply; 13+ messages in thread
From: Attar, AbdulLateef (Abdul Lateef) via groups.io @ 2023-09-23  4:22 UTC (permalink / raw)
  To: devel@edk2.groups.io, jbrasen@nvidia.com, Sami Mujawar
  Cc: Pierre Gondois, Swatisri Kantamsetti, Ashish Singhal, nd

[-- Attachment #1: Type: text/plain, Size: 21088 bytes --]

[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/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 93300 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [edk2-devel] [PATCH v4 2/4] DynamicTablesPkg: Add support for simple method invocation.
  2023-09-23  4:22           ` Attar, AbdulLateef (Abdul Lateef) via groups.io
@ 2023-09-27 15:07             ` PierreGondois
  0 siblings, 0 replies; 13+ messages in thread
From: PierreGondois @ 2023-09-27 15:07 UTC (permalink / raw)
  To: Attar, AbdulLateef (Abdul Lateef), devel@edk2.groups.io,
	jbrasen@nvidia.com, Sami Mujawar
  Cc: Swatisri Kantamsetti, Ashish Singhal, nd

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/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2023-09-27 15:07 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-18 15:46 [edk2-devel] [PATCH v4 0/4] Add support for generating ACPI ThermalZones Jeff Brasen via groups.io
2023-09-18 15:46 ` [edk2-devel] [PATCH v4 1/4] DynamicTablesPkg: Add ThermalZone CodeGen function Jeff Brasen via groups.io
2023-09-18 15:46 ` [edk2-devel] [PATCH v4 2/4] DynamicTablesPkg: Add support for simple method invocation Jeff Brasen via groups.io
2023-09-21 16:49   ` Sami Mujawar
2023-09-21 17:38     ` Jeff Brasen via groups.io
2023-09-21 20:07       ` Sami Mujawar
2023-09-21 20:48         ` Jeff Brasen via groups.io
2023-09-23  4:22           ` Attar, AbdulLateef (Abdul Lateef) via groups.io
2023-09-27 15:07             ` PierreGondois
2023-09-18 15:46 ` [edk2-devel] [PATCH v4 3/4] DynamicTablesPkg: Add support to add Strings to package Jeff Brasen via groups.io
2023-09-21 16:49   ` Sami Mujawar
2023-09-21 16:52     ` Jeff Brasen via groups.io
2023-09-18 15:47 ` [edk2-devel] [PATCH v4 4/4] DynamicTablesPkg: Add Aml NameUnicodeString API Jeff Brasen via groups.io

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox