Hi SHameer,
On 5/19/26 12:36 PM, Shameer Kolothum wrote:
> Add support for selecting and initializing a CMDQV backend based on the
> cmdqv OnOffAuto property.
>
> If set to OFF, CMDQV is not used and the default IOMMUFD-backed allocation
> path is taken.
>
> If set to AUTO, QEMU attempts to probe a CMDQV backend during device setup.
> If probing succeeds, the selected ops are stored in the accelerated SMMUv3
> state and used. If probing fails, QEMU silently falls back to the default
> path.
>
> If set to ON, QEMU requires CMDQV support. Probing is performed during
> setup and failure results in an error.
>
> When a CMDQV backend is active, its callbacks are used for vIOMMU
> allocation, free, and reset handling. Otherwise, the base implementation
> is used.
>
> The current implementation wires up the Tegra241 CMDQV backend through the
> generic ops interface. Functional CMDQV behaviour is added in subsequent
> patches.
>
> No functional change.
>
> Reviewed-by: Eric Auger
> Reviewed-by: Nicolin Chen
> Signed-off-by: Shameer Kolothum
> ---
> include/hw/arm/smmuv3.h | 2 +
> hw/arm/smmuv3-accel.c | 96 ++---
> 2 files changed, 91 insertions(+), 7 deletions(-)
>
> diff --git a/include/hw/arm/smmuv3.h b/include/hw/arm/smmuv3.h
> index fe0493c1aa..aa6a79237a 100644
> --- a/include/hw/arm/smmuv3.h
> +++ b/include/hw/arm/smmuv3.h
> @@ -74,6 +74,8 @@ struct SMMUv3State {
> OnOffAuto ats;
> OasMode oas;
> SsidSizeMode ssidsize;
> +/* SMMU CMDQV extension */
> +OnOffAuto cmdqv;
>
> Notifier machine_done;
> };
> diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
> index 30c498ffcf..202b1aedd9 100644
> --- a/hw/arm/smmuv3-accel.c
> +++ b/hw/arm/smmuv3-accel.c
> @@ -19,6 +19,7 @@
> #include "smmuv3-internal.h"
> #include "smmuv3-accel.h"
> #include "system/system.h"
> +#include "tegra241-cmdqv.h"
>
> /*
> * The root region aliases the global system memory, and shared_as_sysmem
> @@ -570,6 +571,7 @@ smmuv3_accel_alloc_viommu(SMMUv3State *s,
> HostIOMMUDeviceIOMMUFD *hiodi,
>Error **errp)
> {
> SMMUv3AccelState *accel = s->s_accel;
> +const SMMUv3AccelCmdqvOps *cmdqv_ops = accel->cmdqv_ops;
> struct iommu_hwpt_arm_smmuv3 bypass_data = {
> .ste = { SMMU_STE_CFG_BYPASS | SMMU_STE_VALID, 0x0ULL },
> };
> @@ -580,10 +582,17 @@ smmuv3_accel_alloc_viommu(SMMUv3State *s,
> HostIOMMUDeviceIOMMUFD *hiodi,
> uint32_t viommu_id, hwpt_id;
> IOMMUFDViommu *viommu;
>
> -if (!iommufd_backend_alloc_viommu(hiodi->iommufd, hiodi->devid,
> - IOMMU_VIOMMU_TYPE_ARM_SMMUV3,
> s2_hwpt_id,
> - NULL, 0, &viommu_id, errp)) {
> -return false;
> +if (cmdqv_ops) {
> +if (!cmdqv_ops->alloc_viommu(s, hiodi, &viommu_id, errp)) {
the doc comment in 6/32 says that alloc_iommu is mandatory if probe is
implemented. So it looks there are situations where alloc_iommu may be null.
But actually given the implementation of smmuv3_accel_probe_cmdqv below,
the ops is only non NULL if probe has succeeded. So shouldn't probe be
also mandatory?
> +return false;
> +}
> +} else {
> +if (!iommufd_backend_alloc_viommu(hiodi->iommufd, hiodi->devid,
> + IOMMU_VIOMMU_TYPE_ARM_SMMUV3,
> + s2_hwpt_id, NULL, 0, &viommu_id,
> + errp)) {
> +return false;
> +}
> }
>
> viommu = g_new0(IOMMUFDViommu, 1);
> @@ -629,12 +638,72 @@ free_bypass_hwpt:
> free_abort_hwpt:
> iommufd_backend_free_id(hiodi->iommufd, accel->abort_hwpt_id);
> free_viommu:
> -iommufd_backend_free_id(hiodi->iommufd, viommu->viommu_id);
> +if (cmdqv_ops && cmdqv_ops->free_viommu) {
> +cmdqv_ops->free_viommu(s);
> +} else {
> +iommufd_backend_free_id(hiodi->iommufd, viommu->viommu_id);
> +}
> g_free(viommu);
> accel->viommu = NULL;
> return false;
> }
>
> +static const SMMUv3AccelCmdqvOps *
> +smmuv3_accel_probe_cmdqv(SMMUv3State *s, HostIOMMUDeviceIOMMUFD *idev,
> + Error **errp)
> +{
> +const SMMUv3AccelCmdqvOps *ops = tegra241_cmdqv_get_ops();
> +
> +if (!ops || !ops->probe) {
> +error_setg(errp, "No CMDQV ops found");
> +return NULL;
> +}
> +
> +if (!ops->probe(s, idev, errp)) {
> +return NULL;
> +}
> +return ops;
> +}
> +
> +static bool
> +smmuv3_accel_select_cmdqv(SMMUv3State *s, HostIOMMUDeviceIOMMUFD *idev,
> + Error **errp)
> +{
> +const SMMUv3AccelCmdqvOps *ops = NULL;
> +
> +if (s->s_accel->cmdqv_ops) {
> +return true;
> +}
> +
> +switch (s->cmdqv) {
> +case ON_OFF_AUTO_OFF:
> +s->s_accel->cmdqv_ops = NULL;
> +return true;
> +case ON_OFF_AUTO_AUTO:
> +ops
