Re: [PATCH 12/14] DynamicTablesPkg/AmlLib: Allow larger AccessSize for Pcc address space

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

Hi Sami,

On 10/26/22 14:34, Sami Mujawar wrote:
> Hi Pierre,
> 
> There are some minor changes required marked inline as [SAMI].
> 
> If you agree, I will make the changes before merging.

Yes indeed, thanks for spotting it and for making the modification,

Regards,
Pierre


> 
> Regards,
> 
> Sami Mujawar
> 
> On 10/10/2022 10:20 am, Pierre.Gondois@arm.com wrote:
>> From: Pierre Gondois <pierre.gondois@arm.com>
>>
>> For Pcc address space, the AccessSize field of a Register is
>> used to delcare the Pcc Subspace Id. This Id can be up to 256.
>>
>> Cf. ACPI 6.4, s14.7 Referencing the PCC address space
>>
>> Signed-off-by: Pierre Gondois <Pierre.Gondois@arm.com>
>> ---
>>    .../Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.c | 7 ++++++-
>>    1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.c b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.c
>> index 332962bed441..3901b6e47333 100644
>> --- a/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.c
>> +++ b/DynamicTablesPkg/Library/Common/AmlLib/CodeGen/AmlResourceDataCodeGen.c
>> @@ -1257,7 +1257,12 @@ AmlCodeGenRdRegister (
>>      AML_DATA_NODE                         *RdNode;
>>      EFI_ACPI_GENERIC_REGISTER_DESCRIPTOR  RdRegister;
>>    
>> -  if ((AccessSize > EFI_ACPI_6_4_QWORD)  ||
>> +  // Cf Cf. ACPI 6.4, s14.7 Referencing the PCC address space
> [SAMI] Cf appears twice.
>> +  // The AccessSize represents the Subspace Id for the PCC address space.
>> +  if (((AddressSpace == EFI_ACPI_6_3_PLATFORM_COMMUNICATION_CHANNEL) &&
>> +       (AccessSize > 256)) ||
>> +      ((AddressSpace != EFI_ACPI_6_3_PLATFORM_COMMUNICATION_CHANNEL) &&
> [SAMI] Change EFI_ACPI_6_3_PLATFORM_COMMUNICATION_CHANNEL to
> EFI_ACPI_6_4_PLATFORM_COMMUNICATION_CHANNEL.
>> +       (AccessSize > EFI_ACPI_6_4_QWORD)) ||
>>          ((NameOpNode == NULL) && (NewRdNode == NULL)))
>>      {
>>        ASSERT (0);