Re: [RFC v2 5/9] drm/xe/bo: Handle CPU faults on purged buffer objects

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

On Wed, Dec 03, 2025 at 12:55:52PM +0530, Yadav, Arvind wrote:
> 
> On 03-12-2025 00:18, Matthew Brost wrote:
> > On Tue, Dec 02, 2025 at 10:42:39AM -0800, Matthew Brost wrote:
> > > On Mon, Dec 01, 2025 at 11:20:15AM +0530, Arvind Yadav wrote:
> > > > Modify the CPU page fault handler, `xe_bo_cpu_fault()`, to correctly
> > > > handle access to buffer objects that have been purged.
> > > > 
> > > > When a buffer object is in the `XE_MADV_PURGED` state, its backing
> > > > store has been reclaimed by the kernel. If the CPU attempts to access
> > > > this memory, it is an error that should be reported to the application.
> > > > 
> > > > v2:
> > > >    - Added xe_bo_is_purged(bo) instead of atomic_read.
> > > >    - Avoids leaks and keeps drm_dev_exit() while returning.
> > > > 
> > > > Cc: Matthew Brost <matthew.brost@intel.com>
> > > Reviewed-by: Matthew Brost <matthew.brost@intel.com>
> > > 
> > Ah, actually I think I made a mistake here.
> > 
> > > > Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> > > > Cc: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
> > > > Signed-off-by: Arvind Yadav <arvind.yadav@intel.com>
> > > > ---
> > > >   drivers/gpu/drm/xe/xe_bo.c | 10 ++++++++++
> > > >   1 file changed, 10 insertions(+)
> > > > 
> > > > diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
> > > > index f0b3f7a13114..7f5bcf114ed4 100644
> > > > --- a/drivers/gpu/drm/xe/xe_bo.c
> > > > +++ b/drivers/gpu/drm/xe/xe_bo.c
> > > > @@ -1992,6 +1992,16 @@ static vm_fault_t xe_bo_cpu_fault(struct vm_fault *vmf)
> > > >   	if (!drm_dev_enter(&xe->drm, &idx))
> > > >   		return ttm_bo_vm_dummy_page(vmf, vmf->vma->vm_page_prot);
> > > > +	/*
> > > > +	 * BO content is gone. Signal the user process.
> > > > +	 * Once purged, BO remains permanently invalid (i915 semantics).
> > > > +	 * Application must destroy and recreate the BO.
> > > > +	 */
> > > > +	if (xe_bo_is_purged(bo)) {
> > Doesn't this need to done under the BO's dma-resv lock to avoid a race?
> > Consider the case where after this check, TTM evicts this BO changing
> > the state purged. Now we grab the BO's dma-resv lock and try to get
> > pages on purged BO. Seems like an issue.
> 
> Thanks for catching these issues!.
> 
> > 
> > Also with that, xe_bo_is_purged likely should have lockdep annotation
> > asserting the BOs dma-resv lock is held.
> I initially added xe_bo_assert_held() to xe_bo_is_purged(), but it causes
> crashes because many callers don't hold the lock.
> For example, in xe_pagefault.c (this early check), no lock is held. I’ll
> recheck the call and update accordingly.
> 

I've touched on this in other patches - sorry my reviews sometimes come
in scattered bursts as I look at code - but I think the point is all
purging state changes / critical checks should be done under the BO
dma-resv lock to avoid races. Sure user space shouldn't touching a BO in
WONTNEED state but it could and if purging races with a check outside a
lock it seems like bad things could happen in the kernel.

Matt

> ~Arvind
> > 
> > Matt
> > 
> > > > +		ret = VM_FAULT_SIGBUS;
> > > > +		goto out;
> > > > +	}
> > > > +
> > > >   	ret = xe_bo_cpu_fault_fastpath(vmf, xe, bo, needs_rpm);
> > > >   	if (ret != VM_FAULT_RETRY)
> > > >   		goto out;
> > > > -- 
> > > > 2.43.0
> > > >