Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [Critical] Schedulers for V3D_CACHE_CLEAN and V3D_CPU are configured with a 500ms timeout but do not provide a `.timedout_job` callback, leading to a NULL pointer dereference crash if a job times out. - [High] GPU reset upon timeout causes a NULL pointer dereference on devices without CSD support (e.g., V3D 3.x), due to accessing uninitialized DRM schedulers. -- --- Patch [1]: [PATCH 1/2] drm/v3d: Serialize the scheduler timeout handlers --- commit 4242eaaa6f3d95b4fe7ea352d6d1d5bebceb401d Author: Maíra Canal <[email protected]> drm/v3d: Serialize the scheduler timeout handlers This patch introduces an ordered workqueue to serialize the scheduler timeout handlers for all V3D hardware queues. By sharing a single workqueue, it prevents concurrent execution of timeout handlers and avoids a race condition that corrupts scheduler credits during a global GPU reset. Cc: [email protected] # 6.15 > diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h > index 423bcfe331a01..8779a42c65081 100644 > --- a/drivers/gpu/drm/v3d/v3d_drv.h > +++ b/drivers/gpu/drm/v3d/v3d_drv.h [ ... ] > diff --git a/drivers/gpu/drm/v3d/v3d_sched.c b/drivers/gpu/drm/v3d/v3d_sched.c > index c35f9f70cf8d6..63dd95b828c85 100644 > --- a/drivers/gpu/drm/v3d/v3d_sched.c > +++ b/drivers/gpu/drm/v3d/v3d_sched.c > @@ -837,6 +837,7 @@ v3d_queue_sched_init(struct v3d_dev *v3d, const struct > drm_sched_backend_ops *op > struct drm_sched_init_args args = { > .credit_limit = 1, > .timeout = msecs_to_jiffies(500), > + .timeout_wq = v3d->reset_wq, > .dev = v3d->drm.dev, > }; [Severity: Critical] This is a pre-existing issue, but since we are modifying the timeout configuration for all queues, what happens if a V3D_CPU or V3D_CACHE_CLEAN job times out? Both of these queues are configured with a 500ms timeout by the arguments above, but their backend operations lack a .timedout_job callback: static const struct drm_sched_backend_ops v3d_cpu_sched_ops = { .run_job = v3d_cpu_job_run, .free_job = v3d_sched_job_free }; If a user-space program submits a CPU job (e.g. V3D_CPU_JOB_TYPE_COPY_TIMESTAMP_QUERY) with a very large count parameter that takes longer than 500ms, or if the OS preempts the thread under heavy load, drm_sched_job_timedout() will execute. Since job->sched->ops->timedout_job is NULL for these schedulers, won't this result in a NULL pointer dereference? [ ... ] > @@ -896,4 +901,6 @@ v3d_sched_fini(struct v3d_dev *v3d) > if (v3d->queue[q].sched.ready) > drm_sched_fini(&v3d->queue[q].sched); > } > + > + destroy_workqueue(v3d->reset_wq); > } [Severity: High] This is a pre-existing issue, but noticing the queue iteration here in v3d_sched_fini() where we safely check v3d->queue[q].sched.ready, does v3d_gpu_reset_for_timeout() have a similar safeguard? Looking at v3d_gpu_reset_for_timeout(), it iterates over all queues indiscriminately: for (i = 0; i < V3D_MAX_QUEUES; i++) drm_sched_stop(&v3d->queue[i].sched, sched_job); On devices without CSD support (like V3D 3.x), v3d_has_csd() returns false, meaning the schedulers for V3D_CSD and V3D_CACHE_CLEAN are skipped during initialization and remain zero-initialized. When a timeout occurs on these older variants and v3d_gpu_reset_for_timeout() is triggered, wouldn't calling drm_sched_stop() on these zero-initialized schedulers cause a NULL pointer dereference when it attempts to iterate over sched->pending_list? -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
