[PATCH] iommu/arm-smmu-v3: Fix VCMDQ indexing in tegra241_vintf0_handle_error

[PATCH] iommu/arm-smmu-v3: Fix VCMDQ indexing in tegra241_vintf0_handle_error

[lirongqing]

From: Li RongQing <lirongqing@baidu.com>

In tegra241_vintf0_handle_error(), the driver loops through the
LVCMDQ_ERR_MAP_64(i) registers to detect and handle error flags for
each virtual command queue (VCMDQ).

However, the code erroneously uses the register-local bit offset
returned by __ffs64(map) directly as the global logical queue index
(lidx) into the vintf->lvcmdqs[] array. When 'i' is greater than 0
(i.e., handling queues 64 and above), this logic incorrectly targets
the queues in the first block (0-63) instead of the intended queues
(i * 64 + bit). This leads to handling errors on the wrong VCMDQ
structures and clearing the wrong hardware error status.

Fix this by properly decoupling the register-local bit offset from the
global array index. Use the local bit offset to clear the local map
snapshot, and use 'i * 64 + bit' to correctly index the global
vintf->lvcmdqs[] array.

Fixes: 918eb5c856f6 ("iommu/arm-smmu-v3: Add in-kernel support for NVIDIA Tegra241 (Grace) CMDQV")
Signed-off-by: Li RongQing <lirongqing@baidu.com>
---
 drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c | 5 +++--
 1 file changed, 3 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 67be62a..33ec466 100644
--- a/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
+++ b/drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c
@@ -317,13 +317,14 @@ static void tegra241_vintf0_handle_error(struct tegra241_vintf *vintf)
 		u64 map = readq_relaxed(REG_VINTF(vintf, LVCMDQ_ERR_MAP_64(i)));
 
 		while (map) {
-			unsigned long lidx = __ffs64(map);
+			unsigned long bit = __ffs64(map);
+			unsigned long lidx = i * 64 + bit;
 			struct tegra241_vcmdq *vcmdq = vintf->lvcmdqs[lidx];
 			u32 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);
+			map &= ~BIT_ULL(bit);
 		}
 	}
 }
-- 
2.9.4

Re: [PATCH] iommu/arm-smmu-v3: Fix VCMDQ indexing in tegra241_vintf0_handle_error

[Nicolin Chen]

On Thu, Jun 18, 2026 at 03:59:45PM +0800, lirongqing wrote:
> From: Li RongQing <lirongqing@baidu.com>
> 
> In tegra241_vintf0_handle_error(), the driver loops through the
> LVCMDQ_ERR_MAP_64(i) registers to detect and handle error flags for
> each virtual command queue (VCMDQ).
> 
> However, the code erroneously uses the register-local bit offset
> returned by __ffs64(map) directly as the global logical queue index

Hmm, what do you mean by "global"? It should be just the logical
index to a VINTF:

  		u64 map = readq_relaxed(REG_VINTF(vintf, LVCMDQ_ERR_MAP_64(i)));

So, nothing "global" here.

> (lidx) into the vintf->lvcmdqs[] array. When 'i' is greater than 0
> (i.e., handling queues 64 and above), this logic incorrectly targets
> the queues in the first block (0-63) instead of the intended queues
> (i * 64 + bit).

This should not be reachable: kernel limits num_lvcmdqs_per_vintf
to 2, covered by the first 64-bit map (i=0); in other words, that
"'i' is greater than 0" shouldn't happen.

> This leads to handling errors on the wrong VCMDQ
> structures and clearing the wrong hardware error status.

Neither should this.

So, I don't think this "bug" requires a separate patch to fix.

With that being said, I have prepared a series of VCMDQ patches,
which I plan to send on rc1. And it does cover this part for a
defensive enhancement:

@@ -352,13 +352,20 @@ static void tegra241_vintf0_handle_error(struct tegra241_vintf *vintf)
 		u64 map = readq_relaxed(REG_VINTF(vintf, LVCMDQ_ERR_MAP_64(i)));
 
 		while (map) {
-			unsigned long lidx = __ffs64(map);
-			struct tegra241_vcmdq *vcmdq = vintf->lvcmdqs[lidx];
-			u32 gerror = readl_relaxed(REG_VCMDQ_PAGE0(vcmdq, GERROR));
+			unsigned long map_bit = __ffs64(map);
+			unsigned long lidx = 64 * i + map_bit;
+			struct tegra241_vcmdq *vcmdq;
+			u32 gerror;
 
+			map &= ~BIT_ULL(map_bit);
+
+			vcmdq = 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);
 		}
 	}
 }

Nicolin