Re: [PATCH] [CI-ONLY][DONOT-REVIEW] Squash of Refresh TTM LRU on SVM prefetch

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] drm/gpusvm: type confusion accessing devmem_allocation of foreign DEVICE_COHERENT pages
- [Medium] drm/xe: memory leak of svm_range on prefetch_lru_bump error path
--

commit 5f7e91f97dcbc6067f561bc870cb129bb5353cab
Author: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>

Squash of Refresh TTM LRU on SVM prefetch

This commit adds SVM prefetch LRU bump support for device memory in DRM
GPUSVM, integrating it with the xe driver. It introduces
drm_gpusvm_walk_devmem() to iterate over faulted device pages and invoke a
callback to refresh their TTM LRU.

> diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
> --- a/drivers/gpu/drm/drm_gpusvm.c
> +++ b/drivers/gpu/drm/drm_gpusvm.c
> @@ -1528,7 +1528,49 @@ static bool drm_gpusvm_pages_inlinable(struct drm_gpusvm_pages *svm_pages,
>  }
>  
>  /**
> - * drm_gpusvm_dma_map_pages() - DMA map one drm_gpusvm_pages instance
> + * drm_gpusvm_walk_devmem() - Invoke the devmem callback across faulted pages
> + * @gpusvm: Pointer to the GPU SVM structure
> + * @pfns: The already-faulted pfn array (size @npages)
> + * @npages: Number of pages in the CPU range
> + * @ctx: GPU SVM context, with a non-NULL &drm_gpusvm_ctx.devmem_fn
> + *
> + * Invoke &drm_gpusvm_ctx.devmem_fn once per contiguous run of @pfns backed by
> + * the same device-memory allocation. Must be called under the notifier lock.
> + */
> +static void drm_gpusvm_walk_devmem(struct drm_gpusvm *gpusvm,
> +				   unsigned long *pfns,
> +				   unsigned long npages,
> +				   const struct drm_gpusvm_ctx *ctx)
> +{
> +	struct drm_pagemap_devmem *last = NULL;
> +	unsigned int order = 0;
> +	unsigned long i;
> +
> +	lockdep_assert_held(&gpusvm->notifier_lock);
> +
> +	for (i = 0; i < npages; i += 1 << order) {
> +		struct page *page = hmm_pfn_to_page(pfns[i]);
> +		struct drm_pagemap_devmem *devmem;
> +
> +		order = drm_gpusvm_hmm_pfn_to_order(pfns[i], i, npages);
> +
> +		if (!is_device_private_page(page) &&
> +		    !is_device_coherent_page(page)) {
> +			last = NULL;
> +			continue;
> +		}
> +
> +		devmem = drm_pagemap_page_to_devmem(page);
> +		if (devmem == last)
> +			continue;
> +
> +		last = devmem;
> +		ctx->devmem_fn(devmem);

[Severity: High]
Could this lead to type confusion for foreign DEVICE_COHERENT pages?

If userspace maps memory from another driver (e.g., AMDGPU) that uses
MEMORY_DEVICE_COHERENT and passes it to the xe driver, hmm_range_fault()
will return the PFNs. Because is_device_coherent_page() evaluates to true
here, this code unconditionally calls drm_pagemap_page_to_devmem().

Looking at drm_pagemap_page_to_devmem(), it blindly treats the foreign
driver's page->zone_device_data as a drm_pagemap_zdd struct. That garbage
pointer is then passed to the devmem_fn callback (xe_svm_devmem_lru_bump()),
which casts it to an xe_bo and accesses bo->ttm.base.resv. This could cause
an invalid pointer dereference or memory corruption.

[ ... ]
> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> --- a/drivers/gpu/drm/xe/xe_vm.c
> +++ b/drivers/gpu/drm/xe/xe_vm.c
> @@ -2583,6 +2583,15 @@ vm_bind_ioctl_ops_create(struct xe_vm *vm, struct xe_vma_ops *vops,
>  						  dpagemap, &valid_pages)) {
>  				xe_svm_range_debug(svm_range, "PREFETCH - RANGE IS VALID");
>  				xe_assert(vm->xe, valid_pages);
> +
> +				if (dpagemap) {
> +					err = xe_svm_range_prefetch_lru_bump(vm, vma,
> +									     svm_range,
> +									     dpagemap);
> +					if (err)
> +						goto unwind_prefetch_ops;

[Severity: Medium]
Does this leak the svm_range reference on the error path?

xe_svm_range_find_or_insert() returns an svm_range with an elevated
reference count. If xe_svm_range_validate() returns true, but
xe_svm_range_prefetch_lru_bump() fails (e.g., due to an hmm_range_fault
error), the code jumps to the unwind_prefetch_ops label.

At this point, the svm_range has not been added to op->prefetch_range.range
via xa_alloc(). Consequently, the cleanup function
xe_svm_prefetch_gpuva_ops_fini() will not find it, and the reference
will be permanently leaked.

> +				}
> +
>  				need_put = true;
>  				goto check_next_range;
>  			}
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260916125822.2605659-2-himal.prasad.ghimiray@intel.com?part=1