All of lore.kernel.org
 help / color / mirror / Atom feed
From: Matthew Brost <matthew.brost@intel.com>
To: <sashiko-reviews@lists.linux.dev>
Cc: <intel-xe@lists.freedesktop.org>
Subject: Re: [PATCH v7 21/24] drm/xe: Add ULLS migration job support to GuC submission
Date: Fri, 25 Sep 2026 00:08:14 -0700	[thread overview]
Message-ID: <arYd3ihnEYklAbpR@gsse-cloud1.jf.intel.com> (raw)
In-Reply-To: <20260925064857.7FE391F000FF@smtp.kernel.org>

On Fri, Sep 25, 2026 at 06:48:57AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] drm/xe: TOCTOU race in ULLS migration job prefetching leads to stale command execution
> --
> 
> commit 231db353d11582f236795322ee38df1ad9cad0d1
> Author: Matthew Brost <matthew.brost@intel.com>
> drm/xe: Add ULLS migration job support to GuC submission
> 
> This commit adds Ultra Low Latency Submission (ULLS) migration job support to
> the GuC submission backend. It introduces optimizations for chained ULLS jobs
> by having the GPU publish its own ring tail and suppressing the CPU's RING_TAIL
> MMIO updates.
> 
> > diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c
> > index a2784763e72ba..cd9bb9f9e28c8 100644
> > --- a/drivers/gpu/drm/xe/xe_guc_submit.c
> > +++ b/drivers/gpu/drm/xe/xe_guc_submit.c
> > @@ -1190,9 +1190,10 @@ static void submit_exec_queue(struct xe_exec_queue *q, struct xe_sched_job *job)
> >  	xe_gt_assert(guc_to_gt(guc), exec_queue_registered(q));
> >  
> >  	if (!job->restore_replay || job->last_replay) {
> > +		/* A ULLS job past the first publishes its own ring tail */
> >  		if (xe_exec_queue_is_parallel(q))
> >  			wq_item_append(q);
> > -		else
> > +		else if (!xe_sched_job_ulls_is_chained(job))
> >  			xe_lrc_set_ring_tail(lrc, lrc->ring.tail);
> 
> [Severity: High]

Not observed in testing - semaphore wait acts as prefetch barrier.
Before semaphore release all CPU writes are GPU visable.

Matt 

> Does skipping the RING_TAIL MMIO write here expose a race condition where the
> GPU could execute stale ring buffer memory?
> 
> For ULLS chained jobs, the GPU advances the tail via MI_LRI prior to executing
> MI_SEMAPHORE_WAIT, which happens in emit_ulls_postamble():
> 
> drivers/gpu/drm/xe/xe_ring_ops.c:emit_ulls_postamble()
> 	i = emit_ulls_ring_tail(gt, lrc, dw, i, head);
> 	dw[i++] = MI_SEMAPHORE_WAIT | ...
> 
> Because the CPU intentionally skips the xe_lrc_set_ring_tail() MMIO write for
> these chained jobs, the hardware mechanism that normally invalidates the GPU's
> instruction cache is bypassed. 
> 
> Could the Command Streamer fetch unit see the advanced RING_TAIL and prefetch
> memory for the next job before the CPU has actually written it to the ring
> buffer? If the CPU hasn't populated the commands yet, when the CPU finally
> writes the next job and signals the semaphore, it appears the GPU might wake up
> and execute the stale prefetched instructions from its internal cache rather
> than the newly written migration commands.
> 
> [ ... ]
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260925045320.1325860-1-matthew.brost@intel.com?part=21

  reply	other threads:[~2026-09-25  7:10 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-25  4:52 [PATCH v7 00/24] CPU binds and ULLS on migration queue Matthew Brost
2026-09-25  4:52 ` [PATCH v7 01/24] drm/xe: reference VM from PT BOs Matthew Brost
2026-09-25 12:00   ` Francois Dugast
2026-09-25 16:10     ` Matthew Brost
2026-09-25  4:52 ` [PATCH v7 02/24] drm/xe: Drop struct xe_migrate_pt_update argument from populate/clear vfuns Matthew Brost
2026-09-25  4:52 ` [PATCH v7 03/24] drm/xe: Add xe_migrate_update_pgtables_cpu_execute helper Matthew Brost
2026-09-25  4:53 ` [PATCH v7 04/24] drm/xe: Decouple exec queue idle check from LRC Matthew Brost
2026-09-25  4:53 ` [PATCH v7 05/24] drm/xe: Add job count to GuC exec queue snapshot Matthew Brost
2026-09-25  4:53 ` [PATCH v7 06/24] drm/xe: Update xe_bo_put_deferred arguments to include writeback flag Matthew Brost
2026-09-25  4:53 ` [PATCH v7 07/24] drm/xe: Update scheduler job layer to support PT jobs Matthew Brost
2026-09-25 11:23   ` Francois Dugast
2026-09-25  4:53 ` [PATCH v7 08/24] drm/xe: Add helpers to access PT ops Matthew Brost
2026-09-25  4:53 ` [PATCH v7 09/24] drm/xe: Add struct xe_pt_job_ops Matthew Brost
2026-09-25  4:53 ` [PATCH v7 10/24] drm/xe: Update GuC submission backend to run PT jobs Matthew Brost
2026-09-25  5:39   ` sashiko-bot
2026-09-25  5:58     ` Matthew Brost
2026-09-25  4:53 ` [PATCH v7 11/24] drm/xe: Store level in struct xe_vm_pgtable_update Matthew Brost
2026-09-25  4:53 ` [PATCH v7 12/24] drm/xe: Don't use migrate exec queue for page fault binds Matthew Brost
2026-09-25  4:53 ` [PATCH v7 13/24] drm/xe: Enable CPU binds for jobs Matthew Brost
2026-09-25  6:03   ` sashiko-bot
2026-09-25  6:54     ` Matthew Brost
2026-09-25  4:53 ` [PATCH v7 14/24] drm/xe: Remove unused arguments from xe_migrate_pt_update_ops Matthew Brost
2026-09-25  4:53 ` [PATCH v7 15/24] drm/xe: Make bind queues operate cross-tile Matthew Brost
2026-09-25  4:53 ` [PATCH v7 16/24] drm/xe: Add CPU bind layer Matthew Brost
2026-09-25  4:53 ` [PATCH v7 17/24] drm/xe: Add device flag to enable PT mirroring across tiles Matthew Brost
2026-09-25  6:25   ` sashiko-bot
2026-09-25  7:21     ` Matthew Brost
2026-09-25  9:51   ` Francois Dugast
2026-09-25  4:53 ` [PATCH v7 18/24] drm/xe: Add ULLS support to LRC Matthew Brost
2026-09-25  4:53 ` [PATCH v7 19/24] drm/xe: Add ULLS migration job support to migration layer Matthew Brost
2026-09-25  6:34   ` sashiko-bot
2026-09-25  7:17     ` Matthew Brost
2026-09-25 20:10       ` Matthew Brost
2026-09-25  4:53 ` [PATCH v7 20/24] drm/xe: Add ULLS migration job support to ring ops Matthew Brost
2026-09-25  6:38   ` sashiko-bot
2026-09-25  7:02     ` Matthew Brost
2026-09-25  4:53 ` [PATCH v7 21/24] drm/xe: Add ULLS migration job support to GuC submission Matthew Brost
2026-09-25  6:48   ` sashiko-bot
2026-09-25  7:08     ` Matthew Brost [this message]
2026-09-25  4:53 ` [PATCH v7 22/24] drm/xe: Enter ULLS for migration jobs upon page fault or SVM prefetch Matthew Brost
2026-09-25 17:49   ` Maarten Lankhorst
2026-09-25 18:17     ` Matthew Brost
2026-09-25 18:26       ` Maarten Lankhorst
2026-09-25 19:38         ` Matthew Brost
2026-09-25  4:53 ` [PATCH v7 23/24] drm/xe: add migrate ULLS period configfs attribute Matthew Brost
2026-09-25  6:51   ` sashiko-bot
2026-09-25  7:08     ` Matthew Brost
2026-09-25  4:53 ` [PATCH v7 24/24] drm/xe: Document ULLS for migration jobs Matthew Brost
2026-09-25  5:02 ` ✗ CI.checkpatch: warning for CPU binds and ULLS on migration queue (rev9) Patchwork
2026-09-25  5:04 ` ✓ CI.KUnit: success " Patchwork
2026-09-25  5:47 ` ✓ Xe.CI.BAT: " Patchwork
2026-09-25 15:05 ` ✗ Xe.CI.FULL: failure " Patchwork
2026-09-25 16:01   ` Matthew Brost
2026-09-25 18:08 ` [PATCH v7 00/24] CPU binds and ULLS on migration queue Maarten Lankhorst

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=arYd3ihnEYklAbpR@gsse-cloud1.jf.intel.com \
    --to=matthew.brost@intel.com \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.