> -----Original Message-----
> From: Nicolin Chen <[email protected]>
> Sent: 30 December 2025 18:24
> To: Shameer Kolothum <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]; Nathan Chen
> <[email protected]>; Matt Ochs <[email protected]>; Jason Gunthorpe
> <[email protected]>; [email protected];
> [email protected]; [email protected]; Krishnakant Jaju
> <[email protected]>
> 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 <[email protected]>
> > > > Sent: 29 December 2025 20:11
> > > > To: Shameer Kolothum <[email protected]>
> > > > Cc: [email protected]; [email protected];
> > > > [email protected]; [email protected]; Nathan Chen
> > > > <[email protected]>; Matt Ochs <[email protected]>; Jason
> > > > Gunthorpe <[email protected]>; [email protected];
> > > > [email protected]; [email protected]; Krishnakant
> > > > Jaju <[email protected]>
> > > > 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
