Hi Eric, > -----Original Message----- > From: Eric Auger <[email protected]> > Sent: 12 February 2026 14:42 > To: Shameer Kolothum Thodi <[email protected]>; Nicolin Chen > <[email protected]> > Cc: [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]> > Subject: Re: [PATCH v2 05/24] hw/arm/smmuv3-accel: Introduce CMDQV ops > interface > > External email: Use caution opening links or attachments > > > On 2/10/26 9:46 AM, Shameer Kolothum Thodi wrote: > > > >> -----Original Message----- > >> From: Nicolin Chen <[email protected]> > >> Sent: 09 February 2026 19:42 > >> 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]> > >> Subject: Re: [PATCH v2 05/24] hw/arm/smmuv3-accel: Introduce CMDQV > ops > >> interface > >> > >> On Fri, Feb 06, 2026 at 02:48:04PM +0000, Shameer Kolothum wrote: > >>> CMDQ-Virtualization (CMDQV) is a hardware extension to SMMUv3 that > >> enables > >>> virtualization of multiple command queues (VCMDQs). > >> Let's mention "NVIDIA", noting it's a non-standard extension. > > The idea of this ops based callback is to make it more generic and not > > NVIDIA specific to start with. This leaves room in case ARM introduces > > a standardised version of something similar in the future, or if another > > vendor provides a similar extension. > > > >>> CMDQV support is a specialization of the IOMMUFD backed accelerated > >>> SMMUv3 path. Introduce an ops interface to factor CMDQV specific > >>> initialization and CMDQV vIOMMU/vEVENTQ allocation behavior out of > >>> the base implementation. The ops pointer and associated state are > >>> stored in the accelerated SMMUv3 state. > >>> > >>> No functional change > >>> > >>> Signed-off-by: Shameer Kolothum <[email protected]> > >>> --- > >>> hw/arm/smmuv3-accel.h | 19 +++++++++++++++++++ > >>> 1 file changed, 19 insertions(+) > >>> > >>> diff --git a/hw/arm/smmuv3-accel.h b/hw/arm/smmuv3-accel.h > >>> index c9c10e55c3..ca087240e5 100644 > >>> --- a/hw/arm/smmuv3-accel.h > >>> +++ b/hw/arm/smmuv3-accel.h > >>> @@ -16,6 +16,23 @@ > >>> #endif > >>> #include CONFIG_DEVICES > >>> > >>> +/* > >>> + * CMDQ-Virtualization (CMDQV) hardware support, extends the > SMMUv3 > >> to > >>> + * support multiple VCMDQs with virtualization capabilities. > >> Ditto > >> > >>> + * CMDQV specific behavior is factored behind this ops interface. > >>> + */ > >>> +typedef struct SMMUv3AccelCmdqvOps { > >>> + bool (*init)(SMMUv3State *s, Error **errp); > >>> + bool (*alloc_viommu)(SMMUv3State *s, > >>> + HostIOMMUDeviceIOMMUFD *idev, > >>> + uint32_t *out_viommu_id, > >>> + Error **errp); > >>> + void (*free_viommu)(SMMUv3State *s); > >> I don't see much value of having alloc/free_viommu.. > >> > >> Type and data structure are the only difference from the standard > >> viommu routine, and both are already in the uAPI header. > > That is right. But again, this is mainly to keep the interface generic. > > IIUC, Eric had suggested, using an ops or object style abstraction > > to separate extension specific behaviour from the core code. > > My initial thought was callbacks to specialize implementation for accel > smmu without vcmdq versus accel smmu with vcmdqs. Effectively we could > later imagine to have other implementations with vcmdqs but it is > difficult to guess the specialization needs at that stage. I think in > the original series you already add the first kind of differentiation. > > At least reset and init need to be specialized. > > Maybe alloc_viommu does not need to because the differentiation only > happens on the type arg. But originally you where also turning off > > s->tegra241_cmdqv if IOMMU_VIOMMU_TYPE_TEGRA241_CMDQV failed so > I had > the impression you reverted to a fallback impl. If there is > specialization, I still think ops are relevant. Eric
There is no fallback implementation. So just to confirm, you are suggesting to remove the ops completely or just keep the ops for reset/init only? Thanks, Shameer
