Re: [PATCH v6 05/12] drm/xe/vm: Prevent binding of purged buffer objects

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

On Tue, 2026-03-03 at 20:50 +0530, Arvind Yadav wrote:
> Add purge checking 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 flag in struct xe_vma_lock_and_validate_flags
> 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 (Matt)
>   - 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)
> 
> v5:
>   - Replace three boolean parameters with struct
> xe_vma_lock_and_validate_flags
>     to improve readability and prevent argument transposition (Matt)
>   - Use u32 bitfields instead of bool members to match
> xe_bo_shrink_flags
>     pattern - more efficient packing and follows xe driver
> conventions (Thomas)
>   - Pass struct as const since flags are read-only (Thomas)
> 
> v6:
>   - Block VM_BIND to DONTNEED BOs with -EBUSY (Thomas, Matt)
> 
> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> Cc: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
> Cc: Matthew Brost <matthew.brost@intel.com>
> Signed-off-by: Arvind Yadav <arvind.yadav@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_vm.c | 71 ++++++++++++++++++++++++++++++++----
> --
>  1 file changed, 60 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> index c65d014c7491..4a8abdcfb912 100644
> --- a/drivers/gpu/drm/xe/xe_vm.c
> +++ b/drivers/gpu/drm/xe/xe_vm.c
> @@ -2917,8 +2917,20 @@ static void vm_bind_ioctl_ops_unwind(struct
> xe_vm *vm,
>  	}
>  }
>  
> +/**
> + * struct xe_vma_lock_and_validate_flags - Flags for
> vma_lock_and_validate()
> + * @res_evict: Allow evicting resources during validation
> + * @validate: Perform BO validation
> + * @check_purged: Reject operation if BO is purged
> + */
> +struct xe_vma_lock_and_validate_flags {
> +	u32 res_evict : 1;
> +	u32 validate : 1;
> +	u32 check_purged : 1;
> +};
> +
>  static int vma_lock_and_validate(struct drm_exec *exec, struct
> xe_vma *vma,
> -				 bool res_evict, bool validate)
> +				 const struct
> xe_vma_lock_and_validate_flags *flags)

Small structs like these can be passed by value rather than by
reference. It's done elsewhere in the xe driver, but we've also seen
people complain about ABI incompatibilities. I'd recommend passing by
value here to make the driver style consistent.

Other than that LGTM.



>  {
>  	struct xe_bo *bo = xe_vma_bo(vma);
>  	struct xe_vm *vm = xe_vma_vm(vma);
> @@ -2927,10 +2939,19 @@ 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);
> -		if (!err && validate)
> +
> +		/* Reject new mappings to DONTNEED/purged BOs; allow
> cleanup operations */
> +		if (!err && flags->check_purged) {
> +			if (xe_bo_madv_is_dontneed(bo))
> +				err = -EBUSY;  /* BO marked
> purgeable */
> +			else if (xe_bo_is_purged(bo))
> +				err = -EINVAL; /* BO already purged
> */
> +		}
> +
> +		if (!err && flags->validate)
>  			err = xe_bo_validate(bo, vm,
>  					    
> xe_vm_allow_vm_eviction(vm) &&
> -					     res_evict, exec);
> +					     flags->res_evict,
> exec);
>  	}
>  
>  	return err;
> @@ -3023,9 +3044,12 @@ static int op_lock_and_prep(struct drm_exec
> *exec, struct xe_vm *vm,
>  	case DRM_GPUVA_OP_MAP:
>  		if (!op->map.invalidate_on_bind)
>  			err = vma_lock_and_validate(exec, op-
> >map.vma,
> -						    res_evict,
> -						   
> !xe_vm_in_fault_mode(vm) ||
> -						    op-
> >map.immediate);
> +						    &(struct
> xe_vma_lock_and_validate_flags) {
> +							   
> .res_evict = res_evict,
> +							   
> .validate = !xe_vm_in_fault_mode(vm) ||
> +								
> 	op->map.immediate,
> +							   
> .check_purged = true
> +						    });
>  		break;
>  	case DRM_GPUVA_OP_REMAP:
>  		err = check_ufence(gpuva_to_vma(op-
> >base.remap.unmap->va));
> @@ -3034,13 +3058,25 @@ 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);
> +					    &(struct
> xe_vma_lock_and_validate_flags) {
> +						    .res_evict =
> res_evict,
> +						    .validate =
> false,
> +						    .check_purged =
> false
> +					    });
>  		if (!err && op->remap.prev)
>  			err = vma_lock_and_validate(exec, op-
> >remap.prev,
> -						    res_evict,
> true);
> +						    &(struct
> xe_vma_lock_and_validate_flags) {
> +							   
> .res_evict = res_evict,
> +							   
> .validate = true,
> +							   
> .check_purged = true
> +						    });
>  		if (!err && op->remap.next)
>  			err = vma_lock_and_validate(exec, op-
> >remap.next,
> -						    res_evict,
> true);
> +						    &(struct
> xe_vma_lock_and_validate_flags) {
> +							   
> .res_evict = res_evict,
> +							   
> .validate = true,
> +							   
> .check_purged = true
> +						    });
>  		break;
>  	case DRM_GPUVA_OP_UNMAP:
>  		err = check_ufence(gpuva_to_vma(op->base.unmap.va));
> @@ -3049,7 +3085,11 @@ 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);
> +					    &(struct
> xe_vma_lock_and_validate_flags) {
> +						    .res_evict =
> res_evict,
> +						    .validate =
> false,
> +						    .check_purged =
> false
> +					    });
>  		break;
>  	case DRM_GPUVA_OP_PREFETCH:
>  	{
> @@ -3062,9 +3102,18 @@ static int op_lock_and_prep(struct drm_exec
> *exec, struct xe_vm *vm,
>  				  region <=
> ARRAY_SIZE(region_to_mem_type));
>  		}
>  
> +		/*
> +		 * Prefetch attempts to migrate BO's backing store
> without
> +		 * repopulating it first. Purged BOs have no backing
> store
> +		 * to migrate, so reject the operation.
> +		 */
>  		err = vma_lock_and_validate(exec,
>  					    gpuva_to_vma(op-
> >base.prefetch.va),
> -					    res_evict, false);
> +					    &(struct
> xe_vma_lock_and_validate_flags) {
> +						    .res_evict =
> res_evict,
> +						    .validate =
> false,
> +						    .check_purged =
> true
> +					    });
>  		if (!err && !xe_vma_has_no_bo(vma))
>  			err = xe_bo_migrate(xe_vma_bo(vma),
>  					   
> region_to_mem_type[region],