Re: [PATCH] DynamicTablesPkg: AcpiSsdtPcieLibArm : Add UID to slot creation

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

Hello Jeff,

On 8/22/22 18:21, Jeff Brasen wrote:
> Expose the UID value to GeneratePciSlots().
> This is needed for some cases for example:
> https://docs.microsoft.com/en-us/windows-hardware/drivers/pci/dsd-for-pcie-root-ports#identifying-externally-exposed-pcie-root-ports

I guess the UID would be used in the Nvidia library and not in:
   DynamicTablesPkg/Library/Common/SsdtPcieSupportLib/SsdtPcieSupportLib.c
If the external link breaks one day, there would remain no
example of how the UID is be used. Would it be possible to
add one of the examples from the guide you linked, or one made up
example like:

Name (_DSD, Package () {
   ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
   Package () {
     Package (2) {"UID", 0},
   }
}

in the commit message ? Other than this:
Reviewed-by: Pierre Gondois <pierre.gondois@arm.com>

Regards,
Pierre

> 
> Signed-off-by: Jeff Brasen <jbrasen@nvidia.com>
> ---
>   DynamicTablesPkg/Include/Library/SsdtPcieSupportLib.h     | 8 +++++---
>   .../Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c       | 5 ++++-
>   .../Common/SsdtPcieSupportLib/SsdtPcieSupportLib.c        | 8 +++++---
>   3 files changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/DynamicTablesPkg/Include/Library/SsdtPcieSupportLib.h b/DynamicTablesPkg/Include/Library/SsdtPcieSupportLib.h
> index 85e283a994..4171dabc33 100644
> --- a/DynamicTablesPkg/Include/Library/SsdtPcieSupportLib.h
> +++ b/DynamicTablesPkg/Include/Library/SsdtPcieSupportLib.h
> @@ -54,9 +54,10 @@ AddOscMethod (
>     used. It should be possible to enumerate them, but this is additional
>     information.
>   
> -  @param [in]       PciInfo     Pci device information.
> -  @param [in]  MappingTable     The mapping table structure.
> -  @param [in, out]  PciNode     Pci node to amend.
> +  @param [in]       PciInfo       Pci device information.
> +  @param [in]       MappingTable  The mapping table structure.
> +  @param [in]       Uid           Unique Id of the Pci device.
> +  @param [in, out]  PciNode       Pci node to amend.
>   
>     @retval EFI_SUCCESS            Success.
>     @retval EFI_INVALID_PARAMETER  Invalid parameter.
> @@ -67,6 +68,7 @@ EFIAPI
>   GeneratePciSlots (
>     IN      CONST CM_ARM_PCI_CONFIG_SPACE_INFO  *PciInfo,
>     IN      CONST MAPPING_TABLE                 *MappingTable,
> +  IN            UINT32                        Uid,
>     IN  OUT       AML_OBJECT_NODE_HANDLE        PciNode
>     );
>   
> diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
> index ceffe2838c..24e074ea90 100644
> --- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
> +++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtPcieLibArm/SsdtPcieGenerator.c
> @@ -295,6 +295,7 @@ GeneratePciDeviceInfo (
>     @param [in]       CfgMgrProtocol  Pointer to the Configuration Manager
>                                       Protocol interface.
>     @param [in]       PciInfo         Pci device information.
> +  @param [in]       Uid             Unique Id of the Pci device.
>     @param [in, out]  PciNode         Pci node to amend.
>   
>     @retval EFI_SUCCESS             The function completed successfully.
> @@ -308,6 +309,7 @@ GeneratePrt (
>     IN            ACPI_PCI_GENERATOR                            *Generator,
>     IN      CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL  *CONST  CfgMgrProtocol,
>     IN      CONST CM_ARM_PCI_CONFIG_SPACE_INFO                  *PciInfo,
> +  IN            UINT32                                        Uid,
>     IN  OUT       AML_OBJECT_NODE_HANDLE                        PciNode
>     )
>   {
> @@ -416,7 +418,7 @@ GeneratePrt (
>     PrtNode = NULL;
>   
>     // Generate the Pci slots once all the device have been added.
> -  Status = GeneratePciSlots (PciInfo, &Generator->DeviceTable, PciNode);
> +  Status = GeneratePciSlots (PciInfo, &Generator->DeviceTable, Uid, PciNode);
>     if (EFI_ERROR (Status)) {
>       ASSERT (0);
>       goto exit_handler;
> @@ -687,6 +689,7 @@ GeneratePciDevice (
>                  Generator,
>                  CfgMgrProtocol,
>                  PciInfo,
> +               Uid,
>                  PciNode
>                  );
>       if (EFI_ERROR (Status)) {
> diff --git a/DynamicTablesPkg/Library/Common/SsdtPcieSupportLib/SsdtPcieSupportLib.c b/DynamicTablesPkg/Library/Common/SsdtPcieSupportLib/SsdtPcieSupportLib.c
> index e5ab3a3ca8..b35fb6a7dd 100644
> --- a/DynamicTablesPkg/Library/Common/SsdtPcieSupportLib/SsdtPcieSupportLib.c
> +++ b/DynamicTablesPkg/Library/Common/SsdtPcieSupportLib/SsdtPcieSupportLib.c
> @@ -41,9 +41,10 @@
>     used. It should be possible to enumerate them, but this is additional
>     information.
>   
> -  @param [in]       PciInfo     Pci device information.
> -  @param [in]  MappingTable     The mapping table structure.
> -  @param [in, out]  PciNode     Pci node to amend.
> +  @param [in]       PciInfo       Pci device information.
> +  @param [in]       MappingTable  The mapping table structure.
> +  @param [in]       Uid           Unique Id of the Pci device.
> +  @param [in, out]  PciNode       Pci node to amend.
>   
>     @retval EFI_SUCCESS            Success.
>     @retval EFI_INVALID_PARAMETER  Invalid parameter.
> @@ -54,6 +55,7 @@ EFIAPI
>   GeneratePciSlots (
>     IN      CONST CM_ARM_PCI_CONFIG_SPACE_INFO  *PciInfo,
>     IN      CONST MAPPING_TABLE                 *MappingTable,
> +  IN            UINT32                        Uid,
>     IN  OUT       AML_OBJECT_NODE_HANDLE        PciNode
>     )
>   {