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

["Sami Mujawar" <sami.mujawar@arm.com>]

[-- 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 --]