Hi Shameer,
Some cosmetic comments below.
On 6/13/25 4:44 PM, Shameer Kolothum wrote:
> Introduces a new struct AcpiIortSMMUv3Dev to hold all the information
Introduce
> required for SMMUv3 IORT node and use that for populating the node.
>
> The current machine wide SMMUv3 is named as legacy SMMUv3 as we will
> soon add support for user-creatable SMMUv3 devices. These changes will
> be useful to have common code paths when we add that support.
>
> Tested-by: Nathan Chen <nath...@nvidia.com>
> Signed-off-by: Shameer Kolothum <shameerali.kolothum.th...@huawei.com>
> ---
> hw/arm/virt-acpi-build.c | 111 +++++++++++++++++++++++++++------------
> hw/arm/virt.c | 1 +
> include/hw/arm/virt.h | 1 +
> 3 files changed, 79 insertions(+), 34 deletions(-)
>
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 7e8e0f0298..d39506179a 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -266,6 +266,36 @@ static int iort_idmap_compare(gconstpointer a,
> gconstpointer b)
> return idmap_a->input_base - idmap_b->input_base;
> }
>
> +struct AcpiIortSMMUv3Dev {
> + int irq;
> + hwaddr base;
> + GArray *idmaps;
> + /* Offset of the SMMUv3 IORT Node relative to the start of the IORT. */
remove "."?
> + size_t offset;
> +};
> +typedef struct AcpiIortSMMUv3Dev AcpiIortSMMUv3Dev;
> +
> +static void
> +populate_smmuv3_legacy_dev(GArray *sdev_blob)
> +{
> + VirtMachineState *vms = VIRT_MACHINE(qdev_get_machine());
> + AcpiIortSMMUv3Dev sdev;
> +
> + sdev.idmaps = g_array_new(false, true, sizeof(AcpiIortIdMapping));
> + object_child_foreach_recursive(object_get_root(),
> + iort_host_bridges, sdev.idmaps);
> +
> + /*
> + * There will be only one legacy SMMUv3 as it is a machine wide one.
there can be only one "legacy" machine wide SMMUv3?
> + * And since it covers all the PCIe RCs in the machine, may have
nit: Remove And. I would slighly rephrase into
"Since it may cover several PCIe RCs ..."
Some may be bypassed.
> + * multiple SMMUv3 idmaps. Sort it by input_base.
> + */
> + g_array_sort(sdev.idmaps, iort_idmap_compare);
> + sdev.base = vms->memmap[VIRT_SMMU].base;
> + sdev.irq = vms->irqmap[VIRT_SMMU] + ARM_SPI_BASE;
> + g_array_append_val(sdev_blob, sdev);
> +}
> +
> /*
> * Input Output Remapping Table (IORT)
> * Conforms to "IO Remapping Table System Software on ARM Platforms",
> @@ -274,11 +304,12 @@ static int iort_idmap_compare(gconstpointer a,
> gconstpointer b)
> static void
> 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;
> + int i, j, nb_nodes, rc_mapping_count;
> + AcpiIortSMMUv3Dev *sdev;
> + size_t node_size;
> + int num_smmus = 0;
> uint32_t id = 0;
> - GArray *smmu_idmaps = g_array_new(false, true,
> sizeof(AcpiIortIdMapping));
> + GArray *smmuv3_devs = g_array_new(false, true,
> sizeof(AcpiIortSMMUv3Dev));
> GArray *its_idmaps = g_array_new(false, true, sizeof(AcpiIortIdMapping));
>
> AcpiTable table = { .sig = "IORT", .rev = 3, .oem_id = vms->oem_id,
> @@ -286,28 +317,32 @@ build_iort(GArray *table_data, BIOSLinker *linker,
> VirtMachineState *vms)
> /* Table 2 The IORT */
> 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);
> + nb_nodes = 2; /* RC, ITS */
> + if (vms->legacy_smmuv3_present) {
> + populate_smmuv3_legacy_dev(smmuv3_devs);
> + }
>
> + num_smmus = smmuv3_devs->len;
> + if (num_smmus) {
> + AcpiIortIdMapping next_range = {0};
> + int smmu_map_cnt = 0;
> /*
> * 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);
> + for (i = 0; i < num_smmus; i++) {
> + sdev = &g_array_index(smmuv3_devs, AcpiIortSMMUv3Dev, i);
> + for (j = 0; j < sdev->idmaps->len; j++) {
> + AcpiIortIdMapping *idmap = &g_array_index(sdev->idmaps,
> + AcpiIortIdMapping,
> j);
> + 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;
> + smmu_map_cnt++;
> }
> -
> - next_range.input_base = idmap->input_base + idmap->id_count;
> }
>
> /* Append the last RC -> ITS ID mapping */
> @@ -316,10 +351,9 @@ build_iort(GArray *table_data, BIOSLinker *linker,
> VirtMachineState *vms)
> g_array_append_val(its_idmaps, next_range);
> }
>
> - nb_nodes = 3; /* RC, ITS, SMMUv3 */
> - rc_mapping_count = smmu_idmaps->len + its_idmaps->len;
> + nb_nodes += num_smmus;
> + rc_mapping_count = smmu_map_cnt + its_idmaps->len;
> } else {
> - nb_nodes = 2; /* RC, ITS */
> rc_mapping_count = 1;
> }
> /* Number of IORT Nodes */
> @@ -341,10 +375,11 @@ build_iort(GArray *table_data, BIOSLinker *linker,
> VirtMachineState *vms)
> /* GIC ITS Identifier Array */
> build_append_int_noprefix(table_data, 0 /* MADT translation_id */, 4);
>
> - if (vms->iommu == VIRT_IOMMU_SMMUV3) {
> - int irq = vms->irqmap[VIRT_SMMU] + ARM_SPI_BASE;
> + for (i = 0; i < num_smmus; i++) {
> + sdev = &g_array_index(smmuv3_devs, AcpiIortSMMUv3Dev, i);
> + int irq = sdev->irq;
>
> - smmu_offset = table_data->len - table.table_offset;
> + sdev->offset = table_data->len - table.table_offset;
> /* Table 9 SMMUv3 Format */
> build_append_int_noprefix(table_data, 4 /* SMMUv3 */, 1); /* Type */
> node_size = SMMU_V3_ENTRY_SIZE + ID_MAPPING_ENTRY_SIZE;
> @@ -355,7 +390,7 @@ build_iort(GArray *table_data, BIOSLinker *linker,
> VirtMachineState *vms)
> /* Reference to ID Array */
> build_append_int_noprefix(table_data, SMMU_V3_ENTRY_SIZE, 4);
> /* Base address */
> - build_append_int_noprefix(table_data, vms->memmap[VIRT_SMMU].base,
> 8);
> + build_append_int_noprefix(table_data, sdev->base, 8);
> /* Flags */
> build_append_int_noprefix(table_data, 1 /* COHACC Override */, 4);
> build_append_int_noprefix(table_data, 0, 4); /* Reserved */
> @@ -404,15 +439,19 @@ build_iort(GArray *table_data, BIOSLinker *linker,
> VirtMachineState *vms)
> build_append_int_noprefix(table_data, 0, 3); /* Reserved */
>
> /* Output Reference */
> - if (vms->iommu == VIRT_IOMMU_SMMUV3) {
> + if (num_smmus) {
> AcpiIortIdMapping *range;
>
> /* translated RIDs connect to SMMUv3 node: RC -> SMMUv3 -> ITS */
> - for (i = 0; i < smmu_idmaps->len; i++) {
> - range = &g_array_index(smmu_idmaps, AcpiIortIdMapping, i);
> - /* output IORT node is the smmuv3 node */
> - build_iort_id_mapping(table_data, range->input_base,
> - range->id_count, smmu_offset);
> + for (i = 0; i < num_smmus; i++) {
> + sdev = &g_array_index(smmuv3_devs, AcpiIortSMMUv3Dev, i);
> +
> + for (j = 0; j < sdev->idmaps->len; j++) {
> + range = &g_array_index(sdev->idmaps, AcpiIortIdMapping, j);
> + /* output IORT node is the smmuv3 node */
> + build_iort_id_mapping(table_data, range->input_base,
> + range->id_count, sdev->offset);
> + }
> }
>
> /* bypassed RIDs connect to ITS group node directly: RC -> ITS */
> @@ -428,8 +467,12 @@ build_iort(GArray *table_data, BIOSLinker *linker,
> VirtMachineState *vms)
> }
>
> acpi_table_end(linker, &table);
> - g_array_free(smmu_idmaps, true);
> g_array_free(its_idmaps, true);
> + for (i = 0; i < num_smmus; i++) {
> + sdev = &g_array_index(smmuv3_devs, AcpiIortSMMUv3Dev, i);
> + g_array_free(sdev->idmaps, true);
> + }
> + g_array_free(smmuv3_devs, true);
> }
>
> /*
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 9a6cd085a3..73bd2bd5f2 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1614,6 +1614,7 @@ static void create_pcie(VirtMachineState *vms)
> create_smmu(vms, vms->bus);
> qemu_fdt_setprop_cells(ms->fdt, nodename, "iommu-map",
> 0x0, vms->iommu_phandle, 0x0, 0x10000);
> + vms->legacy_smmuv3_present = true;
> break;
> default:
> g_assert_not_reached();
> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> index 9a1b0f53d2..8b1404b5f6 100644
> --- a/include/hw/arm/virt.h
> +++ b/include/hw/arm/virt.h
> @@ -174,6 +174,7 @@ struct VirtMachineState {
> char *oem_id;
> char *oem_table_id;
> bool ns_el2_virt_timer_irq;
> + bool legacy_smmuv3_present;
> };
>
> #define VIRT_ECAM_ID(high) (high ? VIRT_HIGH_PCIE_ECAM : VIRT_PCIE_ECAM)
Thanks
Eric
