Re: [PATCH v4 6/6] DynamicTablesPkg: AcpiSsdtPcieLibArm: Added case handling for PCI config

["Kun Qin" <kuqin12@gmail.com>]

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

Hi Pierre,

You are correct that if CM_ARM_PCI_ADDRESS_MAP_INFO.PCI_SS_CONFIG
is no longer being used, this patch is not needed. Thanks for catching this.

On the other hand, just for my learning purpose, could you please let me 
know
what the use case for "PCI_SS_CONFIG" is? It does not seem to be used at 
all.

Thanks again for testing these patches!

Regards,
Kun

On 8/16/2022 8:33 AM, Pierre Gondois wrote:
> Hello Kun,
>
> Is this patch still required ?
> Cf: https://edk2.groups.io/g/devel/message/92204
>
> The CM_ARM_PCI_CONFIG_SPACE_INFO struct should be enough to describe
> the PCI ECAM, so CM_ARM_PCI_ADDRESS_MAP_INFO.SpaceCode being set to
> PCI_SS_CONFIG should be an invalid case.
> If not I don't think a v5 should be necessary.
>
> Also I ran the patchset on KvmTool and everything was working.
>
> Regards,
> Pierre
>
> On 8/11/22 00:28, Kun Qin wrote:
>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3998
>>
>> This change added a switch case handling for PCI_SS_CONFIG during SSDT
>> generation. This will allow PCI config case return EFI_SUCCESS 
>> instead of
>> EFI_INVALID_PARAMETER.
>>
>> Cc: Sami Mujawar <Sami.Mujawar@arm.com>
>> Cc: Alexei Fedorov <Alexei.Fedorov@arm.com>
>>
>> Co-authored-by: Joe Lopez <joelopez@microsoft.com>
>> Signed-off-by: Kun Qin <kuqin12@gmail.com>
>> Reviewed-by: Pierre Gondois <pierre.gondois@arm.com>
>> Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
>> ---
>>
>> Notes:
>>      v2:
>>      - Added Reviewed-by tag [Pierre]
>>           v3:
>>      - No change
>>           v4:
>>      - Added Reviewed-by tag [Sami]
>>
>> DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c 
>> | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git 
>> a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c 
>> b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c 
>>
>> index dd75fc27e60e..c6fbd09c43f8 100644
>> --- 
>> a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
>> +++ 
>> b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
>> @@ -606,6 +606,11 @@ GeneratePciCrs (
>>                      );
>>           break;
>>   +      case PCI_SS_CONFIG:
>> +        // Do nothing
>> +        Status = EFI_SUCCESS;
>> +        break;
>> +
>>         default:
>>           Status = EFI_INVALID_PARAMETER;
>>       } // switch

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