Re: [PATCH 1/2] DynamicTablesPkg: IORT generator updates for Rev E.e spec

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

Hello Swatisri,

On 1/25/23 19:40, Name wrote:
> From: Swatisri Kantamsetti <swatisrik@nvidia.com>
> 
> The IO Remapping Table, Platform Design Document, Revision E.e,
> Sept 2022 (https://developer.arm.com/documentation/den0049/ee)
> added flags in SMMUv3 node for validity of ID mappings for MSIs
> related to control interrupts.
> It makes one small addition to SMMUv3 nodes to
> describe MSI support independently of wired GSIV support
> 
> Therefore, update the IORT generator to:
> - increment IORT table revision to 6
> - increment SMMUV3 node revision to 5
> - for SMMUV3 node revision >=5 check the DeviceID mapping index
>    valid flag to populate DeviceIdMappingIndex field
> - preserve backward compatibility.
> 
> Signed-off-by: Swatisri Kantamsetti <swatisrik@nvidia.com>
> ---
>   .../Acpi/Arm/AcpiIortLibArm/IortGenerator.c   | 36 ++++++++++++++-----
>   1 file changed, 28 insertions(+), 8 deletions(-)
> 
> diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c
> index f28973c1a8..235a475629 100644
> --- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c
> +++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiIortLibArm/IortGenerator.c
> @@ -1554,9 +1554,14 @@ AddSmmuV3Nodes (
>       {
>         SmmuV3Node->Node.Revision   = 2;
>         SmmuV3Node->Node.Identifier = EFI_ACPI_RESERVED_DWORD;
> -    } else {
> +    } else if (AcpiTableInfo->AcpiTableRevision <
> +               EFI_ACPI_IO_REMAPPING_TABLE_REVISION_06)
> +    {
>         SmmuV3Node->Node.Revision   = 4;
>         SmmuV3Node->Node.Identifier = NodeList->Identifier;
> +    } else {
> +      SmmuV3Node->Node.Revision   = 5;
> +      SmmuV3Node->Node.Identifier = NodeList->Identifier;
>       }
>   
>       // SMMUv3 specific data
> @@ -1577,12 +1582,27 @@ AddSmmuV3Nodes (
>         SmmuV3Node->ProximityDomain = 0;
>       }
>   
> -    if ((SmmuV3Node->Event != 0) && (SmmuV3Node->Pri != 0) &&
> -        (SmmuV3Node->Gerr != 0) && (SmmuV3Node->Sync != 0))
> -    {
> -      // If all the SMMU control interrupts are GSIV based,
> -      // the DeviceID mapping index field is ignored.
> -      SmmuV3Node->DeviceIdMappingIndex = 0;
> +    /* DeviceID mapping valid flag was introduced in IORT rev E.e
> +     * for SMMUV3 nodes rev. > 5.
> +     * For older revisions, if all the SMMU control interrupts are GSIV
> +     * based, DeviceID mapping index field is ignored.
> +     * If the DeviceID mapping index valid flag is set to 0,
> +     * DeviceID mapping index field must be ignored.
> +     * Where the SMMU uses message signaled interrupts for
> +     * its control interrupts, DeviceId Mapping Index contains an
> +     * index into the array of ID mapping.
> +     */
> +
> +    if ((SmmuV3Node->Node.Revision < 5) || (!(SmmuV3Node->Flags & EFI_ACPI_IORT_SMMUv3_FLAG_DEVICEID_VALID))) {

The definitions of:
- EFI_ACPI_IORT_SMMUv3_FLAG_DEVICEID_VALID
- EFI_ACPI_IO_REMAPPING_TABLE_REVISION_06
are in the next patch, so a build in between these 2 patches will fail,
other than that:
Reviewed-by: Pierre Gondois <pierre.gondois@arm.com>


> +      if ((SmmuV3Node->Event != 0) && (SmmuV3Node->Pri != 0) &&
> +          (SmmuV3Node->Gerr != 0) && (SmmuV3Node->Sync != 0))
> +      {
> +        // If all the SMMU control interrupts are GSIV based,
> +        // the DeviceID mapping index field is ignored.
> +        SmmuV3Node->DeviceIdMappingIndex = 0;
> +      } else {
> +        SmmuV3Node->DeviceIdMappingIndex = NodeList->DeviceIdMappingIndex;
> +      }
>       } else {
>         SmmuV3Node->DeviceIdMappingIndex = NodeList->DeviceIdMappingIndex;
>       }
> @@ -2819,7 +2839,7 @@ ACPI_IORT_GENERATOR  IortGenerator = {
>       // ACPI Table Signature
>       EFI_ACPI_6_4_IO_REMAPPING_TABLE_SIGNATURE,
>       // ACPI Table Revision supported by this Generator
> -    EFI_ACPI_IO_REMAPPING_TABLE_REVISION_05,
> +    EFI_ACPI_IO_REMAPPING_TABLE_REVISION_06,
>       // Minimum supported ACPI Table Revision
>       EFI_ACPI_IO_REMAPPING_TABLE_REVISION_00,
>       // Creator ID