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

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 <[email protected]>
> > 
> > ---
> > 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);
>