1. Patchew
  2. EDK2
  3. [edk2-devel] [PATCH v4 0/6] Enhance DynamicTablesPkg modules
  4. View patch

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

Kun Qin posted 6 patches 3 years, 6 months ago
[edk2-devel] [PATCH v4 6/6] DynamicTablesPkg: AcpiSsdtPcieLibArm: Added case handling for PCI config
Posted by Kun Qin 3 years, 6 months ago 
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
-- 
2.37.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#92318): https://edk2.groups.io/g/devel/message/92318
Mute This Topic: https://groups.io/mt/92947269/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v4 6/6] DynamicTablesPkg: AcpiSsdtPcieLibArm: Added case handling for PCI config
Posted by PierreGondois 3 years, 5 months ago 
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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#92478): https://edk2.groups.io/g/devel/message/92478
Mute This Topic: https://groups.io/mt/92947269/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v4 6/6] DynamicTablesPkg: AcpiSsdtPcieLibArm: Added case handling for PCI config
Posted by Kun Qin 3 years, 5 months ago 
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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#92495): https://edk2.groups.io/g/devel/message/92495
Mute This Topic: https://groups.io/mt/92947269/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v4 6/6] DynamicTablesPkg: AcpiSsdtPcieLibArm: Added case handling for PCI config
Posted by PierreGondois 3 years, 5 months ago 

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@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


-=-=-=-=-=-=-=-=-=-=-=-
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/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v4 6/6] DynamicTablesPkg: AcpiSsdtPcieLibArm: Added case handling for PCI config
Posted by Sami Mujawar 3 years, 5 months ago 
Hi Kun,

I plan to get this series merged when the merge window opens.

If you agree, I will drop this patch before merging. Please let me know 
if that is ok.

Regards,

Sami Mujawar

On 17/08/2022 09:53 am, Pierre Gondois wrote:
>
>
> 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@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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#92517): https://edk2.groups.io/g/devel/message/92517
Mute This Topic: https://groups.io/mt/92947269/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v4 6/6] DynamicTablesPkg: AcpiSsdtPcieLibArm: Added case handling for PCI config
Posted by Kun Qin 3 years, 5 months ago 
Hi Sami,

Thank you for the help! I agree that we can drop this patch and merge 
the rest when the window is open.

Pierre,

Thanks for your input on the usage as well!

Regards,
Kun

On 8/17/2022 5:06 AM, Sami Mujawar wrote:
> Hi Kun,
>
> I plan to get this series merged when the merge window opens.
>
> If you agree, I will drop this patch before merging. Please let me 
> know if that is ok.
>
> Regards,
>
> Sami Mujawar
>
> On 17/08/2022 09:53 am, Pierre Gondois wrote:
>>
>>
>> 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@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
>
>
> 
>
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#92529): https://edk2.groups.io/g/devel/message/92529
Mute This Topic: https://groups.io/mt/92947269/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v4 6/6] DynamicTablesPkg: AcpiSsdtPcieLibArm: Added case handling for PCI config
Posted by Kun Qin 3 years, 5 months ago 
Hi Sami,

Now that the merge window opens, could you please help us to merge in
these patch series (after dropping this specific patch) when you have a 
chance?

Please let me know if you ran into issues when doing that. Thanks in 
advance!

Regards,
Kun

On 8/17/2022 10:42 AM, Kun Qin wrote:
> Hi Sami,
>
> Thank you for the help! I agree that we can drop this patch and merge 
> the rest when the window is open.
>
> Pierre,
>
> Thanks for your input on the usage as well!
>
> Regards,
> Kun
>
> On 8/17/2022 5:06 AM, Sami Mujawar wrote:
>> Hi Kun,
>>
>> I plan to get this series merged when the merge window opens.
>>
>> If you agree, I will drop this patch before merging. Please let me 
>> know if that is ok.
>>
>> Regards,
>>
>> Sami Mujawar
>>
>> On 17/08/2022 09:53 am, Pierre Gondois wrote:
>>>
>>>
>>> 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@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
>>
>>
>> 
>>
>>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#92984): https://edk2.groups.io/g/devel/message/92984
Mute This Topic: https://groups.io/mt/92947269/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.