Date   

Re: [PATCH v1 08/13] DynamicTablesPkg: AML code generation to Return a NameString

Sami Mujawar
 

Hi Pierre,

Please find my feedback inline marked [SAMI].

Regards,

Sami Mujawar


On 23/06/2021 12:40 PM, Pierre.Gondois@... wrote:
From: Pierre Gondois <Pierre.Gondois@...>

Add AmlCodeGenReturnNameString() to generate AML code for a
Return object node, returning the object as a NameString.

AmlCodeGenReturn ("NAM1", ParentNode, NewObjectNode) is
equivalent of the following ASL code:
  Return(NAM1)

Signed-off-by: Pierre Gondois <Pierre.Gondois@...>
---
 .../Common/AmlLib/CodeGen/AmlCodeGen.c        | 178 ++++++++++++++++++
 1 file changed, 178 insertions(+)

diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
index 32665f7f8d8f..75dadbaf4ac3 100644
--- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
+++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
@@ -1137,3 +1137,181 @@ error_handler1:
   }
   return Status;
 }
+
+/** AML code generation for a Return object node.
+
+  AmlCodeGenReturn (ReturnNode, ParentNode, NewObjectNode) is
+  equivalent of the following ASL code:
+    Return([Content of the ReturnNode])
+
+  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]  ReturnNode     The object returned by the Return ASL statement.
+                              This node is deleted if an error occurs.
+  @param [in]  ParentNode     If provided, set ParentNode as the parent
+                              of the node created.
[SAMI] Can we mention here that the ParentNode must be a MethodOp Node, please? Also is it possible to add a check to that effect.
+  @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
+AmlCodeGenReturn (
+  IN  AML_NODE_HEADER     * ReturnNode,
+  IN  AML_NODE_HEADER     * ParentNode,     OPTIONAL
+  OUT AML_OBJECT_NODE    ** NewObjectNode   OPTIONAL
+  )
+{
+  EFI_STATUS        Status;
+  AML_OBJECT_NODE * ObjectNode;
+
+  if ((ReturnNode == NULL)  ||
+      ((ParentNode == NULL) && (NewObjectNode == NULL))) {
+    ASSERT (0);
+    return EFI_INVALID_PARAMETER;
+  }
+
+  ObjectNode = NULL;
+
+  Status = AmlCreateObjectNode (
+             AmlGetByteEncodingByOpCode (AML_RETURN_OP, 0),
+             0,
+             &ObjectNode
+             );
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    goto error_handler;
+  }
+
+  Status = AmlSetFixedArgument (
+             ObjectNode,
+             EAmlParseIndexTerm0,
+             (AML_NODE_HEADER*)ReturnNode
+             );
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    goto error_handler;
+  }
+
+  ReturnNode = NULL;
+
+  Status = LinkNode (
+             ObjectNode,
+             ParentNode,
+             NewObjectNode
+             );
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    goto error_handler;
+  }
+
+  return Status;
+
+error_handler:
+  AmlDeleteTree (ReturnNode);
[SAMI] Add a check to see if ReturnNode is NULL before deleting.
+  if (ObjectNode != NULL) {
+    AmlDeleteTree ((AML_NODE_HEADER*)ObjectNode);
+  }
+  return Status;
+}
+
+/** AML code generation for a Return object node,
+    returning the object as an input NameString.
+
+  AmlCodeGenReturn ("NAM1", ParentNode, NewObjectNode) is
+  equivalent of the following ASL code:
+    Return(NAM1)
+
+  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]  ParentNode     If provided, set ParentNode as the parent
+                              of the node created.
[SAMI] Same comment as above.
+  @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
+AmlCodeGenReturnNameString (
+  IN  CONST CHAR8               * NameString,
+  IN        AML_NODE_HEADER     * ParentNode,     OPTIONAL
+  OUT       AML_OBJECT_NODE    ** NewObjectNode   OPTIONAL
+  )
+{
+  EFI_STATUS          Status;
+  AML_DATA_NODE     * DataNode;
+  CHAR8             * AmlNameString;
+  UINT32              AmlNameStringSize;
+
+  DataNode = 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;
+  }
+
+  // AmlCodeGenReturn() deletes DataNode if error.
+  Status = AmlCodeGenReturn (
+             (AML_NODE_HEADER*)DataNode,
+             ParentNode,
+             NewObjectNode
+             );
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+  }
[SAMI] Can we replace this with ASSERT_EFI_ERROR()?
+
+exit_handler:
+  if (AmlNameString != NULL) {
+    FreePool (AmlNameString);
+  }
+  return Status;
+}


Re: [PATCH v1 07/13] DynamicTablesPkg: AML code generation for a Method

Sami Mujawar
 

Hi Pierre,

I have a minor suggestion marked inline as [SAMI].

Otherwise this patch looks good to me.

Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>

Regards,

Sami Mujawar


On 23/06/2021 12:40 PM, Pierre.Gondois@arm.com wrote:
From: Pierre Gondois <Pierre.Gondois@arm.com>

Add AmlCodeGenMethod() to generate code for a control method.

AmlCodeGenMethod ("MET0", 1, TRUE, 3, ParentNode, NewObjectNode)
is equivalent of the following ASL code:
Method(MET0, 1, Serialized, 3) {}

Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
---
.../Common/AmlLib/CodeGen/AmlCodeGen.c | 166 ++++++++++++++++++
1 file changed, 166 insertions(+)

diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
index faf7902c1f21..32665f7f8d8f 100644
--- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
+++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
@@ -971,3 +971,169 @@ error_handler1:
return Status;
}
+
+/** AML code generation for a Method object node.
+
+ AmlCodeGenMethod ("MET0", 1, TRUE, 3, ParentNode, NewObjectNode) is
+ equivalent of the following ASL code:
+ Method(MET0, 1, Serialized, 3) {}
+
+ The ASL parameters "ReturnType" and "ParameterTypes" are not asked
+ in this function. They are optional parameters in ASL.
+
+ @param [in] NameString 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] 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] 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.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+AmlCodeGenMethod (
+ IN CONST CHAR8 * NameString,
+ IN UINT8 NumArgs,
+ IN BOOLEAN IsSerialized,
+ IN UINT8 SyncLevel,
+ IN AML_NODE_HEADER * ParentNode, OPTIONAL
+ OUT AML_OBJECT_NODE ** NewObjectNode OPTIONAL
+ )
+{
+ EFI_STATUS Status;
+ UINT32 PkgLen;
+ UINT8 Flags;
+ AML_OBJECT_NODE * ObjectNode;
+ AML_DATA_NODE * DataNode;
+ CHAR8 * AmlNameString;
+ UINT32 AmlNameStringSize;
+
+ if ((NameString == NULL) ||
+ (NumArgs > 6) ||
+ (SyncLevel > 15) ||
+ ((ParentNode == NULL) && (NewObjectNode == NULL))) {
+ ASSERT (0);
+ return EFI_INVALID_PARAMETER;
+ }
+
+ ObjectNode = NULL;
+ DataNode = 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;
+ }
+
+ // Compute the size to write in the PkgLen.
+ // Add 1 byte (ByteData) for MethodFlags.
[SAMI] Is it possible to add description of the encoding for MethodOp, please? Also it will help if some comments are added describing each encoding step using a numberd list.
+ Status = AmlComputePkgLength (AmlNameStringSize + 1, &PkgLen);
+ if (EFI_ERROR (Status)) {
+ ASSERT (0);
+ goto error_handler1;
+ }
+
+ Status = AmlCreateObjectNode (
+ AmlGetByteEncodingByOpCode (AML_METHOD_OP, 0),
+ PkgLen,
+ &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);
+ goto error_handler2;
+ }
+
+ DataNode = NULL;
+
+ Flags = NumArgs |
+ (IsSerialized ? BIT3 : 0) |
+ (SyncLevel << 4);
+
+ Status = AmlCreateDataNode (EAmlNodeDataTypeUInt, &Flags, 1, &DataNode);
+ if (EFI_ERROR (Status)) {
+ ASSERT (0);
+ goto error_handler2;
+ }
+
+ Status = AmlSetFixedArgument (
+ ObjectNode,
+ EAmlParseIndexTerm1,
+ (AML_NODE_HEADER*)DataNode
+ );
+ if (EFI_ERROR (Status)) {
+ ASSERT (0);
+ goto error_handler2;
+ }
+
+ // Data node is attached so set the pointer to
+ // NULL to ensure correct error handling.
+ DataNode = NULL;
+
+ 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().
[SAMI] You could mention the encoding step here.
+ goto error_handler1;
+
+error_handler2:
+ if (ObjectNode != NULL) {
+ AmlDeleteTree ((AML_NODE_HEADER*)ObjectNode);
+ }
+ if (DataNode != NULL) {
+ AmlDeleteTree ((AML_NODE_HEADER*)DataNode);
+ }
+
+error_handler1:
+ if (AmlNameString != NULL) {
+ FreePool (AmlNameString);
+ }
+ return Status;
+}


Re: [PATCH v1 01/13] DynamicTablesPkg: Make AmlNodeGetIntegerValue public

Sami Mujawar
 

Hi Pierre,

Thank you for this patch. This patch looks good to me.

Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>

Regards,

Sami Mujawar

On 23/06/2021 12:40 PM, Pierre.Gondois@arm.com wrote:
From: Pierre Gondois <Pierre.Gondois@arm.com>

Remove the STATIC qualifier for the AmlUtility function
AmlNodeGetIntegerValue() and add the definition to the
header file so that it can be used by other AmlLib
sub-modules.

Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
---
.../Library/Common/AmlLib/Utils/AmlUtility.c | 3 +--
.../Library/Common/AmlLib/Utils/AmlUtility.h | 18 +++++++++++++++++-
2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/DynamicTablesPkg/Library/Common/AmlLib/Utils/AmlUtility.c b/DynamicTablesPkg/Library/Common/AmlLib/Utils/AmlUtility.c
index 7ebd08f945c0..3c8927acda6a 100644
--- a/DynamicTablesPkg/Library/Common/AmlLib/Utils/AmlUtility.c
+++ b/DynamicTablesPkg/Library/Common/AmlLib/Utils/AmlUtility.c
@@ -1,7 +1,7 @@
/** @file
AML Utility.
- Copyright (c) 2019 - 2020, Arm Limited. All rights reserved.<BR>
+ Copyright (c) 2019 - 2021, Arm Limited. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -192,7 +192,6 @@ AmlComputeSize (
@retval EFI_SUCCESS The function completed successfully.
@retval EFI_INVALID_PARAMETER Invalid parameter.
**/
-STATIC
EFI_STATUS
EFIAPI
AmlNodeGetIntegerValue (
diff --git a/DynamicTablesPkg/Library/Common/AmlLib/Utils/AmlUtility.h b/DynamicTablesPkg/Library/Common/AmlLib/Utils/AmlUtility.h
index c57d780140d4..5013bfb81d2d 100644
--- a/DynamicTablesPkg/Library/Common/AmlLib/Utils/AmlUtility.h
+++ b/DynamicTablesPkg/Library/Common/AmlLib/Utils/AmlUtility.h
@@ -1,7 +1,7 @@
/** @file
AML Utility.
- Copyright (c) 2019 - 2020, Arm Limited. All rights reserved.<BR>
+ Copyright (c) 2019 - 2021, Arm Limited. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -39,6 +39,22 @@ AmlComputeSize (
IN OUT UINT32 * Size
);
+/** Get the value contained in an integer node.
+
+ @param [in] Node Pointer to an integer node.
+ Must be an object node.
+ @param [out] Value Value contained in the integer node.
+
+ @retval EFI_SUCCESS The function completed successfully.
+ @retval EFI_INVALID_PARAMETER Invalid parameter.
+**/
+EFI_STATUS
+EFIAPI
+AmlNodeGetIntegerValue (
+ IN AML_OBJECT_NODE * Node,
+ OUT UINT64 * Value
+ );
+
/** Set the value contained in an integer node.
The OpCode is updated accordingly to the new value


Re: [PATCH v1 06/13] DynamicTablesPkg: AML code generation for a ResourceTemplate

Sami Mujawar
 

Hi Pierre,

Thank you for this patch.

Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>

Regards,

Sami Mujawar

On 23/06/2021 12:40 PM, Pierre.Gondois@arm.com wrote:
From: Pierre Gondois <Pierre.Gondois@arm.com>

ASL provides a ResourceTemplate macro that creates a Buffer in which
resource descriptor macros can be listed. The ResourceTemplate macro
automatically generates an End descriptor and calculates the checksum
for the resource template.

Therefore, add AmlCodeGenResourceTemplate() to generate AML code for
the ResourceTemplate() macro. This function generates a Buffer node
with an EndTag resource data descriptor, which is similar to the ASL
ResourceTemplate() macro.

Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
---
.../Common/AmlLib/CodeGen/AmlCodeGen.c | 192 ++++++++++++++++++
1 file changed, 192 insertions(+)

diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
index ea9b73b464a4..faf7902c1f21 100644
--- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
+++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
@@ -12,6 +12,7 @@
#include <AmlCoreInterface.h>
#include <AmlEncoding/Aml.h>
+#include <CodeGen/AmlResourceDataCodeGen.h>
#include <Tree/AmlNode.h>
#include <Tree/AmlTree.h>
#include <String/AmlString.h>
@@ -315,6 +316,197 @@ error_handler:
return Status;
}
+/** AML code generation for a Buffer object node.
+
+ To create a Buffer object node with an empty buffer,
+ call the function with (Buffer=NULL, BufferSize=0).
+
+ @param [in] Buffer Buffer to set for the created Buffer
+ object node. The Buffer's content is copied.
+ NULL if there is no buffer to set for
+ the Buffer node.
+ @param [in] BufferSize Size of the Buffer.
+ 0 if there is no buffer to set for
+ the Buffer node.
+ @param [out] NewObjectNode If success, contains the created
+ Buffer object node.
+
+ @retval EFI_SUCCESS Success.
+ @retval EFI_INVALID_PARAMETER Invalid parameter.
+ @retval EFI_OUT_OF_RESOURCES Failed to allocate memory.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+AmlCodeGenBuffer (
+ IN CONST UINT8 * Buffer, OPTIONAL
+ IN UINT32 BufferSize, OPTIONAL
+ OUT AML_OBJECT_NODE ** NewObjectNode
+ )
+{
+ EFI_STATUS Status;
+ AML_OBJECT_NODE * BufferNode;
+ AML_OBJECT_NODE * BufferSizeNode;
+ UINT32 BufferSizeNodeSize;
+ AML_DATA_NODE * DataNode;
+ UINT32 PkgLen;
+
+ // Buffer and BufferSize must be either both set, or both clear.
+ if ((NewObjectNode == NULL) ||
+ ((Buffer == NULL) != (BufferSize == 0))) {
+ ASSERT (0);
+ return EFI_INVALID_PARAMETER;
+ }
+
+ BufferNode = NULL;
+ DataNode = NULL;
+
+ // Cf ACPI 6.3 specification, s20.2.5.4 "Type 2 Opcodes Encoding"
+ // DefBuffer := BufferOp PkgLength BufferSize ByteList
+ // BufferOp := 0x11
+ // BufferSize := TermArg => Integer
+
+ Status = AmlCodeGenInteger (BufferSize, &BufferSizeNode);
+ if (EFI_ERROR (Status)) {
+ ASSERT (0);
+ return Status;
+ }
+
+ // Get the number of bytes required to encode the BufferSizeNode.
+ Status = AmlComputeSize (
+ (AML_NODE_HEADER*)BufferSizeNode,
+ &BufferSizeNodeSize
+ );
+ if (EFI_ERROR (Status)) {
+ ASSERT (0);
+ goto error_handler;
+ }
+
+ // Compute the size to write in the PkgLen.
+ Status = AmlComputePkgLength (BufferSizeNodeSize + BufferSize, &PkgLen);
+ if (EFI_ERROR (Status)) {
+ ASSERT (0);
+ goto error_handler;
+ }
+
+ // Create an object node for the buffer.
+ Status = AmlCreateObjectNode (
+ AmlGetByteEncodingByOpCode (AML_BUFFER_OP, 0),
+ PkgLen,
+ &BufferNode
+ );
+ if (EFI_ERROR (Status)) {
+ ASSERT (0);
+ goto error_handler;
+ }
+
+ // Set the BufferSizeNode as a fixed argument of the BufferNode.
+ Status = AmlSetFixedArgument (
+ BufferNode,
+ EAmlParseIndexTerm0,
+ (AML_NODE_HEADER*)BufferSizeNode
+ );
+ if (EFI_ERROR (Status)) {
+ ASSERT (0);
+ goto error_handler;
+ }
+
+ // BufferSizeNode is now attached.
+ BufferSizeNode = NULL;
+
+ // If there is a buffer, create a DataNode and attach it to the BufferNode.
+ if (Buffer != NULL) {
+ Status = AmlCreateDataNode (
+ EAmlNodeDataTypeRaw,
+ Buffer,
+ BufferSize,
+ &DataNode
+ );
+ if (EFI_ERROR (Status)) {
+ ASSERT (0);
+ goto error_handler;
+ }
+
+ Status = AmlVarListAddTail (
+ (AML_NODE_HEADER*)BufferNode,
+ (AML_NODE_HEADER*)DataNode
+ );
+ if (EFI_ERROR (Status)) {
+ ASSERT (0);
+ goto error_handler;
+ }
+ }
+
+ *NewObjectNode = BufferNode;
+ return Status;
+
+error_handler:
+ if (BufferSizeNode != NULL) {
+ AmlDeleteTree ((AML_NODE_HEADER*)BufferSizeNode);
+ }
+ if (BufferNode != NULL) {
+ AmlDeleteTree ((AML_NODE_HEADER*)BufferNode);
+ }
+ if (DataNode != NULL) {
+ AmlDeleteTree ((AML_NODE_HEADER*)DataNode);
+ }
+ return Status;
+}
+
+/** AML code generation for a ResourceTemplate.
+
+ "ResourceTemplate" is a macro defined in ACPI 6.3, s19.3.3
+ "ASL Resource Templates". It allows to store resource data elements.
+
+ In AML, a ResourceTemplate is implemented as a Buffer storing resource
+ data elements. An EndTag resource data descriptor must be at the end
+ of the list of resource data elements.
+ This function generates a Buffer node with an EndTag resource data
+ descriptor. It can be seen as an empty list of resource data elements.
+
+ @param [out] NewObjectNode If success, contains the created
+ ResourceTemplate object node.
+
+ @retval EFI_SUCCESS Success.
+ @retval EFI_INVALID_PARAMETER Invalid parameter.
+ @retval EFI_OUT_OF_RESOURCES Failed to allocate memory.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+AmlCodeGenResourceTemplate (
+ OUT AML_OBJECT_NODE ** NewObjectNode
+ )
+{
+ EFI_STATUS Status;
+ AML_OBJECT_NODE * BufferNode;
+
+ if (NewObjectNode == NULL) {
+ ASSERT (0);
+ return EFI_INVALID_PARAMETER;
+ }
+
+ BufferNode = NULL;
+
+ // Create a BufferNode with an empty buffer.
+ Status = AmlCodeGenBuffer (NULL, 0, &BufferNode);
+ if (EFI_ERROR (Status)) {
+ ASSERT (0);
+ return Status;
+ }
+
+ // Create an EndTag resource data element and attach it to the Buffer.
+ Status = AmlCodeGenEndTag (0, BufferNode, NULL);
+ if (EFI_ERROR (Status)) {
+ ASSERT (0);
+ AmlDeleteTree ((AML_NODE_HEADER*)BufferNode);
+ return Status;
+ }
+
+ *NewObjectNode = BufferNode;
+ return Status;
+}
+
/** AML code generation for a Name object node.
@param [in] NameString The new variable name.


Re: [PATCH v1 05/13] DynamicTablesPkg: Helper function to compute package length

Sami Mujawar
 

Hi Pierre,

This patch looks good to me.

Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>

Regards,

Sami Mujawar

On 23/06/2021 12:40 PM, Pierre.Gondois@arm.com wrote:
From: Pierre Gondois <Pierre.Gondois@arm.com>

Some AML object have a PkgLen which indicates the size of the
AML object. The package length can be encoded in 1 to 4 bytes.
The bytes used to encode the PkgLen is itself counted in the
PkgLen value. So, if an AML object's size increments/decrements,
the number of bytes used to encode the PkgLen value can itself
increment/decrement.

Therefore, a helper function AmlComputePkgLength() is introduced
to simply computation of the PkgLen.

Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
---
.../Library/Common/AmlLib/AmlEncoding/Aml.c | 87 ++++++++++++++++++-
.../Library/Common/AmlLib/AmlEncoding/Aml.h | 47 +++++++++-
2 files changed, 132 insertions(+), 2 deletions(-)

diff --git a/DynamicTablesPkg/Library/Common/AmlLib/AmlEncoding/Aml.c b/DynamicTablesPkg/Library/Common/AmlLib/AmlEncoding/Aml.c
index eadafca97ea5..d829b1869846 100644
--- a/DynamicTablesPkg/Library/Common/AmlLib/AmlEncoding/Aml.c
+++ b/DynamicTablesPkg/Library/Common/AmlLib/AmlEncoding/Aml.c
@@ -2,7 +2,7 @@
AML grammar definitions.
Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved. <BR>
- Copyright (c) 2019 - 2020, Arm Limited. All rights reserved.<BR>
+ Copyright (c) 2019 - 2021, Arm Limited. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -803,3 +803,88 @@ AmlComputePkgLengthWidth (
// Length < 2^6
return 1;
}
+
+/** Given a length, compute the value of a PkgLen.
+
+ In AML, some object have a PkgLen, telling the size of the AML object.
+ It can be encoded in 1 to 4 bytes. The bytes used to encode the PkgLen is
+ itself counted in the PkgLen value.
+ This means that if an AML object sees its size increment/decrement,
+ the number of bytes used to encode the PkgLen value can itself
+ increment/decrement.
+
+ For instance, the AML encoding of a DeviceOp is:
+ DefDevice := DeviceOp PkgLength NameString TermList
+ If:
+ - sizeof (NameString) = 4 (the name is "DEV0" for instance);
+ - sizeof (TermList) = (2^6-6)
+ then the PkgLen is encoded on 1 byte. Indeed, its value is:
+ sizeof (PkgLen) + sizeof (NameString) + sizeof (TermList) =
+ sizeof (PkgLen) + 4 + (2^6-6)
+ So:
+ PkgLen = sizeof (PkgLen) + (2^6-2)
+
+ The input arguments Length and PkgLen represent, for the DefDevice:
+ DefDevice := DeviceOp PkgLength NameString TermList
+ |------Length-----|
+ |--------*PgkLength---------|
+
+ @param [in] Length The length to encode as a PkgLen.
+ Length cannot exceed 2^28 - 4 (4 bytes for the
+ PkgLen encoding).
+ The size of the PkgLen encoding bytes should not be
+ counted in this length value.
+ @param [out] PkgLen If success, contains the value of the PkgLen,
+ ready to encode in the PkgLen format.
+ This value takes into account the size of PkgLen
+ encoding.
+
+ @retval EFI_SUCCESS The function completed successfully.
+ @retval EFI_INVALID_PARAMETER Invalid parameter.
+**/
+EFI_STATUS
+EFIAPI
+AmlComputePkgLength (
+ IN UINT32 Length,
+ OUT UINT32 * PkgLen
+ )
+{
+ UINT32 PkgLenWidth;
+ UINT32 ReComputedPkgLenWidth;
+
+ if (PkgLen == NULL) {
+ ASSERT (0);
+ return EFI_INVALID_PARAMETER;
+ }
+
+ // Compute the PkgLenWidth.
+ PkgLenWidth = AmlComputePkgLengthWidth (Length);
+ if (PkgLenWidth == 0) {
+ ASSERT (0);
+ return EFI_INVALID_PARAMETER;
+ }
+
+ // Add it to the Length.
+ Length += PkgLenWidth;
+
+ // Check that adding the PkgLenWidth didn't trigger a domino effect,
+ // increasing the encoding width of the PkgLen again.
+ // The PkgLen is encoded in at most 4 bytes. It is possible to increase
+ // the PkgLen width if its encoding is less than 3 bytes.
+ ReComputedPkgLenWidth = AmlComputePkgLengthWidth (Length);
+ if (ReComputedPkgLenWidth != PkgLenWidth) {
+ if ((ReComputedPkgLenWidth != 0) &&
+ (ReComputedPkgLenWidth < 4)) {
+ // No need to recompute the PkgLen since a new threshold cannot
+ // be reached by incrementing the value by one.
+ Length += 1;
+ } else {
+ ASSERT (0);
+ return EFI_INVALID_PARAMETER;
+ }
+ }
+
+ *PkgLen = Length;
+
+ return EFI_SUCCESS;
+}
diff --git a/DynamicTablesPkg/Library/Common/AmlLib/AmlEncoding/Aml.h b/DynamicTablesPkg/Library/Common/AmlLib/AmlEncoding/Aml.h
index 35c0680b6159..0641500fcd5f 100644
--- a/DynamicTablesPkg/Library/Common/AmlLib/AmlEncoding/Aml.h
+++ b/DynamicTablesPkg/Library/Common/AmlLib/AmlEncoding/Aml.h
@@ -2,7 +2,7 @@
AML grammar definitions.
Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved. <BR>
- Copyright (c) 2019 - 2020, Arm Limited. All rights reserved.<BR>
+ Copyright (c) 2019 - 2021, Arm Limited. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -326,5 +326,50 @@ AmlComputePkgLengthWidth (
IN UINT32 Length
);
+/** Given a length, compute the value of a PkgLen.
+
+ In AML, some object have a PkgLen, telling the size of the AML object.
+ It can be encoded in 1 to 4 bytes. The bytes used to encode the PkgLen is
+ itself counted in the PkgLen value.
+ This means that if an AML object sees its size increment/decrement,
+ the number of bytes used to encode the PkgLen value can itself
+ increment/decrement.
+
+ For instance, the AML encoding of a DeviceOp is:
+ DefDevice := DeviceOp PkgLength NameString TermList
+ If:
+ - sizeof (NameString) = 4 (the name is "DEV0" for instance);
+ - sizeof (TermList) = (2^6-6)
+ then the PkgLen is encoded on 1 byte. Indeed, its value is:
+ sizeof (PkgLen) + sizeof (NameString) + sizeof (TermList) =
+ sizeof (PkgLen) + 4 + (2^6-6)
+ So:
+ PkgLen = sizeof (PkgLen) + (2^6-2)
+
+ The input arguments Length and PkgLen represent, for the DefDevice:
+ DefDevice := DeviceOp PkgLength NameString TermList
+ |------Length-----|
+ |--------*PgkLength---------|
+
+ @param [in] Length The length to encode as a PkgLen.
+ Length cannot exceed 2^28 - 4 (4 bytes for the
+ PkgLen encoding).
+ The size of the PkgLen encoding bytes should not be
+ counted in this length value.
+ @param [out] PkgLen If success, contains the value of the PkgLen,
+ ready to encode in the PkgLen format.
+ This value takes into account the size of PkgLen
+ encoding.
+
+ @retval EFI_SUCCESS The function completed successfully.
+ @retval EFI_INVALID_PARAMETER Invalid parameter.
+**/
+EFI_STATUS
+EFIAPI
+AmlComputePkgLength (
+ IN UINT32 Length,
+ OUT UINT32 * PkgLen
+ );
+
#endif // AML_H_


Re: [PATCH v1 04/13] DynamicTablesPkg: AML code generation for a Package

Sami Mujawar
 

Hi Pierre,

Thank you for this patch. This patch looks good to me.

Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>

Regards,

Sami Mujawar

On 23/06/2021 12:40 PM, Pierre.Gondois@arm.com wrote:
From: Pierre Gondois <Pierre.Gondois@arm.com>

Add AmlCodeGenPackage() to generate AML code for declaring
a Package() object. This function generates an empty package
node. New elements can then be added to the package's variable
argument list.

Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
---
.../Common/AmlLib/CodeGen/AmlCodeGen.c | 82 ++++++++++++++++++-
1 file changed, 81 insertions(+), 1 deletion(-)

diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
index 5d310f201319..ea9b73b464a4 100644
--- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
+++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
@@ -1,7 +1,7 @@
/** @file
AML Code Generation.
- Copyright (c) 2020, Arm Limited. All rights reserved.<BR>
+ Copyright (c) 2020 - 2021, Arm Limited. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -235,6 +235,86 @@ AmlCodeGenInteger (
return Status;
}
+/** AML code generation for a Package object node.
+
+ The package generated is empty. New elements can be added via its
+ list of variable arguments.
+
+ @param [out] NewObjectNode If success, contains the created
+ Package object node.
+
+ @retval EFI_SUCCESS Success.
+ @retval EFI_INVALID_PARAMETER Invalid parameter.
+ @retval EFI_OUT_OF_RESOURCES Failed to allocate memory.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+AmlCodeGenPackage (
+ OUT AML_OBJECT_NODE ** NewObjectNode
+ )
+{
+ EFI_STATUS Status;
+ AML_DATA_NODE * DataNode;
+ UINT8 NodeCount;
+
+ if (NewObjectNode == NULL) {
+ ASSERT (0);
+ return EFI_INVALID_PARAMETER;
+ }
+
+ DataNode = NULL;
+ NodeCount = 0;
+
+ // Create an object node.
+ // PkgLen is 2:
+ // - one byte to store the PkgLength
+ // - one byte for the NumElements.
+ // Cf ACPI6.3, s20.2.5 "Term Objects Encoding"
+ // DefPackage := PackageOp PkgLength NumElements PackageElementList
+ // NumElements := ByteData
+ Status = AmlCreateObjectNode (
+ AmlGetByteEncodingByOpCode (AML_PACKAGE_OP, 0),
+ 2,
+ NewObjectNode
+ );
+ if (EFI_ERROR (Status)) {
+ ASSERT (0);
+ return Status;
+ }
+
+ // NumElements is a ByteData.
+ Status = AmlCreateDataNode (
+ EAmlNodeDataTypeUInt,
+ &NodeCount,
+ sizeof (NodeCount),
+ &DataNode
+ );
+ if (EFI_ERROR (Status)) {
+ ASSERT (0);
+ goto error_handler;
+ }
+
+ Status = AmlSetFixedArgument (
+ *NewObjectNode,
+ EAmlParseIndexTerm0,
+ (AML_NODE_HEADER*)DataNode
+ );
+ if (EFI_ERROR (Status)) {
+ ASSERT (0);
+ goto error_handler;
+ }
+
+ return Status;
+
+error_handler:
+ AmlDeleteTree ((AML_NODE_HEADER*)*NewObjectNode);
+ if (DataNode != NULL) {
+ AmlDeleteTree ((AML_NODE_HEADER*)DataNode);
+ }
+ return Status;
+}
+
/** AML code generation for a Name object node.
@param [in] NameString The new variable name.


Re: [PATCH v1 03/13] DynamicTablesPkg: AML Code generation for Resource data EndTag

Sami Mujawar
 

Hi Pierre,

Thank you for this patch.

Please find my response inline marked [SAMI].

Regards,

Sami Mujawar


On 23/06/2021 12:40 PM, Pierre.Gondois@arm.com wrote:
From: Pierre Gondois <Pierre.Gondois@arm.com>

Add a helper function AmlCodeGenEndTag() to generate AML Resource Data
EndTag. The EndTag resource data is automatically generated by the ASL
compiler at the end of a list of resource data elements. Therefore, an
equivalent function is not present in ASL.

However, AmlCodeGenEndTag() is useful when generating AML code for the
ResourceTemplate() macro.

Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
---
.../AmlLib/CodeGen/AmlResourceDataCodeGen.c | 104 ++++++++++++++++++
.../AmlLib/CodeGen/AmlResourceDataCodeGen.h | 34 ++++++
2 files changed, 138 insertions(+)

diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.c b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.c
index 07a96725a4ef..78910cc5d4b4 100644
--- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.c
+++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.c
@@ -274,6 +274,110 @@ AmlCodeGenRdRegister (
return LinkRdNode (RdNode, NameOpNode, NewRdNode);
}
+/** Code generation for the EndTag resource data.
+
+ The EndTag resource data is automatically generated by the ASL compiler
+ at the end of a list of resource data elements. Thus, it doesn't have
+ a corresponding ASL function.
+
+ This function allocates memory to create a data node. It is the caller's
+ responsibility to either:
+ - attach this node to an AML tree;
+ - delete this node.
+
+ @param [in] CheckSum CheckSum to store in the EndTag.
+ Optional: can be let to 0. It is not
+ updated when new resource data elements
+ are added/removed/modified in the list.
[SAMI] Can you rephrase the description, please? If I understand correctly, the current implementation does not compute the checksum. However, if the checksum value provided is not zero then an update to the resource nodes would need to recompute the checksum, right?
+ @param [in] ParentNode If not NULL, add the generated node
+ to the end of the variable list of
+ argument of the ParentNode.
+ The ParentNode must not initially contain
+ an EndTag resource data element.
+ @param [out] NewRdNode If success, contains the generated node.
+
+ @retval EFI_SUCCESS The function completed successfully.
+ @retval EFI_INVALID_PARAMETER Invalid parameter.
+ @retval EFI_OUT_OF_RESOURCES Could not allocate memory.
+**/
+EFI_STATUS
+EFIAPI
+AmlCodeGenEndTag (
+ IN UINT8 CheckSum, OPTIONAL
+ IN AML_OBJECT_NODE * ParentNode, OPTIONAL
+ OUT AML_DATA_NODE ** NewRdNode OPTIONAL
+ )
+{
+ EFI_STATUS Status;
+ AML_DATA_NODE * RdNode;
+ EFI_ACPI_END_TAG_DESCRIPTOR EndTag;
+ ACPI_SMALL_RESOURCE_HEADER SmallResHdr;
+
+ if ((ParentNode == NULL) && (NewRdNode == NULL)) {
+ ASSERT (0);
+ return EFI_INVALID_PARAMETER;
+ }
+
+ RdNode = NULL;
+
+ // Header
+ SmallResHdr.Bits.Length = sizeof (EFI_ACPI_END_TAG_DESCRIPTOR) -
+ sizeof (ACPI_SMALL_RESOURCE_HEADER);
+ SmallResHdr.Bits.Name = ACPI_SMALL_END_TAG_DESCRIPTOR_NAME;
+ SmallResHdr.Bits.Type = ACPI_SMALL_ITEM_FLAG;
+
+ // Body
+ EndTag.Desc = SmallResHdr.Byte;
+ EndTag.Checksum = CheckSum;
+
+ Status = AmlCreateDataNode (
+ EAmlNodeDataTypeResourceData,
+ (UINT8*)&EndTag,
+ sizeof (EFI_ACPI_END_TAG_DESCRIPTOR),
+ &RdNode
+ );
+ if (EFI_ERROR (Status)) {
+ ASSERT (0);
+ return Status;
+ }
+
+ if (NewRdNode != NULL) {
+ *NewRdNode = RdNode;
+ }
+
+ if (ParentNode != NULL) {
+ // Check the BufferOp doesn't contain any resource data yet.
+ // This is a hard check: do not allow to add an EndTag if the BufferNode
+ // already has resource data elements attached. Indeed, the EndTag should
+ // have already been added.
+ if (AmlGetNextVariableArgument ((AML_NODE_HEADER*)ParentNode, NULL) !=
+ NULL) {
+ ASSERT (0);
+ Status = EFI_INVALID_PARAMETER;
+ goto error_handler;
+ }
+
+ // Manually add the EndTag RdNode. Indeed, the AmlAppendRdNode function
+ // is looking for an EndTag, which we are adding here.
+ Status = AmlVarListAddTail (
+ (AML_NODE_HEADER*)ParentNode,
+ (AML_NODE_HEADER*)RdNode
+ );
+ if (EFI_ERROR (Status)) {
+ ASSERT (0);
+ goto error_handler;
+ }
+ }
+
+ return Status;
+
+error_handler:
+ if (RdNode != NULL) {
+ AmlDeleteTree ((AML_NODE_HEADER*)RdNode);
+ }
+ return Status;
+}
+
// DEPRECATED APIS
#ifndef DISABLE_NEW_DEPRECATED_INTERFACES
diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.h b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.h
index 3c9217d9ddab..0b464305da40 100644
--- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.h
+++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.h
@@ -104,4 +104,38 @@ AmlCodeGenRdRegister (
OUT AML_DATA_NODE_HANDLE *NewRdNode OPTIONAL
);
+/** Code generation for the EndTag resource data.
+
+ The EndTag resource data is automatically generated by the ASL compiler
+ at the end of a list of resource data elements. Thus, it doesn't have
+ a corresponding ASL function.
+
+ This function allocates memory to create a data node. It is the caller's
+ responsibility to either:
+ - attach this node to an AML tree;
+ - delete this node.
+
+ @param [in] CheckSum CheckSum to store in the EndTag.
+ Optional: can be let to 0. It is not
+ updated when new resource data elements
+ are added/removed/modified in the list.
+ @param [in] ParentNode If not NULL, add the generated node
+ to the end of the variable list of
+ argument of the ParentNode.
+ The ParentNode must not initially contain
+ an EndTag resource data element.
+ @param [out] NewRdNode If success, contains the generated node.
+
+ @retval EFI_SUCCESS The function completed successfully.
+ @retval EFI_INVALID_PARAMETER Invalid parameter.
+ @retval EFI_OUT_OF_RESOURCES Could not allocate memory.
+**/
+EFI_STATUS
+EFIAPI
+AmlCodeGenEndTag (
+ IN UINT8 CheckSum, OPTIONAL
+ IN AML_OBJECT_NODE * ParentNode, OPTIONAL
+ OUT AML_DATA_NODE ** NewRdNode OPTIONAL
+ );
+
#endif // AML_RESOURCE_DATA_CODE_GEN_H_


Re: [PATCH v1 02/13] DynamicTablesPkg: AML Code generation for Register()

Sami Mujawar
 

Hi Pierre,

Thank you for this patch.

I have a minor suggestion marked inline as [SAMI].

With that changed,

Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>

Regards,

Sami Mujawar


On 23/06/2021 12:40 PM, Pierre.Gondois@arm.com wrote:
From: Pierre Gondois <Pierre.Gondois@arm.com>

Add AmlCodeGenRegister() to generate AML code for the
Generic Register Resource Descriptor. This function is
equivalent to the ASL macro Register().

Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
---
.../AmlLib/CodeGen/AmlResourceDataCodeGen.c | 87 +++++++++++++++++++
.../AmlLib/CodeGen/AmlResourceDataCodeGen.h | 49 +++++++++++
2 files changed, 136 insertions(+)

diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.c b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.c
index 089597a6c906..07a96725a4ef 100644
--- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.c
+++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.c
@@ -187,6 +187,93 @@ AmlCodeGenRdInterrupt (
return LinkRdNode (RdNode, NameOpNode, NewRdNode);
}
+/** Code generation for the "Register ()" ASL function.
+
+ The Resource Data effectively created is a Generic Register Descriptor.
+ Data. Cf ACPI 6.4:
+ - s6.4.3.7 "Generic Register Descriptor".
+ - s19.6.114 "Register".
+
+ The created resource data node can be:
+ - appended to the list of resource data elements of the NameOpNode.
+ In such case NameOpNode must be defined by a the "Name ()" ASL statement
+ and initially contain a "ResourceTemplate ()".
+ - returned through the NewRdNode parameter.
+
+ @param [in] AddressSpace Address space where the register exists.
+ Can be one of I/O space, System Memory, etc.
+ @param [in] BitWidth Number of bits in the register.
+ @param [in] BitOffset Offset in bits from the start of the register
+ indicated by the Address.
+ @param [in] Address Register address.
+ @param [in] AccessSize Size of data values used when accessing the
+ address space. Can be one of:
+ 0 - Undefined, legacy (EFI_ACPI_6_3_UNDEFINED)
+ 1 - Byte access (EFI_ACPI_6_3_BYTE)
+ 2 - Word access (EFI_ACPI_6_3_WORD)
+ 3 - DWord access (EFI_ACPI_6_3_DWORD)
+ 4 - QWord access (EFI_ACPI_6_3_QWORD)
+ @param [in] NameOpNode NameOp object node defining a named object.
+ If provided, append the new resource data node
+ to the list of resource data elements of this
+ node.
+ @param [out] NewRdNode If provided and success,
+ contain the created node.
+
+ @retval EFI_SUCCESS The function completed successfully.
+ @retval EFI_INVALID_PARAMETER Invalid parameter.
+ @retval EFI_OUT_OF_RESOURCES Could not allocate memory.
+**/
+EFI_STATUS
+EFIAPI
+AmlCodeGenRdRegister (
+ IN UINT8 AddressSpace,
+ IN UINT8 BitWidth,
+ IN UINT8 BitOffset,
+ IN UINT64 Address,
+ IN UINT8 AccessSize,
+ IN AML_OBJECT_NODE_HANDLE NameOpNode, OPTIONAL
+ OUT AML_DATA_NODE_HANDLE *NewRdNode OPTIONAL
+ )
+{
+ EFI_STATUS Status;
+ AML_DATA_NODE * RdNode;
+ EFI_ACPI_GENERIC_REGISTER_DESCRIPTOR RdRegister;
+
+ if ((AccessSize > EFI_ACPI_6_3_QWORD) ||
[SAMI] Can we use the EFI_ACPI_6_4* defines, please? Corresponding changes are also needed at other places in this patch.
+ ((NameOpNode == NULL) && (NewRdNode == NULL))) {
+ ASSERT (0);
+ return EFI_INVALID_PARAMETER;
+ }
+
+ // Header
+ RdRegister.Header.Header.Bits.Name =
+ ACPI_LARGE_GENERIC_REGISTER_DESCRIPTOR_NAME;
+ RdRegister.Header.Header.Bits.Type = ACPI_LARGE_ITEM_FLAG;
+ RdRegister.Header.Length = sizeof (EFI_ACPI_GENERIC_REGISTER_DESCRIPTOR) -
+ sizeof (ACPI_LARGE_RESOURCE_HEADER);
+
+ // Body
+ RdRegister.AddressSpaceId = AddressSpace;
+ RdRegister.RegisterBitWidth = BitWidth;
+ RdRegister.RegisterBitOffset = BitOffset;
+ RdRegister.AddressSize = AccessSize;
+ RdRegister.RegisterAddress = Address;
+
+ Status = AmlCreateDataNode (
+ EAmlNodeDataTypeResourceData,
+ (UINT8*)&RdRegister,
+ sizeof (EFI_ACPI_GENERIC_REGISTER_DESCRIPTOR),
+ &RdNode
+ );
+ if (EFI_ERROR (Status)) {
+ ASSERT (0);
+ return Status;
+ }
+
+ return LinkRdNode (RdNode, NameOpNode, NewRdNode);
+}
+
// DEPRECATED APIS
#ifndef DISABLE_NEW_DEPRECATED_INTERFACES
diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.h b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.h
index 764051e3d7c9..3c9217d9ddab 100644
--- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.h
+++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.h
@@ -55,4 +55,53 @@ AmlCodeGenRdInterrupt (
OUT AML_DATA_NODE_HANDLE *NewRdNode OPTIONAL
);
+/** Code generation for the "Register ()" ASL function.
+
+ The Resource Data effectively created is a Generic Register Descriptor.
+ Data. Cf ACPI 6.4:
+ - s6.4.3.7 "Generic Register Descriptor".
+ - s19.6.114 "Register".
+
+ The created resource data node can be:
+ - appended to the list of resource data elements of the NameOpNode.
+ In such case NameOpNode must be defined by a the "Name ()" ASL statement
+ and initially contain a "ResourceTemplate ()".
+ - returned through the NewRdNode parameter.
+
+ @param [in] AddressSpace Address space where the register exists.
+ Can be one of I/O space, System Memory, etc.
+ @param [in] BitWidth Number of bits in the register.
+ @param [in] BitOffset Offset in bits from the start of the register
+ indicated by the Address.
+ @param [in] Address Register address.
+ @param [in] AccessSize Size of data values used when accessing the
+ address space. Can be one of:
+ 0 - Undefined, legacy (EFI_ACPI_6_3_UNDEFINED)
+ 1 - Byte access (EFI_ACPI_6_3_BYTE)
+ 2 - Word access (EFI_ACPI_6_3_WORD)
+ 3 - DWord access (EFI_ACPI_6_3_DWORD)
+ 4 - QWord access (EFI_ACPI_6_3_QWORD)
+ @param [in] NameOpNode NameOp object node defining a named object.
+ If provided, append the new resource data node
+ to the list of resource data elements of this
+ node.
+ @param [out] NewRdNode If provided and success,
+ contain the created node.
+
+ @retval EFI_SUCCESS The function completed successfully.
+ @retval EFI_INVALID_PARAMETER Invalid parameter.
+ @retval EFI_OUT_OF_RESOURCES Could not allocate memory.
+**/
+EFI_STATUS
+EFIAPI
+AmlCodeGenRdRegister (
+ IN UINT8 AddressSpace,
+ IN UINT8 BitWidth,
+ IN UINT8 BitOffset,
+ IN UINT64 Address,
+ IN UINT8 AccessSize,
+ IN AML_OBJECT_NODE_HANDLE NameOpNode, OPTIONAL
+ OUT AML_DATA_NODE_HANDLE *NewRdNode OPTIONAL
+ );
+
#endif // AML_RESOURCE_DATA_CODE_GEN_H_


Re: [PATCH v2 00/10] Various DynamicTablesPkg modifications

Sami Mujawar
 

Merged as 422e5d2f7f1a..22873f58c40c

Regards,

Sami Mujawar


On 30/09/2021 08:48 AM, Pierre.Gondois@arm.com wrote:
From: Pierre Gondois <Pierre.Gondois@arm.com>

This patch-set aggregates various modifications in the
DynamicTablesPkg:
- Extract an AcpiTableHelperLib from TableHelperLib to remove
the dependency of some utility functions over configuration
manager definitions
- Add a HexFromAscii() function
- Add a AmlGetEisaIdFromString() function
- Add a configuration manager object parser
- Use %a instead of %s in when printing AmlLib
- Update the .ci.yaml once to prepare for other incoming patches
- Modify the generic prototype of the AmlResourceDataCodeGen
functions. This also means deprecating some functions.

The modifications can be seen at: https://github.com/PierreARM/edk2/tree/1718_Various_DynamicTablesPkg_modifications_v2

v2:
- Corrections in commit messages. [Sami]
- Document returned error code for some functions. [Sami]
- Correct wrong field name in Configuration Manager Object parser.
[Joey]

Pierre Gondois (9):
DynamicTablesPkg: Extract AcpiHelperLib from TableHelperLib
DynamicTablesPkg: Update TableHelperLib.inf
DynamicTablesPkg: Rename single char input parameter
DynamicTablesPkg: Add HexFromAscii() to AcpiHelperLib
DynamicTablesPkg: Add AmlGetEisaIdFromString() to AcpiHelperLib
DynamicTablesPkg: Use %a formatter in AmlDbgPrint
DynamicTablesPkg: Update DynamicTablesPkg.ci.yaml
DynamicTablesPkg: Deprecate Crs specific methods in AmlLib
DynamicTablesPkg: Rework AmlResourceDataCodegen.c/h

Sami Mujawar (1):
DynamicTablesPkg: Add Configuration Manager Object parser

DynamicTablesPkg/DynamicTables.dsc.inc | 3 +-
DynamicTablesPkg/DynamicTablesPkg.ci.yaml | 29 +
DynamicTablesPkg/DynamicTablesPkg.dec | 4 +
DynamicTablesPkg/DynamicTablesPkg.dsc | 1 +
.../Include/Library/AcpiHelperLib.h | 93 +++
.../Include/Library/AmlLib/AmlLib.h | 225 ++++--
.../Include/Library/TableHelperLib.h | 49 +-
.../SsdtCmn600Generator.c | 14 +-
.../AcpiSsdtCmn600LibArm/SsdtCmn600LibArm.inf | 3 +-
.../SsdtSerialPortGenerator.c | 3 +-
.../SsdtSerialPortLibArm.inf | 4 +-
.../Library/Common/AcpiHelperLib/AcpiHelper.c | 210 ++++++
.../Common/AcpiHelperLib/AcpiHelperLib.inf | 25 +
.../Common/AmlLib/AmlDbgPrint/AmlDbgPrint.c | 16 +-
.../Library/Common/AmlLib/AmlLib.inf | 3 +-
.../Library/Common/AmlLib/Api/AmlApi.c | 147 +++-
.../AmlLib/CodeGen/AmlResourceDataCodeGen.c | 194 ++---
.../AmlLib/CodeGen/AmlResourceDataCodeGen.h | 67 +-
.../SsdtSerialPortFixupLib.c | 6 +-
.../SsdtSerialPortFixupLib.inf | 1 +
.../ConfigurationManagerObjectParser.c | 678 ++++++++++++++++++
.../ConfigurationManagerObjectParser.h | 73 ++
.../Common/TableHelperLib/TableHelper.c | 96 ---
.../Common/TableHelperLib/TableHelperLib.inf | 13 +-
24 files changed, 1579 insertions(+), 378 deletions(-)
create mode 100644 DynamicTablesPkg/Include/Library/AcpiHelperLib.h
create mode 100644 DynamicTablesPkg/Library/Common/AcpiHelperLib/AcpiHelper.c
create mode 100644 DynamicTablesPkg/Library/Common/AcpiHelperLib/AcpiHelperLib.inf
create mode 100644 DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.c
create mode 100644 DynamicTablesPkg/Library/Common/TableHelperLib/ConfigurationManagerObjectParser.h
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.


[PATCH] IntelFsp2Pkg: Adopt FSP 2.3 specification.

Chiu, Chasel
 

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3D3674

Add ExtendedImageRevision in FSP_INFO_HEADER structure, also add
FSP_NON_VOLATILE_STORAGE_HOB2 header.

Cc: Maurice Ma <maurice.ma@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>
---
IntelFsp2Pkg/Include/Guid/FspHeaderFile.h | 33 +++++++++++++++=
+++++++++++++++++-
IntelFsp2Pkg/Include/Guid/FspNonVolatileStorageHob2.h | 27 +++++++++++++++=
++++++++++++
IntelFsp2Pkg/IntelFsp2Pkg.dec | 1 +
3 files changed, 60 insertions(+), 1 deletion(-)

diff --git a/IntelFsp2Pkg/Include/Guid/FspHeaderFile.h b/IntelFsp2Pkg/Inclu=
de/Guid/FspHeaderFile.h
index 3474bac1de..11e0ede65b 100644
--- a/IntelFsp2Pkg/Include/Guid/FspHeaderFile.h
+++ b/IntelFsp2Pkg/Include/Guid/FspHeaderFile.h
@@ -2,7 +2,7 @@
Intel FSP Header File definition from Intel Firmware Support Package Ext=
ernal=0D
Architecture Specification v2.0 and above.=0D
=0D
- Copyright (c) 2014 - 2020, Intel Corporation. All rights reserved.<BR>=0D
+ Copyright (c) 2014 - 2021, Intel Corporation. All rights reserved.<BR>=0D
SPDX-License-Identifier: BSD-2-Clause-Patent=0D
=0D
**/=0D
@@ -44,14 +44,28 @@ typedef struct {
UINT8 Reserved1[2];=0D
///=0D
/// Byte 0x0A: Indicates compliance with a revision of this specificatio=
n in the BCD format.=0D
+ /// For revision v2.3 the value will be 0x23.=0D
///=0D
UINT8 SpecVersion;=0D
///=0D
/// Byte 0x0B: Revision of the FSP Information Header.=0D
+ /// The Current value for this field is 0x6.=0D
///=0D
UINT8 HeaderRevision;=0D
///=0D
/// Byte 0x0C: Revision of the FSP binary.=0D
+ /// Major.Minor.Revision.Build=0D
+ /// If FSP HeaderRevision is <=3D 5, the ImageRevision can be=
decoded as follows:=0D
+ /// 7 : 0 - Build Number=0D
+ /// 15 : 8 - Revision=0D
+ /// 23 : 16 - Minor Version=0D
+ /// 31 : 24 - Major Version=0D
+ /// If FSP HeaderRevision is >=3D 6, ImageRevision specifies =
the low-order bytes of the build number and revision=0D
+ /// while ExtendedImageRevision specifies the high-order byte=
s of the build number and revision.=0D
+ /// 7 : 0 - Low Byte of Build Number=0D
+ /// 15 : 8 - Low Byte of Revision=0D
+ /// 23 : 16 - Minor Version=0D
+ /// 31 : 24 - Major Version=0D
///=0D
UINT32 ImageRevision;=0D
///=0D
@@ -116,6 +130,23 @@ typedef struct {
/// If the value is set to 0x00000000, then this API is not a=
vailable in this component.=0D
///=0D
UINT32 FspMultiPhaseSiInitEntryOffset;=0D
+ ///=0D
+ /// Byte 0x4C: Extended revision of the FSP binary.=0D
+ /// This value is only valid if FSP HeaderRevision is >=3D 6.=
=0D
+ /// ExtendedImageRevision specifies the high-order byte of th=
e revision and build number in the FSP binary revision.=0D
+ /// 7 : 0 - High Byte of Build Number=0D
+ /// 15 : 8 - High Byte of Revision=0D
+ /// The FSP binary build number can be decoded as follows:=0D
+ /// Build Number =3D (ExtendedImageRevision[7:0] << 8) | Imag=
eRevision[7:0]=0D
+ /// Revision =3D (ExtendedImageRevision[15:8] << 8) | ImageRe=
vision[15:8]=0D
+ /// Minor Version =3D ImageRevision[23:16]=0D
+ /// Major Version =3D ImageRevision[31:24]=0D
+ ///=0D
+ UINT16 ExtendedImageRevision;=0D
+ ///=0D
+ /// Byte 0x4E: Reserved4.=0D
+ ///=0D
+ UINT16 Reserved4;=0D
} FSP_INFO_HEADER;=0D
=0D
///=0D
diff --git a/IntelFsp2Pkg/Include/Guid/FspNonVolatileStorageHob2.h b/IntelF=
sp2Pkg/Include/Guid/FspNonVolatileStorageHob2.h
new file mode 100644
index 0000000000..8c07dc7e7e
--- /dev/null
+++ b/IntelFsp2Pkg/Include/Guid/FspNonVolatileStorageHob2.h
@@ -0,0 +1,27 @@
+/** @file=0D
+ Intel FSP Non-Volatile Storage (NVS) HOB version 2 definition from=0D
+ Intel Firmware Support Package External Architecture Specification v2.3.=
=0D
+=0D
+ Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>=0D
+ SPDX-License-Identifier: BSD-2-Clause-Patent=0D
+=0D
+**/=0D
+=0D
+#ifndef __FSP_NON_VOLATILE_STORAGE_HOB2_H__=0D
+#define __FSP_NON_VOLATILE_STORAGE_HOB2_H__=0D
+=0D
+///=0D
+/// The Non-Volatile Storage (NVS) HOB version 2 provides > 64KB buffer su=
pport.=0D
+///=0D
+#define FSP_NON_VOLATILE_STORAGE_HOB2_GUID \=0D
+ { 0x4866788f, 0x6ba8, 0x47d8, { 0x83, 0x06, 0xac, 0xf7, 0x7f, 0x55, 0x10=
, 0x46 } }=0D
+=0D
+typedef struct {=0D
+ EFI_HOB_GUID_TYPE GuidHob;=0D
+ EFI_PHYSICAL_ADDRESS NvsDataPtr;=0D
+ UINT64 NvsDataLength;=0D
+} FSP_NON_VOLATILE_STORAGE_HOB2;=0D
+=0D
+extern EFI_GUID gFspNonVolatileStorageHob2Guid;=0D
+=0D
+#endif=0D
diff --git a/IntelFsp2Pkg/IntelFsp2Pkg.dec b/IntelFsp2Pkg/IntelFsp2Pkg.dec
index ec7b9a7702..58eac7220d 100644
--- a/IntelFsp2Pkg/IntelFsp2Pkg.dec
+++ b/IntelFsp2Pkg/IntelFsp2Pkg.dec
@@ -68,6 +68,7 @@
# Guid define in FSP EAS=0D
gFspHeaderFileGuid =3D { 0x912740BE, 0x2284, 0x4734, =
{ 0xB9, 0x71, 0x84, 0xB0, 0x27, 0x35, 0x3F, 0x0C } }=0D
gFspReservedMemoryResourceHobGuid =3D { 0x69a79759, 0x1373, 0x4367, =
{ 0xa6, 0xc4, 0xc7, 0xf5, 0x9e, 0xfd, 0x98, 0x6e } }=0D
+ gFspNonVolatileStorageHob2Guid =3D { 0x4866788f, 0x6ba8, 0x47d8, =
{ 0x83, 0x06, 0xac, 0xf7, 0x7f, 0x55, 0x10, 0x46 } }=0D
gFspNonVolatileStorageHobGuid =3D { 0x721acf02, 0x4d77, 0x4c2a, =
{ 0xb3, 0xdc, 0x27, 0x0b, 0x7b, 0xa9, 0xe4, 0xb0 } }=0D
gFspBootLoaderTolumHobGuid =3D { 0x73ff4f56, 0xaa8e, 0x4451, =
{ 0xb3, 0x16, 0x36, 0x35, 0x36, 0x67, 0xad, 0x44 } } # FSP EAS v1.1=0D
=0D
--=20
2.28.0.windows.1


Re: [PATCH 1/1] UefiPayloadPkg: Add PCI root bridge info hob support for SBL

Guo Dong
 

Reviewed-by: Guo Dong <guo.dong@intel.com>

-----Original Message-----
From: Ma, Maurice <maurice.ma@intel.com>
Sent: Thursday, September 30, 2021 9:59 AM
To: devel@edk2.groups.io
Cc: Ma, Maurice <maurice.ma@intel.com>; Ni, Ray <ray.ni@intel.com>; Dong, Guo <guo.dong@intel.com>; You, Benjamin <benjamin.you@intel.com>
Subject: [edk2-devel] [PATCH 1/1] UefiPayloadPkg: Add PCI root bridge info hob support for SBL

Current UefiPayloadPkg can suport PCI root bridge info HOB provided by bootloader. For UniversalPayload, bootloader can directly provide this HOB for payload consumption. However, for legacy UEFI payload, it is required to migrate the HOB information from bootloader HOB space to UEFI payload HOB space. This patch added the missing part for the bootloader ParseLib in order to support both legacy and universal UEFI payload.

This patch was tested on Slim Bootloader with latest UEFI payload, and it worked as expected.

Cc: Ray Ni <ray.ni@intel.com>
Cc: Guo Dong <guo.dong@intel.com>
Cc: Benjamin You <benjamin.you@intel.com>
Signed-off-by: Maurice Ma <maurice.ma@intel.com>
---
UefiPayloadPkg/Include/Library/BlParseLib.h | 14 ++++++
.../Library/CbParseLib/CbParseLib.c | 16 +++++++
.../Library/SblParseLib/SblParseLib.c | 47 ++++++++++++++++++-
.../Library/SblParseLib/SblParseLib.inf | 1 +
.../UefiPayloadEntry/UefiPayloadEntry.c | 8 ++++
5 files changed, 84 insertions(+), 2 deletions(-)

diff --git a/UefiPayloadPkg/Include/Library/BlParseLib.h b/UefiPayloadPkg/Include/Library/BlParseLib.h
index 1244190d4e87..49eac3124818 100644
--- a/UefiPayloadPkg/Include/Library/BlParseLib.h
+++ b/UefiPayloadPkg/Include/Library/BlParseLib.h
@@ -116,4 +116,18 @@ ParseGfxDeviceInfo (
OUT EFI_PEI_GRAPHICS_DEVICE_INFO_HOB *GfxDeviceInfo ); +/**+ Parse and handle the misc info provided by bootloader++ @retval RETURN_SUCCESS The misc information was parsed successfully.+ @retval RETURN_NOT_FOUND Could not find required misc info.+ @retval RETURN_OUT_OF_RESOURCES Insufficant memory space.++**/+RETURN_STATUS+EFIAPI+ParseMiscInfo (+ VOID+ );+ #endifdiff --git a/UefiPayloadPkg/Library/CbParseLib/CbParseLib.c b/UefiPayloadPkg/Library/CbParseLib/CbParseLib.c
index 4f90687e407e..f81aa0f301d8 100644
--- a/UefiPayloadPkg/Library/CbParseLib/CbParseLib.c
+++ b/UefiPayloadPkg/Library/CbParseLib/CbParseLib.c
@@ -560,3 +560,19 @@ ParseGfxDeviceInfo (
return RETURN_NOT_FOUND; } +/**+ Parse and handle the misc info provided by bootloader++ @retval RETURN_SUCCESS The misc information was parsed successfully.+ @retval RETURN_NOT_FOUND Could not find required misc info.+ @retval RETURN_OUT_OF_RESOURCES Insufficant memory space.++**/+RETURN_STATUS+EFIAPI+ParseMiscInfo (+ VOID+ )+{+ return RETURN_SUCCESS;+}diff --git a/UefiPayloadPkg/Library/SblParseLib/SblParseLib.c b/UefiPayloadPkg/Library/SblParseLib/SblParseLib.c
index 7214fd87d20c..ccdcbfc07db9 100644
--- a/UefiPayloadPkg/Library/SblParseLib/SblParseLib.c
+++ b/UefiPayloadPkg/Library/SblParseLib/SblParseLib.c
@@ -1,7 +1,7 @@
/** @file This library will parse the Slim Bootloader to get required information. - Copyright (c) 2014 - 2019, Intel Corporation. All rights reserved.<BR>+ Copyright (c) 2014 - 2021, Intel Corporation. All rights reserved.<BR> SPDX-License-Identifier: BSD-2-Clause-Patent **/@@ -15,7 +15,7 @@
#include <Library/HobLib.h> #include <Library/BlParseLib.h> #include <IndustryStandard/Acpi.h>-+#include <UniversalPayload/PciRootBridges.h> /** This function retrieves the parameter base address from boot loader.@@ -221,3 +221,46 @@ ParseGfxDeviceInfo (
return RETURN_SUCCESS; } +/**+ Parse and handle the misc info provided by bootloader++ @retval RETURN_SUCCESS The misc information was parsed successfully.+ @retval RETURN_NOT_FOUND Could not find required misc info.+ @retval RETURN_OUT_OF_RESOURCES Insufficant memory space.++**/+RETURN_STATUS+EFIAPI+ParseMiscInfo (+ VOID+ )+{+ RETURN_STATUS Status;+ UNIVERSAL_PAYLOAD_PCI_ROOT_BRIDGES *BlRootBridgesHob;+ UNIVERSAL_PAYLOAD_PCI_ROOT_BRIDGES *PldRootBridgesHob;++ Status = RETURN_NOT_FOUND;+ BlRootBridgesHob = (UNIVERSAL_PAYLOAD_PCI_ROOT_BRIDGES *) GetGuidHobDataFromSbl (+ &gUniversalPayloadPciRootBridgeInfoGuid+ );+ if (BlRootBridgesHob != NULL) {+ //+ // Migrate bootloader root bridge info hob from bootloader to payload.+ //+ PldRootBridgesHob = BuildGuidHob (+ &gUniversalPayloadPciRootBridgeInfoGuid,+ BlRootBridgesHob->Header.Length+ );+ ASSERT (PldRootBridgesHob != NULL);+ if (PldRootBridgesHob != NULL) {+ CopyMem (PldRootBridgesHob, BlRootBridgesHob, BlRootBridgesHob->Header.Length);+ DEBUG ((DEBUG_INFO, "Create PCI root bridge info guid hob\n"));+ Status = RETURN_SUCCESS;+ } else {+ Status = RETURN_OUT_OF_RESOURCES;+ }+ }++ return Status;+}+diff --git a/UefiPayloadPkg/Library/SblParseLib/SblParseLib.inf b/UefiPayloadPkg/Library/SblParseLib/SblParseLib.inf
index 665a5a8adcef..535cca58a63c 100644
--- a/UefiPayloadPkg/Library/SblParseLib/SblParseLib.inf
+++ b/UefiPayloadPkg/Library/SblParseLib/SblParseLib.inf
@@ -41,6 +41,7 @@
gLoaderMemoryMapInfoGuid gEfiGraphicsInfoHobGuid gEfiGraphicsDeviceInfoHobGuid+ gUniversalPayloadPciRootBridgeInfoGuid [Pcd] gUefiPayloadPkgTokenSpaceGuid.PcdBootloaderParameterdiff --git a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
index f2ac3d2c6925..5a1e5786687a 100644
--- a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
+++ b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
@@ -321,6 +321,14 @@ BuildHobFromBl (
return Status; } + //+ // Parse the misc info provided by bootloader+ //+ Status = ParseMiscInfo ();+ if (EFI_ERROR (Status)) {+ DEBUG ((DEBUG_WARN, "Error when parsing misc info, Status = %r\n", Status));+ }+ // // Parse platform specific information. //--
2.29.2.windows.2


Re: [PATCH V8 3/3] OvmfPkg: Enable TDX in ResetVector

Brijesh Singh
 

[AMD Official Use Only]


Yes, I will try to make it work for the unified Metadata. Let's do it indepent of SNP and TDX series. You can pick the generic patch from my series and add the additional fields we need for the TDX and submit it.


From: Xu, Min M <min.m.xu@...>
Sent: Thursday, September 30, 2021 12:31:56 AM
To: devel@edk2.groups.io <devel@edk2.groups.io>; Singh, Brijesh <brijesh.singh@...>; kraxel@... <kraxel@...>; Yao, Jiewen <jiewen.yao@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>; Justen, Jordan L <jordan.l.justen@...>; Erdem Aktas <erdemaktas@...>; James Bottomley <jejb@...>; Lendacky, Thomas <Thomas.Lendacky@...>
Subject: RE: [edk2-devel] [PATCH V8 3/3] OvmfPkg: Enable TDX in ResetVector
 

[AMD Official Use Only]


Hi, Brijesh

In the current discussion there are 2 options for the metadata, a unified Metadata and 2 separate Metadata (SEV and TDX metadata).

My understanding to your last mail is that you’re going to use the unified Metadata option, right?

 

As to the offset of metadata, absolute offset is a good idea. I will update it in my next version.

 

Thanks!

Min

From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Brijesh Singh via groups.io
Sent: Tuesday, September 28, 2021 11:24 PM
To: Xu, Min M <min.m.xu@...>; devel@edk2.groups.io; kraxel@...
Cc: Yao, Jiewen <jiewen.yao@...>; Ard Biesheuvel <ardb+tianocore@...>; Justen, Jordan L <jordan.l.justen@...>; Erdem Aktas <erdemaktas@...>; James Bottomley <jejb@...>; Lendacky, Thomas <Thomas.Lendacky@...>
Subject: Re: [edk2-devel] [PATCH V8 3/3] OvmfPkg: Enable TDX in ResetVector

 

[AMD Official Use Only]

 

May I ask to use the OvmfMetadata instead of the of TdxMetadata for the Guided structure name label (same as what I did in SNP series patch #4). If you can send the metadata introduction as a patch separately then add the TDX descriptor in TDX series. I can try to make it work for the SNP series and add SNP specific descriptors. Additionally, I think you want to provide an absolute offset for the start of the metadata instead relative value so that VMM can very easily reach to the start of metadata. 

e.g

 

+OvmfMetadataOffsetStart:
+  DD      (fourGigabytes - OvmfMetadataGuid - 16)
+  DW      OvmfMetadataOffsetEnd - OvmfMetadataOffsetStart
+  DB      0x35, 0x65, 0x7a, 0xe4, 0x4a, 0x98, 0x98, 0x47
+  DB      0x86, 0x5e, 0x46, 0x85, 0xa7, 0xbf, 0x8e, 0xc2
+OvmfMetadataOffsetEnd:

 

For SNP series, I will 3 section types #1 CPUID, # Secrets, and #3 SEC_MEM and will probably add a total of 3 more descriptors. 

 


From: Xu, Min M <min.m.xu@...>
Sent: Tuesday, September 28, 2021 2:35 AM
To: devel@edk2.groups.io <devel@edk2.groups.io>; kraxel@... <kraxel@...>
Cc: Yao, Jiewen <jiewen.yao@...>; Ard Biesheuvel <ardb+tianocore@...>; Justen, Jordan L <jordan.l.justen@...>; Singh, Brijesh <brijesh.singh@...>; Erdem Aktas <erdemaktas@...>; James Bottomley <jejb@...>; Lendacky, Thomas <Thomas.Lendacky@...>
Subject: RE: [edk2-devel] [PATCH V8 3/3] OvmfPkg: Enable TDX in ResetVector

 

On September 28, 2021 12:43 PM, Gerd Hoffmann wrote:
>   Hi,
>
> > > Can you move the metadata changes to a separate patch please?
> > Yes, the metadata changes will be in a separate patch in the next version.
>
> Can you also add a comment block documenting the format?  Not only those
> parts which are used for TDVF, but everything?  The description in tdx-virtual-
> firmware-design-guide-rev-1.pdf seems to be incomplete, specifically the
> option to use the table for TD memory allocation (as mentioned by Jiewen) is
> not covered.  And possibly there is more which is missing ...
Sure. I will add the comment in IntelTdxMetadata.asm to describe the format of Tdx Metadata.
Here is the PR I would send as the next version. https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ftianocore%2Fedk2%2Fpull%2F2018&amp;data=04%7C01%7Cbrijesh.singh%40amd.com%7Cf49ea5bc7d79474e572108d982529cbd%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637684113590273535%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=bGOxYMIKtHYKhcfk0Wt4qoIgiz3b9DM%2FAD%2Fui3ByVrU%3D&amp;reserved=0
You can have a preliminary review if you want.
>
> thanks,
>   Gerd
>
>
>
>
>


Cancelled Event: TianoCore Design Meeting - APAC/NAMO - Friday, October 1, 2021 #cal-cancelled

devel@edk2.groups.io Calendar <noreply@...>
 

Cancelled: TianoCore Design Meeting - APAC/NAMO

This event has been cancelled.

When:
Friday, October 1, 2021
9:30am to 10:30am
(UTC+08:00) Asia/Shanghai

Where:
Microsoft Teams

Organizer: Ray Ni ray.ni@...

Description:

TOPIC

  1. NA

For more info, see here: https://www.tianocore.org/design-meeting/


Microsoft Teams meeting

Join on your computer or mobile app

Click here to join the meeting

Join with a video conferencing device

teams@...

Video Conference ID: 119 715 416 0

Alternate VTC dialing instructions

Learn More | Meeting options


Re: [PATCH 1/1] UefiPayloadPkg: Add PCI root bridge info hob support for SBL

Ma, Maurice
 

Sorry for the duplicated msg. Please ignore the duplicated one.

Thanks,
-Maurice

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ma,
Maurice
Sent: Thursday, September 30, 2021 9:59
To: devel@edk2.groups.io
Cc: Ma, Maurice <maurice.ma@intel.com>; Ni, Ray <ray.ni@intel.com>; Dong,
Guo <guo.dong@intel.com>; You, Benjamin <benjamin.you@intel.com>
Subject: [edk2-devel] [PATCH 1/1] UefiPayloadPkg: Add PCI root bridge info
hob support for SBL

Current UefiPayloadPkg can suport PCI root bridge info HOB provided by
bootloader. For UniversalPayload, bootloader can directly provide this HOB
for payload consumption. However, for legacy UEFI payload, it is required to
migrate the HOB information from bootloader HOB space to UEFI payload
HOB space. This patch added the missing part for the bootloader ParseLib in
order to support both legacy and universal UEFI payload.

This patch was tested on Slim Bootloader with latest UEFI payload, and it
worked as expected.

Cc: Ray Ni <ray.ni@intel.com>
Cc: Guo Dong <guo.dong@intel.com>
Cc: Benjamin You <benjamin.you@intel.com>
Signed-off-by: Maurice Ma <maurice.ma@intel.com>
---
UefiPayloadPkg/Include/Library/BlParseLib.h | 14 ++++++
.../Library/CbParseLib/CbParseLib.c | 16 +++++++
.../Library/SblParseLib/SblParseLib.c | 47 ++++++++++++++++++-
.../Library/SblParseLib/SblParseLib.inf | 1 +
.../UefiPayloadEntry/UefiPayloadEntry.c | 8 ++++
5 files changed, 84 insertions(+), 2 deletions(-)

diff --git a/UefiPayloadPkg/Include/Library/BlParseLib.h
b/UefiPayloadPkg/Include/Library/BlParseLib.h
index 1244190d4e87..49eac3124818 100644
--- a/UefiPayloadPkg/Include/Library/BlParseLib.h
+++ b/UefiPayloadPkg/Include/Library/BlParseLib.h
@@ -116,4 +116,18 @@ ParseGfxDeviceInfo (
OUT EFI_PEI_GRAPHICS_DEVICE_INFO_HOB *GfxDeviceInfo ); +/**+
Parse and handle the misc info provided by bootloader++ @retval
RETURN_SUCCESS The misc information was parsed successfully.+
@retval RETURN_NOT_FOUND Could not find required misc info.+
@retval RETURN_OUT_OF_RESOURCES Insufficant memory
space.++**/+RETURN_STATUS+EFIAPI+ParseMiscInfo (+ VOID+ );+
#endifdiff --git a/UefiPayloadPkg/Library/CbParseLib/CbParseLib.c
b/UefiPayloadPkg/Library/CbParseLib/CbParseLib.c
index 4f90687e407e..f81aa0f301d8 100644
--- a/UefiPayloadPkg/Library/CbParseLib/CbParseLib.c
+++ b/UefiPayloadPkg/Library/CbParseLib/CbParseLib.c
@@ -560,3 +560,19 @@ ParseGfxDeviceInfo (
return RETURN_NOT_FOUND; } +/**+ Parse and handle the misc info
provided by bootloader++ @retval RETURN_SUCCESS The misc
information was parsed successfully.+ @retval RETURN_NOT_FOUND
Could not find required misc info.+ @retval RETURN_OUT_OF_RESOURCES
Insufficant memory space.++**/+RETURN_STATUS+EFIAPI+ParseMiscInfo
(+ VOID+ )+{+ return RETURN_SUCCESS;+}diff --git
a/UefiPayloadPkg/Library/SblParseLib/SblParseLib.c
b/UefiPayloadPkg/Library/SblParseLib/SblParseLib.c
index 7214fd87d20c..ccdcbfc07db9 100644
--- a/UefiPayloadPkg/Library/SblParseLib/SblParseLib.c
+++ b/UefiPayloadPkg/Library/SblParseLib/SblParseLib.c
@@ -1,7 +1,7 @@
/** @file This library will parse the Slim Bootloader to get required
information. - Copyright (c) 2014 - 2019, Intel Corporation. All rights
reserved.<BR>+ Copyright (c) 2014 - 2021, Intel Corporation. All rights
reserved.<BR> SPDX-License-Identifier: BSD-2-Clause-Patent **/@@ -15,7
+15,7 @@
#include <Library/HobLib.h> #include <Library/BlParseLib.h> #include
<IndustryStandard/Acpi.h>-+#include <UniversalPayload/PciRootBridges.h>
/** This function retrieves the parameter base address from boot
loader.@@ -221,3 +221,46 @@ ParseGfxDeviceInfo (
return RETURN_SUCCESS; } +/**+ Parse and handle the misc info provided
by bootloader++ @retval RETURN_SUCCESS The misc information was
parsed successfully.+ @retval RETURN_NOT_FOUND Could not find
required misc info.+ @retval RETURN_OUT_OF_RESOURCES Insufficant
memory space.++**/+RETURN_STATUS+EFIAPI+ParseMiscInfo (+
VOID+ )+{+ RETURN_STATUS Status;+
UNIVERSAL_PAYLOAD_PCI_ROOT_BRIDGES *BlRootBridgesHob;+
UNIVERSAL_PAYLOAD_PCI_ROOT_BRIDGES *PldRootBridgesHob;++
Status = RETURN_NOT_FOUND;+ BlRootBridgesHob =
(UNIVERSAL_PAYLOAD_PCI_ROOT_BRIDGES *) GetGuidHobDataFromSbl (+
&gUniversalPayloadPciRootBridgeInfoGuid+ );+ if
(BlRootBridgesHob != NULL) {+ //+ // Migrate bootloader root bridge info
hob from bootloader to payload.+ //+ PldRootBridgesHob = BuildGuidHob
(+ &gUniversalPayloadPciRootBridgeInfoGuid,+
BlRootBridgesHob->Header.Length+ );+ ASSERT
(PldRootBridgesHob != NULL);+ if (PldRootBridgesHob != NULL) {+
CopyMem (PldRootBridgesHob, BlRootBridgesHob, BlRootBridgesHob-
Header.Length);+ DEBUG ((DEBUG_INFO, "Create PCI root bridge info
guid hob\n"));+ Status = RETURN_SUCCESS;+ } else {+ Status =
RETURN_OUT_OF_RESOURCES;+ }+ }++ return Status;+}+diff --git
a/UefiPayloadPkg/Library/SblParseLib/SblParseLib.inf
b/UefiPayloadPkg/Library/SblParseLib/SblParseLib.inf
index 665a5a8adcef..535cca58a63c 100644
--- a/UefiPayloadPkg/Library/SblParseLib/SblParseLib.inf
+++ b/UefiPayloadPkg/Library/SblParseLib/SblParseLib.inf
@@ -41,6 +41,7 @@
gLoaderMemoryMapInfoGuid gEfiGraphicsInfoHobGuid
gEfiGraphicsDeviceInfoHobGuid+ gUniversalPayloadPciRootBridgeInfoGuid
[Pcd] gUefiPayloadPkgTokenSpaceGuid.PcdBootloaderParameterdiff --git
a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
index f2ac3d2c6925..5a1e5786687a 100644
--- a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
+++ b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
@@ -321,6 +321,14 @@ BuildHobFromBl (
return Status; } + //+ // Parse the misc info provided by bootloader+ //+
Status = ParseMiscInfo ();+ if (EFI_ERROR (Status)) {+ DEBUG
((DEBUG_WARN, "Error when parsing misc info, Status = %r\n", Status));+ }+
// // Parse platform specific information. //--
2.29.2.windows.2



-=-=-=-=-=-=
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#81353): https://edk2.groups.io/g/devel/message/81353
Mute This Topic: https://groups.io/mt/85978857/1773972
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [maurice.ma@intel.com]
-=-=-=-=-=-=


[PATCH 1/1] UefiPayloadPkg: Add PCI root bridge info hob support for SBL

Ma, Maurice
 

Current UefiPayloadPkg can suport PCI root bridge info HOB
provided by bootloader. For UniversalPayload, bootloader can
directly provide this HOB for payload consumption. However,
for legacy UEFI payload, it is required to migrate the HOB
information from bootloader HOB space to UEFI payload HOB
space. This patch added the missing part for the bootloader
ParseLib in order to support both legacy and universal UEFI
payload.

This patch was tested on Slim Bootloader with latest UEFI
payload, and it worked as expected.

Cc: Ray Ni <ray.ni@intel.com>
Cc: Guo Dong <guo.dong@intel.com>
Cc: Benjamin You <benjamin.you@intel.com>
Signed-off-by: Maurice Ma <maurice.ma@intel.com>
---
UefiPayloadPkg/Include/Library/BlParseLib.h | 14 ++++++
.../Library/CbParseLib/CbParseLib.c | 16 +++++++
.../Library/SblParseLib/SblParseLib.c | 47 ++++++++++++++++++-
.../Library/SblParseLib/SblParseLib.inf | 1 +
.../UefiPayloadEntry/UefiPayloadEntry.c | 8 ++++
5 files changed, 84 insertions(+), 2 deletions(-)

diff --git a/UefiPayloadPkg/Include/Library/BlParseLib.h b/UefiPayloadPkg/I=
nclude/Library/BlParseLib.h
index 1244190d4e87..49eac3124818 100644
--- a/UefiPayloadPkg/Include/Library/BlParseLib.h
+++ b/UefiPayloadPkg/Include/Library/BlParseLib.h
@@ -116,4 +116,18 @@ ParseGfxDeviceInfo (
OUT EFI_PEI_GRAPHICS_DEVICE_INFO_HOB *GfxDeviceInfo=0D
);=0D
=0D
+/**=0D
+ Parse and handle the misc info provided by bootloader=0D
+=0D
+ @retval RETURN_SUCCESS The misc information was parsed success=
fully.=0D
+ @retval RETURN_NOT_FOUND Could not find required misc info.=0D
+ @retval RETURN_OUT_OF_RESOURCES Insufficant memory space.=0D
+=0D
+**/=0D
+RETURN_STATUS=0D
+EFIAPI=0D
+ParseMiscInfo (=0D
+ VOID=0D
+ );=0D
+=0D
#endif=0D
diff --git a/UefiPayloadPkg/Library/CbParseLib/CbParseLib.c b/UefiPayloadPk=
g/Library/CbParseLib/CbParseLib.c
index 4f90687e407e..f81aa0f301d8 100644
--- a/UefiPayloadPkg/Library/CbParseLib/CbParseLib.c
+++ b/UefiPayloadPkg/Library/CbParseLib/CbParseLib.c
@@ -560,3 +560,19 @@ ParseGfxDeviceInfo (
return RETURN_NOT_FOUND;=0D
}=0D
=0D
+/**=0D
+ Parse and handle the misc info provided by bootloader=0D
+=0D
+ @retval RETURN_SUCCESS The misc information was parsed success=
fully.=0D
+ @retval RETURN_NOT_FOUND Could not find required misc info.=0D
+ @retval RETURN_OUT_OF_RESOURCES Insufficant memory space.=0D
+=0D
+**/=0D
+RETURN_STATUS=0D
+EFIAPI=0D
+ParseMiscInfo (=0D
+ VOID=0D
+ )=0D
+{=0D
+ return RETURN_SUCCESS;=0D
+}=0D
diff --git a/UefiPayloadPkg/Library/SblParseLib/SblParseLib.c b/UefiPayload=
Pkg/Library/SblParseLib/SblParseLib.c
index 7214fd87d20c..ccdcbfc07db9 100644
--- a/UefiPayloadPkg/Library/SblParseLib/SblParseLib.c
+++ b/UefiPayloadPkg/Library/SblParseLib/SblParseLib.c
@@ -1,7 +1,7 @@
/** @file=0D
This library will parse the Slim Bootloader to get required information.=
=0D
=0D
- Copyright (c) 2014 - 2019, Intel Corporation. All rights reserved.<BR>=0D
+ Copyright (c) 2014 - 2021, Intel Corporation. All rights reserved.<BR>=0D
SPDX-License-Identifier: BSD-2-Clause-Patent=0D
=0D
**/=0D
@@ -15,7 +15,7 @@
#include <Library/HobLib.h>=0D
#include <Library/BlParseLib.h>=0D
#include <IndustryStandard/Acpi.h>=0D
-=0D
+#include <UniversalPayload/PciRootBridges.h>=0D
=0D
/**=0D
This function retrieves the parameter base address from boot loader.=0D
@@ -221,3 +221,46 @@ ParseGfxDeviceInfo (
return RETURN_SUCCESS;=0D
}=0D
=0D
+/**=0D
+ Parse and handle the misc info provided by bootloader=0D
+=0D
+ @retval RETURN_SUCCESS The misc information was parsed success=
fully.=0D
+ @retval RETURN_NOT_FOUND Could not find required misc info.=0D
+ @retval RETURN_OUT_OF_RESOURCES Insufficant memory space.=0D
+=0D
+**/=0D
+RETURN_STATUS=0D
+EFIAPI=0D
+ParseMiscInfo (=0D
+ VOID=0D
+ )=0D
+{=0D
+ RETURN_STATUS Status;=0D
+ UNIVERSAL_PAYLOAD_PCI_ROOT_BRIDGES *BlRootBridgesHob;=0D
+ UNIVERSAL_PAYLOAD_PCI_ROOT_BRIDGES *PldRootBridgesHob;=0D
+=0D
+ Status =3D RETURN_NOT_FOUND;=0D
+ BlRootBridgesHob =3D (UNIVERSAL_PAYLOAD_PCI_ROOT_BRIDGES *) GetGuidHobDa=
taFromSbl (=0D
+ &gUniversalPayloadPciRootBridgeInfoGuid=0D
+ );=0D
+ if (BlRootBridgesHob !=3D NULL) {=0D
+ //=0D
+ // Migrate bootloader root bridge info hob from bootloader to payload.=
=0D
+ //=0D
+ PldRootBridgesHob =3D BuildGuidHob (=0D
+ &gUniversalPayloadPciRootBridgeInfoG=
uid,=0D
+ BlRootBridgesHob->Header.Length=0D
+ );=0D
+ ASSERT (PldRootBridgesHob !=3D NULL);=0D
+ if (PldRootBridgesHob !=3D NULL) {=0D
+ CopyMem (PldRootBridgesHob, BlRootBridgesHob, BlRootBridgesHob->Head=
er.Length);=0D
+ DEBUG ((DEBUG_INFO, "Create PCI root bridge info guid hob\n"));=0D
+ Status =3D RETURN_SUCCESS;=0D
+ } else {=0D
+ Status =3D RETURN_OUT_OF_RESOURCES;=0D
+ }=0D
+ }=0D
+=0D
+ return Status;=0D
+}=0D
+=0D
diff --git a/UefiPayloadPkg/Library/SblParseLib/SblParseLib.inf b/UefiPaylo=
adPkg/Library/SblParseLib/SblParseLib.inf
index 665a5a8adcef..535cca58a63c 100644
--- a/UefiPayloadPkg/Library/SblParseLib/SblParseLib.inf
+++ b/UefiPayloadPkg/Library/SblParseLib/SblParseLib.inf
@@ -41,6 +41,7 @@
gLoaderMemoryMapInfoGuid=0D
gEfiGraphicsInfoHobGuid=0D
gEfiGraphicsDeviceInfoHobGuid=0D
+ gUniversalPayloadPciRootBridgeInfoGuid=0D
=0D
[Pcd]=0D
gUefiPayloadPkgTokenSpaceGuid.PcdBootloaderParameter=0D
diff --git a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c b/UefiPaylo=
adPkg/UefiPayloadEntry/UefiPayloadEntry.c
index f2ac3d2c6925..5a1e5786687a 100644
--- a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
+++ b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
@@ -321,6 +321,14 @@ BuildHobFromBl (
return Status;=0D
}=0D
=0D
+ //=0D
+ // Parse the misc info provided by bootloader=0D
+ //=0D
+ Status =3D ParseMiscInfo ();=0D
+ if (EFI_ERROR (Status)) {=0D
+ DEBUG ((DEBUG_WARN, "Error when parsing misc info, Status =3D %r\n", S=
tatus));=0D
+ }=0D
+=0D
//=0D
// Parse platform specific information.=0D
//=0D
--=20
2.29.2.windows.2


Re: error C0DE: Tools code failure

Andrew Fish
 

Are you using Python 2.7? Try using Python 3.*

It looks like array.fromstring() got renamed to array.frombytes() in Python 3.2, so I’m guessing you are using Python 2.7?

Thanks,

Andrew Fish

On Sep 30, 2021, at 1:19 AM, Kumar Jain, Piyush <piyush.kumar.jain@...> wrote:

Traceback (most recent call last):
  File "C:\Users\pkumarj1\IFWI3\rocketlake\Edk2\BaseTools\Source\Python\GenFds\GenFds.py", line 371, in GenFdsApi
    GenFds.GenFd('', FdfParserObj, BuildWorkSpace, ArchList)
  File "C:\Users\pkumarj1\IFWI3\rocketlake\Edk2\BaseTools\Source\Python\GenFds\GenFds.py", line 510, in GenFd
    FdObj.GenFd()
  File "C:\Users\pkumarj1\IFWI3\rocketlake\Edk2\BaseTools\Source\Python\GenFds\Fd.py", line 131, in GenFd
    RegionObj.AddToBuffer (FdBuffer, self.BaseAddress, self.BlockSizeList, self.ErasePolarity, GenFdsGlobalVariable.ImageBinDict, self.DefineVarDict, Flag=Flag)
  File "C:\Users\pkumarj1\IFWI3\rocketlake\Edk2\BaseTools\Source\Python\GenFds\Region.py", line 134, in AddToBuffer
    FvObj.AddToBuffer(FvBuffer, FvBaseAddress, BlockSize, BlockNum, ErasePolarity, Flag=Flag)
  File "C:\Users\pkumarj1\IFWI3\rocketlake\Edk2\BaseTools\Source\Python\GenFds\Fv.py", line 127, in AddToBuffer
    FileName = FfsFile.GenFfs(MacroDict, FvParentAddr=BaseAddress, IsMakefile=Flag, FvName=self.UiFvName)
  File "C:\Users\pkumarj1\IFWI3\rocketlake\Edk2\BaseTools\Source\Python\GenFds\FfsFileStatement.py", line 156, in GenFfs
    sectList, align = section.GenSection(OutputDir, self.NameGuid, SecIndex, self.KeyStringList, None, Dict)
  File "C:\Users\pkumarj1\IFWI3\rocketlake\Edk2\BaseTools\Source\Python\GenFds\CompressSection.py", line 67, in GenSection
    ReturnSectList, AlignValue = Sect.GenSection(OutputPath, ModuleName, SecIndex, KeyStringList, FfsInf, Dict, IsMakefile=IsMakefile)
  File "C:\Users\pkumarj1\IFWI3\rocketlake\Edk2\BaseTools\Source\Python\GenFds\UiSection.py", line 70, in GenSection
    GenFdsGlobalVariable.GenerateSection(OutputFile, None, 'EFI_SECTION_USER_INTERFACE', Ui=NameString, IsMakefile=IsMakefile)
  File "C:\Users\pkumarj1\IFWI3\rocketlake\Edk2\BaseTools\Source\Python\GenFds\GenFdsGlobalVariable.py", line 466, in GenerateSection
    SectionData.fromstring(Ui.encode("utf_16_le"))
AttributeError: 'array.array' object has no attribute 'fromstring'


[PATCH edk2-platforms v1 2/2] Platform/ARM/Juno: Add RNG support using FW-TRNG interface

Sami Mujawar
 

TF-A for Juno has been updated to implement the Arm FW-TRNG interface
that can be used to access entropy from the TRNG hardware on Juno.

Similarly, the EFI_RNG_PROTOCOL in RngDxe has been updated to add
EFI_RNG_ALGORITHM_RAW support using the FW-TRNG interface.

Therefore, enable the EFI_RNG_PROTOCOL for Juno platform.

Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
Platform/ARM/JunoPkg/ArmJuno.dsc | 10 +++++++++-
Platform/ARM/JunoPkg/ArmJuno.fdf | 7 ++++++-
2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/Platform/ARM/JunoPkg/ArmJuno.dsc b/Platform/ARM/JunoPkg/ArmJuno.dsc
index fdfc8cd9e20f57e4d56fc7e2712bdc9afbc7148e..47a3fab333cdc3896df789407267e2fa81cf8055 100644
--- a/Platform/ARM/JunoPkg/ArmJuno.dsc
+++ b/Platform/ARM/JunoPkg/ArmJuno.dsc
@@ -1,5 +1,5 @@
#
-# Copyright (c) 2013-2018, ARM Limited. All rights reserved.
+# Copyright (c) 2013-2021, ARM Limited. All rights reserved.
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
@@ -55,6 +55,9 @@ [LibraryClasses.common]
LcdHwLib|ArmPlatformPkg/Library/HdLcd/HdLcd.inf
!endif

+ TrngLib|ArmPkg/Library/ArmFwTrngLib/ArmFwTrngLib.inf
+ ArmHvcLib|ArmPkg/Library/ArmHvcLib/ArmHvcLib.inf
+
[LibraryClasses.common.SEC]
PrePiLib|EmbeddedPkg/Library/PrePiLib/PrePiLib.inf
ExtractGuidedSectionLib|EmbeddedPkg/Library/PrePiExtractGuidedSectionLib/PrePiExtractGuidedSectionLib.inf
@@ -383,6 +386,11 @@ [Components.common]
# SCMI Driver
ArmPkg/Drivers/ArmScmiDxe/ArmScmiDxe.inf

+ #
+ # Rng Support
+ #
+ SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf
+
[Components.AARCH64]
#
# EBC
diff --git a/Platform/ARM/JunoPkg/ArmJuno.fdf b/Platform/ARM/JunoPkg/ArmJuno.fdf
index f70d30c6a9d9d6eb73087dc673f0c9287d23d666..f4b38da8f9e634743395fd6a2a8d30360b2710a7 100644
--- a/Platform/ARM/JunoPkg/ArmJuno.fdf
+++ b/Platform/ARM/JunoPkg/ArmJuno.fdf
@@ -1,5 +1,5 @@
#
-# Copyright (c) 2013-2019, Arm Limited. All rights reserved.<BR>
+# Copyright (c) 2013-2021, Arm Limited. All rights reserved.<BR>
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
@@ -237,6 +237,11 @@ [FV.FvMain]
# SCMI Driver
INF ArmPkg/Drivers/ArmScmiDxe/ArmScmiDxe.inf

+ #
+ # Rng Support
+ #
+ INF SecurityPkg/RandomNumberGenerator/RngDxe/RngDxe.inf
+
!if $(ARCH) == AARCH64
#
# EBC
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


[PATCH edk2-platforms v1 0/2] Enable RNG support for Juno

Sami Mujawar
 

The Arm True Random Number Generator Firmware, Interface 1.0, specification
defines an interface between an Operating System (OS) executing at EL1 and
Firmware (FW) exposing a conditioned entropy source that is provided by a
TRNG back end.

The edk2 patch series at:
https://edk2.groups.io/g/devel/topic/patch_v1_0_9_add_raw/85977024
adds RawAlgorithm support to RngDxe for Arm architecture using the Arm
FW-TRNG interface:

Ref: Bugzilla: 3668 (https://bugzilla.tianocore.org/show_bug.cgi?id=3668)

FW-TRNG interface service is already supported in TF-A for Juno. Therefore,
enable RNG support for Juno. Also, flink the NULL instance of TRNG library
for platforms that do not implement the FW-TRNG interface.

The changes can be seen at:
https://github.com/samimujawar/edk2-platforms/tree/1829_arm_fw_trng_v1

Sami Mujawar (2):
Platform/ARM: Add NULL instance of TRNG lib to Libraries
Platform/ARM/Juno: Add RNG support using FW-TRNG interface

Platform/ARM/JunoPkg/ArmJuno.dsc | 10 +++++++++-
Platform/ARM/JunoPkg/ArmJuno.fdf | 7 ++++++-
Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc | 3 ++-
3 files changed, 17 insertions(+), 3 deletions(-)

--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


[PATCH edk2-platforms v1 1/2] Platform/ARM: Add NULL instance of TRNG lib to Libraries

Sami Mujawar
 

The EFI_RNG_PROTOCOL published by RngDxe has been updated to
implement the EFI_RNG_ALGORITHM_RAW using the Arm FW-TRNG
interface to provide access to entropy.

The TRNG support is implemented by the Arm FW-TRNG library
if supported by the platform. If the platform does not support
the FW-TRNG interface a NULL instance of the TRNG library is
provided.

Therefore, include the reference to the NULL instance of the
TRNG library as default. Platforms implementing the FW-TRNG
interface can override this in their respective platforms
workspaces.

Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc b/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
index d6f31ecda42f65ecc44235c195976da8f18e9b8b..6106677b58e8212de565609e8138b3d91ee3e108 100644
--- a/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
+++ b/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
@@ -1,5 +1,5 @@
#
-# Copyright (c) 2011-2020, Arm Limited. All rights reserved.
+# Copyright (c) 2011-2021, Arm Limited. All rights reserved.
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
@@ -142,6 +142,7 @@ [LibraryClasses.common]
IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
RngLib|MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
+ TrngLib|MdePkg/Library/BaseTrngLibNull/BaseTrngLibNull.inf
VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf
VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


[PATCH v1 2/9] ArmPkg: PCD to select conduit for monitor calls

Sami Mujawar
 

Bugzilla: 3668 (https://bugzilla.tianocore.org/show_bug.cgi?id=3668)

Define a PCD 'PcdMonitorConduitHvc' to select the conduit to use for
monitor calls. PcdMonitorConduitHvc is defined as FALSE by default,
meaning the SMC conduit is enabled as default.

Adding PcdMonitorConduitHvc allows selection of HVC conduit to be used
by virtual firmware implementations.

Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
ArmPkg/ArmPkg.dec | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec
index 6ed51edd0340605639d4b34f77bdb59dca1827be..395d64d8aaeb772a6e094b3d90cd3920b844e372 100644
--- a/ArmPkg/ArmPkg.dec
+++ b/ArmPkg/ArmPkg.dec
@@ -131,6 +131,11 @@ [PcdsFeatureFlag.common]
# Define if the GICv3 controller should use the GICv2 legacy
gArmTokenSpaceGuid.PcdArmGicV3WithV2Legacy|FALSE|BOOLEAN|0x00000042

+ ## Define the conduit to use for monitor calls.
+ # Default PcdMonitorConduitHvc = FALSE, conduit = SMC
+ # If PcdMonitorConduitHvc = TRUE, conduit = HVC
+ gArmTokenSpaceGuid.PcdMonitorConduitHvc|FALSE|BOOLEAN|0x00000047
+
[PcdsFeatureFlag.ARM]
# Whether to map normal memory as non-shareable. FALSE is the safe choice, but
# TRUE may be appropriate to fix performance problems if you don't care about
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'

4701 - 4720 of 85992