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

["Rebecca Cran" <quic_rcran@quicinc.com>]

Thanks. I've fixed these issues in the v2 patch which I'll send out soon.


-- 

Rebecca Cran


On 1/11/22 04:34, Pierre Gondois wrote:
> 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
>