> -----Original Message-----
> From: Eric Auger <[email protected]>
> Sent: 09 March 2026 11:09
> To: Shameer Kolothum Thodi <[email protected]>; qemu-
> [email protected]; [email protected]
> Cc: [email protected]; [email protected]; [email protected]; Nicolin
> Chen <[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
>
> External email: Use caution opening links or attachments
>
>
> On 2/26/26 11:50 AM, Shameer Kolothum wrote:
> > From: Nicolin Chen <[email protected]>
> >
> > Replace the stub implementation with real vIOMMU allocation for
> > Tegra241 CMDQV.
> >
> > Free the vIOMMU ID on teardown.
> >
> > Signed-off-by: Nicolin Chen <[email protected]>
> > Signed-off-by: Shameer Kolothum <[email protected]>
> > ---
> > hw/arm/tegra241-cmdqv.c | 19 +++++++++++++++++--
> > 1 file changed, 17 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/arm/tegra241-cmdqv.c b/hw/arm/tegra241-cmdqv.c index
> > 6959766129..d487612ba2 100644
> > --- a/hw/arm/tegra241-cmdqv.c
> > +++ b/hw/arm/tegra241-cmdqv.c
> > @@ -25,14 +25,29 @@ static void tegra241_cmdqv_write(void *opaque,
> > hwaddr offset, uint64_t value,
> >
> > static void tegra241_cmdqv_free_viommu(SMMUv3State *s) {
> > + SMMUv3AccelState *accel = s->s_accel;
> > + IOMMUFDViommu *viommu = accel->viommu;
> > +
> > + if (!viommu) {
> > + return;
> > + }
> > + iommufd_backend_free_id(viommu->iommufd, viommu->viommu_id);
> > }
> >
> > static bool
> > tegra241_cmdqv_alloc_viommu(SMMUv3State *s,
> HostIOMMUDeviceIOMMUFD *idev,
> > uint32_t *out_viommu_id, Error **errp) {
> > - 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.
Thanks,
Shameer
>
> Eric
>
>
> > + idev->hwpt_id, &cmdqv->cmdqv_data,
> > + sizeof(cmdqv->cmdqv_data),
> > out_viommu_id,
> > + errp)) {
> > + return false;
> > + }
> > + return true;
> > }
> >
> > static void tegra241_cmdqv_reset(SMMUv3State *s)
