[RFC 0/4] Refresh TTM LRU on SVM prefetch

[RFC 0/4] Refresh TTM LRU on SVM prefetch

[Himal Prasad Ghimiray]

Himal Prasad Ghimiray (4):
  drm/pagemap: Add helper to access backing devmem allocation
  drm/gpusvm: Add devmem callback to get_pages
  drm/xe: Bump prefetch BO LRU via GPUSVM devmem callback
  drm/xe: Bump prefetch BO LRU for already-valid ranges

 drivers/gpu/drm/drm_gpusvm.c  | 88 ++++++++++++++++++++++++++---------
 drivers/gpu/drm/drm_pagemap.c | 15 ++++++
 drivers/gpu/drm/xe/xe_svm.c   | 53 +++++++++++++++++++++
 drivers/gpu/drm/xe/xe_svm.h   |  6 +++
 drivers/gpu/drm/xe/xe_vm.c    | 10 ++++
 include/drm/drm_gpusvm.h      |  6 +++
 include/drm/drm_pagemap.h     |  7 +++
 7 files changed, 162 insertions(+), 23 deletions(-)

-- 
2.43.0

[RFC 1/4] drm/pagemap: Add helper to access backing devmem allocation

[Himal Prasad Ghimiray]

drm_pagemap_zdd is private, so add drm_pagemap_page_to_devmem() to let
callers get the drm_pagemap_devmem backing a device-private/coherent
page. Add a NULL stub for !CONFIG_ZONE_DEVICE.

Cc: Matthew Brost <matthew.brost@intel.com>
Signed-off-by: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
---
 drivers/gpu/drm/drm_pagemap.c | 15 +++++++++++++++
 include/drm/drm_pagemap.h     |  7 +++++++
 2 files changed, 22 insertions(+)

diff --git a/drivers/gpu/drm/drm_pagemap.c b/drivers/gpu/drm/drm_pagemap.c
index 892b325fa99b..fc1a2825c807 100644
--- a/drivers/gpu/drm/drm_pagemap.c
+++ b/drivers/gpu/drm/drm_pagemap.c
@@ -1432,6 +1432,21 @@ struct drm_pagemap *drm_pagemap_page_to_dpagemap(struct page *page)
 }
 EXPORT_SYMBOL_GPL(drm_pagemap_page_to_dpagemap);
 
+/**
+ * drm_pagemap_page_to_devmem() - Return the devmem allocation backing a page
+ * @page: The struct page.
+ *
+ * Return: The &drm_pagemap_devmem backing @page. Undefined if @page was not
+ * populated from a &drm_pagemap.
+ */
+struct drm_pagemap_devmem *drm_pagemap_page_to_devmem(struct page *page)
+{
+	struct drm_pagemap_zdd *zdd = drm_pagemap_page_zone_device_data(page);
+
+	return zdd->devmem_allocation;
+}
+EXPORT_SYMBOL_GPL(drm_pagemap_page_to_devmem);
+
 /**
  * drm_pagemap_populate_mm() - Populate a virtual range with device memory pages
  * @dpagemap: Pointer to the drm_pagemap managing the device memory
diff --git a/include/drm/drm_pagemap.h b/include/drm/drm_pagemap.h
index 95eb4b66b057..376d7d400c8e 100644
--- a/include/drm/drm_pagemap.h
+++ b/include/drm/drm_pagemap.h
@@ -257,6 +257,8 @@ struct drm_pagemap *drm_pagemap_create(struct drm_device *drm,
 
 struct drm_pagemap *drm_pagemap_page_to_dpagemap(struct page *page);
 
+struct drm_pagemap_devmem *drm_pagemap_page_to_devmem(struct page *page);
+
 void drm_pagemap_put(struct drm_pagemap *dpagemap);
 
 #else
@@ -266,6 +268,11 @@ static inline struct drm_pagemap *drm_pagemap_page_to_dpagemap(struct page *page
 	return NULL;
 }
 
+static inline struct drm_pagemap_devmem *drm_pagemap_page_to_devmem(struct page *page)
+{
+	return NULL;
+}
+
 static inline void drm_pagemap_put(struct drm_pagemap *dpagemap)
 {
 }
-- 
2.43.0

[RFC 2/4] drm/gpusvm: Add devmem callback to get_pages

[Himal Prasad Ghimiray]

Add an optional drm_gpusvm_ctx.devmem_fn, invoked for each device-memory
allocation backing the range's faulted pages. It runs under the notifier
lock and independently of the DMA map, so it also fires with no_dma_map.
Adjacent pages of one allocation are coalesced, so the callback must be
idempotent.

Suggested-by: Matthew Brost <matthew.brost@intel.com>
Signed-off-by: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
---
 drivers/gpu/drm/drm_gpusvm.c | 88 ++++++++++++++++++++++++++----------
 include/drm/drm_gpusvm.h     |  6 +++
 2 files changed, 71 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
index b6c9d3a07dc8..6cec70d6f185 100644
--- a/drivers/gpu/drm/drm_gpusvm.c
+++ b/drivers/gpu/drm/drm_gpusvm.c
@@ -1528,7 +1528,47 @@ 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))
+			continue;
+
+		devmem = drm_pagemap_page_to_devmem(page);
+		if (devmem == last)
+			continue;
+
+		last = devmem;
+		ctx->devmem_fn(devmem);
+	}
+}
+
+/**
+ * drm_gpusvm_dma_map_pages() - Walk and DMA map one drm_gpusvm_pages instance
  * @gpusvm: Pointer to the GPU SVM structure
  * @svm_pages: The SVM pages instance to populate with dma-addresses
  * @pfns: The already-faulted pfn array (size @npages)
@@ -1536,10 +1576,10 @@ static bool drm_gpusvm_pages_inlinable(struct drm_gpusvm_pages *svm_pages,
  * @ctx: GPU SVM context
  * @dma_dir: DMA data direction for the mappings
  *
- * Map the faulted @pfns into @svm_pages for DMA access through its owning
- * drm_device. Must be called under the notifier lock and only for an instance
- * without a live mapping. On failure this unwinds the partial mapping of this
- * instance before returning.
+ * Walk the faulted @pfns and map them into @svm_pages for DMA access through
+ * its owning drm_device. Must be called under the notifier lock and only for
+ * an instance without a live mapping. On failure this unwinds the partial
+ * mapping of this instance before returning.
  *
  * Return: 0 on success, negative error code on failure.
  */
@@ -1774,7 +1814,7 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
 
 	hmm_range.notifier_seq = mmu_interval_read_begin(notifier);
 
-	if (map_dma &&
+	if (map_dma && !ctx->devmem_fn &&
 	    drm_gpusvm_pages_valid_unlocked(gpusvm, svm_pages, num_pages))
 		goto set_seqno;
 
@@ -1829,28 +1869,30 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
 		goto retry;
 	}
 
-	if (!map_dma)
-		goto done_mapping;
+	if (ctx->devmem_fn)
+		drm_gpusvm_walk_devmem(gpusvm, pfns, npages, ctx);
 
-	for (p = 0; p < num_pages; ++p) {
-		if (drm_gpusvm_pages_valid(gpusvm, &svm_pages[p]))
-			continue;
+	if (map_dma) {
+		for (p = 0; p < num_pages; ++p) {
+			if (drm_gpusvm_pages_valid(gpusvm, &svm_pages[p]))
+				continue;
 
-		err = drm_gpusvm_dma_map_pages(gpusvm, &svm_pages[p], pfns,
-					       npages, ctx, dma_dir);
-		if (err) {
-			/*
-			 * The failing instance was unwound by the helper. Keep
-			 * the ones mapped earlier: the -EAGAIN retry reuses
-			 * them, and the driver unmaps every instance with the
-			 * range on the other error paths.
-			 */
-			drm_gpusvm_notifier_unlock(gpusvm);
-			goto err_free;
+			err = drm_gpusvm_dma_map_pages(gpusvm, &svm_pages[p], pfns,
+						       npages, ctx, dma_dir);
+			if (err) {
+				/*
+				 * The failing instance was unwound by the
+				 * helper. Keep the ones mapped earlier: the
+				 * -EAGAIN retry reuses them, and the driver
+				 * unmaps every instance with the range on the
+				 * other error paths.
+				 */
+				drm_gpusvm_notifier_unlock(gpusvm);
+				goto err_free;
+			}
 		}
 	}
 
-done_mapping:
 	drm_gpusvm_notifier_unlock(gpusvm);
 	kvfree(pfns);
 set_seqno:
diff --git a/include/drm/drm_gpusvm.h b/include/drm/drm_gpusvm.h
index 9e35584812fd..4d31c6bba745 100644
--- a/include/drm/drm_gpusvm.h
+++ b/include/drm/drm_gpusvm.h
@@ -274,6 +274,11 @@ struct drm_gpusvm {
  *              pages as valid; the caller revalidates the snapshot itself, see
  *              drm_gpusvm_get_pages(). @devmem_only is rejected and no page
  *              type check is performed, so @allow_mixed has no effect.
+ * @devmem_fn: Optional callback invoked under the notifier lock, once per
+ *             contiguous run of pages backed by the same &drm_pagemap_devmem.
+ *             Must not sleep and must be idempotent, as a non-contiguous
+ *             allocation is reported more than once. Fires even with
+ *             @no_dma_map. May be NULL.
  *
  * Context that is DRM GPUSVM is operating in (i.e. user arguments).
  */
@@ -281,6 +286,7 @@ struct drm_gpusvm_ctx {
 	void *device_private_page_owner;
 	unsigned long check_pages_threshold;
 	unsigned long timeslice_ms;
+	void (*devmem_fn)(struct drm_pagemap_devmem *devmem);
 	unsigned int in_notifier :1;
 	unsigned int read_only :1;
 	unsigned int devmem_possible :1;
-- 
2.43.0

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

[Himal Prasad Ghimiray]

Add xe_svm_devmem_lru_bump() and set it as drm_gpusvm_ctx.devmem_fn in
the prefetch worker so each backing BO is moved to the LRU tail during
get_pages.

Suggested-by: Matthew Brost <matthew.brost@intel.com>
Signed-off-by: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
---
 drivers/gpu/drm/xe/xe_svm.c | 20 ++++++++++++++++++++
 drivers/gpu/drm/xe/xe_svm.h |  2 ++
 drivers/gpu/drm/xe/xe_vm.c  |  1 +
 3 files changed, 23 insertions(+)

diff --git a/drivers/gpu/drm/xe/xe_svm.c b/drivers/gpu/drm/xe/xe_svm.c
index 6c3033fc4db7..7de6ed818d45 100644
--- a/drivers/gpu/drm/xe/xe_svm.c
+++ b/drivers/gpu/drm/xe/xe_svm.c
@@ -9,6 +9,7 @@
 #include <drm/drm_managed.h>
 #include <drm/drm_pagemap.h>
 #include <drm/drm_pagemap_util.h>
+#include <drm/ttm/ttm_bo.h>
 
 #include "xe_bo.h"
 #include "xe_exec_queue_types.h"
@@ -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);
+
+	if (!dma_resv_trylock(bo->ttm.base.resv))
+		return;
+
+	ttm_bo_move_to_lru_tail_unlocked(&bo->ttm);
+	dma_resv_unlock(bo->ttm.base.resv);
+}
+
 /**
  * xe_svm_ranges_zap_ptes_in_range - clear ptes of svm ranges in input range
  * @vm: Pointer to the xe_vm structure
diff --git a/drivers/gpu/drm/xe/xe_svm.h b/drivers/gpu/drm/xe/xe_svm.h
index 2ef4ef026ccd..74566094b026 100644
--- a/drivers/gpu/drm/xe/xe_svm.h
+++ b/drivers/gpu/drm/xe/xe_svm.h
@@ -128,6 +128,8 @@ struct xe_svm_range *xe_svm_range_find_or_insert(struct xe_vm *vm, u64 addr,
 int xe_svm_range_get_pages(struct xe_vm *vm, struct xe_svm_range *range,
 			   struct drm_gpusvm_ctx *ctx);
 
+void xe_svm_devmem_lru_bump(struct drm_pagemap_devmem *devmem_allocation);
+
 bool xe_svm_range_needs_migrate_to_vram(struct xe_svm_range *range, struct xe_vma *vma,
 					const struct drm_pagemap *dpagemap);
 
diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
index cc96e2b7f3a7..4243825228d9 100644
--- a/drivers/gpu/drm/xe/xe_vm.c
+++ b/drivers/gpu/drm/xe/xe_vm.c
@@ -3259,6 +3259,7 @@ static int prefetch_ranges(struct xe_vm *vm, struct xe_vma_ops *vops,
 	ctx.devmem_possible = devmem_possible;
 	ctx.check_pages_threshold = devmem_possible ? SZ_64K : 0;
 	ctx.device_private_page_owner = xe_svm_private_page_owner(vm, !dpagemap);
+	ctx.devmem_fn = xe_svm_devmem_lru_bump;
 
 	skip_threads =  op->prefetch_range.ranges_count == 1 ||
 		(!dpagemap && !(vops->flags &
-- 
2.43.0

[RFC 4/4] drm/xe: Bump prefetch BO LRU for already-valid ranges

[Himal Prasad Ghimiray]

A valid prefetch range skips migration and binding, so nothing refreshes
its backing BOs on the LRU. Add xe_svm_range_prefetch_lru_bump(), which
re-faults with no_dma_map and bumps each backing BO, and call it on the
valid path.

Suggested-by: Matthew Brost <matthew.brost@intel.com>
Signed-off-by: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
---
 drivers/gpu/drm/xe/xe_svm.c | 33 +++++++++++++++++++++++++++++++++
 drivers/gpu/drm/xe/xe_svm.h |  4 ++++
 drivers/gpu/drm/xe/xe_vm.c  |  9 +++++++++
 3 files changed, 46 insertions(+)

diff --git a/drivers/gpu/drm/xe/xe_svm.c b/drivers/gpu/drm/xe/xe_svm.c
index 7de6ed818d45..8997e619874b 100644
--- a/drivers/gpu/drm/xe/xe_svm.c
+++ b/drivers/gpu/drm/xe/xe_svm.c
@@ -1632,6 +1632,39 @@ void xe_svm_devmem_lru_bump(struct drm_pagemap_devmem *devmem_allocation)
 	dma_resv_unlock(bo->ttm.base.resv);
 }
 
+/**
+ * xe_svm_range_prefetch_lru_bump() - Bump the TTM LRU for an already-valid range
+ * @vm: Pointer to the struct xe_vm
+ * @vma: Pointer to the VMA covering the range
+ * @range: Pointer to the xe SVM range structure
+ * @dpagemap: Target pagemap of the prefetch, or %NULL for system memory
+ *
+ * A valid range skips migration and binding, so nothing else refreshes its
+ * backing BOs on the LRU. Re-fault the CPU pages without touching the DMA
+ * mappings (@no_dma_map) and move each backing BO to the LRU tail.
+ *
+ * Return: 0 on success, negative error code on failure.
+ */
+int xe_svm_range_prefetch_lru_bump(struct xe_vm *vm, struct xe_vma *vma,
+				   struct xe_svm_range *range,
+				   struct drm_pagemap *dpagemap)
+{
+	struct drm_gpusvm_ctx ctx = {
+		.read_only = xe_vma_read_only(vma),
+		.device_private_page_owner =
+			xe_svm_private_page_owner(vm, !dpagemap),
+		.no_dma_map = 1,
+		.devmem_fn = xe_svm_devmem_lru_bump,
+	};
+
+	guard(mutex)(&range->lock);
+
+	if (xe_svm_range_is_removed(range))
+		return 0;
+
+	return xe_svm_range_get_pages(vm, range, &ctx);
+}
+
 /**
  * xe_svm_ranges_zap_ptes_in_range - clear ptes of svm ranges in input range
  * @vm: Pointer to the xe_vm structure
diff --git a/drivers/gpu/drm/xe/xe_svm.h b/drivers/gpu/drm/xe/xe_svm.h
index 74566094b026..53d31feae907 100644
--- a/drivers/gpu/drm/xe/xe_svm.h
+++ b/drivers/gpu/drm/xe/xe_svm.h
@@ -130,6 +130,10 @@ int xe_svm_range_get_pages(struct xe_vm *vm, struct xe_svm_range *range,
 
 void xe_svm_devmem_lru_bump(struct drm_pagemap_devmem *devmem_allocation);
 
+int xe_svm_range_prefetch_lru_bump(struct xe_vm *vm, struct xe_vma *vma,
+				   struct xe_svm_range *range,
+				   struct drm_pagemap *dpagemap);
+
 bool xe_svm_range_needs_migrate_to_vram(struct xe_svm_range *range, struct xe_vma *vma,
 					const struct drm_pagemap *dpagemap);
 
diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
index 4243825228d9..41eebdabe1c2 100644
--- 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;
+				}
+
 				need_put = true;
 				goto check_next_range;
 			}
-- 
2.43.0

✗ CI.KUnit: failure for Refresh TTM LRU on SVM prefetch

[Patchwork]

== Series Details ==

Series: Refresh TTM LRU on SVM prefetch
URL   : https://patchwork.freedesktop.org/series/174276/
State : failure

== Summary ==

+ trap cleanup EXIT
+ kunitconfigs=('/kernel/drivers/gpu/tests/.kunitconfig' '/kernel/drivers/gpu/drm/xe/.kunitconfig' '/kernel/drivers/gpu/drm/tests/.kunitconfig' '/kernel/drivers/gpu/drm/ttm/tests/.kunitconfig' '/kernel/drivers/dma-buf/.kunitconfig')
+ for kcfg in "${kunitconfigs[@]}"
+ [[ ! -f /kernel/drivers/gpu/tests/.kunitconfig ]]
+ /kernel/tools/testing/kunit/kunit.py run --kunitconfig /kernel/drivers/gpu/tests/.kunitconfig
[11:31:41] Configuring KUnit Kernel ...
Generating .config ...
Populating config with:
$ make ARCH=um O=.kunit olddefconfig
[11:31:45] Building KUnit Kernel ...
Populating config with:
$ make ARCH=um O=.kunit olddefconfig
Building with:
$ make all compile_commands.json scripts_gdb ARCH=um O=.kunit --jobs=48
[11:32:05] Starting KUnit Kernel (1/1)...
[11:32:05] ============================================================
Running tests with:
$ .kunit/linux kunit.enable=1 mem=1G console=tty kunit_shutdown=halt
[11:32:06] ============= refcount_interrupt (4 subtests) ==============
[11:32:06] [PASSED] test_single_irq_change
[11:32:06] [PASSED] test_nested_irq_change
[11:32:06] [PASSED] test_multiple_irq_change
[11:32:06] [PASSED] test_irq_save
[11:32:06] =============== [PASSED] refcount_interrupt ================
[11:32:06] ================= gpu_buddy (14 subtests) ==================
[11:32:06] [PASSED] gpu_test_buddy_alloc_limit
[11:32:06] [PASSED] gpu_test_buddy_alloc_optimistic
[11:32:06] [PASSED] gpu_test_buddy_alloc_pessimistic
[11:32:06] [PASSED] gpu_test_buddy_alloc_pathological
[11:32:06] [PASSED] gpu_test_buddy_alloc_contiguous
[11:32:06] [PASSED] gpu_test_buddy_alloc_clear
[11:32:06] [PASSED] gpu_test_buddy_alloc_range
[11:32:06] [PASSED] gpu_test_buddy_alloc_range_bias
[11:32:07] [PASSED] gpu_test_buddy_fragmentation_performance
[11:32:07] [PASSED] gpu_test_buddy_dirty_tracker_performance
[11:32:07] [PASSED] gpu_test_buddy_alloc_exceeds_max_order
[11:32:07] [PASSED] gpu_test_buddy_offset_aligned_allocation
[11:32:07] [PASSED] gpu_test_buddy_subtree_offset_alignment_stress
[11:32:07] [PASSED] gpu_test_buddy_addr_to_block
[11:32:07] ==================== [PASSED] gpu_buddy ====================
[11:32:07] ============================================================
[11:32:07] Testing complete. Ran 18 tests: passed: 18
[11:32:07] Elapsed time: 26.841s total, 4.422s configuring, 20.451s building, 1.919s running

+ for kcfg in "${kunitconfigs[@]}"
+ [[ ! -f /kernel/drivers/gpu/drm/xe/.kunitconfig ]]
+ /kernel/tools/testing/kunit/kunit.py run --kunitconfig /kernel/drivers/gpu/drm/xe/.kunitconfig
[11:32:08] Configuring KUnit Kernel ...
Regenerating .config ...
Populating config with:
$ make ARCH=um O=.kunit olddefconfig
[11:32:09] Building KUnit Kernel ...
Populating config with:
$ make ARCH=um O=.kunit olddefconfig
Building with:
$ make all compile_commands.json scripts_gdb ARCH=um O=.kunit --jobs=48
ERROR:root:../drivers/gpu/drm/xe/xe_vm.c: In function ‘vm_bind_ioctl_ops_create’:
../drivers/gpu/drm/xe/xe_vm.c:2588:47: error: implicit declaration of function ‘xe_svm_range_prefetch_lru_bump’ [-Werror=implicit-function-declaration]
 2588 |                                         err = xe_svm_range_prefetch_lru_bump(vm, vma,
      |                                               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../drivers/gpu/drm/xe/xe_vm.c: In function ‘prefetch_ranges’:
../drivers/gpu/drm/xe/xe_vm.c:3271:25: error: ‘xe_svm_devmem_lru_bump’ undeclared (first use in this function)
 3271 |         ctx.devmem_fn = xe_svm_devmem_lru_bump;
      |                         ^~~~~~~~~~~~~~~~~~~~~~
../drivers/gpu/drm/xe/xe_vm.c:3271:25: note: each undeclared identifier is reported only once for each function it appears in
cc1: some warnings being treated as errors
make[7]: *** [../scripts/Makefile.build:290: drivers/gpu/drm/xe/xe_vm.o] Error 1
make[7]: *** Waiting for unfinished jobs....
make[6]: *** [../scripts/Makefile.build:551: drivers/gpu/drm/xe] Error 2
make[5]: *** [../scripts/Makefile.build:551: drivers/gpu/drm] Error 2
make[4]: *** [../scripts/Makefile.build:551: drivers/gpu] Error 2
make[3]: *** [../scripts/Makefile.build:551: drivers] Error 2
make[2]: *** [/kernel/Makefile:2229: .] Error 2
make[1]: *** [/kernel/Makefile:248: __sub-make] Error 2
make: *** [Makefile:248: __sub-make] Error 2

+ cleanup
++ stat -c %u:%g /kernel
+ chown -R 1003:1003 /kernel

Re: [RFC 4/4] drm/xe: Bump prefetch BO LRU for already-valid ranges

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/xe: memory leak of svm_range on prefetch_lru_bump failure
--

commit 09ec19c469b99c340c07a04780a85f997af72901
Author: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>

drm/xe: Bump prefetch BO LRU for already-valid ranges

This commit introduces xe_svm_range_prefetch_lru_bump to re-fault CPU pages
without touching DMA mappings and move backing BOs to the LRU tail for
already-valid prefetch ranges.

> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> index fe9ccaa129122..948060a702470 100644
> --- 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 code leak the svm_range?

During a VM_BIND ioctl with prefetch, xe_svm_range_find_or_insert() returns
an svm_range with an incremented reference count. If 
xe_svm_range_prefetch_lru_bump() fails here, we jump to unwind_prefetch_ops.

Because this error path is taken before the svm_range is tracked in the
op->prefetch_range.range xarray, the cleanup routine
xe_svm_prefetch_gpuva_ops_fini() will not know about it and will not free it.

Should this error path call xe_svm_range_put(svm_range) before jumping to
unwind_prefetch_ops to avoid leaking the reference?

> +				}
> +
>  				need_put = true;
>  				goto check_next_range;
>  			}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260916112828.2528881-6-himal.prasad.ghimiray@intel.com?part=4

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

[sashiko-bot]

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);
> +
> +	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

Re: [RFC 2/4] drm/gpusvm: Add devmem callback to get_pages

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] drm/gpusvm: Type confusion and memory corruption on foreign device coherent pages
- [Low] drm/gpusvm: Non-contiguous device memory runs merged when separated by system pages
--

commit 26edb821373f785b22418001719e2435aed329fe
Author: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>

drm/gpusvm: Add devmem callback to get_pages

This patch adds an optional drm_gpusvm_ctx.devmem_fn callback to get_pages.
The callback is invoked for each contiguous run of pages backed by the same
device-memory allocation when processing faulted pages under the notifier
lock.

> diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
> index b6c9d3a07dc81..6cec70d6f1859 100644
> --- a/drivers/gpu/drm/drm_gpusvm.c
> +++ b/drivers/gpu/drm/drm_gpusvm.c
[ ... ]
> +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))
> +			continue;

[Severity: Low]
Does this continue statement incorrectly skip resetting the 'last' tracking
pointer for system pages?

If a virtual memory range contains a device memory run, followed by system
memory, and then the same device memory run again, 'last' will retain its
previous value during the system memory iteration. When the second run of
device memory is reached, the subsequent 'devmem == last' check will
evaluate to true, incorrectly skipping the callback for the second run and
violating the API contract that non-contiguous allocations are reported more
than once.

> +
> +		devmem = drm_pagemap_page_to_devmem(page);

[Severity: High]
Can this cause type confusion and memory corruption if the page is a foreign
device coherent page?

HMM's hmm_range_fault() does not filter device coherent pages by
dev_private_owner, so an unprivileged user mapping a device coherent page
from a foreign driver (e.g., AMDGPU) will pass the
is_device_coherent_page(page) check above.

The call to drm_pagemap_page_to_devmem(page) will then blindly cast the
foreign driver's zone_device_data to a struct drm_pagemap_zdd * and
dereference it, leading to invalid memory access.

> +		if (devmem == last)
> +			continue;
> +
> +		last = devmem;
> +		ctx->devmem_fn(devmem);
> +	}
> +}
[ ... ]

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

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

[Matthew Brost]

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.

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

Re: [RFC 1/4] drm/pagemap: Add helper to access backing devmem allocation

[Matthew Brost]

On Wed, Sep 16, 2026 at 04:58:30PM +0530, Himal Prasad Ghimiray wrote:
> drm_pagemap_zdd is private, so add drm_pagemap_page_to_devmem() to let
> callers get the drm_pagemap_devmem backing a device-private/coherent
> page. Add a NULL stub for !CONFIG_ZONE_DEVICE.
> 
> Cc: Matthew Brost <matthew.brost@intel.com>

Reviewed-by: Matthew Brost <matthew.brost@intel.com>

> Signed-off-by: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
> ---
>  drivers/gpu/drm/drm_pagemap.c | 15 +++++++++++++++
>  include/drm/drm_pagemap.h     |  7 +++++++
>  2 files changed, 22 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_pagemap.c b/drivers/gpu/drm/drm_pagemap.c
> index 892b325fa99b..fc1a2825c807 100644
> --- a/drivers/gpu/drm/drm_pagemap.c
> +++ b/drivers/gpu/drm/drm_pagemap.c
> @@ -1432,6 +1432,21 @@ struct drm_pagemap *drm_pagemap_page_to_dpagemap(struct page *page)
>  }
>  EXPORT_SYMBOL_GPL(drm_pagemap_page_to_dpagemap);
>  
> +/**
> + * drm_pagemap_page_to_devmem() - Return the devmem allocation backing a page
> + * @page: The struct page.
> + *
> + * Return: The &drm_pagemap_devmem backing @page. Undefined if @page was not
> + * populated from a &drm_pagemap.
> + */
> +struct drm_pagemap_devmem *drm_pagemap_page_to_devmem(struct page *page)
> +{
> +	struct drm_pagemap_zdd *zdd = drm_pagemap_page_zone_device_data(page);
> +
> +	return zdd->devmem_allocation;
> +}
> +EXPORT_SYMBOL_GPL(drm_pagemap_page_to_devmem);
> +
>  /**
>   * drm_pagemap_populate_mm() - Populate a virtual range with device memory pages
>   * @dpagemap: Pointer to the drm_pagemap managing the device memory
> diff --git a/include/drm/drm_pagemap.h b/include/drm/drm_pagemap.h
> index 95eb4b66b057..376d7d400c8e 100644
> --- a/include/drm/drm_pagemap.h
> +++ b/include/drm/drm_pagemap.h
> @@ -257,6 +257,8 @@ struct drm_pagemap *drm_pagemap_create(struct drm_device *drm,
>  
>  struct drm_pagemap *drm_pagemap_page_to_dpagemap(struct page *page);
>  
> +struct drm_pagemap_devmem *drm_pagemap_page_to_devmem(struct page *page);
> +
>  void drm_pagemap_put(struct drm_pagemap *dpagemap);
>  
>  #else
> @@ -266,6 +268,11 @@ static inline struct drm_pagemap *drm_pagemap_page_to_dpagemap(struct page *page
>  	return NULL;
>  }
>  
> +static inline struct drm_pagemap_devmem *drm_pagemap_page_to_devmem(struct page *page)
> +{
> +	return NULL;
> +}
> +
>  static inline void drm_pagemap_put(struct drm_pagemap *dpagemap)
>  {
>  }
> -- 
> 2.43.0
> 

Re: [RFC 2/4] drm/gpusvm: Add devmem callback to get_pages

[Matthew Brost]

On Wed, Sep 16, 2026 at 11:36:38AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] drm/gpusvm: Type confusion and memory corruption on foreign device coherent pages
> - [Low] drm/gpusvm: Non-contiguous device memory runs merged when separated by system pages
> --
> 
> commit 26edb821373f785b22418001719e2435aed329fe
> Author: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
> 
> drm/gpusvm: Add devmem callback to get_pages
> 
> This patch adds an optional drm_gpusvm_ctx.devmem_fn callback to get_pages.
> The callback is invoked for each contiguous run of pages backed by the same
> device-memory allocation when processing faulted pages under the notifier
> lock.
> 
> > diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
> > index b6c9d3a07dc81..6cec70d6f1859 100644
> > --- a/drivers/gpu/drm/drm_gpusvm.c
> > +++ b/drivers/gpu/drm/drm_gpusvm.c
> [ ... ]
> > +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))
> > +			continue;
> 
> [Severity: Low]
> Does this continue statement incorrectly skip resetting the 'last' tracking
> pointer for system pages?
> 
> If a virtual memory range contains a device memory run, followed by system
> memory, and then the same device memory run again, 'last' will retain its
> previous value during the system memory iteration. When the second run of
> device memory is reached, the subsequent 'devmem == last' check will
> evaluate to true, incorrectly skipping the callback for the second run and
> violating the API contract that non-contiguous allocations are reported more
> than once.
> 

I'm not sure what semantics we want here but per kernel doc last should
get reset to NULL on the continue. In practice likely doesn't matter
though but I guess let's adhere to the kernel doc.

> > +
> > +		devmem = drm_pagemap_page_to_devmem(page);
> 
> [Severity: High]
> Can this cause type confusion and memory corruption if the page is a foreign
> device coherent page?
> 
> HMM's hmm_range_fault() does not filter device coherent pages by
> dev_private_owner, so an unprivileged user mapping a device coherent page
> from a foreign driver (e.g., AMDGPU) will pass the
> is_device_coherent_page(page) check above.
> 
> The call to drm_pagemap_page_to_devmem(page) will then blindly cast the
> foreign driver's zone_device_data to a struct drm_pagemap_zdd * and
> dereference it, leading to invalid memory access.

This is probably right for coherent but likely out of scope for this
patch as all GPUSVM is broken here.

I tried to fix this here [1] but Thomas didn't like what I came up with.

Let's maybe throw this one on the backlog of known issues that should be
cleaned up.

Matt

[1] https://patchwork.freedesktop.org/patch/717215/?series=164587&rev=1

> 
> > +		if (devmem == last)
> > +			continue;
> > +
> > +		last = devmem;
> > +		ctx->devmem_fn(devmem);
> > +	}
> > +}
> [ ... ]
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260916112828.2528881-6-himal.prasad.ghimiray@intel.com?part=2

Re: [RFC 4/4] drm/xe: Bump prefetch BO LRU for already-valid ranges

[Matthew Brost]

On Wed, Sep 16, 2026 at 11:33:01AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] drm/xe: memory leak of svm_range on prefetch_lru_bump failure
> --
> 
> commit 09ec19c469b99c340c07a04780a85f997af72901
> Author: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
> 
> drm/xe: Bump prefetch BO LRU for already-valid ranges
> 
> This commit introduces xe_svm_range_prefetch_lru_bump to re-fault CPU pages
> without touching DMA mappings and move backing BOs to the LRU tail for
> already-valid prefetch ranges.
> 
> > diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
> > index fe9ccaa129122..948060a702470 100644
> > --- 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,

How about we just make xe_svm_range_prefetch_lru_bump non-failable? All
prefetching is fundamentally an eager optimization: we hope it succeeds,
but if it races, oh well, a fault will fix it. I think the LRU bump fits
that mindset as well.

Matt

> > +									     dpagemap);
> > +					if (err)
> > +						goto unwind_prefetch_ops;
> 
> [Severity: Medium]
> Does this code leak the svm_range?
> 
> During a VM_BIND ioctl with prefetch, xe_svm_range_find_or_insert() returns
> an svm_range with an incremented reference count. If 
> xe_svm_range_prefetch_lru_bump() fails here, we jump to unwind_prefetch_ops.
> 
> Because this error path is taken before the svm_range is tracked in the
> op->prefetch_range.range xarray, the cleanup routine
> xe_svm_prefetch_gpuva_ops_fini() will not know about it and will not free it.
> 
> Should this error path call xe_svm_range_put(svm_range) before jumping to
> unwind_prefetch_ops to avoid leaking the reference?
> 
> > +				}
> > +
> >  				need_put = true;
> >  				goto check_next_range;
> >  			}
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260916112828.2528881-6-himal.prasad.ghimiray@intel.com?part=4

Re: [RFC 2/4] drm/gpusvm: Add devmem callback to get_pages

[Ghimiray, Himal Prasad]

On 19-09-2026 02:54, Matthew Brost wrote:
> On Wed, Sep 16, 2026 at 11:36:38AM +0000, sashiko-bot@kernel.org wrote:
>> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>> - [High] drm/gpusvm: Type confusion and memory corruption on foreign device coherent pages
>> - [Low] drm/gpusvm: Non-contiguous device memory runs merged when separated by system pages
>> --
>>
>> commit 26edb821373f785b22418001719e2435aed329fe
>> Author: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
>>
>> drm/gpusvm: Add devmem callback to get_pages
>>
>> This patch adds an optional drm_gpusvm_ctx.devmem_fn callback to get_pages.
>> The callback is invoked for each contiguous run of pages backed by the same
>> device-memory allocation when processing faulted pages under the notifier
>> lock.
>>
>>> diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
>>> index b6c9d3a07dc81..6cec70d6f1859 100644
>>> --- a/drivers/gpu/drm/drm_gpusvm.c
>>> +++ b/drivers/gpu/drm/drm_gpusvm.c
>> [ ... ]
>>> +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))
>>> +			continue;
>>
>> [Severity: Low]
>> Does this continue statement incorrectly skip resetting the 'last' tracking
>> pointer for system pages?
>>
>> If a virtual memory range contains a device memory run, followed by system
>> memory, and then the same device memory run again, 'last' will retain its
>> previous value during the system memory iteration. When the second run of
>> device memory is reached, the subsequent 'devmem == last' check will
>> evaluate to true, incorrectly skipping the callback for the second run and
>> violating the API contract that non-contiguous allocations are reported more
>> than once.
>>
> 
> I'm not sure what semantics we want here but per kernel doc last should
> get reset to NULL on the continue. In practice likely doesn't matter
> though but I guess let's adhere to the kernel doc.

Sure.

> 
>>> +
>>> +		devmem = drm_pagemap_page_to_devmem(page);
>>
>> [Severity: High]
>> Can this cause type confusion and memory corruption if the page is a foreign
>> device coherent page?
>>
>> HMM's hmm_range_fault() does not filter device coherent pages by
>> dev_private_owner, so an unprivileged user mapping a device coherent page
>> from a foreign driver (e.g., AMDGPU) will pass the
>> is_device_coherent_page(page) check above.
>>
>> The call to drm_pagemap_page_to_devmem(page) will then blindly cast the
>> foreign driver's zone_device_data to a struct drm_pagemap_zdd * and
>> dereference it, leading to invalid memory access.
> 
> This is probably right for coherent but likely out of scope for this
> patch as all GPUSVM is broken here.
> 
> I tried to fix this here [1] but Thomas didn't like what I came up with.
> 
> Let's maybe throw this one on the backlog of known issues that should be
> cleaned up.
> 
> Matt
> 
> [1] https://patchwork.freedesktop.org/patch/717215/?series=164587&rev=1
> 
>>
>>> +		if (devmem == last)
>>> +			continue;
>>> +
>>> +		last = devmem;
>>> +		ctx->devmem_fn(devmem);
>>> +	}
>>> +}
>> [ ... ]
>>
>> -- 
>> Sashiko AI review · https://sashiko.dev/#/patchset/20260916112828.2528881-6-himal.prasad.ghimiray@intel.com?part=2

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

[Ghimiray, Himal Prasad]

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

Re: [RFC 4/4] drm/xe: Bump prefetch BO LRU for already-valid ranges

[Ghimiray, Himal Prasad]

On 19-09-2026 02:58, Matthew Brost wrote:
> On Wed, Sep 16, 2026 at 11:33:01AM +0000, sashiko-bot@kernel.org wrote:
>> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>> - [Medium] drm/xe: memory leak of svm_range on prefetch_lru_bump failure
>> --
>>
>> commit 09ec19c469b99c340c07a04780a85f997af72901
>> Author: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
>>
>> drm/xe: Bump prefetch BO LRU for already-valid ranges
>>
>> This commit introduces xe_svm_range_prefetch_lru_bump to re-fault CPU pages
>> without touching DMA mappings and move backing BOs to the LRU tail for
>> already-valid prefetch ranges.
>>
>>> diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
>>> index fe9ccaa129122..948060a702470 100644
>>> --- 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,
> 
> How about we just make xe_svm_range_prefetch_lru_bump non-failable? All
> prefetching is fundamentally an eager optimization: we hope it succeeds,
> but if it races, oh well, a fault will fix it. I think the LRU bump fits
> that mindset as well.

Makes sense.

> 
> Matt
> 
>>> +									     dpagemap);
>>> +					if (err)
>>> +						goto unwind_prefetch_ops;
>>
>> [Severity: Medium]
>> Does this code leak the svm_range?
>>
>> During a VM_BIND ioctl with prefetch, xe_svm_range_find_or_insert() returns
>> an svm_range with an incremented reference count. If
>> xe_svm_range_prefetch_lru_bump() fails here, we jump to unwind_prefetch_ops.
>>
>> Because this error path is taken before the svm_range is tracked in the
>> op->prefetch_range.range xarray, the cleanup routine
>> xe_svm_prefetch_gpuva_ops_fini() will not know about it and will not free it.
>>
>> Should this error path call xe_svm_range_put(svm_range) before jumping to
>> unwind_prefetch_ops to avoid leaking the reference?
>>
>>> +				}
>>> +
>>>   				need_put = true;
>>>   				goto check_next_range;
>>>   			}
>>
>> -- 
>> Sashiko AI review · https://sashiko.dev/#/patchset/20260916112828.2528881-6-himal.prasad.ghimiray@intel.com?part=4