Re: [PATCH v5 10/25] drm/xe: Update GuC submission backend to run PT jobs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] PT jobs silently falsely report successful completion to the DRM scheduler when skipped due to a killed, banned, or wedged execution queue, or when the job has an error.
- [Medium] Memory leak of `pt_job_ops` when a PT job is canceled or destroyed before execution.
--

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

drm/xe: Update GuC submission backend to run PT jobs

This commit updates the GuC submission backend to execute PT (page table)
jobs via the CPU. These jobs bypass normal GPU execution to program page
tables for the final step of a bind job.

Link: https://patch.msgid.link/20260228013501.106680-11-matthew.brost@intel.com

> diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c
> index cf306568ef307..528869928c1d3 100644
> --- a/drivers/gpu/drm/xe/xe_guc_submit.c
> +++ b/drivers/gpu/drm/xe/xe_guc_submit.c
[ ... ]
> @@ -1261,14 +1277,25 @@ guc_exec_queue_run_job(struct drm_sched_job *drm_job)
[ ... ]
>  		job->restore_replay = false;
>  	}
>  
> +	if (is_pt_job(job)) {

[Severity: Critical]
Does returning NULL here cause PT jobs to silently report successful
completion to the DRM scheduler when they were actually skipped?

If the execution queue is killed, banned, or wedged, or if the job has
an error, the execution block earlier in guc_exec_queue_run_job() is
skipped. However, this check unconditionally returns NULL for PT jobs
even in those cases.

Could this regression result in physical memory being freed and reallocated
while the page tables were never updated by the CPU, potentially allowing
a different queue in the same VM to access the reallocated memory?

> +		xe_pt_job_ops_put(job->pt_update[0].pt_job_ops);

[Severity: Medium]
Is there a potential memory leak of the xe_pt_job_ops structures if a
PT job is canceled or destroyed before it reaches this execution phase?

This drops the reference to pt_job_ops during the run_job execution phase.
If the DRM scheduler is torn down (e.g. due to queue destruction, wedge,
or driver unload) while jobs are pending, the backend free_job callback
is invoked instead, bypassing this execution path.

Does the teardown path in xe_sched_job_destroy() need to drop these
pt_job_ops references as well to prevent a regression?

> +		dma_fence_put(job->fence);	/* Drop ref from xe_sched_job_arm */
> +		return NULL;
> +	}
> +
>  run_job_out:
>  
>  	return job->fence;

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