[PATCH 0/4] drm/gpuvm: Add support for single-page-filled mappings

[PATCH 0/4] drm/gpuvm: Add support for single-page-filled mappings

[Asahi Lina]

Some hardware requires dummy page mappings to efficiently implement
Vulkan sparse features. These mappings consist of the same physical
memory page, repeated for a large range of address space (e.g. 16GiB).

Add support for this to drm_gpuvm. Currently, drm_gpuvm expects BO
ranges to correspond 1:1 to virtual memory ranges that are mapped, and
does math on the BO offset accordingly. To make single page mappings
work, we need a way to turn off that math, keeping the BO offset always
constant and pointing to the same page (typically BO offset 0).

To make this work, we need to handle all the corner cases when these
mappings intersect with regular mappings. The rules are simply to never
mix or merge a "regular" mapping with a single page mapping.

drm_gpuvm has support for a flags field in drm_gpuva objects. This is
normally managed by drivers directly. We can introduce a
DRM_GPUVA_SINGLE_PAGE flag to handle this. However, to make it work,
sm_map and friends need to know ahead of time whether the new mapping is
a single page mapping or not. Therefore, we need to add an argument to
these functions so drivers can provide the flags to be filled into
drm_gpuva.flags.

These changes should not affect any existing drivers that use drm_gpuvm
other than the API change:

- imagination: Does not use flags at all
- nouveau: Only uses drm_gpuva_invalidate(), which is only called on
  existing drm_gpuva objects (after the map steps)
- panthor: Does not use flags at all
- xe: Does not use drm_gpuva_init_from_op() or
  drm_gpuva_remap()/drm_gpuva_map() (which call it). This means that the
flags field of the gpuva object is managed by the driver only, so these
changes cannot clobber it.

Note that the way this is implemented, drm_gpuvm does not need to know
the GPU page size. It only has to never do math on the BO offset to meet
the requirements.

I suspect that after this change there could be some cleanup possible in
the xe driver (which right now passes flags around in various
driver-specific ways from the map step through to drm_gpuva objects),
but I'll leave that to the Xe folks.

Signed-off-by: Asahi Lina <lina@asahilina.net>
---
Asahi Lina (4):
      drm/gpuvm: Add a flags argument to drm_gpuvm_sm_map[_*]
      drm/gpuvm: Plumb through flags into drm_gpuva_op_map
      drm/gpuvm: Add DRM_GPUVA_SINGLE_PAGE flag and logic
      drm/gpuvm: Plumb through flags into drm_gpuva_init

 drivers/gpu/drm/drm_gpuvm.c            | 72 ++++++++++++++++++++++++++--------
 drivers/gpu/drm/imagination/pvr_vm.c   |  3 +-
 drivers/gpu/drm/nouveau/nouveau_uvmm.c |  3 +-
 drivers/gpu/drm/panthor/panthor_mmu.c  |  3 +-
 drivers/gpu/drm/xe/xe_vm.c             |  3 +-
 include/drm/drm_gpuvm.h                | 26 +++++++++---
 6 files changed, 84 insertions(+), 26 deletions(-)
---
base-commit: ffd294d346d185b70e28b1a28abe367bbfe53c04
change-id: 20250202-gpuvm-single-page-253346a74677

Cheers,
~~ Lina

[PATCH 1/4] drm/gpuvm: Add a flags argument to drm_gpuvm_sm_map[_*]

[Asahi Lina]

drm_gpuva objects have a flags field. Currently, this can be managed by
drivers out-of-band, without any special handling in drm_gpuvm.

To be able to introduce flags that do affect the logic in the drm_gpuvm
core, we need to plumb it through the map calls. This will allow the
core to check the flags on map and alter the merge/split logic depending
on the requested flags and the flags of the existing drm_gpuva ranges
that are being split.

First, just add the argument to the API and do nothing with it.

Signed-off-by: Asahi Lina <lina@asahilina.net>
---
 drivers/gpu/drm/drm_gpuvm.c            | 6 ++++--
 drivers/gpu/drm/imagination/pvr_vm.c   | 3 ++-
 drivers/gpu/drm/nouveau/nouveau_uvmm.c | 3 ++-
 drivers/gpu/drm/panthor/panthor_mmu.c  | 3 ++-
 drivers/gpu/drm/xe/xe_vm.c             | 3 ++-
 include/drm/drm_gpuvm.h                | 6 ++++--
 6 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_gpuvm.c b/drivers/gpu/drm/drm_gpuvm.c
index f9eb56f24bef291e084a15d844d4ececda8412d9..9733460d737667035541b488657154afb56872e3 100644
--- a/drivers/gpu/drm/drm_gpuvm.c
+++ b/drivers/gpu/drm/drm_gpuvm.c
@@ -2333,7 +2333,8 @@ __drm_gpuvm_sm_unmap(struct drm_gpuvm *gpuvm,
 int
 drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm, void *priv,
 		 u64 req_addr, u64 req_range,
-		 struct drm_gem_object *req_obj, u64 req_offset)
+		 struct drm_gem_object *req_obj, u64 req_offset,
+		 enum drm_gpuva_flags req_flags)
 {
 	const struct drm_gpuvm_ops *ops = gpuvm->ops;
 
@@ -2516,7 +2517,8 @@ static const struct drm_gpuvm_ops gpuvm_list_ops = {
 struct drm_gpuva_ops *
 drm_gpuvm_sm_map_ops_create(struct drm_gpuvm *gpuvm,
 			    u64 req_addr, u64 req_range,
-			    struct drm_gem_object *req_obj, u64 req_offset)
+			    struct drm_gem_object *req_obj, u64 req_offset,
+			    enum drm_gpuva_flags req_flags)
 {
 	struct drm_gpuva_ops *ops;
 	struct {
diff --git a/drivers/gpu/drm/imagination/pvr_vm.c b/drivers/gpu/drm/imagination/pvr_vm.c
index 363f885a709826efa3d45a906c5f65131f7ed7b9..c57c7adcbe987dfc559bc00fc24862f5d99bbc0e 100644
--- a/drivers/gpu/drm/imagination/pvr_vm.c
+++ b/drivers/gpu/drm/imagination/pvr_vm.c
@@ -190,7 +190,8 @@ static int pvr_vm_bind_op_exec(struct pvr_vm_bind_op *bind_op)
 					bind_op, bind_op->device_addr,
 					bind_op->size,
 					gem_from_pvr_gem(bind_op->pvr_obj),
-					bind_op->offset);
+					bind_op->offset,
+					0);
 
 	case PVR_VM_BIND_TYPE_UNMAP:
 		return drm_gpuvm_sm_unmap(&bind_op->vm_ctx->gpuvm_mgr,
diff --git a/drivers/gpu/drm/nouveau/nouveau_uvmm.c b/drivers/gpu/drm/nouveau/nouveau_uvmm.c
index 48f105239f42d8ffa3cefd253bd83d52dbb3255f..d548154c0a38c08c74cfbb9c66d703699d8d882b 100644
--- a/drivers/gpu/drm/nouveau/nouveau_uvmm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_uvmm.c
@@ -1304,7 +1304,8 @@ nouveau_uvmm_bind_job_submit(struct nouveau_job *job,
 							      op->va.addr,
 							      op->va.range,
 							      op->gem.obj,
-							      op->gem.offset);
+							      op->gem.offset,
+							      0);
 			if (IS_ERR(op->ops)) {
 				ret = PTR_ERR(op->ops);
 				goto unwind_continue;
diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c
index a49132f3778b3a7a0855de37e4ab008e9476f740..ebfb399af8752afdd26c33723b8ac6f616d02fc5 100644
--- a/drivers/gpu/drm/panthor/panthor_mmu.c
+++ b/drivers/gpu/drm/panthor/panthor_mmu.c
@@ -2155,7 +2155,8 @@ panthor_vm_exec_op(struct panthor_vm *vm, struct panthor_vm_op_ctx *op,
 		}
 
 		ret = drm_gpuvm_sm_map(&vm->base, vm, op->va.addr, op->va.range,
-				       op->map.vm_bo->obj, op->map.bo_offset);
+				       op->map.vm_bo->obj, op->map.bo_offset,
+				       0);
 		break;
 
 	case DRM_PANTHOR_VM_BIND_OP_TYPE_UNMAP:
diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c
index c99380271de62f8659fdd909a5bd9980d09de4d9..2d1fde089c382c363e354200cfb0281869b97f56 100644
--- a/drivers/gpu/drm/xe/xe_vm.c
+++ b/drivers/gpu/drm/xe/xe_vm.c
@@ -1926,7 +1926,8 @@ vm_bind_ioctl_ops_create(struct xe_vm *vm, struct xe_bo *bo,
 	case DRM_XE_VM_BIND_OP_MAP:
 	case DRM_XE_VM_BIND_OP_MAP_USERPTR:
 		ops = drm_gpuvm_sm_map_ops_create(&vm->gpuvm, addr, range,
-						  obj, bo_offset_or_userptr);
+						  obj, bo_offset_or_userptr,
+						  0);
 		break;
 	case DRM_XE_VM_BIND_OP_UNMAP:
 		ops = drm_gpuvm_sm_unmap_ops_create(&vm->gpuvm, addr, range);
diff --git a/include/drm/drm_gpuvm.h b/include/drm/drm_gpuvm.h
index 00d4e43b76b6c12e10e422d250ab0ac1c9009bc5..cb045378803646a9645c002c57183c915d35befb 100644
--- a/include/drm/drm_gpuvm.h
+++ b/include/drm/drm_gpuvm.h
@@ -1056,7 +1056,8 @@ struct drm_gpuva_ops {
 struct drm_gpuva_ops *
 drm_gpuvm_sm_map_ops_create(struct drm_gpuvm *gpuvm,
 			    u64 addr, u64 range,
-			    struct drm_gem_object *obj, u64 offset);
+			    struct drm_gem_object *obj, u64 offset,
+			    enum drm_gpuva_flags flags);
 struct drm_gpuva_ops *
 drm_gpuvm_sm_unmap_ops_create(struct drm_gpuvm *gpuvm,
 			      u64 addr, u64 range);
@@ -1201,7 +1202,8 @@ struct drm_gpuvm_ops {
 
 int drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm, void *priv,
 		     u64 addr, u64 range,
-		     struct drm_gem_object *obj, u64 offset);
+		     struct drm_gem_object *obj, u64 offset,
+		     enum drm_gpuva_flags flags);
 
 int drm_gpuvm_sm_unmap(struct drm_gpuvm *gpuvm, void *priv,
 		       u64 addr, u64 range);

-- 
2.47.1

[PATCH 2/4] drm/gpuvm: Plumb through flags into drm_gpuva_op_map

[Asahi Lina]

Now that the map API functions take a flags argument, plumb it through
into the drm_gpuva_op_map structure so that drivers can retrieve the
value that was passed. Similarly, for remap calls, take the flags from
the existing drm_gpuva.

Signed-off-by: Asahi Lina <lina@asahilina.net>
---
 drivers/gpu/drm/drm_gpuvm.c | 22 +++++++++++++++++-----
 include/drm/drm_gpuvm.h     |  5 +++++
 2 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_gpuvm.c b/drivers/gpu/drm/drm_gpuvm.c
index 9733460d737667035541b488657154afb56872e3..7443be1fe4de4653ec40ca4b874df30297af7faf 100644
--- a/drivers/gpu/drm/drm_gpuvm.c
+++ b/drivers/gpu/drm/drm_gpuvm.c
@@ -2054,7 +2054,8 @@ EXPORT_SYMBOL_GPL(drm_gpuva_unmap);
 static int
 op_map_cb(const struct drm_gpuvm_ops *fn, void *priv,
 	  u64 addr, u64 range,
-	  struct drm_gem_object *obj, u64 offset)
+	  struct drm_gem_object *obj, u64 offset,
+	  enum drm_gpuva_flags flags)
 {
 	struct drm_gpuva_op op = {};
 
@@ -2063,6 +2064,7 @@ op_map_cb(const struct drm_gpuvm_ops *fn, void *priv,
 	op.map.va.range = range;
 	op.map.gem.obj = obj;
 	op.map.gem.offset = offset;
+	op.map.flags = flags;
 
 	return fn->sm_step_map(&op, priv);
 }
@@ -2102,7 +2104,8 @@ static int
 __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm,
 		   const struct drm_gpuvm_ops *ops, void *priv,
 		   u64 req_addr, u64 req_range,
-		   struct drm_gem_object *req_obj, u64 req_offset)
+		   struct drm_gem_object *req_obj, u64 req_offset,
+		   enum drm_gpuva_flags req_flags)
 {
 	struct drm_gpuva *va, *next;
 	u64 req_end = req_addr + req_range;
@@ -2143,6 +2146,7 @@ __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm,
 					.va.range = range - req_range,
 					.gem.obj = obj,
 					.gem.offset = offset + req_range,
+					.flags = va->flags,
 				};
 				struct drm_gpuva_op_unmap u = {
 					.va = va,
@@ -2161,6 +2165,7 @@ __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm,
 				.va.range = ls_range,
 				.gem.obj = obj,
 				.gem.offset = offset,
+				.flags = va->flags,
 			};
 			struct drm_gpuva_op_unmap u = { .va = va };
 
@@ -2189,6 +2194,7 @@ __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm,
 					.gem.obj = obj,
 					.gem.offset = offset + ls_range +
 						      req_range,
+					.flags = va->flags,
 				};
 
 				ret = op_remap_cb(ops, priv, &p, &n, &u);
@@ -2221,6 +2227,7 @@ __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm,
 					.va.range = end - req_end,
 					.gem.obj = obj,
 					.gem.offset = offset + req_end - addr,
+					.flags = va->flags,
 				};
 				struct drm_gpuva_op_unmap u = {
 					.va = va,
@@ -2237,7 +2244,8 @@ __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm,
 
 	return op_map_cb(ops, priv,
 			 req_addr, req_range,
-			 req_obj, req_offset);
+			 req_obj, req_offset,
+			 req_flags);
 }
 
 static int
@@ -2266,6 +2274,7 @@ __drm_gpuvm_sm_unmap(struct drm_gpuvm *gpuvm,
 			prev.va.range = req_addr - addr;
 			prev.gem.obj = obj;
 			prev.gem.offset = offset;
+			prev.flags = va->flags;
 
 			prev_split = true;
 		}
@@ -2275,6 +2284,7 @@ __drm_gpuvm_sm_unmap(struct drm_gpuvm *gpuvm,
 			next.va.range = end - req_end;
 			next.gem.obj = obj;
 			next.gem.offset = offset + (req_end - addr);
+			next.flags = va->flags;
 
 			next_split = true;
 		}
@@ -2345,7 +2355,8 @@ drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm, void *priv,
 
 	return __drm_gpuvm_sm_map(gpuvm, ops, priv,
 				  req_addr, req_range,
-				  req_obj, req_offset);
+				  req_obj, req_offset,
+				  req_flags);
 }
 EXPORT_SYMBOL_GPL(drm_gpuvm_sm_map);
 
@@ -2538,7 +2549,8 @@ drm_gpuvm_sm_map_ops_create(struct drm_gpuvm *gpuvm,
 
 	ret = __drm_gpuvm_sm_map(gpuvm, &gpuvm_list_ops, &args,
 				 req_addr, req_range,
-				 req_obj, req_offset);
+				 req_obj, req_offset,
+				 req_flags);
 	if (ret)
 		goto err_free_ops;
 
diff --git a/include/drm/drm_gpuvm.h b/include/drm/drm_gpuvm.h
index cb045378803646a9645c002c57183c915d35befb..42b29adfabdaf193b1e1a02f9ab48ab0dd0e60d4 100644
--- a/include/drm/drm_gpuvm.h
+++ b/include/drm/drm_gpuvm.h
@@ -851,6 +851,11 @@ struct drm_gpuva_op_map {
 		 */
 		struct drm_gem_object *obj;
 	} gem;
+
+	/**
+	 * @flags: requested flags for the &drm_gpuva for this mapping
+	 */
+	enum drm_gpuva_flags flags;
 };
 
 /**

-- 
2.47.1

[PATCH 3/4] drm/gpuvm: Add DRM_GPUVA_SINGLE_PAGE flag and logic

[Asahi Lina]

To be able to support "fake sparse" mappings without relying on GPU page
fault handling, drivers may need to create large (e.g. 4GiB) mappings of
the same page repeatedly. Doing this through individual single-page
mappings would be very wasteful. This can be handled better by using a
flag on map creation, but to do it safely, drm_gpuvm needs to be aware
of this special case.

Add a flag that signals that a given mapping is a single page mapping,
which is repeated all over the entire requested VA range. This does two
things in drm_gpuvm:
- Restricts the merge logic, so only drm_gpuvas with the same flag state
  are considered "mergeable" (both SINGLE_PAGE or both not)
- Removes the GEM buffer offset logic for SINGLE_PAGE mappings. Since
  a single page from the buffer is repeatedly mapped across the entire
  VA range, the offset never needs to have an offset added to it when
  mappings are split.

Note that this does not require drm_gpuva to know anything about the
page size.

Signed-off-by: Asahi Lina <lina@asahilina.net>
---
 drivers/gpu/drm/drm_gpuvm.c | 44 ++++++++++++++++++++++++++++++++++----------
 include/drm/drm_gpuvm.h     |  9 ++++++++-
 2 files changed, 42 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/drm_gpuvm.c b/drivers/gpu/drm/drm_gpuvm.c
index 7443be1fe4de4653ec40ca4b874df30297af7faf..51429d90875e3f2370c4f8975d6fd813842b8976 100644
--- a/drivers/gpu/drm/drm_gpuvm.c
+++ b/drivers/gpu/drm/drm_gpuvm.c
@@ -670,6 +670,12 @@
  *	}
  */
 
+/**
+ * Mask of flags which must match to consider a drm_gpuva eligible for merging
+ * with a new overlaid mapping.
+ */
+#define DRM_GPUVA_UNMERGEABLE_FLAGS DRM_GPUVA_SINGLE_PAGE
+
 /**
  * get_next_vm_bo_from_list() - get the next vm_bo element
  * @__gpuvm: the &drm_gpuvm
@@ -2121,6 +2127,9 @@ __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm,
 		u64 range = va->va.range;
 		u64 end = addr + range;
 		bool merge = !!va->gem.obj;
+		bool single_page = va->flags & DRM_GPUVA_SINGLE_PAGE;
+
+		merge &= !((va->flags ^ req_flags) & DRM_GPUVA_UNMERGEABLE_FLAGS);
 
 		if (addr == req_addr) {
 			merge &= obj == req_obj &&
@@ -2145,7 +2154,8 @@ __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm,
 					.va.addr = req_end,
 					.va.range = range - req_range,
 					.gem.obj = obj,
-					.gem.offset = offset + req_range,
+					.gem.offset = offset +
+						(single_page ? 0 : req_range),
 					.flags = va->flags,
 				};
 				struct drm_gpuva_op_unmap u = {
@@ -2169,8 +2179,12 @@ __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm,
 			};
 			struct drm_gpuva_op_unmap u = { .va = va };
 
-			merge &= obj == req_obj &&
-				 offset + ls_range == req_offset;
+			merge &= obj == req_obj;
+			if (single_page)
+				merge &= offset == req_offset;
+			else
+				merge &= offset + ls_range == req_offset;
+
 			u.keep = merge;
 
 			if (end == req_end) {
@@ -2192,8 +2206,9 @@ __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm,
 					.va.addr = req_end,
 					.va.range = end - req_end,
 					.gem.obj = obj,
-					.gem.offset = offset + ls_range +
-						      req_range,
+					.gem.offset = offset +
+						(single_page ? 0 :
+						 ls_range + req_range),
 					.flags = va->flags,
 				};
 
@@ -2203,9 +2218,13 @@ __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm,
 				break;
 			}
 		} else if (addr > req_addr) {
-			merge &= obj == req_obj &&
-				 offset == req_offset +
-					   (addr - req_addr);
+			merge &= obj == req_obj;
+
+			if (single_page)
+				merge &= offset == req_offset;
+			else
+				merge &= offset == req_offset +
+					 (addr - req_addr);
 
 			if (end == req_end) {
 				ret = op_unmap_cb(ops, priv, va, merge);
@@ -2226,7 +2245,9 @@ __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm,
 					.va.addr = req_end,
 					.va.range = end - req_end,
 					.gem.obj = obj,
-					.gem.offset = offset + req_end - addr,
+					.gem.offset = offset +
+						(single_page ? 0 :
+						 req_end - addr),
 					.flags = va->flags,
 				};
 				struct drm_gpuva_op_unmap u = {
@@ -2268,6 +2289,7 @@ __drm_gpuvm_sm_unmap(struct drm_gpuvm *gpuvm,
 		u64 addr = va->va.addr;
 		u64 range = va->va.range;
 		u64 end = addr + range;
+		bool single_page = va->flags & DRM_GPUVA_SINGLE_PAGE;
 
 		if (addr < req_addr) {
 			prev.va.addr = addr;
@@ -2283,7 +2305,9 @@ __drm_gpuvm_sm_unmap(struct drm_gpuvm *gpuvm,
 			next.va.addr = req_end;
 			next.va.range = end - req_end;
 			next.gem.obj = obj;
-			next.gem.offset = offset + (req_end - addr);
+			next.gem.offset = offset;
+			if (!single_page)
+				next.gem.offset += req_end - addr;
 			next.flags = va->flags;
 
 			next_split = true;
diff --git a/include/drm/drm_gpuvm.h b/include/drm/drm_gpuvm.h
index 42b29adfabdaf193b1e1a02f9ab48ab0dd0e60d4..dfeec61908b1a8295ae08b26bef211d3d4fda85b 100644
--- a/include/drm/drm_gpuvm.h
+++ b/include/drm/drm_gpuvm.h
@@ -56,10 +56,17 @@ enum drm_gpuva_flags {
 	 */
 	DRM_GPUVA_SPARSE = (1 << 1),
 
+	/**
+	 * @DRM_GPUVA_SINGLE_PAGE:
+	 *
+	 * Flag indicating that the &drm_gpuva is a single-page mapping.
+	 */
+	DRM_GPUVA_SINGLE_PAGE = (1 << 2),
+
 	/**
 	 * @DRM_GPUVA_USERBITS: user defined bits
 	 */
-	DRM_GPUVA_USERBITS = (1 << 2),
+	DRM_GPUVA_USERBITS = (1 << 3),
 };
 
 /**

-- 
2.47.1

[PATCH 4/4] drm/gpuvm: Plumb through flags into drm_gpuva_init

[Asahi Lina]

Now that drm_gpuva_op_map has a flags field, plumb it through to the
helper that populates a drm_gpuva.

This helper is only used by drm_gpuva_remap(), which in turn is only
used by drivers which do not use flags at all (panthor, imagination),
so this change has no effect on existing drivers.

Signed-off-by: Asahi Lina <lina@asahilina.net>
---
 include/drm/drm_gpuvm.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/include/drm/drm_gpuvm.h b/include/drm/drm_gpuvm.h
index dfeec61908b1a8295ae08b26bef211d3d4fda85b..16e6dcb8755bfedca5d1f184d72db9b2e9b857d0 100644
--- a/include/drm/drm_gpuvm.h
+++ b/include/drm/drm_gpuvm.h
@@ -168,12 +168,14 @@ struct drm_gpuva *drm_gpuva_find_prev(struct drm_gpuvm *gpuvm, u64 start);
 struct drm_gpuva *drm_gpuva_find_next(struct drm_gpuvm *gpuvm, u64 end);
 
 static inline void drm_gpuva_init(struct drm_gpuva *va, u64 addr, u64 range,
-				  struct drm_gem_object *obj, u64 offset)
+				  struct drm_gem_object *obj, u64 offset,
+				  enum drm_gpuva_flags flags)
 {
 	va->va.addr = addr;
 	va->va.range = range;
 	va->gem.obj = obj;
 	va->gem.offset = offset;
+	va->flags = flags;
 }
 
 /**
@@ -1088,7 +1090,7 @@ static inline void drm_gpuva_init_from_op(struct drm_gpuva *va,
 					  struct drm_gpuva_op_map *op)
 {
 	drm_gpuva_init(va, op->va.addr, op->va.range,
-		       op->gem.obj, op->gem.offset);
+		       op->gem.obj, op->gem.offset, op->flags);
 }
 
 /**

-- 
2.47.1

Re: [PATCH 0/4] drm/gpuvm: Add support for single-page-filled mappings

[Danilo Krummrich]

Hi Lina,

On Sun, Feb 02, 2025 at 10:34:49PM +0900, Asahi Lina wrote:
> Some hardware requires dummy page mappings to efficiently implement
> Vulkan sparse features. These mappings consist of the same physical
> memory page, repeated for a large range of address space (e.g. 16GiB).
> 
> Add support for this to drm_gpuvm. Currently, drm_gpuvm expects BO
> ranges to correspond 1:1 to virtual memory ranges that are mapped, and
> does math on the BO offset accordingly. To make single page mappings
> work, we need a way to turn off that math, keeping the BO offset always
> constant and pointing to the same page (typically BO offset 0).
> 
> To make this work, we need to handle all the corner cases when these
> mappings intersect with regular mappings. The rules are simply to never
> mix or merge a "regular" mapping with a single page mapping.
> 
> drm_gpuvm has support for a flags field in drm_gpuva objects. This is
> normally managed by drivers directly. We can introduce a
> DRM_GPUVA_SINGLE_PAGE flag to handle this. However, to make it work,
> sm_map and friends need to know ahead of time whether the new mapping is
> a single page mapping or not. Therefore, we need to add an argument to
> these functions so drivers can provide the flags to be filled into
> drm_gpuva.flags.
> 
> These changes should not affect any existing drivers that use drm_gpuvm
> other than the API change:
> 
> - imagination: Does not use flags at all
> - nouveau: Only uses drm_gpuva_invalidate(), which is only called on
>   existing drm_gpuva objects (after the map steps)
> - panthor: Does not use flags at all
> - xe: Does not use drm_gpuva_init_from_op() or
>   drm_gpuva_remap()/drm_gpuva_map() (which call it). This means that the
> flags field of the gpuva object is managed by the driver only, so these
> changes cannot clobber it.
> 
> Note that the way this is implemented, drm_gpuvm does not need to know
> the GPU page size. It only has to never do math on the BO offset to meet
> the requirements.
> 
> I suspect that after this change there could be some cleanup possible in
> the xe driver (which right now passes flags around in various
> driver-specific ways from the map step through to drm_gpuva objects),
> but I'll leave that to the Xe folks.
> 
> Signed-off-by: Asahi Lina <lina@asahilina.net>
> ---
> Asahi Lina (4):
>       drm/gpuvm: Add a flags argument to drm_gpuvm_sm_map[_*]
>       drm/gpuvm: Plumb through flags into drm_gpuva_op_map
>       drm/gpuvm: Add DRM_GPUVA_SINGLE_PAGE flag and logic
>       drm/gpuvm: Plumb through flags into drm_gpuva_init

Without looking into any details yet:

This is a bit of tricky one, since we're not even close to having a user for
this new feature upstream yet, are we?

I wonder if we could do an exception by adding some KUnit tests (which
admittedly I never got to) validating things with and without this new feature.

Speaking of tests, how did you validate this change to validate the behavior
without DRM_GPUVA_SINGLE_PAGE?

- Danilo

Re: [PATCH 0/4] drm/gpuvm: Add support for single-page-filled mappings

[Asahi Lina]

On 2/3/25 3:53 AM, Danilo Krummrich wrote:
> Hi Lina,
> 
> On Sun, Feb 02, 2025 at 10:34:49PM +0900, Asahi Lina wrote:
>> Some hardware requires dummy page mappings to efficiently implement
>> Vulkan sparse features. These mappings consist of the same physical
>> memory page, repeated for a large range of address space (e.g. 16GiB).
>>
>> Add support for this to drm_gpuvm. Currently, drm_gpuvm expects BO
>> ranges to correspond 1:1 to virtual memory ranges that are mapped, and
>> does math on the BO offset accordingly. To make single page mappings
>> work, we need a way to turn off that math, keeping the BO offset always
>> constant and pointing to the same page (typically BO offset 0).
>>
>> To make this work, we need to handle all the corner cases when these
>> mappings intersect with regular mappings. The rules are simply to never
>> mix or merge a "regular" mapping with a single page mapping.
>>
>> drm_gpuvm has support for a flags field in drm_gpuva objects. This is
>> normally managed by drivers directly. We can introduce a
>> DRM_GPUVA_SINGLE_PAGE flag to handle this. However, to make it work,
>> sm_map and friends need to know ahead of time whether the new mapping is
>> a single page mapping or not. Therefore, we need to add an argument to
>> these functions so drivers can provide the flags to be filled into
>> drm_gpuva.flags.
>>
>> These changes should not affect any existing drivers that use drm_gpuvm
>> other than the API change:
>>
>> - imagination: Does not use flags at all
>> - nouveau: Only uses drm_gpuva_invalidate(), which is only called on
>>   existing drm_gpuva objects (after the map steps)
>> - panthor: Does not use flags at all
>> - xe: Does not use drm_gpuva_init_from_op() or
>>   drm_gpuva_remap()/drm_gpuva_map() (which call it). This means that the
>> flags field of the gpuva object is managed by the driver only, so these
>> changes cannot clobber it.
>>
>> Note that the way this is implemented, drm_gpuvm does not need to know
>> the GPU page size. It only has to never do math on the BO offset to meet
>> the requirements.
>>
>> I suspect that after this change there could be some cleanup possible in
>> the xe driver (which right now passes flags around in various
>> driver-specific ways from the map step through to drm_gpuva objects),
>> but I'll leave that to the Xe folks.
>>
>> Signed-off-by: Asahi Lina <lina@asahilina.net>
>> ---
>> Asahi Lina (4):
>>       drm/gpuvm: Add a flags argument to drm_gpuvm_sm_map[_*]
>>       drm/gpuvm: Plumb through flags into drm_gpuva_op_map
>>       drm/gpuvm: Add DRM_GPUVA_SINGLE_PAGE flag and logic
>>       drm/gpuvm: Plumb through flags into drm_gpuva_init
> 
> Without looking into any details yet:
> 
> This is a bit of tricky one, since we're not even close to having a user for
> this new feature upstream yet, are we?

I'd hope we're at least somewhere "this year" close to upstreaming
drm/asahi!

> 
> I wonder if we could do an exception by adding some KUnit tests (which
> admittedly I never got to) validating things with and without this new feature.
> 
> Speaking of tests, how did you validate this change to validate the behavior
> without DRM_GPUVA_SINGLE_PAGE?

Mostly just making sure our driver passes GL/Vulkan CTS including sparse
after this change. I do want to put together some low-level tests in
igt-gpu-tools, but I haven't gotten around to it yet...

~~ Lina

Re: [PATCH 0/4] drm/gpuvm: Add support for single-page-filled mappings

[Boris Brezillon]

+Akash with whom we've been discussing adding a 'REPEAT' mode to
drm_gpuvm/panthor.

On Sun, 2 Feb 2025 19:53:47 +0100
Danilo Krummrich <dakr@kernel.org> wrote:

> Hi Lina,
> 
> On Sun, Feb 02, 2025 at 10:34:49PM +0900, Asahi Lina wrote:
> > Some hardware requires dummy page mappings to efficiently implement
> > Vulkan sparse features. These mappings consist of the same physical
> > memory page, repeated for a large range of address space (e.g. 16GiB).
> > 
> > Add support for this to drm_gpuvm. Currently, drm_gpuvm expects BO
> > ranges to correspond 1:1 to virtual memory ranges that are mapped, and
> > does math on the BO offset accordingly. To make single page mappings
> > work, we need a way to turn off that math, keeping the BO offset always
> > constant and pointing to the same page (typically BO offset 0).
> > 
> > To make this work, we need to handle all the corner cases when these
> > mappings intersect with regular mappings. The rules are simply to never
> > mix or merge a "regular" mapping with a single page mapping.
> > 
> > drm_gpuvm has support for a flags field in drm_gpuva objects. This is
> > normally managed by drivers directly. We can introduce a
> > DRM_GPUVA_SINGLE_PAGE flag to handle this. However, to make it work,
> > sm_map and friends need to know ahead of time whether the new mapping is
> > a single page mapping or not. Therefore, we need to add an argument to
> > these functions so drivers can provide the flags to be filled into
> > drm_gpuva.flags.
> > 
> > These changes should not affect any existing drivers that use drm_gpuvm
> > other than the API change:
> > 
> > - imagination: Does not use flags at all
> > - nouveau: Only uses drm_gpuva_invalidate(), which is only called on
> >   existing drm_gpuva objects (after the map steps)
> > - panthor: Does not use flags at all
> > - xe: Does not use drm_gpuva_init_from_op() or
> >   drm_gpuva_remap()/drm_gpuva_map() (which call it). This means that the
> > flags field of the gpuva object is managed by the driver only, so these
> > changes cannot clobber it.
> > 
> > Note that the way this is implemented, drm_gpuvm does not need to know
> > the GPU page size. It only has to never do math on the BO offset to meet
> > the requirements.
> > 
> > I suspect that after this change there could be some cleanup possible in
> > the xe driver (which right now passes flags around in various
> > driver-specific ways from the map step through to drm_gpuva objects),
> > but I'll leave that to the Xe folks.
> > 
> > Signed-off-by: Asahi Lina <lina@asahilina.net>
> > ---
> > Asahi Lina (4):
> >       drm/gpuvm: Add a flags argument to drm_gpuvm_sm_map[_*]
> >       drm/gpuvm: Plumb through flags into drm_gpuva_op_map
> >       drm/gpuvm: Add DRM_GPUVA_SINGLE_PAGE flag and logic
> >       drm/gpuvm: Plumb through flags into drm_gpuva_init  
> 
> Without looking into any details yet:
> 
> This is a bit of tricky one, since we're not even close to having a user for
> this new feature upstream yet, are we?

Actually, we would be interesting in having this feature hooked up in
panthor. One use case we have is Vulkan sparse bindings, of course. But
we also have cases where we need to map a dummy page repeatedly on the
FW side. The approach we've been considering is slightly different:
pass a DRM_GPUVA_REPEAT_FLAG along with GEM range, so we can repeat a
range of the GEM (see the below diff, which is completely untested by
the way), but I think we'd be fine with this SINGLE_PAGE flag.

--->8---
diff --git a/drivers/gpu/drm/drm_gpuvm.c b/drivers/gpu/drm/drm_gpuvm.c
index f9eb56f24bef..ea61f3ffaddf 100644
--- a/drivers/gpu/drm/drm_gpuvm.c
+++ b/drivers/gpu/drm/drm_gpuvm.c
@@ -2053,16 +2053,17 @@ EXPORT_SYMBOL_GPL(drm_gpuva_unmap);
 
 static int
 op_map_cb(const struct drm_gpuvm_ops *fn, void *priv,
-      u64 addr, u64 range,
-      struct drm_gem_object *obj, u64 offset)
+      u64 addr, u64 va_range,
+      struct drm_gem_object *obj, u64 offset, u64 gem_range)
 {
     struct drm_gpuva_op op = {};
 
     op.op = DRM_GPUVA_OP_MAP;
     op.map.va.addr = addr;
-    op.map.va.range = range;
+    op.map.va.range = va_range;
     op.map.gem.obj = obj;
     op.map.gem.offset = offset;
+    op.map.gem.range = gem_range;
 
     return fn->sm_step_map(&op, priv);
 }
@@ -2102,7 +2103,8 @@ static int
 __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm,
            const struct drm_gpuvm_ops *ops, void *priv,
            u64 req_addr, u64 req_range,
-           struct drm_gem_object *req_obj, u64 req_offset)
+           struct drm_gem_object *req_obj,
+           u64 req_offset, u64 req_gem_range)
 {
     struct drm_gpuva *va, *next;
     u64 req_end = req_addr + req_range;
@@ -2237,7 +2239,7 @@ __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm,
 
     return op_map_cb(ops, priv,
              req_addr, req_range,
-             req_obj, req_offset);
+             req_obj, req_offset, req_gem_range);
 }
 
 static int
@@ -2344,10 +2346,43 @@ drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm, void *priv,
 
     return __drm_gpuvm_sm_map(gpuvm, ops, priv,
                   req_addr, req_range,
-                  req_obj, req_offset);
+                  req_obj, req_offset, 0);
 }
 EXPORT_SYMBOL_GPL(drm_gpuvm_sm_map);
 
+/**
+ * drm_gpuvm_sm_map_repeat() - repeatedly maps a GEM range over a VA range
+ * @gpuvm: the &drm_gpuvm representing the GPU VA space
+ * @priv: pointer to a driver private data structure
+ * @req_addr: the start address of the new mapping
+ * @req_range: the range of the new mapping
+ * @req_obj: the &drm_gem_object to map
+ * @req_offset: the offset within the &drm_gem_object
+ * @req_gem_range: the offset within the &drm_gem_object
+ *
+ * Same as drm_gpuvm_sm_map() except it repeats a GEM range over a VA range
+ *
+ * Returns: 0 on success or a negative error code
+ */
+int
+drm_gpuvm_sm_map_repeat(struct drm_gpuvm *gpuvm, void *priv,
+            u64 req_addr, u64 req_range,
+            struct drm_gem_object *req_obj,
+            u64 req_offset, u64 req_gem_range)
+{
+    const struct drm_gpuvm_ops *ops = gpuvm->ops;
+
+    if (unlikely(!(ops && ops->sm_step_map &&
+               ops->sm_step_remap &&
+               ops->sm_step_unmap)))
+        return -EINVAL;
+
+    return __drm_gpuvm_sm_map(gpuvm, ops, priv,
+                  req_addr, req_range,
+                  req_obj, req_offset, req_gem_range);
+}
+EXPORT_SYMBOL_GPL(drm_gpuvm_sm_map_repeat);
+
 /**
  * drm_gpuvm_sm_unmap() - creates the &drm_gpuva_ops to split on unmap
  * @gpuvm: the &drm_gpuvm representing the GPU VA space
@@ -2536,7 +2571,7 @@ drm_gpuvm_sm_map_ops_create(struct drm_gpuvm *gpuvm,
 
     ret = __drm_gpuvm_sm_map(gpuvm, &gpuvm_list_ops, &args,
                  req_addr, req_range,
-                 req_obj, req_offset);
+                 req_obj, req_offset, 0);
     if (ret)
         goto err_free_ops;
 
diff --git a/include/drm/drm_gpuvm.h b/include/drm/drm_gpuvm.h
index 00d4e43b76b6..8157ede365d1 100644
--- a/include/drm/drm_gpuvm.h
+++ b/include/drm/drm_gpuvm.h
@@ -846,6 +846,14 @@ struct drm_gpuva_op_map {
          */
         u64 offset;
 
+        /**
+         * @gem.range: the range of the GEM to map
+         *
+         * If smaller than va.range, the GEM range should be mapped
+         * multiple times over the VA range.
+         */
+        u64 range;
+
         /**
          * @gem.obj: the &drm_gem_object to map
          */
@@ -1203,6 +1211,11 @@ int drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm, void *priv,
              u64 addr, u64 range,
              struct drm_gem_object *obj, u64 offset);
 
+int drm_gpuvm_sm_map_repeat(struct drm_gpuvm *gpuvm, void *priv,
+                u64 addr, u64 range,
+                struct drm_gem_object *obj,
+                u64 offset, u64 gem_range);
+
 int drm_gpuvm_sm_unmap(struct drm_gpuvm *gpuvm, void *priv,
                u64 addr, u64 range);

Re: [PATCH 0/4] drm/gpuvm: Add support for single-page-filled mappings

[Liviu Dudau]

On Mon, Feb 03, 2025 at 10:21:53AM +0100, Boris Brezillon wrote:
> +Akash with whom we've been discussing adding a 'REPEAT' mode to
> drm_gpuvm/panthor.
> 
> On Sun, 2 Feb 2025 19:53:47 +0100
> Danilo Krummrich <dakr@kernel.org> wrote:
> 
> > Hi Lina,
> > 
> > On Sun, Feb 02, 2025 at 10:34:49PM +0900, Asahi Lina wrote:
> > > Some hardware requires dummy page mappings to efficiently implement
> > > Vulkan sparse features. These mappings consist of the same physical
> > > memory page, repeated for a large range of address space (e.g. 16GiB).
> > > 
> > > Add support for this to drm_gpuvm. Currently, drm_gpuvm expects BO
> > > ranges to correspond 1:1 to virtual memory ranges that are mapped, and
> > > does math on the BO offset accordingly. To make single page mappings
> > > work, we need a way to turn off that math, keeping the BO offset always
> > > constant and pointing to the same page (typically BO offset 0).
> > > 
> > > To make this work, we need to handle all the corner cases when these
> > > mappings intersect with regular mappings. The rules are simply to never
> > > mix or merge a "regular" mapping with a single page mapping.
> > > 
> > > drm_gpuvm has support for a flags field in drm_gpuva objects. This is
> > > normally managed by drivers directly. We can introduce a
> > > DRM_GPUVA_SINGLE_PAGE flag to handle this. However, to make it work,
> > > sm_map and friends need to know ahead of time whether the new mapping is
> > > a single page mapping or not. Therefore, we need to add an argument to
> > > these functions so drivers can provide the flags to be filled into
> > > drm_gpuva.flags.
> > > 
> > > These changes should not affect any existing drivers that use drm_gpuvm
> > > other than the API change:
> > > 
> > > - imagination: Does not use flags at all
> > > - nouveau: Only uses drm_gpuva_invalidate(), which is only called on
> > >   existing drm_gpuva objects (after the map steps)
> > > - panthor: Does not use flags at all
> > > - xe: Does not use drm_gpuva_init_from_op() or
> > >   drm_gpuva_remap()/drm_gpuva_map() (which call it). This means that the
> > > flags field of the gpuva object is managed by the driver only, so these
> > > changes cannot clobber it.
> > > 
> > > Note that the way this is implemented, drm_gpuvm does not need to know
> > > the GPU page size. It only has to never do math on the BO offset to meet
> > > the requirements.
> > > 
> > > I suspect that after this change there could be some cleanup possible in
> > > the xe driver (which right now passes flags around in various
> > > driver-specific ways from the map step through to drm_gpuva objects),
> > > but I'll leave that to the Xe folks.
> > > 
> > > Signed-off-by: Asahi Lina <lina@asahilina.net>
> > > ---
> > > Asahi Lina (4):
> > >       drm/gpuvm: Add a flags argument to drm_gpuvm_sm_map[_*]
> > >       drm/gpuvm: Plumb through flags into drm_gpuva_op_map
> > >       drm/gpuvm: Add DRM_GPUVA_SINGLE_PAGE flag and logic
> > >       drm/gpuvm: Plumb through flags into drm_gpuva_init  
> > 
> > Without looking into any details yet:
> > 
> > This is a bit of tricky one, since we're not even close to having a user for
> > this new feature upstream yet, are we?
> 
> Actually, we would be interesting in having this feature hooked up in
> panthor. One use case we have is Vulkan sparse bindings, of course. But
> we also have cases where we need to map a dummy page repeatedly on the
> FW side. The approach we've been considering is slightly different:
> pass a DRM_GPUVA_REPEAT_FLAG along with GEM range, so we can repeat a
> range of the GEM (see the below diff, which is completely untested by
> the way), but I think we'd be fine with this SINGLE_PAGE flag.

Unless I've misunderstood the intent completely, it looks like Xe also wants
something similar although they call it CPU_ADDR_MIRROR for some reason:

https://lore.kernel.org/r/20250129195212.745731-9-matthew.brost@intel.com

Best regards,
Liviu

> 
> --->8---
> diff --git a/drivers/gpu/drm/drm_gpuvm.c b/drivers/gpu/drm/drm_gpuvm.c
> index f9eb56f24bef..ea61f3ffaddf 100644
> --- a/drivers/gpu/drm/drm_gpuvm.c
> +++ b/drivers/gpu/drm/drm_gpuvm.c
> @@ -2053,16 +2053,17 @@ EXPORT_SYMBOL_GPL(drm_gpuva_unmap);
>  
>  static int
>  op_map_cb(const struct drm_gpuvm_ops *fn, void *priv,
> -      u64 addr, u64 range,
> -      struct drm_gem_object *obj, u64 offset)
> +      u64 addr, u64 va_range,
> +      struct drm_gem_object *obj, u64 offset, u64 gem_range)
>  {
>      struct drm_gpuva_op op = {};
>  
>      op.op = DRM_GPUVA_OP_MAP;
>      op.map.va.addr = addr;
> -    op.map.va.range = range;
> +    op.map.va.range = va_range;
>      op.map.gem.obj = obj;
>      op.map.gem.offset = offset;
> +    op.map.gem.range = gem_range;
>  
>      return fn->sm_step_map(&op, priv);
>  }
> @@ -2102,7 +2103,8 @@ static int
>  __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm,
>             const struct drm_gpuvm_ops *ops, void *priv,
>             u64 req_addr, u64 req_range,
> -           struct drm_gem_object *req_obj, u64 req_offset)
> +           struct drm_gem_object *req_obj,
> +           u64 req_offset, u64 req_gem_range)
>  {
>      struct drm_gpuva *va, *next;
>      u64 req_end = req_addr + req_range;
> @@ -2237,7 +2239,7 @@ __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm,
>  
>      return op_map_cb(ops, priv,
>               req_addr, req_range,
> -             req_obj, req_offset);
> +             req_obj, req_offset, req_gem_range);
>  }
>  
>  static int
> @@ -2344,10 +2346,43 @@ drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm, void *priv,
>  
>      return __drm_gpuvm_sm_map(gpuvm, ops, priv,
>                    req_addr, req_range,
> -                  req_obj, req_offset);
> +                  req_obj, req_offset, 0);
>  }
>  EXPORT_SYMBOL_GPL(drm_gpuvm_sm_map);
>  
> +/**
> + * drm_gpuvm_sm_map_repeat() - repeatedly maps a GEM range over a VA range
> + * @gpuvm: the &drm_gpuvm representing the GPU VA space
> + * @priv: pointer to a driver private data structure
> + * @req_addr: the start address of the new mapping
> + * @req_range: the range of the new mapping
> + * @req_obj: the &drm_gem_object to map
> + * @req_offset: the offset within the &drm_gem_object
> + * @req_gem_range: the offset within the &drm_gem_object
> + *
> + * Same as drm_gpuvm_sm_map() except it repeats a GEM range over a VA range
> + *
> + * Returns: 0 on success or a negative error code
> + */
> +int
> +drm_gpuvm_sm_map_repeat(struct drm_gpuvm *gpuvm, void *priv,
> +            u64 req_addr, u64 req_range,
> +            struct drm_gem_object *req_obj,
> +            u64 req_offset, u64 req_gem_range)
> +{
> +    const struct drm_gpuvm_ops *ops = gpuvm->ops;
> +
> +    if (unlikely(!(ops && ops->sm_step_map &&
> +               ops->sm_step_remap &&
> +               ops->sm_step_unmap)))
> +        return -EINVAL;
> +
> +    return __drm_gpuvm_sm_map(gpuvm, ops, priv,
> +                  req_addr, req_range,
> +                  req_obj, req_offset, req_gem_range);
> +}
> +EXPORT_SYMBOL_GPL(drm_gpuvm_sm_map_repeat);
> +
>  /**
>   * drm_gpuvm_sm_unmap() - creates the &drm_gpuva_ops to split on unmap
>   * @gpuvm: the &drm_gpuvm representing the GPU VA space
> @@ -2536,7 +2571,7 @@ drm_gpuvm_sm_map_ops_create(struct drm_gpuvm *gpuvm,
>  
>      ret = __drm_gpuvm_sm_map(gpuvm, &gpuvm_list_ops, &args,
>                   req_addr, req_range,
> -                 req_obj, req_offset);
> +                 req_obj, req_offset, 0);
>      if (ret)
>          goto err_free_ops;
>  
> diff --git a/include/drm/drm_gpuvm.h b/include/drm/drm_gpuvm.h
> index 00d4e43b76b6..8157ede365d1 100644
> --- a/include/drm/drm_gpuvm.h
> +++ b/include/drm/drm_gpuvm.h
> @@ -846,6 +846,14 @@ struct drm_gpuva_op_map {
>           */
>          u64 offset;
>  
> +        /**
> +         * @gem.range: the range of the GEM to map
> +         *
> +         * If smaller than va.range, the GEM range should be mapped
> +         * multiple times over the VA range.
> +         */
> +        u64 range;
> +
>          /**
>           * @gem.obj: the &drm_gem_object to map
>           */
> @@ -1203,6 +1211,11 @@ int drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm, void *priv,
>               u64 addr, u64 range,
>               struct drm_gem_object *obj, u64 offset);
>  
> +int drm_gpuvm_sm_map_repeat(struct drm_gpuvm *gpuvm, void *priv,
> +                u64 addr, u64 range,
> +                struct drm_gem_object *obj,
> +                u64 offset, u64 gem_range);
> +
>  int drm_gpuvm_sm_unmap(struct drm_gpuvm *gpuvm, void *priv,
>                 u64 addr, u64 range);

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯

Re: [PATCH 0/4] drm/gpuvm: Add support for single-page-filled mappings

[Boris Brezillon]

On Mon, 3 Feb 2025 11:23:53 +0000
Liviu Dudau <liviu.dudau@arm.com> wrote:

> On Mon, Feb 03, 2025 at 10:21:53AM +0100, Boris Brezillon wrote:
> > +Akash with whom we've been discussing adding a 'REPEAT' mode to
> > drm_gpuvm/panthor.
> > 
> > On Sun, 2 Feb 2025 19:53:47 +0100
> > Danilo Krummrich <dakr@kernel.org> wrote:
> >   
> > > Hi Lina,
> > > 
> > > On Sun, Feb 02, 2025 at 10:34:49PM +0900, Asahi Lina wrote:  
> > > > Some hardware requires dummy page mappings to efficiently implement
> > > > Vulkan sparse features. These mappings consist of the same physical
> > > > memory page, repeated for a large range of address space (e.g. 16GiB).
> > > > 
> > > > Add support for this to drm_gpuvm. Currently, drm_gpuvm expects BO
> > > > ranges to correspond 1:1 to virtual memory ranges that are mapped, and
> > > > does math on the BO offset accordingly. To make single page mappings
> > > > work, we need a way to turn off that math, keeping the BO offset always
> > > > constant and pointing to the same page (typically BO offset 0).
> > > > 
> > > > To make this work, we need to handle all the corner cases when these
> > > > mappings intersect with regular mappings. The rules are simply to never
> > > > mix or merge a "regular" mapping with a single page mapping.
> > > > 
> > > > drm_gpuvm has support for a flags field in drm_gpuva objects. This is
> > > > normally managed by drivers directly. We can introduce a
> > > > DRM_GPUVA_SINGLE_PAGE flag to handle this. However, to make it work,
> > > > sm_map and friends need to know ahead of time whether the new mapping is
> > > > a single page mapping or not. Therefore, we need to add an argument to
> > > > these functions so drivers can provide the flags to be filled into
> > > > drm_gpuva.flags.
> > > > 
> > > > These changes should not affect any existing drivers that use drm_gpuvm
> > > > other than the API change:
> > > > 
> > > > - imagination: Does not use flags at all
> > > > - nouveau: Only uses drm_gpuva_invalidate(), which is only called on
> > > >   existing drm_gpuva objects (after the map steps)
> > > > - panthor: Does not use flags at all
> > > > - xe: Does not use drm_gpuva_init_from_op() or
> > > >   drm_gpuva_remap()/drm_gpuva_map() (which call it). This means that the
> > > > flags field of the gpuva object is managed by the driver only, so these
> > > > changes cannot clobber it.
> > > > 
> > > > Note that the way this is implemented, drm_gpuvm does not need to know
> > > > the GPU page size. It only has to never do math on the BO offset to meet
> > > > the requirements.
> > > > 
> > > > I suspect that after this change there could be some cleanup possible in
> > > > the xe driver (which right now passes flags around in various
> > > > driver-specific ways from the map step through to drm_gpuva objects),
> > > > but I'll leave that to the Xe folks.
> > > > 
> > > > Signed-off-by: Asahi Lina <lina@asahilina.net>
> > > > ---
> > > > Asahi Lina (4):
> > > >       drm/gpuvm: Add a flags argument to drm_gpuvm_sm_map[_*]
> > > >       drm/gpuvm: Plumb through flags into drm_gpuva_op_map
> > > >       drm/gpuvm: Add DRM_GPUVA_SINGLE_PAGE flag and logic
> > > >       drm/gpuvm: Plumb through flags into drm_gpuva_init    
> > > 
> > > Without looking into any details yet:
> > > 
> > > This is a bit of tricky one, since we're not even close to having a user for
> > > this new feature upstream yet, are we?  
> > 
> > Actually, we would be interesting in having this feature hooked up in
> > panthor. One use case we have is Vulkan sparse bindings, of course. But
> > we also have cases where we need to map a dummy page repeatedly on the
> > FW side. The approach we've been considering is slightly different:
> > pass a DRM_GPUVA_REPEAT_FLAG along with GEM range, so we can repeat a
> > range of the GEM (see the below diff, which is completely untested by
> > the way), but I think we'd be fine with this SINGLE_PAGE flag.  
> 
> Unless I've misunderstood the intent completely, it looks like Xe also wants
> something similar although they call it CPU_ADDR_MIRROR for some reason:
> 
> https://lore.kernel.org/r/20250129195212.745731-9-matthew.brost@intel.com

At first glance, it doesn't seem related. The Xe stuff looks more like
an alloc-on-fault mechanism. SINGLE_PAGE is about mapping a dummy page
repeatedly over a virtual address range so that sparse Vulkan images
never get a fault when an unbound image region is accessed.

Re: [PATCH 0/4] drm/gpuvm: Add support for single-page-filled mappings

[Matthew Auld]

On 03/02/2025 12:12, Boris Brezillon wrote:
> On Mon, 3 Feb 2025 11:23:53 +0000
> Liviu Dudau <liviu.dudau@arm.com> wrote:
> 
>> On Mon, Feb 03, 2025 at 10:21:53AM +0100, Boris Brezillon wrote:
>>> +Akash with whom we've been discussing adding a 'REPEAT' mode to
>>> drm_gpuvm/panthor.
>>>
>>> On Sun, 2 Feb 2025 19:53:47 +0100
>>> Danilo Krummrich <dakr@kernel.org> wrote:
>>>    
>>>> Hi Lina,
>>>>
>>>> On Sun, Feb 02, 2025 at 10:34:49PM +0900, Asahi Lina wrote:
>>>>> Some hardware requires dummy page mappings to efficiently implement
>>>>> Vulkan sparse features. These mappings consist of the same physical
>>>>> memory page, repeated for a large range of address space (e.g. 16GiB).
>>>>>
>>>>> Add support for this to drm_gpuvm. Currently, drm_gpuvm expects BO
>>>>> ranges to correspond 1:1 to virtual memory ranges that are mapped, and
>>>>> does math on the BO offset accordingly. To make single page mappings
>>>>> work, we need a way to turn off that math, keeping the BO offset always
>>>>> constant and pointing to the same page (typically BO offset 0).
>>>>>
>>>>> To make this work, we need to handle all the corner cases when these
>>>>> mappings intersect with regular mappings. The rules are simply to never
>>>>> mix or merge a "regular" mapping with a single page mapping.
>>>>>
>>>>> drm_gpuvm has support for a flags field in drm_gpuva objects. This is
>>>>> normally managed by drivers directly. We can introduce a
>>>>> DRM_GPUVA_SINGLE_PAGE flag to handle this. However, to make it work,
>>>>> sm_map and friends need to know ahead of time whether the new mapping is
>>>>> a single page mapping or not. Therefore, we need to add an argument to
>>>>> these functions so drivers can provide the flags to be filled into
>>>>> drm_gpuva.flags.
>>>>>
>>>>> These changes should not affect any existing drivers that use drm_gpuvm
>>>>> other than the API change:
>>>>>
>>>>> - imagination: Does not use flags at all
>>>>> - nouveau: Only uses drm_gpuva_invalidate(), which is only called on
>>>>>    existing drm_gpuva objects (after the map steps)
>>>>> - panthor: Does not use flags at all
>>>>> - xe: Does not use drm_gpuva_init_from_op() or
>>>>>    drm_gpuva_remap()/drm_gpuva_map() (which call it). This means that the
>>>>> flags field of the gpuva object is managed by the driver only, so these
>>>>> changes cannot clobber it.
>>>>>
>>>>> Note that the way this is implemented, drm_gpuvm does not need to know
>>>>> the GPU page size. It only has to never do math on the BO offset to meet
>>>>> the requirements.
>>>>>
>>>>> I suspect that after this change there could be some cleanup possible in
>>>>> the xe driver (which right now passes flags around in various
>>>>> driver-specific ways from the map step through to drm_gpuva objects),
>>>>> but I'll leave that to the Xe folks.
>>>>>
>>>>> Signed-off-by: Asahi Lina <lina@asahilina.net>
>>>>> ---
>>>>> Asahi Lina (4):
>>>>>        drm/gpuvm: Add a flags argument to drm_gpuvm_sm_map[_*]
>>>>>        drm/gpuvm: Plumb through flags into drm_gpuva_op_map
>>>>>        drm/gpuvm: Add DRM_GPUVA_SINGLE_PAGE flag and logic
>>>>>        drm/gpuvm: Plumb through flags into drm_gpuva_init
>>>>
>>>> Without looking into any details yet:
>>>>
>>>> This is a bit of tricky one, since we're not even close to having a user for
>>>> this new feature upstream yet, are we?
>>>
>>> Actually, we would be interesting in having this feature hooked up in
>>> panthor. One use case we have is Vulkan sparse bindings, of course. But
>>> we also have cases where we need to map a dummy page repeatedly on the
>>> FW side. The approach we've been considering is slightly different:
>>> pass a DRM_GPUVA_REPEAT_FLAG along with GEM range, so we can repeat a
>>> range of the GEM (see the below diff, which is completely untested by
>>> the way), but I think we'd be fine with this SINGLE_PAGE flag.
>>
>> Unless I've misunderstood the intent completely, it looks like Xe also wants
>> something similar although they call it CPU_ADDR_MIRROR for some reason:
>>
>> https://lore.kernel.org/r/20250129195212.745731-9-matthew.brost@intel.com
> 
> At first glance, it doesn't seem related. The Xe stuff looks more like
> an alloc-on-fault mechanism. SINGLE_PAGE is about mapping a dummy page
> repeatedly over a virtual address range so that sparse Vulkan images
> never get a fault when an unbound image region is accessed.

Yeah, the CPU_ADDR_MIRROR is for upcoming svm stuff and not related. I 
believe in xe the sparse/repeat stuff in this series is rather done 
using a special HW feature called "NULL page", which is just a bit you 
can set in the pte (can also use huge pages) that creates a special 
mapping that doesn't actually point to any real memory underneath, but 
when doing a read it will always return zeroes and any writes to it will 
be dropped by the HW.

Re: [PATCH 0/4] drm/gpuvm: Add support for single-page-filled mappings

[Asahi Lina]

Hi,

On 2/3/25 6:21 PM, Boris Brezillon wrote:
> +Akash with whom we've been discussing adding a 'REPEAT' mode to
> drm_gpuvm/panthor.
> 
> On Sun, 2 Feb 2025 19:53:47 +0100
> Danilo Krummrich <dakr@kernel.org> wrote:
> 
>> Hi Lina,
>>
>> On Sun, Feb 02, 2025 at 10:34:49PM +0900, Asahi Lina wrote:
>>> Some hardware requires dummy page mappings to efficiently implement
>>> Vulkan sparse features. These mappings consist of the same physical
>>> memory page, repeated for a large range of address space (e.g. 16GiB).
>>>
>>> Add support for this to drm_gpuvm. Currently, drm_gpuvm expects BO
>>> ranges to correspond 1:1 to virtual memory ranges that are mapped, and
>>> does math on the BO offset accordingly. To make single page mappings
>>> work, we need a way to turn off that math, keeping the BO offset always
>>> constant and pointing to the same page (typically BO offset 0).
>>>
>>> To make this work, we need to handle all the corner cases when these
>>> mappings intersect with regular mappings. The rules are simply to never
>>> mix or merge a "regular" mapping with a single page mapping.
>>>
>>> drm_gpuvm has support for a flags field in drm_gpuva objects. This is
>>> normally managed by drivers directly. We can introduce a
>>> DRM_GPUVA_SINGLE_PAGE flag to handle this. However, to make it work,
>>> sm_map and friends need to know ahead of time whether the new mapping is
>>> a single page mapping or not. Therefore, we need to add an argument to
>>> these functions so drivers can provide the flags to be filled into
>>> drm_gpuva.flags.
>>>
>>> These changes should not affect any existing drivers that use drm_gpuvm
>>> other than the API change:
>>>
>>> - imagination: Does not use flags at all
>>> - nouveau: Only uses drm_gpuva_invalidate(), which is only called on
>>>   existing drm_gpuva objects (after the map steps)
>>> - panthor: Does not use flags at all
>>> - xe: Does not use drm_gpuva_init_from_op() or
>>>   drm_gpuva_remap()/drm_gpuva_map() (which call it). This means that the
>>> flags field of the gpuva object is managed by the driver only, so these
>>> changes cannot clobber it.
>>>
>>> Note that the way this is implemented, drm_gpuvm does not need to know
>>> the GPU page size. It only has to never do math on the BO offset to meet
>>> the requirements.
>>>
>>> I suspect that after this change there could be some cleanup possible in
>>> the xe driver (which right now passes flags around in various
>>> driver-specific ways from the map step through to drm_gpuva objects),
>>> but I'll leave that to the Xe folks.
>>>
>>> Signed-off-by: Asahi Lina <lina@asahilina.net>
>>> ---
>>> Asahi Lina (4):
>>>       drm/gpuvm: Add a flags argument to drm_gpuvm_sm_map[_*]
>>>       drm/gpuvm: Plumb through flags into drm_gpuva_op_map
>>>       drm/gpuvm: Add DRM_GPUVA_SINGLE_PAGE flag and logic
>>>       drm/gpuvm: Plumb through flags into drm_gpuva_init  
>>
>> Without looking into any details yet:
>>
>> This is a bit of tricky one, since we're not even close to having a user for
>> this new feature upstream yet, are we?
> 
> Actually, we would be interesting in having this feature hooked up in
> panthor. One use case we have is Vulkan sparse bindings, of course. But
> we also have cases where we need to map a dummy page repeatedly on the
> FW side. The approach we've been considering is slightly different:
> pass a DRM_GPUVA_REPEAT_FLAG along with GEM range, so we can repeat a
> range of the GEM (see the below diff, which is completely untested by
> the way), but I think we'd be fine with this SINGLE_PAGE flag.

That sounds similar, though your patch does not handle gpuva
splitting/remapping and all the other corner cases. I think you'll find
that once you handle those, the logic will become significantly more
complicated, since you need to start storing the start offset within the
repeat range on GPUVAs to be able to split them while keeping the
mappings identical, and do modular arithmetic to keep it all consistent
across all the corner cases.

If SINGLE_PAGE works for you then I would advocate for that. It keeps
complexity down to a minimum in drm_gpuvm. You can still have a range
that's greater than one page in practice, you'd just have to handle it
driver-internal and pass the desired range out of band as a flag or
other field. For example, you could decide that the mapping is always
congruent to the VA (GEM page offset = start offset + VA % range) and
always treat SINGLE_PAGE mappings like that when you actually set up the
page tables, or pass in an extra offset to be able to shift the phase of
the mapping to whatever you want. You just need to ensure that, if you
mix range sizes or other configuration, you don't do that for the same
GEM BO at the same offset, so that the drm_gpuvm core does not wrongly
consider them equivalent.

Maybe I should rename SINGLE_PAGE to something else, since it isn't
technically limited to that as far as gpuvm is concerned. Something like
FIXED_OFFSET?

> 
> --->8---
> diff --git a/drivers/gpu/drm/drm_gpuvm.c b/drivers/gpu/drm/drm_gpuvm.c
> index f9eb56f24bef..ea61f3ffaddf 100644
> --- a/drivers/gpu/drm/drm_gpuvm.c
> +++ b/drivers/gpu/drm/drm_gpuvm.c
> @@ -2053,16 +2053,17 @@ EXPORT_SYMBOL_GPL(drm_gpuva_unmap);
>  
>  static int
>  op_map_cb(const struct drm_gpuvm_ops *fn, void *priv,
> -      u64 addr, u64 range,
> -      struct drm_gem_object *obj, u64 offset)
> +      u64 addr, u64 va_range,
> +      struct drm_gem_object *obj, u64 offset, u64 gem_range)
>  {
>      struct drm_gpuva_op op = {};
>  
>      op.op = DRM_GPUVA_OP_MAP;
>      op.map.va.addr = addr;
> -    op.map.va.range = range;
> +    op.map.va.range = va_range;
>      op.map.gem.obj = obj;
>      op.map.gem.offset = offset;
> +    op.map.gem.range = gem_range;
>  
>      return fn->sm_step_map(&op, priv);
>  }
> @@ -2102,7 +2103,8 @@ static int
>  __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm,
>             const struct drm_gpuvm_ops *ops, void *priv,
>             u64 req_addr, u64 req_range,
> -           struct drm_gem_object *req_obj, u64 req_offset)
> +           struct drm_gem_object *req_obj,
> +           u64 req_offset, u64 req_gem_range)
>  {
>      struct drm_gpuva *va, *next;
>      u64 req_end = req_addr + req_range;
> @@ -2237,7 +2239,7 @@ __drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm,
>  
>      return op_map_cb(ops, priv,
>               req_addr, req_range,
> -             req_obj, req_offset);
> +             req_obj, req_offset, req_gem_range);
>  }
>  
>  static int
> @@ -2344,10 +2346,43 @@ drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm, void *priv,
>  
>      return __drm_gpuvm_sm_map(gpuvm, ops, priv,
>                    req_addr, req_range,
> -                  req_obj, req_offset);
> +                  req_obj, req_offset, 0);
>  }
>  EXPORT_SYMBOL_GPL(drm_gpuvm_sm_map);
>  
> +/**
> + * drm_gpuvm_sm_map_repeat() - repeatedly maps a GEM range over a VA range
> + * @gpuvm: the &drm_gpuvm representing the GPU VA space
> + * @priv: pointer to a driver private data structure
> + * @req_addr: the start address of the new mapping
> + * @req_range: the range of the new mapping
> + * @req_obj: the &drm_gem_object to map
> + * @req_offset: the offset within the &drm_gem_object
> + * @req_gem_range: the offset within the &drm_gem_object
> + *
> + * Same as drm_gpuvm_sm_map() except it repeats a GEM range over a VA range
> + *
> + * Returns: 0 on success or a negative error code
> + */
> +int
> +drm_gpuvm_sm_map_repeat(struct drm_gpuvm *gpuvm, void *priv,
> +            u64 req_addr, u64 req_range,
> +            struct drm_gem_object *req_obj,
> +            u64 req_offset, u64 req_gem_range)
> +{
> +    const struct drm_gpuvm_ops *ops = gpuvm->ops;
> +
> +    if (unlikely(!(ops && ops->sm_step_map &&
> +               ops->sm_step_remap &&
> +               ops->sm_step_unmap)))
> +        return -EINVAL;
> +
> +    return __drm_gpuvm_sm_map(gpuvm, ops, priv,
> +                  req_addr, req_range,
> +                  req_obj, req_offset, req_gem_range);
> +}
> +EXPORT_SYMBOL_GPL(drm_gpuvm_sm_map_repeat);
> +
>  /**
>   * drm_gpuvm_sm_unmap() - creates the &drm_gpuva_ops to split on unmap
>   * @gpuvm: the &drm_gpuvm representing the GPU VA space
> @@ -2536,7 +2571,7 @@ drm_gpuvm_sm_map_ops_create(struct drm_gpuvm *gpuvm,
>  
>      ret = __drm_gpuvm_sm_map(gpuvm, &gpuvm_list_ops, &args,
>                   req_addr, req_range,
> -                 req_obj, req_offset);
> +                 req_obj, req_offset, 0);
>      if (ret)
>          goto err_free_ops;
>  
> diff --git a/include/drm/drm_gpuvm.h b/include/drm/drm_gpuvm.h
> index 00d4e43b76b6..8157ede365d1 100644
> --- a/include/drm/drm_gpuvm.h
> +++ b/include/drm/drm_gpuvm.h
> @@ -846,6 +846,14 @@ struct drm_gpuva_op_map {
>           */
>          u64 offset;
>  
> +        /**
> +         * @gem.range: the range of the GEM to map
> +         *
> +         * If smaller than va.range, the GEM range should be mapped
> +         * multiple times over the VA range.
> +         */
> +        u64 range;
> +
>          /**
>           * @gem.obj: the &drm_gem_object to map
>           */
> @@ -1203,6 +1211,11 @@ int drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm, void *priv,
>               u64 addr, u64 range,
>               struct drm_gem_object *obj, u64 offset);
>  
> +int drm_gpuvm_sm_map_repeat(struct drm_gpuvm *gpuvm, void *priv,
> +                u64 addr, u64 range,
> +                struct drm_gem_object *obj,
> +                u64 offset, u64 gem_range);
> +
>  int drm_gpuvm_sm_unmap(struct drm_gpuvm *gpuvm, void *priv,
>                 u64 addr, u64 range);
> 

~~ Lina

Re: [PATCH 0/4] drm/gpuvm: Add support for single-page-filled mappings

[Boris Brezillon]

On Mon, 3 Feb 2025 22:46:15 +0900
Asahi Lina <lina@asahilina.net> wrote:

> Hi,
> 
> On 2/3/25 6:21 PM, Boris Brezillon wrote:
> > +Akash with whom we've been discussing adding a 'REPEAT' mode to
> > drm_gpuvm/panthor.
> > 
> > On Sun, 2 Feb 2025 19:53:47 +0100
> > Danilo Krummrich <dakr@kernel.org> wrote:
> >   
> >> Hi Lina,
> >>
> >> On Sun, Feb 02, 2025 at 10:34:49PM +0900, Asahi Lina wrote:  
> >>> Some hardware requires dummy page mappings to efficiently implement
> >>> Vulkan sparse features. These mappings consist of the same physical
> >>> memory page, repeated for a large range of address space (e.g. 16GiB).
> >>>
> >>> Add support for this to drm_gpuvm. Currently, drm_gpuvm expects BO
> >>> ranges to correspond 1:1 to virtual memory ranges that are mapped, and
> >>> does math on the BO offset accordingly. To make single page mappings
> >>> work, we need a way to turn off that math, keeping the BO offset always
> >>> constant and pointing to the same page (typically BO offset 0).
> >>>
> >>> To make this work, we need to handle all the corner cases when these
> >>> mappings intersect with regular mappings. The rules are simply to never
> >>> mix or merge a "regular" mapping with a single page mapping.
> >>>
> >>> drm_gpuvm has support for a flags field in drm_gpuva objects. This is
> >>> normally managed by drivers directly. We can introduce a
> >>> DRM_GPUVA_SINGLE_PAGE flag to handle this. However, to make it work,
> >>> sm_map and friends need to know ahead of time whether the new mapping is
> >>> a single page mapping or not. Therefore, we need to add an argument to
> >>> these functions so drivers can provide the flags to be filled into
> >>> drm_gpuva.flags.
> >>>
> >>> These changes should not affect any existing drivers that use drm_gpuvm
> >>> other than the API change:
> >>>
> >>> - imagination: Does not use flags at all
> >>> - nouveau: Only uses drm_gpuva_invalidate(), which is only called on
> >>>   existing drm_gpuva objects (after the map steps)
> >>> - panthor: Does not use flags at all
> >>> - xe: Does not use drm_gpuva_init_from_op() or
> >>>   drm_gpuva_remap()/drm_gpuva_map() (which call it). This means that the
> >>> flags field of the gpuva object is managed by the driver only, so these
> >>> changes cannot clobber it.
> >>>
> >>> Note that the way this is implemented, drm_gpuvm does not need to know
> >>> the GPU page size. It only has to never do math on the BO offset to meet
> >>> the requirements.
> >>>
> >>> I suspect that after this change there could be some cleanup possible in
> >>> the xe driver (which right now passes flags around in various
> >>> driver-specific ways from the map step through to drm_gpuva objects),
> >>> but I'll leave that to the Xe folks.
> >>>
> >>> Signed-off-by: Asahi Lina <lina@asahilina.net>
> >>> ---
> >>> Asahi Lina (4):
> >>>       drm/gpuvm: Add a flags argument to drm_gpuvm_sm_map[_*]
> >>>       drm/gpuvm: Plumb through flags into drm_gpuva_op_map
> >>>       drm/gpuvm: Add DRM_GPUVA_SINGLE_PAGE flag and logic
> >>>       drm/gpuvm: Plumb through flags into drm_gpuva_init    
> >>
> >> Without looking into any details yet:
> >>
> >> This is a bit of tricky one, since we're not even close to having a user for
> >> this new feature upstream yet, are we?  
> > 
> > Actually, we would be interesting in having this feature hooked up in
> > panthor. One use case we have is Vulkan sparse bindings, of course. But
> > we also have cases where we need to map a dummy page repeatedly on the
> > FW side. The approach we've been considering is slightly different:
> > pass a DRM_GPUVA_REPEAT_FLAG along with GEM range, so we can repeat a
> > range of the GEM (see the below diff, which is completely untested by
> > the way), but I think we'd be fine with this SINGLE_PAGE flag.  
> 
> That sounds similar, though your patch does not handle gpuva
> splitting/remapping and all the other corner cases.

Indeed, I didn't really consider the remapping could be in the middle
of a repeated region, and I see how it complicates things.

> I think you'll find
> that once you handle those, the logic will become significantly more
> complicated, since you need to start storing the start offset within the
> repeat range on GPUVAs to be able to split them while keeping the
> mappings identical, and do modular arithmetic to keep it all consistent
> across all the corner cases.
> 
> If SINGLE_PAGE works for you then I would advocate for that.

I'm perfectly fine with that.

> It keeps
> complexity down to a minimum in drm_gpuvm. You can still have a range
> that's greater than one page in practice, you'd just have to handle it
> driver-internal and pass the desired range out of band as a flag or
> other field. For example, you could decide that the mapping is always
> congruent to the VA (GEM page offset = start offset + VA % range) and
> always treat SINGLE_PAGE mappings like that when you actually set up the
> page tables, or pass in an extra offset to be able to shift the phase of
> the mapping to whatever you want. You just need to ensure that, if you
> mix range sizes or other configuration, you don't do that for the same
> GEM BO at the same offset, so that the drm_gpuvm core does not wrongly
> consider them equivalent.
> 
> Maybe I should rename SINGLE_PAGE to something else, since it isn't
> technically limited to that as far as gpuvm is concerned. Something like
> FIXED_OFFSET?

FWIW, I think I prefer SINGLE_PAGE or REPEAT over FIXED_OFFSET. I mean,
the documentation should clear any confusion, but I like when names are
obvious enough that people can guess their purpose without having to go
read the doc, and I don't think FIXED_OFFSET is clear enough in this
regard.