All of lore.kernel.org
 help / color / mirror / Atom feed
From: Matthew Brost <matthew.brost@intel.com>
To: "Thomas Hellström" <thomas.hellstrom@linux.intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>,
	<intel-xe@lists.freedesktop.org>, <stable@vger.kernel.org>
Subject: Re: [PATCH] drm/xe/vm: nuke PTs only after unlinking contested VMAs
Date: Fri, 18 Sep 2026 14:53:48 -0700	[thread overview]
Message-ID: <aq2y7LEdPDqFWTql@gsse-cloud1.jf.intel.com> (raw)
In-Reply-To: <8c1744e4012ab5b30d1f26f8129d4944da1dcfa3.camel@linux.intel.com>

On Fri, Sep 18, 2026 at 04:35:43PM +0200, Thomas Hellström wrote:
> On Fri, 2026-09-18 at 14:10 +0100, Matthew Auld wrote:
> > In xe_vm_close_and_put(), external-BO VMAs are queued on the
> > contested
> > list for deferred destruction via xe_vma_destroy_unlocked(). However,
> > xe_vm_pt_destroy() was previously invoked before processing contested
> > VMAs, destroying vm->pt_root while those VMAs were still linked to
> > their
> > respective buffer objects (vm_bo->list.gpuva).
> > 
> > If a concurrent thread evicts one of those shared buffer objects,
> > xe_bo_trigger_rebind() holding only bo->resv walks the BO's VMAs and,
> > in
> > fault mode, calls xe_vm_invalidate_vma() -> xe_pt_zap_ptes(). Because
> > vm->pt_root[tile->id] is already NULL, dereferencing pt->level causes
> > a
> > NULL ptr deref.
> > 
> > Fix this by deferring xe_vm_free_scratch() and xe_vm_pt_destroy()
> > until
> > after all contested VMAs have been unlinked and destroyed.
> > 
> > User is reporting hitting a NULL ptr deref in xe_pt_zap_ptes(), which
> > could be explained by this race.
> > 
> > Assisted-by: LLM
> > Fixes: b06d47be7c83 ("drm/xe: Port Xe to GPUVA")
> > Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/9290
> > Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> > Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> > Cc: Matthew Brost <matthew.brost@intel.com>
> > Cc: <stable@vger.kernel.org> # v6.12+
> 
> Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> 

I actually had a similar fix here [1] but I believe I missed the scratch
part.

Glad to see this fix getting in:
Reviewed-by: Matthew Brost <matthew.brost@intel.com>

[1] https://patchwork.freedesktop.org/patch/738642/?series=170045&rev=1

> 
> > ---
> >  drivers/gpu/drm/xe/xe_vm.c | 21 +++++++++------------
> >  1 file changed, 9 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> > index 17dc4debe7c1..390da884c727 100644
> > --- a/drivers/gpu/drm/xe/xe_vm.c
> > +++ b/drivers/gpu/drm/xe/xe_vm.c
> > @@ -1979,21 +1979,13 @@ void xe_vm_close_and_put(struct xe_vm *vm)
> >  		vma->gpuva.flags |= XE_VMA_DESTROYED;
> >  	}
> >  
> > -	/*
> > -	 * All vm operations will add shared fences to resv.
> > -	 * The only exception is eviction for a shared object,
> > -	 * but even so, the unbind when evicted would still
> > -	 * install a fence to resv. Hence it's safe to
> > -	 * destroy the pagetables immediately.
> > -	 */
> > -	xe_vm_free_scratch(vm);
> > -	xe_vm_pt_destroy(vm);
> >  	xe_vm_unlock(vm);
> >  
> >  	/*
> > -	 * VM is now dead, cannot re-add nodes to vm->vmas if it's
> > NULL
> > -	 * Since we hold a refcount to the bo, we can remove and
> > free
> > -	 * the members safely without locking.
> > +	 * Unlink and destroy all contested external-BO VMAs before
> > destroying
> > +	 * the page tables. Otherwise, concurrent eviction holding
> > only bo->resv
> > +	 * can walk the BO's VMAs and attempt to invalidate/zap page
> > tables that
> > +	 * have already been freed.
> >  	 */
> >  	list_for_each_entry_safe(vma, next_vma, &contested,
> >  				 combined_links.destroy) {
> > @@ -2001,6 +1993,11 @@ void xe_vm_close_and_put(struct xe_vm *vm)
> >  		xe_vma_destroy_unlocked(vma);
> >  	}
> >  
> > +	xe_vm_lock(vm, false);
> > +	xe_vm_free_scratch(vm);
> > +	xe_vm_pt_destroy(vm);
> > +	xe_vm_unlock(vm);
> > +
> >  	xe_svm_fini(vm);
> >  
> >  	up_write(&vm->lock);

  reply	other threads:[~2026-09-18 21:53 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-18 13:10 [PATCH] drm/xe/vm: nuke PTs only after unlinking contested VMAs Matthew Auld
2026-09-18 13:19 ` ✓ CI.KUnit: success for " Patchwork
2026-09-18 14:35 ` [PATCH] " Thomas Hellström
2026-09-18 21:53   ` Matthew Brost [this message]
2026-09-18 14:50 ` ✗ Xe.CI.BAT: failure for " Patchwork
2026-09-18 22:38 ` ✗ Xe.CI.FULL: " 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=aq2y7LEdPDqFWTql@gsse-cloud1.jf.intel.com \
    --to=matthew.brost@intel.com \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=matthew.auld@intel.com \
    --cc=stable@vger.kernel.org \
    --cc=thomas.hellstrom@linux.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.