Hi Eric,
> -----Original Message-----
> From: Eric Auger <[email protected]>
> Sent: 09 March 2026 09:18
> To: Shameer Kolothum Thodi <[email protected]>; qemu-
> [email protected]; [email protected]
> Cc: [email protected]; [email protected]; [email protected]; Nicolin
> Chen <[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 06/32] hw/arm/tegra241-cmdqv: Add Tegra241
> CMDQV ops backend stub
>
> External email: Use caution opening links or attachments
>
>
> Hi Shameer,
>
> On 2/26/26 11:50 AM, Shameer Kolothum wrote:
> > Introduce a Tegra241 CMDQV backend that plugs into the SMMUv3
> > accelerated CMDQV ops interface.
> >
> > This patch wires up the Tegra241 CMDQV backend and provides a stub
> > implementation for CMDQV probe, initialization, vIOMMU allocation and
> > reset handling.
> >
> > Functional CMDQV support is added in follow-up patches.
> >
> > Signed-off-by: Shameer Kolothum <[email protected]>
> > ---
> > hw/arm/tegra241-cmdqv.h | 15 ++++++++++
> > hw/arm/tegra241-cmdqv-stubs.c | 18 +++++++++++
> > hw/arm/tegra241-cmdqv.c | 56
> +++++++++++++++++++++++++++++++++++
> > hw/arm/Kconfig | 5 ++++
> > hw/arm/meson.build | 2 ++
> > 5 files changed, 96 insertions(+)
> > create mode 100644 hw/arm/tegra241-cmdqv.h create mode 100644
> > hw/arm/tegra241-cmdqv-stubs.c create mode 100644
> > hw/arm/tegra241-cmdqv.c
> >
> > diff --git a/hw/arm/tegra241-cmdqv.h b/hw/arm/tegra241-cmdqv.h new
> > file mode 100644 index 0000000000..07e10e86ee
> > --- /dev/null
> > +++ b/hw/arm/tegra241-cmdqv.h
> > @@ -0,0 +1,15 @@
> > +/*
> > + * Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All
> > +rights reserved
> > + * NVIDIA Tegra241 CMDQ-Virtualiisation extension for SMMUv3
> > + *
> > + * Written by Nicolin Chen, Shameer Kolothum
> > + *
> > + * SPDX-License-Identifier: GPL-2.0-or-later */
> > +
> > +#ifndef HW_ARM_TEGRA241_CMDQV_H
> > +#define HW_ARM_TEGRA241_CMDQV_H
> > +
> > +const SMMUv3AccelCmdqvOps *tegra241_cmdqv_get_ops(void);
> > +
> > +#endif /* HW_ARM_TEGRA241_CMDQV_H */
> > diff --git a/hw/arm/tegra241-cmdqv-stubs.c
> > b/hw/arm/tegra241-cmdqv-stubs.c new file mode 100644 index
> > 0000000000..eedc7bfdcd
> > --- /dev/null
> > +++ b/hw/arm/tegra241-cmdqv-stubs.c
> > @@ -0,0 +1,18 @@
> > +/*
> > + * Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All
> > +rights reserved
> > + *
> > + * Stubs for Tegra241 CMDQ-Virtualiisation extension for SMMUv3
> ^typo
> > + *
> > + * SPDX-License-Identifier: GPL-2.0-or-later */
> > +
> > +#include "qemu/osdep.h"
> > +#include "hw/arm/smmuv3.h"
> > +#include "smmuv3-accel.h"
> > +#include "hw/arm/tegra241-cmdqv.h"
> > +
> > +const SMMUv3AccelCmdqvOps *tegra241_cmdqv_get_ops(void) {
> > + return NULL;
> > +}
> > +
> > diff --git a/hw/arm/tegra241-cmdqv.c b/hw/arm/tegra241-cmdqv.c new
> > file mode 100644 index 0000000000..ad5a0d4611
> > --- /dev/null
> > +++ b/hw/arm/tegra241-cmdqv.c
> > @@ -0,0 +1,56 @@
> > +/*
> > + * Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All
> > +rights reserved
> > + * NVIDIA Tegra241 CMDQ-Virtualization extension for SMMUv3
> > + *
> > + * Written by Nicolin Chen, Shameer Kolothum
> > + *
> > + * SPDX-License-Identifier: GPL-2.0-or-later */
> > +
> > +#include "qemu/osdep.h"
> > +
> > +#include "hw/arm/smmuv3.h"
> > +#include "smmuv3-accel.h"
> > +#include "tegra241-cmdqv.h"
> > +
> > +static void tegra241_cmdqv_free_viommu(SMMUv3State *s) { }
> > +
> > +static bool
> > +tegra241_cmdqv_alloc_viommu(SMMUv3State *s,
> HostIOMMUDeviceIOMMUFD *idev,
> > + uint32_t *out_viommu_id, Error **errp) {
> > + error_setg(errp, "NVIDIA Tegra241 CMDQV is unsupported");
> > + return false;
> > +}
> > +
> > +static void tegra241_cmdqv_reset(SMMUv3State *s) { }
> > +
> > +static bool tegra241_cmdqv_init(SMMUv3State *s, Error **errp) {
> > + error_setg(errp, "NVIDIA Tegra241 CMDQV is unsupported");
> > + return false;
> > +}
> > +
> > +static bool tegra241_cmdqv_probe(SMMUv3State *s,
> HostIOMMUDeviceIOMMUFD *idev,
> > + Error **errp) {
> > + error_setg(errp, "NVIDIA Tegra241 CMDQV is unsupported");
> > + return false;
> > +}
> > +
> > +static const SMMUv3AccelCmdqvOps tegra241_cmdqv_ops = {
> > + .probe = tegra241_cmdqv_probe,
> > + .init = tegra241_cmdqv_init,
> > + .alloc_viommu = tegra241_cmdqv_alloc_viommu,
> > + .free_viommu = tegra241_cmdqv_free_viommu,
> > + .reset = tegra241_cmdqv_reset,
>
> I think I would create a no_cmdqv_ops to convert the existing code without
> any cmdqv support to the same api. This would avoid the repeating:
>
> if (cmdqv_ops && cmdqv_ops-><func>) check in subsequent patches.
I guess you are suggesting having something like,
static const SMMUv3AccelCmdqvOps no_cmdqv_ops = {
.probe = no_cmdqv_probe,
.alloc_viommu = no_cmdqv_alloc_viommu,
...
};
static bool no_cmdqv_probe(SMMUv3State *s,
HostIOMMUDeviceIOMMUFD *idev,
Error **errp)
{
return true;
}
static bool no_cmdqv_alloc_viommu(SMMUv3State *s,
HostIOMMUDeviceIOMMUFD *idev,
uint32_t *id,
Error **errp)
{
return false; /* We have to call the default vIOMMU alloc*/
}
And then call them directly. I will take a look at that approach.
>
> Also this looks a bit strange to have different signatures for the functions
> used
> with both vcmdq on and off
Is the concern here is mainly around alloc_viommu( that is the one which
require fallback
to default vIOMMU alloc)?
static bool
smmuv3_accel_alloc_viommu(SMMUv3State *s, HostIOMMUDeviceIOMMUFD *idev,
Error **errp)
and
bool (*alloc_viommu)(SMMUv3State *s,
HostIOMMUDeviceIOMMUFD *idev,
uint32_t *out_viommu_id,
Error **errp);
Making those consistent would likely require some refactoring of the
existing path. I will have a play with that and see.
Thanks,
Shameer
> Eric
>
>
> > +};
> > +
> > +const SMMUv3AccelCmdqvOps *tegra241_cmdqv_get_ops(void) {
> > + return &tegra241_cmdqv_ops;
> > +}
> > diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig index
> > c66c452737..3305c6e76e 100644
> > --- a/hw/arm/Kconfig
> > +++ b/hw/arm/Kconfig
> > @@ -626,6 +626,10 @@ config FSL_IMX8MP_EVK
> > depends on TCG
> > select FSL_IMX8MP
> >
> > +config TEGRA241_CMDQV
> > + bool
> > + depends on ARM_SMMUV3_ACCEL
> > +
> > config ARM_SMMUV3_ACCEL
> > bool
> > depends on ARM_SMMUV3
> > @@ -633,6 +637,7 @@ config ARM_SMMUV3_ACCEL config
> ARM_SMMUV3
> > bool
> > select ARM_SMMUV3_ACCEL if IOMMUFD
> > + imply TEGRA241_CMDQV
> >
> > config FSL_IMX6UL
> > bool
> > diff --git a/hw/arm/meson.build b/hw/arm/meson.build
> > index 8f834c32b1..fc83b635e7 100644
> > --- a/hw/arm/meson.build
> > +++ b/hw/arm/meson.build
> > @@ -88,6 +88,8 @@ arm_common_ss.add(when:
> 'CONFIG_FSL_IMX8MP_EVK', if_true: files('imx8mp-evk.c'))
> > arm_common_ss.add(when: 'CONFIG_ARM_SMMUV3', if_true:
> files('smmuv3.c'))
> > arm_common_ss.add(when: 'CONFIG_ARM_SMMUV3_ACCEL', if_true:
> files('smmuv3-accel.c'))
> > stub_ss.add(files('smmuv3-accel-stubs.c'))
> > +arm_common_ss.add(when: 'CONFIG_TEGRA241_CMDQV', if_true:
> files('tegra241-cmdqv.c'))
> > +stub_ss.add(files('tegra241-cmdqv-stubs.c'))
> > arm_common_ss.add(when: 'CONFIG_FSL_IMX6UL', if_true: files('fsl-
> imx6ul.c', 'mcimx6ul-evk.c'))
> > arm_common_ss.add(when: 'CONFIG_NRF51_SOC', if_true:
> files('nrf51_soc.c'))
> > arm_common_ss.add(when: 'CONFIG_XEN', if_true: files(
