Re: [PATCH v5 4/9] hw/arm/virt-acpi-build: Improve comment in build_iort

[Eric Auger <eric.auger@redhat.com>]

Hi Gustavo,

On 6/23/25 3:57 PM, Gustavo Romero wrote:
> When building the Root Complex table, the comment about the code that
s/table/node? or do you refer to the IORT table?
> maps the RC node to SMMU node is misleading because it reads
> "RC -> SMMUv3 -> ITS", but the code is only mapping the RCs IDs to the
> SMMUv3 node. The step of mapping from the SMMUv3 IDs to the ITS Group
> node is actually defined in another table (in the SMMUv3 node). So
> change the comment to read "RC -> SMMUv3" instead.
>
> Signed-off-by Gustavo Romero <gustavo.romero@linaro.org>
> ---
>  hw/arm/virt-acpi-build.c | 25 +++++++++++++++++++------
>  1 file changed, 19 insertions(+), 6 deletions(-)
>
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 9eee284c80..e9cd3fb351 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -370,7 +370,7 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>          /* DeviceID mapping index (ignored since interrupts are GSIV based) */
>          build_append_int_noprefix(table_data, 0, 4);
>  
> -        /* output IORT node is the ITS group node (the first node) */
> +        /* Output IORT node is the ITS Group node (the first node) */
>          build_iort_id_mapping(table_data, 0, 0x10000, IORT_NODE_OFFSET);
>      }
>  
> @@ -407,23 +407,36 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>      if (vms->iommu == VIRT_IOMMU_SMMUV3) {
>          AcpiIortIdMapping *range;
>  
> -        /* translated RIDs connect to SMMUv3 node: RC -> SMMUv3 -> ITS */
> +        /*
> +         * Map RIDs (input) from RC to SMMUv3 nodes: RC -> SMMUv3.
> +         *
> +         * N.B.: The mapping from SMMUv3 to ITS Group node (SMMUv3 -> ITS) is
> +         * defined in the SMMUv3 table, where all SMMUv3 IDs are mapped to the
s/table/node
> +         * ITS Group node.
> +         */
>          for (i = 0; i < smmu_idmaps->len; i++) {
>              range = &g_array_index(smmu_idmaps, AcpiIortIdMapping, i);
> -            /* output IORT node is the smmuv3 node */
> +            /* Output IORT node is the SMMUv3 node. */
>              build_iort_id_mapping(table_data, range->input_base,
>                                    range->id_count, smmu_offset);
>          }
>  
> -        /* bypassed RIDs connect to ITS group node directly: RC -> ITS */
> +        /*
> +         * Map bypassed (don't go throught the SMMU) RIDs (input) to ITS Group
> +         * node directly: RC -> ITS.
> +         */
>          for (i = 0; i < its_idmaps->len; i++) {
>              range = &g_array_index(its_idmaps, AcpiIortIdMapping, i);
> -            /* output IORT node is the ITS group node (the first node) */
> +            /* Output IORT node is the ITS Group node (the first node). */
>              build_iort_id_mapping(table_data, range->input_base,
>                                    range->id_count, IORT_NODE_OFFSET);
>          }
>      } else {
> -        /* output IORT node is the ITS group node (the first node) */
> +        /*
> +         * Map all RIDs (input) to ITS Group node directly, since there is no
> +         * SMMU: RC -> ITS.
> +         * Output IORT node is the ITS Group node (the first node).
> +         */
>          build_iort_id_mapping(table_data, 0, 0x10000, IORT_NODE_OFFSET);
>      }
>  
Besides:
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Eric