From: Matthew Brost <matthew.brost@intel.com>
To: "Summers, Stuart" <stuart.summers@intel.com>
Cc: "intel-xe@lists.freedesktop.org" <intel-xe@lists.freedesktop.org>
Subject: Re: [PATCH] drm/xe: Skip GT TLB invalidation when VM has no queues mapped
Date: Thu, 6 Aug 2026 12:14:37 -0700 [thread overview]
Message-ID: <anTdHVUUQ+P86IrR@gsse-cloud1.jf.intel.com> (raw)
In-Reply-To: <68a4c12e90c7f3fc08a62a6de41c9bb26b38fc23.camel@intel.com>
On Thu, Aug 06, 2026 at 01:04:28PM -0600, Summers, Stuart wrote:
> On Wed, 2026-08-05 at 20:32 -0700, Matthew Brost wrote:
> > If no exec queues from a VM are mapped on a GT, issuing a PPGTT TLB
> > invalidation for that GT can require an rc6 wake which is expensive.
> >
> > Skip the media TLB invalidation when the VM has no exec queues
> > mapped on it. If TLB invalidations are already in-flight on that GT
> > we can't break fence ordering, so issue a dummy GGTT invalidation
> > instead to maintain seqno ordering.
> >
> > This optimization is particularly impactful for SVM workloads which
> > may or may not use the media GT. Average TLB invalidation time drops
> > from ~75us to ~18us in such benchmarks on certain BMG parts - the
> > improvement varies based on platform.
>
> Still going through the code changes, but is there a chance we could
> deregister a context (so no queues exist), then mmap to invalidate,
> then register a new context and read stale data here? I think in the
> context invalidation case where we don't have queues we would normally
> send a full invalidation instead rather than just skipping it. That
> seems risky...
We don't call `xe_vm_remove_exec_queue()` until
`__xe_exec_queue_free()`. The latter is only called after all GuC
references are gone (i.e., deregistration has completed, or GuC state
has otherwise been torn down due to PM events or a GT reset). Therefore,
there is no race where we accidentally fail to send a required TLB
invalidation. If anything, the opposite can happen: we may send an
unnecessary TLB invalidation.
We don't need to use the queue reference-counting trick here to prevent
deregistration, because whether a queue is registered is immaterial to
successful TLB invalidation. The interface invalidates an ASID rather
than the CTXID that the GuC tracks and requires a valid queue.
So all of this is safe.
Matt
>
> Thanks,
> Stuart
>
> >
> > Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> >
> > ---
> > v2:
> > - Make GT generic rather than just media GT (Thomas)
> > - Fix accounting bug in empty vs non-empty (CI)
> > ---
> > drivers/gpu/drm/xe/xe_guc_tlb_inval.c | 20 ++++++++++++++++++--
> > drivers/gpu/drm/xe/xe_vm.c | 12 ++----------
> > 2 files changed, 20 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/xe/xe_guc_tlb_inval.c
> > b/drivers/gpu/drm/xe/xe_guc_tlb_inval.c
> > index 046d0655122f..ab04b87cf1c3 100644
> > --- a/drivers/gpu/drm/xe/xe_guc_tlb_inval.c
> > +++ b/drivers/gpu/drm/xe/xe_guc_tlb_inval.c
> > @@ -205,11 +205,27 @@ static int send_tlb_inval_asid_ppgtt(struct
> > xe_tlb_inval *tlb_inval, u32 seqno,
> > struct drm_suballoc *prl_sa)
> > {
> > struct xe_guc *guc = tlb_inval->private;
> > + struct xe_device *xe = guc_to_xe(guc);
> > + struct xe_vm *vm;
> > + int err, id = guc_to_gt(guc)->info.id;
> >
> > lockdep_assert_held(&tlb_inval->seqno_lock);
> >
> > - return send_tlb_inval_ppgtt(guc, seqno, start, end, asid,
> > - XE_GUC_TLB_INVAL_PAGE_SELECTIVE,
> > prl_sa);
> > + vm = xe_device_asid_to_vm(xe, asid);
> > + if (IS_ERR(vm))
> > + return PTR_ERR(vm);
> > +
> > + down_read(&vm->exec_queues.lock);
> > + if (!vm->exec_queues.count[id] &&
> > xe_tlb_inval_idle(tlb_inval))
> > + err = -ECANCELED;
> > + else
> > + err = send_tlb_inval_ppgtt(guc, seqno, start, end,
> > asid,
> > +
> > XE_GUC_TLB_INVAL_PAGE_SELECTIVE,
> > + prl_sa);
> > + up_read(&vm->exec_queues.lock);
> > + xe_vm_put(vm);
> > +
> > + return err;
> > }
> >
> > static int send_tlb_inval_ctx_ppgtt(struct xe_tlb_inval *tlb_inval,
> > u32 seqno,
> > diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> > index 9e0176861cb6..e2667200462c 100644
> > --- a/drivers/gpu/drm/xe/xe_vm.c
> > +++ b/drivers/gpu/drm/xe/xe_vm.c
> > @@ -4946,8 +4946,7 @@ int xe_vm_alloc_cpu_addr_mirror_vma(struct
> > xe_vm *vm, uint64_t start, uint64_t r
> > * @vm: The VM.
> > * @q: The exec_queue
> > *
> > - * Add exec queue to VM, skipped if the device does not have context
> > based TLB
> > - * invalidations.
> > + * Add exec queue to VM.
> > */
> > void xe_vm_add_exec_queue(struct xe_vm *vm, struct xe_exec_queue *q)
> > {
> > @@ -4961,9 +4960,6 @@ void xe_vm_add_exec_queue(struct xe_vm *vm,
> > struct xe_exec_queue *q)
> > xe_assert(xe, vm->xef);
> > xe_assert(xe, vm == q->vm);
> >
> > - if (!xe->info.has_ctx_tlb_inval)
> > - return;
> > -
> > down_write(&vm->exec_queues.lock);
> > list_add(&q->vm_exec_queue_link, &vm->exec_queues.list[q->gt-
> > >info.id]);
> > ++vm->exec_queues.count[q->gt->info.id];
> > @@ -4975,14 +4971,10 @@ void xe_vm_add_exec_queue(struct xe_vm *vm,
> > struct xe_exec_queue *q)
> > * @vm: The VM.
> > * @q: The exec_queue
> > *
> > - * Remove exec queue from VM, skipped if the device does not have
> > context based
> > - * TLB invalidations.
> > + * Remove exec queue from VM.
> > */
> > void xe_vm_remove_exec_queue(struct xe_vm *vm, struct xe_exec_queue
> > *q)
> > {
> > - if (!vm->xe->info.has_ctx_tlb_inval)
> > - return;
> > -
> > down_write(&vm->exec_queues.lock);
> > if (!list_empty(&q->vm_exec_queue_link)) {
> > list_del(&q->vm_exec_queue_link);
>
next prev parent reply other threads:[~2026-08-06 19:15 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-06 3:32 [PATCH] drm/xe: Skip GT TLB invalidation when VM has no queues mapped Matthew Brost
2026-08-06 3:39 ` ✓ CI.KUnit: success for " Patchwork
2026-08-06 4:16 ` ✓ Xe.CI.BAT: " Patchwork
2026-08-06 14:03 ` ✓ Xe.CI.FULL: " Patchwork
2026-08-06 19:04 ` [PATCH] " Summers, Stuart
2026-08-06 19:14 ` Matthew Brost [this message]
2026-08-07 19:58 ` Summers, Stuart
2026-08-07 20:28 ` Matthew Brost
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=anTdHVUUQ+P86IrR@gsse-cloud1.jf.intel.com \
--to=matthew.brost@intel.com \
--cc=intel-xe@lists.freedesktop.org \
--cc=stuart.summers@intel.com \
/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