[PATCH v4 0/2] drm/v3d: Fix indirect CSD jobs with zeroed workgroups

[PATCH v4 0/2] drm/v3d: Fix indirect CSD jobs with zeroed workgroups

[Maíra Canal]

Hi,

Indirect CSD lets userspace defer the workgroup counts to a GPU buffer
that is only filled at runtime, so the counts are unknown at submission
time and can legitimately turn out to be zero.

However, exercing this case exposed two issues in the CSD path.

  1. Virtual address leaks when the indirect CSD has zeroed workgroups.

  2. CSD jobs with zeroed workgroups shouldn't be submitted to hardware.

This series intends to address both issues.

Tested with the following CTS tests:

 - dEQP-VK.compute.*.indirect_dispatch.upload_buffer.empty_command_x*
 - dEQP-VK.compute.*.indirect_dispatch.upload_buffer.empty_command_y*
 - dEQP-VK.compute.*.indirect_dispatch.upload_buffer.empty_command_z*

Best regards,
- Maíra

---
v1 -> v2: https://lore.kernel.org/r/20260530-v3d-fix-indirect-csd-v1-0-15533948663f@igalia.com

- [2/2] Don't check the whole cfg[0-2], check only the number of workgroups (Iago Toral)
- [2/2] Add a comment about how the HW interprets 0 (Iago Toral)

v2 -> v3: https://lore.kernel.org/r/20260601-v3d-fix-indirect-csd-v2-0-aaebf035b936@igalia.com

- [1/2, 2/2] Add Iago's R-b (Iago Toral)
- [2/2] Adjust the comment to make it more accurate (Iago Toral)

v3 -> v4: https://lore.kernel.org/r/20260602-v3d-fix-indirect-csd-v3-0-cc79e06e543c@igalia.com

- [2/2] Always rewrite CFG[0..2] from the indirect buffer to avoid
        preserving stale contents from user space.

---
Maíra Canal (2):
      drm/v3d: Fix vaddr leak when indirect CSD has zeroed workgroups
      drm/v3d: Skip CSD when it has zeroed workgroups

 drivers/gpu/drm/v3d/v3d_sched.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)
---
base-commit: ae0383e5a9a4b12d68c76c4769857def4665deff
change-id: 20260516-v3d-fix-indirect-csd-6806cf20a57a

[PATCH v4 1/2] drm/v3d: Fix vaddr leak when indirect CSD has zeroed workgroups

[Maíra Canal]

v3d_rewrite_csd_job_wg_counts_from_indirect() maps both the indirect
buffer and the workgroup buffer and is expected to release them before
returning. When any of the workgroup counts read from the buffer is zero,
the function bailed out early and skipped the cleanup, leaking the vaddr
mappings of both BOs.

Jump to the cleanup path instead of returning directly, so the mappings
are always dropped.

Cc: stable@vger.kernel.org
Fixes: 18b8413b25b7 ("drm/v3d: Create a CPU job extension for a indirect CSD job")
Suggested-by: Jose Maria Casanova Crespo <jmcasanova@igalia.com>
Signed-off-by: Maíra Canal <mcanal@igalia.com>
Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>
---
 drivers/gpu/drm/v3d/v3d_sched.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/v3d/v3d_sched.c b/drivers/gpu/drm/v3d/v3d_sched.c
index 94bf628dc91c..47f83936cd73 100644
--- a/drivers/gpu/drm/v3d/v3d_sched.c
+++ b/drivers/gpu/drm/v3d/v3d_sched.c
@@ -403,7 +403,7 @@ v3d_rewrite_csd_job_wg_counts_from_indirect(struct v3d_cpu_job *job)
 	wg_counts = (uint32_t *)(bo->vaddr + indirect_csd->offset);
 
 	if (wg_counts[0] == 0 || wg_counts[1] == 0 || wg_counts[2] == 0)
-		return;
+		goto unmap_bo;
 
 	args->cfg[0] = wg_counts[0] << V3D_CSD_CFG012_WG_COUNT_SHIFT;
 	args->cfg[1] = wg_counts[1] << V3D_CSD_CFG012_WG_COUNT_SHIFT;
@@ -428,6 +428,7 @@ v3d_rewrite_csd_job_wg_counts_from_indirect(struct v3d_cpu_job *job)
 		}
 	}
 
+unmap_bo:
 	v3d_put_bo_vaddr(indirect);
 	v3d_put_bo_vaddr(bo);
 }

-- 
2.54.0

[PATCH v4 2/2] drm/v3d: Skip CSD when it has zeroed workgroups

[Maíra Canal]

A compute shader dispatch encodes its workgroup counts in the CFG0..CFG2
registers. Kicking off a dispatch with a zero count in any of the three
dimensions is invalid. First, the hardware will process 0 as 65536,
while the user-space driver exposes a maximum of 65535. Over that, a
submission with a zeroed workgroup dimension should be a no-op.

These zeroed counts can reach the dispatch path through an indirect CSD
job, whose workgroup counts are only known once the indirect buffer is
read and may legitimately be zero, but such scenario should only result in
a no-op.

Overwrite the indirect CSD job workgroup counts with the indirect BO
ones, even if they are zeroed, and don't submit the job to the hardware
when any of the workgroup counts is zero, so the job completes immediately
instead of running the shader.

Cc: stable@vger.kernel.org
Fixes: d223f98f0209 ("drm/v3d: Add support for compute shader dispatch.")
Suggested-by: Jose Maria Casanova Crespo <jmcasanova@igalia.com>
Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/gpu/drm/v3d/v3d_sched.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/v3d/v3d_sched.c b/drivers/gpu/drm/v3d/v3d_sched.c
index 47f83936cd73..8a635a9ec046 100644
--- a/drivers/gpu/drm/v3d/v3d_sched.c
+++ b/drivers/gpu/drm/v3d/v3d_sched.c
@@ -352,6 +352,16 @@ v3d_csd_job_run(struct drm_sched_job *sched_job)
 		return NULL;
 	}
 
+	/* The HW interprets a workgroup size of 0 as 65536; however, the
+	 * user-space driver exposes a maximum of 65535. Therefore, a 0 in
+	 * any dimension means that we have no workgroups and the compute
+	 * shader should not be dispatched.
+	 */
+	if (!V3D_GET_FIELD(job->args.cfg[0], V3D_CSD_QUEUED_CFG0_NUM_WGS_X) ||
+	    !V3D_GET_FIELD(job->args.cfg[1], V3D_CSD_QUEUED_CFG1_NUM_WGS_Y) ||
+	    !V3D_GET_FIELD(job->args.cfg[2], V3D_CSD_QUEUED_CFG2_NUM_WGS_Z))
+		return NULL;
+
 	v3d->queue[V3D_CSD].active_job = &job->base;
 
 	v3d_invalidate_caches(v3d);
@@ -402,13 +412,13 @@ v3d_rewrite_csd_job_wg_counts_from_indirect(struct v3d_cpu_job *job)
 
 	wg_counts = (uint32_t *)(bo->vaddr + indirect_csd->offset);
 
-	if (wg_counts[0] == 0 || wg_counts[1] == 0 || wg_counts[2] == 0)
-		goto unmap_bo;
-
 	args->cfg[0] = wg_counts[0] << V3D_CSD_CFG012_WG_COUNT_SHIFT;
 	args->cfg[1] = wg_counts[1] << V3D_CSD_CFG012_WG_COUNT_SHIFT;
 	args->cfg[2] = wg_counts[2] << V3D_CSD_CFG012_WG_COUNT_SHIFT;
 
+	if (wg_counts[0] == 0 || wg_counts[1] == 0 || wg_counts[2] == 0)
+		goto unmap_bo;
+
 	num_batches = DIV_ROUND_UP(indirect_csd->wg_size, 16) *
 		      (wg_counts[0] * wg_counts[1] * wg_counts[2]);
 

-- 
2.54.0

Re: [PATCH v4 2/2] drm/v3d: Skip CSD when it has zeroed workgroups

[Iago Toral]

Reviewed-by: Iago Toral Quiroga <itoral@igalia.com>

El mar, 02-06-2026 a las 14:50 -0300, Maíra Canal escribió:
> A compute shader dispatch encodes its workgroup counts in the
> CFG0..CFG2
> registers. Kicking off a dispatch with a zero count in any of the
> three
> dimensions is invalid. First, the hardware will process 0 as 65536,
> while the user-space driver exposes a maximum of 65535. Over that, a
> submission with a zeroed workgroup dimension should be a no-op.
> 
> These zeroed counts can reach the dispatch path through an indirect
> CSD
> job, whose workgroup counts are only known once the indirect buffer
> is
> read and may legitimately be zero, but such scenario should only
> result in
> a no-op.
> 
> Overwrite the indirect CSD job workgroup counts with the indirect BO
> ones, even if they are zeroed, and don't submit the job to the
> hardware
> when any of the workgroup counts is zero, so the job completes
> immediately
> instead of running the shader.
> 
> Cc: stable@vger.kernel.org
> Fixes: d223f98f0209 ("drm/v3d: Add support for compute shader
> dispatch.")
> Suggested-by: Jose Maria Casanova Crespo <jmcasanova@igalia.com>
> Signed-off-by: Maíra Canal <mcanal@igalia.com>
> ---
>  drivers/gpu/drm/v3d/v3d_sched.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/v3d/v3d_sched.c
> b/drivers/gpu/drm/v3d/v3d_sched.c
> index 47f83936cd73..8a635a9ec046 100644
> --- a/drivers/gpu/drm/v3d/v3d_sched.c
> +++ b/drivers/gpu/drm/v3d/v3d_sched.c
> @@ -352,6 +352,16 @@ v3d_csd_job_run(struct drm_sched_job *sched_job)
>  		return NULL;
>  	}
>  
> +	/* The HW interprets a workgroup size of 0 as 65536;
> however, the
> +	 * user-space driver exposes a maximum of 65535. Therefore,
> a 0 in
> +	 * any dimension means that we have no workgroups and the
> compute
> +	 * shader should not be dispatched.
> +	 */
> +	if (!V3D_GET_FIELD(job->args.cfg[0],
> V3D_CSD_QUEUED_CFG0_NUM_WGS_X) ||
> +	    !V3D_GET_FIELD(job->args.cfg[1],
> V3D_CSD_QUEUED_CFG1_NUM_WGS_Y) ||
> +	    !V3D_GET_FIELD(job->args.cfg[2],
> V3D_CSD_QUEUED_CFG2_NUM_WGS_Z))
> +		return NULL;
> +
>  	v3d->queue[V3D_CSD].active_job = &job->base;
>  
>  	v3d_invalidate_caches(v3d);
> @@ -402,13 +412,13 @@
> v3d_rewrite_csd_job_wg_counts_from_indirect(struct v3d_cpu_job *job)
>  
>  	wg_counts = (uint32_t *)(bo->vaddr + indirect_csd->offset);
>  
> -	if (wg_counts[0] == 0 || wg_counts[1] == 0 || wg_counts[2]
> == 0)
> -		goto unmap_bo;
> -
>  	args->cfg[0] = wg_counts[0] <<
> V3D_CSD_CFG012_WG_COUNT_SHIFT;
>  	args->cfg[1] = wg_counts[1] <<
> V3D_CSD_CFG012_WG_COUNT_SHIFT;
>  	args->cfg[2] = wg_counts[2] <<
> V3D_CSD_CFG012_WG_COUNT_SHIFT;
>  
> +	if (wg_counts[0] == 0 || wg_counts[1] == 0 || wg_counts[2]
> == 0)
> +		goto unmap_bo;
> +
>  	num_batches = DIV_ROUND_UP(indirect_csd->wg_size, 16) *
>  		      (wg_counts[0] * wg_counts[1] * wg_counts[2]);
>  
> 

Re: [PATCH v4 0/2] drm/v3d: Fix indirect CSD jobs with zeroed workgroups

[Maíra Canal]

On Tue, 02 Jun 2026 14:50:13 -0300, Maíra Canal wrote:
> Indirect CSD lets userspace defer the workgroup counts to a GPU buffer
> that is only filled at runtime, so the counts are unknown at submission
> time and can legitimately turn out to be zero.
> 
> However, exercing this case exposed two issues in the CSD path.
> 
>   1. Virtual address leaks when the indirect CSD has zeroed workgroups.
> 
> [...]

Applied, thanks!

[1/2] drm/v3d: Fix vaddr leak when indirect CSD has zeroed workgroups
      commit: ae7676952790f421c40918e2586a2c9f12a682b6
[2/2] drm/v3d: Skip CSD when it has zeroed workgroups
      commit: 7f93fad5ea0affc9e1505dd0f7596c0fdb496213

Best regards,
- Maíra

Claude review: drm/v3d: Fix indirect CSD jobs with zeroed workgroups

[Claude Code Review Bot]

Overall Series Review

Subject: drm/v3d: Fix indirect CSD jobs with zeroed workgroups
Author: =?utf-8?q?Ma=C3=ADra_Canal?= <mcanal@igalia.com>
Patches: 5
Reviewed: 2026-06-04T12:23:39.573582

---

This is a well-structured 2-patch bugfix series addressing two related issues with indirect CSD (Compute Shader Dispatch) on V3D when the GPU buffer contains zeroed workgroup counts. The series is authored by Maíra Canal at Igalia and is at v4, with Iago Toral's Reviewed-by on patch 1. Both patches target stable.

The problems are:
1. A vaddr leak from early-return without unmapping BOs.
2. Hardware misinterpretation of zero workgroup counts as 65536.

The fixes are clean, minimal, and correctly ordered — the leak fix comes first so it can be backported independently. The v4 revision improves on v3 by always writing CFG[0..2] from the indirect buffer before checking for zeros, which avoids leaving stale userspace values in the config registers. The series is tested against relevant CTS tests.

**Verdict: Series looks good.** One minor observation on patch 2 below (missing `active_job = NULL` for consistency), but it's not a correctness bug.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/v3d: Fix vaddr leak when indirect CSD has zeroed workgroups

[Claude Code Review Bot]

Patch Review

**Status: Good — correct and minimal fix.**

The bug: `v3d_rewrite_csd_job_wg_counts_from_indirect()` maps two BOs via `v3d_get_bo_vaddr()` at the top, but when any workgroup count is zero, it returned early via `return;`, skipping the `v3d_put_bo_vaddr()` cleanup at the bottom. This leaks the vaddr mappings.

The fix replaces the bare `return` with `goto unmap_bo`, jumping to a label placed just before the existing cleanup:

```c
	if (wg_counts[0] == 0 || wg_counts[1] == 0 || wg_counts[2] == 0)
-		return;
+		goto unmap_bo;
```

```c
+unmap_bo:
 	v3d_put_bo_vaddr(indirect);
 	v3d_put_bo_vaddr(bo);
```

This is the obvious correct fix. The `goto` is the standard kernel pattern for cleanup-on-early-exit. The Fixes tag correctly references the commit that introduced the indirect CSD path. Has Iago's Reviewed-by.

No issues.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/v3d: Skip CSD when it has zeroed workgroups

[Claude Code Review Bot]

Patch Review

**Status: Good, with one minor style observation.**

This patch addresses two things:

**Part A — Always rewrite CFG[0..2] from the indirect buffer.** In the rewrite function, the cfg writes are now done unconditionally *before* the zero check:

```c
 	args->cfg[0] = wg_counts[0] << V3D_CSD_CFG012_WG_COUNT_SHIFT;
 	args->cfg[1] = wg_counts[1] << V3D_CSD_CFG012_WG_COUNT_SHIFT;
 	args->cfg[2] = wg_counts[2] << V3D_CSD_CFG012_WG_COUNT_SHIFT;
 
+	if (wg_counts[0] == 0 || wg_counts[1] == 0 || wg_counts[2] == 0)
+		goto unmap_bo;
```

This is the v3→v4 improvement. It ensures CFG[0..2] always reflect the actual indirect buffer contents rather than preserving stale userspace values. The `goto unmap_bo` still correctly skips the `num_batches` calculation, which would produce `0 - 1 = 0xFFFFFFFF` (triggering the existing `WARN_ON(args->cfg[4] == ~0)`) if any count is zero.

**Part B — Skip hardware dispatch for zero workgroups.** In `v3d_csd_job_run()`:

```c
+	if (!V3D_GET_FIELD(job->args.cfg[0], V3D_CSD_QUEUED_CFG0_NUM_WGS_X) ||
+	    !V3D_GET_FIELD(job->args.cfg[1], V3D_CSD_QUEUED_CFG1_NUM_WGS_Y) ||
+	    !V3D_GET_FIELD(job->args.cfg[2], V3D_CSD_QUEUED_CFG2_NUM_WGS_Z))
+		return NULL;
```

Using `V3D_GET_FIELD` with the proper register field macros is the right approach — it extracts bits [31:16] from each CFG register, which is exactly where the workgroup counts live (confirmed: `V3D_CSD_QUEUED_CFG0_NUM_WGS_X_SHIFT = 16` in `v3d_regs.h` matches `V3D_CSD_CFG012_WG_COUNT_SHIFT = 16`). This works for both indirect and direct CSD submissions. The comment explaining the hardware's 0→65536 interpretation is useful.

**Minor observation:** The existing error path in this function does `v3d->queue[V3D_CSD].active_job = NULL` before returning NULL:

```c
	if (unlikely(job->base.base.s_fence->finished.error)) {
		v3d->queue[V3D_CSD].active_job = NULL;
		return NULL;
	}
```

The new zero-workgroup path returns NULL without this. Since `active_job` is set only on the *next* line (`v3d->queue[V3D_CSD].active_job = &job->base`), the value at this point should already be NULL from the previous job's IRQ handler, so this is not a correctness issue. However, for consistency with the existing error path and defensive coding, adding `v3d->queue[V3D_CSD].active_job = NULL;` before the return would be slightly cleaner. Not a blocker.

No other issues. The Fixes tag (`d223f98f0209`) correctly references the original CSD dispatch support, as the zero-workgroup check belongs there at the hardware-facing level.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/v3d: Fix vaddr leak when indirect CSD has zeroed workgroups

[Claude Code Review Bot]

Patch Review

**Status: Good**

This patch fixes a resource leak in `v3d_rewrite_csd_job_wg_counts_from_indirect()`. Looking at the existing code in the kernel tree (`v3d_sched.c:400-406`):

```c
v3d_get_bo_vaddr(bo);
v3d_get_bo_vaddr(indirect);

wg_counts = (uint32_t *)(bo->vaddr + indirect_csd->offset);

if (wg_counts[0] == 0 || wg_counts[1] == 0 || wg_counts[2] == 0)
    return;
```

The bare `return` at line 406 skips the cleanup at lines 431-432 (`v3d_put_bo_vaddr(indirect)` / `v3d_put_bo_vaddr(bo)`), leaking both mappings. The fix correctly replaces `return` with `goto unmap_bo`, which jumps to the cleanup path:

```c
if (wg_counts[0] == 0 || wg_counts[1] == 0 || wg_counts[2] == 0)
    goto unmap_bo;
```

```c
unmap_bo:
	v3d_put_bo_vaddr(indirect);
	v3d_put_bo_vaddr(bo);
```

The label placement is correct — it sits right before both `v3d_put_bo_vaddr` calls, matching the acquisition order (bo, indirect acquired; indirect, bo released). The label name `unmap_bo` is slightly ambiguous since it unmaps both BOs, but this is a minor style nit that doesn't affect correctness.

One observation: when the zero-workgroup path is taken, `args->cfg[0..4]` are left untouched — they retain whatever userspace submitted. This is fine because patch 2 ensures the CSD job is never submitted to hardware when counts are zero.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/v3d: Fix vaddr leak when indirect CSD has zeroed workgroups

[Claude Code Review Bot]

Patch Review

**Status: Good**

This patch fixes a straightforward resource leak. In `v3d_rewrite_csd_job_wg_counts_from_indirect()`, two BOs are mapped at the top of the function:

```c
v3d_get_bo_vaddr(bo);
v3d_get_bo_vaddr(indirect);
```

When any workgroup count is zero, the early `return` at line 406 skips the cleanup:

```c
if (wg_counts[0] == 0 || wg_counts[1] == 0 || wg_counts[2] == 0)
    return;  // leaked both vaddr mappings
```

The fix correctly changes `return` to `goto unmap_bo`, jumping to the existing cleanup that calls `v3d_put_bo_vaddr()` on both BOs. The label placement is correct — it's right before both `v3d_put_bo_vaddr` calls at the end of the function.

One minor observation: the `goto` label name `unmap_bo` references only one BO, but both `indirect` and `bo` are unmapped there. A name like `unmap` would be slightly more precise, but this is a very minor style nit and not worth a respin given it follows common kernel convention where the label names the first cleanup action.

No issues with this patch.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/v3d: Fix vaddr leak when indirect CSD has zeroed workgroups

[Claude Code Review Bot]

Patch Review

**Assessment: Good. No issues.**

This is a straightforward and correct fix. The function `v3d_rewrite_csd_job_wg_counts_from_indirect()` maps two BOs via `v3d_get_bo_vaddr()` (which calls `vmap()` internally), then checks if any workgroup count is zero:

```c
if (wg_counts[0] == 0 || wg_counts[1] == 0 || wg_counts[2] == 0)
    return;
```

The bare `return` skips the cleanup at the end of the function:

```c
v3d_put_bo_vaddr(indirect);
v3d_put_bo_vaddr(bo);
```

The fix correctly changes `return` to `goto unmap_bo` and adds the label before the cleanup calls. The unmap order (indirect first, then bo) matches the reverse of the mapping order, which is good practice. The label name `unmap_bo` is a reasonable label — arguably `unmap` would be more precise since it unmaps both `bo` and `indirect`, but this follows common kernel style where the label names the first cleanup action.

---

---
Generated by Claude Code Patch Reviewer