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

["PierreGondois" <pierre.gondois@arm.com>]

Hello Rebecca,

On 1/8/22 10:57 PM, Rebecca Cran via groups.io wrote:
> Add a Memory32Fixed function to generate code for the corresponding
> Memory32Fixed macro in AML.
>
> Signed-off-by: Rebecca Cran <quic_rcran@quicinc.com>
> ---
>  DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h                        | 33 +++++++++++
>  DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.c | 59 ++++++++++++++++++++
>  2 files changed, 92 insertions(+)
>
> diff --git a/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h b/DynamicTablesPkg/Include/Library/AmlLib/AmlLib.h
> index af18bf8e4871..8b3e80b61466 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
> +AmlCodeGenMemory32Fixed (
> +  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..b9e8429cc6ca 100644
> --- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.c
> +++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.c
> @@ -609,6 +609,65 @@ AmlCodeGenRdDWordMemory (
>             );
>  }
>  
> +/** Code generation for the "Memory32Fixed ()" ASL macro.
> +
> + The Resource Data effectively created is a 32-bit Memory Resource
I think there are 2 spaces for the indentation (instead of 1 above).
> +  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
> +AmlCodeGenMemory32Fixed (
> +  BOOLEAN                 IsReadWrite,
> +  UINT32                  Address,
> +  UINT32                  RangeLength,
> +  AML_OBJECT_NODE_HANDLE  NameOpNode,
> +  AML_DATA_NODE_HANDLE    *NewMemNode
> +  )
> +{
> +  EFI_STATUS     Status;
> +  AML_DATA_NODE  *MemNode;
> +  UINT8          Data[12];
> +
> +  Data[0]  = 0x86;
> +  Data[1]  = 0x09;
> +  Data[2]  = 0x00;
> +  Data[3]  = IsReadWrite;

Is it possible to use BITx instead as this is a bit field, as:

= IsReadWrite ? BIT0 : 0;

> +  Data[4]  = Address & 0xFF;
> +  Data[5]  = (Address & 0xFF00) >> 8;
> +  Data[6]  = (Address & 0xFF0000) >> 16;
> +  Data[7]  = (Address & 0xFF000000) >> 24;
> +  Data[8]  = RangeLength & 0xFF;
> +  Data[9]  = (RangeLength & 0xFF00) >> 8;
> +  Data[10] = (RangeLength & 0xFF0000) >> 16;
> +  Data[11] = (RangeLength & 0xFF000000) >> 24;
Is it possible to use a EFI_ACPI_32_BIT_MEMORY_RANGE_DESCRIPTOR structure instead of a raw buffer ?
> +
> +  Status = AmlCreateDataNode (EAmlNodeDataTypeResourceData, Data, sizeof (Data), &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

Thanks for the patch,

Regards,

Pierre