RE: [PATCH v3 3/6] hw/arm/virt-acpi-build: Update IORT for multiple smmuv3 devices

[Shameerali Kolothum Thodi via <qemu-arm@nongnu.org>]

> -----Original Message-----
> From: Eric Auger <eric.auger@redhat.com>
> Sent: Thursday, June 5, 2025 10:58 AM
> To: Shameerali Kolothum Thodi
> <shameerali.kolothum.thodi@huawei.com>; qemu-arm@nongnu.org;
> qemu-devel@nongnu.org
> Cc: peter.maydell@linaro.org; jgg@nvidia.com; nicolinc@nvidia.com;
> ddutile@redhat.com; berrange@redhat.com; nathanc@nvidia.com;
> mochs@nvidia.com; smostafa@google.com; Linuxarm
> <linuxarm@huawei.com>; Wangzhou (B) <wangzhou1@hisilicon.com>;
> jiangkunkun <jiangkunkun@huawei.com>; Jonathan Cameron
> <jonathan.cameron@huawei.com>; zhangfei.gao@linaro.org
> Subject: Re: [PATCH v3 3/6] hw/arm/virt-acpi-build: Update IORT for
> multiple smmuv3 devices
> 
> Hi Shameer,
> On 6/2/25 5:41 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.thodi@huawei.com>
> > ---
> >  hw/arm/virt-acpi-build.c | 54
> > ++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 54 insertions(+)
> >
> > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index
> > bd26853ef6..b4ff71b8b7 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"
> > @@ -288,6 +289,54 @@ get_smmuv3_legacy_dev(VirtMachineState *vms,
> void *opaque)
> >      g_array_append_val(sdev_blob, sdev);  }
> >
> > +static int smmuv3_dev_idmap_compare(gconstpointer a, gconstpointer
> b)
> > +{
> > +    AcpiIortSMMUv3Dev *sdev_a = (AcpiIortSMMUv3Dev *)a;
> > +    AcpiIortSMMUv3Dev *sdev_b = (AcpiIortSMMUv3Dev *)b;
> > +    AcpiIortIdMapping *map_a = &g_array_index(sdev_a->idmaps,
> > +                                              AcpiIortIdMapping, 0);
> > +    AcpiIortIdMapping *map_b = &g_array_index(sdev_b->idmaps,
> > +                                              AcpiIortIdMapping, 0);
> > +    return map_a->input_base - map_b->input_base; }
> > +
> > +static int get_smmuv3_devices(Object *obj, void *opaque) {
> > +    VirtMachineState *vms = VIRT_MACHINE(qdev_get_machine());
> > +    GArray *sdev_blob = opaque;
> > +    AcpiIortIdMapping idmap;
> > +    PlatformBusDevice *pbus;
> > +    AcpiIortSMMUv3Dev sdev;
> > +    int min_bus, max_bus;
> > +    SysBusDevice *sbdev;
> > +    PCIBus *bus;
> > +
> > +    if (!object_dynamic_cast(obj, TYPE_ARM_SMMUV3)) {
> > +        return 0;
> > +    }
> > +
> > +    bus = PCI_BUS(object_property_get_link(obj, "primary-bus",
> &error_abort));
> > +    if (!bus) {
> > +        return 0;
> > +    }
> can it happen after patch 1?

Not really. Will remove.

> > +
> > +    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.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.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",
> > @@ -319,6 +368,11 @@ build_iort(GArray *table_data, BIOSLinker
> *linker, VirtMachineState *vms)
> >           */
> >          sdev = &g_array_index(smmuv3_devs, AcpiIortSMMUv3Dev, 0);
> >          g_array_sort(sdev->idmaps, iort_idmap_compare);
> > +    } else {
> > +        object_child_foreach_recursive(object_get_root(),
> > +                                       get_smmuv3_devices, smmuv3_devs);
> > +        /* Sort the smmuv3 devices(if any) by smmu idmap input_base */
> > +        g_array_sort(smmuv3_devs, smmuv3_dev_idmap_compare);
> Given the gorwing complexity of the IDMAP computation computation I
> would suggest to add some IDMAP examples either in comment or commit
> msg to illustrate how the idmaps look like (similar to what can be found in
> the IORT spec).

Ok. I will add some comments with example to describe the mappings.

Thanks,
Shameer