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@xxxxxxxxx>
> 
> 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