Re: [PATCH] drm/xe: Skip GT TLB invalidation when VM has no queues mapped

[Matthew Brost <matthew.brost@intel.com>]

On Fri, Aug 07, 2026 at 01:58:02PM -0600, Summers, Stuart wrote:
> On Thu, 2026-08-06 at 12:14 -0700, Matthew Brost wrote:
> > 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.
> 
> Ok.. so in the event that something like this does become required, the
> expectation is we'd add the required reference counts at that time?
>

The existing code always issues an ASID TLB invalidation, regardless of
whether any queues are attached, which is the a performance issue. This
change makes that scenario significantly less likely to occur.

If something changes that makes this approach unsafe, then we would need
to update the existing implementation as well, likely by adopting
something similar to what is proposed here, combined with a
reference-counting mechanism akin to the context TLB invalidation
scheme.

> I'm just a little worried we're overoptimising here and will cause some
> issues down the road.

CI is green. I wouldn't call this over-optimization. It's a structural
fix that correctly identifies work that can be skipped, which improves
performance across the board.

A TLB invalidation blocks the GuC from doing anything else, so avoiding
an unnecessary TLB invalidation allows the GuC to make forward progress
on other work. The RC6 case is where this really hurts. If a TLB
invalidation required to service a page fault takes 70 µs (compared to a
20 µs baseline) and the copy itself takes 90 µs, we've already lost a
significant amount of throughput. Likewise, if the shrinker is under
memory pressure, this invalidation adds 50 µs or so before the move
operation can proceed. Likewise if userspace wants to shrink is caches
via unbind, now unbinds take 50 µs longer. This ripple effect propagates
throughout the stack, and small wins like this add up over time.

It's also incredibly common for nothing to be mapped in the media GT. I
have a BMG part running with this display active and a number of WebGL
Chrome tabs open, yet there isn't a single exec queue open on the media
GT. In that scenario, the media GT is likely sitting in RC6. I think
anything Mesa or level0 based, won't have anything on the media GT (not
100% sure on this though).

Matt

> 
> Thanks,
> Stuart
> 
> > 
> > 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);
> > > 
>