From: Matthew Brost <matthew.brost@intel.com>
To: <sashiko-reviews@lists.linux.dev>
Cc: <intel-xe@lists.freedesktop.org>
Subject: Re: [PATCH v4 22/25] drm/xe: Add ULLS migration job support to GuC submission
Date: Thu, 3 Sep 2026 18:41:42 -0700 [thread overview]
Message-ID: <apoh1jwRR1jd5395@gsse-cloud1.jf.intel.com> (raw)
In-Reply-To: <20260904003837.BD1D71F000E9@smtp.kernel.org>
On Fri, Sep 04, 2026 at 12:38:37AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Critical] Direct MMIO write to the physical engine's RING_TAIL for GuC-scheduled ULLS jobs causes hardware state corruption if the context is preempted or evicted.
> --
>
> commit f60d4c34d363517fe528679e6ff3d059b7861099
> Author: Matthew Brost <matthew.brost@intel.com>
>
> drm/xe: Add ULLS migration job support to GuC submission
>
> This commit adds ULLS migration job support to the GuC submission backend.
> It modifies the migration queue job limits and introduces direct hardware
> engine tail writes for subsequent ULLS jobs.
>
> Link: https://patch.msgid.link/20260228013501.106680-24-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 dcb4b8a4f3b78..1578d80b01b82 100644
> > --- a/drivers/gpu/drm/xe/xe_guc_submit.c
> > +++ b/drivers/gpu/drm/xe/xe_guc_submit.c
> > @@ -1207,6 +1207,11 @@ static void submit_exec_queue(struct xe_exec_queue *q, struct xe_sched_job *job)
> > if (exec_queue_suspended(q))
> > return;
> >
> > + if (job->is_ulls && !job->is_ulls_first) {
> > + xe_hw_engine_write_ring_tail(q->hwe, lrc->ring.tail);
>
> [Severity: Critical]
> Can this direct MMIO write to the engine's RING_TAIL cause hardware state
> corruption if the ULLS context is preempted or evicted?
>
The GuC context can't ever be evicted or preempted as the migrate queue
completely owns the engine state which is way this is safe.
It is questionable if we should wait on the first ULLS job signaling so
we know the queue is on the hardware but I've yet to see any failures in
my testing. This is something to watch out for though - if we get a
random kernel queue hang, we should adjust to wait until first ULLS job
signals.
It we ever use multiple queues on the paging engine, we will also have
to use fences + ordering to ensure either all ULLS jobs are complete or
all non-ULLS jobs are complete before before jobs reach run_job().
Matt
> The driver bypasses the GuC submission interface (H2G) for ULLS jobs and
> directly writes to the engine's MMIO RING_TAIL register. If the ULLS
> migration context is not actively resident on the hardware engine at the exact
> moment of the write, the write will advance the ring tail of whichever context
> is currently resident on that engine (e.g., VM bind contexts sharing the same
> USM engine).
>
> Since the GuC scheduler fundamentally owns engine context switching, it can
> preempt or evict the ULLS context for internal tasks or schedule other queues
> that explicitly share the USM engine. If the CPU submits a ULLS job while the
> ULLS context is swapped out, would the direct MMIO tail write hit the
> currently running context instead?
>
> > + xe_lrc_set_ulls_semaphore(lrc, xe_sched_job_lrc_seqno(job));
> > + }
> > +
> > if (!exec_queue_enabled(q)) {
> > action[len++] = XE_GUC_ACTION_SCHED_CONTEXT_MODE_SET;
> > action[len++] = q->guc->id;
>
> [ ... ]
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260903235842.3401722-1-matthew.brost@intel.com?part=22
next prev parent reply other threads:[~2026-09-04 1:41 UTC|newest]
Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-03 23:58 [PATCH v4 00/25] CPU binds and ULLS on migration queue Matthew Brost
2026-09-03 23:58 ` [PATCH v4 01/25] drm/xe: Drop struct xe_migrate_pt_update argument from populate/clear vfuns Matthew Brost
2026-09-03 23:58 ` [PATCH v4 02/25] drm/xe: Add xe_migrate_update_pgtables_cpu_execute helper Matthew Brost
2026-09-04 0:15 ` sashiko-bot
2026-09-03 23:58 ` [PATCH v4 03/25] drm/xe: Decouple exec queue idle check from LRC Matthew Brost
2026-09-03 23:58 ` [PATCH v4 04/25] drm/xe: Add job count to GuC exec queue snapshot Matthew Brost
2026-09-03 23:58 ` [PATCH v4 05/25] drm/xe: Update xe_bo_put_deferred arguments to include writeback flag Matthew Brost
2026-09-03 23:58 ` [PATCH v4 06/25] drm/xe: Add XE_BO_FLAG_PUT_VM_ASYNC Matthew Brost
2026-09-04 0:18 ` sashiko-bot
2026-09-04 0:41 ` Matthew Brost
2026-09-03 23:58 ` [PATCH v4 07/25] drm/xe: Update scheduler job layer to support PT jobs Matthew Brost
2026-09-04 0:25 ` sashiko-bot
2026-09-03 23:58 ` [PATCH v4 08/25] drm/xe: Add helpers to access PT ops Matthew Brost
2026-09-03 23:58 ` [PATCH v4 09/25] drm/xe: Add struct xe_pt_job_ops Matthew Brost
2026-09-03 23:58 ` [PATCH v4 10/25] drm/xe: Update GuC submission backend to run PT jobs Matthew Brost
2026-09-04 0:36 ` sashiko-bot
2026-09-04 0:57 ` Matthew Brost
2026-09-03 23:58 ` [PATCH v4 11/25] drm/xe: Store level in struct xe_vm_pgtable_update Matthew Brost
2026-09-04 0:19 ` sashiko-bot
2026-09-03 23:58 ` [PATCH v4 12/25] drm/xe: Don't use migrate exec queue for page fault binds Matthew Brost
2026-09-03 23:58 ` [PATCH v4 13/25] drm/xe: Enable CPU binds for jobs Matthew Brost
2026-09-04 0:31 ` sashiko-bot
2026-09-04 1:04 ` Matthew Brost
2026-09-03 23:58 ` [PATCH v4 14/25] drm/xe: Remove unused arguments from xe_migrate_pt_update_ops Matthew Brost
2026-09-03 23:58 ` [PATCH v4 15/25] drm/xe: Make bind queues operate cross-tile Matthew Brost
2026-09-03 23:58 ` [PATCH v4 16/25] drm/xe: Add CPU bind layer Matthew Brost
2026-09-04 0:31 ` sashiko-bot
2026-09-04 1:18 ` Matthew Brost
2026-09-03 23:58 ` [PATCH v4 17/25] drm/xe: Add device flag to enable PT mirroring across tiles Matthew Brost
2026-09-04 0:29 ` sashiko-bot
2026-09-04 1:33 ` Matthew Brost
2026-09-03 23:58 ` [PATCH v4 18/25] drm/xe: Add xe_hw_engine_write_ring_tail Matthew Brost
2026-09-03 23:58 ` [PATCH v4 19/25] drm/xe: Add ULLS support to LRC Matthew Brost
2026-09-03 23:58 ` [PATCH v4 20/25] drm/xe: Add ULLS migration job support to migration layer Matthew Brost
2026-09-04 0:27 ` sashiko-bot
2026-09-04 1:35 ` Matthew Brost
2026-09-03 23:58 ` [PATCH v4 21/25] drm/xe: Add ULLS migration job support to ring ops Matthew Brost
2026-09-03 23:58 ` [PATCH v4 22/25] drm/xe: Add ULLS migration job support to GuC submission Matthew Brost
2026-09-04 0:38 ` sashiko-bot
2026-09-04 1:41 ` Matthew Brost [this message]
2026-09-03 23:58 ` [PATCH v4 23/25] drm/xe: Enter ULLS for migration jobs upon page fault or SVM prefetch Matthew Brost
2026-09-04 0:28 ` sashiko-bot
2026-09-04 1:32 ` Matthew Brost
2026-09-03 23:58 ` [PATCH v4 24/25] drm/xe: Add modparam to enable / disable ULLS on migrate queue Matthew Brost
2026-09-03 23:58 ` [PATCH v4 25/25] drm/xe: Document ULLS for migration jobs Matthew Brost
2026-09-04 0:47 ` ✗ CI.checkpatch: warning for CPU binds and ULLS on migration queue (rev6) Patchwork
2026-09-04 0:49 ` ✓ CI.KUnit: success " Patchwork
2026-09-04 1:33 ` ✓ Xe.CI.BAT: " Patchwork
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=apoh1jwRR1jd5395@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox