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

Yadav, Arvind
2026-03-20T02:34:50Z

On 05-03-2026 21:08, Thomas Hellström wrote:
> 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.

Noted,

Thanks,
Arvind

>
>
>
>>  {
>>   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],