From: Nicolin Chen <nicolinc@nvidia.com>

Add ACPI DSDT support for Tegra241 CMDQV when the SMMUv3 instance is
created with tegra241-cmdqv=on.

Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
---
 hw/arm/trace-events      |  1 +
 hw/arm/virt-acpi-build.c | 74 ++++++++++++++++++++++++++++++++++++++++
 include/hw/arm/virt.h    |  2 ++
 3 files changed, 77 insertions(+)

diff --git a/hw/arm/trace-events b/hw/arm/trace-events
index ef495c040c..e7e3ccfe9f 100644
--- a/hw/arm/trace-events
+++ b/hw/arm/trace-events
@@ -9,6 +9,7 @@ omap1_lpg_led(const char *onoff) "omap1 LPG: LED is %s"
 
 # virt-acpi-build.c
 virt_acpi_setup(void) "No fw cfg or ACPI disabled. Bailing out."
+virt_acpi_dsdt_tegra241_cmdqv(int smmu_id, uint64_t base, uint32_t irq) "DSDT: add cmdqv node for (id=%d), base=0x%" PRIx64 ", irq=%d"
 
 # smmu-common.c
 smmu_add_mr(const char *name) "%s"
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 4f8d36dae0..11494b29ad 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -1115,6 +1115,78 @@ static void build_fadt_rev6(GArray *table_data, BIOSLinker *linker,
     build_fadt(table_data, linker, &fadt, vms->oem_id, vms->oem_table_id);
 }
 
+static int smmuv3_cmdqv_devices(Object *obj, void *opaque)
+{
+    VirtMachineState *vms = VIRT_MACHINE(qdev_get_machine());
+    GArray *sdev_blob = opaque;
+    PlatformBusDevice *pbus;
+    AcpiSMMUv3Dev sdev;
+    SysBusDevice *sbdev;
+
+    if (!object_dynamic_cast(obj, TYPE_ARM_SMMUV3)) {
+        return 0;
+    }
+
+    if (!object_property_get_bool(obj, "tegra241-cmdqv", NULL)) {
+        return 0;
+    }
+
+    pbus = PLATFORM_BUS_DEVICE(vms->platform_bus_dev);
+    sbdev = SYS_BUS_DEVICE(obj);
+    sdev.base = platform_bus_get_mmio_addr(pbus, sbdev, 1);
+    sdev.base += vms->memmap[VIRT_PLATFORM_BUS].base;
+    sdev.irq = platform_bus_get_irqn(pbus, sbdev, NUM_SMMU_IRQS);
+    sdev.irq += vms->irqmap[VIRT_PLATFORM_BUS];
+    sdev.irq += ARM_SPI_BASE;
+    g_array_append_val(sdev_blob, sdev);
+    return 0;
+}
+
+static void acpi_dsdt_add_tegra241_cmdqv(Aml *scope, VirtMachineState *vms)
+{
+    GArray *smmuv3_devs = g_array_new(false, true, sizeof(AcpiSMMUv3Dev));
+    int i;
+
+    if (vms->legacy_smmuv3_present) {
+        return;
+    }
+
+    object_child_foreach_recursive(object_get_root(), smmuv3_cmdqv_devices,
+                                   smmuv3_devs);
+
+    for (i = 0; i < smmuv3_devs->len; i++) {
+        uint32_t identifier = i;
+        AcpiSMMUv3Dev *sdev;
+        Aml *dev, *crs, *addr;
+
+        sdev = &g_array_index(smmuv3_devs, AcpiSMMUv3Dev, i);
+
+        dev = aml_device("CV%.02u", identifier);
+        aml_append(dev, aml_name_decl("_HID", aml_string("NVDA200C")));
+        if (vms->its) {
+            identifier++;
+        }
+        aml_append(dev, aml_name_decl("_UID", aml_int(identifier)));
+        aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
+
+        crs = aml_resource_template();
+        addr = aml_qword_memory(AML_POS_DECODE, AML_MIN_FIXED, AML_MAX_FIXED,
+                                AML_CACHEABLE, AML_READ_WRITE, 0x0, sdev->base,
+                                sdev->base + TEGRA241_CMDQV_IO_LEN - 0x1, 0x0,
+                                TEGRA241_CMDQV_IO_LEN);
+        aml_append(crs, addr);
+        aml_append(crs, aml_interrupt(AML_CONSUMER, AML_EDGE,
+                                      AML_ACTIVE_HIGH, AML_EXCLUSIVE,
+                                      (uint32_t *)&sdev->irq, 1));
+        aml_append(dev, aml_name_decl("_CRS", crs));
+
+        aml_append(scope, dev);
+
+        trace_virt_acpi_dsdt_tegra241_cmdqv(identifier, sdev->base, sdev->irq);
+    }
+    g_array_free(smmuv3_devs, true);
+}
+
 /* DSDT */
 static void
 build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
@@ -1179,6 +1251,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
     acpi_dsdt_add_tpm(scope, vms);
 #endif
 
+    acpi_dsdt_add_tegra241_cmdqv(scope, vms);
+
     aml_append(dsdt, scope);
 
     pci0_scope = aml_scope("\\_SB.PCI0");
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index efbc1758c5..842143cc85 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -46,6 +46,8 @@
 #define NUM_VIRTIO_TRANSPORTS 32
 #define NUM_SMMU_IRQS          4
 
+#define TEGRA241_CMDQV_IO_LEN 0x50000
+
 /* See Linux kernel arch/arm64/include/asm/pvclock-abi.h */
 #define PVTIME_SIZE_PER_CPU 64
 
-- 
2.43.0

On 12/10/25 2:37 PM, Shameer Kolothum wrote:
> From: Nicolin Chen <nicolinc@nvidia.com>
>
> Add ACPI DSDT support for Tegra241 CMDQV when the SMMUv3 instance is
> created with tegra241-cmdqv=on.

Pls can you give a point to the DSDT spec to understand why this is
needed and what needs to be added.
>
> Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
> Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
> ---
>  hw/arm/trace-events      |  1 +
>  hw/arm/virt-acpi-build.c | 74 ++++++++++++++++++++++++++++++++++++++++
>  include/hw/arm/virt.h    |  2 ++
>  3 files changed, 77 insertions(+)
>
> diff --git a/hw/arm/trace-events b/hw/arm/trace-events
> index ef495c040c..e7e3ccfe9f 100644
> --- a/hw/arm/trace-events
> +++ b/hw/arm/trace-events
> @@ -9,6 +9,7 @@ omap1_lpg_led(const char *onoff) "omap1 LPG: LED is %s"
>  
>  # virt-acpi-build.c
>  virt_acpi_setup(void) "No fw cfg or ACPI disabled. Bailing out."
> +virt_acpi_dsdt_tegra241_cmdqv(int smmu_id, uint64_t base, uint32_t irq) "DSDT: add cmdqv node for (id=%d), base=0x%" PRIx64 ", irq=%d"
>  
>  # smmu-common.c
>  smmu_add_mr(const char *name) "%s"
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 4f8d36dae0..11494b29ad 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -1115,6 +1115,78 @@ static void build_fadt_rev6(GArray *table_data, BIOSLinker *linker,
>      build_fadt(table_data, linker, &fadt, vms->oem_id, vms->oem_table_id);
>  }
>  
> +static int smmuv3_cmdqv_devices(Object *obj, void *opaque)
> +{
> +    VirtMachineState *vms = VIRT_MACHINE(qdev_get_machine());
> +    GArray *sdev_blob = opaque;
> +    PlatformBusDevice *pbus;
> +    AcpiSMMUv3Dev sdev;
> +    SysBusDevice *sbdev;
> +
> +    if (!object_dynamic_cast(obj, TYPE_ARM_SMMUV3)) {
> +        return 0;
> +    }
> +
> +    if (!object_property_get_bool(obj, "tegra241-cmdqv", NULL)) {
> +        return 0;
> +    }
> +
> +    pbus = PLATFORM_BUS_DEVICE(vms->platform_bus_dev);
> +    sbdev = SYS_BUS_DEVICE(obj);
> +    sdev.base = platform_bus_get_mmio_addr(pbus, sbdev, 1);
> +    sdev.base += vms->memmap[VIRT_PLATFORM_BUS].base;
> +    sdev.irq = platform_bus_get_irqn(pbus, sbdev, NUM_SMMU_IRQS);
> +    sdev.irq += vms->irqmap[VIRT_PLATFORM_BUS];
> +    sdev.irq += ARM_SPI_BASE;
> +    g_array_append_val(sdev_blob, sdev);
> +    return 0;
> +}
> +
> +static void acpi_dsdt_add_tegra241_cmdqv(Aml *scope, VirtMachineState *vms)
> +{
> +    GArray *smmuv3_devs = g_array_new(false, true, sizeof(AcpiSMMUv3Dev));
> +    int i;
> +
> +    if (vms->legacy_smmuv3_present) {
> +        return;
> +    }
> +
> +    object_child_foreach_recursive(object_get_root(), smmuv3_cmdqv_devices,
> +                                   smmuv3_devs);
> +
> +    for (i = 0; i < smmuv3_devs->len; i++) {
> +        uint32_t identifier = i;
> +        AcpiSMMUv3Dev *sdev;
> +        Aml *dev, *crs, *addr;
> +
> +        sdev = &g_array_index(smmuv3_devs, AcpiSMMUv3Dev, i);
> +
> +        dev = aml_device("CV%.02u", identifier);
> +        aml_append(dev, aml_name_decl("_HID", aml_string("NVDA200C")));
> +        if (vms->its) {
> +            identifier++;
> +        }
> +        aml_append(dev, aml_name_decl("_UID", aml_int(identifier)));
> +        aml_append(dev, aml_name_decl("_CCA", aml_int(1)));
> +
> +        crs = aml_resource_template();
> +        addr = aml_qword_memory(AML_POS_DECODE, AML_MIN_FIXED, AML_MAX_FIXED,
> +                                AML_CACHEABLE, AML_READ_WRITE, 0x0, sdev->base,
> +                                sdev->base + TEGRA241_CMDQV_IO_LEN - 0x1, 0x0,
> +                                TEGRA241_CMDQV_IO_LEN);
> +        aml_append(crs, addr);
> +        aml_append(crs, aml_interrupt(AML_CONSUMER, AML_EDGE,
> +                                      AML_ACTIVE_HIGH, AML_EXCLUSIVE,
> +                                      (uint32_t *)&sdev->irq, 1));
> +        aml_append(dev, aml_name_decl("_CRS", crs));
> +
> +        aml_append(scope, dev);
> +
> +        trace_virt_acpi_dsdt_tegra241_cmdqv(identifier, sdev->base, sdev->irq);
> +    }
> +    g_array_free(smmuv3_devs, true);
> +}
> +
>  /* DSDT */
>  static void
>  build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
> @@ -1179,6 +1251,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>      acpi_dsdt_add_tpm(scope, vms);
>  #endif
>  
> +    acpi_dsdt_add_tegra241_cmdqv(scope, vms);
> +
>      aml_append(dsdt, scope);
>  
>      pci0_scope = aml_scope("\\_SB.PCI0");
> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> index efbc1758c5..842143cc85 100644
> --- a/include/hw/arm/virt.h
> +++ b/include/hw/arm/virt.h
> @@ -46,6 +46,8 @@
>  #define NUM_VIRTIO_TRANSPORTS 32
>  #define NUM_SMMU_IRQS          4
>  
> +#define TEGRA241_CMDQV_IO_LEN 0x50000
isn't it already defined elsewhere, use the corresponding include?

Eric
> +
>  /* See Linux kernel arch/arm64/include/asm/pvclock-abi.h */
>  #define PVTIME_SIZE_PER_CPU 64
>

On Mon, Jan 26, 2026 at 04:28:24PM +0100, Eric Auger wrote:
> On 12/10/25 2:37 PM, Shameer Kolothum wrote:
> > From: Nicolin Chen <nicolinc@nvidia.com>
> >
> > Add ACPI DSDT support for Tegra241 CMDQV when the SMMUv3 instance is
> > created with tegra241-cmdqv=on.
> 
> Pls can you give a point to the DSDT spec to understand why this is
> needed and what needs to be added.

This was an implementation decision by ARM that CMDQV node resides
in DSDT v.s. IORT.

Kernel driver scans DSDT unconditionally:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c#n4572

NVIDIA also has a public DSDT:
https://github.com/NVIDIA/edk2-nvidia/blob/main/Silicon/NVIDIA/Drivers/ConfigurationManagerData/AcpiTableList/PlatformASLTablesLib/Dsdt_TH500.asl#L1005

I don't think there is a DSDT spec though...

Nicolin

On Wed, Dec 10, 2025 at 01:37:36PM +0000, Shameer Kolothum wrote:
> +static int smmuv3_cmdqv_devices(Object *obj, void *opaque)
> +{
> +    VirtMachineState *vms = VIRT_MACHINE(qdev_get_machine());
> +    GArray *sdev_blob = opaque;
> +    PlatformBusDevice *pbus;
> +    AcpiSMMUv3Dev sdev;
> +    SysBusDevice *sbdev;
> +
> +    if (!object_dynamic_cast(obj, TYPE_ARM_SMMUV3)) {
> +        return 0;
> +    }
> +
> +    if (!object_property_get_bool(obj, "tegra241-cmdqv", NULL)) {
> +        return 0;
> +    }
> +
> +    pbus = PLATFORM_BUS_DEVICE(vms->platform_bus_dev);
> +    sbdev = SYS_BUS_DEVICE(obj);
> +    sdev.base = platform_bus_get_mmio_addr(pbus, sbdev, 1);
> +    sdev.base += vms->memmap[VIRT_PLATFORM_BUS].base;
> +    sdev.irq = platform_bus_get_irqn(pbus, sbdev, NUM_SMMU_IRQS);
> +    sdev.irq += vms->irqmap[VIRT_PLATFORM_BUS];
> +    sdev.irq += ARM_SPI_BASE;
> +    g_array_append_val(sdev_blob, sdev);
> +    return 0;

This is pre-building SMMU's IORT nodes right? Maybe a different
naming? And can be shared with the existing iort_smmuv3_devices?

We do so, because we need to link CMDQV's DSDT node to the SMMU's
IORT node but it is created in build_iort() that is called after
build_dsdt().

Let's explain why we have this pre-building.

Nicolin

> -----Original Message-----
> From: Nicolin Chen <nicolinc@nvidia.com>
> Sent: 29 December 2025 20:11
> To: Shameer Kolothum <skolothumtho@nvidia.com>
> Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org;
> eric.auger@redhat.com; peter.maydell@linaro.org; Nathan Chen
> <nathanc@nvidia.com>; Matt Ochs <mochs@nvidia.com>; Jason Gunthorpe
> <jgg@nvidia.com>; jonathan.cameron@huawei.com;
> zhangfei.gao@linaro.org; zhenzhong.duan@intel.com; Krishnakant Jaju
> <kjaju@nvidia.com>
> Subject: Re: [RFC PATCH 15/16] hw/arm/virt-acpi: Advertise Tegra241
> CMDQV nodes in DSDT
> 
> On Wed, Dec 10, 2025 at 01:37:36PM +0000, Shameer Kolothum wrote:
> > +static int smmuv3_cmdqv_devices(Object *obj, void *opaque)
> > +{
> > +    VirtMachineState *vms = VIRT_MACHINE(qdev_get_machine());
> > +    GArray *sdev_blob = opaque;
> > +    PlatformBusDevice *pbus;
> > +    AcpiSMMUv3Dev sdev;
> > +    SysBusDevice *sbdev;
> > +
> > +    if (!object_dynamic_cast(obj, TYPE_ARM_SMMUV3)) {
> > +        return 0;
> > +    }
> > +
> > +    if (!object_property_get_bool(obj, "tegra241-cmdqv", NULL)) {
> > +        return 0;
> > +    }
> > +
> > +    pbus = PLATFORM_BUS_DEVICE(vms->platform_bus_dev);
> > +    sbdev = SYS_BUS_DEVICE(obj);
> > +    sdev.base = platform_bus_get_mmio_addr(pbus, sbdev, 1);
> > +    sdev.base += vms->memmap[VIRT_PLATFORM_BUS].base;
> > +    sdev.irq = platform_bus_get_irqn(pbus, sbdev, NUM_SMMU_IRQS);
> > +    sdev.irq += vms->irqmap[VIRT_PLATFORM_BUS];
> > +    sdev.irq += ARM_SPI_BASE;
> > +    g_array_append_val(sdev_blob, sdev);
> > +    return 0;
> 
> This is pre-building SMMU's IORT nodes right? Maybe a different
> naming? And can be shared with the existing iort_smmuv3_devices?

Not really, if you are referring about this patch here,
https://github.com/NVIDIA/QEMU/commit/cc3b65e6a49a9b7addf44b377d4ef1de99bfee3f

I didn't find a clean way to store the pre-built smmuv3_devs other than
placing them in struct AcpiBuildTables. It's not clear we gain much by
restructuring things to populate and store them separately. At best,
it might slightly improve boot time, and if that becomes important we
can always add it as an optimization later.

This patch enumerates SMMUv3 accel instances with CMDQV separately, uses
that information to build the DSDT, and then frees the device array.

> 
> We do so, because we need to link CMDQV's DSDT node to the SMMU's
> IORT node but it is created in build_iort() that is called after
> build_dsdt().

Hmm..not sure I get that. Does this mean IORT has a link to DSDT? I can't find one..
Maybe I missed. Please let me know.

Thanks,
Shameer

On Tue, Dec 30, 2025 at 02:13:12AM -0800, Shameer Kolothum wrote:
> 
> 
> > -----Original Message-----
> > From: Nicolin Chen <nicolinc@nvidia.com>
> > Sent: 29 December 2025 20:11
> > To: Shameer Kolothum <skolothumtho@nvidia.com>
> > Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org;
> > eric.auger@redhat.com; peter.maydell@linaro.org; Nathan Chen
> > <nathanc@nvidia.com>; Matt Ochs <mochs@nvidia.com>; Jason Gunthorpe
> > <jgg@nvidia.com>; jonathan.cameron@huawei.com;
> > zhangfei.gao@linaro.org; zhenzhong.duan@intel.com; Krishnakant Jaju
> > <kjaju@nvidia.com>
> > Subject: Re: [RFC PATCH 15/16] hw/arm/virt-acpi: Advertise Tegra241
> > CMDQV nodes in DSDT
> > 
> > On Wed, Dec 10, 2025 at 01:37:36PM +0000, Shameer Kolothum wrote:
> > > +static int smmuv3_cmdqv_devices(Object *obj, void *opaque)
> > > +{
> > > +    VirtMachineState *vms = VIRT_MACHINE(qdev_get_machine());
> > > +    GArray *sdev_blob = opaque;
> > > +    PlatformBusDevice *pbus;
> > > +    AcpiSMMUv3Dev sdev;
> > > +    SysBusDevice *sbdev;
> > > +
> > > +    if (!object_dynamic_cast(obj, TYPE_ARM_SMMUV3)) {
> > > +        return 0;
> > > +    }
> > > +
> > > +    if (!object_property_get_bool(obj, "tegra241-cmdqv", NULL)) {
> > > +        return 0;
> > > +    }
> > > +
> > > +    pbus = PLATFORM_BUS_DEVICE(vms->platform_bus_dev);
> > > +    sbdev = SYS_BUS_DEVICE(obj);
> > > +    sdev.base = platform_bus_get_mmio_addr(pbus, sbdev, 1);
> > > +    sdev.base += vms->memmap[VIRT_PLATFORM_BUS].base;
> > > +    sdev.irq = platform_bus_get_irqn(pbus, sbdev, NUM_SMMU_IRQS);
> > > +    sdev.irq += vms->irqmap[VIRT_PLATFORM_BUS];
> > > +    sdev.irq += ARM_SPI_BASE;
> > > +    g_array_append_val(sdev_blob, sdev);
> > > +    return 0;
> > 
> > This is pre-building SMMU's IORT nodes right? Maybe a different
> > naming? And can be shared with the existing iort_smmuv3_devices?
> 
> Not really, if you are referring about this patch here,
> https://github.com/NVIDIA/QEMU/commit/cc3b65e6a49a9b7addf44b377d4ef1de99bfee3f

Yes. I am talking about this. But you are actually pre-building an
ACPI SMMU device array here. So, my question wasn't accurate..
 
> I didn’t find a clean way to store the pre-built smmuv3_devs other than
> placing them in struct AcpiBuildTables. It’s not clear we gain much by
> restructuring things to populate and store them separately. At best,
> it might slightly improve boot time, and if that becomes important we
> can always add it as an optimization later.
> 
> This patch enumerates SMMUv3 accel instances with CMDQV separately, uses
> that information to build the DSDT, and then frees the device array.

Oh, I missed the g_array_free() in acpi_dsdt_add_tegra241_cmdqv().

Let's have a note inline and copy to the commit message too.

> > We do so, because we need to link CMDQV's DSDT node to the SMMU's
> > IORT node but it is created in build_iort() that is called after
> > build_dsdt().
> 
> Hmm..not sure I get that. Does this mean IORT has a link to DSDT? I can't find one..
> Maybe I missed. Please let me know.

The _UID field of the DSDT node links to the Identifier field of an
SMMU's IORT node. E.g.

[DSDT] // Qemu builds DSDT first
CMDQV0 {
    _UID = 0;
}

[IORT] // Then builds IORT
SMMU0 {
    Identifier = 0;
}

If we are sure that the SMMU sequence is fixed every time we rebuild
the sdev array so that IORT.Identifier matches with DSDT._UID, we'd
be fine. That being said, we should probably make a node inline too.

Otherwise, it would be safer to pre-build the sdev array and use the
same array in both DSDT and IORT.

Thanks
Nicolin

On Tue, Dec 30, 2025 at 10:11:20AM -0800, Nicolin Chen wrote:
> On Tue, Dec 30, 2025 at 02:13:12AM -0800, Shameer Kolothum wrote:
> > 
> > 
> > > -----Original Message-----
> > > From: Nicolin Chen <nicolinc@nvidia.com>
> > > Sent: 29 December 2025 20:11
> > > To: Shameer Kolothum <skolothumtho@nvidia.com>
> > > Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org;
> > > eric.auger@redhat.com; peter.maydell@linaro.org; Nathan Chen
> > > <nathanc@nvidia.com>; Matt Ochs <mochs@nvidia.com>; Jason Gunthorpe
> > > <jgg@nvidia.com>; jonathan.cameron@huawei.com;
> > > zhangfei.gao@linaro.org; zhenzhong.duan@intel.com; Krishnakant Jaju
> > > <kjaju@nvidia.com>
> > > Subject: Re: [RFC PATCH 15/16] hw/arm/virt-acpi: Advertise Tegra241
> > > CMDQV nodes in DSDT
> > > 
> > > On Wed, Dec 10, 2025 at 01:37:36PM +0000, Shameer Kolothum wrote:
> > > > +static int smmuv3_cmdqv_devices(Object *obj, void *opaque)
> > > > +{
> > > > +    VirtMachineState *vms = VIRT_MACHINE(qdev_get_machine());
> > > > +    GArray *sdev_blob = opaque;
> > > > +    PlatformBusDevice *pbus;
> > > > +    AcpiSMMUv3Dev sdev;
> > > > +    SysBusDevice *sbdev;
> > > > +
> > > > +    if (!object_dynamic_cast(obj, TYPE_ARM_SMMUV3)) {
> > > > +        return 0;
> > > > +    }
> > > > +
> > > > +    if (!object_property_get_bool(obj, "tegra241-cmdqv", NULL)) {
> > > > +        return 0;
> > > > +    }
> > > > +
> > > > +    pbus = PLATFORM_BUS_DEVICE(vms->platform_bus_dev);
> > > > +    sbdev = SYS_BUS_DEVICE(obj);
> > > > +    sdev.base = platform_bus_get_mmio_addr(pbus, sbdev, 1);
> > > > +    sdev.base += vms->memmap[VIRT_PLATFORM_BUS].base;
> > > > +    sdev.irq = platform_bus_get_irqn(pbus, sbdev, NUM_SMMU_IRQS);
> > > > +    sdev.irq += vms->irqmap[VIRT_PLATFORM_BUS];
> > > > +    sdev.irq += ARM_SPI_BASE;
> > > > +    g_array_append_val(sdev_blob, sdev);
> > > > +    return 0;
> > > 
> > > This is pre-building SMMU's IORT nodes right? Maybe a different
> > > naming? And can be shared with the existing iort_smmuv3_devices?
> > 
> > Not really, if you are referring about this patch here,
> > https://github.com/NVIDIA/QEMU/commit/cc3b65e6a49a9b7addf44b377d4ef1de99bfee3f
> 
> Yes. I am talking about this. But you are actually pre-building an
> ACPI SMMU device array here. So, my question wasn't accurate..
>  
> > I didn’t find a clean way to store the pre-built smmuv3_devs other than
> > placing them in struct AcpiBuildTables. It’s not clear we gain much by
> > restructuring things to populate and store them separately. At best,
> > it might slightly improve boot time, and if that becomes important we
> > can always add it as an optimization later.
> > 
> > This patch enumerates SMMUv3 accel instances with CMDQV separately, uses
> > that information to build the DSDT, and then frees the device array.
> 
> Oh, I missed the g_array_free() in acpi_dsdt_add_tegra241_cmdqv().
> 
> Let's have a note inline and copy to the commit message too.
> 
> > > We do so, because we need to link CMDQV's DSDT node to the SMMU's
> > > IORT node but it is created in build_iort() that is called after
> > > build_dsdt().
> > 
> > Hmm..not sure I get that. Does this mean IORT has a link to DSDT? I can't find one..
> > Maybe I missed. Please let me know.
> 
> The _UID field of the DSDT node links to the Identifier field of an
> SMMU's IORT node. E.g.
> 
> [DSDT] // Qemu builds DSDT first
> CMDQV0 {
>     _UID = 0;
> }
> 
> [IORT] // Then builds IORT
> SMMU0 {
>     Identifier = 0;
> }
> 
> If we are sure that the SMMU sequence is fixed every time we rebuild
> the sdev array so that IORT.Identifier matches with DSDT._UID, we'd
> be fine. That being said, we should probably make a node inline too.

Typo: node->note

> Otherwise, it would be safer to pre-build the sdev array and use the
> same array in both DSDT and IORT.
> 
> Thanks
> Nicolin

> -----Original Message-----
> From: Nicolin Chen <nicolinc@nvidia.com>
> Sent: 30 December 2025 18:24
> To: Shameer Kolothum <skolothumtho@nvidia.com>
> Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org;
> eric.auger@redhat.com; peter.maydell@linaro.org; Nathan Chen
> <nathanc@nvidia.com>; Matt Ochs <mochs@nvidia.com>; Jason Gunthorpe
> <jgg@nvidia.com>; jonathan.cameron@huawei.com;
> zhangfei.gao@linaro.org; zhenzhong.duan@intel.com; Krishnakant Jaju
> <kjaju@nvidia.com>
> Subject: Re: [RFC PATCH 15/16] hw/arm/virt-acpi: Advertise Tegra241
> CMDQV nodes in DSDT
> 
> On Tue, Dec 30, 2025 at 10:11:20AM -0800, Nicolin Chen wrote:
> > On Tue, Dec 30, 2025 at 02:13:12AM -0800, Shameer Kolothum wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Nicolin Chen <nicolinc@nvidia.com>
> > > > Sent: 29 December 2025 20:11
> > > > To: Shameer Kolothum <skolothumtho@nvidia.com>
> > > > Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org;
> > > > eric.auger@redhat.com; peter.maydell@linaro.org; Nathan Chen
> > > > <nathanc@nvidia.com>; Matt Ochs <mochs@nvidia.com>; Jason
> > > > Gunthorpe <jgg@nvidia.com>; jonathan.cameron@huawei.com;
> > > > zhangfei.gao@linaro.org; zhenzhong.duan@intel.com; Krishnakant
> > > > Jaju <kjaju@nvidia.com>
> > > > Subject: Re: [RFC PATCH 15/16] hw/arm/virt-acpi: Advertise
> > > > Tegra241 CMDQV nodes in DSDT
> > > >
> > > > On Wed, Dec 10, 2025 at 01:37:36PM +0000, Shameer Kolothum wrote:
> > > > > +static int smmuv3_cmdqv_devices(Object *obj, void *opaque) {
> > > > > +    VirtMachineState *vms = VIRT_MACHINE(qdev_get_machine());
> > > > > +    GArray *sdev_blob = opaque;
> > > > > +    PlatformBusDevice *pbus;
> > > > > +    AcpiSMMUv3Dev sdev;
> > > > > +    SysBusDevice *sbdev;
> > > > > +
> > > > > +    if (!object_dynamic_cast(obj, TYPE_ARM_SMMUV3)) {
> > > > > +        return 0;
> > > > > +    }
> > > > > +
> > > > > +    if (!object_property_get_bool(obj, "tegra241-cmdqv", NULL)) {
> > > > > +        return 0;
> > > > > +    }
> > > > > +
> > > > > +    pbus = PLATFORM_BUS_DEVICE(vms->platform_bus_dev);
> > > > > +    sbdev = SYS_BUS_DEVICE(obj);
> > > > > +    sdev.base = platform_bus_get_mmio_addr(pbus, sbdev, 1);
> > > > > +    sdev.base += vms->memmap[VIRT_PLATFORM_BUS].base;
> > > > > +    sdev.irq = platform_bus_get_irqn(pbus, sbdev, NUM_SMMU_IRQS);
> > > > > +    sdev.irq += vms->irqmap[VIRT_PLATFORM_BUS];
> > > > > +    sdev.irq += ARM_SPI_BASE;
> > > > > +    g_array_append_val(sdev_blob, sdev);
> > > > > +    return 0;
> > > >
> > > > This is pre-building SMMU's IORT nodes right? Maybe a different
> > > > naming? And can be shared with the existing iort_smmuv3_devices?
> > >
> > > Not really, if you are referring about this patch here,
> > >
> https://github.com/NVIDIA/QEMU/commit/cc3b65e6a49a9b7addf44b377d
> 4ef1
> > > de99bfee3f
> >
> > Yes. I am talking about this. But you are actually pre-building an
> > ACPI SMMU device array here. So, my question wasn't accurate..
> >
> > > I didn't find a clean way to store the pre-built smmuv3_devs other
> > > than placing them in struct AcpiBuildTables. It's not clear we gain
> > > much by restructuring things to populate and store them separately.
> > > At best, it might slightly improve boot time, and if that becomes
> > > important we can always add it as an optimization later.
> > >
> > > This patch enumerates SMMUv3 accel instances with CMDQV separately,
> > > uses that information to build the DSDT, and then frees the device array.
> >
> > Oh, I missed the g_array_free() in acpi_dsdt_add_tegra241_cmdqv().
> >
> > Let's have a note inline and copy to the commit message too.
> >
> > > > We do so, because we need to link CMDQV's DSDT node to the SMMU's
> > > > IORT node but it is created in build_iort() that is called after
> > > > build_dsdt().
> > >
> > > Hmm..not sure I get that. Does this mean IORT has a link to DSDT? I can't
> find one..
> > > Maybe I missed. Please let me know.
> >
> > The _UID field of the DSDT node links to the Identifier field of an
> > SMMU's IORT node. E.g.
> >
> > [DSDT] // Qemu builds DSDT first
> > CMDQV0 {
> >     _UID = 0;
> > }
> >
> > [IORT] // Then builds IORT
> > SMMU0 {
> >     Identifier = 0;
> > }
> >
> > If we are sure that the SMMU sequence is fixed every time we rebuild
> > the sdev array so that IORT.Identifier matches with DSDT._UID, we'd be
> > fine. That being said, we should probably make a node inline too.

I see that the IORT node Identifier is used in the kernel SMMUv3 driver
acpi_smmu_dsdt_probe_tegra241_cmdqv() to retrieve the corresponding
ACPI device. This appears to be the relationship you were referring to.

However, I don't think this holds for the current series as is. It will
likely fail if we have SMMUv3 accel devices both with and without
tegra241-cmdqv enabled, since the "identifier" will not match in that
case. I will fix that.

Regarding QEMU keeping the same sequence across both iterations, I
couldn't find any guarantee that object_child_foreach_recursive()
does that. Need to check.

Thanks,
Shameer