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. > 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. > + bool (*alloc_veventq)(SMMUv3State *s, Error **errp); > + void (*free_veventq)(SMMUv3State *s); alloc_/free_veventq can be moved into alloc/free_viommu functions. > + void (*reset)(SMMUv3State *s); > +} SMMUv3AccelCmdqvOps; Overall, an ops structure feels unnecessary to me. Maybe init and reset are somewhat plausible. But nobody else would reuse this ops structure that is exclusive to Tegra241CMDQV? > /* > * Represents an accelerated SMMU instance backed by an iommufd vIOMMU > object. > * Holds bypass and abort proxy HWPT IDs used for device attachment. > @@ -28,6 +45,8 @@ typedef struct SMMUv3AccelState { > uint32_t bypass_hwpt_id; > uint32_t abort_hwpt_id; > QLIST_HEAD(, SMMUv3AccelDevice) device_list; > + const SMMUv3AccelCmdqvOps *cmdqv_ops; > + void *cmdqv; Having two pointers for the same extension feels redundant. Maybe merge them with: typedef struct SMMUv3AccelCmdqv { void *private; // points to Tegra241CMDQV struct iommu_viommu_tegra241_cmdqv viommu_data; } SMMUv3AccelCmdqv; ... SMMUv3AccelState { ... SMMUv3AccelCmdqv *cmdqv; }; ? Nicolin
