* [PATCH v6 0/3] drm/imagination: Propagate all KCCB command submission code errors
@ 2026-09-24 6:19 Alexandru Dadu
2026-09-24 6:19 ` [PATCH v6 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 6:19 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 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 | 52 +++++++++++++++++++++++----------
5 files changed, 82 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 v6 1/3] drm/imagination: Release KCCB slot on error path in pvr_queue_run_job()
2026-09-24 6:19 [PATCH v6 0/3] drm/imagination: Propagate all KCCB command submission code errors Alexandru Dadu
@ 2026-09-24 6:19 ` Alexandru Dadu
2026-09-24 6:19 ` [PATCH v6 2/3] drm/imagination: Propagate all errors from KCCB command submission code Alexandru Dadu
2026-09-24 6:19 ` [PATCH v6 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 6:19 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 v6 2/3] drm/imagination: Propagate all errors from KCCB command submission code
2026-09-24 6:19 [PATCH v6 0/3] drm/imagination: Propagate all KCCB command submission code errors Alexandru Dadu
2026-09-24 6:19 ` [PATCH v6 1/3] drm/imagination: Release KCCB slot on error path in pvr_queue_run_job() Alexandru Dadu
@ 2026-09-24 6:19 ` Alexandru Dadu
2026-09-24 6:31 ` sashiko-bot
2026-09-24 6:19 ` [PATCH v6 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 6:19 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 v6 3/3] drm/imagination: Release pm references in case of error
2026-09-24 6:19 [PATCH v6 0/3] drm/imagination: Propagate all KCCB command submission code errors Alexandru Dadu
2026-09-24 6:19 ` [PATCH v6 1/3] drm/imagination: Release KCCB slot on error path in pvr_queue_run_job() Alexandru Dadu
2026-09-24 6:19 ` [PATCH v6 2/3] drm/imagination: Propagate all errors from KCCB command submission code Alexandru Dadu
@ 2026-09-24 6:19 ` Alexandru Dadu
2026-09-24 6:29 ` sashiko-bot
2 siblings, 1 reply; 6+ messages in thread
From: Alexandru Dadu @ 2026-09-24 6:19 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 | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/imagination/pvr_queue.c b/drivers/gpu/drm/imagination/pvr_queue.c
index b58c0887cafe..1b013791897d 100644
--- a/drivers/gpu/drm/imagination/pvr_queue.c
+++ b/drivers/gpu/drm/imagination/pvr_queue.c
@@ -817,9 +817,19 @@ static struct dma_fence *pvr_queue_run_job(struct drm_sched_job *sched_job)
trace_pvr_job_submit_fw(job);
}
+ 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 v6 3/3] drm/imagination: Release pm references in case of error
2026-09-24 6:19 ` [PATCH v6 3/3] drm/imagination: Release pm references in case of error Alexandru Dadu
@ 2026-09-24 6:29 ` sashiko-bot
0 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-09-24 6:29 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/pvr: misplaced pm_ref get clobbers submission error
- [Medium] drm/pvr: misapplied PM reference error handling is dead code
--
commit 5439537ea7866cb6e72eceafcbe08fbac4918034
Author: Alexandru Dadu <alexandru.dadu@imgtec.com>
drm/imagination: Release pm references in case of error
This commit attempts to add early release of power references in case of
errors during job submission.
> diff --git a/drivers/gpu/drm/imagination/pvr_queue.c b/drivers/gpu/drm/imagination/pvr_queue.c
> index b58c0887cafe5..1b013791897dd 100644
> --- a/drivers/gpu/drm/imagination/pvr_queue.c
> +++ b/drivers/gpu/drm/imagination/pvr_queue.c
[ ... ]
> @@ -817,9 +817,19 @@ static struct dma_fence *pvr_queue_run_job(struct drm_sched_job *sched_job)
> trace_pvr_job_submit_fw(job);
> }
>
> + err = pvr_job_get_pm_ref(job);
> +
[Severity: High]
Does this assignment overwrite the err variable containing the result of the
KCCB kick?
If pvr_cccb_send_kccb_kick() or pvr_cccb_send_kccb_combined_kick() returned
an error, pvr_job_get_pm_ref() will return 0 since the reference was already
acquired at the start of pvr_queue_run_job(). This causes the hardware
submission error to be lost, and the scheduler will receive a success
indication instead of an error.
> 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]
Is this error handling logic unreachable?
Because the PM reference for job->paired_job was already acquired at the
beginning of the function, pvr_job_get_pm_ref() will return 0, making the
WARN_ON(err) condition unreachable.
If this abort path were somehow reached after the job was successfully
submitted to the hardware, jumping to err_release would inappropriately call
pvr_kccb_release_slot(). Could this corrupt KCCB slot accounting since the
hardware kick already occurred?
> + }
> + }
> +
> return dma_fence_get(job->done_fence);
>
> err_release:
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260924-b4-upstream-propagate-all-errors-from-kccb-cmd-submission-code-v6-0-d9d5375665ed@imgtec.com?part=3
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v6 2/3] drm/imagination: Propagate all errors from KCCB command submission code
2026-09-24 6:19 ` [PATCH v6 2/3] drm/imagination: Propagate all errors from KCCB command submission code Alexandru Dadu
@ 2026-09-24 6:31 ` sashiko-bot
0 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-09-24 6:31 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: use-after-free and state corruption due to missing CCCB rollback on KCCB kick failure
- [Medium] drm/imagination: resource leak of KCCB slot reservations on error
--
commit a2bdc84028e3fc1451749b3764c3318c1a8c6d75
Author: Alessio Belle <alessio.belle@imgtec.com>
drm/imagination: Propagate all errors from KCCB command submission code
This commit updates the KCCB command submission code to propagate errors
back to the caller. The DRM scheduler's run_job callback is updated to
return these errors instead of unconditionally succeeding.
> 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]
If we return early here, does it leak the KCCB slot reservation?
When this is called from pvr_kccb_send_cmd_powered(), a slot has already
been reserved by incrementing pvr_dev->kccb.reserved_count. The success
path decrements this count later in the function, but this early return
does not, which could lead to slot starvation and eventual -EBUSY errors
on all future KCCB submissions.
[ ... ]
> 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
[ ... ]
> @@ -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]
If pvr_cccb_send_kccb_kick() fails here and pvr_queue_run_job() returns
ERR_PTR(err), could this lead to a use-after-free and state corruption?
Earlier in pvr_queue_run_job(), pvr_queue_submit_job_to_cccb() is called
which increments queue->in_flight_job_count and writes the job's commands
into the firmware's CCCB ring buffer. If the kick fails and the DRM
scheduler frees this job, the commands remain in the CCCB without being
rolled back.
When a subsequent job is successfully submitted, won't the firmware
process the leftover commands from the freed job, accessing freed resources?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260924-b4-upstream-propagate-all-errors-from-kccb-cmd-submission-code-v6-0-d9d5375665ed@imgtec.com?part=2
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-09-24 6:31 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-24 6:19 [PATCH v6 0/3] drm/imagination: Propagate all KCCB command submission code errors Alexandru Dadu
2026-09-24 6:19 ` [PATCH v6 1/3] drm/imagination: Release KCCB slot on error path in pvr_queue_run_job() Alexandru Dadu
2026-09-24 6:19 ` [PATCH v6 2/3] drm/imagination: Propagate all errors from KCCB command submission code Alexandru Dadu
2026-09-24 6:31 ` sashiko-bot
2026-09-24 6:19 ` [PATCH v6 3/3] drm/imagination: Release pm references in case of error Alexandru Dadu
2026-09-24 6:29 ` sashiko-bot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.