Re: [PATCH 1/2] drm/xe: Read scheduler message opcode under lock

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Unprocessed STATIC_MSG_CLEANUP messages leak the entire xe_exec_queue and drm_device reference when the device is wedged.
--

commit 1cf668d3355bee8d104370398164a7bf010492ee
Author: Matthew Brost <matthew.brost@intel.com>

drm/xe: Read scheduler message opcode under lock

This commit modifies the DRM Xe scheduler to read the message opcode
under the message lock, as the opcode is only guaranteed to remain
stable while the message is on the pending list for static messages.

> diff --git a/drivers/gpu/drm/xe/xe_gpu_scheduler.c b/drivers/gpu/drm/xe/xe_gpu_scheduler.c
> index 9c8004d5dd915..d656db758cb3c 100644
> --- a/drivers/gpu/drm/xe/xe_gpu_scheduler.c
> +++ b/drivers/gpu/drm/xe/xe_gpu_scheduler.c
[ ... ]
> @@ -42,13 +46,14 @@ static void xe_sched_process_msg_work(struct work_struct *w)
>  	struct xe_gpu_scheduler *sched =
>  		container_of(w, struct xe_gpu_scheduler, work_process_msg);
>  	struct xe_sched_msg *msg;
> +	unsigned int opcode;
>  
>  	if (drm_sched_is_stopped(&sched->base))
>  		return;

[Severity: High]
This isn't a bug introduced by this patch, but this early return appears
to leak the xe_exec_queue memory and a reference to drm_device when the
device is wedged.

When the device is wedged, xe_sched_submission_stop() sets sched->pause_submit
to true and cancels the message processing worker. 

Later, when queues are destroyed (for example via FD close or
guc_submit_fini()), guc_exec_queue_destroy() adds a STATIC_MSG_CLEANUP message
to the scheduler's message list:

drivers/gpu/drm/xe/xe_guc_submit.c:guc_exec_queue_destroy() {
    ...
    struct xe_sched_msg *msg = q->guc->static_msgs + STATIC_MSG_CLEANUP;

    guc_exec_queue_add_msg(q, msg, CLEANUP);
    ...
}

Because the scheduler is stopped, xe_sched_process_msg_work() immediately
returns without processing the cleanup message.

As a result, the queue's async destruction
(__guc_exec_queue_process_msg_cleanup() -> guc_exec_queue_do_destroy()) is
never invoked.

This can be reached by any user that opens the DRM FD, runs a workload that
causes a GPU hang (triggering a wedge), and then closes the FD.

Could this be modified to allow processing of cleanup messages even when the
scheduler is stopped, or handled in an alternate teardown path, so we don't
permanently leak the xe_exec_queue and prevent the DRM module from being
cleanly unloaded?

>  
> -	msg = xe_sched_get_msg(sched);
> +	msg = xe_sched_get_msg(sched, &opcode);
>  	if (msg) {
> -		sched->ops->process_msg(msg);
> +		sched->ops->process_msg(msg, opcode);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901011712.2617466-1-matthew.brost@intel.com?part=1