Re: [PATCH v4 5/8] drm/xe/vm: Prevent binding of purged buffer objects

["Thomas Hellström" <thomas.hellstrom@linux.intel.com>]

On Fri, 2026-01-23 at 11:11 +0530, Yadav, Arvind wrote:
> 
> On 20-01-2026 22:57, Matthew Brost wrote:
> > On Tue, Jan 20, 2026 at 11:38:51AM +0530, Arvind Yadav wrote:
> > > Add check_purged parameter to vma_lock_and_validate() to block
> > > new mapping operations on purged BOs while allowing cleanup
> > > operations to proceed.
> > > 
> > > Purged BOs have their backing pages freed by the kernel. New
> > > mapping operations (MAP, PREFETCH, REMAP) must be rejected with
> > > -EINVAL to prevent GPU access to invalid memory. Cleanup
> > > operations (UNMAP) must be allowed so applications can release
> > > resources after detecting purge via the retained field.
> > > 
> > > REMAP operations require mixed handling - reject new prev/next
> > > VMAs if the BO is purged, but allow the unmap portion to proceed
> > > for cleanup.
> > > 
> > > The check_purged parameter distinguishes between these cases:
> > > true for new mappings (must reject), false for cleanup (allow).
> > > 
> > > v2:
> > >    - Clarify that purged BOs are permanently invalid (i915
> > > semantics)
> > >    - Remove incorrect claim about madvise(WILLNEED) restoring
> > > purged BOs
> > > 
> > > v3:
> > >    - Move xe_bo_is_purged check under vma_lock_and_validate
> > > (Matthew Brost)
> > >    - Add check_purged parameter to distinguish new mappings from
> > > cleanup
> > >    - Allow UNMAP operations to prevent resource leaks
> > >    - Handle REMAP operation's dual nature (cleanup + new
> > > mappings)
> > > 
> > > Cc: Matthew Brost<matthew.brost@intel.com>
> > > 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_vm.c | 20 +++++++++++++-------
> > >   1 file changed, 13 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/xe/xe_vm.c
> > > b/drivers/gpu/drm/xe/xe_vm.c
> > > index c3a5fe76ff96..f250daae3012 100644
> > > --- a/drivers/gpu/drm/xe/xe_vm.c
> > > +++ b/drivers/gpu/drm/xe/xe_vm.c
> > > @@ -2883,7 +2883,7 @@ static void vm_bind_ioctl_ops_unwind(struct
> > > xe_vm *vm,
> > >   }
> > > 
> > >   static int vma_lock_and_validate(struct drm_exec *exec, struct
> > > xe_vma *vma,
> > > -				 bool res_evict, bool validate)
> > > +				 bool res_evict, bool validate,
> > > bool check_purged)
> > It probably time to add something like this to avoid transposing
> > arguments.
> > 
> > struct lock_and_validate_flags {
> > 	bool res_evict;
> > 	bool validate;
> > 	bool check_purged;
> > };
> > 
> > Logic in the patch looks correct though.
> 
> 
> Noted, I will add "struct xe_lock_and_validate_flags" Thanks, Arvind

Note that if you follow the pattern of struct xe_bo_shink_flags,
passing the struct as a const and using

struct lock_and_validate_flags {
	u32 res_evict : 1;
        u32 validate : 1;
        u32 check_purged : 1;
};

This will be more or less equivalent to passing a bit-field with type-
checking.

Some reviewers frown on using "bool" in compound types although we
accept that in the xe driver.

Otherwise patch LGTM as well.

/Thomas


> 
> > 
> > Matt
> > 
> > >   {
> > >   	struct xe_bo *bo = xe_vma_bo(vma);
> > >   	struct xe_vm *vm = xe_vma_vm(vma);
> > > @@ -2892,6 +2892,11 @@ static int vma_lock_and_validate(struct
> > > drm_exec *exec, struct xe_vma *vma,
> > >   	if (bo) {
> > >   		if (!bo->vm)
> > >   			err = drm_exec_lock_obj(exec, &bo-
> > > >ttm.base);
> > > +
> > > +		/* Reject new mappings to purged BOs; allow
> > > cleanup operations */
> > > +		if (!err && check_purged && xe_bo_is_purged(bo))
> > > +			err = -EINVAL;
> > > +
> > >   		if (!err && validate)
> > >   			err = xe_bo_validate(bo, vm,
> > >   					    
> > > !xe_vm_in_preempt_fence_mode(vm) &&
> > > @@ -2990,7 +2995,8 @@ static int op_lock_and_prep(struct drm_exec
> > > *exec, struct xe_vm *vm,
> > >   			err = vma_lock_and_validate(exec, op-
> > > >map.vma,
> > >   						    res_evict,
> > >   						   
> > > !xe_vm_in_fault_mode(vm) ||
> > > -						    op-
> > > >map.immediate);
> > > +						    op-
> > > >map.immediate,
> > > +						    true);
> > >   		break;
> > >   	case DRM_GPUVA_OP_REMAP:
> > >   		err = check_ufence(gpuva_to_vma(op-
> > > >base.remap.unmap->va));
> > > @@ -2999,13 +3005,13 @@ static int op_lock_and_prep(struct
> > > drm_exec *exec, struct xe_vm *vm,
> > >   
> > >   		err = vma_lock_and_validate(exec,
> > >   					    gpuva_to_vma(op-
> > > >base.remap.unmap->va),
> > > -					    res_evict, false);
> > > +					    res_evict, false,
> > > false);
> > >   		if (!err && op->remap.prev)
> > >   			err = vma_lock_and_validate(exec, op-
> > > >remap.prev,
> > > -						    res_evict,
> > > true);
> > > +						    res_evict,
> > > true, true);
> > >   		if (!err && op->remap.next)
> > >   			err = vma_lock_and_validate(exec, op-
> > > >remap.next,
> > > -						    res_evict,
> > > true);
> > > +						    res_evict,
> > > true, true);
> > >   		break;
> > >   	case DRM_GPUVA_OP_UNMAP:
> > >   		err = check_ufence(gpuva_to_vma(op-
> > > >base.unmap.va));
> > > @@ -3014,7 +3020,7 @@ static int op_lock_and_prep(struct drm_exec
> > > *exec, struct xe_vm *vm,
> > >   
> > >   		err = vma_lock_and_validate(exec,
> > >   					    gpuva_to_vma(op-
> > > >base.unmap.va),
> > > -					    res_evict, false);
> > > +					    res_evict, false,
> > > false);
> > >   		break;
> > >   	case DRM_GPUVA_OP_PREFETCH:
> > >   	{
> > > @@ -3029,7 +3035,7 @@ static int op_lock_and_prep(struct drm_exec
> > > *exec, struct xe_vm *vm,
> > >   
> > >   		err = vma_lock_and_validate(exec,
> > >   					    gpuva_to_vma(op-
> > > >base.prefetch.va),
> > > -					    res_evict, false);
> > > +					    res_evict, false,
> > > true);
> > >   		if (!err && !xe_vma_has_no_bo(vma))
> > >   			err = xe_bo_migrate(xe_vma_bo(vma),
> > >   					   
> > > region_to_mem_type[region],
> > > -- 
> > > 2.43.0