> -----Original Message-----
> From: Nicolin Chen <[email protected]>
> Sent: 09 March 2026 18:10
> To: Shameer Kolothum Thodi <[email protected]>
> Cc: [email protected]; [email protected]; qemu-
> [email protected]; [email protected]; [email protected];
> [email protected]; Nathan Chen <[email protected]>; Matt Ochs
> <[email protected]>; Jiandi An <[email protected]>; Jason Gunthorpe
> <[email protected]>; [email protected];
> [email protected]; [email protected]; Krishnakant Jaju
> <[email protected]>; [email protected]
> Subject: Re: [PATCH v3 13/32] hw/arm/tegra241-cmdqv: Implement CMDQV
> vIOMMU alloc/free
>
> On Mon, Mar 09, 2026 at 04:31:22AM -0700, Shameer Kolothum Thodi
> wrote:
> > > > - error_setg(errp, "NVIDIA Tegra241 CMDQV is unsupported");
> > > > - return false;
> > > > + Tegra241CMDQV *cmdqv = s->s_accel->cmdqv;
> > > > +
> > > > + if (!iommufd_backend_alloc_viommu(idev->iommufd, idev->devid,
> > > > +
> > > > + IOMMU_VIOMMU_TYPE_TEGRA241_CMDQV,
> > > if the only think that differs compared to no cmdq is the type, ie.
> > >
> > > IOMMU_VIOMMU_TYPE_TEGRA241_CMDQV vs
> > > IOMMU_VIOMMU_TYPE_TEGRA241_CMDQV
> > >
> > > + passing true args, maybe you can just record the type of the cmdqv
> > > + that is being used in the smmu_accel device. Then you can get rid of
> > > + alloc and free
> >
> > It is not just the type. Based on the type we also need to pass,
> >
> > * @data_len: Length of the type specific data
> > * @__reserved: Must be 0
> > * @data_uptr: User pointer to a driver-specific virtual IOMMU data
> > *
> >
> > And the above is implementation specific.
> >
> > If our idea of the "ops" is to allow easier support for different
> > implementations in future, it probably makes sense to keep this.
>
> Any future "cmdqv" will likely have a different viommu type. So,
> essentially this is just a viommu type override, as Eric pointed
> out above.
>
> What we actually need to expose per viommu is type/data_len. The
> data_uptr can be a union viommu_data in the accel structure?
So the suggestion is to get rid of alloc_viommu() and retrieve type/len
during probe() and store the data_uptr in struct accel . Doable, I guess.
But then we need to do something similar for veventq as well, right?
Isn't that actually bloating struct accel then?
IMHO, the ops way looks better.
Thanks,
Shameer
