Intel-XE Archive on lore.kernel.org
 help / color / mirror / Atom feed
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);
> 

  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