Re: [PATCH] drm/xe/vm: Reject invalid prefetch region for non-SVM VMA

["Ghimiray, Himal Prasad" <himal.prasad.ghimiray@intel.com>]

On 14-07-2026 02:31, Matthew Brost wrote:
> On Fri, Jul 10, 2026 at 02:17:00AM +0000, Shuicheng Lin wrote:
>> DRM_XE_CONSULT_MEM_ADVISE_PREF_LOC (-1) is only valid on a
>> CPU-address-mirror (SVM) VMA. On a regular VMA the value is used as
>> an index into region_to_mem_type[], causing an out-of-bounds access:
>>
>>    UBSAN: array-index-out-of-bounds in drivers/gpu/drm/xe/xe_vm.c:3260:28
>>    index 4294967295 is out of range for type 'u32 [3]'
>>    Call Trace:
>>     __ubsan_handle_out_of_bounds+0xa7/0xf0
>>     vm_bind_ioctl_ops_execute+0x9b0/0x9d0 [xe]
>>     xe_vm_bind_ioctl+0x19f1/0x1b10 [xe]
>>
>> Three related changes:
>>
>> - vm_bind_ioctl_ops_create(): For a non-CPU-address-mirror VMA, reject
>>    both DRM_XE_CONSULT_MEM_ADVISE_PREF_LOC and out-of-range prefetch
>>    regions with -EINVAL. This is the primary fix for the OOB.
>>
>> - op_lock_and_prep(): Tighten the xe_assert() to
>>    'region < ARRAY_SIZE(region_to_mem_type)'. The
>>    DRM_XE_CONSULT_MEM_ADVISE_PREF_LOC exemption is no longer needed
>>    since the value is rejected earlier, and '<=' was an off-by-one
>>    bound (valid indices are 0..ARRAY_SIZE-1).
>>
>> - xe_drm.h: Document the CPU-address-mirror constraint on the
>>    DRM_XE_CONSULT_MEM_ADVISE_PREF_LOC UAPI value.
>>
>> Fixes: c1bb69a2e8e2 ("drm/xe/svm: Consult madvise preferred location in prefetch")
>> Assisted-by: Claude:claude-opus-4.7
>> Cc: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
> 
> I think Himal at least fixed the memory safety problem later in the
> pipeline here [1]. I'm unsure if he merged that one yet, but I'm
> inclined to say this is a better solution.
> 
> What do you think Himal?

Hi Matt,

IMO -EINVAL isn't the right behaviour here. Rejecting fails the whole 
ioctl and prefetches nothing — including the SVM VMAs in the range that 
were always valid.

AFAIU prefetching a range that covers mixed VMAs is a perfectly valid 
use case. Today:

region 0 (system) works
region 1 (devmem) works
for -1 (CONSULT_MEM_ADVISE_PREF_LOC) we should prefetch each SVM VMA to 
its preferred location, and for a BO VMA try its preferred location 
(which is effectively always local devmem); if the BO has no VRAM 
placement, fall back to PL_TT — but no ioctl failure.

If we do decide -EINVAL is the right way, then we'd effectively be 
telling UMD that range-based prefetch only works with system/devmem, and 
that a CONSULT_MEM_ADVISE_PREF_LOC prefetch range must not contain any 
BO VMAs. That's a much more awkward contract to put on UMD, since a 
range can span both VMA types, so I'm not aligned with the EINVAL approach.

Patch [1] is not merged yet, once you confirm will go ahead with it.

BR
Himal


> 
> Matt
> 
> [1] https://patchwork.freedesktop.org/series/168913/
> 
>> Cc: Matthew Brost <matthew.brost@intel.com>
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Shuicheng Lin <shuicheng.lin@intel.com>
>> ---
>>   drivers/gpu/drm/xe/xe_vm.c | 12 ++++++++++--
>>   include/uapi/drm/xe_drm.h  |  4 +++-
>>   2 files changed, 13 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
>> index 080c2fff0e95..9430b2be18e4 100644
>> --- a/drivers/gpu/drm/xe/xe_vm.c
>> +++ b/drivers/gpu/drm/xe/xe_vm.c
>> @@ -2495,6 +2495,15 @@ vm_bind_ioctl_ops_create(struct xe_vm *vm, struct xe_vma_ops *vops,
>>   			u32 i;
>>   
>>   			if (!xe_vma_is_cpu_addr_mirror(vma)) {
>> +				if (XE_IOCTL_DBG(vm->xe,
>> +						 prefetch_region ==
>> +						 DRM_XE_CONSULT_MEM_ADVISE_PREF_LOC) ||
>> +				    XE_IOCTL_DBG(vm->xe,
>> +						 prefetch_region >=
>> +						 ARRAY_SIZE(region_to_mem_type))) {
>> +					err = -EINVAL;
>> +					goto unwind_prefetch_ops;
>> +				}
>>   				op->prefetch.region = prefetch_region;
>>   				break;
>>   			}
>> @@ -3236,8 +3245,7 @@ static int op_lock_and_prep(struct drm_exec *exec, struct xe_vm *vm,
>>   
>>   		if (!xe_vma_is_cpu_addr_mirror(vma)) {
>>   			region = op->prefetch.region;
>> -			xe_assert(vm->xe, region == DRM_XE_CONSULT_MEM_ADVISE_PREF_LOC ||
>> -				  region <= ARRAY_SIZE(region_to_mem_type));
>> +			xe_assert(vm->xe, region < ARRAY_SIZE(region_to_mem_type));
>>   		}
>>   
>>   		/*
>> diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h
>> index 509202a7b13e..e159c44e380a 100644
>> --- a/include/uapi/drm/xe_drm.h
>> +++ b/include/uapi/drm/xe_drm.h
>> @@ -1075,7 +1075,9 @@ struct drm_xe_vm_destroy {
>>    *
>>    * The @prefetch_mem_region_instance for %DRM_XE_VM_BIND_OP_PREFETCH can also be:
>>    *  - %DRM_XE_CONSULT_MEM_ADVISE_PREF_LOC, which ensures prefetching occurs in
>> - *    the memory region advised by madvise.
>> + *    the memory region advised by madvise. Only valid when the target VMA
>> + *    was created with %DRM_XE_VM_BIND_FLAG_CPU_ADDR_MIRROR; rejected with
>> + *    -EINVAL otherwise.
>>    */
>>   struct drm_xe_vm_bind_op {
>>   	/** @extensions: Pointer to the first extension struct, if any */
>> -- 
>> 2.43.0
>>