public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "PierreGondois" <pierre.gondois@arm.com>
To: devel@edk2.groups.io, Sami Mujawar <sami.mujawar@arm.com>,
	Alexei.Fedorov@arm.com
Subject: [PATCH v3 06/21] DynamicTablesPkg: Set EndTag's Checksum if RdList is modified
Date: Fri,  8 Oct 2021 15:46:17 +0100	[thread overview]
Message-ID: <20211008144632.31894-7-Pierre.Gondois@arm.com> (raw)
In-Reply-To: <20211008144632.31894-1-Pierre.Gondois@arm.com>

From: Pierre Gondois <Pierre.Gondois@arm.com>

ACPI 6.4, s6.4.2.9 "End Tag":
"This checksum is generated such that adding it to the sum of all the data
bytes will produce a zero sum."
"If the checksum field is zero, the resource data is treated as if the
checksum operation succeeded. Configuration proceeds normally."

To avoid re-computing checksums, if a new resource data elements is
added/removed/modified in a list of resource data elements, the AmlLib
resets the checksum to 0.

This patch also refactors the AmlAppendRdNode() function by getting the
last Resource Data node directly instead of iterating over all the
elements of the list of Resource Data node.

Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
---
 .../Common/AmlLib/Tree/AmlNodeInterface.c     |  7 ++
 .../Library/Common/AmlLib/Tree/AmlTree.c      | 72 +++++++------------
 2 files changed, 34 insertions(+), 45 deletions(-)

diff --git a/DynamicTablesPkg/Library/Common/AmlLib/Tree/AmlNodeInterface.c b/DynamicTablesPkg/Library/Common/AmlLib/Tree/AmlNodeInterface.c
index 870346c40a5d..f91bc5fd55d2 100644
--- a/DynamicTablesPkg/Library/Common/AmlLib/Tree/AmlNodeInterface.c
+++ b/DynamicTablesPkg/Library/Common/AmlLib/Tree/AmlNodeInterface.c
@@ -513,6 +513,13 @@ AmlUpdateDataNode (
         ASSERT (0);
         return EFI_INVALID_PARAMETER;
       }
+
+      Status = AmlSetRdListCheckSum (ParentNode, 0);
+      if (EFI_ERROR (Status)) {
+        ASSERT (0);
+        return Status;
+      }
+
       break;
     }
     case EAmlNodeDataTypeFieldPkgLen:
diff --git a/DynamicTablesPkg/Library/Common/AmlLib/Tree/AmlTree.c b/DynamicTablesPkg/Library/Common/AmlLib/Tree/AmlTree.c
index 0931e751709e..2772e915f468 100644
--- a/DynamicTablesPkg/Library/Common/AmlLib/Tree/AmlTree.c
+++ b/DynamicTablesPkg/Library/Common/AmlLib/Tree/AmlTree.c
@@ -566,8 +566,7 @@ AmlAppendRdNode (
   )
 {
   EFI_STATUS        Status;
-  AML_DATA_NODE   * CurrRdNode;
-  AML_RD_HEADER     RdDataType;
+  AML_DATA_NODE   * LastRdNode;
 
   if (!AmlNodeCompareOpCode (BufferOpNode, AML_BUFFER_OP, 0)  ||
       !IS_AML_DATA_NODE (NewRdNode)                           ||
@@ -576,56 +575,39 @@ AmlAppendRdNode (
     return EFI_INVALID_PARAMETER;
   }
 
-  // Get the first Resource data node in the variable list of
-  // argument of the BufferOp node.
-  CurrRdNode = (AML_DATA_NODE*)AmlGetNextVariableArgument (
+  // To avoid re-computing checksums, if a new resource data elements is
+  // added/removed/modified in a list of resource data elements, the AmlLib
+  // resets the checksum to 0.
+  // It is possible to have only one Resource Data in a BufferOp with
+  // no EndTag, but it should not be possible to add a new Resource Data
+  // in the list in this case.
+  Status = AmlSetRdListCheckSum (BufferOpNode, 0);
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    return Status;
+  }
+
+  // Get the last Resource data node in the variable list of argument of the
+  // BufferOp node. This must be an EndTag, otherwise setting the checksum
+  // would have failed.
+  LastRdNode = (AML_DATA_NODE*)AmlGetPreviousVariableArgument (
                                  (AML_NODE_HEADER*)BufferOpNode,
                                  NULL
                                  );
-  if ((CurrRdNode == NULL)             ||
-      !IS_AML_DATA_NODE (CurrRdNode)   ||
-      (CurrRdNode->DataType != EAmlNodeDataTypeResourceData)) {
+  if ((LastRdNode == NULL)             ||
+      !IS_AML_DATA_NODE (LastRdNode)   ||
+      (LastRdNode->DataType != EAmlNodeDataTypeResourceData)) {
     ASSERT (0);
     return EFI_INVALID_PARAMETER;
   }
 
-  // Iterate through the Resource Data nodes to find the End Tag.
-  while (TRUE) {
-    Status = AmlGetResourceDataType (CurrRdNode, &RdDataType);
-    if (EFI_ERROR (Status)) {
-      ASSERT (0);
-      return Status;
-    }
-
-    // If the Resource Data is an End Tag,
-    // add the new node before and return.
-    if (AmlRdCompareDescId (
-          &RdDataType,
-          AML_RD_BUILD_SMALL_DESC_ID (ACPI_SMALL_END_TAG_DESCRIPTOR_NAME))) {
-      Status = AmlVarListAddBefore (
-                 (AML_NODE_HEADER*)CurrRdNode,
-                 (AML_NODE_HEADER*)NewRdNode)
-                 ;
-      if (EFI_ERROR (Status)) {
-        ASSERT (0);
-      }
-      return Status;
-    }
-
-    // Get the next Resource Data node.
-    // If this was the last node and no End Tag was found, return error.
-    // It is possible to have only one Resource Data in a BufferOp,
-    // but it should not be possible to add a new Resource Data in the list
-    // in this case.
-    CurrRdNode = (AML_DATA_NODE*)AmlGetSiblingVariableArgument (
-                                   (AML_NODE_HEADER*)CurrRdNode
-                                   );
-    if (!IS_AML_DATA_NODE (CurrRdNode)  ||
-        (CurrRdNode->DataType != EAmlNodeDataTypeResourceData)) {
-      ASSERT (0);
-      return EFI_INVALID_PARAMETER;
-    }
-  } // while
+  // Add NewRdNode before the EndTag.
+  Status = AmlVarListAddBefore (
+             (AML_NODE_HEADER*)LastRdNode,
+             (AML_NODE_HEADER*)NewRdNode)
+             ;
+  ASSERT_EFI_ERROR (Status);
+  return Status;
 }
 
 /** Replace the fixed argument at the Index of the ParentNode with the NewNode.
-- 
2.17.1


  parent reply	other threads:[~2021-10-08 14:46 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-08 14:46 [PATCH v3 00/21] Create a SSDT CPU topology generator PierreGondois
2021-10-08 14:46 ` [PATCH v3 01/21] DynamicTablesPkg: Remove unnecessary includes PierreGondois
2021-10-08 14:46 ` [PATCH v3 02/21] DynamicTablesPkg: Add missing parameter check PierreGondois
2021-10-08 14:46 ` [PATCH v3 03/21] DynamicTablesPkg: Add AddSsdtAcpiHeader() PierreGondois
2021-10-08 14:46 ` [PATCH v3 04/21] DynamicTablesPkg: Add AmlRdSetEndTagChecksum() PierreGondois
2021-10-08 14:46 ` [PATCH v3 05/21] DynamicTablesPkg: Add AmlSetRdListCheckSum() PierreGondois
2021-10-08 14:46 ` PierreGondois [this message]
2021-10-08 14:46 ` [PATCH v3 07/21] DynamicTablesPkg: Clear pointer in node creation fcts PierreGondois
2021-10-08 14:46 ` [PATCH v3 08/21] DynamicTablesPkg: Update error handling for node creation PierreGondois
2021-10-08 14:46 ` [PATCH v3 09/21] DynamicTablesPkg: Make AmlNodeGetIntegerValue public PierreGondois
2021-10-08 14:46 ` [PATCH v3 10/21] DynamicTablesPkg: AML Code generation for Register() PierreGondois
2021-10-08 14:46 ` [PATCH v3 11/21] DynamicTablesPkg: AML Code generation for Resource data EndTag PierreGondois
2021-10-08 14:46 ` [PATCH v3 12/21] DynamicTablesPkg: AML code generation for a Package PierreGondois
2021-10-08 14:46 ` [PATCH v3 13/21] DynamicTablesPkg: Helper function to compute package length PierreGondois
2021-10-08 14:46 ` [PATCH v3 14/21] DynamicTablesPkg: AML code generation for a ResourceTemplate PierreGondois
2021-10-08 14:46 ` [PATCH v3 15/21] DynamicTablesPkg: AML code generation for a Method PierreGondois
2021-10-08 14:46 ` [PATCH v3 16/21] DynamicTablesPkg: AML code generation to Return a NameString PierreGondois
2021-10-08 14:46 ` [PATCH v3 17/21] DynamicTablesPkg: AML code generation for a Method returning a NS PierreGondois
2021-10-08 14:46 ` [PATCH v3 18/21] DynamicTablesPkg: AML code generation for a _LPI object PierreGondois
2021-10-08 14:46 ` [PATCH v3 19/21] DynamicTablesPkg: AML code generation to add an _LPI state PierreGondois
2021-10-08 14:46 ` [PATCH v3 20/21] DynamicTablesPkg: Add CM_ARM_LPI_INFO object PierreGondois
2021-10-08 14:46 ` [PATCH v3 21/21] DynamicTablesPkg: SSDT CPU topology and LPI state generator PierreGondois
2021-10-08 16:05 ` [PATCH v3 00/21] Create a SSDT CPU topology generator Sami Mujawar
2021-10-11  1:16   ` 回复: [edk2-devel] " gaoliming
2021-10-11  7:40     ` PierreGondois

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211008144632.31894-7-Pierre.Gondois@arm.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox