Re: [PATCH v2 2/2] drm/xe: Set job credits based on LRC size

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/xe: Ring buffer overflow due to static LRC initialization not accounted in credit limit
--

commit 89815f911de168c13f3cdfa254a0cefac8d7b016
Author: Maarten Lankhorst <dev@lankhorst.se>
drm/xe: Set job credits based on LRC size

This commit changes the scheduler credit limit to be based on the full LRC
size minus 4, rather than dividing by MAX_JOB_SIZE_BYTES, to allow filling
the entire LRC without throttling.

> diff --git a/drivers/gpu/drm/xe/xe_execlist.c b/drivers/gpu/drm/xe/xe_execlist.c
> index a36db39dcda8a..24469990ece97 100644
> --- a/drivers/gpu/drm/xe/xe_execlist.c
> +++ b/drivers/gpu/drm/xe/xe_execlist.c
> @@ -329,7 +329,7 @@ static int execlist_exec_queue_init(struct xe_exec_queue *q)
>  	const struct drm_sched_init_args args = {
>  		.ops = &drm_sched_ops,
>  		.num_rqs = 1,
> -		.credit_limit = xe_lrc_ring_size() / MAX_JOB_SIZE_BYTES,
> +		.credit_limit = xe_lrc_ring_size() - 4,

[Severity: High]
Does this limit account for the static initialization bytes emitted by
xe_lrc_init()? Since xe_lrc_init() emits 8 or 24 bytes of static initialization
commands into the ring buffer, advancing the initial tail pointer, a
credit_limit of xe_lrc_ring_size() - 4 (16380 bytes) only tracks dynamic job
payloads.

If a burst of jobs is submitted to a newly created queue before the hardware
processes the initial commands (while head == 0), could the total bytes in the
ring buffer reach 16376 + 8 = 16384 or 16376 + 24 = 16400? Since the ring
capacity is 16384, this could wrap the tail pointer to exactly head (0) or past
it, overwriting unconsumed initial bytes or appearing empty to the hardware,
which would lead to a permanent GPU hang or command corruption.

>  		.hang_limit = XE_SCHED_HANG_LIMIT,
>  		.timeout = XE_SCHED_JOB_TIMEOUT,
>  		.name = q->hwe->name,
> diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c
> index f3ba8abfc228e..264e961fd7d71 100644
> --- a/drivers/gpu/drm/xe/xe_guc_submit.c
> +++ b/drivers/gpu/drm/xe/xe_guc_submit.c
> @@ -2089,7 +2089,7 @@ static int guc_exec_queue_init(struct xe_exec_queue *q)
>  	}
>  
>  	err = xe_sched_init(&ge->sched, &drm_sched_ops, &xe_sched_ops,
> -			    submit_wq, xe_lrc_ring_size() / MAX_JOB_SIZE_BYTES, 64,
> +			    submit_wq, xe_lrc_ring_size() - 4, 64,

[Severity: High]
Does the GuC submission path also suffer from the same ring buffer overflow
issue as described above? It looks like this initialization uses the same
unsafe xe_lrc_ring_size() - 4 capacity limit which might not account for the
static LRC initialization bytes.

>  			    timeout, guc_to_gt(guc)->ordered_wq, NULL,
>  			    ge->name, gt_to_xe(q->gt)->drm.dev);
>  	if (err)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260925172603.302050-4-dev@lankhorst.se?part=2