Re: [PATCH v5 5/9] hw/arm/virt-acpi-build: Factor out create_its_idmaps

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

Hi Gustavo,

On 6/23/25 3:57 PM, Gustavo Romero wrote:
> Factor out a new function, create_its_idmaps(), from the current
I would call it build_rc_its_idmap() to be clearer on what relationship
we build.
> build_iort code. Add proper comments to it clarifying how the ID ranges
> that go directly to the ITS Group node are computed based on the ones
> that go to the SMMU node.
>
> Suggested-by: Eric Auger <eric.auger@redhat.com>
> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
> ---
>  hw/arm/virt-acpi-build.c | 64 +++++++++++++++++++++++++---------------
>  1 file changed, 41 insertions(+), 23 deletions(-)
>
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index e9cd3fb351..40a782a498 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -266,6 +266,42 @@ static int iort_idmap_compare(gconstpointer a, gconstpointer b)
>      return idmap_a->input_base - idmap_b->input_base;
>  }
>  
> +/* Compute ID ranges (RIDs) from RC that do directly to the ITS Group node */
s/do/go
Ior use the spec terminology: that are directed to the ITS Group node
> +static void create_its_idmaps(GArray *its_idmaps, GArray *smmu_idmaps)
> +{
> +    AcpiIortIdMapping *idmap;
> +    AcpiIortIdMapping next_range = {0};
> +
> +    /*
> +     * Based on the RID ranges that go to the SMMU, determine the bypassed RID
same here
> +     * ranges, i.e., the ones that go directly to the ITS Group node, by
> +     * subtracting the SMMU-bound ranges from the full RID range, 0x0000–0xFFFF.
substracting
> +     */
> +     for (int i = 0; i < smmu_idmaps->len; i++) {
> +        idmap = &g_array_index(smmu_idmaps, AcpiIortIdMapping, i);
> +
> +        if (next_range.input_base < idmap->input_base) {
> +            next_range.id_count = idmap->input_base - next_range.input_base;
> +            g_array_append_val(its_idmaps, next_range);
> +        }
> +
> +        next_range.input_base = idmap->input_base + idmap->id_count;
> +    }
> +
> +    /*
> +     * Append the last RC -> ITS ID mapping.
> +     *
> +     * RIDs are 16-bit, according to the PCI Express 2.0 Base Specification, rev
> +     * 0.9, section 2.2.6.2, "Transaction Descriptor - Transaction ID Field",
> +     * hence, the end of the range is 0x10000.
> +     */
> +    if (next_range.input_base < 0x10000) {
> +        next_range.id_count = 0x10000 - next_range.input_base;
> +        g_array_append_val(its_idmaps, next_range);
> +    }
> +}
> +
> +
>  /*
>   * Input Output Remapping Table (IORT)
>   * Conforms to "IO Remapping Table System Software on ARM Platforms",
> @@ -276,7 +312,6 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>  {
>      int i, nb_nodes, rc_mapping_count;
>      size_t node_size, smmu_offset = 0;
> -    AcpiIortIdMapping *idmap;
>      uint32_t id = 0;
>      GArray *smmu_idmaps = g_array_new(false, true, sizeof(AcpiIortIdMapping));
>      GArray *its_idmaps = g_array_new(false, true, sizeof(AcpiIortIdMapping));
> @@ -287,34 +322,17 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>      acpi_table_begin(&table, table_data);
>  
>      if (vms->iommu == VIRT_IOMMU_SMMUV3) {
> -        AcpiIortIdMapping next_range = {0};
> -
>          object_child_foreach_recursive(object_get_root(),
>                                         iort_host_bridges, smmu_idmaps);
>  
>          /* Sort the smmu idmap by input_base */
>          g_array_sort(smmu_idmaps, iort_idmap_compare);
>  
> -        /*
> -         * Split the whole RIDs by mapping from RC to SMMU,
> -         * build the ID mapping from RC to ITS directly.
> -         */
> -        for (i = 0; i < smmu_idmaps->len; i++) {
> -            idmap = &g_array_index(smmu_idmaps, AcpiIortIdMapping, i);
> -
> -            if (next_range.input_base < idmap->input_base) {
> -                next_range.id_count = idmap->input_base - next_range.input_base;
> -                g_array_append_val(its_idmaps, next_range);
> -            }
> -
> -            next_range.input_base = idmap->input_base + idmap->id_count;
> -        }
> -
> -        /* Append the last RC -> ITS ID mapping */
> -        if (next_range.input_base < 0x10000) {
> -            next_range.id_count = 0x10000 - next_range.input_base;
> -            g_array_append_val(its_idmaps, next_range);
> -        }
> +	/*
> +	 * Knowing the ID ranges from the RC to the SMMU, it's possible to
> +	 * determine the ID ranges from RC that go directly to ITS.
are directed to
> +	 */
> +        create_its_idmaps(its_idmaps, smmu_idmaps);
>  
>          nb_nodes = 3; /* RC, ITS, SMMUv3 */
>          rc_mapping_count = smmu_idmaps->len + its_idmaps->len;
Thanks

Eric