Re: [PATCH v5 13/25] drm/xe: Enable CPU binds for jobs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing handling for CPU-based PT jobs in the execlist backend causes the GPU to execute kernel pointers as a batch address and skips page table updates.
- [Medium] Leaked `xe_pt_job_ops` and `dma_fence` references when a PT job is canceled by the DRM scheduler before execution.
--

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

drm/xe: Enable CPU binds for jobs

This commit transitions the driver to use CPU-based binds instead of the
GPU for page table updates. It formulates a PT job that issues the CPU
bind in the submission backend and removes previous GPU-based binding code.

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

> diff --git a/drivers/gpu/drm/xe/xe_migrate.c b/drivers/gpu/drm/xe/xe_migrate.c
> index ba8e195afcc89..e5c46e0fa9607 100644
> --- a/drivers/gpu/drm/xe/xe_migrate.c
> +++ b/drivers/gpu/drm/xe/xe_migrate.c
[ ... ]
> @@ -2104,6 +1889,12 @@ __xe_migrate_update_pgtables(struct xe_migrate *m,
>  	if (is_migrate)
>  		mutex_lock(&m->job_mutex);
>  
> +	job->pt_update[0].vm = pt_update->vops->vm;
> +	job->pt_update[0].tile = tile;
> +	job->pt_update[0].ops = ops;
> +	job->pt_update[0].pt_job_ops =
> +		xe_pt_job_ops_get(pt_update_ops->pt_job_ops);
> +

[Severity: High]
Does this new CPU-based PT job cause the GPU to execute garbage when using
the execlist submission backend?

When GuC is disabled, the execlist backend processes the job. However,
execlist_run_job() in drivers/gpu/drm/xe/xe_execlist.c unconditionally
pushes jobs to the GPU without checking if it is a CPU-only PT job.

Because struct xe_sched_job uses a union for these arguments:

	union {
		/** @ptrs: per instance pointers. */
		DECLARE_FLEX_ARRAY(struct xe_job_ptrs, ptrs);
		/** @pt_update: PT update arguments */
		DECLARE_FLEX_ARRAY(struct xe_pt_update_args, pt_update);
	};

the execlist backend will interpret job->ptrs[0].batch_addr (which perfectly
aliases the pt_job_ops pointer) as a physical batch address. This emits the
kernel pointer to the hardware ring, leading to a GPU hang, invalid
instruction faults, and unexecuted page table updates.

[Severity: Medium]
Will the xe_pt_job_ops and dma_fence references leak if the PT job is
canceled by the DRM scheduler before execution?

When a PT job is canceled (e.g., due to process termination or a wedged
device), it bypasses .run_job (like guc_exec_queue_run_job, where cleanup
normally occurs) and directly goes to .free_job, which resolves to
xe_sched_job_destroy() in drivers/gpu/drm/xe/xe_sched_job.c.

The generic xe_sched_job_destroy() routine only drops a single fence
reference and lacks the logic to drop the xe_pt_job_ops reference acquired
here, permanently leaking both.

>  	xe_sched_job_arm(job);
>  	fence = dma_fence_get(&job->drm.s_fence->finished);
>  	xe_sched_job_push(job);

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