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

[PATCH 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.

Best regards,
- Maíra

---
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 | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
---
base-commit: 5ab62dd3687bcc2cc542b99385aabac5c996db6f
change-id: 20260516-v3d-fix-indirect-csd-6806cf20a57a

[PATCH 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>
---
 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 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,
causing an illegitimate submission. But 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.

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 | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/v3d/v3d_sched.c b/drivers/gpu/drm/v3d/v3d_sched.c
index 47f83936cd73..5476fcf43793 100644
--- a/drivers/gpu/drm/v3d/v3d_sched.c
+++ b/drivers/gpu/drm/v3d/v3d_sched.c
@@ -352,6 +352,9 @@ v3d_csd_job_run(struct drm_sched_job *sched_job)
 		return NULL;
 	}
 
+	if (!job->args.cfg[0] || !job->args.cfg[1] || !job->args.cfg[2])
+		return NULL;
+
 	v3d->queue[V3D_CSD].active_job = &job->base;
 
 	v3d_invalidate_caches(v3d);

-- 
2.54.0

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

[Iago Toral]

El sáb, 30-05-2026 a las 16:51 -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,
> causing an illegitimate submission. But 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.
> 
> 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 | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/v3d/v3d_sched.c
> b/drivers/gpu/drm/v3d/v3d_sched.c
> index 47f83936cd73..5476fcf43793 100644
> --- a/drivers/gpu/drm/v3d/v3d_sched.c
> +++ b/drivers/gpu/drm/v3d/v3d_sched.c
> @@ -352,6 +352,9 @@ v3d_csd_job_run(struct drm_sched_job *sched_job)
>  		return NULL;
>  	}
>  
> +	if (!job->args.cfg[0] || !job->args.cfg[1] || !job-
> >args.cfg[2])
> +		return NULL;

I think this is not correct: cfg[0-2] have the actual dispatch sizes
encoded in the 16 MSB bits of these registers, allowing the lower 16-
bit to specify a base offset for the generated workgroup ids that may
not be zero. Therefore, I think we would want to rewrite this check as:

if ((job->args.cfg[0] & 0xffff0000u) == 0 ||
    (job->args.cfg[1] & 0xffff0000u) == 0 ||
    (job->args.cfg[2] & 0xffff0000u) == 0) {
        return NULL;
}

Also, we probably want to add a comment here explaining that at the hw
level, 0 is interpreted as 65536 but the user-space driver only exposes
65535 as the maximum workgroup size allowed.

Iago

> +
>  	v3d->queue[V3D_CSD].active_job = &job->base;
>  
>  	v3d_invalidate_caches(v3d);
> 

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: 4
Reviewed: 2026-06-04T15:15:36.533079

---

This is a clean, well-motivated two-patch series fixing real bugs in the V3D indirect CSD (Compute Shader Dispatch) path. Indirect CSD defers workgroup counts to a GPU buffer that may legitimately contain zeros at runtime. The series fixes two problems: (1) a vaddr mapping leak when the early-return path skips cleanup, and (2) submitting zero-count dispatches to hardware where the V3D block interprets 0 as 65536, causing invalid execution.

Both patches are small, well-targeted, and have appropriate `Cc: stable` and `Fixes:` tags. The series is correctly ordered — patch 1 fixes the leak in the CPU-side rewrite function, and patch 2 adds the guard in the hardware submission path.

One minor concern worth raising on patch 2, detailed below.

**Verdict: Series looks good. One minor style/consistency question on patch 2.**

---

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

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

[Claude Code Review Bot]

Patch Review

**Assessment: Correct in principle, one minor concern.**

The core idea is sound. The V3D CSD hardware treats a zero workgroup count as 65536, so submitting a zero-count dispatch causes an illegitimate execution. Returning `NULL` from `run_job` is the correct mechanism — the DRM scheduler treats a `NULL` return as "job completed synchronously with success" (calls `drm_sched_job_done(sched_job, 0)`), which is the right semantic for a no-op dispatch. No fences, perf counters, or IRQ infrastructure are set up, which is all appropriate since no hardware work occurs.

The placement of the check is correct — it's after the error-fence check but before `active_job` is set and before any hardware interaction:

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

if (!job->args.cfg[0] || !job->args.cfg[1] || !job->args.cfg[2])
    return NULL;

v3d->queue[V3D_CSD].active_job = &job->base;
```

**Minor concern — cfg[0..2] may contain more than the workgroup count.** Since `V3D_CSD_CFG012_WG_COUNT_SHIFT` is 16, the workgroup count occupies bits [31:16] of each cfg register. The check `!job->args.cfg[0]` tests the entire 32-bit value, not just the count field. This works for the indirect CSD path because `v3d_rewrite_csd_job_wg_counts_from_indirect()` does a full assignment (`args->cfg[0] = wg_counts[0] << 16`), so the lower 16 bits are always zero after rewrite. And in the zero-count case (with patch 1), the rewrite is skipped entirely, so cfg[0..2] retain their userspace-provided values — which must be zero-initialized for this check to trigger.

This is presumably fine because Mesa would initialize cfg[0..2] to zero for indirect CSD (the real values come from the indirect buffer), but it might be worth a brief comment or, alternatively, masking to check only the count field explicitly:

```c
#define V3D_CSD_CFG012_WG_COUNT_MASK  GENMASK(31, V3D_CSD_CFG012_WG_COUNT_SHIFT)

if (!(job->args.cfg[0] & V3D_CSD_CFG012_WG_COUNT_MASK) || ...)
```

This would make the check robust regardless of what userspace puts in the lower bits. That said, given the rewrite function's full assignment behavior, the current check works correctly in practice.

**Nit — `Fixes:` tag.** The tag references `d223f98f0209 ("drm/v3d: Add support for compute shader dispatch.")` (the original CSD commit), while the actual trigger scenario (zero counts from an indirect buffer) was only introduced later with indirect CSD support (`18b8413b25b7`). The Fixes tag is defensible since the hardware's 0-as-65536 behavior was always a latent issue, but it does stretch the stable backport scope. Not wrong, just worth noting.

---
Generated by Claude Code Patch Reviewer