> -----Original Message-----
> From: Nicolin Chen <[email protected]>
> Sent: 09 March 2026 17:24
> To: Shameer Kolothum Thodi <[email protected]>
> Cc: [email protected]; [email protected];
> [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 22/32] hw/arm/tegra241-cmdqv: Add vEVENTQ
> allocation and free
>
> On Thu, Feb 26, 2026 at 10:50:46AM +0000, Shameer Kolothum wrote:
> > Allocate a CMDQV specific vEVENTQ via IOMMUFD, and add the
> > corresponding teardown path to free the vEVENTQ during cleanup.
> >
> > Signed-off-by: Shameer Kolothum <[email protected]>
> > ---
> > hw/arm/smmuv3-accel.h | 2 ++
> > hw/arm/tegra241-cmdqv.h | 1 +
> > hw/arm/smmuv3-accel.c | 10 ++++++++-
> > hw/arm/tegra241-cmdqv.c | 47
> +++++++++++++++++++++++++++++++++++++++++
> > 4 files changed, 59 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/arm/smmuv3-accel.h b/hw/arm/smmuv3-accel.h
> > index 7d6e4c6b76..4bff90e2c1 100644
> > --- a/hw/arm/smmuv3-accel.h
> > +++ b/hw/arm/smmuv3-accel.h
> > @@ -28,6 +28,8 @@ typedef struct SMMUv3AccelCmdqvOps {
> > uint32_t *out_viommu_id,
> > Error **errp);
> > void (*free_viommu)(SMMUv3State *s);
> > + bool (*alloc_veventq)(SMMUv3State *s, Error **errp);
> > + void (*free_veventq)(SMMUv3State *s);
>
> As I replied in v2, this should really depend on viommu and should
> be simply added to tegra241_cmdqv_alloc/free_viommu().
Yes, I remember that. That was actually my initial plan, which is why
this was not part of the original callbacks in the ops struct.
However, I then realised that we are storing the viommu pointer
in veventq,
veventq->viommu = accel->viommu;
And that struct viommu is only allocated after alloc_viommu() returns.
I didn't find an easy way to resolve that. Need to take another look.
>
> The alloc_viommu is an override of the standard viommu type, so it
> might be useful to have an op.
>
> > diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
> > index 4373bbd97b..f6602f51aa 100644
> > --- a/hw/arm/smmuv3-accel.c
> > +++ b/hw/arm/smmuv3-accel.c
> > @@ -576,13 +576,21 @@ smmuv3_accel_alloc_viommu(SMMUv3State *s,
> HostIOMMUDeviceIOMMUFD *idev,
> > goto free_bypass_hwpt;
> > }
> >
> > + if (cmdqv_ops && !cmdqv_ops->alloc_veventq(s, errp)) {
> > + goto free_veventq;
> > + }
> > +
>
> But, this veventq op is completely in parallel to the standard one.
> There is no need for the smmuv3-accel to initiate the allocation?
If we can resolve the above dependency, then yes it can be tucked
inside .
Thanks,
Shameer
