Intel-XE Archive on 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: Wait on external BO kernel fences in exec IOCTL
Date: Thu, 9 Jul 2026 12:02:30 -0700	[thread overview]
Message-ID: <ak/wRrF9DWBdykiP@gsse-cloud1.jf.intel.com> (raw)
In-Reply-To: <42f4a99a3b572f7141ff1a2d7db2854d457300c4.camel@linux.intel.com>

On Thu, Jul 09, 2026 at 01:01:07PM +0200, Thomas Hellström wrote:
> On Fri, 2026-07-03 at 09:45 +0100, Matthew Auld wrote:
> > On 02/07/2026 22:58, Matthew Brost wrote:
> > > Before arming a user job, xe_exec_ioctl() only added the VM's
> > > dma-resv KERNEL slot as a dependency. That slot covers rebinds and
> > > the kernel operations of the VM's private BOs, but not external BOs
> > > (bo->vm == NULL), which carry their kernel operations (evictions,
> > > moves, ...) in their own dma-resv KERNEL slot.
> > > 
> > > The DMA_RESV_USAGE_KERNEL slot is the cross-driver contract for
> > > memory management operations that must complete before the BO or
> > > its
> > > backing store may be used: any accessor is required to wait on the
> > > KERNEL fences before touching the resv. By skipping the external
> > > BOs'
> > > KERNEL slots, the exec path violated that contract and could
> > > schedule
> > > a user job while a kernel operation on an external BO mapped by the
> > > VM
> > > was still in flight, racing against it and potentially reading or
> > > writing memory that was being moved.
> > > 
> > > Replace the VM-only dependency with an iteration over every object
> > > locked by the exec, adding each object's KERNEL slot as a job
> > > dependency. This covers the VM resv (rebinds and private BOs) as
> > > well
> > > as every external BO, mirroring the drm_gpuvm_resv_add_fence() call
> > > that later publishes the job fence to the same set of objects.
> > > Long-running mode continues to skip this, as before.
> > > 
> > > Fixes: dd08ebf6c352 ("drm/xe: Introduce a new DRM driver for Intel
> > > GPUs")
> > > Cc: stable@vger.kernel.org
> > > Assisted-by: GitHub_Copilot:claude-opus-4.8
> > > Signed-off-by: Matthew Brost <matthew.brost@intel.com>
> > 
> > Wow, kind of surprised we missed this.
> 
> Hm. Does this actually add any additional kernel fences to the exec
> dep?
> 
> Isn't the safety mechanism we have that no valid GPU PTEs are allowed
> to be set up with active kernel fences, and in the cases (rebinds,
> munmap split) we generate a VM kernel fence. 

I think that is an arbitrary Xe-enforced rule that just happens to be
true today. A different driver could install a KERNEL fence on a shared
BO that effectively says, "don't touch this until I'm done," without
triggering any rebind flows, and we'd break.

What actually exposed this issue is some local WIP where I use the
`dma_iova_*` functions to manage TT mappings. In that model, when memory
moves, a rebind does not need to be triggered because the IOVA remains
the same. What does change is the IOVA linkage, which is protected by a
KERNEL fence. The exec IOCTL did not detect that fence and subsequently
hit a CAT[33] error.

So, in my opinion, this is fixing a clear violation of the semantics of
a KERNEL fence.

Per the doc:

 79          * Drivers *always* must wait for those fences before accessing the
 80          * resource protected by the dma_resv object. The only exception for
 81          * that is when the resource is known to be locked down in place by
 82          * pinning it previously.

Matt

> 
> So if an exec runs trying to access such a bo with an active clear, for
> example, it would typically generate a pagefault?
> 
> Thomas
> 
> 
> 
> 
> 
> > 
> > Reviewed-by: Matthew Auld <matthew.auld@intel.com>
> > 
> > > ---
> > >   drivers/gpu/drm/xe/xe_exec.c | 22 ++++++++++++++++------
> > >   1 file changed, 16 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/xe/xe_exec.c
> > > b/drivers/gpu/drm/xe/xe_exec.c
> > > index e05dabfcd43c..d5293bc33a67 100644
> > > --- a/drivers/gpu/drm/xe/xe_exec.c
> > > +++ b/drivers/gpu/drm/xe/xe_exec.c
> > > @@ -292,13 +292,23 @@ int xe_exec_ioctl(struct drm_device *dev,
> > > void *data, struct drm_file *file)
> > >   		goto err_exec;
> > >   	}
> > >   
> > > -	/* Wait behind rebinds */
> > > +	/*
> > > +	 * Wait behind rebinds and any kernel operations
> > > (evictions, defrag
> > > +	 * moves, ...) on the VM and all external BOs. The VM's
> > > private BOs
> > > +	 * carry their kernel ops in the VM dma-resv KERNEL slot,
> > > while each
> > > +	 * external BO carries them in its own dma-resv KERNEL
> > > slot; both are
> > > +	 * covered by iterating every object locked by the exec,
> > > mirroring the
> > > +	 * drm_gpuvm_resv_add_fence() below.
> > > +	 */
> > >   	if (!xe_vm_in_lr_mode(vm)) {
> > > -		err = xe_sched_job_add_deps(job,
> > > -					    xe_vm_resv(vm),
> > > -					   
> > > DMA_RESV_USAGE_KERNEL);
> > > -		if (err)
> > > -			goto err_put_job;
> > > +		struct drm_gem_object *obj;
> > > +
> > > +		drm_exec_for_each_locked_object(exec, obj) {
> > > +			err = xe_sched_job_add_deps(job, obj-
> > > >resv,
> > > +						   
> > > DMA_RESV_USAGE_KERNEL);
> > > +			if (err)
> > > +				goto err_put_job;
> > > +		}
> > >   	}
> > >   
> > >   	for (i = 0; i < num_syncs && !err; i++)

  reply	other threads:[~2026-07-09 19:02 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-02 21:58 [PATCH] drm/xe: Wait on external BO kernel fences in exec IOCTL Matthew Brost
2026-07-02 22:38 ` ✓ CI.KUnit: success for " Patchwork
2026-07-02 23:19 ` ✓ Xe.CI.BAT: " Patchwork
2026-07-03  8:45 ` [PATCH] " Matthew Auld
2026-07-09 11:01   ` Thomas Hellström
2026-07-09 19:02     ` Matthew Brost [this message]
2026-07-09 21:14       ` Thomas Hellström
2026-07-03 16:44 ` ✗ Xe.CI.FULL: failure for " 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=ak/wRrF9DWBdykiP@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox