[PATCH v3 0/3] Add Memory32Fixed and AmlCodeGenMethodRetInteger functions

[PATCH v3 0/3] Add Memory32Fixed and AmlCodeGenMethodRetInteger functions

[Rebecca Cran]

Add functions to generate code for the Memory32Fixed ASL macro and a
method returning an Integer.

Remove a redundant cast from AmlCodeGenReturn.


Changes from v2 to v3:

o Rename function to AmlCodeGenRdMemory32Fixed.
o Use define ACPI_32_BIT_FIXED_MEMORY_RANGE_DESCRIPTOR.

Rebecca Cran (3):
  DynamicTablesPkg: Add Memory32Fixed function
  DynamicTablesPkg: Remove redundant cast in AmlCodeGenReturn
  DynamicTablesPkg: Add AmlCodeGenMethodRetInteger function

 DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h                        |  80 ++++++++++
 DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c             | 158 +++++++++++++++++++-
 DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.c |  57 +++++++
 3 files changed, 294 insertions(+), 1 deletion(-)

-- 
2.31.1

[PATCH v3 1/3] DynamicTablesPkg: Add Memory32Fixed function

[Rebecca Cran]

Add a Memory32Fixed function to generate code for the corresponding
Memory32Fixed macro in AML.

Signed-off-by: Rebecca Cran <quic_rcran@quicinc.com>
Reviewed-by: Pierre Gondois <pierre.gondois@arm.com>
---
 DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h                        | 33 ++++++++++++
 DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.c | 57 ++++++++++++++++++++
 2 files changed, 90 insertions(+)

diff --git a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
index af18bf8e4871..80d05f74ee69 100644
--- a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
+++ b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
@@ -592,6 +592,39 @@ AmlCodeGenRdDWordMemory (
   OUT       AML_DATA_NODE_HANDLE    *NewRdNode  OPTIONAL
   );
 
+/** Code generation for the "Memory32Fixed ()" ASL macro.
+
+  The Resource Data effectively created is a 32-bit Memory Resource
+  Data. Cf ACPI 6.4:
+   - s19.6.83 "Memory Resource Descriptor Macro".
+   - s19.2.8 "Memory32FixedTerm".
+
+  See ACPI 6.4 spec, s19.2.8 for more.
+
+  @param [in]  IsReadWrite          ReadAndWrite parameter.
+  @param [in]  Address              AddressBase parameter.
+  @param [in]  RangeLength          Range length.
+  @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] NewMemNode           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
+AmlCodeGenRdMemory32Fixed (
+  BOOLEAN                 IsReadWrite,
+  UINT32                  Address,
+  UINT32                  RangeLength,
+  AML_OBJECT_NODE_HANDLE  NameOpNode,
+  AML_DATA_NODE_HANDLE    *NewMemNode
+  );
+
 /** Code generation for the "WordBusNumber ()" ASL function.
 
   The Resource Data effectively created is a Word Address Space Resource
diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.c b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.c
index 40d8c2b07ae3..fb18c5a77e2f 100644
--- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.c
+++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.c
@@ -609,6 +609,63 @@ AmlCodeGenRdDWordMemory (
            );
 }
 
+/** Code generation for the "Memory32Fixed ()" ASL macro.
+
+  The Resource Data effectively created is a 32-bit Memory Resource
+  Data. Cf ACPI 6.4:
+   - s19.6.83 "Memory Resource Descriptor Macro".
+   - s19.2.8 "Memory32FixedTerm".
+
+  See ACPI 6.4 spec, s19.2.8 for more.
+
+  @param [in]  IsReadWrite          ReadAndWrite parameter.
+  @param [in]  Addres               AddressBase parameter.
+  @param [in]  RangeLength          Range length.
+  @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] NewMemNode           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
+AmlCodeGenRdMemory32Fixed (
+  BOOLEAN                 IsReadWrite,
+  UINT32                  Address,
+  UINT32                  RangeLength,
+  AML_OBJECT_NODE_HANDLE  NameOpNode,
+  AML_DATA_NODE_HANDLE    *NewMemNode
+  )
+{
+  EFI_STATUS                                     Status;
+  AML_DATA_NODE                                  *MemNode;
+  EFI_ACPI_32_BIT_FIXED_MEMORY_RANGE_DESCRIPTOR  RangeDesc;
+
+  RangeDesc.Header.Header.Byte = ACPI_32_BIT_FIXED_MEMORY_RANGE_DESCRIPTOR;
+  RangeDesc.Header.Length      = 0x09;
+  RangeDesc.Information        = IsReadWrite ? BIT0 : 0;
+  RangeDesc.BaseAddress        = Address;
+  RangeDesc.Length             = RangeLength;
+
+  Status = AmlCreateDataNode (
+             EAmlNodeDataTypeResourceData,
+             (UINT8 *)&RangeDesc,
+             sizeof (RangeDesc),
+             &MemNode
+             );
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    return Status;
+  }
+
+  return LinkRdNode (MemNode, NameOpNode, NewMemNode);
+}
+
 /** Code generation for the "WordSpace ()" ASL function.
 
   The Resource Data effectively created is a Word Address Space Resource
-- 
2.31.1

[PATCH v3 2/3] DynamicTablesPkg: Remove redundant cast in AmlCodeGenReturn

[Rebecca Cran]

In AmlCodeGenReturn, the cast to AML_NODE_HEADER* in the call to
AmlSetFixedArgument is redundant because ReturnNode is already a
AML_NODE_HEADER* .

Signed-off-by: Rebecca Cran <quic_rcran@quicinc.com>
Reviewed-by: Pierre Gondois <pierre.gondois@arm.com>
---
 DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
index d245848ce3fa..838a892c6b58 100644
--- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
+++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
@@ -1564,7 +1564,7 @@ AmlCodeGenReturn (
   Status = AmlSetFixedArgument (
              ObjectNode,
              EAmlParseIndexTerm0,
-             (AML_NODE_HEADER *)ReturnNode
+             ReturnNode
              );
   if (EFI_ERROR (Status)) {
     ASSERT (0);
-- 
2.31.1

[PATCH v3 3/3] DynamicTablesPkg: Add AmlCodeGenMethodRetInteger function

[Rebecca Cran]

Add AmlCodeGenMethodRetInteger function to generate AML code for
a Method returning an Integer.

Signed-off-by: Rebecca Cran <quic_rcran@quicinc.com>
Reviewed-by: Pierre Gondois <pierre.gondois@arm.com>
---
 DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h            |  47 ++++++
 DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c | 156 ++++++++++++++++++++
 2 files changed, 203 insertions(+)

diff --git a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
index 80d05f74ee69..6f214c0dfad2 100644
--- a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
+++ b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
@@ -1118,6 +1118,53 @@ AmlCodeGenMethodRetNameString (
   OUT       AML_OBJECT_NODE_HANDLE  *NewObjectNode        OPTIONAL
   );
 
+/** AML code generation for a method returning an Integer.
+
+  AmlCodeGenMethodRetInteger (
+    "_CBA", 0, 1, TRUE, 3, ParentNode, NewObjectNode
+    );
+  is equivalent of the following ASL code:
+    Method(_CBA, 1, Serialized, 3) {
+      Return (0)
+    }
+
+  The ASL parameters "ReturnType" and "ParameterTypes" are not asked
+  in this function. They are optional parameters in ASL.
+
+  @param [in]  MethodNameString     The new Method's name.
+                                    Must be a NULL-terminated ASL NameString
+                                    e.g.: "MET0", "_SB.MET0", etc.
+                                    The input string is copied.
+  @param [in]  ReturnedInteger      The value of the integer returned by the
+                                    method.
+  @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.
+**/
+EFI_STATUS
+EFIAPI
+AmlCodeGenMethodRetInteger (
+  IN  CONST CHAR8                   *MethodNameString,
+  IN        UINT64                  ReturnedInteger,
+  IN        UINT8                   NumArgs,
+  IN        BOOLEAN                 IsSerialized,
+  IN        UINT8                   SyncLevel,
+  IN        AML_NODE_HANDLE         ParentNode           OPTIONAL,
+  OUT       AML_OBJECT_NODE_HANDLE  *NewObjectNode        OPTIONAL
+  );
+
 /** Create a _LPI name.
 
   AmlCreateLpiNode ("_LPI", 0, 1, ParentNode, &LpiNode) is
diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
index 838a892c6b58..07822ead5b70 100644
--- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
+++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
@@ -1685,6 +1685,61 @@ exit_handler:
   return Status;
 }
 
+/** AML code generation for a Return object node,
+    returning an Integer.
+
+  AmlCodeGenReturn (0), ParentNode, NewObjectNode) is
+  equivalent of the following ASL code:
+    Return (0)
+
+  The ACPI 6.3 specification, 20.2.8 "Statement Opcodes Encoding" states:
+    DefReturn := ReturnOp ArgObject
+    ReturnOp := 0xA4
+    ArgObject := TermArg => DataRefObject
+
+  Thus, the ReturnNode must be evaluated as a DataRefObject.
+
+  The ReturnNode must be generated inside a Method body scope.
+
+  @param [in]  Integer        The integer is returned by the Return
+                              ASL statement.
+  @param [in]  ParentNode     If provided, set ParentNode as the parent
+                              of the node created.
+                              Must be a MethodOp node.
+  @param [out] NewObjectNode  If success, contains the created node.
+
+  @retval EFI_SUCCESS             Success.
+  @retval EFI_INVALID_PARAMETER   Invalid parameter.
+  @retval EFI_OUT_OF_RESOURCES    Failed to allocate memory.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+AmlCodeGenReturnInteger (
+  IN        UINT64           Integer,
+  IN        AML_NODE_HEADER  *ParentNode      OPTIONAL,
+  OUT       AML_OBJECT_NODE  **NewObjectNode  OPTIONAL
+  )
+{
+  EFI_STATUS       Status;
+  AML_OBJECT_NODE  *IntNode;
+
+  IntNode = NULL;
+
+  Status = AmlCodeGenInteger (Integer, &IntNode);
+  ASSERT_EFI_ERROR (Status);
+
+  // AmlCodeGenReturn() deletes DataNode if error.
+  Status = AmlCodeGenReturn (
+             (AML_NODE_HEADER *)IntNode,
+             ParentNode,
+             NewObjectNode
+             );
+  ASSERT_EFI_ERROR (Status);
+
+  return Status;
+}
+
 /** AML code generation for a method returning a NameString.
 
   AmlCodeGenMethodRetNameString (
@@ -1793,6 +1848,107 @@ error_handler:
   return Status;
 }
 
+/** AML code generation for a method returning an Integer.
+
+  AmlCodeGenMethodRetInteger (
+    "_CBA", 0, 1, TRUE, 3, ParentNode, NewObjectNode
+    );
+  is equivalent of the following ASL code:
+    Method(_CBA, 1, Serialized, 3) {
+      Return (0)
+    }
+
+  The ASL parameters "ReturnType" and "ParameterTypes" are not asked
+  in this function. They are optional parameters in ASL.
+
+  @param [in]  MethodNameString     The new Method's name.
+                                    Must be a NULL-terminated ASL NameString
+                                    e.g.: "MET0", "_SB.MET0", etc.
+                                    The input string is copied.
+  @param [in]  ReturnedInteger      The value of the integer returned by the
+                                    method.
+  @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.
+**/
+EFI_STATUS
+EFIAPI
+AmlCodeGenMethodRetInteger (
+  IN  CONST CHAR8                   *MethodNameString,
+  IN        UINT64                  ReturnedInteger,
+  IN        UINT8                   NumArgs,
+  IN        BOOLEAN                 IsSerialized,
+  IN        UINT8                   SyncLevel,
+  IN        AML_NODE_HANDLE         ParentNode      OPTIONAL,
+  OUT       AML_OBJECT_NODE_HANDLE  *NewObjectNode  OPTIONAL
+  )
+{
+  EFI_STATUS              Status;
+  AML_OBJECT_NODE_HANDLE  MethodNode;
+
+  if ((MethodNameString == NULL)  ||
+      ((ParentNode == NULL) && (NewObjectNode == NULL)))
+  {
+    ASSERT (0);
+    return EFI_INVALID_PARAMETER;
+  }
+
+  // Create a Method named MethodNameString.
+  Status = AmlCodeGenMethod (
+             MethodNameString,
+             NumArgs,
+             IsSerialized,
+             SyncLevel,
+             NULL,
+             &MethodNode
+             );
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    return Status;
+  }
+
+  Status = AmlCodeGenReturnInteger (
+             ReturnedInteger,
+             (AML_NODE_HANDLE)MethodNode,
+             NULL
+             );
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    goto error_handler;
+  }
+
+  Status = LinkNode (
+             MethodNode,
+             ParentNode,
+             NewObjectNode
+             );
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    goto error_handler;
+  }
+
+  return Status;
+
+error_handler:
+  if (MethodNode != NULL) {
+    AmlDeleteTree ((AML_NODE_HANDLE)MethodNode);
+  }
+
+  return Status;
+}
+
 /** Create a _LPI name.
 
   AmlCreateLpiNode ("_LPI", 0, 1, ParentNode, &LpiNode) is
-- 
2.31.1

Re: [PATCH v3 1/3] DynamicTablesPkg: Add Memory32Fixed function

[Sami Mujawar]

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

Hi Rebecca,

Thank you for this patch.

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

Otherwise this patch looks good to me.

Regards,

Sami Mujawar

On 13/01/2022 04:40 PM, Rebecca Cran wrote:

Add a Memory32Fixed function to generate code for the corresponding
Memory32Fixed macro in AML.

Signed-off-by: Rebecca Cran <quic_rcran@quicinc.com><mailto:quic_rcran@quicinc.com>
Reviewed-by: Pierre Gondois <pierre.gondois@arm.com><mailto:pierre.gondois@arm.com>
---
 DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h                        | 33 ++++++++++++
 DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.c | 57 ++++++++++++++++++++
 2 files changed, 90 insertions(+)

diff --git a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
index af18bf8e4871..80d05f74ee69 100644
--- a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
+++ b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
@@ -592,6 +592,39 @@ AmlCodeGenRdDWordMemory (
   OUT       AML_DATA_NODE_HANDLE    *NewRdNode  OPTIONAL
   );

+/** Code generation for the "Memory32Fixed ()" ASL macro.
+
+  The Resource Data effectively created is a 32-bit Memory Resource
+  Data. Cf ACPI 6.4:
+   - s19.6.83 "Memory Resource Descriptor Macro".
+   - s19.2.8 "Memory32FixedTerm".
+
+  See ACPI 6.4 spec, s19.2.8 for more.
+
+  @param [in]  IsReadWrite          ReadAndWrite parameter.
+  @param [in]  Address              AddressBase parameter.
+  @param [in]  RangeLength          Range length.
+  @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] NewMemNode           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
+AmlCodeGenRdMemory32Fixed (
+  BOOLEAN                 IsReadWrite,
+  UINT32                  Address,
+  UINT32                  RangeLength,
+  AML_OBJECT_NODE_HANDLE  NameOpNode,
+  AML_DATA_NODE_HANDLE    *NewMemNode
+  );
+
 /** Code generation for the "WordBusNumber ()" ASL function.

   The Resource Data effectively created is a Word Address Space Resource
diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.c b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.c
index 40d8c2b07ae3..fb18c5a77e2f 100644
--- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.c
+++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.c
@@ -609,6 +609,63 @@ AmlCodeGenRdDWordMemory (
            );
 }

+/** Code generation for the "Memory32Fixed ()" ASL macro.
+
+  The Resource Data effectively created is a 32-bit Memory Resource
+  Data. Cf ACPI 6.4:
+   - s19.6.83 "Memory Resource Descriptor Macro".
+   - s19.2.8 "Memory32FixedTerm".
+
+  See ACPI 6.4 spec, s19.2.8 for more.
+
+  @param [in]  IsReadWrite          ReadAndWrite parameter.
+  @param [in]  Addres               AddressBase parameter.
+  @param [in]  RangeLength          Range length.
+  @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] NewMemNode           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
+AmlCodeGenRdMemory32Fixed (
+  BOOLEAN                 IsReadWrite,
+  UINT32                  Address,
+  UINT32                  RangeLength,
+  AML_OBJECT_NODE_HANDLE  NameOpNode,
+  AML_DATA_NODE_HANDLE    *NewMemNode
+  )
+{
+  EFI_STATUS                                     Status;
+  AML_DATA_NODE                                  *MemNode;
+  EFI_ACPI_32_BIT_FIXED_MEMORY_RANGE_DESCRIPTOR  RangeDesc;
+
+  RangeDesc.Header.Header.Byte = ACPI_32_BIT_FIXED_MEMORY_RANGE_DESCRIPTOR;
+  RangeDesc.Header.Length      = 0x09;

[SAMI] Would it be ok if I change the above line to
RangeDesc.Header.Length     = sizeof (EFI_ACPI_32_BIT_FIXED_MEMORY_RANGE_DESCRIPTOR) -
                                    sizeof (ACPI_LARGE_RESOURCE_HEADER);
before merging?
[/SAMI]


+  RangeDesc.Information        = IsReadWrite ? BIT0 : 0;
+  RangeDesc.BaseAddress        = Address;
+  RangeDesc.Length             = RangeLength;
+
+  Status = AmlCreateDataNode (
+             EAmlNodeDataTypeResourceData,
+             (UINT8 *)&RangeDesc,
+             sizeof (RangeDesc),
+             &MemNode
+             );
+  if (EFI_ERROR (Status)) {
+    ASSERT (0);
+    return Status;
+  }
+
+  return LinkRdNode (MemNode, NameOpNode, NewMemNode);
+}
+
 /** Code generation for the "WordSpace ()" ASL function.

   The Resource Data effectively created is a Word Address Space Resource


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.

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

Re: [PATCH v3 2/3] DynamicTablesPkg: Remove redundant cast in AmlCodeGenReturn

[Sami Mujawar]

Hi Rebecca,

Thank you for this patch.

This change looks good to me.

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

Regards,

Sami Mujawar


On 13/01/2022 04:40 PM, Rebecca Cran wrote:
> In AmlCodeGenReturn, the cast to AML_NODE_HEADER* in the call to
> AmlSetFixedArgument is redundant because ReturnNode is already a
> AML_NODE_HEADER* .
>
> Signed-off-by: Rebecca Cran <quic_rcran@quicinc.com>
> Reviewed-by: Pierre Gondois <pierre.gondois@arm.com>
> ---
>   DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
> index d245848ce3fa..838a892c6b58 100644
> --- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
> +++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
> @@ -1564,7 +1564,7 @@ AmlCodeGenReturn (
>     Status = AmlSetFixedArgument (
>                ObjectNode,
>                EAmlParseIndexTerm0,
> -             (AML_NODE_HEADER *)ReturnNode
> +             ReturnNode
>                );
>     if (EFI_ERROR (Status)) {
>       ASSERT (0);

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.

Re: [PATCH v3 3/3] DynamicTablesPkg: Add AmlCodeGenMethodRetInteger function

[Sami Mujawar]

Hi Rebecca,

Thank you for this patch.

Please find my response inline marked [SAMI].

Regards,

Sami Mujawar


On 13/01/2022 04:40 PM, Rebecca Cran wrote:
> Add AmlCodeGenMethodRetInteger function to generate AML code for
> a Method returning an Integer.
>
> Signed-off-by: Rebecca Cran <quic_rcran@quicinc.com>
> Reviewed-by: Pierre Gondois <pierre.gondois@arm.com>
> ---
>   DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h            |  47 ++++++
>   DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c | 156 ++++++++++++++++++++
>   2 files changed, 203 insertions(+)
>
> diff --git a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
> index 80d05f74ee69..6f214c0dfad2 100644
> --- a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
> +++ b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
> @@ -1118,6 +1118,53 @@ AmlCodeGenMethodRetNameString (
>     OUT       AML_OBJECT_NODE_HANDLE  *NewObjectNode        OPTIONAL
>     );
>
> +/** AML code generation for a method returning an Integer.
> +
> +  AmlCodeGenMethodRetInteger (
> +    "_CBA", 0, 1, TRUE, 3, ParentNode, NewObjectNode
> +    );
> +  is equivalent of the following ASL code:
> +    Method(_CBA, 1, Serialized, 3) {
> +      Return (0)
> +    }
> +
> +  The ASL parameters "ReturnType" and "ParameterTypes" are not asked
> +  in this function. They are optional parameters in ASL.
> +
> +  @param [in]  MethodNameString     The new Method's name.
> +                                    Must be a NULL-terminated ASL NameString
> +                                    e.g.: "MET0", "_SB.MET0", etc.
> +                                    The input string is copied.
> +  @param [in]  ReturnedInteger      The value of the integer returned by the
> +                                    method.
> +  @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.
> +**/
> +EFI_STATUS
> +EFIAPI
> +AmlCodeGenMethodRetInteger (
> +  IN  CONST CHAR8                   *MethodNameString,
> +  IN        UINT64                  ReturnedInteger,
> +  IN        UINT8                   NumArgs,
> +  IN        BOOLEAN                 IsSerialized,
> +  IN        UINT8                   SyncLevel,
> +  IN        AML_NODE_HANDLE         ParentNode           OPTIONAL,
> +  OUT       AML_OBJECT_NODE_HANDLE  *NewObjectNode        OPTIONAL
> +  );
> +
>   /** Create a _LPI name.
>
>     AmlCreateLpiNode ("_LPI", 0, 1, ParentNode, &LpiNode) is
> diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
> index 838a892c6b58..07822ead5b70 100644
> --- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
> +++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c
> @@ -1685,6 +1685,61 @@ exit_handler:
>     return Status;
>   }
>
> +/** AML code generation for a Return object node,
> +    returning an Integer.
> +
> +  AmlCodeGenReturn (0), ParentNode, NewObjectNode) is
> +  equivalent of the following ASL code:
> +    Return (0)
> +
> +  The ACPI 6.3 specification, 20.2.8 "Statement Opcodes Encoding" states:
> +    DefReturn := ReturnOp ArgObject
> +    ReturnOp := 0xA4
> +    ArgObject := TermArg => DataRefObject
> +
> +  Thus, the ReturnNode must be evaluated as a DataRefObject.
> +
> +  The ReturnNode must be generated inside a Method body scope.
> +
> +  @param [in]  Integer        The integer is returned by the Return
> +                              ASL statement.
> +  @param [in]  ParentNode     If provided, set ParentNode as the parent
> +                              of the node created.
> +                              Must be a MethodOp node.
> +  @param [out] NewObjectNode  If success, contains the created node.
> +
> +  @retval EFI_SUCCESS             Success.
> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
> +  @retval EFI_OUT_OF_RESOURCES    Failed to allocate memory.
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +AmlCodeGenReturnInteger (
> +  IN        UINT64           Integer,
> +  IN        AML_NODE_HEADER  *ParentNode      OPTIONAL,
> +  OUT       AML_OBJECT_NODE  **NewObjectNode  OPTIONAL
> +  )
> +{
> +  EFI_STATUS       Status;
> +  AML_OBJECT_NODE  *IntNode;
> +
> +  IntNode = NULL;
> +
> +  Status = AmlCodeGenInteger (Integer, &IntNode);
> +  ASSERT_EFI_ERROR (Status);
[SAMI] In release builds, ASSERT_EFI_ERROR() would vanish and we could
end up accessing an invalid pointer.
If you agree, I will change this to check the status and return the
error code, before merging.
[/SAMI]
> +
> +  // AmlCodeGenReturn() deletes DataNode if error.
> +  Status = AmlCodeGenReturn (
> +             (AML_NODE_HEADER *)IntNode,
> +             ParentNode,
> +             NewObjectNode
> +             );
> +  ASSERT_EFI_ERROR (Status);
> +
> +  return Status;
> +}
> +
>   /** AML code generation for a method returning a NameString.
>
>     AmlCodeGenMethodRetNameString (
> @@ -1793,6 +1848,107 @@ error_handler:
>     return Status;
>   }
>
> +/** AML code generation for a method returning an Integer.
> +
> +  AmlCodeGenMethodRetInteger (
> +    "_CBA", 0, 1, TRUE, 3, ParentNode, NewObjectNode
> +    );
> +  is equivalent of the following ASL code:
> +    Method(_CBA, 1, Serialized, 3) {
> +      Return (0)
> +    }
> +
> +  The ASL parameters "ReturnType" and "ParameterTypes" are not asked
> +  in this function. They are optional parameters in ASL.
> +
> +  @param [in]  MethodNameString     The new Method's name.
> +                                    Must be a NULL-terminated ASL NameString
> +                                    e.g.: "MET0", "_SB.MET0", etc.
> +                                    The input string is copied.
> +  @param [in]  ReturnedInteger      The value of the integer returned by the
> +                                    method.
> +  @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.
> +**/
> +EFI_STATUS
> +EFIAPI
> +AmlCodeGenMethodRetInteger (
> +  IN  CONST CHAR8                   *MethodNameString,
> +  IN        UINT64                  ReturnedInteger,
> +  IN        UINT8                   NumArgs,
> +  IN        BOOLEAN                 IsSerialized,
> +  IN        UINT8                   SyncLevel,
> +  IN        AML_NODE_HANDLE         ParentNode      OPTIONAL,
> +  OUT       AML_OBJECT_NODE_HANDLE  *NewObjectNode  OPTIONAL
> +  )
> +{
> +  EFI_STATUS              Status;
> +  AML_OBJECT_NODE_HANDLE  MethodNode;
> +
> +  if ((MethodNameString == NULL)  ||
> +      ((ParentNode == NULL) && (NewObjectNode == NULL)))
> +  {
> +    ASSERT (0);
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  // Create a Method named MethodNameString.
> +  Status = AmlCodeGenMethod (
> +             MethodNameString,
> +             NumArgs,
> +             IsSerialized,
> +             SyncLevel,
> +             NULL,
> +             &MethodNode
> +             );
> +  if (EFI_ERROR (Status)) {
> +    ASSERT (0);
> +    return Status;
> +  }
> +
> +  Status = AmlCodeGenReturnInteger (
> +             ReturnedInteger,
> +             (AML_NODE_HANDLE)MethodNode,
> +             NULL
> +             );
> +  if (EFI_ERROR (Status)) {
> +    ASSERT (0);
> +    goto error_handler;
> +  }
> +
> +  Status = LinkNode (
> +             MethodNode,
> +             ParentNode,
> +             NewObjectNode
> +             );
> +  if (EFI_ERROR (Status)) {
> +    ASSERT (0);
> +    goto error_handler;
> +  }
> +
> +  return Status;
> +
> +error_handler:
> +  if (MethodNode != NULL) {
> +    AmlDeleteTree ((AML_NODE_HANDLE)MethodNode);
> +  }
> +
> +  return Status;
> +}
> +
>   /** Create a _LPI name.
>
>     AmlCreateLpiNode ("_LPI", 0, 1, ParentNode, &LpiNode) is

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.

Re: [edk2-devel] [PATCH v3 1/3] DynamicTablesPkg: Add Memory32Fixed function

[Rebecca Cran]

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

On 2/2/22 07:52, Sami Mujawar wrote:
>> +AmlCodeGenRdMemory32Fixed (
>> +  BOOLEAN                 IsReadWrite,
>> +  UINT32                  Address,
>> +  UINT32                  RangeLength,
>> +  AML_OBJECT_NODE_HANDLE  NameOpNode,
>> +  AML_DATA_NODE_HANDLE    *NewMemNode
>> +  )
>> +{
>> +  EFI_STATUS                                     Status;
>> +  AML_DATA_NODE                                  *MemNode;
>> +  EFI_ACPI_32_BIT_FIXED_MEMORY_RANGE_DESCRIPTOR  RangeDesc;
>> +
>> +  RangeDesc.Header.Header.Byte = ACPI_32_BIT_FIXED_MEMORY_RANGE_DESCRIPTOR;
>> +  RangeDesc.Header.Length      = 0x09;
> [SAMI] Would it be ok if I change the above line to
> RangeDesc.Header.Length     = sizeof 
> (EFI_ACPI_32_BIT_FIXED_MEMORY_RANGE_DESCRIPTOR) -
>                                     sizeof (ACPI_LARGE_RESOURCE_HEADER);
> before merging?
> [/SAMI]

Yes, that would be good.


Thanks.

Rebecca Cran

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

Re: [edk2-devel] [PATCH v3 3/3] DynamicTablesPkg: Add AmlCodeGenMethodRetInteger function

[Rebecca Cran]

On 2/2/22 07:53, Sami Mujawar wrote:
>> +  Status = AmlCodeGenInteger (Integer, &IntNode);
>> +  ASSERT_EFI_ERROR (Status);
> [SAMI] In release builds, ASSERT_EFI_ERROR() would vanish and we could
> end up accessing an invalid pointer.
> If you agree, I will change this to check the status and return the
> error code, before merging.
> [/SAMI]

Yes, please do.


Thanks.

Rebecca Cran

Re: [PATCH v3 0/3] Add Memory32Fixed and AmlCodeGenMethodRetInteger functions

[Sami Mujawar]

For this series,

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

Merged as 017564d637e9..33189f052783

Regards,

Sami Mujawar


On 13/01/2022 04:40 PM, Rebecca Cran wrote:
> Add functions to generate code for the Memory32Fixed ASL macro and a
> method returning an Integer.
>
> Remove a redundant cast from AmlCodeGenReturn.
>
>
> Changes from v2 to v3:
>
> o Rename function to AmlCodeGenRdMemory32Fixed.
> o Use define ACPI_32_BIT_FIXED_MEMORY_RANGE_DESCRIPTOR.
>
> Rebecca Cran (3):
>    DynamicTablesPkg: Add Memory32Fixed function
>    DynamicTablesPkg: Remove redundant cast in AmlCodeGenReturn
>    DynamicTablesPkg: Add AmlCodeGenMethodRetInteger function
>
>   DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h                        |  80 ++++++++++
>   DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlCodeGen.c             | 158 +++++++++++++++++++-
>   DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.c |  57 +++++++
>   3 files changed, 294 insertions(+), 1 deletion(-)
>

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.