From: Nicolin Chen <nicolinc@nvidia.com>
Introduce initial support for NVIDIA Tegra241 CMDQ-Virtualisation (CMDQV),
an extension to SMMUv3 providing virtualizable hardware command queues.
This adds the basic MMIO handling, and integration hooks in the SMMUv3
accelerated path. When enabled, the SMMUv3 backend allocates a Tegra241
specific vIOMMU object via IOMMUFD and exposes a CMDQV MMIO region and
IRQ to the guest.
The "tegra241-cmdqv" property isn't user visible yet and it will be
introduced in a later patch once all the supporting pieces are ready.
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
---
hw/arm/Kconfig | 5 ++++
hw/arm/meson.build | 1 +
hw/arm/smmuv3-accel.c | 10 +++++--
hw/arm/smmuv3.c | 4 +++
hw/arm/tegra241-cmdqv.c | 65 +++++++++++++++++++++++++++++++++++++++++
hw/arm/tegra241-cmdqv.h | 40 +++++++++++++++++++++++++
include/hw/arm/smmuv3.h | 3 ++
7 files changed, 126 insertions(+), 2 deletions(-)
create mode 100644 hw/arm/tegra241-cmdqv.c
create mode 100644 hw/arm/tegra241-cmdqv.h
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 702b79a02b..42b6b95285 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -37,6 +37,7 @@ config ARM_VIRT
select VIRTIO_MEM_SUPPORTED
select ACPI_CXL
select ACPI_HMAT
+ select TEGRA241_CMDQV
config CUBIEBOARD
bool
@@ -634,6 +635,10 @@ config ARM_SMMUV3_ACCEL
bool
depends on ARM_SMMUV3 && IOMMUFD
+config TEGRA241_CMDQV
+ bool
+ depends on ARM_SMMUV3_ACCEL
+
config FSL_IMX6UL
bool
default y
diff --git a/hw/arm/meson.build b/hw/arm/meson.build
index c250487e64..4ec91db50a 100644
--- a/hw/arm/meson.build
+++ b/hw/arm/meson.build
@@ -86,6 +86,7 @@ arm_common_ss.add(when: 'CONFIG_FSL_IMX8MP', if_true: files('fsl-imx8mp.c'))
arm_common_ss.add(when: 'CONFIG_FSL_IMX8MP_EVK', if_true: files('imx8mp-evk.c'))
arm_ss.add(when: 'CONFIG_ARM_SMMUV3', if_true: files('smmuv3.c'))
arm_ss.add(when: 'CONFIG_ARM_SMMUV3_ACCEL', if_true: files('smmuv3-accel.c'))
+arm_ss.add(when: 'CONFIG_TEGRA241_CMDQV', if_true: files('tegra241-cmdqv.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(
diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
index 939898c9b0..e50c4b3bb7 100644
--- a/hw/arm/smmuv3-accel.c
+++ b/hw/arm/smmuv3-accel.c
@@ -18,6 +18,7 @@
#include "smmuv3-internal.h"
#include "smmuv3-accel.h"
+#include "tegra241-cmdqv.h"
/*
* The root region aliases the global system memory, and shared_as_sysmem
@@ -499,10 +500,15 @@ smmuv3_accel_alloc_viommu(SMMUv3State *s, HostIOMMUDeviceIOMMUFD *idev,
.ste = { SMMU_STE_VALID, 0x0ULL },
};
uint32_t s2_hwpt_id = idev->hwpt_id;
- uint32_t viommu_id, hwpt_id;
+ uint32_t viommu_id = 0, hwpt_id;
SMMUv3AccelState *accel;
- if (!iommufd_backend_alloc_viommu(idev->iommufd, idev->devid,
+ if (s->tegra241_cmdqv && !tegra241_cmdqv_alloc_viommu(s, idev, &viommu_id,
+ errp)) {
+ return false;
+ }
+
+ if (!viommu_id && !iommufd_backend_alloc_viommu(idev->iommufd, idev->devid,
IOMMU_VIOMMU_TYPE_ARM_SMMUV3, s2_hwpt_id,
NULL, 0, &viommu_id, errp)) {
return false;
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 9b7b85fb49..02e1a925a4 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -36,6 +36,7 @@
#include "smmuv3-accel.h"
#include "smmuv3-internal.h"
#include "smmu-internal.h"
+#include "tegra241-cmdqv.h"
#define PTW_RECORD_FAULT(ptw_info, cfg) (((ptw_info).stage == SMMU_STAGE_1 && \
(cfg)->record_faults) || \
@@ -2017,6 +2018,9 @@ static void smmu_realize(DeviceState *d, Error **errp)
smmu_init_irq(s, dev);
smmuv3_init_id_regs(s);
+ if (s->tegra241_cmdqv) {
+ tegra241_cmdqv_init(s);
+ }
}
static const VMStateDescription vmstate_smmuv3_queue = {
diff --git a/hw/arm/tegra241-cmdqv.c b/hw/arm/tegra241-cmdqv.c
new file mode 100644
index 0000000000..899325877e
--- /dev/null
+++ b/hw/arm/tegra241-cmdqv.c
@@ -0,0 +1,65 @@
+/*
+ * Copyright (C) 2025, NVIDIA CORPORATION
+ * 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 uint64_t tegra241_cmdqv_read(void *opaque, hwaddr offset, unsigned size)
+{
+ return 0;
+}
+
+static void tegra241_cmdqv_write(void *opaque, hwaddr offset, uint64_t value,
+ unsigned size)
+{
+}
+
+static const MemoryRegionOps mmio_cmdqv_ops = {
+ .read = tegra241_cmdqv_read,
+ .write = tegra241_cmdqv_write,
+ .endianness = DEVICE_LITTLE_ENDIAN,
+};
+
+bool tegra241_cmdqv_alloc_viommu(SMMUv3State *s, HostIOMMUDeviceIOMMUFD *idev,
+ uint32_t *out_viommu_id, Error **errp)
+{
+ Tegra241CMDQV *cmdqv = s->cmdqv;
+
+ if (!iommufd_backend_alloc_viommu(idev->iommufd, idev->devid,
+ IOMMU_VIOMMU_TYPE_TEGRA241_CMDQV,
+ idev->hwpt_id, &cmdqv->cmdqv_data,
+ sizeof(cmdqv->cmdqv_data), out_viommu_id,
+ errp)) {
+ error_append_hint(errp, "NVIDIA Tegra241 CMDQV is unsupported");
+ s->tegra241_cmdqv = false;
+ return false;
+ }
+ return true;
+}
+
+void tegra241_cmdqv_init(SMMUv3State *s)
+{
+ SysBusDevice *sbd = SYS_BUS_DEVICE(OBJECT(s));
+ Tegra241CMDQV *cmdqv;
+
+ if (!s->tegra241_cmdqv) {
+ return;
+ }
+
+ cmdqv = g_new0(Tegra241CMDQV, 1);
+ memory_region_init_io(&cmdqv->mmio_cmdqv, OBJECT(s), &mmio_cmdqv_ops, cmdqv,
+ "tegra241-cmdqv", TEGRA241_CMDQV_IO_LEN);
+ sysbus_init_mmio(sbd, &cmdqv->mmio_cmdqv);
+ sysbus_init_irq(sbd, &cmdqv->irq);
+ cmdqv->smmu = s;
+ s->cmdqv = cmdqv;
+}
diff --git a/hw/arm/tegra241-cmdqv.h b/hw/arm/tegra241-cmdqv.h
new file mode 100644
index 0000000000..9bc72b24d9
--- /dev/null
+++ b/hw/arm/tegra241-cmdqv.h
@@ -0,0 +1,40 @@
+/*
+ * Copyright (C) 2025, NVIDIA CORPORATION
+ * NVIDIA Tegra241 CMDQ-Virtualiisation extension for SMMUv3
+ *
+ * Written by Nicolin Chen, Shameer Kolothum
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef HW_TEGRA241_CMDQV_H
+#define HW_TEGRA241_CMDQV_H
+
+#include CONFIG_DEVICES
+
+#define TEGRA241_CMDQV_IO_LEN 0x50000
+
+typedef struct Tegra241CMDQV {
+ struct iommu_viommu_tegra241_cmdqv cmdqv_data;
+ SMMUv3State *smmu;
+ MemoryRegion mmio_cmdqv;
+ qemu_irq irq;
+} Tegra241CMDQV;
+
+#ifdef CONFIG_TEGRA241_CMDQV
+bool tegra241_cmdqv_alloc_viommu(SMMUv3State *s, HostIOMMUDeviceIOMMUFD *idev,
+ uint32_t *out_viommu_id, Error **errp);
+void tegra241_cmdqv_init(SMMUv3State *s);
+#else
+static inline void tegra241_cmdqv_init(SMMUv3State *s)
+{
+}
+static inline bool
+tegra241_cmdqv_alloc_viommu(SMMUv3State *s, HostIOMMUDeviceIOMMUFD *idev,
+ uint32_t *out_viommu_id, Error **errp)
+{
+ return true;
+}
+#endif
+
+#endif /* HW_TEGRA241_CMDQV_H */
diff --git a/include/hw/arm/smmuv3.h b/include/hw/arm/smmuv3.h
index 2d4970fe19..8e56e480a0 100644
--- a/include/hw/arm/smmuv3.h
+++ b/include/hw/arm/smmuv3.h
@@ -73,6 +73,9 @@ struct SMMUv3State {
bool ats;
uint8_t oas;
bool pasid;
+ /* Support for NVIDIA Tegra241 SMMU CMDQV extension */
+ struct Tegra241CMDQV *cmdqv;
+ bool tegra241_cmdqv;
};
typedef enum {
--
2.43.0
On 12/10/25 2:37 PM, Shameer Kolothum wrote:
> From: Nicolin Chen <nicolinc@nvidia.com>
>
> Introduce initial support for NVIDIA Tegra241 CMDQ-Virtualisation (CMDQV),
well this is rather a skeleton and at this stage it does not introduce
any support
> an extension to SMMUv3 providing virtualizable hardware command queues.
> This adds the basic MMIO handling, and integration hooks in the SMMUv3
> accelerated path. When enabled, the SMMUv3 backend allocates a Tegra241
> specific vIOMMU object via IOMMUFD and exposes a CMDQV MMIO region and
> IRQ to the guest.
I would elaborate on the nature of region and interrupt here.
>
> The "tegra241-cmdqv" property isn't user visible yet and it will be
> introduced in a later patch once all the supporting pieces are ready.
I would suggest "tegra241_cmdqv will be set by an SMMU property that does not exist yet. As such it is currently unset."
>
> Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
> Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
> ---
> hw/arm/Kconfig | 5 ++++
> hw/arm/meson.build | 1 +
> hw/arm/smmuv3-accel.c | 10 +++++--
> hw/arm/smmuv3.c | 4 +++
> hw/arm/tegra241-cmdqv.c | 65 +++++++++++++++++++++++++++++++++++++++++
> hw/arm/tegra241-cmdqv.h | 40 +++++++++++++++++++++++++
> include/hw/arm/smmuv3.h | 3 ++
> 7 files changed, 126 insertions(+), 2 deletions(-)
> create mode 100644 hw/arm/tegra241-cmdqv.c
> create mode 100644 hw/arm/tegra241-cmdqv.h
>
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index 702b79a02b..42b6b95285 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -37,6 +37,7 @@ config ARM_VIRT
> select VIRTIO_MEM_SUPPORTED
> select ACPI_CXL
> select ACPI_HMAT
> + select TEGRA241_CMDQV
>
> config CUBIEBOARD
> bool
> @@ -634,6 +635,10 @@ config ARM_SMMUV3_ACCEL
> bool
> depends on ARM_SMMUV3 && IOMMUFD
>
> +config TEGRA241_CMDQV
> + bool
> + depends on ARM_SMMUV3_ACCEL
> +
> config FSL_IMX6UL
> bool
> default y
> diff --git a/hw/arm/meson.build b/hw/arm/meson.build
> index c250487e64..4ec91db50a 100644
> --- a/hw/arm/meson.build
> +++ b/hw/arm/meson.build
> @@ -86,6 +86,7 @@ arm_common_ss.add(when: 'CONFIG_FSL_IMX8MP', if_true: files('fsl-imx8mp.c'))
> arm_common_ss.add(when: 'CONFIG_FSL_IMX8MP_EVK', if_true: files('imx8mp-evk.c'))
> arm_ss.add(when: 'CONFIG_ARM_SMMUV3', if_true: files('smmuv3.c'))
> arm_ss.add(when: 'CONFIG_ARM_SMMUV3_ACCEL', if_true: files('smmuv3-accel.c'))
> +arm_ss.add(when: 'CONFIG_TEGRA241_CMDQV', if_true: files('tegra241-cmdqv.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(
> diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
> index 939898c9b0..e50c4b3bb7 100644
> --- a/hw/arm/smmuv3-accel.c
> +++ b/hw/arm/smmuv3-accel.c
> @@ -18,6 +18,7 @@
>
> #include "smmuv3-internal.h"
> #include "smmuv3-accel.h"
> +#include "tegra241-cmdqv.h"
>
> /*
> * The root region aliases the global system memory, and shared_as_sysmem
> @@ -499,10 +500,15 @@ smmuv3_accel_alloc_viommu(SMMUv3State *s, HostIOMMUDeviceIOMMUFD *idev,
> .ste = { SMMU_STE_VALID, 0x0ULL },
> };
> uint32_t s2_hwpt_id = idev->hwpt_id;
> - uint32_t viommu_id, hwpt_id;
> + uint32_t viommu_id = 0, hwpt_id;
> SMMUv3AccelState *accel;
>
> - if (!iommufd_backend_alloc_viommu(idev->iommufd, idev->devid,
> + if (s->tegra241_cmdqv && !tegra241_cmdqv_alloc_viommu(s, idev, &viommu_id,
> + errp)) {
> + return false;
> + }
> +
> + if (!viommu_id && !iommufd_backend_alloc_viommu(idev->iommufd, idev->devid,
> IOMMU_VIOMMU_TYPE_ARM_SMMUV3, s2_hwpt_id,
> NULL, 0, &viommu_id, errp)) {
> return false;
> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> index 9b7b85fb49..02e1a925a4 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -36,6 +36,7 @@
> #include "smmuv3-accel.h"
> #include "smmuv3-internal.h"
> #include "smmu-internal.h"
> +#include "tegra241-cmdqv.h"
>
> #define PTW_RECORD_FAULT(ptw_info, cfg) (((ptw_info).stage == SMMU_STAGE_1 && \
> (cfg)->record_faults) || \
> @@ -2017,6 +2018,9 @@ static void smmu_realize(DeviceState *d, Error **errp)
>
> smmu_init_irq(s, dev);
> smmuv3_init_id_regs(s);
> + if (s->tegra241_cmdqv) {
> + tegra241_cmdqv_init(s);
> + }
> }
>
> static const VMStateDescription vmstate_smmuv3_queue = {
> diff --git a/hw/arm/tegra241-cmdqv.c b/hw/arm/tegra241-cmdqv.c
> new file mode 100644
> index 0000000000..899325877e
> --- /dev/null
> +++ b/hw/arm/tegra241-cmdqv.c
> @@ -0,0 +1,65 @@
> +/*
> + * Copyright (C) 2025, NVIDIA CORPORATION
nit: s/2025/2026
> + * 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 uint64_t tegra241_cmdqv_read(void *opaque, hwaddr offset, unsigned size)
> +{
> + return 0;
> +}
> +
> +static void tegra241_cmdqv_write(void *opaque, hwaddr offset, uint64_t value,
> + unsigned size)
> +{
> +}
> +
> +static const MemoryRegionOps mmio_cmdqv_ops = {
> + .read = tegra241_cmdqv_read,
> + .write = tegra241_cmdqv_write,
> + .endianness = DEVICE_LITTLE_ENDIAN,
> +};
> +
> +bool tegra241_cmdqv_alloc_viommu(SMMUv3State *s, HostIOMMUDeviceIOMMUFD *idev,
> + uint32_t *out_viommu_id, Error **errp)
> +{
> + Tegra241CMDQV *cmdqv = s->cmdqv;
> +
> + if (!iommufd_backend_alloc_viommu(idev->iommufd, idev->devid,
> + IOMMU_VIOMMU_TYPE_TEGRA241_CMDQV,
> + idev->hwpt_id, &cmdqv->cmdqv_data,
> + sizeof(cmdqv->cmdqv_data), out_viommu_id,
> + errp)) {
> + error_append_hint(errp, "NVIDIA Tegra241 CMDQV is unsupported");
are we sure this is the case here and not a radom failure of other source?
> + s->tegra241_cmdqv = false;
> + return false;
> + }
> + return true;
> +}
> +
> +void tegra241_cmdqv_init(SMMUv3State *s)
> +{
> + SysBusDevice *sbd = SYS_BUS_DEVICE(OBJECT(s));
> + Tegra241CMDQV *cmdqv;
> +
> + if (!s->tegra241_cmdqv) {
> + return;
> + }
> +
> + cmdqv = g_new0(Tegra241CMDQV, 1);
> + memory_region_init_io(&cmdqv->mmio_cmdqv, OBJECT(s), &mmio_cmdqv_ops, cmdqv,
> + "tegra241-cmdqv", TEGRA241_CMDQV_IO_LEN);
> + sysbus_init_mmio(sbd, &cmdqv->mmio_cmdqv);
> + sysbus_init_irq(sbd, &cmdqv->irq);
> + cmdqv->smmu = s;
> + s->cmdqv = cmdqv;
> +}
> diff --git a/hw/arm/tegra241-cmdqv.h b/hw/arm/tegra241-cmdqv.h
> new file mode 100644
> index 0000000000..9bc72b24d9
> --- /dev/null
> +++ b/hw/arm/tegra241-cmdqv.h
> @@ -0,0 +1,40 @@
> +/*
> + * Copyright (C) 2025, NVIDIA CORPORATION
> + * NVIDIA Tegra241 CMDQ-Virtualiisation extension for SMMUv3
virtualization
> + *
> + * Written by Nicolin Chen, Shameer Kolothum
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#ifndef HW_TEGRA241_CMDQV_H
> +#define HW_TEGRA241_CMDQV_H
> +
> +#include CONFIG_DEVICES
> +
> +#define TEGRA241_CMDQV_IO_LEN 0x50000
can you explain the size
> +
> +typedef struct Tegra241CMDQV {
> + struct iommu_viommu_tegra241_cmdqv cmdqv_data;
> + SMMUv3State *smmu;
> + MemoryRegion mmio_cmdqv;
> + qemu_irq irq;
> +} Tegra241CMDQV;
> +
> +#ifdef CONFIG_TEGRA241_CMDQV
> +bool tegra241_cmdqv_alloc_viommu(SMMUv3State *s, HostIOMMUDeviceIOMMUFD *idev,
> + uint32_t *out_viommu_id, Error **errp);
> +void tegra241_cmdqv_init(SMMUv3State *s);
> +#else
> +static inline void tegra241_cmdqv_init(SMMUv3State *s)
> +{
> +}
> +static inline bool
> +tegra241_cmdqv_alloc_viommu(SMMUv3State *s, HostIOMMUDeviceIOMMUFD *idev,
> + uint32_t *out_viommu_id, Error **errp)
> +{
> + return true;
> +}
> +#endif
> +
> +#endif /* HW_TEGRA241_CMDQV_H */
> diff --git a/include/hw/arm/smmuv3.h b/include/hw/arm/smmuv3.h
> index 2d4970fe19..8e56e480a0 100644
> --- a/include/hw/arm/smmuv3.h
> +++ b/include/hw/arm/smmuv3.h
> @@ -73,6 +73,9 @@ struct SMMUv3State {
> bool ats;
> uint8_t oas;
> bool pasid;
> + /* Support for NVIDIA Tegra241 SMMU CMDQV extension */
> + struct Tegra241CMDQV *cmdqv;
> + bool tegra241_cmdqv;
> };
>
> typedef enum {
Hi Shameer,
On 12/10/25 2:37 PM, Shameer Kolothum wrote:
> From: Nicolin Chen <nicolinc@nvidia.com>
>
> Introduce initial support for NVIDIA Tegra241 CMDQ-Virtualisation (CMDQV),
> an extension to SMMUv3 providing virtualizable hardware command queues.
> This adds the basic MMIO handling, and integration hooks in the SMMUv3
> accelerated path. When enabled, the SMMUv3 backend allocates a Tegra241
> specific vIOMMU object via IOMMUFD and exposes a CMDQV MMIO region and
> IRQ to the guest.
>
> The "tegra241-cmdqv" property isn't user visible yet and it will be
> introduced in a later patch once all the supporting pieces are ready.
>
> Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
> Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
> ---
> hw/arm/Kconfig | 5 ++++
> hw/arm/meson.build | 1 +
> hw/arm/smmuv3-accel.c | 10 +++++--
> hw/arm/smmuv3.c | 4 +++
> hw/arm/tegra241-cmdqv.c | 65 +++++++++++++++++++++++++++++++++++++++++
> hw/arm/tegra241-cmdqv.h | 40 +++++++++++++++++++++++++
> include/hw/arm/smmuv3.h | 3 ++
> 7 files changed, 126 insertions(+), 2 deletions(-)
> create mode 100644 hw/arm/tegra241-cmdqv.c
> create mode 100644 hw/arm/tegra241-cmdqv.h
>
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index 702b79a02b..42b6b95285 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -37,6 +37,7 @@ config ARM_VIRT
> select VIRTIO_MEM_SUPPORTED
> select ACPI_CXL
> select ACPI_HMAT
> + select TEGRA241_CMDQV
>
> config CUBIEBOARD
> bool
> @@ -634,6 +635,10 @@ config ARM_SMMUV3_ACCEL
> bool
> depends on ARM_SMMUV3 && IOMMUFD
>
> +config TEGRA241_CMDQV
> + bool
> + depends on ARM_SMMUV3_ACCEL
> +
> config FSL_IMX6UL
> bool
> default y
> diff --git a/hw/arm/meson.build b/hw/arm/meson.build
> index c250487e64..4ec91db50a 100644
> --- a/hw/arm/meson.build
> +++ b/hw/arm/meson.build
> @@ -86,6 +86,7 @@ arm_common_ss.add(when: 'CONFIG_FSL_IMX8MP', if_true: files('fsl-imx8mp.c'))
> arm_common_ss.add(when: 'CONFIG_FSL_IMX8MP_EVK', if_true: files('imx8mp-evk.c'))
> arm_ss.add(when: 'CONFIG_ARM_SMMUV3', if_true: files('smmuv3.c'))
> arm_ss.add(when: 'CONFIG_ARM_SMMUV3_ACCEL', if_true: files('smmuv3-accel.c'))
> +arm_ss.add(when: 'CONFIG_TEGRA241_CMDQV', if_true: files('tegra241-cmdqv.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(
> diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
> index 939898c9b0..e50c4b3bb7 100644
> --- a/hw/arm/smmuv3-accel.c
> +++ b/hw/arm/smmuv3-accel.c
> @@ -18,6 +18,7 @@
>
> #include "smmuv3-internal.h"
> #include "smmuv3-accel.h"
> +#include "tegra241-cmdqv.h"
>
> /*
> * The root region aliases the global system memory, and shared_as_sysmem
> @@ -499,10 +500,15 @@ smmuv3_accel_alloc_viommu(SMMUv3State *s, HostIOMMUDeviceIOMMUFD *idev,
> .ste = { SMMU_STE_VALID, 0x0ULL },
> };
> uint32_t s2_hwpt_id = idev->hwpt_id;
> - uint32_t viommu_id, hwpt_id;
> + uint32_t viommu_id = 0, hwpt_id;
> SMMUv3AccelState *accel;
>
> - if (!iommufd_backend_alloc_viommu(idev->iommufd, idev->devid,
> + if (s->tegra241_cmdqv && !tegra241_cmdqv_alloc_viommu(s, idev, &viommu_id,
> + errp)) {
> + return false;
> + }
> +
> + if (!viommu_id && !iommufd_backend_alloc_viommu(idev->iommufd, idev->devid,
> IOMMU_VIOMMU_TYPE_ARM_SMMUV3, s2_hwpt_id,
> NULL, 0, &viommu_id, errp)) {
> return false;
> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> index 9b7b85fb49..02e1a925a4 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -36,6 +36,7 @@
> #include "smmuv3-accel.h"
> #include "smmuv3-internal.h"
> #include "smmu-internal.h"
> +#include "tegra241-cmdqv.h"
>
> #define PTW_RECORD_FAULT(ptw_info, cfg) (((ptw_info).stage == SMMU_STAGE_1 && \
> (cfg)->record_faults) || \
> @@ -2017,6 +2018,9 @@ static void smmu_realize(DeviceState *d, Error **errp)
>
> smmu_init_irq(s, dev);
> smmuv3_init_id_regs(s);
> + if (s->tegra241_cmdqv) {
> + tegra241_cmdqv_init(s);
> + }
> }
>
> static const VMStateDescription vmstate_smmuv3_queue = {
> diff --git a/hw/arm/tegra241-cmdqv.c b/hw/arm/tegra241-cmdqv.c
> new file mode 100644
> index 0000000000..899325877e
> --- /dev/null
> +++ b/hw/arm/tegra241-cmdqv.c
> @@ -0,0 +1,65 @@
> +/*
> + * Copyright (C) 2025, NVIDIA CORPORATION
> + * 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 uint64_t tegra241_cmdqv_read(void *opaque, hwaddr offset, unsigned size)
> +{
> + return 0;
> +}
> +
> +static void tegra241_cmdqv_write(void *opaque, hwaddr offset, uint64_t value,
> + unsigned size)
> +{
> +}
> +
> +static const MemoryRegionOps mmio_cmdqv_ops = {
> + .read = tegra241_cmdqv_read,
> + .write = tegra241_cmdqv_write,
> + .endianness = DEVICE_LITTLE_ENDIAN,
> +};
> +
> +bool tegra241_cmdqv_alloc_viommu(SMMUv3State *s, HostIOMMUDeviceIOMMUFD *idev,
> + uint32_t *out_viommu_id, Error **errp)
> +{
> + Tegra241CMDQV *cmdqv = s->cmdqv;
> +
> + if (!iommufd_backend_alloc_viommu(idev->iommufd, idev->devid,
> + IOMMU_VIOMMU_TYPE_TEGRA241_CMDQV,
> + idev->hwpt_id, &cmdqv->cmdqv_data,
> + sizeof(cmdqv->cmdqv_data), out_viommu_id,
> + errp)) {
> + error_append_hint(errp, "NVIDIA Tegra241 CMDQV is unsupported");
> + s->tegra241_cmdqv = false;
> + return false;
> + }
> + return true;
> +}
> +
> +void tegra241_cmdqv_init(SMMUv3State *s)
> +{
> + SysBusDevice *sbd = SYS_BUS_DEVICE(OBJECT(s));
> + Tegra241CMDQV *cmdqv;
> +
> + if (!s->tegra241_cmdqv) {
> + return;
> + }
> +
> + cmdqv = g_new0(Tegra241CMDQV, 1);
> + memory_region_init_io(&cmdqv->mmio_cmdqv, OBJECT(s), &mmio_cmdqv_ops, cmdqv,
> + "tegra241-cmdqv", TEGRA241_CMDQV_IO_LEN);
> + sysbus_init_mmio(sbd, &cmdqv->mmio_cmdqv);
> + sysbus_init_irq(sbd, &cmdqv->irq);
> + cmdqv->smmu = s;
> + s->cmdqv = cmdqv;
> +}
> diff --git a/hw/arm/tegra241-cmdqv.h b/hw/arm/tegra241-cmdqv.h
> new file mode 100644
> index 0000000000..9bc72b24d9
> --- /dev/null
> +++ b/hw/arm/tegra241-cmdqv.h
> @@ -0,0 +1,40 @@
> +/*
> + * Copyright (C) 2025, NVIDIA CORPORATION
> + * NVIDIA Tegra241 CMDQ-Virtualiisation extension for SMMUv3
> + *
> + * Written by Nicolin Chen, Shameer Kolothum
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#ifndef HW_TEGRA241_CMDQV_H
> +#define HW_TEGRA241_CMDQV_H
> +
> +#include CONFIG_DEVICES
> +
> +#define TEGRA241_CMDQV_IO_LEN 0x50000
> +
> +typedef struct Tegra241CMDQV {
> + struct iommu_viommu_tegra241_cmdqv cmdqv_data;
> + SMMUv3State *smmu;
> + MemoryRegion mmio_cmdqv;
please use a _mr suffix. It eases the review of subsequent patches
Eric
> + qemu_irq irq;
> +} Tegra241CMDQV;
> +
> +#ifdef CONFIG_TEGRA241_CMDQV
> +bool tegra241_cmdqv_alloc_viommu(SMMUv3State *s, HostIOMMUDeviceIOMMUFD *idev,
> + uint32_t *out_viommu_id, Error **errp);
> +void tegra241_cmdqv_init(SMMUv3State *s);
> +#else
> +static inline void tegra241_cmdqv_init(SMMUv3State *s)
> +{
> +}
> +static inline bool
> +tegra241_cmdqv_alloc_viommu(SMMUv3State *s, HostIOMMUDeviceIOMMUFD *idev,
> + uint32_t *out_viommu_id, Error **errp)
> +{
> + return true;
> +}
> +#endif
> +
> +#endif /* HW_TEGRA241_CMDQV_H */
> diff --git a/include/hw/arm/smmuv3.h b/include/hw/arm/smmuv3.h
> index 2d4970fe19..8e56e480a0 100644
> --- a/include/hw/arm/smmuv3.h
> +++ b/include/hw/arm/smmuv3.h
> @@ -73,6 +73,9 @@ struct SMMUv3State {
> bool ats;
> uint8_t oas;
> bool pasid;
> + /* Support for NVIDIA Tegra241 SMMU CMDQV extension */
> + struct Tegra241CMDQV *cmdqv;
> + bool tegra241_cmdqv;
> };
>
> typedef enum {
On 12/10/25 2:37 PM, Shameer Kolothum wrote:
> From: Nicolin Chen <nicolinc@nvidia.com>
>
> Introduce initial support for NVIDIA Tegra241 CMDQ-Virtualisation (CMDQV),
> an extension to SMMUv3 providing virtualizable hardware command queues.
> This adds the basic MMIO handling, and integration hooks in the SMMUv3
> accelerated path. When enabled, the SMMUv3 backend allocates a Tegra241
> specific vIOMMU object via IOMMUFD and exposes a CMDQV MMIO region and
> IRQ to the guest.
>
> The "tegra241-cmdqv" property isn't user visible yet and it will be
> introduced in a later patch once all the supporting pieces are ready.
>
> Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
> Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
> ---
> hw/arm/Kconfig | 5 ++++
> hw/arm/meson.build | 1 +
> hw/arm/smmuv3-accel.c | 10 +++++--
> hw/arm/smmuv3.c | 4 +++
> hw/arm/tegra241-cmdqv.c | 65 +++++++++++++++++++++++++++++++++++++++++
> hw/arm/tegra241-cmdqv.h | 40 +++++++++++++++++++++++++
> include/hw/arm/smmuv3.h | 3 ++
> 7 files changed, 126 insertions(+), 2 deletions(-)
> create mode 100644 hw/arm/tegra241-cmdqv.c
> create mode 100644 hw/arm/tegra241-cmdqv.h
>
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index 702b79a02b..42b6b95285 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -37,6 +37,7 @@ config ARM_VIRT
> select VIRTIO_MEM_SUPPORTED
> select ACPI_CXL
> select ACPI_HMAT
> + select TEGRA241_CMDQV
>
> config CUBIEBOARD
> bool
> @@ -634,6 +635,10 @@ config ARM_SMMUV3_ACCEL
> bool
> depends on ARM_SMMUV3 && IOMMUFD
>
> +config TEGRA241_CMDQV
> + bool
> + depends on ARM_SMMUV3_ACCEL
> +
> config FSL_IMX6UL
> bool
> default y
> diff --git a/hw/arm/meson.build b/hw/arm/meson.build
> index c250487e64..4ec91db50a 100644
> --- a/hw/arm/meson.build
> +++ b/hw/arm/meson.build
> @@ -86,6 +86,7 @@ arm_common_ss.add(when: 'CONFIG_FSL_IMX8MP', if_true: files('fsl-imx8mp.c'))
> arm_common_ss.add(when: 'CONFIG_FSL_IMX8MP_EVK', if_true: files('imx8mp-evk.c'))
> arm_ss.add(when: 'CONFIG_ARM_SMMUV3', if_true: files('smmuv3.c'))
> arm_ss.add(when: 'CONFIG_ARM_SMMUV3_ACCEL', if_true: files('smmuv3-accel.c'))
> +arm_ss.add(when: 'CONFIG_TEGRA241_CMDQV', if_true: files('tegra241-cmdqv.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(
> diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
> index 939898c9b0..e50c4b3bb7 100644
> --- a/hw/arm/smmuv3-accel.c
> +++ b/hw/arm/smmuv3-accel.c
> @@ -18,6 +18,7 @@
>
> #include "smmuv3-internal.h"
> #include "smmuv3-accel.h"
> +#include "tegra241-cmdqv.h"
>
> /*
> * The root region aliases the global system memory, and shared_as_sysmem
> @@ -499,10 +500,15 @@ smmuv3_accel_alloc_viommu(SMMUv3State *s, HostIOMMUDeviceIOMMUFD *idev,
> .ste = { SMMU_STE_VALID, 0x0ULL },
> };
> uint32_t s2_hwpt_id = idev->hwpt_id;
> - uint32_t viommu_id, hwpt_id;
> + uint32_t viommu_id = 0, hwpt_id;
> SMMUv3AccelState *accel;
>
> - if (!iommufd_backend_alloc_viommu(idev->iommufd, idev->devid,
> + if (s->tegra241_cmdqv && !tegra241_cmdqv_alloc_viommu(s, idev, &viommu_id,
> + errp)) {
> + return false;
I am confused. In tegra241_cmdqv_alloc_viommu() it returns false if
alloc_viommu fails. but you seem to reset s->tegra241_cmdqv as if you
would fall back to non cmdqv setup. What do you try do, fallback or
execute either tegra241 code or default code. Or maybe I misunderstand
the uapi call sequence?
I would not reset a property value in general under the hood. If the end
user has set up this option, I guess he expects it to be enabled when he
queries it back, no?
> + }
> +
> + if (!viommu_id && !iommufd_backend_alloc_viommu(idev->iommufd, idev->devid,
> IOMMU_VIOMMU_TYPE_ARM_SMMUV3, s2_hwpt_id,
> NULL, 0, &viommu_id, errp)) {
If this is a specialization of alloc_viommu code, it generally points to
a class or ops interface. You may face such kind of comments later on so
better to justify that choice or adopt a new one ;-)
same for init which is overloaded compared to original implementation.
> return false;
> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> index 9b7b85fb49..02e1a925a4 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -36,6 +36,7 @@
> #include "smmuv3-accel.h"
> #include "smmuv3-internal.h"
> #include "smmu-internal.h"
> +#include "tegra241-cmdqv.h"
>
> #define PTW_RECORD_FAULT(ptw_info, cfg) (((ptw_info).stage == SMMU_STAGE_1 && \
> (cfg)->record_faults) || \
> @@ -2017,6 +2018,9 @@ static void smmu_realize(DeviceState *d, Error **errp)
>
> smmu_init_irq(s, dev);
> smmuv3_init_id_regs(s);
> + if (s->tegra241_cmdqv) {
> + tegra241_cmdqv_init(s);
> + }
> }
>
> static const VMStateDescription vmstate_smmuv3_queue = {
> diff --git a/hw/arm/tegra241-cmdqv.c b/hw/arm/tegra241-cmdqv.c
> new file mode 100644
> index 0000000000..899325877e
> --- /dev/null
> +++ b/hw/arm/tegra241-cmdqv.c
> @@ -0,0 +1,65 @@
> +/*
> + * Copyright (C) 2025, NVIDIA CORPORATION
> + * 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 uint64_t tegra241_cmdqv_read(void *opaque, hwaddr offset, unsigned size)
> +{
> + return 0;
> +}
> +
> +static void tegra241_cmdqv_write(void *opaque, hwaddr offset, uint64_t value,
> + unsigned size)
> +{
> +}
> +
> +static const MemoryRegionOps mmio_cmdqv_ops = {
> + .read = tegra241_cmdqv_read,
> + .write = tegra241_cmdqv_write,
> + .endianness = DEVICE_LITTLE_ENDIAN,
> +};
> +
> +bool tegra241_cmdqv_alloc_viommu(SMMUv3State *s, HostIOMMUDeviceIOMMUFD *idev,
> + uint32_t *out_viommu_id, Error **errp)
> +{
> + Tegra241CMDQV *cmdqv = s->cmdqv;
> +
> + if (!iommufd_backend_alloc_viommu(idev->iommufd, idev->devid,
> + IOMMU_VIOMMU_TYPE_TEGRA241_CMDQV,
> + idev->hwpt_id, &cmdqv->cmdqv_data,
> + sizeof(cmdqv->cmdqv_data), out_viommu_id,
> + errp)) {
> + error_append_hint(errp, "NVIDIA Tegra241 CMDQV is unsupported");
> + s->tegra241_cmdqv = false;
> + return false;
> + }
> + return true;
> +}
> +
> +void tegra241_cmdqv_init(SMMUv3State *s)
> +{
> + SysBusDevice *sbd = SYS_BUS_DEVICE(OBJECT(s));
> + Tegra241CMDQV *cmdqv;
> +
> + if (!s->tegra241_cmdqv) {
> + return;
> + }
> +
> + cmdqv = g_new0(Tegra241CMDQV, 1);
> + memory_region_init_io(&cmdqv->mmio_cmdqv, OBJECT(s), &mmio_cmdqv_ops, cmdqv,
> + "tegra241-cmdqv", TEGRA241_CMDQV_IO_LEN);
> + sysbus_init_mmio(sbd, &cmdqv->mmio_cmdqv);
> + sysbus_init_irq(sbd, &cmdqv->irq);
> + cmdqv->smmu = s;
> + s->cmdqv = cmdqv;
> +}
> diff --git a/hw/arm/tegra241-cmdqv.h b/hw/arm/tegra241-cmdqv.h
> new file mode 100644
> index 0000000000..9bc72b24d9
> --- /dev/null
> +++ b/hw/arm/tegra241-cmdqv.h
> @@ -0,0 +1,40 @@
> +/*
> + * Copyright (C) 2025, NVIDIA CORPORATION
> + * NVIDIA Tegra241 CMDQ-Virtualiisation extension for SMMUv3
> + *
> + * Written by Nicolin Chen, Shameer Kolothum
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#ifndef HW_TEGRA241_CMDQV_H
> +#define HW_TEGRA241_CMDQV_H
> +
> +#include CONFIG_DEVICES
> +
> +#define TEGRA241_CMDQV_IO_LEN 0x50000
> +
> +typedef struct Tegra241CMDQV {
> + struct iommu_viommu_tegra241_cmdqv cmdqv_data;
> + SMMUv3State *smmu;
> + MemoryRegion mmio_cmdqv;
> + qemu_irq irq;
> +} Tegra241CMDQV;
> +
> +#ifdef CONFIG_TEGRA241_CMDQV
> +bool tegra241_cmdqv_alloc_viommu(SMMUv3State *s, HostIOMMUDeviceIOMMUFD *idev,
> + uint32_t *out_viommu_id, Error **errp);
> +void tegra241_cmdqv_init(SMMUv3State *s);
> +#else
> +static inline void tegra241_cmdqv_init(SMMUv3State *s)
> +{
> +}
> +static inline bool
> +tegra241_cmdqv_alloc_viommu(SMMUv3State *s, HostIOMMUDeviceIOMMUFD *idev,
> + uint32_t *out_viommu_id, Error **errp)
> +{
> + return true;
> +}
> +#endif
> +
> +#endif /* HW_TEGRA241_CMDQV_H */
> diff --git a/include/hw/arm/smmuv3.h b/include/hw/arm/smmuv3.h
> index 2d4970fe19..8e56e480a0 100644
> --- a/include/hw/arm/smmuv3.h
> +++ b/include/hw/arm/smmuv3.h
> @@ -73,6 +73,9 @@ struct SMMUv3State {
> bool ats;
> uint8_t oas;
> bool pasid;
> + /* Support for NVIDIA Tegra241 SMMU CMDQV extension */
> + struct Tegra241CMDQV *cmdqv;
> + bool tegra241_cmdqv;
> };
>
> typedef enum {
Thanks
Eric
Hi Eric,
> -----Original Message-----
> From: Eric Auger <eric.auger@redhat.com>
> Sent: 12 January 2026 15:29
> To: Shameer Kolothum <skolothumtho@nvidia.com>; qemu-
> arm@nongnu.org; qemu-devel@nongnu.org
> Cc: peter.maydell@linaro.org; Nicolin Chen <nicolinc@nvidia.com>; Nathan
> Chen <nathanc@nvidia.com>; Matt Ochs <mochs@nvidia.com>; Jason
> Gunthorpe <jgg@nvidia.com>; jonathan.cameron@huawei.com;
> zhangfei.gao@linaro.org; zhenzhong.duan@intel.com; Krishnakant Jaju
> <kjaju@nvidia.com>
> Subject: Re: [RFC PATCH 05/16] hw/arm/tegra241-cmdqv: Add initial
> Tegra241 CMDQ-Virtualisation support
>
> External email: Use caution opening links or attachments
>
>
> On 12/10/25 2:37 PM, Shameer Kolothum wrote:
> > From: Nicolin Chen <nicolinc@nvidia.com>
> >
> > Introduce initial support for NVIDIA Tegra241 CMDQ-Virtualisation
> (CMDQV),
> > an extension to SMMUv3 providing virtualizable hardware command
> queues.
> > This adds the basic MMIO handling, and integration hooks in the SMMUv3
> > accelerated path. When enabled, the SMMUv3 backend allocates a Tegra241
> > specific vIOMMU object via IOMMUFD and exposes a CMDQV MMIO region
> and
> > IRQ to the guest.
> >
> > The "tegra241-cmdqv" property isn't user visible yet and it will be
> > introduced in a later patch once all the supporting pieces are ready.
> >
> > Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
> > Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
> > ---
> > hw/arm/Kconfig | 5 ++++
> > hw/arm/meson.build | 1 +
> > hw/arm/smmuv3-accel.c | 10 +++++--
> > hw/arm/smmuv3.c | 4 +++
> > hw/arm/tegra241-cmdqv.c | 65
> +++++++++++++++++++++++++++++++++++++++++
> > hw/arm/tegra241-cmdqv.h | 40 +++++++++++++++++++++++++
> > include/hw/arm/smmuv3.h | 3 ++
> > 7 files changed, 126 insertions(+), 2 deletions(-)
> > create mode 100644 hw/arm/tegra241-cmdqv.c
> > create mode 100644 hw/arm/tegra241-cmdqv.h
> >
> > diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> > index 702b79a02b..42b6b95285 100644
> > --- a/hw/arm/Kconfig
> > +++ b/hw/arm/Kconfig
> > @@ -37,6 +37,7 @@ config ARM_VIRT
> > select VIRTIO_MEM_SUPPORTED
> > select ACPI_CXL
> > select ACPI_HMAT
> > + select TEGRA241_CMDQV
> >
> > config CUBIEBOARD
> > bool
> > @@ -634,6 +635,10 @@ config ARM_SMMUV3_ACCEL
> > bool
> > depends on ARM_SMMUV3 && IOMMUFD
> >
> > +config TEGRA241_CMDQV
> > + bool
> > + depends on ARM_SMMUV3_ACCEL
> > +
> > config FSL_IMX6UL
> > bool
> > default y
> > diff --git a/hw/arm/meson.build b/hw/arm/meson.build
> > index c250487e64..4ec91db50a 100644
> > --- a/hw/arm/meson.build
> > +++ b/hw/arm/meson.build
> > @@ -86,6 +86,7 @@ arm_common_ss.add(when: 'CONFIG_FSL_IMX8MP',
> if_true: files('fsl-imx8mp.c'))
> > arm_common_ss.add(when: 'CONFIG_FSL_IMX8MP_EVK', if_true:
> files('imx8mp-evk.c'))
> > arm_ss.add(when: 'CONFIG_ARM_SMMUV3', if_true: files('smmuv3.c'))
> > arm_ss.add(when: 'CONFIG_ARM_SMMUV3_ACCEL', if_true:
> files('smmuv3-accel.c'))
> > +arm_ss.add(when: 'CONFIG_TEGRA241_CMDQV', if_true: files('tegra241-
> cmdqv.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(
> > diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
> > index 939898c9b0..e50c4b3bb7 100644
> > --- a/hw/arm/smmuv3-accel.c
> > +++ b/hw/arm/smmuv3-accel.c
> > @@ -18,6 +18,7 @@
> >
> > #include "smmuv3-internal.h"
> > #include "smmuv3-accel.h"
> > +#include "tegra241-cmdqv.h"
> >
> > /*
> > * The root region aliases the global system memory, and
> shared_as_sysmem
> > @@ -499,10 +500,15 @@ smmuv3_accel_alloc_viommu(SMMUv3State *s,
> HostIOMMUDeviceIOMMUFD *idev,
> > .ste = { SMMU_STE_VALID, 0x0ULL },
> > };
> > uint32_t s2_hwpt_id = idev->hwpt_id;
> > - uint32_t viommu_id, hwpt_id;
> > + uint32_t viommu_id = 0, hwpt_id;
> > SMMUv3AccelState *accel;
> >
> > - if (!iommufd_backend_alloc_viommu(idev->iommufd, idev->devid,
> > + if (s->tegra241_cmdqv && !tegra241_cmdqv_alloc_viommu(s, idev,
> &viommu_id,
> > + errp)) {
> > + return false;
> I am confused. In tegra241_cmdqv_alloc_viommu() it returns false if
> alloc_viommu fails. but you seem to reset s->tegra241_cmdqv as if you
> would fall back to non cmdqv setup. What do you try do, fallback or
> execute either tegra241 code or default code. Or maybe I misunderstand
> the uapi call sequence?
No fallback intended. Currently, if the user has enabled tegra241_cmdqv and
tegra241_cmdqv_alloc_viommu() fails, we fail the device init. Sorry about
that to reset s->tegra241_cmdqv and !viommu_id logic, , that was a left
over logic from previous internal branch I had.
@Nicolin, is there any such requirement for a fallback in this case?
> I would not reset a property value in general under the hood. If the end
> user has set up this option, I guess he expects it to be enabled when he
> queries it back, no?
Yes, that’s the idea.
> > + }
> > +
> > + if (!viommu_id && !iommufd_backend_alloc_viommu(idev->iommufd,
> idev->devid,
> > IOMMU_VIOMMU_TYPE_ARM_SMMUV3, s2_hwpt_id,
> > NULL, 0, &viommu_id, errp)) {
>
> If this is a specialization of alloc_viommu code, it generally points to
> a class or ops interface. You may face such kind of comments later on so
> better to justify that choice or adopt a new one ;-)
> same for init which is overloaded compared to original implementation.
I agree. I will take another look at this. Also, this is where any help would
be really appreciated 😊. Please let me know if you have some ideas on
how we can improve this.
Thanks,
Shameer
On Tue, Jan 13, 2026 at 06:41:03AM -0800, Shameer Kolothum wrote:
> > > - if (!iommufd_backend_alloc_viommu(idev->iommufd, idev->devid,
> > > + if (s->tegra241_cmdqv && !tegra241_cmdqv_alloc_viommu(s, idev,
> > &viommu_id,
> > > + errp)) {
> > > + return false;
> > I am confused. In tegra241_cmdqv_alloc_viommu() it returns false if
> > alloc_viommu fails. but you seem to reset s->tegra241_cmdqv as if you
> > would fall back to non cmdqv setup. What do you try do, fallback or
> > execute either tegra241 code or default code. Or maybe I misunderstand
> > the uapi call sequence?
>
> No fallback intended. Currently, if the user has enabled tegra241_cmdqv and
> tegra241_cmdqv_alloc_viommu() fails, we fail the device init. Sorry about
> that to reset s->tegra241_cmdqv and !viommu_id logic, , that was a left
> over logic from previous internal branch I had.
>
> @Nicolin, is there any such requirement for a fallback in this case?
Likely no. I agree that we should do in the cleaner way.
Nicolin
On Wed, Dec 10, 2025 at 01:37:26PM +0000, Shameer Kolothum wrote:
> +void tegra241_cmdqv_init(SMMUv3State *s)
> +{
> + SysBusDevice *sbd = SYS_BUS_DEVICE(OBJECT(s));
> + Tegra241CMDQV *cmdqv;
> +
> + if (!s->tegra241_cmdqv) {
> + return;
> + }
Maybe g_assert?
> +typedef struct Tegra241CMDQV {
> + struct iommu_viommu_tegra241_cmdqv cmdqv_data;
> + SMMUv3State *smmu;
I see all the cmdqv functions want "smmu->s_accel", so maybe store
"s_accel" instead?
> +#ifdef CONFIG_TEGRA241_CMDQV
> +bool tegra241_cmdqv_alloc_viommu(SMMUv3State *s, HostIOMMUDeviceIOMMUFD *idev,
> + uint32_t *out_viommu_id, Error **errp);
> +void tegra241_cmdqv_init(SMMUv3State *s);
> +#else
> +static inline void tegra241_cmdqv_init(SMMUv3State *s)
> +{
> +}
> +static inline bool
> +tegra241_cmdqv_alloc_viommu(SMMUv3State *s, HostIOMMUDeviceIOMMUFD *idev,
> + uint32_t *out_viommu_id, Error **errp)
> +{
> + return true;
Should it return false?
> index 2d4970fe19..8e56e480a0 100644
> --- a/include/hw/arm/smmuv3.h
> +++ b/include/hw/arm/smmuv3.h
> @@ -73,6 +73,9 @@ struct SMMUv3State {
> bool ats;
> uint8_t oas;
> bool pasid;
> + /* Support for NVIDIA Tegra241 SMMU CMDQV extension */
> + struct Tegra241CMDQV *cmdqv;
> + bool tegra241_cmdqv;
tegra241_cmdqv is a Property, so it has to stay with SMMUv3State.
But "struct Tegra241CMDQV *cmdqv" might be in the SMMUv3AccelState?
Nicolin
> -----Original Message-----
> From: Nicolin Chen <nicolinc@nvidia.com>
> Sent: 29 December 2025 19:07
> To: Shameer Kolothum <skolothumtho@nvidia.com>
> Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org;
> eric.auger@redhat.com; peter.maydell@linaro.org; Nathan Chen
> <nathanc@nvidia.com>; Matt Ochs <mochs@nvidia.com>; Jason Gunthorpe
> <jgg@nvidia.com>; jonathan.cameron@huawei.com;
> zhangfei.gao@linaro.org; zhenzhong.duan@intel.com; Krishnakant Jaju
> <kjaju@nvidia.com>
> Subject: Re: [RFC PATCH 05/16] hw/arm/tegra241-cmdqv: Add initial
> Tegra241 CMDQ-Virtualisation support
>
> On Wed, Dec 10, 2025 at 01:37:26PM +0000, Shameer Kolothum wrote:
> > +void tegra241_cmdqv_init(SMMUv3State *s)
> > +{
> > + SysBusDevice *sbd = SYS_BUS_DEVICE(OBJECT(s));
> > + Tegra241CMDQV *cmdqv;
> > +
> > + if (!s->tegra241_cmdqv) {
> > + return;
> > + }
>
> Maybe g_assert?
Sure.
>
> > +typedef struct Tegra241CMDQV {
> > + struct iommu_viommu_tegra241_cmdqv cmdqv_data;
> > + SMMUv3State *smmu;
>
> I see all the cmdqv functions want "smmu->s_accel", so maybe store
> "s_accel" instead?
Yes. But see below.
> > +#ifdef CONFIG_TEGRA241_CMDQV
> > +bool tegra241_cmdqv_alloc_viommu(SMMUv3State *s,
> HostIOMMUDeviceIOMMUFD *idev,
> > + uint32_t *out_viommu_id, Error **errp);
> > +void tegra241_cmdqv_init(SMMUv3State *s);
> > +#else
> > +static inline void tegra241_cmdqv_init(SMMUv3State *s)
> > +{
> > +}
> > +static inline bool
> > +tegra241_cmdqv_alloc_viommu(SMMUv3State *s,
> HostIOMMUDeviceIOMMUFD *idev,
> > + uint32_t *out_viommu_id, Error **errp)
> > +{
> > + return true;
>
> Should it return false?
Hmm..it should be, given how it is invoked here.
>
> > index 2d4970fe19..8e56e480a0 100644
> > --- a/include/hw/arm/smmuv3.h
> > +++ b/include/hw/arm/smmuv3.h
> > @@ -73,6 +73,9 @@ struct SMMUv3State {
> > bool ats;
> > uint8_t oas;
> > bool pasid;
> > + /* Support for NVIDIA Tegra241 SMMU CMDQV extension */
> > + struct Tegra241CMDQV *cmdqv;
> > + bool tegra241_cmdqv;
>
> tegra241_cmdqv is a Property, so it has to stay with SMMUv3State.
>
> But "struct Tegra241CMDQV *cmdqv" might be in the SMMUv3AccelState?
I have considered this. Currently, SMMUv3AccelState is allocated in
smmuv3_accel_alloc_viommu(), which happens later than tegra241_cmdqv_init().
Changing this would require some rework. I will look into whether we can address
that as part of the next respin of the SMMUv3 accel series to make this cleaner.
Thanks,
Shameer
© 2016 - 2026 Red Hat, Inc.