public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH] DynamicTablesPkg: Add support to build _DSD
@ 2022-08-05 15:14 Jeff Brasen
  2022-08-12 13:36 ` [edk2-devel] " PierreGondois
  0 siblings, 1 reply; 3+ messages in thread
From: Jeff Brasen @ 2022-08-05 15:14 UTC (permalink / raw)
  To: devel; +Cc: Sami.Mujawar, Alexei.Fedorov, Jeff Brasen

Add APIs needed to build _DSD with different UUIDs.
This is per ACPI specification 6.4 s6.2.5.

Adds support for building data packages with format
Package {"Name", Integer}

Signed-off-by: Jeff Brasen <jbrasen@nvidia.com>
---
 .../Include/Library/AmlLib/AmlLib.h           |  50 ++++
 .../Common/AmlLib/CodeGen/AmlCodeGen.c        | 236 ++++++++++++++++++
 2 files changed, 286 insertions(+)

diff --git a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
index 6f214c0dfa..30cab3f6bb 100644
--- a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
+++ b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
@@ -1280,6 +1280,56 @@ AmlAddLpiState (
   IN  AML_OBJECT_NODE_HANDLE                  LpiNode
   );
 
+/** AML code generation for a _DSD device data object.
+
+  AmlAddDeviceDataDescriptorPackage (Uuid, ParentNode, NewObjectNode) is
+  equivalent of the following ASL code:
+    ToUUID(Uuid),
+    Package () {}
+
+  @ingroup CodeGenApis
+
+  @param [in]  Uuid           The Uuid of the descriptor to be created
+  @param [in]  DsdNode        Node of the DSD Package.
+  @param [out] PackageNode    If success, contains the created package node.
+
+  @retval EFI_SUCCESS             Success.
+  @retval EFI_INVALID_PARAMETER   Invalid parameter.
+  @retval EFI_OUT_OF_RESOURCES    Failed to allocate memory.
+**/
+EFI_STATUS
+EFIAPI
+AmlAddDeviceDataDescriptorPackage (
+  IN  CONST EFI_GUID                *Uuid,
+  IN        AML_OBJECT_NODE_HANDLE  DsdNode,
+  OUT       AML_OBJECT_NODE_HANDLE  *PackageNode
+  );
+
+/** AML code generation to add a package with a name and value,
+ *  to a parent package
+
+  AmlAddNameValuePackage ("Name", Value, ParentNode) is
+  equivalent of the following ASL code:
+    Package (2) {"Name", Value}
+
+  @ingroup CodeGenApis
+
+  @param [in]  Name           String to place in first entry of package
+  @param [in]  Value          Integer to place in second entry of package
+  @param [in]  PackageNode    Package to add new sub package to.
+
+  @retval EFI_SUCCESS             Success.
+  @retval EFI_INVALID_PARAMETER   Invalid parameter.
+  @retval EFI_OUT_OF_RESOURCES    Failed to allocate memory.
+**/
+EFI_STATUS
+EFIAPI
+AmlAddNameIntegerPackage (
+  IN CHAR8                   *Name,
+  IN UINT64                  Value,
+  IN AML_OBJECT_NODE_HANDLE  PackageNode
+  );
+
 // DEPRECATED APIS
 #ifndef DISABLE_NEW_DEPRECATED_INTERFACES
 
diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
index e51d2dd7f0..80e43d0e62 100644
--- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
+++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
@@ -2600,3 +2600,239 @@ error_handler:
 
   return Status;
 }
+
+/** AML code generation for a _DSD device data object.
+
+  AmlAddDeviceDataDescriptorPackage (Uuid, ParentNode, NewObjectNode) is
+  equivalent of the following ASL code:
+    ToUUID(Uuid),
+    Package () {}
+
+  @ingroup CodeGenApis
+
+  @param [in]  Uuid           The Uuid of the descriptor to be created
+  @param [in]  DsdNode        Node of the DSD Package.
+  @param [out] PackageNode    If success, contains the created package node.
+
+  @retval EFI_SUCCESS             Success.
+  @retval EFI_INVALID_PARAMETER   Invalid parameter.
+  @retval EFI_OUT_OF_RESOURCES    Failed to allocate memory.
+**/
+EFI_STATUS
+EFIAPI
+AmlAddDeviceDataDescriptorPackage (
+  IN  CONST EFI_GUID                *Uuid,
+  IN        AML_OBJECT_NODE_HANDLE  DsdNode,
+  OUT       AML_OBJECT_NODE_HANDLE  *PackageNode
+  )
+{
+  EFI_STATUS              Status;
+  AML_OBJECT_NODE         *UuidNode;
+  AML_DATA_NODE           *UuidDataNode;
+  AML_OBJECT_NODE_HANDLE  DsdEntryList;
+
+  if ((Uuid == NULL)     ||
+      (PackageNode == NULL) ||
+      (AmlGetNodeType ((AML_NODE_HANDLE)DsdNode) != EAmlNodeObject) ||
+      (!AmlNodeHasOpCode (DsdNode, AML_NAME_OP, 0))                 ||
+      !AmlNameOpCompareName (DsdNode, "_DSD"))
+  {
+    ASSERT (0);
+    return EFI_INVALID_PARAMETER;
+  }
+
+  // Get the Package object node of the _DSD node,
+  // which is the 2nd fixed argument (i.e. index 1).
+  DsdEntryList = (AML_OBJECT_NODE_HANDLE)AmlGetFixedArgument (
+                                           DsdNode,
+                                           EAmlParseIndexTerm1
+                                           );
+  if ((DsdEntryList == NULL)                                              ||
+      (AmlGetNodeType ((AML_NODE_HANDLE)DsdEntryList) != EAmlNodeObject)  ||
+      (!AmlNodeHasOpCode (DsdEntryList, AML_PACKAGE_OP, 0)))
+  {
+    ASSERT (0);
+    return EFI_INVALID_PARAMETER;
+  }
+
+  *PackageNode = NULL;
+  UuidNode     = NULL;
+
+  Status = AmlCodeGenBuffer (NULL, 0, &UuidNode);
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    goto error_handler;
+  }
+
+  Status = AmlCreateDataNode (
+             EAmlNodeDataTypeRaw,
+             (CONST UINT8 *)Uuid,
+             sizeof (EFI_GUID),
+             &UuidDataNode
+             );
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    goto error_handler;
+  }
+
+  Status = AmlVarListAddTail (
+             (AML_NODE_HEADER *)UuidNode,
+             (AML_NODE_HEADER *)UuidDataNode
+             );
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    goto error_handler;
+  }
+  UuidDataNode = NULL;
+
+  // Append to the the list of _DSD entries.
+  Status = AmlVarListAddTail (
+             (AML_NODE_HANDLE)DsdEntryList,
+             (AML_NODE_HANDLE)UuidNode
+             );
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    goto error_handler;
+  }
+
+  Status = AmlCodeGenPackage (PackageNode);
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    goto error_handler;
+  }
+
+  // Append to the the list of _DSD entries.
+  Status = AmlVarListAddTail (
+             (AML_NODE_HANDLE)DsdEntryList,
+             (AML_NODE_HANDLE)*PackageNode
+             );
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    goto error_handler;
+  }
+
+  return Status;
+
+error_handler:
+  if (UuidNode != NULL) {
+    //Need to check to detach as there is an error path that have
+    //this attached to the DsdEntry
+    if (AML_NODE_HAS_PARENT (UuidNode)) {
+      AmlDetachNode ((AML_NODE_HANDLE)UuidNode);
+    }
+    AmlDeleteTree ((AML_NODE_HANDLE)UuidNode);
+  }
+
+  if (*PackageNode != NULL) {
+    AmlDeleteTree ((AML_NODE_HANDLE)*PackageNode);
+    *PackageNode = NULL;
+  }
+  if (UuidDataNode != NULL) {
+    AmlDeleteTree ((AML_NODE_HANDLE)UuidDataNode);
+  }
+
+  return Status;
+}
+
+/** AML code generation to add a package with a name and value,
+ *  to a parent package
+
+  AmlAddNameValuePackage ("Name", Value, ParentNode) is
+  equivalent of the following ASL code:
+    Package (2) {"Name", Value}
+
+  @ingroup CodeGenApis
+
+  @param [in]  Name           String to place in first entry of package
+  @param [in]  Value          Integer to place in second entry of package
+  @param [in]  PackageNode    Package to add new sub package to.
+
+  @retval EFI_SUCCESS             Success.
+  @retval EFI_INVALID_PARAMETER   Invalid parameter.
+  @retval EFI_OUT_OF_RESOURCES    Failed to allocate memory.
+**/
+EFI_STATUS
+EFIAPI
+AmlAddNameIntegerPackage (
+  IN CHAR8                   *Name,
+  IN UINT64                  Value,
+  IN AML_OBJECT_NODE_HANDLE  PackageNode
+  )
+{
+  EFI_STATUS       Status;
+  AML_OBJECT_NODE  *NameNode;
+  AML_OBJECT_NODE  *ValueNode;
+  AML_OBJECT_NODE  *NewPackageNode;
+
+  if ((Name == NULL) ||
+      (AmlGetNodeType ((AML_NODE_HANDLE)PackageNode) != EAmlNodeObject) ||
+      (!AmlNodeHasOpCode (PackageNode, AML_PACKAGE_OP, 0)))
+  {
+    ASSERT (0);
+    return EFI_INVALID_PARAMETER;
+  }
+
+  NewPackageNode = NULL;
+  // The new package entry.
+  Status = AmlCodeGenPackage (&NewPackageNode);
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    return Status;
+  }
+
+  Status = AmlCodeGenString (Name, &NameNode);
+  if (EFI_ERROR (Status)) {
+    ASSERT_EFI_ERROR (Status);
+    goto error_handler;
+  }
+
+  Status = AmlVarListAddTail (
+             (AML_NODE_HANDLE)NewPackageNode,
+             (AML_NODE_HANDLE)NameNode
+             );
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    goto error_handler;
+  }
+  NameNode = NULL;
+
+  Status = AmlCodeGenInteger (Value, &ValueNode);
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    goto error_handler;
+  }
+
+  Status = AmlVarListAddTail (
+             (AML_NODE_HANDLE)NewPackageNode,
+             (AML_NODE_HANDLE)ValueNode
+             );
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    goto error_handler;
+  }
+  ValueNode = NULL;
+
+  Status = AmlVarListAddTail (
+             (AML_NODE_HANDLE)PackageNode,
+             (AML_NODE_HANDLE)NewPackageNode
+             );
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    goto error_handler;
+  }
+
+  return Status;
+
+error_handler:
+  if (NewPackageNode != NULL) {
+    AmlDeleteTree ((AML_NODE_HANDLE)NewPackageNode);
+  }
+  if (NameNode != NULL) {
+    AmlDeleteTree ((AML_NODE_HANDLE)NameNode);
+  }
+  if (ValueNode != NULL) {
+    AmlDeleteTree ((AML_NODE_HANDLE)ValueNode);
+  }
+
+  return Status;
+}
-- 
2.25.1


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

* Re: [edk2-devel] [PATCH] DynamicTablesPkg: Add support to build _DSD
  2022-08-05 15:14 [PATCH] DynamicTablesPkg: Add support to build _DSD Jeff Brasen
@ 2022-08-12 13:36 ` PierreGondois
  2022-08-15  9:49   ` Sami Mujawar
  0 siblings, 1 reply; 3+ messages in thread
From: PierreGondois @ 2022-08-12 13:36 UTC (permalink / raw)
  To: devel, jbrasen; +Cc: Sami.Mujawar, Alexei.Fedorov

Hi Jeff,
Please find some inline comments below:

On 8/5/22 17:14, Jeff Brasen via groups.io wrote:
> Add APIs needed to build _DSD with different UUIDs.
> 
> This is per ACPI specification 6.4 s6.2.5.
> 
> 
> 
> Adds support for building data packages with format
> 
> Package {"Name", Integer}
> 
> 
> 
> Signed-off-by: Jeff Brasen <jbrasen@nvidia.com>
> 
> ---
> 
>   .../Include/Library/AmlLib/AmlLib.h           |  50 ++++
> 
>   .../Common/AmlLib/CodeGen/AmlCodeGen.c        | 236 ++++++++++++++++++
> 
>   2 files changed, 286 insertions(+)
> 
> 
> 
> diff --git a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
> 
> index 6f214c0dfa..30cab3f6bb 100644
> 
> --- a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
> 
> +++ b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
> 
> @@ -1280,6 +1280,56 @@ AmlAddLpiState (
> 
>     IN  AML_OBJECT_NODE_HANDLE                  LpiNode
> 
>     );
> 
>   
> 
> +/** AML code generation for a _DSD device data object.
> 
> +
> 
> +  AmlAddDeviceDataDescriptorPackage (Uuid, ParentNode, NewObjectNode) is
> 
> +  equivalent of the following ASL code:
> 
> +    ToUUID(Uuid),
> 
> +    Package () {}
> 
> +
> 
> +  @ingroup CodeGenApis
> 
> +
> 
> +  @param [in]  Uuid           The Uuid of the descriptor to be created
> 
> +  @param [in]  DsdNode        Node of the DSD Package.
> 
> +  @param [out] PackageNode    If success, contains the created package node.
> 
> +
> 
> +  @retval EFI_SUCCESS             Success.
> 
> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
> 
> +  @retval EFI_OUT_OF_RESOURCES    Failed to allocate memory.
> 
> +**/
> 
> +EFI_STATUS
> 
> +EFIAPI
> 
> +AmlAddDeviceDataDescriptorPackage (
> 
> +  IN  CONST EFI_GUID                *Uuid,
> 
> +  IN        AML_OBJECT_NODE_HANDLE  DsdNode,
> 
> +  OUT       AML_OBJECT_NODE_HANDLE  *PackageNode
> 
> +  );
> 
> +
> 
> +/** AML code generation to add a package with a name and value,
> 
> + *  to a parent package> 
> +
> 
> +  AmlAddNameValuePackage ("Name", Value, ParentNode) is
> 
> +  equivalent of the following ASL code:
> 
> +    Package (2) {"Name", Value}
> 
> +
> 
> +  @ingroup CodeGenApis
> 
> +
> 
> +  @param [in]  Name           String to place in first entry of package
> 
> +  @param [in]  Value          Integer to place in second entry of package
> 
> +  @param [in]  PackageNode    Package to add new sub package to.
> 
> +
> 
> +  @retval EFI_SUCCESS             Success.
> 
> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
> 
> +  @retval EFI_OUT_OF_RESOURCES    Failed to allocate memory.
> 
> +**/
> 
> +EFI_STATUS
> 
> +EFIAPI
> 
> +AmlAddNameIntegerPackage (
> 
> +  IN CHAR8                   *Name,
> 
> +  IN UINT64                  Value,
> 
> +  IN AML_OBJECT_NODE_HANDLE  PackageNode
> 
> +  );
> 
> +
> 
>   // DEPRECATED APIS
> 
>   #ifndef DISABLE_NEW_DEPRECATED_INTERFACES
> 
>   
> 
> diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
> 
> index e51d2dd7f0..80e43d0e62 100644
> 
> --- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
> 
> +++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
> 
> @@ -2600,3 +2600,239 @@ error_handler:
> 
>   
> 
>     return Status;
> 
>   }
> 
> +
> 
> +/** AML code generation for a _DSD device data object.
> 
> +
> 
> +  AmlAddDeviceDataDescriptorPackage (Uuid, ParentNode, NewObjectNode) is

I think this is DsdNode and PackageNode (same for the header file).
> 
> +  equivalent of the following ASL code:
> 
> +    ToUUID(Uuid),
> 
> +    Package () {}
> 
> +
> 
> +  @ingroup CodeGenApis
> 
> +
> 
> +  @param [in]  Uuid           The Uuid of the descriptor to be created
> 
> +  @param [in]  DsdNode        Node of the DSD Package.
> 
> +  @param [out] PackageNode    If success, contains the created package node.
> 
> +
> 
> +  @retval EFI_SUCCESS             Success.
> 
> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
> 
> +  @retval EFI_OUT_OF_RESOURCES    Failed to allocate memory.
> 
> +**/
> 
> +EFI_STATUS
> 
> +EFIAPI
> 
> +AmlAddDeviceDataDescriptorPackage (
> 
> +  IN  CONST EFI_GUID                *Uuid,
> 
> +  IN        AML_OBJECT_NODE_HANDLE  DsdNode,
> 
> +  OUT       AML_OBJECT_NODE_HANDLE  *PackageNode
> 
> +  )
> 
> +{
> 
> +  EFI_STATUS              Status;
> 
> +  AML_OBJECT_NODE         *UuidNode;
> 
> +  AML_DATA_NODE           *UuidDataNode;
> 
> +  AML_OBJECT_NODE_HANDLE  DsdEntryList;
> 
> +
> 
> +  if ((Uuid == NULL)     ||
> 
> +      (PackageNode == NULL) ||
> 
> +      (AmlGetNodeType ((AML_NODE_HANDLE)DsdNode) != EAmlNodeObject) ||
> 
> +      (!AmlNodeHasOpCode (DsdNode, AML_NAME_OP, 0))                 ||
> 
> +      !AmlNameOpCompareName (DsdNode, "_DSD"))
> 
> +  {
> 
> +    ASSERT (0);

This is not consistent in the file, but it could be:
   ASSERT_EFI_ERROR (ASSERT_EFI_ERROR)

> 
> +    return EFI_INVALID_PARAMETER;
> 
> +  }
> 
> +
> 
> +  // Get the Package object node of the _DSD node,
> 
> +  // which is the 2nd fixed argument (i.e. index 1).
> 
> +  DsdEntryList = (AML_OBJECT_NODE_HANDLE)AmlGetFixedArgument (
> 
> +                                           DsdNode,
> 
> +                                           EAmlParseIndexTerm1
> 
> +                                           );
> 
> +  if ((DsdEntryList == NULL)                                              ||
> 
> +      (AmlGetNodeType ((AML_NODE_HANDLE)DsdEntryList) != EAmlNodeObject)  ||
> 
> +      (!AmlNodeHasOpCode (DsdEntryList, AML_PACKAGE_OP, 0)))
> 
> +  {
> 
> +    ASSERT (0);
> 
> +    return EFI_INVALID_PARAMETER;
> 
> +  }
> 
> +
> 
> +  *PackageNode = NULL;
> 
> +  UuidNode     = NULL;

As UuidNode is the first node allocated, we either fail and return
(no need to free it), or succeed and the error handler should
free it. So it is not necessary to initialize it to NULL.
However UuidDataNode should be initialized to NULL.

> 
> +
> 
> +  Status = AmlCodeGenBuffer (NULL, 0, &UuidNode);
> 
> +  if (EFI_ERROR (Status)) {
> 
> +    ASSERT (0);
> 
> +    goto error_handler;

If it failed here then no allocation happened yet,
so it could just be a return.

> 
> +  }
> 
> +
> 
> +  Status = AmlCreateDataNode (
> 
> +             EAmlNodeDataTypeRaw,
> 
> +             (CONST UINT8 *)Uuid,
> 
> +             sizeof (EFI_GUID),
> 
> +             &UuidDataNode
> 
> +             );
> 
> +  if (EFI_ERROR (Status)) {
> 
> +    ASSERT (0);
> 
> +    goto error_handler;
> 
> +  }

For the record, I tested the function and the Uuid is correctly generated.
For Sami: would it be good to have a ToUUID() function as in ASL ?

> 
> +
> 
> +  Status = AmlVarListAddTail (
> 
> +             (AML_NODE_HEADER *)UuidNode,
> 
> +             (AML_NODE_HEADER *)UuidDataNode
> 
> +             );
> 
> +  if (EFI_ERROR (Status)) {
> 
> +    ASSERT (0);
> 
> +    goto error_handler;
> 
> +  }
> 
> +  UuidDataNode = NULL;
> 
> +
> 
> +  // Append to the the list of _DSD entries.
> 
> +  Status = AmlVarListAddTail (
> 
> +             (AML_NODE_HANDLE)DsdEntryList,
> 
> +             (AML_NODE_HANDLE)UuidNode
> 
> +             );
> 
> +  if (EFI_ERROR (Status)) {
> 
> +    ASSERT (0);
> 
> +    goto error_handler;
> 
> +  }
> 
> +
> 
> +  Status = AmlCodeGenPackage (PackageNode);
> 
> +  if (EFI_ERROR (Status)) {
> 
> +    ASSERT (0);
> 
> +    goto error_handler;
> 
> +  }
> 
> +
> 
> +  // Append to the the list of _DSD entries.

duplicated 'the' here and above

> 
> +  Status = AmlVarListAddTail (
> 
> +             (AML_NODE_HANDLE)DsdEntryList,
> 
> +             (AML_NODE_HANDLE)*PackageNode
> 
> +             );
> 
> +  if (EFI_ERROR (Status)) {
> 
> +    ASSERT (0);
> 
> +    goto error_handler;
> 
> +  }
> 
> +
> 
> +  return Status;
> 
> +
> 
> +error_handler:
> 
> +  if (UuidNode != NULL) {
> 
> +    //Need to check to detach as there is an error path that have
> 
> +    //this attached to the DsdEntry
> 
> +    if (AML_NODE_HAS_PARENT (UuidNode)) {

Would it be possible to have a second error_handler instead ?
Ideally AmlCodeGen.c should not be able to see the internal node
structure.
With the second error handler, it should look like:

   Status = AmlCodeGenPackage (PackageNode);
   if (EFI_ERROR (Status)) {
     ASSERT (0);
     goto error_handler1;
   }

   // Append to the the list of _DSD entries.
   Status = AmlVarListAddTail (
              (AML_NODE_HANDLE)DsdEntryList,
              (AML_NODE_HANDLE)*PackageNode
              );
   if (EFI_ERROR (Status)) {
     ASSERT (0);
     goto error_handler1;
   }

   return Status;

error_handler1:
   AmlDetachNode ((AML_NODE_HANDLE)UuidNode);

error_handler:
   if (UuidNode != NULL) {
     AmlDeleteTree ((AML_NODE_HANDLE)UuidNode);
   }
...


> 
> +      AmlDetachNode ((AML_NODE_HANDLE)UuidNode);
> 
> +    }
> 
> +    AmlDeleteTree ((AML_NODE_HANDLE)UuidNode);
> 
> +  }
> 
> +
> 
> +  if (*PackageNode != NULL) {
> 
> +    AmlDeleteTree ((AML_NODE_HANDLE)*PackageNode);
> 
> +    *PackageNode = NULL;
> 
> +  }
> 
> +  if (UuidDataNode != NULL) {
> 
> +    AmlDeleteTree ((AML_NODE_HANDLE)UuidDataNode);
> 
> +  }
> 
> +
> 
> +  return Status;
> 
> +}
> 
> +
> 
> +/** AML code generation to add a package with a name and value,
> 
> + *  to a parent package

Is it possible to remove the '*' at the beginning of the line
(same for the prototype) and also add a reference to
s6.2.5 _DSD (Device Specific Data) in the two new function
descriptions ?

> 
> +
> 
> +  AmlAddNameValuePackage ("Name", Value, ParentNode) is
> 
> +  equivalent of the following ASL code:
> 
> +    Package (2) {"Name", Value}
> 
> +
> 
> +  @ingroup CodeGenApis
> 
> +
> 
> +  @param [in]  Name           String to place in first entry of package
> 
> +  @param [in]  Value          Integer to place in second entry of package
> 
> +  @param [in]  PackageNode    Package to add new sub package to.
> 
> +
> 
> +  @retval EFI_SUCCESS             Success.
> 
> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
> 
> +  @retval EFI_OUT_OF_RESOURCES    Failed to allocate memory.
> 
> +**/
> 
> +EFI_STATUS
> 
> +EFIAPI
> 
> +AmlAddNameIntegerPackage (

To regroup the functions related to _DSD objects,
would it be better to name this function:
   AmlAddDsdIntegerProperty()
and the function above:
   AmlAddDsdDescriptor()
(Sami/Jeff this is an open question)

> 
> +  IN CHAR8                   *Name,
> 
> +  IN UINT64                  Value,
> 
> +  IN AML_OBJECT_NODE_HANDLE  PackageNode
> 
> +  )
> 
> +{
> 
> +  EFI_STATUS       Status;
> 
> +  AML_OBJECT_NODE  *NameNode;
> 
> +  AML_OBJECT_NODE  *ValueNode;
> 
> +  AML_OBJECT_NODE  *NewPackageNode;
> 
> +
> 
> +  if ((Name == NULL) ||
> 
> +      (AmlGetNodeType ((AML_NODE_HANDLE)PackageNode) != EAmlNodeObject) ||
> 
> +      (!AmlNodeHasOpCode (PackageNode, AML_PACKAGE_OP, 0)))
> 
> +  {
> 
> +    ASSERT (0);
> 
> +    return EFI_INVALID_PARAMETER;
> 
> +  }
> 
> +
> 
> +  NewPackageNode = NULL;

I think setting NewPackageNode to NULL is not necessary as:
- if AmlCodeGenPackage() fails, we return without going to the error handler
   and without trying to free it
- if we fail later, NameNode contains a valid node which is freed in the
   error handler

However NameNode and ValueNode should be initialized to NULL as if
AmlCodeGenString() fails, we go to the error handler and try to free
non-initialized values.

> 
> +  // The new package entry.
> 
> +  Status = AmlCodeGenPackage (&NewPackageNode);
> 
> +  if (EFI_ERROR (Status)) {
> 
> +    ASSERT (0);

Even though this is not consistent in the file,
it could be ASSERT_EFI_ERROR (Status)
(same comment for the other ASSERT).

> 
> +    return Status;
> 
> +  }
> 
> +
> 
> +  Status = AmlCodeGenString (Name, &NameNode);
> 
> +  if (EFI_ERROR (Status)) {
> 
> +    ASSERT_EFI_ERROR (Status);
> 
> +    goto error_handler;
> 
> +  }
> 
> +
> 
> +  Status = AmlVarListAddTail (
> 
> +             (AML_NODE_HANDLE)NewPackageNode,
> 
> +             (AML_NODE_HANDLE)NameNode
> 
> +             );
> 
> +  if (EFI_ERROR (Status)) {
> 
> +    ASSERT (0);
> 
> +    goto error_handler;
> 
> +  }
> 
> +  NameNode = NULL;
> 
> +
> 
> +  Status = AmlCodeGenInteger (Value, &ValueNode);
> 
> +  if (EFI_ERROR (Status)) {
> 
> +    ASSERT (0);
> 
> +    goto error_handler;
> 
> +  }
> 
> +
> 
> +  Status = AmlVarListAddTail (
> 
> +             (AML_NODE_HANDLE)NewPackageNode,
> 
> +             (AML_NODE_HANDLE)ValueNode
> 
> +             );
> 
> +  if (EFI_ERROR (Status)) {
> 
> +    ASSERT (0);
> 
> +    goto error_handler;
> 
> +  }
> 
> +  ValueNode = NULL;
> 
> +
> 
> +  Status = AmlVarListAddTail (
> 
> +             (AML_NODE_HANDLE)PackageNode,
> 
> +             (AML_NODE_HANDLE)NewPackageNode
> 
> +             );
> 
> +  if (EFI_ERROR (Status)) {
> 
> +    ASSERT (0);
> 
> +    goto error_handler;
> 
> +  }
> 
> +
> 
> +  return Status;
> 
> +
> 
> +error_handler:
> 
> +  if (NewPackageNode != NULL) {
> 
> +    AmlDeleteTree ((AML_NODE_HANDLE)NewPackageNode);
> 
> +  }
> 
> +  if (NameNode != NULL) {
> 
> +    AmlDeleteTree ((AML_NODE_HANDLE)NameNode);
> 
> +  }
> 
> +  if (ValueNode != NULL) {
> 
> +    AmlDeleteTree ((AML_NODE_HANDLE)ValueNode);
> 
> +  }
> 
> +
> 
> +  return Status;
> 
> +}
> 

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

* Re: [edk2-devel] [PATCH] DynamicTablesPkg: Add support to build _DSD
  2022-08-12 13:36 ` [edk2-devel] " PierreGondois
@ 2022-08-15  9:49   ` Sami Mujawar
  0 siblings, 0 replies; 3+ messages in thread
From: Sami Mujawar @ 2022-08-15  9:49 UTC (permalink / raw)
  To: Pierre Gondois, devel, jbrasen; +Cc: Alexei.Fedorov, nd@arm.com

Hi Jeff, Pierre,

Please findmy response inline marked [SAMI].

Regards,

Sami Mujawar

On 12/08/2022 02:36 pm, Pierre Gondois wrote:
> Hi Jeff,
> Please find some inline comments below:
>
> On 8/5/22 17:14, Jeff Brasen via groups.io wrote:
>> Add APIs needed to build _DSD with different UUIDs.
>>
>> This is per ACPI specification 6.4 s6.2.5.
>>
>>
>>
>> Adds support for building data packages with format
>>
>> Package {"Name", Integer}
>>
>>
>>
>> Signed-off-by: Jeff Brasen <jbrasen@nvidia.com>
>>
>> ---
>>
>>   .../Include/Library/AmlLib/AmlLib.h           |  50 ++++
>>
>>   .../Common/AmlLib/CodeGen/AmlCodeGen.c        | 236 ++++++++++++++++++
>>
>>   2 files changed, 286 insertions(+)
>>
>>
>>
>> diff --git a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h 
>> b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
>>
>> index 6f214c0dfa..30cab3f6bb 100644
>>
>> --- a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
>>
>> +++ b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
>>
>> @@ -1280,6 +1280,56 @@ AmlAddLpiState (
>>
>>     IN  AML_OBJECT_NODE_HANDLE                  LpiNode
>>
>>     );
>>
>>
>> +/** AML code generation for a _DSD device data object.
>>
>> +
>>
>> +  AmlAddDeviceDataDescriptorPackage (Uuid, ParentNode, 
>> NewObjectNode) is
>>
>> +  equivalent of the following ASL code:
>>
>> +    ToUUID(Uuid),
>>
>> +    Package () {}
>>
>> +
>>
>> +  @ingroup CodeGenApis
>>
>> +
>>
>> +  @param [in]  Uuid           The Uuid of the descriptor to be created
>>
>> +  @param [in]  DsdNode        Node of the DSD Package.
>>
>> +  @param [out] PackageNode    If success, contains the created 
>> package node.
>>
>> +
>>
>> +  @retval EFI_SUCCESS             Success.
>>
>> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
>>
>> +  @retval EFI_OUT_OF_RESOURCES    Failed to allocate memory.
>>
>> +**/
>>
>> +EFI_STATUS
>>
>> +EFIAPI
>>
>> +AmlAddDeviceDataDescriptorPackage (
>>
>> +  IN  CONST EFI_GUID                *Uuid,
>>
>> +  IN        AML_OBJECT_NODE_HANDLE  DsdNode,
>>
>> +  OUT       AML_OBJECT_NODE_HANDLE  *PackageNode
>>
>> +  );
>>
>> +
>>
>> +/** AML code generation to add a package with a name and value,
>>
>> + *  to a parent package> +
>>
>> +  AmlAddNameValuePackage ("Name", Value, ParentNode) is
>>
>> +  equivalent of the following ASL code:
>>
>> +    Package (2) {"Name", Value}
>>
>> +
>>
>> +  @ingroup CodeGenApis
>>
>> +
>>
>> +  @param [in]  Name           String to place in first entry of package
>>
>> +  @param [in]  Value          Integer to place in second entry of 
>> package
>>
>> +  @param [in]  PackageNode    Package to add new sub package to.
>>
>> +
>>
>> +  @retval EFI_SUCCESS             Success.
>>
>> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
>>
>> +  @retval EFI_OUT_OF_RESOURCES    Failed to allocate memory.
>>
>> +**/
>>
>> +EFI_STATUS
>>
>> +EFIAPI
>>
>> +AmlAddNameIntegerPackage (
>>
>> +  IN CHAR8                   *Name,
>>
>> +  IN UINT64                  Value,
>>
>> +  IN AML_OBJECT_NODE_HANDLE  PackageNode
>>
>> +  );
>>
>> +
>>
>>   // DEPRECATED APIS
>>
>>   #ifndef DISABLE_NEW_DEPRECATED_INTERFACES
>>
>>
>> diff --git 
>> a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c 
>> b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
>>
>> index e51d2dd7f0..80e43d0e62 100644
>>
>> --- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
>>
>> +++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
>>
>> @@ -2600,3 +2600,239 @@ error_handler:
>>
>>
>>     return Status;
>>
>>   }
>>
>> +
>>
>> +/** AML code generation for a _DSD device data object.
>>
>> +
>>
>> +  AmlAddDeviceDataDescriptorPackage (Uuid, ParentNode, 
>> NewObjectNode) is
>
> I think this is DsdNode and PackageNode (same for the header file).
>>
>> +  equivalent of the following ASL code:
>>
>> +    ToUUID(Uuid),
>>
>> +    Package () {}
>>
>> +
>>
>> +  @ingroup CodeGenApis
>>
>> +
>>
>> +  @param [in]  Uuid           The Uuid of the descriptor to be created
>>
>> +  @param [in]  DsdNode        Node of the DSD Package.
>>
>> +  @param [out] PackageNode    If success, contains the created 
>> package node.
>>
>> +
>>
>> +  @retval EFI_SUCCESS             Success.
>>
>> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
>>
>> +  @retval EFI_OUT_OF_RESOURCES    Failed to allocate memory.
>>
>> +**/
>>
>> +EFI_STATUS
>>
>> +EFIAPI
>>
>> +AmlAddDeviceDataDescriptorPackage (
>>
>> +  IN  CONST EFI_GUID                *Uuid,
>>
>> +  IN        AML_OBJECT_NODE_HANDLE  DsdNode,
>>
>> +  OUT       AML_OBJECT_NODE_HANDLE  *PackageNode
>>
>> +  )
>>
>> +{
>>
>> +  EFI_STATUS              Status;
>>
>> +  AML_OBJECT_NODE         *UuidNode;
>>
>> +  AML_DATA_NODE           *UuidDataNode;
>>
>> +  AML_OBJECT_NODE_HANDLE  DsdEntryList;
>>
>> +
>>
>> +  if ((Uuid == NULL)     ||
>>
>> +      (PackageNode == NULL) ||
>>
>> +      (AmlGetNodeType ((AML_NODE_HANDLE)DsdNode) != EAmlNodeObject) ||
>>
>> +      (!AmlNodeHasOpCode (DsdNode, AML_NAME_OP, 0))                 ||
>>
>> +      !AmlNameOpCompareName (DsdNode, "_DSD"))
>>
>> +  {
>>
>> +    ASSERT (0);
>
> This is not consistent in the file, but it could be:
>   ASSERT_EFI_ERROR (ASSERT_EFI_ERROR)
[SAMI] I am fine with using either ASSERTxxx macros. The only think that 
needs to be taken care of is that the errors are handled in release 
builds (as the ASSERTxxx macros will vanish in release builds).
>
>>
>> +    return EFI_INVALID_PARAMETER;
>>
>> +  }
>>
>> +
>>
>> +  // Get the Package object node of the _DSD node,
>>
>> +  // which is the 2nd fixed argument (i.e. index 1).
>>
>> +  DsdEntryList = (AML_OBJECT_NODE_HANDLE)AmlGetFixedArgument (
>>
>> +                                           DsdNode,
>>
>> +                                           EAmlParseIndexTerm1
>>
>> +                                           );
>>
>> +  if ((DsdEntryList == 
>> NULL)                                              ||
>>
>> +      (AmlGetNodeType ((AML_NODE_HANDLE)DsdEntryList) != 
>> EAmlNodeObject)  ||
>>
>> +      (!AmlNodeHasOpCode (DsdEntryList, AML_PACKAGE_OP, 0)))
>>
>> +  {
>>
>> +    ASSERT (0);
>>
>> +    return EFI_INVALID_PARAMETER;
>>
>> +  }
>>
>> +
>>
>> +  *PackageNode = NULL;
>>
>> +  UuidNode     = NULL;
>
> As UuidNode is the first node allocated, we either fail and return
> (no need to free it), or succeed and the error handler should
> free it. So it is not necessary to initialize it to NULL.
> However UuidDataNode should be initialized to NULL.
>
>>
>> +
>>
>> +  Status = AmlCodeGenBuffer (NULL, 0, &UuidNode);
>>
>> +  if (EFI_ERROR (Status)) {
>>
>> +    ASSERT (0);
>>
>> +    goto error_handler;
>
> If it failed here then no allocation happened yet,
> so it could just be a return.
>
>>
>> +  }
>>
>> +
>>
>> +  Status = AmlCreateDataNode (
>>
>> +             EAmlNodeDataTypeRaw,
>>
>> +             (CONST UINT8 *)Uuid,
>>
>> +             sizeof (EFI_GUID),
>>
>> +             &UuidDataNode
>>
>> +             );
>>
>> +  if (EFI_ERROR (Status)) {
>>
>> +    ASSERT (0);
>>
>> +    goto error_handler;
>>
>> +  }
>
> For the record, I tested the function and the Uuid is correctly 
> generated.
> For Sami: would it be good to have a ToUUID() function as in ASL ?
[SAMI] Yes, that could be useful.
>
>>
>> +
>>
>> +  Status = AmlVarListAddTail (
>>
>> +             (AML_NODE_HEADER *)UuidNode,
>>
>> +             (AML_NODE_HEADER *)UuidDataNode
>>
>> +             );
>>
>> +  if (EFI_ERROR (Status)) {
>>
>> +    ASSERT (0);
>>
>> +    goto error_handler;
>>
>> +  }
>>
>> +  UuidDataNode = NULL;
>>
>> +
>>
>> +  // Append to the the list of _DSD entries.
>>
>> +  Status = AmlVarListAddTail (
>>
>> +             (AML_NODE_HANDLE)DsdEntryList,
>>
>> +             (AML_NODE_HANDLE)UuidNode
>>
>> +             );
>>
>> +  if (EFI_ERROR (Status)) {
>>
>> +    ASSERT (0);
>>
>> +    goto error_handler;
>>
>> +  }
>>
>> +
>>
>> +  Status = AmlCodeGenPackage (PackageNode);
>>
>> +  if (EFI_ERROR (Status)) {
>>
>> +    ASSERT (0);
>>
>> +    goto error_handler;
>>
>> +  }
>>
>> +
>>
>> +  // Append to the the list of _DSD entries.
>
> duplicated 'the' here and above
>
>>
>> +  Status = AmlVarListAddTail (
>>
>> +             (AML_NODE_HANDLE)DsdEntryList,
>>
>> +             (AML_NODE_HANDLE)*PackageNode
>>
>> +             );
>>
>> +  if (EFI_ERROR (Status)) {
>>
>> +    ASSERT (0);
>>
>> +    goto error_handler;
>>
>> +  }
>>
>> +
>>
>> +  return Status;
>>
>> +
>>
>> +error_handler:
>>
>> +  if (UuidNode != NULL) {
>>
>> +    //Need to check to detach as there is an error path that have
>>
>> +    //this attached to the DsdEntry
>>
>> +    if (AML_NODE_HAS_PARENT (UuidNode)) {
>
> Would it be possible to have a second error_handler instead ?
> Ideally AmlCodeGen.c should not be able to see the internal node
> structure.
> With the second error handler, it should look like:
>
>   Status = AmlCodeGenPackage (PackageNode);
>   if (EFI_ERROR (Status)) {
>     ASSERT (0);
>     goto error_handler1;
>   }
>
>   // Append to the the list of _DSD entries.
>   Status = AmlVarListAddTail (
>              (AML_NODE_HANDLE)DsdEntryList,
>              (AML_NODE_HANDLE)*PackageNode
>              );
>   if (EFI_ERROR (Status)) {
>     ASSERT (0);
>     goto error_handler1;
>   }
>
>   return Status;
>
> error_handler1:
>   AmlDetachNode ((AML_NODE_HANDLE)UuidNode);
>
> error_handler:
>   if (UuidNode != NULL) {
>     AmlDeleteTree ((AML_NODE_HANDLE)UuidNode);
>   }
> ...
>
>
>>
>> +      AmlDetachNode ((AML_NODE_HANDLE)UuidNode);
>>
>> +    }
>>
>> +    AmlDeleteTree ((AML_NODE_HANDLE)UuidNode);
>>
>> +  }
>>
>> +
>>
>> +  if (*PackageNode != NULL) {
>>
>> +    AmlDeleteTree ((AML_NODE_HANDLE)*PackageNode);
>>
>> +    *PackageNode = NULL;
>>
>> +  }
>>
>> +  if (UuidDataNode != NULL) {
>>
>> +    AmlDeleteTree ((AML_NODE_HANDLE)UuidDataNode);
>>
>> +  }
>>
>> +
>>
>> +  return Status;
>>
>> +}
>>
>> +
>>
>> +/** AML code generation to add a package with a name and value,
>>
>> + *  to a parent package
>
> Is it possible to remove the '*' at the beginning of the line
> (same for the prototype) and also add a reference to
> s6.2.5 _DSD (Device Specific Data) in the two new function
> descriptions ?
>
>>
>> +
>>
>> +  AmlAddNameValuePackage ("Name", Value, ParentNode) is
>>
>> +  equivalent of the following ASL code:
>>
>> +    Package (2) {"Name", Value}
>>
>> +
>>
>> +  @ingroup CodeGenApis
>>
>> +
>>
>> +  @param [in]  Name           String to place in first entry of package
>>
>> +  @param [in]  Value          Integer to place in second entry of 
>> package
>>
>> +  @param [in]  PackageNode    Package to add new sub package to.
>>
>> +
>>
>> +  @retval EFI_SUCCESS             Success.
>>
>> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
>>
>> +  @retval EFI_OUT_OF_RESOURCES    Failed to allocate memory.
>>
>> +**/
>>
>> +EFI_STATUS
>>
>> +EFIAPI
>>
>> +AmlAddNameIntegerPackage (
>
> To regroup the functions related to _DSD objects,
> would it be better to name this function:
>   AmlAddDsdIntegerProperty()
> and the function above:
>   AmlAddDsdDescriptor()
> (Sami/Jeff this is an open question)

[SAMI] Is there any other usecase where 'Package (2) {"Name", Value}' 
may be required?

I think AmlAddNameIntegerPackage() is generic and could be useful 
elsewhere. Would defining AmlAddDsdIntegerProperty as a macro that maps 
to AmlAddNameIntegerPackage() be an alternative?

We could then use the macro in the DSD specific code.

[/SAMI]
>>
>> +  IN CHAR8                   *Name,
>>
>> +  IN UINT64                  Value,
>>
>> +  IN AML_OBJECT_NODE_HANDLE  PackageNode
>>
>> +  )
>>
>> +{
>>
>> +  EFI_STATUS       Status;
>>
>> +  AML_OBJECT_NODE  *NameNode;
>>
>> +  AML_OBJECT_NODE  *ValueNode;
>>
>> +  AML_OBJECT_NODE  *NewPackageNode;
>>
>> +
>>
>> +  if ((Name == NULL) ||
>>
>> +      (AmlGetNodeType ((AML_NODE_HANDLE)PackageNode) != 
>> EAmlNodeObject) ||
>>
>> +      (!AmlNodeHasOpCode (PackageNode, AML_PACKAGE_OP, 0)))
>>
>> +  {
>>
>> +    ASSERT (0);
>>
>> +    return EFI_INVALID_PARAMETER;
>>
>> +  }
>>
>> +
>>
>> +  NewPackageNode = NULL;
>
> I think setting NewPackageNode to NULL is not necessary as:
> - if AmlCodeGenPackage() fails, we return without going to the error 
> handler
>   and without trying to free it
> - if we fail later, NameNode contains a valid node which is freed in the
>   error handler
>
> However NameNode and ValueNode should be initialized to NULL as if
> AmlCodeGenString() fails, we go to the error handler and try to free
> non-initialized values.
>
>>
>> +  // The new package entry.
>>
>> +  Status = AmlCodeGenPackage (&NewPackageNode);
>>
>> +  if (EFI_ERROR (Status)) {
>>
>> +    ASSERT (0);
>
> Even though this is not consistent in the file,
> it could be ASSERT_EFI_ERROR (Status)
> (same comment for the other ASSERT).
[SAMI] I am fine with either ASSERT versions being used. But please 
handle errors for release builds.
>
>>
>> +    return Status;
>>
>> +  }
>>
>> +
>>
>> +  Status = AmlCodeGenString (Name, &NameNode);
>>
>> +  if (EFI_ERROR (Status)) {
>>
>> +    ASSERT_EFI_ERROR (Status);
>>
>> +    goto error_handler;
>>
>> +  }
>>
>> +
>>
>> +  Status = AmlVarListAddTail (
>>
>> +             (AML_NODE_HANDLE)NewPackageNode,
>>
>> +             (AML_NODE_HANDLE)NameNode
>>
>> +             );
>>
>> +  if (EFI_ERROR (Status)) {
>>
>> +    ASSERT (0);
>>
>> +    goto error_handler;
>>
>> +  }
>>
>> +  NameNode = NULL;
>>
>> +
>>
>> +  Status = AmlCodeGenInteger (Value, &ValueNode);
>>
>> +  if (EFI_ERROR (Status)) {
>>
>> +    ASSERT (0);
>>
>> +    goto error_handler;
>>
>> +  }
>>
>> +
>>
>> +  Status = AmlVarListAddTail (
>>
>> +             (AML_NODE_HANDLE)NewPackageNode,
>>
>> +             (AML_NODE_HANDLE)ValueNode
>>
>> +             );
>>
>> +  if (EFI_ERROR (Status)) {
>>
>> +    ASSERT (0);
>>
>> +    goto error_handler;
>>
>> +  }
>>
>> +  ValueNode = NULL;
>>
>> +
>>
>> +  Status = AmlVarListAddTail (
>>
>> +             (AML_NODE_HANDLE)PackageNode,
>>
>> +             (AML_NODE_HANDLE)NewPackageNode
>>
>> +             );
>>
>> +  if (EFI_ERROR (Status)) {
>>
>> +    ASSERT (0);
>>
>> +    goto error_handler;
>>
>> +  }
>>
>> +
>>
>> +  return Status;
>>
>> +
>>
>> +error_handler:
>>
>> +  if (NewPackageNode != NULL) {
>>
>> +    AmlDeleteTree ((AML_NODE_HANDLE)NewPackageNode);
>>
>> +  }
>>
>> +  if (NameNode != NULL) {
>>
>> +    AmlDeleteTree ((AML_NODE_HANDLE)NameNode);
>>
>> +  }
>>
>> +  if (ValueNode != NULL) {
>>
>> +    AmlDeleteTree ((AML_NODE_HANDLE)ValueNode);
>>
>> +  }
>>
>> +
>>
>> +  return Status;
>>
>> +}
>>

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

end of thread, other threads:[~2022-08-15  9:49 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-08-05 15:14 [PATCH] DynamicTablesPkg: Add support to build _DSD Jeff Brasen
2022-08-12 13:36 ` [edk2-devel] " PierreGondois
2022-08-15  9:49   ` Sami Mujawar

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