[edk2-devel] [PATCH v4 6/6] DynamicTablesPkg: AcpiSsdtPcieLibArm: Added case handling for PCI config

[PierreGondois]

On 8/17/22 02:17, Kun Qin wrote:
> 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.

I haven't seen any usecase neither so far, but it is to be used to reference a
PCI bus/device/function/register location to identify/configure a device
from what I understood.

> 
> 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 at arm.com>
>>> Cc: Alexei Fedorov <Alexei.Fedorov at arm.com>
>>>
>>> Co-authored-by: Joe Lopez <joelopez at microsoft.com>
>>> Signed-off-by: Kun Qin <kuqin12 at gmail.com>
>>> Reviewed-by: Pierre Gondois <pierre.gondois at arm.com>
>>> Reviewed-by: Sami Mujawar <sami.mujawar at 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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#92506): https://edk2.groups.io/g/devel/message/92506
Mute This Topic: https://groups.io/mt/92947269/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-