Re: [RFC PATCH 15/16] hw/arm/virt-acpi: Advertise Tegra241 CMDQV nodes in DSDT

[Nicolin Chen <nicolinc@nvidia.com>]

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