Hi Shameer,
On 5/2/25 12:27 PM, Shameer Kolothum wrote:
> With the soon to be introduced user-creatable SMMUv3 devices for
> virt, it is possible to have multiple SMMUv3 devices associated
> with different PCIe root complexes.
>
> Update IORT nodes accordingly.
>
> Signed-off-by: Shameer Kolothum <shameerali.kolothum.th...@huawei.com>
> ---
> hw/arm/virt-acpi-build.c | 162 +++++++++++++++++++++++++++++++--------
I would recommend to split that patch. I think you can first introduce
the new struct and adapting the exicting code for the "legacy
instantiation mode" and then have a saparet patch for handling user
created instances. It will be easier to review.
At some point in the series you shall check the user is not attempting
to use legacy mode and user creatable mode concurrently. I don't know if
it is done yet.
> hw/arm/virt.c | 1 +
> include/hw/arm/virt.h | 1 +
> 3 files changed, 131 insertions(+), 33 deletions(-)
>
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 3ac8f8e178..3ce70f8fa9 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -43,6 +43,7 @@
> #include "hw/acpi/generic_event_device.h"
> #include "hw/acpi/tpm.h"
> #include "hw/acpi/hmat.h"
> +#include "hw/arm/smmuv3.h"
> #include "hw/pci/pcie_host.h"
> #include "hw/pci/pci.h"
> #include "hw/pci/pci_bus.h"
> @@ -266,6 +267,75 @@ static int iort_idmap_compare(gconstpointer a,
> gconstpointer b)
> return idmap_a->input_base - idmap_b->input_base;
> }
>
> +struct SMMUv3Device {
> + int irq;
> + hwaddr base;
> + GArray *smmu_idmaps;
> + size_t offset;
> +};
> +typedef struct SMMUv3Device SMMUv3Device;
> +
> +static int smmuv3_dev_idmap_compare(gconstpointer a, gconstpointer b)
> +{
> + SMMUv3Device *sdev_a = (SMMUv3Device *)a;
> + SMMUv3Device *sdev_b = (SMMUv3Device *)b;
> + AcpiIortIdMapping *map_a = &g_array_index(sdev_a->smmu_idmaps,
> + AcpiIortIdMapping, 0);
> + AcpiIortIdMapping *map_b = &g_array_index(sdev_b->smmu_idmaps,
> + AcpiIortIdMapping, 0);
> + return map_a->input_base - map_b->input_base;
> +}
> +
> +static void
> +get_smmuv3_legacy_dev(VirtMachineState *vms, GArray * smmuv3_devices)
> +{
> + SMMUv3Device sdev;
> +
> + sdev.smmu_idmaps = g_array_new(false, true, sizeof(AcpiIortIdMapping));
> + object_child_foreach_recursive(object_get_root(),
> + iort_host_bridges, sdev.smmu_idmaps);
> + sdev.base = vms->memmap[VIRT_SMMU].base;
> + sdev.irq = vms->irqmap[VIRT_SMMU] + ARM_SPI_BASE;
> + g_array_append_val(smmuv3_devices, sdev);
> +}
> +
> +static int get_smmuv3_devices(Object *obj, void *opaque)
> +{
> + PCIBus *bus;
> + SMMUv3Device sdev;
> + SysBusDevice *sbdev;
> + int min_bus, max_bus;
> + AcpiIortIdMapping idmap;
> + PlatformBusDevice *pbus;
> + GArray *sdev_blob = opaque;
> + VirtMachineState *vms = VIRT_MACHINE(qdev_get_machine());
> +
> + if (!object_dynamic_cast(obj, TYPE_ARM_SMMUV3)) {
> + return 0;
> + }
> +
> + bus = PCI_BUS(object_property_get_link(obj, "primary-bus",
> &error_abort));
> + if (!bus || pci_bus_bypass_iommu(bus)) {
can't we simply prevent a user creatable SMMU to be attached to
pci_bus_bypass_iommu(bus) ?
> + return 0;
> + }
> +
> + pbus = PLATFORM_BUS_DEVICE(vms->platform_bus_dev);
> + sbdev = SYS_BUS_DEVICE(obj);
> + sdev.base = platform_bus_get_mmio_addr(pbus, sbdev, 0);
> + sdev.base += vms->memmap[VIRT_PLATFORM_BUS].base;
> + sdev.irq = platform_bus_get_irqn(pbus, sbdev, 0);
> + sdev.irq += vms->irqmap[VIRT_PLATFORM_BUS];
> + sdev.irq += ARM_SPI_BASE;
> +
> + pci_bus_range(bus, &min_bus, &max_bus);
> + sdev.smmu_idmaps = g_array_new(false, true, sizeof(AcpiIortIdMapping));
> + idmap.input_base = min_bus << 8,
> + idmap.id_count = (max_bus - min_bus + 1) << 8,
> + g_array_append_val(sdev.smmu_idmaps, idmap);
> + g_array_append_val(sdev_blob, sdev);
> + return 0;
> +}
> +
> /*
> * Input Output Remapping Table (IORT)
> * Conforms to "IO Remapping Table System Software on ARM Platforms",
> @@ -274,11 +344,11 @@ 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;
> + size_t node_size;
> + int num_smmus = 0;
> uint32_t id = 0;
> - GArray *smmu_idmaps = g_array_new(false, true,
> sizeof(AcpiIortIdMapping));
> + GArray *smmuv3_devices = g_array_new(false, true, sizeof(SMMUv3Device));
> GArray *its_idmaps = g_array_new(false, true, sizeof(AcpiIortIdMapping));
>
> AcpiTable table = { .sig = "IORT", .rev = 3, .oem_id = vms->oem_id,
> @@ -286,28 +356,46 @@ 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};
> -
> + nb_nodes = 2; /* RC, ITS */
> + if (vms->legacy_smmuv3_present) {
> + get_smmuv3_legacy_dev(vms, smmuv3_devices);
> + /*
> + * There will be only one legacy SMMUv3 as it is a machine wide one.
> + * And since it covers all the PCIe RCs in the machine, may have
> + * multiple SMMUv3 idmaps. Sort it by input_base.
> + */
> + SMMUv3Device *s = &g_array_index(smmuv3_devices, SMMUv3Device, 0);
> + g_array_sort(s->smmu_idmaps, iort_idmap_compare);
> + } else {
> 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);
> + get_smmuv3_devices, smmuv3_devices);
> + /* Sort the smmuv3 devices(if any) by smmu idmap input_base */
> + g_array_sort(smmuv3_devices, smmuv3_dev_idmap_compare);
> + }
>
> + num_smmus = smmuv3_devices->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++) {
> + AcpiIortIdMapping *idmap;
extra line needed
> + SMMUv3Device *s = &g_array_index(smmuv3_devices, SMMUv3Device,
> i);
> +
> + for (j = 0; j < s->smmu_idmaps->len; j++) {
> + idmap = &g_array_index(s->smmu_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 +404,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 +428,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++) {
> + SMMUv3Device *s = &g_array_index(smmuv3_devices, SMMUv3Device, i);
> + int irq = s->irq;
>
> - smmu_offset = table_data->len - table.table_offset;
> + s->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 +443,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, s->base, 8);
> /* Flags */
> build_append_int_noprefix(table_data, 1 /* COHACC Override */, 4);
> build_append_int_noprefix(table_data, 0, 4); /* Reserved */
> @@ -404,15 +492,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++) {
> + SMMUv3Device *s = &g_array_index(smmuv3_devices, SMMUv3Device,
> i);
> +
> + for (j = 0; j < s->smmu_idmaps->len; j++) {
> + range = &g_array_index(s->smmu_idmaps, AcpiIortIdMapping, j);
> + /* output IORT node is the smmuv3 node */
> + build_iort_id_mapping(table_data, range->input_base,
> + range->id_count, s->offset);
> + }
> }
>
> /* bypassed RIDs connect to ITS group node directly: RC -> ITS */
> @@ -428,8 +520,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++) {
> + SMMUv3Device *s = &g_array_index(smmuv3_devices, SMMUv3Device, i);
> + g_array_free(s->smmu_idmaps, true);
> + }
> + g_array_free(smmuv3_devices, true);
> }
>
> /*
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 3bae4e374f..dd355f4454 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1620,6 +1620,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 c8e94e6aed..2e2867c906 100644
> --- a/include/hw/arm/virt.h
> +++ b/include/hw/arm/virt.h
> @@ -180,6 +180,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)
Eric
