On Mon, Mar 09, 2026 at 10:41:24AM -0700, Shameer Kolothum Thodi wrote:
> 
> 
> > -----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.

Looks like we only needed iommufd pointer v.s. accel->viommu? This
should allow us to merge alloc_veventq into alloc_viommu.

Alternatively, as we discussed on the other side, we could drop the
alloc/free_viommu. Then, have a pair of viommu_init/destroy to have
the veventq part (and any viommu specific routine in the future).

I feel okay with either approach. So, pick one that works the best.

Nicolin

Reply via email to