From 8cce048d4834d6967a568f3a0adc1efcf97c80c7 Mon Sep 17 00:00:00 2001
From: Jeff Brasen <jbrasen@nvidia.com>
Date: Tue, 1 Oct 2024 08:57:30 -0700
Subject: [PATCH] DynamicTablesPkg: Correct _PSD package format

The _PSD structure should have nested packages.

The current implementation generates

Name (_PSD, Package()
   {
   NumEntries             // Integer
   Revision               // Integer (BYTE)
   Domain                 // Integer (DWORD)
   CoordType              // Integer (DWORD)
   NumProcessors          // Integer (DWORD)
})

when this should be

Name (_PSD, Package()
   {
    Package()
    {
    NumEntries             // Integer
    Revision               // Integer (BYTE)
    Domain                 // Integer (DWORD)
    CoordType              // Integer (DWORD)
    NumProcessors          // Integer (DWORD)
    }
})

REF: https://uefi.org/specs/ACPI/6.5/08_Processor_Configuration_and_Control.html#psd-p-state-dependency
Signed-off-by: Jeff Brasen <jbrasen@nvidia.com>
---
 .../Common/AmlLib/CodeGen/AmlCodeGen.c        | 56 ++++++++++++++-----
 1 file changed, 43 insertions(+), 13 deletions(-)

diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
index 35ec2aaa6703..0f28e27c9449 100644
--- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
+++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
@@ -4183,11 +4183,13 @@ AmlCodeGenInvokeMethod (
   Creates and optionally adds the following node
    Name(_PSD, Package()
    {
-    NumEntries,  // Integer
-    Revision,    // Integer
-    Domain,      // Integer
-    CoordType,   // Integer
-    NumProc,     // Integer
+    Package () {
+      NumEntries,  // Integer
+      Revision,    // Integer
+      Domain,      // Integer
+      CoordType,   // Integer
+      NumProc,     // Integer
+    }
   })
 
   Cf. ACPI 6.5, s8.4.5.5 _PSD (P-State Dependency)
@@ -4213,10 +4215,14 @@ AmlCreatePsdNode (
 {
   EFI_STATUS              Status;
   AML_OBJECT_NODE_HANDLE  PsdNode;
+  AML_OBJECT_NODE_HANDLE  PsdParentPackage;
   AML_OBJECT_NODE_HANDLE  PsdPackage;
   AML_OBJECT_NODE_HANDLE  IntegerNode;
   UINT32                  NumberOfEntries;
 
+  PsdParentPackage = NULL;
+  PsdPackage       = NULL;
+
   if ((PsdInfo == NULL) ||
       ((ParentNode == NULL) && (NewPsdNode == NULL)))
   {
@@ -4253,19 +4259,25 @@ AmlCreatePsdNode (
 
   // Get the Package object node of the _PSD node,
   // which is the 2nd fixed argument (i.e. index 1).
-  PsdPackage = (AML_OBJECT_NODE_HANDLE)AmlGetFixedArgument (
-                                         PsdNode,
-                                         EAmlParseIndexTerm1
-                                         );
-  if ((PsdPackage == NULL)                                              ||
-      (AmlGetNodeType ((AML_NODE_HANDLE)PsdPackage) != EAmlNodeObject)  ||
-      (!AmlNodeHasOpCode (PsdPackage, AML_PACKAGE_OP, 0)))
+  PsdParentPackage = (AML_OBJECT_NODE_HANDLE)AmlGetFixedArgument (
+                                               PsdNode,
+                                               EAmlParseIndexTerm1
+                                               );
+  if ((PsdParentPackage == NULL)                                              ||
+      (AmlGetNodeType ((AML_NODE_HANDLE)PsdParentPackage) != EAmlNodeObject)  ||
+      (!AmlNodeHasOpCode (PsdParentPackage, AML_PACKAGE_OP, 0)))
   {
     Status = EFI_INVALID_PARAMETER;
     ASSERT_EFI_ERROR (Status);
     goto error_handler;
   }
 
+  Status = AmlCodeGenPackage (&PsdPackage);
+  if (EFI_ERROR (Status)) {
+    ASSERT_EFI_ERROR (Status);
+    goto error_handler;
+  }
+
   // NumEntries
   Status = AmlCodeGenInteger (NumberOfEntries, &IntegerNode);
   if (EFI_ERROR (Status)) {
@@ -4351,6 +4363,17 @@ AmlCreatePsdNode (
     goto error_handler;
   }
 
+  Status = AmlVarListAddTail (
+             (AML_NODE_HANDLE)PsdParentPackage,
+             (AML_NODE_HANDLE)PsdPackage
+             );
+  if (EFI_ERROR (Status)) {
+    ASSERT_EFI_ERROR (Status);
+    goto error_handler;
+  }
+
+  PsdPackage = NULL; // Prevent double free if error occurs after this point
+
   Status = LinkNode (PsdNode, ParentNode, NewPsdNode);
   if (EFI_ERROR (Status)) {
     ASSERT_EFI_ERROR (Status);
@@ -4360,6 +4383,13 @@ AmlCreatePsdNode (
   return Status;
 
 error_handler:
-  AmlDeleteTree ((AML_NODE_HANDLE)PsdNode);
+  if (PsdPackage != NULL) {
+    AmlDeleteTree ((AML_NODE_HANDLE)PsdPackage);
+  }
+
+  if (PsdParentPackage != NULL) {
+    AmlDeleteTree ((AML_NODE_HANDLE)PsdParentPackage);
+  }
+
   return Status;
 }