The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH v1 00/11] iommu/tegra241-cmdqv: Fix error-interrupt races and VINTF lifecycle bugs
@ 2026-07-03  5:31 Nicolin Chen
  2026-07-03  5:31 ` [PATCH v1 01/11] iommu/tegra241-cmdqv: Publish an LVCMDQ only after it is fully initialized Nicolin Chen
                   ` (10 more replies)
  0 siblings, 11 replies; 13+ messages in thread
From: Nicolin Chen @ 2026-07-03  5:31 UTC (permalink / raw)
  To: Will Deacon
  Cc: Robin Murphy, Joerg Roedel (AMD), Jason Gunthorpe, linux-tegra,
	linux-arm-kernel, iommu, linux-kernel

These fix a cluster of bugs reported by Sashiko during patch reviews. The
patches are ordered roughly most-critical-first, so some later ones fix
smaller pre-existing issues in the same functions that earlier patches
touch.

Issues fixed:
  - the error ISR racing VINTF (de)init and reading a NULL, freed, or not
    yet fully initialized slot
  - the probe fallback dereferencing an smmu freed by devm_krealloc()
  - a guest vSID programmed without validating its width or the device's
    Stream ID count
  - VINTF0 leaked on an init-failure path
  - error-map index/bounds handling and a VCMDQ base above the 48-bit limit
  - the error ISR flooding the kernel log under repeated guest errors

False positives raised by Sashiko:
  - a viommu outliving an SMMU unbind and touching freed memory on close: a
    physical IOMMU is not a pluggable device, so iommufd holds no reference
    on the one behind a viommu, and this teardown cannot arise.
  - the ISR running after cmdqv is freed on probe failure: free_irq() runs
    first from tegra241_cmdqv_remove(), the devm device_remove action,
    which devres invokes before the cmdqv allocation is released.
  - a guest never acking its VCMDQ error wedging the shared interrupt: the
    interrupt is edge-signaled per event, and the host ISR only snapshots
    the error map into the guest's bounded vEVENTQ, never depending on a
    guest-side GERRORN ack.
  - the ISR accessing a de-assigned LVCMDQ page after a VINTF hw_init()
    failure: the page remains a mapped MMIO region backed by empty
    registers, so reads are benign and writes are dropped.

In parallel to Shameer's Tegra241 CMDQV CMD_SYNC use-after-free fix:
https://lore.kernel.org/all/20260629094106.251694-1-skolothumtho@nvidia.com/

This is on github:
https://github.com/nicolinc/iommufd/commits/fix_cmdqv_sashiko-v1

Nicolin Chen (11):
  iommu/tegra241-cmdqv: Publish an LVCMDQ only after it is fully
    initialized
  iommu/tegra241-cmdqv: Synchronize the error ISR against VINTF (de)init
  iommu/tegra241-cmdqv: Harden error-map index handling in the error ISR
  iommu/tegra241-cmdqv: Don't run the error ISR before probe sets up
    vintfs
  iommu/tegra241-cmdqv: Don't fall back to a freed smmu after
    devm_krealloc()
  iommu/tegra241-cmdqv: Free the error IRQ before tearing down VINTFs
  iommu/tegra241-cmdqv: Reject a vSID wider than the SID_MATCH field
  iommu/tegra241-cmdqv: Require exactly one Stream ID for a vSID
  iommu/tegra241-cmdqv: Fix VINTF0 leak on the init-failure path
  iommu/tegra241-cmdqv: Warn on a VCMDQ base above the 48-bit hardware
    limit
  iommu/tegra241-cmdqv: Rate-limit the error ISR's log message

 .../iommu/arm/arm-smmu-v3/tegra241-cmdqv.c    | 224 ++++++++++++------
 1 file changed, 156 insertions(+), 68 deletions(-)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v1 01/11] iommu/tegra241-cmdqv: Publish an LVCMDQ only after it is fully initialized
  2026-07-03  5:31 [PATCH v1 00/11] iommu/tegra241-cmdqv: Fix error-interrupt races and VINTF lifecycle bugs Nicolin Chen
@ 2026-07-03  5:31 ` Nicolin Chen
  2026-07-03  5:31 ` [PATCH v1 02/11] iommu/tegra241-cmdqv: Synchronize the error ISR against VINTF (de)init Nicolin Chen
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 13+ messages in thread
From: Nicolin Chen @ 2026-07-03  5:31 UTC (permalink / raw)
  To: Will Deacon
  Cc: Robin Murphy, Joerg Roedel (AMD), Jason Gunthorpe, linux-tegra,
	linux-arm-kernel, iommu, linux-kernel

tegra241_vintf_init_lvcmdq() stores the freshly allocated vcmdq pointer to
the vintf->lvcmdqs[] array, before tegra241_vcmdq_alloc_smmu_cmdq() builds
the vcmdq->cmdq. The error ISR dereferences that cmdq, so a latched LVCMDQ
error (e.g. one inherited across a kexec) firing in this window would make
tegra241_vintf0_handle_error() pass the still-zeroed arm_smmu_cmdq down to
__arm_smmu_cmdq_skip_err(), dereferencing NULL queue register pointers.

Drop the store from tegra241_vintf_init_lvcmdq() and publish the vcmdq at
the end of the allocation instead, with an smp_store_release() that pairs
with an smp_load_acquire() in the ISR, which can see a fully built LVCMDQ
or NULL.

The user-owned LVCMDQ allocation moves accordingly, publishing the vcmdq
once tegra241_vcmdq_hw_init_user() succeeds, using a plain store since a
user VINTF's lvcmdqs[] has no lockless reader -- the error ISR only walks
the VINTF0 array.

Fixes: 918eb5c856f6 ("iommu/arm-smmu-v3: Add in-kernel support for NVIDIA Tegra241 (Grace) CMDQV")
Assisted-by: Claude:claude-fable-5
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 .../iommu/arm/arm-smmu-v3/tegra241-cmdqv.c    | 25 +++++++++++++------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c b/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
index 67be62a6e7640..f843d607c8134 100644
--- a/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
+++ b/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
@@ -318,12 +318,19 @@ static void tegra241_vintf0_handle_error(struct tegra241_vintf *vintf)
 
 		while (map) {
 			unsigned long lidx = __ffs64(map);
-			struct tegra241_vcmdq *vcmdq = vintf->lvcmdqs[lidx];
-			u32 gerror = readl_relaxed(REG_VCMDQ_PAGE0(vcmdq, GERROR));
+			struct tegra241_vcmdq *vcmdq;
+			u32 gerror;
 
+			map &= ~BIT_ULL(lidx);
+
+			/* Pairs with smp_store_release() publishing it */
+			vcmdq = smp_load_acquire(&vintf->lvcmdqs[lidx]);
+			if (!vcmdq)
+				continue;
+
+			gerror = readl_relaxed(REG_VCMDQ_PAGE0(vcmdq, GERROR));
 			__arm_smmu_cmdq_skip_err(&vintf->cmdqv->smmu, &vcmdq->cmdq);
 			writel(gerror, REG_VCMDQ_PAGE0(vcmdq, GERRORN));
-			map &= ~BIT_ULL(lidx);
 		}
 	}
 }
@@ -666,7 +673,6 @@ static int tegra241_vintf_init_lvcmdq(struct tegra241_vintf *vintf, u16 lidx,
 	vcmdq->page0 = cmdqv->base + TEGRA241_VINTFi_LVCMDQ_PAGE0(idx, lidx);
 	vcmdq->page1 = cmdqv->base + TEGRA241_VINTFi_LVCMDQ_PAGE1(idx, lidx);
 
-	vintf->lvcmdqs[lidx] = vcmdq;
 	return 0;
 }
 
@@ -705,14 +711,15 @@ tegra241_vintf_alloc_lvcmdq(struct tegra241_vintf *vintf, u16 lidx)
 	/* Build an arm_smmu_cmdq for each LVCMDQ */
 	ret = tegra241_vcmdq_alloc_smmu_cmdq(vcmdq);
 	if (ret)
-		goto deinit_lvcmdq;
+		goto free_vcmdq;
+
+	/* Pairs with the smp_load_acquire() in the error ISR */
+	smp_store_release(&vintf->lvcmdqs[lidx], vcmdq);
 
 	dev_dbg(cmdqv->dev,
 		"%sallocated\n", lvcmdq_error_header(vcmdq, header, 64));
 	return vcmdq;
 
-deinit_lvcmdq:
-	tegra241_vintf_deinit_lvcmdq(vintf, lidx);
 free_vcmdq:
 	kfree(vcmdq);
 	return ERR_PTR(ret);
@@ -1130,13 +1137,15 @@ static int tegra241_vintf_alloc_lvcmdq_user(struct iommufd_hw_queue *hw_queue,
 	if (ret)
 		goto unmap_lvcmdq;
 
+	/* No lockless reader of a user VINTF's lvcmdqs[]; mutex-serialized */
+	vintf->lvcmdqs[lidx] = vcmdq;
+
 	hw_queue->destroy = &tegra241_vintf_destroy_lvcmdq_user;
 	mutex_unlock(&vintf->lvcmdq_mutex);
 	return 0;
 
 unmap_lvcmdq:
 	tegra241_vcmdq_unmap_lvcmdq(vcmdq);
-	tegra241_vintf_deinit_lvcmdq(vintf, lidx);
 undepend_vcmdq:
 	if (vcmdq->prev)
 		iommufd_hw_queue_undepend(vcmdq, vcmdq->prev, core);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v1 02/11] iommu/tegra241-cmdqv: Synchronize the error ISR against VINTF (de)init
  2026-07-03  5:31 [PATCH v1 00/11] iommu/tegra241-cmdqv: Fix error-interrupt races and VINTF lifecycle bugs Nicolin Chen
  2026-07-03  5:31 ` [PATCH v1 01/11] iommu/tegra241-cmdqv: Publish an LVCMDQ only after it is fully initialized Nicolin Chen
@ 2026-07-03  5:31 ` Nicolin Chen
  2026-07-03  5:31 ` [PATCH v1 03/11] iommu/tegra241-cmdqv: Harden error-map index handling in the error ISR Nicolin Chen
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 13+ messages in thread
From: Nicolin Chen @ 2026-07-03  5:31 UTC (permalink / raw)
  To: Will Deacon
  Cc: Robin Murphy, Joerg Roedel (AMD), Jason Gunthorpe, linux-tegra,
	linux-arm-kernel, iommu, linux-kernel

A user VINTF is torn down by tegra241_cmdqv_deinit_vintf(), which runs from
the destroy callback and from the init-failure unwind in the alloc handler.
It clears the cmdqv->vintfs[] slot and lets the iommufd core free it, but
nothing serializes that against the error interrupt: tegra241_cmdqv_isr()
reads cmdqv->vintfs[idx] and dereferences the vintf. A concurrent error can
make the ISR read a slot mid-clear (a NULL deref) or use a vintf which is
about to be freed (a use-after-free).

deinit_vintf() also returns idx to the IDA before clearing the slot, so a
concurrent create that reuses idx can publish its new vintf into the slot,
only for this teardown to erase it again with the stale NULL store.

On the other end, tegra241_cmdqv_init_vintf() publishes a new vintf with a
plain store to the cmdqv->vintfs[] slot, and the ISR dereferences fields of
a published vintf such as vintf->base. A plain store gives no ordering on a
weakly-ordered CPU, and a stale VINTF_ERR_MAP bit on a reused idx can make
the ISR pick a vintf the moment it is published, before its fields are set
or tegra241_vintf_hw_init() runs.

The cmdqv->vintfs[0] slot stays NULL until tegra241_cmdqv_init_structures()
first creates VINTF0, so the slot 0 read needs the same NULL check.

Publish every slot with an smp_store_release(), and read each slot in the
ISR with an smp_load_acquire() under a NULL check, so the ISR always sees
a fully built vintf or NULL. Also make deinit_vintf() clear the slot, and
synchronize_irq() prior to returning idx to the IDA, so no vintf is freed
under a running handler and no reused idx is clobbered.

Fixes: 4dc0d12474f9 ("iommu/tegra241-cmdqv: Add user-space use support")
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 .../iommu/arm/arm-smmu-v3/tegra241-cmdqv.c    | 37 +++++++++++++++++--
 1 file changed, 33 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c b/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
index f843d607c8134..5fbc1c85874a5 100644
--- a/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
+++ b/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
@@ -335,6 +335,13 @@ static void tegra241_vintf0_handle_error(struct tegra241_vintf *vintf)
 	}
 }
 
+/*
+ * The CMDQV error interrupt is edge-triggered, so a pending VINTF error fires
+ * this ISR once and does not re-assert. An unacked guest therefore cannot
+ * storm the host. The HW latches and forwards each new error event on its
+ * own, so an already-set ERR_MAP bit does not suppress the interrupt for a
+ * new error.
+ */
 static irqreturn_t tegra241_cmdqv_isr(int irq, void *devid)
 {
 	struct tegra241_cmdqv *cmdqv = (struct tegra241_cmdqv *)devid;
@@ -357,16 +364,27 @@ static irqreturn_t tegra241_cmdqv_isr(int irq, void *devid)
 
 	/* Handle VINTF0 and its LVCMDQs */
 	if (vintf_map & BIT_ULL(0)) {
-		tegra241_vintf0_handle_error(cmdqv->vintfs[0]);
+		struct tegra241_vintf *vintf0;
+
 		vintf_map &= ~BIT_ULL(0);
+
+		/* NULL until tegra241_cmdqv_init_structures() publishes it */
+		vintf0 = smp_load_acquire(&cmdqv->vintfs[0]);
+		if (vintf0)
+			tegra241_vintf0_handle_error(vintf0);
 	}
 
 	/* Handle other user VINTFs and their LVCMDQs */
 	while (vintf_map) {
 		unsigned long idx = __ffs64(vintf_map);
+		struct tegra241_vintf *vintf;
 
-		tegra241_vintf_user_handle_error(cmdqv->vintfs[idx]);
 		vintf_map &= ~BIT_ULL(idx);
+
+		/* The slot may be published or torn down (NULL'd) concurrently */
+		vintf = smp_load_acquire(&cmdqv->vintfs[idx]);
+		if (vintf)
+			tegra241_vintf_user_handle_error(vintf);
 	}
 
 	return IRQ_HANDLED;
@@ -730,8 +748,18 @@ tegra241_vintf_alloc_lvcmdq(struct tegra241_vintf *vintf, u16 lidx)
 static void tegra241_cmdqv_deinit_vintf(struct tegra241_cmdqv *cmdqv, u16 idx)
 {
 	kfree(cmdqv->vintfs[idx]->lvcmdqs);
+	/*
+	 * Clear the slot and drain any in-flight ISR before returning idx to
+	 * the IDA, so a concurrent create that reuses idx cannot have its
+	 * freshly published VINTF erased here. A plain WRITE_ONCE() suffices
+	 * since clearing the slot publishes no data. This also covers the
+	 * init-failure unwind, which reaches deinit_vintf() without the
+	 * destroy callback.
+	 */
+	WRITE_ONCE(cmdqv->vintfs[idx], NULL);
+	if (cmdqv->irq > 0)
+		synchronize_irq(cmdqv->irq);
 	ida_free(&cmdqv->vintf_ids, idx);
-	cmdqv->vintfs[idx] = NULL;
 }
 
 static int tegra241_cmdqv_init_vintf(struct tegra241_cmdqv *cmdqv, u16 max_idx,
@@ -757,7 +785,8 @@ static int tegra241_cmdqv_init_vintf(struct tegra241_cmdqv *cmdqv, u16 max_idx,
 		return -ENOMEM;
 	}
 
-	cmdqv->vintfs[idx] = vintf;
+	/* Pairs with the smp_load_acquire() in tegra241_cmdqv_isr() */
+	smp_store_release(&cmdqv->vintfs[idx], vintf);
 	return ret;
 }
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v1 03/11] iommu/tegra241-cmdqv: Harden error-map index handling in the error ISR
  2026-07-03  5:31 [PATCH v1 00/11] iommu/tegra241-cmdqv: Fix error-interrupt races and VINTF lifecycle bugs Nicolin Chen
  2026-07-03  5:31 ` [PATCH v1 01/11] iommu/tegra241-cmdqv: Publish an LVCMDQ only after it is fully initialized Nicolin Chen
  2026-07-03  5:31 ` [PATCH v1 02/11] iommu/tegra241-cmdqv: Synchronize the error ISR against VINTF (de)init Nicolin Chen
@ 2026-07-03  5:31 ` Nicolin Chen
  2026-07-03  5:31 ` [PATCH v1 04/11] iommu/tegra241-cmdqv: Don't run the error ISR before probe sets up vintfs Nicolin Chen
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 13+ messages in thread
From: Nicolin Chen @ 2026-07-03  5:31 UTC (permalink / raw)
  To: Will Deacon
  Cc: Robin Murphy, Joerg Roedel (AMD), Jason Gunthorpe, linux-tegra,
	linux-arm-kernel, iommu, linux-kernel

tegra241_vintf0_handle_error() reads both 64-bit LVCMDQ error-map registers
but used the register-local __ffs64() bit directly as the vintf->lvcmdqs[]
index. For the second register that selects the wrong queue instead of 64 *
i + bit, clearing the wrong queue's error status.

The index is unbounded too: a bit at or beyond num_lvcmdqs_per_vintf would
walk the read off vintf->lvcmdqs[].

tegra241_cmdqv_isr() has the same flaw one level up: a VINTF_ERR_MAP bit
at or beyond num_vintfs would walk the read off cmdqv->vintfs[].

Use 64 * i + bit for the index and clear the snapshot with the local bit.
In both handlers, WARN_ON_ONCE() and skip an out-of-bounds index. Only a
malfunctioning device sets such a bit, so the _ONCE form keeps a wedged map
from flooding the log.

Note that 64 * i + bit is not reachable with the current configuration as a
VINTF is pre-assigned with 2 lvcmdqs, this is not treated as bug fix but an
defensive hardening.

Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c b/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
index 5fbc1c85874a5..b89f021ba0b86 100644
--- a/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
+++ b/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
@@ -311,25 +311,30 @@ static void tegra241_vintf_user_handle_error(struct tegra241_vintf *vintf)
 
 static void tegra241_vintf0_handle_error(struct tegra241_vintf *vintf)
 {
+	struct tegra241_cmdqv *cmdqv = vintf->cmdqv;
 	int i;
 
 	for (i = 0; i < LVCMDQ_ERR_MAP_NUM_64; i++) {
 		u64 map = readq_relaxed(REG_VINTF(vintf, LVCMDQ_ERR_MAP_64(i)));
 
 		while (map) {
-			unsigned long lidx = __ffs64(map);
+			unsigned long map_bit = __ffs64(map);
+			unsigned long lidx = 64 * i + map_bit;
 			struct tegra241_vcmdq *vcmdq;
 			u32 gerror;
 
-			map &= ~BIT_ULL(lidx);
+			map &= ~BIT_ULL(map_bit);
 
+			/* A bit beyond the count means a HW error; skip it */
+			if (WARN_ON_ONCE(lidx >= cmdqv->num_lvcmdqs_per_vintf))
+				continue;
 			/* Pairs with smp_store_release() publishing it */
 			vcmdq = smp_load_acquire(&vintf->lvcmdqs[lidx]);
 			if (!vcmdq)
 				continue;
 
 			gerror = readl_relaxed(REG_VCMDQ_PAGE0(vcmdq, GERROR));
-			__arm_smmu_cmdq_skip_err(&vintf->cmdqv->smmu, &vcmdq->cmdq);
+			__arm_smmu_cmdq_skip_err(&cmdqv->smmu, &vcmdq->cmdq);
 			writel(gerror, REG_VCMDQ_PAGE0(vcmdq, GERRORN));
 		}
 	}
@@ -381,6 +386,9 @@ static irqreturn_t tegra241_cmdqv_isr(int irq, void *devid)
 
 		vintf_map &= ~BIT_ULL(idx);
 
+		/* A bit beyond the count means a HW error; skip it */
+		if (WARN_ON_ONCE(idx >= cmdqv->num_vintfs))
+			continue;
 		/* The slot may be published or torn down (NULL'd) concurrently */
 		vintf = smp_load_acquire(&cmdqv->vintfs[idx]);
 		if (vintf)
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v1 04/11] iommu/tegra241-cmdqv: Don't run the error ISR before probe sets up vintfs
  2026-07-03  5:31 [PATCH v1 00/11] iommu/tegra241-cmdqv: Fix error-interrupt races and VINTF lifecycle bugs Nicolin Chen
                   ` (2 preceding siblings ...)
  2026-07-03  5:31 ` [PATCH v1 03/11] iommu/tegra241-cmdqv: Harden error-map index handling in the error ISR Nicolin Chen
@ 2026-07-03  5:31 ` Nicolin Chen
  2026-07-03  5:31 ` [PATCH v1 05/11] iommu/tegra241-cmdqv: Don't fall back to a freed smmu after devm_krealloc() Nicolin Chen
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 13+ messages in thread
From: Nicolin Chen @ 2026-07-03  5:31 UTC (permalink / raw)
  To: Will Deacon
  Cc: Robin Murphy, Joerg Roedel (AMD), Jason Gunthorpe, linux-tegra,
	linux-arm-kernel, iommu, linux-kernel

__tegra241_cmdqv_probe() requests the error IRQ before it has allocated the
cmdqv->vintfs array and set cmdqv->num_vintfs. A CMDQV left enabled with a
latched error across a kexec fires the IRQ as soon as it is requested, and
tegra241_cmdqv_isr() then walks the uninitialized cmdqv->vintfs array.

Request the IRQ only after cmdqv->vintfs is allocated and zeroed, so that
a latched interrupt firing early runs the ISR against a valid array of NULL
slots that it safely skips.

Fixes: 918eb5c856f6 ("iommu/arm-smmu-v3: Add in-kernel support for NVIDIA Tegra241 (Grace) CMDQV")
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 .../iommu/arm/arm-smmu-v3/tegra241-cmdqv.c    | 34 +++++++++++--------
 1 file changed, 19 insertions(+), 15 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c b/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
index b89f021ba0b86..7e366030d9d70 100644
--- a/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
+++ b/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
@@ -973,17 +973,6 @@ __tegra241_cmdqv_probe(struct arm_smmu_device *smmu, struct resource *res,
 	cmdqv->dev = smmu->impl_dev;
 	cmdqv->base_phys = res->start;
 
-	if (cmdqv->irq > 0) {
-		ret = request_threaded_irq(irq, NULL, tegra241_cmdqv_isr,
-					   IRQF_ONESHOT, "tegra241-cmdqv",
-					   cmdqv);
-		if (ret) {
-			dev_err(cmdqv->dev, "failed to request irq (%d): %d\n",
-				cmdqv->irq, ret);
-			goto iounmap;
-		}
-	}
-
 	regval = readl_relaxed(REG_CMDQV(cmdqv, PARAM));
 	cmdqv->num_vintfs = 1 << FIELD_GET(CMDQV_NUM_VINTF_LOG2, regval);
 	cmdqv->num_vcmdqs = 1 << FIELD_GET(CMDQV_NUM_VCMDQ_LOG2, regval);
@@ -994,10 +983,25 @@ __tegra241_cmdqv_probe(struct arm_smmu_device *smmu, struct resource *res,
 	cmdqv->vintfs =
 		kzalloc_objs(*cmdqv->vintfs, cmdqv->num_vintfs);
 	if (!cmdqv->vintfs)
-		goto free_irq;
+		goto iounmap;
 
 	ida_init(&cmdqv->vintf_ids);
 
+	/*
+	 * Request the IRQ only after cmdqv->vintfs is allocated and zeroed, so
+	 * the ISR would not walk an uninitialized array.
+	 */
+	if (cmdqv->irq > 0) {
+		ret = request_threaded_irq(irq, NULL, tegra241_cmdqv_isr,
+					   IRQF_ONESHOT, "tegra241-cmdqv",
+					   cmdqv);
+		if (ret) {
+			dev_err(cmdqv->dev, "failed to request irq (%d): %d\n",
+				cmdqv->irq, ret);
+			goto free_vintfs;
+		}
+	}
+
 #ifdef CONFIG_IOMMU_DEBUGFS
 	if (!cmdqv_debugfs_dir) {
 		cmdqv_debugfs_dir =
@@ -1012,9 +1016,9 @@ __tegra241_cmdqv_probe(struct arm_smmu_device *smmu, struct resource *res,
 
 	return new_smmu;
 
-free_irq:
-	if (cmdqv->irq > 0)
-		free_irq(cmdqv->irq, cmdqv);
+free_vintfs:
+	ida_destroy(&cmdqv->vintf_ids);
+	kfree(cmdqv->vintfs);
 iounmap:
 	iounmap(base);
 	return NULL;
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v1 05/11] iommu/tegra241-cmdqv: Don't fall back to a freed smmu after devm_krealloc()
  2026-07-03  5:31 [PATCH v1 00/11] iommu/tegra241-cmdqv: Fix error-interrupt races and VINTF lifecycle bugs Nicolin Chen
                   ` (3 preceding siblings ...)
  2026-07-03  5:31 ` [PATCH v1 04/11] iommu/tegra241-cmdqv: Don't run the error ISR before probe sets up vintfs Nicolin Chen
@ 2026-07-03  5:31 ` Nicolin Chen
  2026-07-03  5:31 ` [PATCH v1 06/11] iommu/tegra241-cmdqv: Free the error IRQ before tearing down VINTFs Nicolin Chen
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 13+ messages in thread
From: Nicolin Chen @ 2026-07-03  5:31 UTC (permalink / raw)
  To: Will Deacon
  Cc: Robin Murphy, Joerg Roedel (AMD), Jason Gunthorpe, linux-tegra,
	linux-arm-kernel, iommu, linux-kernel

__tegra241_cmdqv_probe() uses devm_krealloc() to grow @smmu into the larger
tegra241_cmdqv, which frees the original @smmu once it relocates. A failure
after that returned NULL, and the caller then dereferenced the freed @smmu
on its fallback path.

Return an int and take @smmu by reference instead, then update *smmu to the
reallocated pointer after devm_krealloc() succeeds, so the caller and its
fallback path both use the live @smmu rather than the freed original.

Fixes: 918eb5c856f6 ("iommu/arm-smmu-v3: Add in-kernel support for NVIDIA Tegra241 (Grace) CMDQV")
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 .../iommu/arm/arm-smmu-v3/tegra241-cmdqv.c    | 54 +++++++++++--------
 1 file changed, 32 insertions(+), 22 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c b/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
index 7e366030d9d70..262798cd6b8a8 100644
--- a/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
+++ b/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
@@ -934,16 +934,22 @@ static int tegra241_cmdqv_init_structures(struct arm_smmu_device *smmu)
 static struct dentry *cmdqv_debugfs_dir;
 #endif
 
-static struct arm_smmu_device *
-__tegra241_cmdqv_probe(struct arm_smmu_device *smmu, struct resource *res,
-		       int irq)
+/*
+ * Probe the CMDQV and reallocate @smmu into the larger cmdqv->smmu.
+ *
+ * devm_krealloc() may relocate and free the original @smmu, so update *smmu to
+ * the new pointer once it succeeds. The error paths after it do the same, so a
+ * caller falling back keeps a live @smmu instead of the freed original.
+ */
+static int __tegra241_cmdqv_probe(struct arm_smmu_device **smmu,
+				  struct resource *res, int irq)
 {
 	static const struct arm_smmu_impl_ops init_ops = {
 		.init_structures = tegra241_cmdqv_init_structures,
 		.device_remove = tegra241_cmdqv_remove,
 	};
-	struct tegra241_cmdqv *cmdqv = NULL;
-	struct arm_smmu_device *new_smmu;
+	struct device *dev = (*smmu)->dev;
+	struct tegra241_cmdqv *cmdqv;
 	void __iomem *base;
 	u32 regval;
 	int ret;
@@ -952,25 +958,28 @@ __tegra241_cmdqv_probe(struct arm_smmu_device *smmu, struct resource *res,
 
 	base = ioremap(res->start, resource_size(res));
 	if (!base) {
-		dev_err(smmu->dev, "failed to ioremap\n");
-		return NULL;
+		dev_err(dev, "failed to ioremap\n");
+		return -ENOMEM;
 	}
 
 	regval = readl(base + TEGRA241_CMDQV_CONFIG);
 	if (disable_cmdqv) {
-		dev_info(smmu->dev, "Detected disable_cmdqv=true\n");
+		dev_info(dev, "Detected disable_cmdqv=true\n");
 		writel(regval & ~CMDQV_EN, base + TEGRA241_CMDQV_CONFIG);
+		ret = -ENODEV;
 		goto iounmap;
 	}
 
-	cmdqv = devm_krealloc(smmu->dev, smmu, sizeof(*cmdqv), GFP_KERNEL);
-	if (!cmdqv)
+	cmdqv = devm_krealloc(dev, *smmu, sizeof(*cmdqv), GFP_KERNEL);
+	if (!cmdqv) {
+		ret = -ENOMEM;
 		goto iounmap;
-	new_smmu = &cmdqv->smmu;
+	}
+	*smmu = &cmdqv->smmu;
 
 	cmdqv->irq = irq;
 	cmdqv->base = base;
-	cmdqv->dev = smmu->impl_dev;
+	cmdqv->dev = (*smmu)->impl_dev;
 	cmdqv->base_phys = res->start;
 
 	regval = readl_relaxed(REG_CMDQV(cmdqv, PARAM));
@@ -982,8 +991,10 @@ __tegra241_cmdqv_probe(struct arm_smmu_device *smmu, struct resource *res,
 
 	cmdqv->vintfs =
 		kzalloc_objs(*cmdqv->vintfs, cmdqv->num_vintfs);
-	if (!cmdqv->vintfs)
+	if (!cmdqv->vintfs) {
+		ret = -ENOMEM;
 		goto iounmap;
+	}
 
 	ida_init(&cmdqv->vintf_ids);
 
@@ -1012,24 +1023,23 @@ __tegra241_cmdqv_probe(struct arm_smmu_device *smmu, struct resource *res,
 #endif
 
 	/* Provide init-level ops only, until tegra241_cmdqv_init_structures */
-	new_smmu->impl_ops = &init_ops;
+	cmdqv->smmu.impl_ops = &init_ops;
 
-	return new_smmu;
+	return 0;
 
 free_vintfs:
 	ida_destroy(&cmdqv->vintf_ids);
 	kfree(cmdqv->vintfs);
 iounmap:
 	iounmap(base);
-	return NULL;
+	return ret;
 }
 
 struct arm_smmu_device *tegra241_cmdqv_probe(struct arm_smmu_device *smmu)
 {
 	struct platform_device *pdev = to_platform_device(smmu->impl_dev);
-	struct arm_smmu_device *new_smmu;
 	struct resource *res;
-	int irq;
+	int irq, ret;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!res) {
@@ -1042,15 +1052,15 @@ struct arm_smmu_device *tegra241_cmdqv_probe(struct arm_smmu_device *smmu)
 		dev_warn(&pdev->dev,
 			 "no interrupt. errors will not be reported\n");
 
-	new_smmu = __tegra241_cmdqv_probe(smmu, res, irq);
-	if (new_smmu)
-		return new_smmu;
+	ret = __tegra241_cmdqv_probe(&smmu, res, irq);
+	if (!ret)
+		return smmu;
 
 out_fallback:
 	dev_info(smmu->impl_dev, "Falling back to standard SMMU CMDQ\n");
 	smmu->options &= ~ARM_SMMU_OPT_TEGRA241_CMDQV;
 	put_device(smmu->impl_dev);
-	return ERR_PTR(-ENODEV);
+	return smmu;
 }
 
 /* User space VINTF and VCMDQ Functions */
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v1 06/11] iommu/tegra241-cmdqv: Free the error IRQ before tearing down VINTFs
  2026-07-03  5:31 [PATCH v1 00/11] iommu/tegra241-cmdqv: Fix error-interrupt races and VINTF lifecycle bugs Nicolin Chen
                   ` (4 preceding siblings ...)
  2026-07-03  5:31 ` [PATCH v1 05/11] iommu/tegra241-cmdqv: Don't fall back to a freed smmu after devm_krealloc() Nicolin Chen
@ 2026-07-03  5:31 ` Nicolin Chen
  2026-07-03  5:31 ` [PATCH v1 07/11] iommu/tegra241-cmdqv: Reject a vSID wider than the SID_MATCH field Nicolin Chen
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 13+ messages in thread
From: Nicolin Chen @ 2026-07-03  5:31 UTC (permalink / raw)
  To: Will Deacon
  Cc: Robin Murphy, Joerg Roedel (AMD), Jason Gunthorpe, linux-tegra,
	linux-arm-kernel, iommu, linux-kernel

tegra241_cmdqv_remove() tears each VINTF down first, then calls free_irq().
Tearing a VINTF down frees vintf0 and clears cmdqv->vintfs[0]. An error in
that window makes tegra241_cmdqv_isr() read the stale slot and hand it to
tegra241_vintf0_handle_error(), which dereferences a NULL or freed pointer.

Free the IRQ before tearing the VINTFs down. free_irq() waits for in-flight
handlers to finish and blocks new ones, so no ISR can observe a VINTF as it
is torn down.

Note: a user-owned VINTF (viommu) could outlive this teardown, which unmaps
cmdqv->base and frees cmdqv->vintfs, so a later viommu close then touches
freed memory. This is neither introduced nor fixed here: a physical IOMMU
is not a pluggable device, so iommufd by design holds no reference on the
one behind a viommu, and this teardown is not expected while that viommu is
still alive.

Fixes: 918eb5c856f6 ("iommu/arm-smmu-v3: Add in-kernel support for NVIDIA Tegra241 (Grace) CMDQV")
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c b/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
index 262798cd6b8a8..fbf7ecf043a8c 100644
--- a/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
+++ b/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
@@ -829,6 +829,14 @@ static void tegra241_cmdqv_remove(struct arm_smmu_device *smmu)
 		container_of(smmu, struct tegra241_cmdqv, smmu);
 	u16 idx;
 
+	/*
+	 * Free the IRQ before tearing down the VINTFs. free_irq() waits for any
+	 * in-flight tegra241_cmdqv_isr() to finish and blocks new ones, so the
+	 * ISR cannot dereference a VINTF that is freed by the loop below.
+	 */
+	if (cmdqv->irq > 0)
+		free_irq(cmdqv->irq, cmdqv);
+
 	/* Remove VINTF resources */
 	for (idx = 0; idx < cmdqv->num_vintfs; idx++) {
 		if (cmdqv->vintfs[idx]) {
@@ -841,8 +849,6 @@ static void tegra241_cmdqv_remove(struct arm_smmu_device *smmu)
 	/* Remove cmdqv resources */
 	ida_destroy(&cmdqv->vintf_ids);
 
-	if (cmdqv->irq > 0)
-		free_irq(cmdqv->irq, cmdqv);
 	iounmap(cmdqv->base);
 	kfree(cmdqv->vintfs);
 	put_device(cmdqv->dev); /* smmu->impl_dev */
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v1 07/11] iommu/tegra241-cmdqv: Reject a vSID wider than the SID_MATCH field
  2026-07-03  5:31 [PATCH v1 00/11] iommu/tegra241-cmdqv: Fix error-interrupt races and VINTF lifecycle bugs Nicolin Chen
                   ` (5 preceding siblings ...)
  2026-07-03  5:31 ` [PATCH v1 06/11] iommu/tegra241-cmdqv: Free the error IRQ before tearing down VINTFs Nicolin Chen
@ 2026-07-03  5:31 ` Nicolin Chen
  2026-07-03  5:31 ` [PATCH v1 08/11] iommu/tegra241-cmdqv: Require exactly one Stream ID for a vSID Nicolin Chen
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 13+ messages in thread
From: Nicolin Chen @ 2026-07-03  5:31 UTC (permalink / raw)
  To: Will Deacon
  Cc: Robin Murphy, Joerg Roedel (AMD), Jason Gunthorpe, linux-tegra,
	linux-arm-kernel, iommu, linux-kernel

tegra241_vintf_init_vsid() programs the guest-provided vSID into SID_MATCH,
whose VIRT_SID field spans bits [20:1] with bit 0 as the match-enable flag.
The HW therefore matches only a 20-bit Stream ID.

The bound check rejects only virt_sid > UINT_MAX, which admits a value far
wider than the field. The write "virt_sid << 1 | 0x1" then drops every bit
above 20: a virt_sid of 0x80000000 lands as SID_MATCH = 0x1, a valid match
on vSID 0, so the entry aliases the wrong Stream ID. Because vdev->virt_id
is guest-controlled, a VMM can trigger it.

Validate virt_sid against the field width with FIELD_MAX(), and program the
register with FIELD_PREP() so the value and the field stay consistent.

Fixes: 4dc0d12474f9 ("iommu/tegra241-cmdqv: Add user-space use support")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c b/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
index fbf7ecf043a8c..812cc500b4a1e 100644
--- a/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
+++ b/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
@@ -56,6 +56,8 @@
 #define  VINTF_ENABLED			BIT(0)
 
 #define TEGRA241_VINTF_SID_MATCH(s)	(0x0040 + 0x4*(s))
+#define  VINTF_SID_MATCH_VIRT_SID	GENMASK(20, 1)
+#define  VINTF_SID_MATCH_ENABLE		BIT(0)
 #define TEGRA241_VINTF_SID_REPLACE(s)	(0x0080 + 0x4*(s))
 
 #define TEGRA241_VINTF_LVCMDQ_ERR_MAP_64(m) \
@@ -1244,7 +1246,7 @@ static int tegra241_vintf_init_vsid(struct iommufd_vdevice *vdev)
 	u64 virt_sid = vdev->virt_id;
 	int sidx;
 
-	if (virt_sid > UINT_MAX)
+	if (virt_sid > FIELD_MAX(VINTF_SID_MATCH_VIRT_SID))
 		return -EINVAL;
 
 	WARN_ON_ONCE(master->num_streams != 1);
@@ -1256,7 +1258,9 @@ static int tegra241_vintf_init_vsid(struct iommufd_vdevice *vdev)
 		return sidx;
 
 	writel(stream->id, REG_VINTF(vintf, SID_REPLACE(sidx)));
-	writel(virt_sid << 1 | 0x1, REG_VINTF(vintf, SID_MATCH(sidx)));
+	writel(FIELD_PREP(VINTF_SID_MATCH_VIRT_SID, virt_sid) |
+		       VINTF_SID_MATCH_ENABLE,
+	       REG_VINTF(vintf, SID_MATCH(sidx)));
 	dev_dbg(vintf->cmdqv->dev,
 		"VINTF%u: allocated SID_REPLACE%d for pSID=%x, vSID=%x\n",
 		vintf->idx, sidx, stream->id, (u32)virt_sid);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v1 08/11] iommu/tegra241-cmdqv: Require exactly one Stream ID for a vSID
  2026-07-03  5:31 [PATCH v1 00/11] iommu/tegra241-cmdqv: Fix error-interrupt races and VINTF lifecycle bugs Nicolin Chen
                   ` (6 preceding siblings ...)
  2026-07-03  5:31 ` [PATCH v1 07/11] iommu/tegra241-cmdqv: Reject a vSID wider than the SID_MATCH field Nicolin Chen
@ 2026-07-03  5:31 ` Nicolin Chen
  2026-07-03  7:11   ` Nicolin Chen
  2026-07-03  5:31 ` [PATCH v1 09/11] iommu/tegra241-cmdqv: Fix VINTF0 leak on the init-failure path Nicolin Chen
                   ` (2 subsequent siblings)
  10 siblings, 1 reply; 13+ messages in thread
From: Nicolin Chen @ 2026-07-03  5:31 UTC (permalink / raw)
  To: Will Deacon
  Cc: Robin Murphy, Joerg Roedel (AMD), Jason Gunthorpe, linux-tegra,
	linux-arm-kernel, iommu, linux-kernel

tegra241_vintf_init_vsid() maps a guest vSID to a single physical Stream ID
taken from master->streams[0], and only warns when the device does not have
exactly one stream. A device with several streams gets only its first one
mapped, so a guest vSID invalidation cannot reach the others' ATC and IOTLB
entries; a device with none makes master->streams a ZERO_SIZE_PTR, read out
of bounds.

Reject the mapping with -EINVAL when master->num_streams is not one, rather
than warning and pressing on.

Fixes: 4dc0d12474f9 ("iommu/tegra241-cmdqv: Add user-space use support")
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c b/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
index 812cc500b4a1e..aa0568e328356 100644
--- a/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
+++ b/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
@@ -1249,7 +1249,8 @@ static int tegra241_vintf_init_vsid(struct iommufd_vdevice *vdev)
 	if (virt_sid > FIELD_MAX(VINTF_SID_MATCH_VIRT_SID))
 		return -EINVAL;
 
-	WARN_ON_ONCE(master->num_streams != 1);
+	if (master->num_streams != 1)
+		return -EINVAL;
 
 	/* Find an empty pair of SID_REPLACE and SID_MATCH */
 	sidx = ida_alloc_max(&vintf->sids, vintf->cmdqv->num_sids_per_vintf - 1,
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v1 09/11] iommu/tegra241-cmdqv: Fix VINTF0 leak on the init-failure path
  2026-07-03  5:31 [PATCH v1 00/11] iommu/tegra241-cmdqv: Fix error-interrupt races and VINTF lifecycle bugs Nicolin Chen
                   ` (7 preceding siblings ...)
  2026-07-03  5:31 ` [PATCH v1 08/11] iommu/tegra241-cmdqv: Require exactly one Stream ID for a vSID Nicolin Chen
@ 2026-07-03  5:31 ` Nicolin Chen
  2026-07-03  5:31 ` [PATCH v1 10/11] iommu/tegra241-cmdqv: Warn on a VCMDQ base above the 48-bit hardware limit Nicolin Chen
  2026-07-03  5:31 ` [PATCH v1 11/11] iommu/tegra241-cmdqv: Rate-limit the error ISR's log message Nicolin Chen
  10 siblings, 0 replies; 13+ messages in thread
From: Nicolin Chen @ 2026-07-03  5:31 UTC (permalink / raw)
  To: Will Deacon
  Cc: Robin Murphy, Joerg Roedel (AMD), Jason Gunthorpe, linux-tegra,
	linux-arm-kernel, iommu, linux-kernel

tegra241_cmdqv_init_structures() allocates VINTF0 with kzalloc_obj(), inits
it, and preallocates its logical VCMDQs. Two of its error paths leak.

When tegra241_cmdqv_init_vintf() fails it returns before VINTF0 reaches the
cmdqv->vintfs[] array, so the devres unwind on probe failure cannot reach
it; free it directly there.

A later VCMDQ preallocation failure instead leaves VINTF0 published, and so
this time the unwind does reach tegra241_cmdqv_remove_vintf(), which then
frees it from vintf->hyp_own. But tegra241_vintf_hw_init() sets that flag
only afterward, from a HW read-back, so the still-uninited VINTF0 reads as
guest-owned and leaks, with mutex_destroy() and ida_destroy() run on fields
it never set up.

Decide ownership from vintf->idx instead, the index assigned when its id is
allocated: idx 0 is the kernel-owned VINTF0, while idx >= 1 marks a guest
VINTF. So the in-kernel free decision in tegra241_cmdqv_remove_vintf() and
tegra241_vintf_free_lvcmdq() now keys on idx too, and hyp_own stays a pure
HW-readback state.

Fixes: 918eb5c856f6 ("iommu/arm-smmu-v3: Add in-kernel support for NVIDIA Tegra241 (Grace) CMDQV")
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c b/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
index aa0568e328356..2d832d633d030 100644
--- a/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
+++ b/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
@@ -716,7 +716,7 @@ static void tegra241_vintf_free_lvcmdq(struct tegra241_vintf *vintf, u16 lidx)
 	dev_dbg(vintf->cmdqv->dev,
 		"%sdeallocated\n", lvcmdq_error_header(vcmdq, header, 64));
 	/* Guest-owned VCMDQ is free-ed with hw_queue by iommufd core */
-	if (vcmdq->vintf->hyp_own)
+	if (!vcmdq->vintf->idx)
 		kfree(vcmdq);
 }
 
@@ -816,7 +816,7 @@ static void tegra241_cmdqv_remove_vintf(struct tegra241_cmdqv *cmdqv, u16 idx)
 
 	dev_dbg(cmdqv->dev, "VINTF%u: deallocated\n", vintf->idx);
 	tegra241_cmdqv_deinit_vintf(cmdqv, idx);
-	if (!vintf->hyp_own) {
+	if (vintf->idx) {
 		mutex_destroy(&vintf->lvcmdq_mutex);
 		ida_destroy(&vintf->sids);
 		/* Guest-owned VINTF is free-ed with viommu by iommufd core */
@@ -921,6 +921,12 @@ static int tegra241_cmdqv_init_structures(struct arm_smmu_device *smmu)
 	ret = tegra241_cmdqv_init_vintf(cmdqv, 0, vintf);
 	if (ret) {
 		dev_err(cmdqv->dev, "failed to init vintf0: %d\n", ret);
+		/*
+		 * tegra241_cmdqv_init_vintf() failed to publish the vintf0 to
+		 * cmdqv->vintfs[], so the probe unwind path that goes through
+		 * cmdqv->vintfs[] would miss it. Free it here.
+		 */
+		kfree(vintf);
 		return ret;
 	}
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v1 10/11] iommu/tegra241-cmdqv: Warn on a VCMDQ base above the 48-bit hardware limit
  2026-07-03  5:31 [PATCH v1 00/11] iommu/tegra241-cmdqv: Fix error-interrupt races and VINTF lifecycle bugs Nicolin Chen
                   ` (8 preceding siblings ...)
  2026-07-03  5:31 ` [PATCH v1 09/11] iommu/tegra241-cmdqv: Fix VINTF0 leak on the init-failure path Nicolin Chen
@ 2026-07-03  5:31 ` Nicolin Chen
  2026-07-03  5:31 ` [PATCH v1 11/11] iommu/tegra241-cmdqv: Rate-limit the error ISR's log message Nicolin Chen
  10 siblings, 0 replies; 13+ messages in thread
From: Nicolin Chen @ 2026-07-03  5:31 UTC (permalink / raw)
  To: Will Deacon
  Cc: Robin Murphy, Joerg Roedel (AMD), Jason Gunthorpe, linux-tegra,
	linux-arm-kernel, iommu, linux-kernel

tegra241_vcmdq_alloc_smmu_cmdq() allocates the VCMDQ buffer via the common
arm_smmu_init_one_queue(), which uses smmu->dev and its coherent DMA mask
of DMA_BIT_MASK(smmu->oas). The architectural Q_BASE_ADDR_MASK is 52 bits,
but the Tegra241 VCMDQ_BASE holds only 48 (VCMDQ_ADDR), so the masked write
"q_base = base_dma & VCMDQ_ADDR" silently drops bits 48 and up.

A real Tegra241 never reports a 52-bit OAS alongside the 48-bit VCMDQ, so
this cannot happen on correct hardware, but a buggy or hostile hypervisor
could still advertise such an OAS to a guest. The user-VCMDQ path already
rejects a base_addr_pa with bits set outside VCMDQ_ADDR; add the same guard
to the kernel-allocated path, failing the queue init rather than silently
truncating the base. Use dev_warn_once() rather than WARN_ON(): the OAS is
hypervisor-controlled, and a WARN_ON() would let it panic a guest that is
booted with panic_on_warn.

Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c b/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
index 2d832d633d030..41046605de9ca 100644
--- a/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
+++ b/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
@@ -674,6 +674,19 @@ static int tegra241_vcmdq_alloc_smmu_cmdq(struct tegra241_vcmdq *vcmdq)
 	if (ret)
 		return ret;
 
+	/*
+	 * The q->base_dma is bounded by DMA_BIT_MASK of SMMU's IDR5.OAS. On a
+	 * real hardware, VCMDQ_ADDR mask and IDR5.OAS are always aligned. But
+	 * a buggy VM might set a mismatched IDR5.OAS for SMMU. Spit a warning
+	 * instead of a silent truncation.
+	 */
+	if (q->base_dma & ~VCMDQ_ADDR) {
+		dev_warn_once(
+			vcmdq->cmdqv->dev,
+			"VCMDQ base %pad exceeds the 48-bit VCMDQ_ADDR limit\n",
+			&q->base_dma);
+		return -EINVAL;
+	}
 	/* ...override q_base to write VCMDQ_BASE registers */
 	q->q_base = q->base_dma & VCMDQ_ADDR;
 	q->q_base |= FIELD_PREP(VCMDQ_LOG2SIZE, q->llq.max_n_shift);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v1 11/11] iommu/tegra241-cmdqv: Rate-limit the error ISR's log message
  2026-07-03  5:31 [PATCH v1 00/11] iommu/tegra241-cmdqv: Fix error-interrupt races and VINTF lifecycle bugs Nicolin Chen
                   ` (9 preceding siblings ...)
  2026-07-03  5:31 ` [PATCH v1 10/11] iommu/tegra241-cmdqv: Warn on a VCMDQ base above the 48-bit hardware limit Nicolin Chen
@ 2026-07-03  5:31 ` Nicolin Chen
  10 siblings, 0 replies; 13+ messages in thread
From: Nicolin Chen @ 2026-07-03  5:31 UTC (permalink / raw)
  To: Will Deacon
  Cc: Robin Murphy, Joerg Roedel (AMD), Jason Gunthorpe, linux-tegra,
	linux-arm-kernel, iommu, linux-kernel

tegra241_cmdqv_isr() logs the error-map registers on every error interrupt.
A malfunctioning device, or a guest deliberately faulting its own VCMDQs,
can raise these interrupts rapidly, and the unconditional dev_warn() then
floods the kernel log.

Rate-limit the message with dev_warn_ratelimited(), and pass the error-map
registers straight to it so their four MMIO reads run only when the limiter
prints, instead of building the string on every interrupt.

Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c b/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
index 41046605de9ca..21a12a20e7436 100644
--- a/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
+++ b/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
@@ -353,21 +353,19 @@ static irqreturn_t tegra241_cmdqv_isr(int irq, void *devid)
 {
 	struct tegra241_cmdqv *cmdqv = (struct tegra241_cmdqv *)devid;
 	void __iomem *reg_vintf_map = REG_CMDQV(cmdqv, VINTF_ERR_MAP);
-	char err_str[256];
 	u64 vintf_map;
 
 	/* Use readl_relaxed() as register addresses are not 64-bit aligned */
 	vintf_map = (u64)readl_relaxed(reg_vintf_map + 0x4) << 32 |
 		    (u64)readl_relaxed(reg_vintf_map);
 
-	snprintf(err_str, sizeof(err_str),
-		 "vintf_map: %016llx, vcmdq_map %08x:%08x:%08x:%08x", vintf_map,
-		 readl_relaxed(REG_CMDQV(cmdqv, CMDQ_ERR_MAP(3))),
-		 readl_relaxed(REG_CMDQV(cmdqv, CMDQ_ERR_MAP(2))),
-		 readl_relaxed(REG_CMDQV(cmdqv, CMDQ_ERR_MAP(1))),
-		 readl_relaxed(REG_CMDQV(cmdqv, CMDQ_ERR_MAP(0))));
-
-	dev_warn(cmdqv->dev, "unexpected error reported. %s\n", err_str);
+	dev_warn_ratelimited(
+		cmdqv->dev,
+		"unexpected error reported. vintf_map: %016llx, vcmdq_map %08x:%08x:%08x:%08x\n",
+		vintf_map, readl_relaxed(REG_CMDQV(cmdqv, CMDQ_ERR_MAP(3))),
+		readl_relaxed(REG_CMDQV(cmdqv, CMDQ_ERR_MAP(2))),
+		readl_relaxed(REG_CMDQV(cmdqv, CMDQ_ERR_MAP(1))),
+		readl_relaxed(REG_CMDQV(cmdqv, CMDQ_ERR_MAP(0))));
 
 	/* Handle VINTF0 and its LVCMDQs */
 	if (vintf_map & BIT_ULL(0)) {
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v1 08/11] iommu/tegra241-cmdqv: Require exactly one Stream ID for a vSID
  2026-07-03  5:31 ` [PATCH v1 08/11] iommu/tegra241-cmdqv: Require exactly one Stream ID for a vSID Nicolin Chen
@ 2026-07-03  7:11   ` Nicolin Chen
  0 siblings, 0 replies; 13+ messages in thread
From: Nicolin Chen @ 2026-07-03  7:11 UTC (permalink / raw)
  To: Will Deacon
  Cc: Robin Murphy, Joerg Roedel (AMD), Jason Gunthorpe, linux-tegra,
	linux-arm-kernel, iommu, linux-kernel

On Thu, Jul 02, 2026 at 10:31:34PM -0700, Nicolin Chen wrote:
> diff --git a/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c b/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
> index 812cc500b4a1e..aa0568e328356 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
> @@ -1249,7 +1249,8 @@ static int tegra241_vintf_init_vsid(struct iommufd_vdevice *vdev)
>  	if (virt_sid > FIELD_MAX(VINTF_SID_MATCH_VIRT_SID))
>  		return -EINVAL;
>  
> -	WARN_ON_ONCE(master->num_streams != 1);
> +	if (master->num_streams != 1)
> +		return -EINVAL;

Kevin pointed out in the other smmu patch that this should return
-EOPNOTSUPP instead.
https://lore.kernel.org/linux-iommu/SA2PR11MB4844DD8A0367D8E909123F868CF42@SA2PR11MB4844.namprd11.prod.outlook.com/

So, it'd be better to return -EOPNOTSUPP in both places. I'll fix
this when doing a v2.

Nicolin

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2026-07-03  7:11 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-03  5:31 [PATCH v1 00/11] iommu/tegra241-cmdqv: Fix error-interrupt races and VINTF lifecycle bugs Nicolin Chen
2026-07-03  5:31 ` [PATCH v1 01/11] iommu/tegra241-cmdqv: Publish an LVCMDQ only after it is fully initialized Nicolin Chen
2026-07-03  5:31 ` [PATCH v1 02/11] iommu/tegra241-cmdqv: Synchronize the error ISR against VINTF (de)init Nicolin Chen
2026-07-03  5:31 ` [PATCH v1 03/11] iommu/tegra241-cmdqv: Harden error-map index handling in the error ISR Nicolin Chen
2026-07-03  5:31 ` [PATCH v1 04/11] iommu/tegra241-cmdqv: Don't run the error ISR before probe sets up vintfs Nicolin Chen
2026-07-03  5:31 ` [PATCH v1 05/11] iommu/tegra241-cmdqv: Don't fall back to a freed smmu after devm_krealloc() Nicolin Chen
2026-07-03  5:31 ` [PATCH v1 06/11] iommu/tegra241-cmdqv: Free the error IRQ before tearing down VINTFs Nicolin Chen
2026-07-03  5:31 ` [PATCH v1 07/11] iommu/tegra241-cmdqv: Reject a vSID wider than the SID_MATCH field Nicolin Chen
2026-07-03  5:31 ` [PATCH v1 08/11] iommu/tegra241-cmdqv: Require exactly one Stream ID for a vSID Nicolin Chen
2026-07-03  7:11   ` Nicolin Chen
2026-07-03  5:31 ` [PATCH v1 09/11] iommu/tegra241-cmdqv: Fix VINTF0 leak on the init-failure path Nicolin Chen
2026-07-03  5:31 ` [PATCH v1 10/11] iommu/tegra241-cmdqv: Warn on a VCMDQ base above the 48-bit hardware limit Nicolin Chen
2026-07-03  5:31 ` [PATCH v1 11/11] iommu/tegra241-cmdqv: Rate-limit the error ISR's log message Nicolin Chen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox