Re: [RFC 3/4] drm/xe: Bump prefetch BO LRU via GPUSVM devmem callback

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

On 19-09-2026 02:32, Matthew Brost wrote:
> On Wed, Sep 16, 2026 at 11:36:32AM +0000, sashiko-bot@kernel.org wrote:
>> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>> - [High] drm/xe: type confusion in xe_svm_devmem_lru_bump() due to unverified cast
>> --
>>
>> commit 09484da3688fb659e04396330f11c6ca63c00b90
>> Author: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
>>
>> drm/xe: Bump prefetch BO LRU via GPUSVM devmem callback
>>
>> This commit adds xe_svm_devmem_lru_bump() and sets it as the
>> drm_gpusvm_ctx.devmem_fn callback in the prefetch worker to move backing
>> BOs to the LRU tail during get_pages.
>>
>>> diff --git a/drivers/gpu/drm/xe/xe_svm.c b/drivers/gpu/drm/xe/xe_svm.c
>>> index 6c3033fc4db73..7de6ed818d45e 100644
>>> --- a/drivers/gpu/drm/xe/xe_svm.c
>>> +++ b/drivers/gpu/drm/xe/xe_svm.c
>>
>> [ ... ]
>>
>>> @@ -1612,6 +1613,25 @@ int xe_svm_range_get_pages(struct xe_vm *vm, struct xe_svm_range *range,
>>>   	return err;
>>>   }
>>>   
>>> +/**
>>> + * xe_svm_devmem_lru_bump() - Move a range's backing BO to the TTM LRU tail
>>> + * @devmem_allocation: The device-memory allocation backing the range's pages
>>> + *
>>> + * Intended as a &drm_gpusvm_ctx.devmem_fn. Runs under the GPUSVM notifier lock;
>>> + * the dma-resv trylock avoids inverting against eviction/shrinker, which take
>>> + * dma-resv before the notifier lock. A contended BO is simply skipped.
>>> + */
>>> +void xe_svm_devmem_lru_bump(struct drm_pagemap_devmem *devmem_allocation)
>>> +{
>>> +	struct xe_bo *bo = to_xe_bo(devmem_allocation);
> 
> Based on Sashiko feedback - we probably want a check here...
> 
> if (devmem_allocation->ops != &dpagemap_devmem_ops)
> 	return;
> 
> This would future-proof us if we ever start using SVM with device memory
> sharing on non-Xe devices while still sharing the same
> device_private_page_owner. At that point, we would likely need to
> rethink the LRU bumping, perhaps via a dpagemap_devmem_ops vfunc, but we
> would not immediately explode.

Agreed.

> 
> Matt
> 
>>> +
>>> +	if (!dma_resv_trylock(bo->ttm.base.resv))
>>
>> [Severity: High]
>> Does this cast safely handle generic device memory allocations?
>>
>> If userspace maps device memory from another DRM driver into the SVM address
>> space, hmm_range_fault() can return those foreign device pages. The drm_gpusvm
>> core will then loop over them in drm_gpusvm_walk_devmem() and invoke this
>> callback with the foreign driver's devmem_allocation.
>>
>> Since the generic struct drm_pagemap_devmem * is cast to a driver-specific
>> struct xe_bo * without verifying that the allocation actually belongs to the
>> xe driver, can this lead to a type confusion and invalid pointer dereference
>> on bo->ttm.base.resv?
>>
>> Unprivileged users mapping memory from multiple devices in the same process
>> could trigger a GPU fault or prefetch, making this a potential denial of
>> service vector.
>>
>>> +		return;
>>> +
>>> +	ttm_bo_move_to_lru_tail_unlocked(&bo->ttm);
>>> +	dma_resv_unlock(bo->ttm.base.resv);
>>> +}
>>> +
>>
>> -- 
>> Sashiko AI review · https://sashiko.dev/#/patchset/20260916112828.2528881-6-himal.prasad.ghimiray@intel.com?part=3