* [PATCH v7 0/3] drm/imagination: Propagate all KCCB command submission code errors
@ 2026-09-24 7:29 Alexandru Dadu
2026-09-24 7:29 ` [PATCH v7 1/3] drm/imagination: Release KCCB slot on error path in pvr_queue_run_job() Alexandru Dadu
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Alexandru Dadu @ 2026-09-24 7:29 UTC (permalink / raw)
To: Alessio Belle, Luigi Santivetti, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter
Cc: imagination, dri-devel, linux-kernel, Alexandru Dadu
- Set up an error path for pvr_queue_run_job().
- Propagate errors from KCCB submission functions.
- Early release pm references in case of errors.
Signed-off-by: Alexandru Dadu <alexandru.dadu@imgtec.com>
---
Changes in v7:
- Fix for the early release pm references in case of errors commit.
Error variable was being overwritten before having a chance to be
checked.
- Link to v6: https://patch.msgid.link/20260924-b4-upstream-propagate-all-errors-from-kccb-cmd-submission-code-v6-0-d9d5375665ed@imgtec.com
Changes in v6:
- Refactor the patch into 3 commits.
- Rebased on a more recent drm-misc-next.
- Link to v5: https://patch.msgid.link/20260908-b4-upstream-propagate-all-errors-from-kccb-cmd-submission-code-v5-1-5e262721454b@imgtec.com
Changes in v5:
- Added pvr_job_release_pm_ref() to the kccb kick command return value
check.
- Update v4 description typo.
- Link to v4: https://patch.msgid.link/20260907-b4-upstream-propagate-all-errors-from-kccb-cmd-submission-code-v4-1-b684f58a06ce@imgtec.com
Changes in v4:
- Added a check over the return value of the kccb kick command.
- Link to v3: https://patch.msgid.link/20260907-b4-upstream-propagate-all-errors-from-kccb-cmd-submission-code-v3-1-4578a0fadf69@imgtec.com
Changes in v3:
- Dropped the job->kccb_fence check since pvr_queue_prepare_job() makes
sure that the check would always be true.
- Dropped the power references from the patch.
- Link to v2: https://patch.msgid.link/20260814-b4-upstream-propagate-all-errors-from-kccb-cmd-submission-code-v2-1-35355fad50ae@imgtec.com
Changes in v2:
- Provide an error path in pvr_queue_run_job(). The path will use
pvr_kccb_release_slot() to avoid resource leaks.
- Link to v1: https://patch.msgid.link/20260811-b4-upstream-propagate-all-errors-from-kccb-cmd-submission-code-v1-1-ffd55254d6d2@imgtec.com
To: Alessio Belle <alessio.belle@imgtec.com>
To: Luigi Santivetti <luigi.santivetti@imgtec.com>
To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
To: Maxime Ripard <mripard@kernel.org>
To: Thomas Zimmermann <tzimmermann@suse.de>
To: David Airlie <airlied@gmail.com>
To: Simona Vetter <simona@ffwll.ch>
Cc: imagination@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org
Cc: linux-kernel@vger.kernel.org
---
Alessio Belle (1):
drm/imagination: Propagate all errors from KCCB command submission code
Alexandru Dadu (2):
drm/imagination: Release KCCB slot on error path in pvr_queue_run_job()
drm/imagination: Release pm references in case of error
drivers/gpu/drm/imagination/pvr_ccb.c | 33 +++++++++++++++-----
drivers/gpu/drm/imagination/pvr_ccb.h | 6 ++--
drivers/gpu/drm/imagination/pvr_cccb.c | 12 ++++---
drivers/gpu/drm/imagination/pvr_cccb.h | 20 ++++++------
drivers/gpu/drm/imagination/pvr_queue.c | 55 +++++++++++++++++++++++----------
5 files changed, 85 insertions(+), 41 deletions(-)
---
base-commit: a8fe4e162ba8212b7e51fdb024b37800f36b496c
change-id: 20260810-b4-upstream-propagate-all-errors-from-kccb-cmd-submission-code-0dae4f05fcb5
Best regards,
--
Alexandru Dadu <alexandru.dadu@imgtec.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v7 1/3] drm/imagination: Release KCCB slot on error path in pvr_queue_run_job()
2026-09-24 7:29 [PATCH v7 0/3] drm/imagination: Propagate all KCCB command submission code errors Alexandru Dadu
@ 2026-09-24 7:29 ` Alexandru Dadu
2026-09-24 7:29 ` [PATCH v7 2/3] drm/imagination: Propagate all errors from KCCB command submission code Alexandru Dadu
2026-09-24 7:29 ` [PATCH v7 3/3] drm/imagination: Release pm references in case of error Alexandru Dadu
2 siblings, 0 replies; 6+ messages in thread
From: Alexandru Dadu @ 2026-09-24 7:29 UTC (permalink / raw)
To: Alessio Belle, Luigi Santivetti, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter
Cc: imagination, dri-devel, linux-kernel, Alexandru Dadu
Add an error path to pvr_queue_run_job() instead of just a warning and
release the KCCB in that path.
Signed-off-by: Alexandru Dadu <alexandru.dadu@imgtec.com>
---
drivers/gpu/drm/imagination/pvr_queue.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/imagination/pvr_queue.c b/drivers/gpu/drm/imagination/pvr_queue.c
index 24a8645aa913..8dd87192fabc 100644
--- a/drivers/gpu/drm/imagination/pvr_queue.c
+++ b/drivers/gpu/drm/imagination/pvr_queue.c
@@ -767,17 +767,19 @@ static struct dma_fence *pvr_queue_run_job(struct drm_sched_job *sched_job)
(job->type != DRM_PVR_JOB_TYPE_GEOMETRY ||
job->paired_job->type != DRM_PVR_JOB_TYPE_FRAGMENT ||
job->hwrt != job->paired_job->hwrt ||
- job->ctx != job->paired_job->ctx)))
- return ERR_PTR(-EINVAL);
+ job->ctx != job->paired_job->ctx))) {
+ err = -EINVAL;
+ goto err_release;
+ }
err = pvr_job_get_pm_ref(job);
if (WARN_ON(err))
- return ERR_PTR(err);
+ goto err_release;
if (job->paired_job) {
err = pvr_job_get_pm_ref(job->paired_job);
if (WARN_ON(err))
- return ERR_PTR(err);
+ goto err_release;
}
/* Submit our job to the CCCB */
@@ -814,7 +816,14 @@ static struct dma_fence *pvr_queue_run_job(struct drm_sched_job *sched_job)
trace_pvr_job_submit_fw(job);
}
+ if (WARN_ON(err))
+ goto err_release;
+
return dma_fence_get(job->done_fence);
+
+err_release:
+ pvr_kccb_release_slot(pvr_dev);
+ return ERR_PTR(err);
}
static void pvr_queue_stop(struct pvr_queue *queue, struct pvr_job *bad_job)
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v7 2/3] drm/imagination: Propagate all errors from KCCB command submission code
2026-09-24 7:29 [PATCH v7 0/3] drm/imagination: Propagate all KCCB command submission code errors Alexandru Dadu
2026-09-24 7:29 ` [PATCH v7 1/3] drm/imagination: Release KCCB slot on error path in pvr_queue_run_job() Alexandru Dadu
@ 2026-09-24 7:29 ` Alexandru Dadu
2026-09-24 7:43 ` sashiko-bot
2026-09-24 7:29 ` [PATCH v7 3/3] drm/imagination: Release pm references in case of error Alexandru Dadu
2 siblings, 1 reply; 6+ messages in thread
From: Alexandru Dadu @ 2026-09-24 7:29 UTC (permalink / raw)
To: Alessio Belle, Luigi Santivetti, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter
Cc: imagination, dri-devel, linux-kernel, Alexandru Dadu
From: Alessio Belle <alessio.belle@imgtec.com>
pvr_kccb_send_cmd_reserved_powered() returned void while the other two
variants of pvr_kccb_send_cmd*() returned int.
The error is now propagated all the way to the DRM scheduler's run_job()
callback, which is the only user of pvr_kccb_send_cmd_reserved_powered()
outside of the other variants of pvr_kccb_send_cmd*().
Signed-off-by: Alessio Belle <alessio.belle@imgtec.com>
Signed-off-by: Alexandru Dadu <alexandru.dadu@imgtec.com>
---
drivers/gpu/drm/imagination/pvr_ccb.c | 33 +++++++++++++++++++++++++--------
drivers/gpu/drm/imagination/pvr_ccb.h | 6 +++---
drivers/gpu/drm/imagination/pvr_cccb.c | 12 ++++++++----
drivers/gpu/drm/imagination/pvr_cccb.h | 20 ++++++++++----------
drivers/gpu/drm/imagination/pvr_queue.c | 25 +++++++++++++------------
5 files changed, 59 insertions(+), 37 deletions(-)
diff --git a/drivers/gpu/drm/imagination/pvr_ccb.c b/drivers/gpu/drm/imagination/pvr_ccb.c
index e408df74853d..2b3dfbe725dd 100644
--- a/drivers/gpu/drm/imagination/pvr_ccb.c
+++ b/drivers/gpu/drm/imagination/pvr_ccb.c
@@ -253,8 +253,13 @@ pvr_kccb_used_slot_count_locked(struct pvr_device *pvr_dev)
* @pvr_dev: Device pointer.
* @cmd: Command to send.
* @kccb_slot: Address to store the KCCB slot for this command. May be %NULL.
+ *
+ * Returns:
+ * * Zero on success,
+ * * -EIO if the device is lost, or
+ * * -EINVAL if a KCCB slot was not reserved or is not available.
*/
-void
+int
pvr_kccb_send_cmd_reserved_powered(struct pvr_device *pvr_dev,
struct rogue_fwif_kccb_cmd *cmd,
u32 *kccb_slot)
@@ -264,19 +269,25 @@ pvr_kccb_send_cmd_reserved_powered(struct pvr_device *pvr_dev,
struct rogue_fwif_ccb_ctl *ctrl = pvr_ccb->ctrl;
u32 old_write_offset;
u32 new_write_offset;
+ int err;
- WARN_ON(pvr_dev->lost);
+ if (pvr_dev->lost)
+ return -EIO;
mutex_lock(&pvr_ccb->lock);
- if (WARN_ON(!pvr_dev->kccb.reserved_count))
+ if (WARN_ON(!pvr_dev->kccb.reserved_count)) {
+ err = -EINVAL;
goto out_unlock;
+ }
old_write_offset = READ_ONCE(ctrl->write_offset);
/* We reserved the slot, we should have one available. */
- if (WARN_ON(!pvr_ccb_slot_available_locked(pvr_ccb, &new_write_offset)))
+ if (WARN_ON(!pvr_ccb_slot_available_locked(pvr_ccb, &new_write_offset))) {
+ err = -EINVAL;
goto out_unlock;
+ }
memcpy(&kccb[old_write_offset], cmd,
sizeof(struct rogue_fwif_kccb_cmd));
@@ -294,8 +305,14 @@ pvr_kccb_send_cmd_reserved_powered(struct pvr_device *pvr_dev,
pvr_fw_mts_schedule(pvr_dev,
PVR_FWIF_DM_GP & ~ROGUE_CR_MTS_SCHEDULE_DM_CLRMSK);
+ mutex_unlock(&pvr_ccb->lock);
+
+ return 0;
+
out_unlock:
mutex_unlock(&pvr_ccb->lock);
+
+ return err;
}
/**
@@ -361,8 +378,9 @@ static int pvr_kccb_reserve_slot_sync(struct pvr_device *pvr_dev)
* @kccb_slot: Address to store the KCCB slot for this command. May be %NULL.
*
* Returns:
- * * Zero on success, or
- * * -EBUSY if timeout while waiting for a free KCCB slot.
+ * * Zero on success,
+ * * Any error returned by pvr_kccb_reserve_slot_sync(), or
+ * * Any error returned by pvr_kccb_send_cmd_reserved_powered().
*/
int
pvr_kccb_send_cmd_powered(struct pvr_device *pvr_dev, struct rogue_fwif_kccb_cmd *cmd,
@@ -374,8 +392,7 @@ pvr_kccb_send_cmd_powered(struct pvr_device *pvr_dev, struct rogue_fwif_kccb_cmd
if (err)
return err;
- pvr_kccb_send_cmd_reserved_powered(pvr_dev, cmd, kccb_slot);
- return 0;
+ return pvr_kccb_send_cmd_reserved_powered(pvr_dev, cmd, kccb_slot);
}
/**
diff --git a/drivers/gpu/drm/imagination/pvr_ccb.h b/drivers/gpu/drm/imagination/pvr_ccb.h
index 4c8aef31eeb0..8b698206c68b 100644
--- a/drivers/gpu/drm/imagination/pvr_ccb.h
+++ b/drivers/gpu/drm/imagination/pvr_ccb.h
@@ -60,9 +60,9 @@ int pvr_kccb_send_cmd(struct pvr_device *pvr_dev,
int pvr_kccb_send_cmd_powered(struct pvr_device *pvr_dev,
struct rogue_fwif_kccb_cmd *cmd,
u32 *kccb_slot);
-void pvr_kccb_send_cmd_reserved_powered(struct pvr_device *pvr_dev,
- struct rogue_fwif_kccb_cmd *cmd,
- u32 *kccb_slot);
+int pvr_kccb_send_cmd_reserved_powered(struct pvr_device *pvr_dev,
+ struct rogue_fwif_kccb_cmd *cmd,
+ u32 *kccb_slot);
int pvr_kccb_wait_for_completion(struct pvr_device *pvr_dev, u32 slot_nr, u32 timeout,
u32 *rtn_out);
bool pvr_kccb_is_idle(struct pvr_device *pvr_dev);
diff --git a/drivers/gpu/drm/imagination/pvr_cccb.c b/drivers/gpu/drm/imagination/pvr_cccb.c
index 4fabab41bea7..da6e6d94e29f 100644
--- a/drivers/gpu/drm/imagination/pvr_cccb.c
+++ b/drivers/gpu/drm/imagination/pvr_cccb.c
@@ -220,8 +220,12 @@ static void fill_cmd_kick_data(struct pvr_cccb *cccb, u32 ctx_fw_addr,
* You must call pvr_kccb_reserve_slot() and wait for the returned fence to
* signal (if this function didn't return NULL) before calling
* pvr_cccb_send_kccb_kick().
+ *
+ * Returns:
+ * * Zero on success, or
+ * * Any error returned by pvr_kccb_send_cmd_reserved_powered().
*/
-void
+int
pvr_cccb_send_kccb_kick(struct pvr_device *pvr_dev,
struct pvr_cccb *pvr_cccb, u32 cctx_fw_addr,
struct pvr_hwrt_data *hwrt)
@@ -235,10 +239,10 @@ pvr_cccb_send_kccb_kick(struct pvr_device *pvr_dev,
/* Make sure the writes to the CCCB are flushed before sending the KICK. */
wmb();
- pvr_kccb_send_cmd_reserved_powered(pvr_dev, &cmd_kick, NULL);
+ return pvr_kccb_send_cmd_reserved_powered(pvr_dev, &cmd_kick, NULL);
}
-void
+int
pvr_cccb_send_kccb_combined_kick(struct pvr_device *pvr_dev,
struct pvr_cccb *geom_cccb,
struct pvr_cccb *frag_cccb,
@@ -263,5 +267,5 @@ pvr_cccb_send_kccb_combined_kick(struct pvr_device *pvr_dev,
/* Make sure the writes to the CCCB are flushed before sending the KICK. */
wmb();
- pvr_kccb_send_cmd_reserved_powered(pvr_dev, &cmd_kick, NULL);
+ return pvr_kccb_send_cmd_reserved_powered(pvr_dev, &cmd_kick, NULL);
}
diff --git a/drivers/gpu/drm/imagination/pvr_cccb.h b/drivers/gpu/drm/imagination/pvr_cccb.h
index 943fe8f2c963..a2155f732bf1 100644
--- a/drivers/gpu/drm/imagination/pvr_cccb.h
+++ b/drivers/gpu/drm/imagination/pvr_cccb.h
@@ -59,16 +59,16 @@ void pvr_cccb_fini(struct pvr_cccb *cccb);
void pvr_cccb_write_command_with_header(struct pvr_cccb *pvr_cccb,
u32 cmd_type, u32 cmd_size, void *cmd_data,
u32 ext_job_ref, u32 int_job_ref);
-void pvr_cccb_send_kccb_kick(struct pvr_device *pvr_dev,
- struct pvr_cccb *pvr_cccb, u32 cctx_fw_addr,
- struct pvr_hwrt_data *hwrt);
-void pvr_cccb_send_kccb_combined_kick(struct pvr_device *pvr_dev,
- struct pvr_cccb *geom_cccb,
- struct pvr_cccb *frag_cccb,
- u32 geom_ctx_fw_addr,
- u32 frag_ctx_fw_addr,
- struct pvr_hwrt_data *hwrt,
- bool frag_is_pr);
+int pvr_cccb_send_kccb_kick(struct pvr_device *pvr_dev,
+ struct pvr_cccb *pvr_cccb, u32 cctx_fw_addr,
+ struct pvr_hwrt_data *hwrt);
+int pvr_cccb_send_kccb_combined_kick(struct pvr_device *pvr_dev,
+ struct pvr_cccb *geom_cccb,
+ struct pvr_cccb *frag_cccb,
+ u32 geom_ctx_fw_addr,
+ u32 frag_ctx_fw_addr,
+ struct pvr_hwrt_data *hwrt,
+ bool frag_is_pr);
bool pvr_cccb_cmdseq_fits(struct pvr_cccb *pvr_cccb, size_t size);
/**
diff --git a/drivers/gpu/drm/imagination/pvr_queue.c b/drivers/gpu/drm/imagination/pvr_queue.c
index 8dd87192fabc..b58c0887cafe 100644
--- a/drivers/gpu/drm/imagination/pvr_queue.c
+++ b/drivers/gpu/drm/imagination/pvr_queue.c
@@ -793,15 +793,15 @@ static struct dma_fence *pvr_queue_run_job(struct drm_sched_job *sched_job)
/* Submit the fragment job along the geometry job and send a combined kick. */
pvr_queue_submit_job_to_cccb(frag_job);
- pvr_cccb_send_kccb_combined_kick(pvr_dev,
- &geom_queue->cccb, &frag_queue->cccb,
- pvr_context_get_fw_addr(geom_job->ctx) +
- geom_queue->ctx_offset,
- pvr_context_get_fw_addr(frag_job->ctx) +
- frag_queue->ctx_offset,
- job->hwrt,
- frag_job->fw_ccb_cmd_type ==
- ROGUE_FWIF_CCB_CMD_TYPE_FRAG_PR);
+ err = pvr_cccb_send_kccb_combined_kick(pvr_dev,
+ &geom_queue->cccb, &frag_queue->cccb,
+ pvr_context_get_fw_addr(geom_job->ctx) +
+ geom_queue->ctx_offset,
+ pvr_context_get_fw_addr(frag_job->ctx) +
+ frag_queue->ctx_offset,
+ job->hwrt,
+ frag_job->fw_ccb_cmd_type ==
+ ROGUE_FWIF_CCB_CMD_TYPE_FRAG_PR);
trace_pvr_job_submit_fw(geom_job);
trace_pvr_job_submit_fw(frag_job);
@@ -809,9 +809,10 @@ static struct dma_fence *pvr_queue_run_job(struct drm_sched_job *sched_job)
struct pvr_queue *queue = container_of(job->base.sched,
struct pvr_queue, scheduler);
- pvr_cccb_send_kccb_kick(pvr_dev, &queue->cccb,
- pvr_context_get_fw_addr(job->ctx) + queue->ctx_offset,
- job->hwrt);
+ err = pvr_cccb_send_kccb_kick(pvr_dev, &queue->cccb,
+ pvr_context_get_fw_addr(job->ctx) +
+ queue->ctx_offset,
+ job->hwrt);
trace_pvr_job_submit_fw(job);
}
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v7 3/3] drm/imagination: Release pm references in case of error
2026-09-24 7:29 [PATCH v7 0/3] drm/imagination: Propagate all KCCB command submission code errors Alexandru Dadu
2026-09-24 7:29 ` [PATCH v7 1/3] drm/imagination: Release KCCB slot on error path in pvr_queue_run_job() Alexandru Dadu
2026-09-24 7:29 ` [PATCH v7 2/3] drm/imagination: Propagate all errors from KCCB command submission code Alexandru Dadu
@ 2026-09-24 7:29 ` Alexandru Dadu
2026-09-24 7:38 ` sashiko-bot
2 siblings, 1 reply; 6+ messages in thread
From: Alexandru Dadu @ 2026-09-24 7:29 UTC (permalink / raw)
To: Alessio Belle, Luigi Santivetti, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter
Cc: imagination, dri-devel, linux-kernel, Alexandru Dadu
Add early release of the power references in case of errors.
Signed-off-by: Alexandru Dadu <alexandru.dadu@imgtec.com>
---
drivers/gpu/drm/imagination/pvr_queue.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/gpu/drm/imagination/pvr_queue.c b/drivers/gpu/drm/imagination/pvr_queue.c
index b58c0887cafe..c72f1d0b8bc0 100644
--- a/drivers/gpu/drm/imagination/pvr_queue.c
+++ b/drivers/gpu/drm/imagination/pvr_queue.c
@@ -820,6 +820,19 @@ static struct dma_fence *pvr_queue_run_job(struct drm_sched_job *sched_job)
if (WARN_ON(err))
goto err_release;
+ err = pvr_job_get_pm_ref(job);
+
+ if (WARN_ON(err))
+ goto err_release;
+
+ if (job->paired_job) {
+ err = pvr_job_get_pm_ref(job->paired_job);
+ if (WARN_ON(err)) {
+ pvr_job_release_pm_ref(job);
+ goto err_release;
+ }
+ }
+
return dma_fence_get(job->done_fence);
err_release:
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v7 3/3] drm/imagination: Release pm references in case of error
2026-09-24 7:29 ` [PATCH v7 3/3] drm/imagination: Release pm references in case of error Alexandru Dadu
@ 2026-09-24 7:38 ` sashiko-bot
0 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-09-24 7:38 UTC (permalink / raw)
To: Alexandru Dadu; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/imagination: misplaced code adds dead block and fails to fix PM leaks
--
commit ee55e37dd6c9d6ec50a0931a6efbed821fcc945d
Author: Alexandru Dadu <alexandru.dadu@imgtec.com>
drm/imagination: Release pm references in case of error
This commit attempts to fix power management reference leaks by adding early
releases on error.
> diff --git a/drivers/gpu/drm/imagination/pvr_queue.c b/drivers/gpu/drm/imagination/pvr_queue.c
> index b58c0887cafe5..c72f1d0b8bc00 100644
> --- a/drivers/gpu/drm/imagination/pvr_queue.c
> +++ b/drivers/gpu/drm/imagination/pvr_queue.c
> @@ -820,6 +820,19 @@ static struct dma_fence *pvr_queue_run_job(struct drm_sched_job *sched_job)
> if (WARN_ON(err))
> goto err_release;
>
> + err = pvr_job_get_pm_ref(job);
> +
> + if (WARN_ON(err))
> + goto err_release;
> +
> + if (job->paired_job) {
> + err = pvr_job_get_pm_ref(job->paired_job);
> + if (WARN_ON(err)) {
> + pvr_job_release_pm_ref(job);
> + goto err_release;
> + }
> + }
> +
[Severity: Medium]
Does this block act as dead code instead of fixing the error paths?
This code is placed in the success path of pvr_queue_run_job() where the PM
references are already successfully held from earlier in the function. Because
job->has_pm_ref is already true when execution reaches here,
pvr_job_get_pm_ref() simply returns 0 and does nothing.
> return dma_fence_get(job->done_fence);
>
> err_release:
> pvr_kccb_release_slot(pvr_dev);
> return ERR_PTR(err);
> }
[Severity: Medium]
Does the targeted PM leak remain unfixed in this error path?
When an internal job submission fails (for example, if
pvr_cccb_send_kccb_kick() fails) and execution jumps to err_release, the PM
references acquired earlier in the function are not released, which would leak
the references and prevent the GPU from entering runtime suspend.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260924-b4-upstream-propagate-all-errors-from-kccb-cmd-submission-code-v7-0-584796cb599d@imgtec.com?part=3
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v7 2/3] drm/imagination: Propagate all errors from KCCB command submission code
2026-09-24 7:29 ` [PATCH v7 2/3] drm/imagination: Propagate all errors from KCCB command submission code Alexandru Dadu
@ 2026-09-24 7:43 ` sashiko-bot
0 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-09-24 7:43 UTC (permalink / raw)
To: Alexandru Dadu; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] drm/imagination: CCCB state corruption and GPU UAF on job kick failure
- [Medium] drm/imagination: KCCB slot reservation leak on device loss
--
commit ceda87571c98c414f17508dd17e12fa1ee009e5d
Author: Alessio Belle <alessio.belle@imgtec.com>
Date: Thu Sep 24 07:29:44 2026 +0000
drm/imagination: Propagate all errors from KCCB command submission code
This commit modifies pvr_kccb_send_cmd_reserved_powered() to return an int
instead of void, propagating command submission errors up the stack. These
errors now reach the DRM scheduler's run_job() callback.
> diff --git a/drivers/gpu/drm/imagination/pvr_ccb.c b/drivers/gpu/drm/imagination/pvr_ccb.c
> --- a/drivers/gpu/drm/imagination/pvr_ccb.c
> +++ b/drivers/gpu/drm/imagination/pvr_ccb.c
> @@ -264,19 +269,25 @@ pvr_kccb_send_cmd_reserved_powered(struct pvr_device *pvr_dev,
> struct rogue_fwif_ccb_ctl *ctrl = pvr_ccb->ctrl;
> u32 old_write_offset;
> u32 new_write_offset;
> + int err;
>
> - WARN_ON(pvr_dev->lost);
> + if (pvr_dev->lost)
> + return -EIO;
[Severity: Medium]
When the device is lost, does this early return leak the KCCB slot reservation?
Looking at pvr_kccb_send_cmd_powered(), it reserves a slot via
pvr_kccb_reserve_slot_sync() which increments pvr_dev->kccb.reserved_count.
It then calls this function. By returning early here, reserved_count is never
decremented, leaking the slot permanently.
>
> mutex_lock(&pvr_ccb->lock);
>
> - if (WARN_ON(!pvr_dev->kccb.reserved_count))
> + if (WARN_ON(!pvr_dev->kccb.reserved_count)) {
> + err = -EINVAL;
> goto out_unlock;
> + }
[ ... ]
> @@ -374,8 +392,7 @@ pvr_kccb_send_cmd_powered(struct pvr_device *pvr_dev, struct rogue_fwif_kccb_cmd
> if (err)
> return err;
>
> - pvr_kccb_send_cmd_reserved_powered(pvr_dev, cmd, kccb_slot);
> - return 0;
> + return pvr_kccb_send_cmd_reserved_powered(pvr_dev, cmd, kccb_slot);
> }
[ ... ]
> diff --git a/drivers/gpu/drm/imagination/pvr_queue.c b/drivers/gpu/drm/imagination/pvr_queue.c
> --- a/drivers/gpu/drm/imagination/pvr_queue.c
> +++ b/drivers/gpu/drm/imagination/pvr_queue.c
> @@ -793,15 +793,15 @@ static struct dma_fence *pvr_queue_run_job(struct drm_sched_job *sched_job)
>
> /* Submit the fragment job along the geometry job and send a combined kick. */
> pvr_queue_submit_job_to_cccb(frag_job);
> - pvr_cccb_send_kccb_combined_kick(pvr_dev,
> - &geom_queue->cccb, &frag_queue->cccb,
> - pvr_context_get_fw_addr(geom_job->ctx) +
> - geom_queue->ctx_offset,
> - pvr_context_get_fw_addr(frag_job->ctx) +
> - frag_queue->ctx_offset,
> - job->hwrt,
> - frag_job->fw_ccb_cmd_type ==
> - ROGUE_FWIF_CCB_CMD_TYPE_FRAG_PR);
> + err = pvr_cccb_send_kccb_combined_kick(pvr_dev,
> + &geom_queue->cccb, &frag_queue->cccb,
> + pvr_context_get_fw_addr(geom_job->ctx) +
> + geom_queue->ctx_offset,
> + pvr_context_get_fw_addr(frag_job->ctx) +
> + frag_queue->ctx_offset,
> + job->hwrt,
> + frag_job->fw_ccb_cmd_type ==
> + ROGUE_FWIF_CCB_CMD_TYPE_FRAG_PR);
>
> trace_pvr_job_submit_fw(geom_job);
> trace_pvr_job_submit_fw(frag_job);
> @@ -809,9 +809,10 @@ static struct dma_fence *pvr_queue_run_job(struct drm_sched_job *sched_job)
> struct pvr_queue *queue = container_of(job->base.sched,
> struct pvr_queue, scheduler);
>
> - pvr_cccb_send_kccb_kick(pvr_dev, &queue->cccb,
> - pvr_context_get_fw_addr(job->ctx) + queue->ctx_offset,
> - job->hwrt);
> + err = pvr_cccb_send_kccb_kick(pvr_dev, &queue->cccb,
> + pvr_context_get_fw_addr(job->ctx) +
> + queue->ctx_offset,
> + job->hwrt);
>
> trace_pvr_job_submit_fw(job);
> }
[Severity: High]
Does propagating the error here corrupt the CCCB state and potentially cause a
use-after-free?
Looking at the full context of pvr_queue_run_job(), if the kick fails, the
error is caught and handled in the error path:
if (WARN_ON(err))
goto err_release;
...
err_release:
pvr_kccb_release_slot(pvr_dev);
return ERR_PTR(err);
By this point, pvr_queue_submit_job_to_cccb() has already modified the CCCB
state, including incrementing queue->in_flight_job_count and writing the
commands to the CCCB. The err_release path aborts the DRM scheduler job without
reverting these changes.
If the job is aborted, its resources can be freed by userspace, but its
commands remain in the CCCB. When the next job is successfully kicked, the GPU
will execute the aborted job's commands, leading to a use-after-free of the
freed GPU buffers. Additionally, queue->in_flight_job_count is left permanently
desynchronized.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260924-b4-upstream-propagate-all-errors-from-kccb-cmd-submission-code-v7-0-584796cb599d@imgtec.com?part=2
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-09-24 7:43 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-24 7:29 [PATCH v7 0/3] drm/imagination: Propagate all KCCB command submission code errors Alexandru Dadu
2026-09-24 7:29 ` [PATCH v7 1/3] drm/imagination: Release KCCB slot on error path in pvr_queue_run_job() Alexandru Dadu
2026-09-24 7:29 ` [PATCH v7 2/3] drm/imagination: Propagate all errors from KCCB command submission code Alexandru Dadu
2026-09-24 7:43 ` sashiko-bot
2026-09-24 7:29 ` [PATCH v7 3/3] drm/imagination: Release pm references in case of error Alexandru Dadu
2026-09-24 7:38 ` sashiko-bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox