Nouveau - Oct 2023 - [PATCH drm-misc-next v7 4/7] drm/gpuvm: add an abstraction for a VM / BO combination

Add an abstraction layer between the drm_gpuva mappings of a particular
drm_gem_object and this GEM object itself. The abstraction represents a
combination of a drm_gem_object and drm_gpuvm. The drm_gem_object holds
a list of drm_gpuvm_bo structures (the structure representing this
abstraction), while each drm_gpuvm_bo contains list of mappings of this
GEM object.

This has multiple advantages:

1) We can use the drm_gpuvm_bo structure to attach it to various lists
   of the drm_gpuvm. This is useful for tracking external and evicted
   objects per VM, which is introduced in subsequent patches.

2) Finding mappings of a certain drm_gem_object mapped in a certain
   drm_gpuvm becomes much cheaper.

3) Drivers can derive and extend the structure to easily represent
   driver specific states of a BO for a certain GPUVM.

The idea of this abstraction was taken from amdgpu, hence the credit for
this idea goes to the developers of amdgpu.

Cc: Christian K?nig <christian.koenig at amd.com>
Signed-off-by: Danilo Krummrich <dakr at redhat.com>
---
 drivers/gpu/drm/drm_gpuvm.c            | 335 +++++++++++++++++++++----
 drivers/gpu/drm/nouveau/nouveau_uvmm.c |  64 +++--
 include/drm/drm_gem.h                  |  32 +--
 include/drm/drm_gpuvm.h                | 188 +++++++++++++-
 4 files changed, 533 insertions(+), 86 deletions(-)

diff --git a/drivers/gpu/drm/drm_gpuvm.c b/drivers/gpu/drm/drm_gpuvm.c
index c03332883432..7f4f5919f84c 100644
--- a/drivers/gpu/drm/drm_gpuvm.c
+++ b/drivers/gpu/drm/drm_gpuvm.c
@@ -70,6 +70,18 @@
  * &drm_gem_object, such as the &drm_gem_object containing the root
page table,
  * but it can also be a 'dummy' object, which can be allocated with
  * drm_gpuvm_resv_object_alloc().
+ *
+ * In order to connect a struct drm_gpuva its backing &drm_gem_object each
+ * &drm_gem_object maintains a list of &drm_gpuvm_bo structures, and
each
+ * &drm_gpuvm_bo contains a list of &drm_gpuva structures.
+ *
+ * A &drm_gpuvm_bo is an abstraction that represents a combination of a
+ * &drm_gpuvm and a &drm_gem_object. Every such combination should be
unique.
+ * This is ensured by the API through drm_gpuvm_bo_obtain() and
+ * drm_gpuvm_bo_obtain_prealloc() which first look into the corresponding
+ * &drm_gem_object list of &drm_gpuvm_bos for an existing instance of
this
+ * particular combination. If not existent a new instance is created and linked
+ * to the &drm_gem_object.
  */
 
 /**
@@ -395,21 +407,28 @@
 /**
  * DOC: Locking
  *
- * Generally, the GPU VA manager does not take care of locking itself, it is
- * the drivers responsibility to take care about locking. Drivers might want to
- * protect the following operations: inserting, removing and iterating
- * &drm_gpuva objects as well as generating all kinds of operations, such
as
- * split / merge or prefetch.
- *
- * The GPU VA manager also does not take care of the locking of the backing
- * &drm_gem_object buffers GPU VA lists by itself; drivers are responsible
to
- * enforce mutual exclusion using either the GEMs dma_resv lock or
alternatively
- * a driver specific external lock. For the latter see also
- * drm_gem_gpuva_set_lock().
- *
- * However, the GPU VA manager contains lockdep checks to ensure callers of its
- * API hold the corresponding lock whenever the &drm_gem_objects GPU VA
list is
- * accessed by functions such as drm_gpuva_link() or drm_gpuva_unlink().
+ * In terms of managing &drm_gpuva entries DRM GPUVM does not take care of
+ * locking itself, it is the drivers responsibility to take care about locking.
+ * Drivers might want to protect the following operations: inserting, removing
+ * and iterating &drm_gpuva objects as well as generating all kinds of
+ * operations, such as split / merge or prefetch.
+ *
+ * DRM GPUVM also does not take care of the locking of the backing
+ * &drm_gem_object buffers GPU VA lists and &drm_gpuvm_bo abstractions
by
+ * itself; drivers are responsible to enforce mutual exclusion using either the
+ * GEMs dma_resv lock or alternatively a driver specific external lock. For the
+ * latter see also drm_gem_gpuva_set_lock().
+ *
+ * However, DRM GPUVM contains lockdep checks to ensure callers of its API hold
+ * the corresponding lock whenever the &drm_gem_objects GPU VA list is
accessed
+ * by functions such as drm_gpuva_link() or drm_gpuva_unlink(), but also
+ * drm_gpuvm_bo_obtain() and drm_gpuvm_bo_put().
+ *
+ * The latter is required since on creation and destruction of a
&drm_gpuvm_bo
+ * the &drm_gpuvm_bo is attached / removed from the &drm_gem_objects
gpuva list.
+ * Subsequent calls to drm_gpuvm_bo_obtain() for the same &drm_gpuvm and
+ * &drm_gem_object must be able to observe previous creations and
destructions
+ * of &drm_gpuvm_bos in order to keep instances unique.
  */
 
 /**
@@ -439,6 +458,7 @@
  *	{
  *		struct drm_gpuva_ops *ops;
  *		struct drm_gpuva_op *op
+ *		struct drm_gpuvm_bo *vm_bo;
  *
  *		driver_lock_va_space();
  *		ops = drm_gpuvm_sm_map_ops_create(gpuvm, addr, range,
@@ -446,6 +466,10 @@
  *		if (IS_ERR(ops))
  *			return PTR_ERR(ops);
  *
+ *		vm_bo = drm_gpuvm_bo_obtain(gpuvm, obj);
+ *		if (IS_ERR(vm_bo))
+ *			return PTR_ERR(vm_bo);
+ *
  *		drm_gpuva_for_each_op(op, ops) {
  *			struct drm_gpuva *va;
  *
@@ -458,7 +482,7 @@
  *
  *				driver_vm_map();
  *				drm_gpuva_map(gpuvm, va, &op->map);
- *				drm_gpuva_link(va);
+ *				drm_gpuva_link(va, vm_bo);
  *
  *				break;
  *			case DRM_GPUVA_OP_REMAP: {
@@ -485,11 +509,11 @@
  *				driver_vm_remap();
  *				drm_gpuva_remap(prev, next, &op->remap);
  *
- *				drm_gpuva_unlink(va);
  *				if (prev)
- *					drm_gpuva_link(prev);
+ *					drm_gpuva_link(prev, va->vm_bo);
  *				if (next)
- *					drm_gpuva_link(next);
+ *					drm_gpuva_link(next, va->vm_bo);
+ *				drm_gpuva_unlink(va);
  *
  *				break;
  *			}
@@ -505,6 +529,7 @@
  *				break;
  *			}
  *		}
+ *		drm_gpuvm_bo_put(vm_bo);
  *		driver_unlock_va_space();
  *
  *		return 0;
@@ -514,6 +539,7 @@
  *
  *	struct driver_context {
  *		struct drm_gpuvm *gpuvm;
+ *		struct drm_gpuvm_bo *vm_bo;
  *		struct drm_gpuva *new_va;
  *		struct drm_gpuva *prev_va;
  *		struct drm_gpuva *next_va;
@@ -534,6 +560,7 @@
  *				  struct drm_gem_object *obj, u64 offset)
  *	{
  *		struct driver_context ctx;
+ *		struct drm_gpuvm_bo *vm_bo;
  *		struct drm_gpuva_ops *ops;
  *		struct drm_gpuva_op *op;
  *		int ret = 0;
@@ -543,16 +570,23 @@
  *		ctx.new_va = kzalloc(sizeof(*ctx.new_va), GFP_KERNEL);
  *		ctx.prev_va = kzalloc(sizeof(*ctx.prev_va), GFP_KERNEL);
  *		ctx.next_va = kzalloc(sizeof(*ctx.next_va), GFP_KERNEL);
- *		if (!ctx.new_va || !ctx.prev_va || !ctx.next_va) {
+ *		ctx.vm_bo = drm_gpuvm_bo_create(gpuvm, obj);
+ *		if (!ctx.new_va || !ctx.prev_va || !ctx.next_va || !vm_bo) {
  *			ret = -ENOMEM;
  *			goto out;
  *		}
  *
+ *		// Typically protected with a driver specific GEM gpuva lock
+ *		// used in the fence signaling path for drm_gpuva_link() and
+ *		// drm_gpuva_unlink(), hence pre-allocate.
+ *		ctx.vm_bo = drm_gpuvm_bo_obtain_prealloc(ctx.vm_bo);
+ *
  *		driver_lock_va_space();
  *		ret = drm_gpuvm_sm_map(gpuvm, &ctx, addr, range, obj, offset);
  *		driver_unlock_va_space();
  *
  *	out:
+ *		drm_gpuvm_bo_put(ctx.vm_bo);
  *		kfree(ctx.new_va);
  *		kfree(ctx.prev_va);
  *		kfree(ctx.next_va);
@@ -565,7 +599,7 @@
  *
  *		drm_gpuva_map(ctx->vm, ctx->new_va, &op->map);
  *
- *		drm_gpuva_link(ctx->new_va);
+ *		drm_gpuva_link(ctx->new_va, ctx->vm_bo);
  *
  *		// prevent the new GPUVA from being freed in
  *		// driver_mapping_create()
@@ -577,22 +611,23 @@
  *	int driver_gpuva_remap(struct drm_gpuva_op *op, void *__ctx)
  *	{
  *		struct driver_context *ctx = __ctx;
+ *		struct drm_gpuva *va = op->remap.unmap->va;
  *
  *		drm_gpuva_remap(ctx->prev_va, ctx->next_va, &op->remap);
  *
- *		drm_gpuva_unlink(op->remap.unmap->va);
- *		kfree(op->remap.unmap->va);
- *
  *		if (op->remap.prev) {
- *			drm_gpuva_link(ctx->prev_va);
+ *			drm_gpuva_link(ctx->prev_va, va->vm_bo);
  *			ctx->prev_va = NULL;
  *		}
  *
  *		if (op->remap.next) {
- *			drm_gpuva_link(ctx->next_va);
+ *			drm_gpuva_link(ctx->next_va, va->vm_bo);
  *			ctx->next_va = NULL;
  *		}
  *
+ *		drm_gpuva_unlink(va);
+ *		kfree(va);
+ *
  *		return 0;
  *	}
  *
@@ -774,6 +809,194 @@ drm_gpuvm_destroy(struct drm_gpuvm *gpuvm)
 }
 EXPORT_SYMBOL_GPL(drm_gpuvm_destroy);
 
+/**
+ * drm_gpuvm_bo_create() - create a new instance of struct drm_gpuvm_bo
+ * @gpuvm: The &drm_gpuvm the @obj is mapped in.
+ * @obj: The &drm_gem_object being mapped in the @gpuvm.
+ *
+ * If provided by the driver, this function uses the &drm_gpuvm_ops
+ * vm_bo_alloc() callback to allocate.
+ *
+ * Returns: a pointer to the &drm_gpuvm_bo on success, NULL on failure
+ */
+struct drm_gpuvm_bo *
+drm_gpuvm_bo_create(struct drm_gpuvm *gpuvm,
+		    struct drm_gem_object *obj)
+{
+	const struct drm_gpuvm_ops *ops = gpuvm->ops;
+	struct drm_gpuvm_bo *vm_bo;
+
+	if (ops && ops->vm_bo_alloc)
+		vm_bo = ops->vm_bo_alloc();
+	else
+		vm_bo = kzalloc(sizeof(*vm_bo), GFP_KERNEL);
+
+	if (unlikely(!vm_bo))
+		return NULL;
+
+	vm_bo->vm = gpuvm;
+	vm_bo->obj = obj;
+	drm_gem_object_get(obj);
+
+	kref_init(&vm_bo->kref);
+	INIT_LIST_HEAD(&vm_bo->list.gpuva);
+	INIT_LIST_HEAD(&vm_bo->list.entry.gem);
+
+	return vm_bo;
+}
+EXPORT_SYMBOL_GPL(drm_gpuvm_bo_create);
+
+static void
+drm_gpuvm_bo_destroy(struct kref *kref)
+{
+	struct drm_gpuvm_bo *vm_bo = container_of(kref, struct drm_gpuvm_bo,
+						  kref);
+	struct drm_gpuvm *gpuvm = vm_bo->vm;
+	const struct drm_gpuvm_ops *ops = gpuvm->ops;
+	struct drm_gem_object *obj = vm_bo->obj;
+	bool lock = !drm_gpuvm_resv_protected(gpuvm);
+
+	if (!lock)
+		drm_gpuvm_resv_assert_held(gpuvm);
+
+	drm_gem_gpuva_assert_lock_held(obj);
+	list_del(&vm_bo->list.entry.gem);
+
+	if (ops && ops->vm_bo_free)
+		ops->vm_bo_free(vm_bo);
+	else
+		kfree(vm_bo);
+
+	drm_gem_object_put(obj);
+}
+
+/**
+ * drm_gpuvm_bo_put() - drop a struct drm_gpuvm_bo reference
+ * @vm_bo: the &drm_gpuvm_bo to release the reference of
+ *
+ * This releases a reference to @vm_bo.
+ *
+ * If the reference count drops to zero, the &gpuvm_bo is destroyed, which
+ * includes removing it from the GEMs gpuva list. Hence, if a call to this
+ * function can potentially let the reference count to zero the caller must
+ * hold the dma-resv or driver specific GEM gpuva lock.
+ */
+void
+drm_gpuvm_bo_put(struct drm_gpuvm_bo *vm_bo)
+{
+	if (vm_bo)
+		kref_put(&vm_bo->kref, drm_gpuvm_bo_destroy);
+}
+EXPORT_SYMBOL_GPL(drm_gpuvm_bo_put);
+
+static struct drm_gpuvm_bo *
+__drm_gpuvm_bo_find(struct drm_gpuvm *gpuvm,
+		    struct drm_gem_object *obj)
+{
+	struct drm_gpuvm_bo *vm_bo;
+
+	drm_gem_gpuva_assert_lock_held(obj);
+	drm_gem_for_each_gpuvm_bo(vm_bo, obj)
+		if (vm_bo->vm == gpuvm)
+			return vm_bo;
+
+	return NULL;
+}
+
+/**
+ * drm_gpuvm_bo_find() - find the &drm_gpuvm_bo for the given
+ * &drm_gpuvm and &drm_gem_object
+ * @gpuvm: The &drm_gpuvm the @obj is mapped in.
+ * @obj: The &drm_gem_object being mapped in the @gpuvm.
+ *
+ * Find the &drm_gpuvm_bo representing the combination of the given
+ * &drm_gpuvm and &drm_gem_object. If found, increases the reference
+ * count of the &drm_gpuvm_bo accordingly.
+ *
+ * Returns: a pointer to the &drm_gpuvm_bo on success, NULL on failure
+ */
+struct drm_gpuvm_bo *
+drm_gpuvm_bo_find(struct drm_gpuvm *gpuvm,
+		  struct drm_gem_object *obj)
+{
+	struct drm_gpuvm_bo *vm_bo = __drm_gpuvm_bo_find(gpuvm, obj);
+
+	return vm_bo ? drm_gpuvm_bo_get(vm_bo) : NULL;
+}
+EXPORT_SYMBOL_GPL(drm_gpuvm_bo_find);
+
+/**
+ * drm_gpuvm_bo_obtain() - obtains and instance of the &drm_gpuvm_bo for
the
+ * given &drm_gpuvm and &drm_gem_object
+ * @gpuvm: The &drm_gpuvm the @obj is mapped in.
+ * @obj: The &drm_gem_object being mapped in the @gpuvm.
+ *
+ * Find the &drm_gpuvm_bo representing the combination of the given
+ * &drm_gpuvm and &drm_gem_object. If found, increases the reference
+ * count of the &drm_gpuvm_bo accordingly. If not found, allocates a new
+ * &drm_gpuvm_bo.
+ *
+ * A new &drm_gpuvm_bo is added to the GEMs gpuva list.
+ *
+ * Returns: a pointer to the &drm_gpuvm_bo on success, an ERR_PTR on
failure
+ */
+struct drm_gpuvm_bo *
+drm_gpuvm_bo_obtain(struct drm_gpuvm *gpuvm,
+		    struct drm_gem_object *obj)
+{
+	struct drm_gpuvm_bo *vm_bo;
+
+	vm_bo = drm_gpuvm_bo_find(gpuvm, obj);
+	if (vm_bo)
+		return vm_bo;
+
+	vm_bo = drm_gpuvm_bo_create(gpuvm, obj);
+	if (!vm_bo)
+		return ERR_PTR(-ENOMEM);
+
+	drm_gem_gpuva_assert_lock_held(obj);
+	list_add_tail(&vm_bo->list.entry.gem, &obj->gpuva.list);
+
+	return vm_bo;
+}
+EXPORT_SYMBOL_GPL(drm_gpuvm_bo_obtain);
+
+/**
+ * drm_gpuvm_bo_obtain_prealloc() - obtains and instance of the
&drm_gpuvm_bo
+ * for the given &drm_gpuvm and &drm_gem_object
+ * @__vm_bo: A pre-allocated struct drm_gpuvm_bo.
+ *
+ * Find the &drm_gpuvm_bo representing the combination of the given
+ * &drm_gpuvm and &drm_gem_object. If found, increases the reference
+ * count of the found &drm_gpuvm_bo accordingly, while the @__vm_bo
reference
+ * count is decreased. If not found @__vm_bo is returned without further
+ * increase of the reference count.
+ *
+ * A new &drm_gpuvm_bo is added to the GEMs gpuva list.
+ *
+ * Returns: a pointer to the found &drm_gpuvm_bo or @__vm_bo if no existing
+ * &drm_gpuvm_bo was found
+ */
+struct drm_gpuvm_bo *
+drm_gpuvm_bo_obtain_prealloc(struct drm_gpuvm_bo *__vm_bo)
+{
+	struct drm_gpuvm *gpuvm = __vm_bo->vm;
+	struct drm_gem_object *obj = __vm_bo->obj;
+	struct drm_gpuvm_bo *vm_bo;
+
+	vm_bo = drm_gpuvm_bo_find(gpuvm, obj);
+	if (vm_bo) {
+		drm_gpuvm_bo_put(__vm_bo);
+		return vm_bo;
+	}
+
+	drm_gem_gpuva_assert_lock_held(obj);
+	list_add_tail(&__vm_bo->list.entry.gem, &obj->gpuva.list);
+
+	return __vm_bo;
+}
+EXPORT_SYMBOL_GPL(drm_gpuvm_bo_obtain_prealloc);
+
 static int
 __drm_gpuva_insert(struct drm_gpuvm *gpuvm,
 		   struct drm_gpuva *va)
@@ -864,24 +1087,33 @@ EXPORT_SYMBOL_GPL(drm_gpuva_remove);
 /**
  * drm_gpuva_link() - link a &drm_gpuva
  * @va: the &drm_gpuva to link
+ * @vm_bo: the &drm_gpuvm_bo to add the &drm_gpuva to
  *
- * This adds the given &va to the GPU VA list of the &drm_gem_object it
is
- * associated with.
+ * This adds the given &va to the GPU VA list of the &drm_gpuvm_bo and
the
+ * &drm_gpuvm_bo to the &drm_gem_object it is associated with.
+ *
+ * For every &drm_gpuva entry added to the &drm_gpuvm_bo an additional
+ * reference of the latter is taken.
  *
  * This function expects the caller to protect the GEM's GPUVA list against
- * concurrent access using the GEMs dma_resv lock.
+ * concurrent access using either the GEMs dma_resv lock or a driver specific
+ * lock set through drm_gem_gpuva_set_lock().
  */
 void
-drm_gpuva_link(struct drm_gpuva *va)
+drm_gpuva_link(struct drm_gpuva *va, struct drm_gpuvm_bo *vm_bo)
 {
 	struct drm_gem_object *obj = va->gem.obj;
+	struct drm_gpuvm *gpuvm = va->vm;
 
 	if (unlikely(!obj))
 		return;
 
-	drm_gem_gpuva_assert_lock_held(obj);
+	drm_WARN_ON(gpuvm->drm, obj != vm_bo->obj);
 
-	list_add_tail(&va->gem.entry, &obj->gpuva.list);
+	va->vm_bo = drm_gpuvm_bo_get(vm_bo);
+
+	drm_gem_gpuva_assert_lock_held(obj);
+	list_add_tail(&va->gem.entry, &vm_bo->list.gpuva);
 }
 EXPORT_SYMBOL_GPL(drm_gpuva_link);
 
@@ -892,20 +1124,31 @@ EXPORT_SYMBOL_GPL(drm_gpuva_link);
  * This removes the given &va from the GPU VA list of the
&drm_gem_object it is
  * associated with.
  *
+ * This removes the given &va from the GPU VA list of the &drm_gpuvm_bo
and
+ * the &drm_gpuvm_bo from the &drm_gem_object it is associated with in
case
+ * this call unlinks the last &drm_gpuva from the &drm_gpuvm_bo.
+ *
+ * For every &drm_gpuva entry removed from the &drm_gpuvm_bo a
reference of
+ * the latter is dropped.
+ *
  * This function expects the caller to protect the GEM's GPUVA list against
- * concurrent access using the GEMs dma_resv lock.
+ * concurrent access using either the GEMs dma_resv lock or a driver specific
+ * lock set through drm_gem_gpuva_set_lock().
  */
 void
 drm_gpuva_unlink(struct drm_gpuva *va)
 {
 	struct drm_gem_object *obj = va->gem.obj;
+	struct drm_gpuvm_bo *vm_bo = va->vm_bo;
 
 	if (unlikely(!obj))
 		return;
 
 	drm_gem_gpuva_assert_lock_held(obj);
-
 	list_del_init(&va->gem.entry);
+
+	va->vm_bo = NULL;
+	drm_gpuvm_bo_put(vm_bo);
 }
 EXPORT_SYMBOL_GPL(drm_gpuva_unlink);
 
@@ -1050,10 +1293,10 @@ drm_gpuva_remap(struct drm_gpuva *prev,
 		struct drm_gpuva *next,
 		struct drm_gpuva_op_remap *op)
 {
-	struct drm_gpuva *curr = op->unmap->va;
-	struct drm_gpuvm *gpuvm = curr->vm;
+	struct drm_gpuva *va = op->unmap->va;
+	struct drm_gpuvm *gpuvm = va->vm;
 
-	drm_gpuva_remove(curr);
+	drm_gpuva_remove(va);
 
 	if (op->prev) {
 		drm_gpuva_init_from_op(prev, op->prev);
@@ -1695,9 +1938,8 @@ drm_gpuvm_prefetch_ops_create(struct drm_gpuvm *gpuvm,
 EXPORT_SYMBOL_GPL(drm_gpuvm_prefetch_ops_create);
 
 /**
- * drm_gpuvm_gem_unmap_ops_create() - creates the &drm_gpuva_ops to unmap a
GEM
- * @gpuvm: the &drm_gpuvm representing the GPU VA space
- * @obj: the &drm_gem_object to unmap
+ * drm_gpuvm_bo_unmap_ops_create() - creates the &drm_gpuva_ops to unmap a
GEM
+ * @vm_bo: the &drm_gpuvm_bo abstraction
  *
  * This function creates a list of operations to perform unmapping for every
  * GPUVA attached to a GEM.
@@ -1714,15 +1956,14 @@ EXPORT_SYMBOL_GPL(drm_gpuvm_prefetch_ops_create);
  * Returns: a pointer to the &drm_gpuva_ops on success, an ERR_PTR on
failure
  */
 struct drm_gpuva_ops *
-drm_gpuvm_gem_unmap_ops_create(struct drm_gpuvm *gpuvm,
-			       struct drm_gem_object *obj)
+drm_gpuvm_bo_unmap_ops_create(struct drm_gpuvm_bo *vm_bo)
 {
 	struct drm_gpuva_ops *ops;
 	struct drm_gpuva_op *op;
 	struct drm_gpuva *va;
 	int ret;
 
-	drm_gem_gpuva_assert_lock_held(obj);
+	drm_gem_gpuva_assert_lock_held(vm_bo->obj);
 
 	ops = kzalloc(sizeof(*ops), GFP_KERNEL);
 	if (!ops)
@@ -1730,8 +1971,8 @@ drm_gpuvm_gem_unmap_ops_create(struct drm_gpuvm *gpuvm,
 
 	INIT_LIST_HEAD(&ops->list);
 
-	drm_gem_for_each_gpuva(va, obj) {
-		op = gpuva_op_alloc(gpuvm);
+	drm_gpuvm_bo_for_each_va(va, vm_bo) {
+		op = gpuva_op_alloc(vm_bo->vm);
 		if (!op) {
 			ret = -ENOMEM;
 			goto err_free_ops;
@@ -1745,10 +1986,10 @@ drm_gpuvm_gem_unmap_ops_create(struct drm_gpuvm *gpuvm,
 	return ops;
 
 err_free_ops:
-	drm_gpuva_ops_free(gpuvm, ops);
+	drm_gpuva_ops_free(vm_bo->vm, ops);
 	return ERR_PTR(ret);
 }
-EXPORT_SYMBOL_GPL(drm_gpuvm_gem_unmap_ops_create);
+EXPORT_SYMBOL_GPL(drm_gpuvm_bo_unmap_ops_create);
 
 /**
  * drm_gpuva_ops_free() - free the given &drm_gpuva_ops
diff --git a/drivers/gpu/drm/nouveau/nouveau_uvmm.c
b/drivers/gpu/drm/nouveau/nouveau_uvmm.c
index ed439bf4032f..1e95b0a1b047 100644
--- a/drivers/gpu/drm/nouveau/nouveau_uvmm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_uvmm.c
@@ -62,6 +62,8 @@ struct bind_job_op {
 	enum vm_bind_op op;
 	u32 flags;
 
+	struct drm_gpuvm_bo *vm_bo;
+
 	struct {
 		u64 addr;
 		u64 range;
@@ -1113,22 +1115,28 @@ bind_validate_region(struct nouveau_job *job)
 }
 
 static void
-bind_link_gpuvas(struct drm_gpuva_ops *ops, struct nouveau_uvma_prealloc *new)
+bind_link_gpuvas(struct bind_job_op *bop)
 {
+	struct nouveau_uvma_prealloc *new = &bop->new;
+	struct drm_gpuvm_bo *vm_bo = bop->vm_bo;
+	struct drm_gpuva_ops *ops = bop->ops;
 	struct drm_gpuva_op *op;
 
 	drm_gpuva_for_each_op(op, ops) {
 		switch (op->op) {
 		case DRM_GPUVA_OP_MAP:
-			drm_gpuva_link(&new->map->va);
+			drm_gpuva_link(&new->map->va, vm_bo);
 			break;
-		case DRM_GPUVA_OP_REMAP:
+		case DRM_GPUVA_OP_REMAP: {
+			struct drm_gpuva *va = op->remap.unmap->va;
+
 			if (op->remap.prev)
-				drm_gpuva_link(&new->prev->va);
+				drm_gpuva_link(&new->prev->va, va->vm_bo);
 			if (op->remap.next)
-				drm_gpuva_link(&new->next->va);
-			drm_gpuva_unlink(op->remap.unmap->va);
+				drm_gpuva_link(&new->next->va, va->vm_bo);
+			drm_gpuva_unlink(va);
 			break;
+		}
 		case DRM_GPUVA_OP_UNMAP:
 			drm_gpuva_unlink(op->unmap.va);
 			break;
@@ -1150,10 +1158,18 @@ nouveau_uvmm_bind_job_submit(struct nouveau_job *job)
 
 	list_for_each_op(op, &bind_job->ops) {
 		if (op->op == OP_MAP) {
-			op->gem.obj = drm_gem_object_lookup(job->file_priv,
-							    op->gem.handle);
-			if (!op->gem.obj)
+			struct drm_gem_object *obj;
+
+			obj = drm_gem_object_lookup(job->file_priv,
+						    op->gem.handle);
+			if (!(op->gem.obj = obj))
 				return -ENOENT;
+
+			dma_resv_lock(obj->resv, NULL);
+			op->vm_bo = drm_gpuvm_bo_obtain(&uvmm->base, obj);
+			dma_resv_unlock(obj->resv);
+			if (IS_ERR(op->vm_bo))
+				return PTR_ERR(op->vm_bo);
 		}
 
 		ret = bind_validate_op(job, op);
@@ -1364,7 +1380,7 @@ nouveau_uvmm_bind_job_submit(struct nouveau_job *job)
 		case OP_UNMAP_SPARSE:
 		case OP_MAP:
 		case OP_UNMAP:
-			bind_link_gpuvas(op->ops, &op->new);
+			bind_link_gpuvas(op);
 			break;
 		default:
 			break;
@@ -1511,6 +1527,12 @@ nouveau_uvmm_bind_job_free_work_fn(struct work_struct
*work)
 		if (!IS_ERR_OR_NULL(op->ops))
 			drm_gpuva_ops_free(&uvmm->base, op->ops);
 
+		if (!IS_ERR_OR_NULL(op->vm_bo)) {
+			dma_resv_lock(obj->resv, NULL);
+			drm_gpuvm_bo_put(op->vm_bo);
+			dma_resv_unlock(obj->resv);
+		}
+
 		if (obj)
 			drm_gem_object_put(obj);
 	}
@@ -1776,15 +1798,18 @@ void
 nouveau_uvmm_bo_map_all(struct nouveau_bo *nvbo, struct nouveau_mem *mem)
 {
 	struct drm_gem_object *obj = &nvbo->bo.base;
+	struct drm_gpuvm_bo *vm_bo;
 	struct drm_gpuva *va;
 
 	dma_resv_assert_held(obj->resv);
 
-	drm_gem_for_each_gpuva(va, obj) {
-		struct nouveau_uvma *uvma = uvma_from_va(va);
+	drm_gem_for_each_gpuvm_bo(vm_bo, obj) {
+		drm_gpuvm_bo_for_each_va(va, vm_bo) {
+			struct nouveau_uvma *uvma = uvma_from_va(va);
 
-		nouveau_uvma_map(uvma, mem);
-		drm_gpuva_invalidate(va, false);
+			nouveau_uvma_map(uvma, mem);
+			drm_gpuva_invalidate(va, false);
+		}
 	}
 }
 
@@ -1792,15 +1817,18 @@ void
 nouveau_uvmm_bo_unmap_all(struct nouveau_bo *nvbo)
 {
 	struct drm_gem_object *obj = &nvbo->bo.base;
+	struct drm_gpuvm_bo *vm_bo;
 	struct drm_gpuva *va;
 
 	dma_resv_assert_held(obj->resv);
 
-	drm_gem_for_each_gpuva(va, obj) {
-		struct nouveau_uvma *uvma = uvma_from_va(va);
+	drm_gem_for_each_gpuvm_bo(vm_bo, obj) {
+		drm_gpuvm_bo_for_each_va(va, vm_bo) {
+			struct nouveau_uvma *uvma = uvma_from_va(va);
 
-		nouveau_uvma_unmap(uvma);
-		drm_gpuva_invalidate(va, true);
+			nouveau_uvma_unmap(uvma);
+			drm_gpuva_invalidate(va, true);
+		}
 	}
 }
 
diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
index 16364487fde9..369505447acd 100644
--- a/include/drm/drm_gem.h
+++ b/include/drm/drm_gem.h
@@ -580,7 +580,7 @@ int drm_gem_evict(struct drm_gem_object *obj);
  * drm_gem_gpuva_init() - initialize the gpuva list of a GEM object
  * @obj: the &drm_gem_object
  *
- * This initializes the &drm_gem_object's &drm_gpuva list.
+ * This initializes the &drm_gem_object's &drm_gpuvm_bo list.
  *
  * Calling this function is only necessary for drivers intending to support the
  * &drm_driver_feature DRIVER_GEM_GPUVA.
@@ -593,28 +593,28 @@ static inline void drm_gem_gpuva_init(struct
drm_gem_object *obj)
 }
 
 /**
- * drm_gem_for_each_gpuva() - iternator to walk over a list of gpuvas
- * @entry__: &drm_gpuva structure to assign to in each iteration step
- * @obj__: the &drm_gem_object the &drm_gpuvas to walk are associated
with
+ * drm_gem_for_each_gpuvm_bo() - iterator to walk over a list of
&drm_gpuvm_bo
+ * @entry__: &drm_gpuvm_bo structure to assign to in each iteration step
+ * @obj__: the &drm_gem_object the &drm_gpuvm_bo to walk are associated
with
  *
- * This iterator walks over all &drm_gpuva structures associated with the
- * &drm_gpuva_manager.
+ * This iterator walks over all &drm_gpuvm_bo structures associated with
the
+ * &drm_gem_object.
  */
-#define drm_gem_for_each_gpuva(entry__, obj__) \
-	list_for_each_entry(entry__, &(obj__)->gpuva.list, gem.entry)
+#define drm_gem_for_each_gpuvm_bo(entry__, obj__) \
+	list_for_each_entry(entry__, &(obj__)->gpuva.list, list.entry.gem)
 
 /**
- * drm_gem_for_each_gpuva_safe() - iternator to safely walk over a list of
- * gpuvas
- * @entry__: &drm_gpuva structure to assign to in each iteration step
- * @next__: &next &drm_gpuva to store the next step
- * @obj__: the &drm_gem_object the &drm_gpuvas to walk are associated
with
+ * drm_gem_for_each_gpuvm_bo_safe() - iterator to safely walk over a list of
+ * &drm_gpuvm_bo
+ * @entry__: &drm_gpuvm_bostructure to assign to in each iteration step
+ * @next__: &next &drm_gpuvm_bo to store the next step
+ * @obj__: the &drm_gem_object the &drm_gpuvm_bo to walk are associated
with
  *
- * This iterator walks over all &drm_gpuva structures associated with the
+ * This iterator walks over all &drm_gpuvm_bo structures associated with
the
  * &drm_gem_object. It is implemented with list_for_each_entry_safe(),
hence
  * it is save against removal of elements.
  */
-#define drm_gem_for_each_gpuva_safe(entry__, next__, obj__) \
-	list_for_each_entry_safe(entry__, next__, &(obj__)->gpuva.list,
gem.entry)
+#define drm_gem_for_each_gpuvm_bo_safe(entry__, next__, obj__) \
+	list_for_each_entry_safe(entry__, next__, &(obj__)->gpuva.list,
list.entry.gem)
 
 #endif /* __DRM_GEM_H__ */
diff --git a/include/drm/drm_gpuvm.h b/include/drm/drm_gpuvm.h
index 47cbacb244b9..466fdd76c71a 100644
--- a/include/drm/drm_gpuvm.h
+++ b/include/drm/drm_gpuvm.h
@@ -25,6 +25,7 @@
  * OTHER DEALINGS IN THE SOFTWARE.
  */
 
+#include <linux/dma-resv.h>
 #include <linux/list.h>
 #include <linux/rbtree.h>
 #include <linux/types.h>
@@ -33,6 +34,7 @@
 #include <drm/drm_gem.h>
 
 struct drm_gpuvm;
+struct drm_gpuvm_bo;
 struct drm_gpuvm_ops;
 
 /**
@@ -73,6 +75,12 @@ struct drm_gpuva {
 	 */
 	struct drm_gpuvm *vm;
 
+	/**
+	 * @vm_bo: the &drm_gpuvm_bo abstraction for the mapped
+	 * &drm_gem_object
+	 */
+	struct drm_gpuvm_bo *vm_bo;
+
 	/**
 	 * @flags: the &drm_gpuva_flags for this mapping
 	 */
@@ -108,7 +116,7 @@ struct drm_gpuva {
 		struct drm_gem_object *obj;
 
 		/**
-		 * @entry: the &list_head to attach this object to a &drm_gem_object
+		 * @entry: the &list_head to attach this object to a &drm_gpuvm_bo
 		 */
 		struct list_head entry;
 	} gem;
@@ -141,7 +149,7 @@ struct drm_gpuva {
 int drm_gpuva_insert(struct drm_gpuvm *gpuvm, struct drm_gpuva *va);
 void drm_gpuva_remove(struct drm_gpuva *va);
 
-void drm_gpuva_link(struct drm_gpuva *va);
+void drm_gpuva_link(struct drm_gpuva *va, struct drm_gpuvm_bo *vm_bo);
 void drm_gpuva_unlink(struct drm_gpuva *va);
 
 struct drm_gpuva *drm_gpuva_find(struct drm_gpuvm *gpuvm,
@@ -188,10 +196,16 @@ static inline bool drm_gpuva_invalidated(struct drm_gpuva
*va)
  * enum drm_gpuvm_flags - flags for struct drm_gpuvm
  */
 enum drm_gpuvm_flags {
+	/**
+	 * @DRM_GPUVM_RESV_PROTECTED: GPUVM is protected externally by the
+	 * GPUVM's &dma_resv lock
+	 */
+	DRM_GPUVM_RESV_PROTECTED = BIT(0),
+
 	/**
 	 * @DRM_GPUVM_USERBITS: user defined bits
 	 */
-	DRM_GPUVM_USERBITS = BIT(0),
+	DRM_GPUVM_USERBITS = BIT(1),
 };
 
 /**
@@ -280,6 +294,19 @@ bool drm_gpuvm_interval_empty(struct drm_gpuvm *gpuvm, u64
addr, u64 range);
 struct drm_gem_object *
 drm_gpuvm_resv_object_alloc(struct drm_device *drm);
 
+/**
+ * drm_gpuvm_resv_protected() - indicates whether &DRM_GPUVM_RESV_PROTECTED
is
+ * set
+ * @gpuvm: the &drm_gpuvm
+ *
+ * Returns: true if &DRM_GPUVM_RESV_PROTECTED is set, false otherwise.
+ */
+static inline bool
+drm_gpuvm_resv_protected(struct drm_gpuvm *gpuvm)
+{
+	return gpuvm->flags & DRM_GPUVM_RESV_PROTECTED;
+}
+
 /**
  * drm_gpuvm_resv() - returns the &drm_gpuvm's &dma_resv
  * @gpuvm__: the &drm_gpuvm
@@ -298,6 +325,12 @@ drm_gpuvm_resv_object_alloc(struct drm_device *drm);
  */
 #define drm_gpuvm_resv_obj(gpuvm__) ((gpuvm__)->r_obj)
 
+#define drm_gpuvm_resv_held(gpuvm__) \
+	dma_resv_held(drm_gpuvm_resv(gpuvm__))
+
+#define drm_gpuvm_resv_assert_held(gpuvm__) \
+	dma_resv_assert_held(drm_gpuvm_resv(gpuvm__))
+
 #define drm_gpuvm_resv_held(gpuvm__) \
 	dma_resv_held(drm_gpuvm_resv(gpuvm__))
 
@@ -382,6 +415,128 @@ __drm_gpuva_next(struct drm_gpuva *va)
 #define drm_gpuvm_for_each_va_safe(va__, next__, gpuvm__) \
 	list_for_each_entry_safe(va__, next__, &(gpuvm__)->rb.list, rb.entry)
 
+/**
+ * struct drm_gpuvm_bo - structure representing a &drm_gpuvm and
+ * &drm_gem_object combination
+ *
+ * This structure is an abstraction representing a &drm_gpuvm and
+ * &drm_gem_object combination. It serves as an indirection to accelerate
+ * iterating all &drm_gpuvas within a &drm_gpuvm backed by the same
+ * &drm_gem_object.
+ *
+ * Furthermore it is used cache evicted GEM objects for a certain GPU-VM to
+ * accelerate validation.
+ *
+ * Typically, drivers want to create an instance of a struct drm_gpuvm_bo once
+ * a GEM object is mapped first in a GPU-VM and release the instance once the
+ * last mapping of the GEM object in this GPU-VM is unmapped.
+ */
+struct drm_gpuvm_bo {
+	/**
+	 * @vm: The &drm_gpuvm the @obj is mapped in. This pointer is not
+	 * reference counted.
+	 *
+	 * A struct drm_gpuvm_bo is not allowed to out-live its &drm_gpuvm
+	 * context. Implicitly, this is ensured by the fact that the driver is
+	 * responsible to ensure the VM doesn't contain mappings once it's
+	 * freed, since a struct drm_gpuvm_bo should be freed once the last
+	 * mapping being backed by the corresponding buffer object is unmapped.
+	 */
+	struct drm_gpuvm *vm;
+
+	/**
+	 * @obj: The &drm_gem_object being mapped in @vm. This is a reference
+	 * counted pointer.
+	 */
+	struct drm_gem_object *obj;
+
+	/**
+	 * @kref: The reference count for this &drm_gpuvm_bo.
+	 */
+	struct kref kref;
+
+	/**
+	 * @list: Structure containing all &list_heads.
+	 */
+	struct {
+		/**
+		 * @gpuva: The list of linked &drm_gpuvas.
+		 */
+		struct list_head gpuva;
+
+		/**
+		 * @entry: Structure containing all &list_heads serving as
+		 * entry.
+		 */
+		struct {
+			/**
+			 * @gem: List entry to attach to the &drm_gem_objects
+			 * gpuva list.
+			 */
+			struct list_head gem;
+		} entry;
+	} list;
+};
+
+struct drm_gpuvm_bo *
+drm_gpuvm_bo_create(struct drm_gpuvm *gpuvm,
+		    struct drm_gem_object *obj);
+
+struct drm_gpuvm_bo *
+drm_gpuvm_bo_obtain(struct drm_gpuvm *gpuvm,
+		    struct drm_gem_object *obj);
+struct drm_gpuvm_bo *
+drm_gpuvm_bo_obtain_prealloc(struct drm_gpuvm_bo *vm_bo);
+
+/**
+ * drm_gpuvm_bo_get() - acquire a struct drm_gpuvm_bo reference
+ * @vm_bo: the &drm_gpuvm_bo to acquire the reference of
+ *
+ * This function acquires an additional reference to @vm_bo. It is illegal to
+ * call this without already holding a reference. No locks required.
+ */
+static inline struct drm_gpuvm_bo *
+drm_gpuvm_bo_get(struct drm_gpuvm_bo *vm_bo)
+{
+	kref_get(&vm_bo->kref);
+	return vm_bo;
+}
+
+void drm_gpuvm_bo_put(struct drm_gpuvm_bo *vm_bo);
+
+struct drm_gpuvm_bo *
+drm_gpuvm_bo_find(struct drm_gpuvm *gpuvm,
+		  struct drm_gem_object *obj);
+
+/**
+ * drm_gpuvm_bo_for_each_va() - iterator to walk over a list of &drm_gpuva
+ * @va__: &drm_gpuva structure to assign to in each iteration step
+ * @vm_bo__: the &drm_gpuvm_bo the &drm_gpuva to walk are associated
with
+ *
+ * This iterator walks over all &drm_gpuva structures associated with the
+ * &drm_gpuvm_bo.
+ *
+ * The caller must hold the GEM's gpuva lock.
+ */
+#define drm_gpuvm_bo_for_each_va(va__, vm_bo__) \
+	list_for_each_entry(va__, &(vm_bo)->list.gpuva, gem.entry)
+
+/**
+ * drm_gpuvm_bo_for_each_va_safe() - iterator to safely walk over a list of
+ * &drm_gpuva
+ * @va__: &drm_gpuva structure to assign to in each iteration step
+ * @next__: &next &drm_gpuva to store the next step
+ * @vm_bo__: the &drm_gpuvm_bo the &drm_gpuva to walk are associated
with
+ *
+ * This iterator walks over all &drm_gpuva structures associated with the
+ * &drm_gpuvm_bo. It is implemented with list_for_each_entry_safe(), hence
+ * it is save against removal of elements.
+ *
+ * The caller must hold the GEM's gpuva lock.
+ */
+#define drm_gpuvm_bo_for_each_va_safe(va__, next__, vm_bo__) \
+	list_for_each_entry_safe(va__, next__, &(vm_bo)->list.gpuva, gem.entry)
+
 /**
  * enum drm_gpuva_op_type - GPU VA operation type
  *
@@ -651,8 +806,7 @@ drm_gpuvm_prefetch_ops_create(struct drm_gpuvm *gpuvm,
 				 u64 addr, u64 range);
 
 struct drm_gpuva_ops *
-drm_gpuvm_gem_unmap_ops_create(struct drm_gpuvm *gpuvm,
-			       struct drm_gem_object *obj);
+drm_gpuvm_bo_unmap_ops_create(struct drm_gpuvm_bo *vm_bo);
 
 void drm_gpuva_ops_free(struct drm_gpuvm *gpuvm,
 			struct drm_gpuva_ops *ops);
@@ -696,6 +850,30 @@ struct drm_gpuvm_ops {
 	 */
 	void (*op_free)(struct drm_gpuva_op *op);
 
+	/**
+	 * @vm_bo_alloc: called when the &drm_gpuvm allocates
+	 * a struct drm_gpuvm_bo
+	 *
+	 * Some drivers may want to embed struct drm_gpuvm_bo into driver
+	 * specific structures. By implementing this callback drivers can
+	 * allocate memory accordingly.
+	 *
+	 * This callback is optional.
+	 */
+	struct drm_gpuvm_bo *(*vm_bo_alloc)(void);
+
+	/**
+	 * @vm_bo_free: called when the &drm_gpuvm frees a
+	 * struct drm_gpuvm_bo
+	 *
+	 * Some drivers may want to embed struct drm_gpuvm_bo into driver
+	 * specific structures. By implementing this callback drivers can
+	 * free the previously allocated memory accordingly.
+	 *
+	 * This callback is optional.
+	 */
+	void (*vm_bo_free)(struct drm_gpuvm_bo *vm_bo);
+
 	/**
 	 * @sm_step_map: called from &drm_gpuvm_sm_map to finally insert the
 	 * mapping once all previous steps were completed
-- 
2.41.0

On Mon, 2023-10-23 at 22:16 +0200, Danilo Krummrich
wrote:
> Add an abstraction layer between the drm_gpuva mappings of a
> particular
> drm_gem_object and this GEM object itself. The abstraction represents
> a
> combination of a drm_gem_object and drm_gpuvm. The drm_gem_object
> holds
> a list of drm_gpuvm_bo structures (the structure representing this
> abstraction), while each drm_gpuvm_bo contains list of mappings of
> this
> GEM object.
> 
> This has multiple advantages:
> 
> 1) We can use the drm_gpuvm_bo structure to attach it to various
> lists
> ?? of the drm_gpuvm. This is useful for tracking external and evicted
> ?? objects per VM, which is introduced in subsequent patches.
> 
> 2) Finding mappings of a certain drm_gem_object mapped in a certain
> ?? drm_gpuvm becomes much cheaper.
> 
> 3) Drivers can derive and extend the structure to easily represent
> ?? driver specific states of a BO for a certain GPUVM.
> 
> The idea of this abstraction was taken from amdgpu, hence the credit
> for
> this idea goes to the developers of amdgpu.
> 
> Cc: Christian K?nig <christian.koenig at amd.com>
> Signed-off-by: Danilo Krummrich <dakr at redhat.com>
> ---
> ?drivers/gpu/drm/drm_gpuvm.c??????????? | 335 +++++++++++++++++++++--
> --
> ?drivers/gpu/drm/nouveau/nouveau_uvmm.c |? 64 +++--
> ?include/drm/drm_gem.h????????????????? |? 32 +--
> ?include/drm/drm_gpuvm.h??????????????? | 188 +++++++++++++-
> ?4 files changed, 533 insertions(+), 86 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gpuvm.c
> b/drivers/gpu/drm/drm_gpuvm.c
> index c03332883432..7f4f5919f84c 100644
> --- a/drivers/gpu/drm/drm_gpuvm.c
> +++ b/drivers/gpu/drm/drm_gpuvm.c
> @@ -70,6 +70,18 @@
> ? * &drm_gem_object, such as the &drm_gem_object containing the
root
> page table,
> ? * but it can also be a 'dummy' object, which can be allocated
with
> ? * drm_gpuvm_resv_object_alloc().
> + *
> + * In order to connect a struct drm_gpuva its backing
> &drm_gem_object each
> + * &drm_gem_object maintains a list of &drm_gpuvm_bo structures,
and
> each
> + * &drm_gpuvm_bo contains a list of &drm_gpuva structures.
> + *
> + * A &drm_gpuvm_bo is an abstraction that represents a combination
> of a
> + * &drm_gpuvm and a &drm_gem_object. Every such combination should
> be unique.
> + * This is ensured by the API through drm_gpuvm_bo_obtain() and
> + * drm_gpuvm_bo_obtain_prealloc() which first look into the
> corresponding
> + * &drm_gem_object list of &drm_gpuvm_bos for an existing instance
> of this
> + * particular combination. If not existent a new instance is created
> and linked
> + * to the &drm_gem_object.
> ? */
> ?
> ?/**
> @@ -395,21 +407,28 @@
> ?/**
> ? * DOC: Locking
> ? *
> - * Generally, the GPU VA manager does not take care of locking
> itself, it is
> - * the drivers responsibility to take care about locking. Drivers
> might want to
> - * protect the following operations: inserting, removing and
> iterating
> - * &drm_gpuva objects as well as generating all kinds of operations,
> such as
> - * split / merge or prefetch.
> - *
> - * The GPU VA manager also does not take care of the locking of the
> backing
> - * &drm_gem_object buffers GPU VA lists by itself; drivers are
> responsible to
> - * enforce mutual exclusion using either the GEMs dma_resv lock or
> alternatively
> - * a driver specific external lock. For the latter see also
> - * drm_gem_gpuva_set_lock().
> - *
> - * However, the GPU VA manager contains lockdep checks to ensure
> callers of its
> - * API hold the corresponding lock whenever the &drm_gem_objects GPU
> VA list is
> - * accessed by functions such as drm_gpuva_link() or
> drm_gpuva_unlink().
> + * In terms of managing &drm_gpuva entries DRM GPUVM does not take
> care of
> + * locking itself, it is the drivers responsibility to take care
> about locking.
> + * Drivers might want to protect the following operations:
> inserting, removing
> + * and iterating &drm_gpuva objects as well as generating all kinds
> of
> + * operations, such as split / merge or prefetch.
> + *
> + * DRM GPUVM also does not take care of the locking of the backing
> + * &drm_gem_object buffers GPU VA lists and &drm_gpuvm_bo
> abstractions by
> + * itself; drivers are responsible to enforce mutual exclusion using
> either the
> + * GEMs dma_resv lock or alternatively a driver specific external
> lock. For the
> + * latter see also drm_gem_gpuva_set_lock().
> + *
> + * However, DRM GPUVM contains lockdep checks to ensure callers of
> its API hold
> + * the corresponding lock whenever the &drm_gem_objects GPU VA list
> is accessed
> + * by functions such as drm_gpuva_link() or drm_gpuva_unlink(), but
> also
> + * drm_gpuvm_bo_obtain() and drm_gpuvm_bo_put().
> + *
> + * The latter is required since on creation and destruction of a
> &drm_gpuvm_bo
> + * the &drm_gpuvm_bo is attached / removed from the
&drm_gem_objects
> gpuva list.
> + * Subsequent calls to drm_gpuvm_bo_obtain() for the same &drm_gpuvm
> and
> + * &drm_gem_object must be able to observe previous creations and
> destructions
> + * of &drm_gpuvm_bos in order to keep instances unique.
> ? */
> ?
> ?/**
> @@ -439,6 +458,7 @@
> ? *?????{
> ? *?????????????struct drm_gpuva_ops *ops;
> ? *?????????????struct drm_gpuva_op *op
> + *?????????????struct drm_gpuvm_bo *vm_bo;
> ? *
> ? *?????????????driver_lock_va_space();
> ? *?????????????ops = drm_gpuvm_sm_map_ops_create(gpuvm, addr, range,
> @@ -446,6 +466,10 @@
> ? *?????????????if (IS_ERR(ops))
> ? *?????????????????????return PTR_ERR(ops);
> ? *
> + *?????????????vm_bo = drm_gpuvm_bo_obtain(gpuvm, obj);
> + *?????????????if (IS_ERR(vm_bo))
> + *?????????????????????return PTR_ERR(vm_bo);
> + *
> ? *?????????????drm_gpuva_for_each_op(op, ops) {
> ? *?????????????????????struct drm_gpuva *va;
> ? *
> @@ -458,7 +482,7 @@
> ? *
> ? *?????????????????????????????driver_vm_map();
> ? *?????????????????????????????drm_gpuva_map(gpuvm, va, &op->map);
> - *?????????????????????????????drm_gpuva_link(va);
> + *?????????????????????????????drm_gpuva_link(va, vm_bo);
> ? *
> ? *?????????????????????????????break;
> ? *?????????????????????case DRM_GPUVA_OP_REMAP: {
> @@ -485,11 +509,11 @@
> ? *?????????????????????????????driver_vm_remap();
> ? *?????????????????????????????drm_gpuva_remap(prev, next, &op-
> >remap);
> ? *
> - *?????????????????????????????drm_gpuva_unlink(va);
> ? *?????????????????????????????if (prev)
> - *?????????????????????????????????????drm_gpuva_link(prev);
> + *?????????????????????????????????????drm_gpuva_link(prev, va-
> >vm_bo);
> ? *?????????????????????????????if (next)
> - *?????????????????????????????????????drm_gpuva_link(next);
> + *?????????????????????????????????????drm_gpuva_link(next, va-
> >vm_bo);
> + *?????????????????????????????drm_gpuva_unlink(va);
> ? *
> ? *?????????????????????????????break;
> ? *?????????????????????}
> @@ -505,6 +529,7 @@
> ? *?????????????????????????????break;
> ? *?????????????????????}
> ? *?????????????}
> + *?????????????drm_gpuvm_bo_put(vm_bo);
> ? *?????????????driver_unlock_va_space();
> ? *
> ? *?????????????return 0;
> @@ -514,6 +539,7 @@
> ? *
> ? *?????struct driver_context {
> ? *?????????????struct drm_gpuvm *gpuvm;
> + *?????????????struct drm_gpuvm_bo *vm_bo;
> ? *?????????????struct drm_gpuva *new_va;
> ? *?????????????struct drm_gpuva *prev_va;
> ? *?????????????struct drm_gpuva *next_va;
> @@ -534,6 +560,7 @@
> ? *?????????????????????????????? struct drm_gem_object *obj, u64
> offset)
> ? *?????{
> ? *?????????????struct driver_context ctx;
> + *?????????????struct drm_gpuvm_bo *vm_bo;
> ? *?????????????struct drm_gpuva_ops *ops;
> ? *?????????????struct drm_gpuva_op *op;
> ? *?????????????int ret = 0;
> @@ -543,16 +570,23 @@
> ? *?????????????ctx.new_va = kzalloc(sizeof(*ctx.new_va),
> GFP_KERNEL);
> ? *?????????????ctx.prev_va = kzalloc(sizeof(*ctx.prev_va),
> GFP_KERNEL);
> ? *?????????????ctx.next_va = kzalloc(sizeof(*ctx.next_va),
> GFP_KERNEL);
> - *?????????????if (!ctx.new_va || !ctx.prev_va || !ctx.next_va) {
> + *?????????????ctx.vm_bo = drm_gpuvm_bo_create(gpuvm, obj);
> + *?????????????if (!ctx.new_va || !ctx.prev_va || !ctx.next_va ||
> !vm_bo) {
> ? *?????????????????????ret = -ENOMEM;
> ? *?????????????????????goto out;
> ? *?????????????}
> ? *
> + *?????????????// Typically protected with a driver specific GEM
> gpuva lock
> + *?????????????// used in the fence signaling path for
> drm_gpuva_link() and
> + *?????????????// drm_gpuva_unlink(), hence pre-allocate.
> + *?????????????ctx.vm_bo = drm_gpuvm_bo_obtain_prealloc(ctx.vm_bo);
> + *
> ? *?????????????driver_lock_va_space();
> ? *?????????????ret = drm_gpuvm_sm_map(gpuvm, &ctx, addr, range, obj,
> offset);
> ? *?????????????driver_unlock_va_space();
> ? *
> ? *?????out:
> + *?????????????drm_gpuvm_bo_put(ctx.vm_bo);
> ? *?????????????kfree(ctx.new_va);
> ? *?????????????kfree(ctx.prev_va);
> ? *?????????????kfree(ctx.next_va);
> @@ -565,7 +599,7 @@
> ? *
> ? *?????????????drm_gpuva_map(ctx->vm, ctx->new_va, &op->map);
> ? *
> - *?????????????drm_gpuva_link(ctx->new_va);
> + *?????????????drm_gpuva_link(ctx->new_va, ctx->vm_bo);
> ? *
> ? *?????????????// prevent the new GPUVA from being freed in
> ? *?????????????// driver_mapping_create()
> @@ -577,22 +611,23 @@
> ? *?????int driver_gpuva_remap(struct drm_gpuva_op *op, void *__ctx)
> ? *?????{
> ? *?????????????struct driver_context *ctx = __ctx;
> + *?????????????struct drm_gpuva *va = op->remap.unmap->va;
> ? *
> ? *?????????????drm_gpuva_remap(ctx->prev_va, ctx->next_va, &op-
> >remap);
> ? *
> - *?????????????drm_gpuva_unlink(op->remap.unmap->va);
> - *?????????????kfree(op->remap.unmap->va);
> - *
> ? *?????????????if (op->remap.prev) {
> - *?????????????????????drm_gpuva_link(ctx->prev_va);
> + *?????????????????????drm_gpuva_link(ctx->prev_va, va->vm_bo);
> ? *?????????????????????ctx->prev_va = NULL;
> ? *?????????????}
> ? *
> ? *?????????????if (op->remap.next) {
> - *?????????????????????drm_gpuva_link(ctx->next_va);
> + *?????????????????????drm_gpuva_link(ctx->next_va, va->vm_bo);
> ? *?????????????????????ctx->next_va = NULL;
> ? *?????????????}
> ? *
> + *?????????????drm_gpuva_unlink(va);
> + *?????????????kfree(va);
> + *
> ? *?????????????return 0;
> ? *?????}
> ? *
> @@ -774,6 +809,194 @@ drm_gpuvm_destroy(struct drm_gpuvm *gpuvm)
> ?}
> ?EXPORT_SYMBOL_GPL(drm_gpuvm_destroy);
> ?
> +/**
> + * drm_gpuvm_bo_create() - create a new instance of struct
> drm_gpuvm_bo
> + * @gpuvm: The &drm_gpuvm the @obj is mapped in.
> + * @obj: The &drm_gem_object being mapped in the @gpuvm.
> + *
> + * If provided by the driver, this function uses the &drm_gpuvm_ops
> + * vm_bo_alloc() callback to allocate.
> + *
> + * Returns: a pointer to the &drm_gpuvm_bo on success, NULL on
Still needs s/Returns:/Return:/g
> failure
> + */
> +struct drm_gpuvm_bo *
> +drm_gpuvm_bo_create(struct drm_gpuvm *gpuvm,
> +?????????????????? struct drm_gem_object *obj)
> +{
> +???????const struct drm_gpuvm_ops *ops = gpuvm->ops;
> +???????struct drm_gpuvm_bo *vm_bo;
> +
> +???????if (ops && ops->vm_bo_alloc)
> +???????????????vm_bo = ops->vm_bo_alloc();
> +???????else
> +???????????????vm_bo = kzalloc(sizeof(*vm_bo), GFP_KERNEL);
> +
> +???????if (unlikely(!vm_bo))
> +???????????????return NULL;
> +
> +???????vm_bo->vm = gpuvm;
> +???????vm_bo->obj = obj;
> +???????drm_gem_object_get(obj);
> +
> +???????kref_init(&vm_bo->kref);
> +???????INIT_LIST_HEAD(&vm_bo->list.gpuva);
> +???????INIT_LIST_HEAD(&vm_bo->list.entry.gem);
> +
> +???????return vm_bo;
> +}
> +EXPORT_SYMBOL_GPL(drm_gpuvm_bo_create);
> +
> +static void
> +drm_gpuvm_bo_destroy(struct kref *kref)
> +{
> +???????struct drm_gpuvm_bo *vm_bo = container_of(kref, struct
> drm_gpuvm_bo,
> +???????????????????????????????????????????????? kref);
> +???????struct drm_gpuvm *gpuvm = vm_bo->vm;
> +???????const struct drm_gpuvm_ops *ops = gpuvm->ops;
> +???????struct drm_gem_object *obj = vm_bo->obj;
> +???????bool lock = !drm_gpuvm_resv_protected(gpuvm);
> +
> +???????if (!lock)
> +???????????????drm_gpuvm_resv_assert_held(gpuvm);
> +
> +???????drm_gem_gpuva_assert_lock_held(obj);
> +???????list_del(&vm_bo->list.entry.gem);
> +
> +???????if (ops && ops->vm_bo_free)
> +???????????????ops->vm_bo_free(vm_bo);
> +???????else
> +???????????????kfree(vm_bo);
> +
> +???????drm_gem_object_put(obj);
> +}
> +
> +/**
> + * drm_gpuvm_bo_put() - drop a struct drm_gpuvm_bo reference
> + * @vm_bo: the &drm_gpuvm_bo to release the reference of
> + *
> + * This releases a reference to @vm_bo.
> + *
> + * If the reference count drops to zero, the &gpuvm_bo is destroyed,
> which
> + * includes removing it from the GEMs gpuva list. Hence, if a call
> to this
> + * function can potentially let the reference count to zero the
> caller must
> + * hold the dma-resv or driver specific GEM gpuva lock.
> + */
> +void
> +drm_gpuvm_bo_put(struct drm_gpuvm_bo *vm_bo)
> +{
> +???????if (vm_bo)
> +???????????????kref_put(&vm_bo->kref, drm_gpuvm_bo_destroy);
> +}
> +EXPORT_SYMBOL_GPL(drm_gpuvm_bo_put);
> +
> +static struct drm_gpuvm_bo *
> +__drm_gpuvm_bo_find(struct drm_gpuvm *gpuvm,
> +?????????????????? struct drm_gem_object *obj)
> +{
> +???????struct drm_gpuvm_bo *vm_bo;
> +
> +???????drm_gem_gpuva_assert_lock_held(obj);
> +???????drm_gem_for_each_gpuvm_bo(vm_bo, obj)
> +???????????????if (vm_bo->vm == gpuvm)
> +???????????????????????return vm_bo;
> +
> +???????return NULL;
> +}
> +
> +/**
> + * drm_gpuvm_bo_find() - find the &drm_gpuvm_bo for the given
> + * &drm_gpuvm and &drm_gem_object
> + * @gpuvm: The &drm_gpuvm the @obj is mapped in.
> + * @obj: The &drm_gem_object being mapped in the @gpuvm.
> + *
> + * Find the &drm_gpuvm_bo representing the combination of the given
> + * &drm_gpuvm and &drm_gem_object. If found, increases the
reference
> + * count of the &drm_gpuvm_bo accordingly.
> + *
> + * Returns: a pointer to the &drm_gpuvm_bo on success, NULL on
> failure
> + */
> +struct drm_gpuvm_bo *
> +drm_gpuvm_bo_find(struct drm_gpuvm *gpuvm,
> +???????????????? struct drm_gem_object *obj)
> +{
> +???????struct drm_gpuvm_bo *vm_bo = __drm_gpuvm_bo_find(gpuvm, obj);
> +
> +???????return vm_bo ? drm_gpuvm_bo_get(vm_bo) : NULL;
> +}
> +EXPORT_SYMBOL_GPL(drm_gpuvm_bo_find);
> +
> +/**
> + * drm_gpuvm_bo_obtain() - obtains and instance of the &drm_gpuvm_bo
> for the
> + * given &drm_gpuvm and &drm_gem_object
> + * @gpuvm: The &drm_gpuvm the @obj is mapped in.
> + * @obj: The &drm_gem_object being mapped in the @gpuvm.
> + *
> + * Find the &drm_gpuvm_bo representing the combination of the given
> + * &drm_gpuvm and &drm_gem_object. If found, increases the
reference
> + * count of the &drm_gpuvm_bo accordingly. If not found, allocates a
> new
> + * &drm_gpuvm_bo.
> + *
> + * A new &drm_gpuvm_bo is added to the GEMs gpuva list.
> + *
> + * Returns: a pointer to the &drm_gpuvm_bo on success, an ERR_PTR on
> failure
> + */
> +struct drm_gpuvm_bo *
> +drm_gpuvm_bo_obtain(struct drm_gpuvm *gpuvm,
> +?????????????????? struct drm_gem_object *obj)
> +{
> +???????struct drm_gpuvm_bo *vm_bo;
> +
> +???????vm_bo = drm_gpuvm_bo_find(gpuvm, obj);
> +???????if (vm_bo)
> +???????????????return vm_bo;
> +
> +???????vm_bo = drm_gpuvm_bo_create(gpuvm, obj);
> +???????if (!vm_bo)
> +???????????????return ERR_PTR(-ENOMEM);
> +
> +???????drm_gem_gpuva_assert_lock_held(obj);
> +???????list_add_tail(&vm_bo->list.entry.gem,
&obj->gpuva.list);
> +
> +???????return vm_bo;
> +}
> +EXPORT_SYMBOL_GPL(drm_gpuvm_bo_obtain);
> +
> +/**
> + * drm_gpuvm_bo_obtain_prealloc() - obtains and instance of the
> &drm_gpuvm_bo
> + * for the given &drm_gpuvm and &drm_gem_object
> + * @__vm_bo: A pre-allocated struct drm_gpuvm_bo.
> + *
> + * Find the &drm_gpuvm_bo representing the combination of the given
> + * &drm_gpuvm and &drm_gem_object. If found, increases the
reference
> + * count of the found &drm_gpuvm_bo accordingly, while the @__vm_bo
> reference
> + * count is decreased. If not found @__vm_bo is returned without
> further
> + * increase of the reference count.
> + *
> + * A new &drm_gpuvm_bo is added to the GEMs gpuva list.
> + *
> + * Returns: a pointer to the found &drm_gpuvm_bo or @__vm_bo if no
> existing
> + * &drm_gpuvm_bo was found
> + */
> +struct drm_gpuvm_bo *
> +drm_gpuvm_bo_obtain_prealloc(struct drm_gpuvm_bo *__vm_bo)
> +{
> +???????struct drm_gpuvm *gpuvm = __vm_bo->vm;
> +???????struct drm_gem_object *obj = __vm_bo->obj;
> +???????struct drm_gpuvm_bo *vm_bo;
> +
> +???????vm_bo = drm_gpuvm_bo_find(gpuvm, obj);
> +???????if (vm_bo) {
> +???????????????drm_gpuvm_bo_put(__vm_bo);
> +???????????????return vm_bo;
> +???????}
> +
> +???????drm_gem_gpuva_assert_lock_held(obj);
> +???????list_add_tail(&__vm_bo->list.entry.gem,
&obj->gpuva.list);
> +
> +???????return __vm_bo;
> +}
> +EXPORT_SYMBOL_GPL(drm_gpuvm_bo_obtain_prealloc);
> +
> ?static int
> ?__drm_gpuva_insert(struct drm_gpuvm *gpuvm,
> ?????????????????? struct drm_gpuva *va)
> @@ -864,24 +1087,33 @@ EXPORT_SYMBOL_GPL(drm_gpuva_remove);
> ?/**
> ? * drm_gpuva_link() - link a &drm_gpuva
> ? * @va: the &drm_gpuva to link
> + * @vm_bo: the &drm_gpuvm_bo to add the &drm_gpuva to
> ? *
> - * This adds the given &va to the GPU VA list of the
&drm_gem_object
> it is
> - * associated with.
> + * This adds the given &va to the GPU VA list of the &drm_gpuvm_bo
> and the
> + * &drm_gpuvm_bo to the &drm_gem_object it is associated with.
> + *
> + * For every &drm_gpuva entry added to the &drm_gpuvm_bo an
> additional
> + * reference of the latter is taken.
> ? *
> ? * This function expects the caller to protect the GEM's GPUVA list
> against
> - * concurrent access using the GEMs dma_resv lock.
> + * concurrent access using either the GEMs dma_resv lock or a driver
> specific
> + * lock set through drm_gem_gpuva_set_lock().
> ? */
> ?void
> -drm_gpuva_link(struct drm_gpuva *va)
> +drm_gpuva_link(struct drm_gpuva *va, struct drm_gpuvm_bo *vm_bo)
> ?{
> ????????struct drm_gem_object *obj = va->gem.obj;
> +???????struct drm_gpuvm *gpuvm = va->vm;
> ?
> ????????if (unlikely(!obj))
> ????????????????return;
> ?
> -???????drm_gem_gpuva_assert_lock_held(obj);
> +???????drm_WARN_ON(gpuvm->drm, obj != vm_bo->obj);
> ?
> -???????list_add_tail(&va->gem.entry, &obj->gpuva.list);
> +???????va->vm_bo = drm_gpuvm_bo_get(vm_bo);
> +
> +???????drm_gem_gpuva_assert_lock_held(obj);
> +???????list_add_tail(&va->gem.entry, &vm_bo->list.gpuva);
> ?}
> ?EXPORT_SYMBOL_GPL(drm_gpuva_link);
> ?
> @@ -892,20 +1124,31 @@ EXPORT_SYMBOL_GPL(drm_gpuva_link);
> ? * This removes the given &va from the GPU VA list of the
> &drm_gem_object it is
> ? * associated with.
> ? *
> + * This removes the given &va from the GPU VA list of the
> &drm_gpuvm_bo and
> + * the &drm_gpuvm_bo from the &drm_gem_object it is associated
with
> in case
> + * this call unlinks the last &drm_gpuva from the &drm_gpuvm_bo.
> + *
> + * For every &drm_gpuva entry removed from the &drm_gpuvm_bo a
> reference of
> + * the latter is dropped.
> + *
> ? * This function expects the caller to protect the GEM's GPUVA list
> against
> - * concurrent access using the GEMs dma_resv lock.
> + * concurrent access using either the GEMs dma_resv lock or a driver
> specific
> + * lock set through drm_gem_gpuva_set_lock().
> ? */
> ?void
> ?drm_gpuva_unlink(struct drm_gpuva *va)
> ?{
> ????????struct drm_gem_object *obj = va->gem.obj;
> +???????struct drm_gpuvm_bo *vm_bo = va->vm_bo;
> ?
> ????????if (unlikely(!obj))
> ????????????????return;
> ?
> ????????drm_gem_gpuva_assert_lock_held(obj);
> -
> ????????list_del_init(&va->gem.entry);
> +
> +???????va->vm_bo = NULL;
> +???????drm_gpuvm_bo_put(vm_bo);
> ?}
> ?EXPORT_SYMBOL_GPL(drm_gpuva_unlink);
> ?
> @@ -1050,10 +1293,10 @@ drm_gpuva_remap(struct drm_gpuva *prev,
> ????????????????struct drm_gpuva *next,
> ????????????????struct drm_gpuva_op_remap *op)
> ?{
> -???????struct drm_gpuva *curr = op->unmap->va;
> -???????struct drm_gpuvm *gpuvm = curr->vm;
> +???????struct drm_gpuva *va = op->unmap->va;
> +???????struct drm_gpuvm *gpuvm = va->vm;
> ?
> -???????drm_gpuva_remove(curr);
> +???????drm_gpuva_remove(va);
> ?
> ????????if (op->prev) {
> ????????????????drm_gpuva_init_from_op(prev, op->prev);
> @@ -1695,9 +1938,8 @@ drm_gpuvm_prefetch_ops_create(struct drm_gpuvm
> *gpuvm,
> ?EXPORT_SYMBOL_GPL(drm_gpuvm_prefetch_ops_create);
> ?
> ?/**
> - * drm_gpuvm_gem_unmap_ops_create() - creates the &drm_gpuva_ops to
> unmap a GEM
> - * @gpuvm: the &drm_gpuvm representing the GPU VA space
> - * @obj: the &drm_gem_object to unmap
> + * drm_gpuvm_bo_unmap_ops_create() - creates the &drm_gpuva_ops to
> unmap a GEM
> + * @vm_bo: the &drm_gpuvm_bo abstraction
> ? *
> ? * This function creates a list of operations to perform unmapping
> for every
> ? * GPUVA attached to a GEM.
> @@ -1714,15 +1956,14 @@
> EXPORT_SYMBOL_GPL(drm_gpuvm_prefetch_ops_create);
> ? * Returns: a pointer to the &drm_gpuva_ops on success, an ERR_PTR
> on failure
> ? */
> ?struct drm_gpuva_ops *
> -drm_gpuvm_gem_unmap_ops_create(struct drm_gpuvm *gpuvm,
> -????????????????????????????? struct drm_gem_object *obj)
> +drm_gpuvm_bo_unmap_ops_create(struct drm_gpuvm_bo *vm_bo)
> ?{
> ????????struct drm_gpuva_ops *ops;
> ????????struct drm_gpuva_op *op;
> ????????struct drm_gpuva *va;
> ????????int ret;
> ?
> -???????drm_gem_gpuva_assert_lock_held(obj);
> +???????drm_gem_gpuva_assert_lock_held(vm_bo->obj);
> ?
> ????????ops = kzalloc(sizeof(*ops), GFP_KERNEL);
> ????????if (!ops)
> @@ -1730,8 +1971,8 @@ drm_gpuvm_gem_unmap_ops_create(struct drm_gpuvm
> *gpuvm,
> ?
> ????????INIT_LIST_HEAD(&ops->list);
> ?
> -???????drm_gem_for_each_gpuva(va, obj) {
> -???????????????op = gpuva_op_alloc(gpuvm);
> +???????drm_gpuvm_bo_for_each_va(va, vm_bo) {
> +???????????????op = gpuva_op_alloc(vm_bo->vm);
> ????????????????if (!op) {
> ????????????????????????ret = -ENOMEM;
> ????????????????????????goto err_free_ops;
> @@ -1745,10 +1986,10 @@ drm_gpuvm_gem_unmap_ops_create(struct
> drm_gpuvm *gpuvm,
> ????????return ops;
> ?
> ?err_free_ops:
> -???????drm_gpuva_ops_free(gpuvm, ops);
> +???????drm_gpuva_ops_free(vm_bo->vm, ops);
> ????????return ERR_PTR(ret);
> ?}
> -EXPORT_SYMBOL_GPL(drm_gpuvm_gem_unmap_ops_create);
> +EXPORT_SYMBOL_GPL(drm_gpuvm_bo_unmap_ops_create);
> ?
> ?/**
> ? * drm_gpuva_ops_free() - free the given &drm_gpuva_ops
> diff --git a/drivers/gpu/drm/nouveau/nouveau_uvmm.c
> b/drivers/gpu/drm/nouveau/nouveau_uvmm.c
> index ed439bf4032f..1e95b0a1b047 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_uvmm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_uvmm.c
> @@ -62,6 +62,8 @@ struct bind_job_op {
> ????????enum vm_bind_op op;
> ????????u32 flags;
> ?
> +???????struct drm_gpuvm_bo *vm_bo;
> +
> ????????struct {
> ????????????????u64 addr;
> ????????????????u64 range;
> @@ -1113,22 +1115,28 @@ bind_validate_region(struct nouveau_job *job)
> ?}
> ?
> ?static void
> -bind_link_gpuvas(struct drm_gpuva_ops *ops, struct
> nouveau_uvma_prealloc *new)
> +bind_link_gpuvas(struct bind_job_op *bop)
> ?{
> +???????struct nouveau_uvma_prealloc *new = &bop->new;
> +???????struct drm_gpuvm_bo *vm_bo = bop->vm_bo;
> +???????struct drm_gpuva_ops *ops = bop->ops;
> ????????struct drm_gpuva_op *op;
> ?
> ????????drm_gpuva_for_each_op(op, ops) {
> ????????????????switch (op->op) {
> ????????????????case DRM_GPUVA_OP_MAP:
> -???????????????????????drm_gpuva_link(&new->map->va);
> +???????????????????????drm_gpuva_link(&new->map->va, vm_bo);
> ????????????????????????break;
> -???????????????case DRM_GPUVA_OP_REMAP:
> +???????????????case DRM_GPUVA_OP_REMAP: {
> +???????????????????????struct drm_gpuva *va = op->remap.unmap->va;
> +
> ????????????????????????if (op->remap.prev)
> -???????????????????????????????drm_gpuva_link(&new->prev->va);
> +???????????????????????????????drm_gpuva_link(&new->prev->va,
va-
> >vm_bo);
> ????????????????????????if (op->remap.next)
> -???????????????????????????????drm_gpuva_link(&new->next->va);
> -???????????????????????drm_gpuva_unlink(op->remap.unmap->va);
> +???????????????????????????????drm_gpuva_link(&new->next->va,
va-
> >vm_bo);
> +???????????????????????drm_gpuva_unlink(va);
> ????????????????????????break;
> +???????????????}
> ????????????????case DRM_GPUVA_OP_UNMAP:
> ????????????????????????drm_gpuva_unlink(op->unmap.va);
> ????????????????????????break;
> @@ -1150,10 +1158,18 @@ nouveau_uvmm_bind_job_submit(struct
> nouveau_job *job)
> ?
> ????????list_for_each_op(op, &bind_job->ops) {
> ????????????????if (op->op == OP_MAP) {
> -???????????????????????op->gem.obj = drm_gem_object_lookup(job-
> >file_priv,
> -?????????????????????????????????????????????????????????? op-
> >gem.handle);
> -???????????????????????if (!op->gem.obj)
> +???????????????????????struct drm_gem_object *obj;
> +
> +???????????????????????obj = drm_gem_object_lookup(job->file_priv,
> +?????????????????????????????????????????????????? op->gem.handle);
> +???????????????????????if (!(op->gem.obj = obj))
> ????????????????????????????????return -ENOENT;
> +
> +???????????????????????dma_resv_lock(obj->resv, NULL);
> +???????????????????????op->vm_bo =
drm_gpuvm_bo_obtain(&uvmm->base,
> obj);
> +???????????????????????dma_resv_unlock(obj->resv);
> +???????????????????????if (IS_ERR(op->vm_bo))
> +???????????????????????????????return PTR_ERR(op->vm_bo);
> ????????????????}
> ?
> ????????????????ret = bind_validate_op(job, op);
> @@ -1364,7 +1380,7 @@ nouveau_uvmm_bind_job_submit(struct nouveau_job
> *job)
> ????????????????case OP_UNMAP_SPARSE:
> ????????????????case OP_MAP:
> ????????????????case OP_UNMAP:
> -???????????????????????bind_link_gpuvas(op->ops, &op->new);
> +???????????????????????bind_link_gpuvas(op);
> ????????????????????????break;
> ????????????????default:
> ????????????????????????break;
> @@ -1511,6 +1527,12 @@ nouveau_uvmm_bind_job_free_work_fn(struct
> work_struct *work)
> ????????????????if (!IS_ERR_OR_NULL(op->ops))
> ????????????????????????drm_gpuva_ops_free(&uvmm->base, op->ops);
> ?
> +???????????????if (!IS_ERR_OR_NULL(op->vm_bo)) {
> +???????????????????????dma_resv_lock(obj->resv, NULL);
> +???????????????????????drm_gpuvm_bo_put(op->vm_bo);
> +???????????????????????dma_resv_unlock(obj->resv);
> +???????????????}
> +
> ????????????????if (obj)
> ????????????????????????drm_gem_object_put(obj);
> ????????}
> @@ -1776,15 +1798,18 @@ void
> ?nouveau_uvmm_bo_map_all(struct nouveau_bo *nvbo, struct nouveau_mem
> *mem)
> ?{
> ????????struct drm_gem_object *obj = &nvbo->bo.base;
> +???????struct drm_gpuvm_bo *vm_bo;
> ????????struct drm_gpuva *va;
> ?
> ????????dma_resv_assert_held(obj->resv);
> ?
> -???????drm_gem_for_each_gpuva(va, obj) {
> -???????????????struct nouveau_uvma *uvma = uvma_from_va(va);
> +???????drm_gem_for_each_gpuvm_bo(vm_bo, obj) {
> +???????????????drm_gpuvm_bo_for_each_va(va, vm_bo) {
> +???????????????????????struct nouveau_uvma *uvma = uvma_from_va(va);
> ?
> -???????????????nouveau_uvma_map(uvma, mem);
> -???????????????drm_gpuva_invalidate(va, false);
> +???????????????????????nouveau_uvma_map(uvma, mem);
> +???????????????????????drm_gpuva_invalidate(va, false);
> +???????????????}
> ????????}
> ?}
> ?
> @@ -1792,15 +1817,18 @@ void
> ?nouveau_uvmm_bo_unmap_all(struct nouveau_bo *nvbo)
> ?{
> ????????struct drm_gem_object *obj = &nvbo->bo.base;
> +???????struct drm_gpuvm_bo *vm_bo;
> ????????struct drm_gpuva *va;
> ?
> ????????dma_resv_assert_held(obj->resv);
> ?
> -???????drm_gem_for_each_gpuva(va, obj) {
> -???????????????struct nouveau_uvma *uvma = uvma_from_va(va);
> +???????drm_gem_for_each_gpuvm_bo(vm_bo, obj) {
> +???????????????drm_gpuvm_bo_for_each_va(va, vm_bo) {
> +???????????????????????struct nouveau_uvma *uvma = uvma_from_va(va);
> ?
> -???????????????nouveau_uvma_unmap(uvma);
> -???????????????drm_gpuva_invalidate(va, true);
> +???????????????????????nouveau_uvma_unmap(uvma);
> +???????????????????????drm_gpuva_invalidate(va, true);
> +???????????????}
> ????????}
> ?}
> ?
> diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
> index 16364487fde9..369505447acd 100644
> --- a/include/drm/drm_gem.h
> +++ b/include/drm/drm_gem.h
> @@ -580,7 +580,7 @@ int drm_gem_evict(struct drm_gem_object *obj);
> ? * drm_gem_gpuva_init() - initialize the gpuva list of a GEM object
> ? * @obj: the &drm_gem_object
> ? *
> - * This initializes the &drm_gem_object's &drm_gpuva list.
> + * This initializes the &drm_gem_object's &drm_gpuvm_bo list.
> ? *
> ? * Calling this function is only necessary for drivers intending to
> support the
> ? * &drm_driver_feature DRIVER_GEM_GPUVA.
> @@ -593,28 +593,28 @@ static inline void drm_gem_gpuva_init(struct
> drm_gem_object *obj)
> ?}
> ?
> ?/**
> - * drm_gem_for_each_gpuva() - iternator to walk over a list of
> gpuvas
> - * @entry__: &drm_gpuva structure to assign to in each iteration
> step
> - * @obj__: the &drm_gem_object the &drm_gpuvas to walk are
> associated with
> + * drm_gem_for_each_gpuvm_bo() - iterator to walk over a list of
> &drm_gpuvm_bo
> + * @entry__: &drm_gpuvm_bo structure to assign to in each iteration
> step
> + * @obj__: the &drm_gem_object the &drm_gpuvm_bo to walk are
> associated with
> ? *
> - * This iterator walks over all &drm_gpuva structures associated
> with the
> - * &drm_gpuva_manager.
> + * This iterator walks over all &drm_gpuvm_bo structures associated
> with the
> + * &drm_gem_object.
> ? */
> -#define drm_gem_for_each_gpuva(entry__, obj__) \
> -???????list_for_each_entry(entry__, &(obj__)->gpuva.list,
gem.entry)
> +#define drm_gem_for_each_gpuvm_bo(entry__, obj__) \
> +???????list_for_each_entry(entry__, &(obj__)->gpuva.list,
> list.entry.gem)
> ?
> ?/**
> - * drm_gem_for_each_gpuva_safe() - iternator to safely walk over a
> list of
> - * gpuvas
> - * @entry__: &drm_gpuva structure to assign to in each iteration
> step
> - * @next__: &next &drm_gpuva to store the next step
> - * @obj__: the &drm_gem_object the &drm_gpuvas to walk are
> associated with
> + * drm_gem_for_each_gpuvm_bo_safe() - iterator to safely walk over a
> list of
> + * &drm_gpuvm_bo
> + * @entry__: &drm_gpuvm_bostructure to assign to in each iteration
> step
> + * @next__: &next &drm_gpuvm_bo to store the next step
> + * @obj__: the &drm_gem_object the &drm_gpuvm_bo to walk are
> associated with
> ? *
> - * This iterator walks over all &drm_gpuva structures associated
> with the
> + * This iterator walks over all &drm_gpuvm_bo structures associated
> with the
> ? * &drm_gem_object. It is implemented with
> list_for_each_entry_safe(), hence
> ? * it is save against removal of elements.
> ? */
> -#define drm_gem_for_each_gpuva_safe(entry__, next__, obj__) \
> -???????list_for_each_entry_safe(entry__, next__, &(obj__)-
> >gpuva.list, gem.entry)
> +#define drm_gem_for_each_gpuvm_bo_safe(entry__, next__, obj__) \
> +???????list_for_each_entry_safe(entry__, next__, &(obj__)-
> >gpuva.list, list.entry.gem)
> ?
> ?#endif /* __DRM_GEM_H__ */
> diff --git a/include/drm/drm_gpuvm.h b/include/drm/drm_gpuvm.h
> index 47cbacb244b9..466fdd76c71a 100644
> --- a/include/drm/drm_gpuvm.h
> +++ b/include/drm/drm_gpuvm.h
> @@ -25,6 +25,7 @@
> ? * OTHER DEALINGS IN THE SOFTWARE.
> ? */
> ?
> +#include <linux/dma-resv.h>
> ?#include <linux/list.h>
> ?#include <linux/rbtree.h>
> ?#include <linux/types.h>
> @@ -33,6 +34,7 @@
> ?#include <drm/drm_gem.h>
> ?
> ?struct drm_gpuvm;
> +struct drm_gpuvm_bo;
> ?struct drm_gpuvm_ops;
> ?
> ?/**
> @@ -73,6 +75,12 @@ struct drm_gpuva {
> ???????? */
> ????????struct drm_gpuvm *vm;
> ?
> +???????/**
> +??????? * @vm_bo: the &drm_gpuvm_bo abstraction for the mapped
> +??????? * &drm_gem_object
> +??????? */
> +???????struct drm_gpuvm_bo *vm_bo;
> +
> ????????/**
> ???????? * @flags: the &drm_gpuva_flags for this mapping
> ???????? */
> @@ -108,7 +116,7 @@ struct drm_gpuva {
> ????????????????struct drm_gem_object *obj;
> ?
> ????????????????/**
> -??????????????? * @entry: the &list_head to attach this object to a
> &drm_gem_object
> +??????????????? * @entry: the &list_head to attach this object to a
> &drm_gpuvm_bo
> ???????????????? */
> ????????????????struct list_head entry;
> ????????} gem;
> @@ -141,7 +149,7 @@ struct drm_gpuva {
> ?int drm_gpuva_insert(struct drm_gpuvm *gpuvm, struct drm_gpuva *va);
> ?void drm_gpuva_remove(struct drm_gpuva *va);
> ?
> -void drm_gpuva_link(struct drm_gpuva *va);
> +void drm_gpuva_link(struct drm_gpuva *va, struct drm_gpuvm_bo
> *vm_bo);
> ?void drm_gpuva_unlink(struct drm_gpuva *va);
> ?
> ?struct drm_gpuva *drm_gpuva_find(struct drm_gpuvm *gpuvm,
> @@ -188,10 +196,16 @@ static inline bool drm_gpuva_invalidated(struct
> drm_gpuva *va)
> ? * enum drm_gpuvm_flags - flags for struct drm_gpuvm
> ? */
> ?enum drm_gpuvm_flags {
> +???????/**
> +??????? * @DRM_GPUVM_RESV_PROTECTED: GPUVM is protected externally
> by the
> +??????? * GPUVM's &dma_resv lock
> +??????? */
> +???????DRM_GPUVM_RESV_PROTECTED = BIT(0),
> +
> ????????/**
> ???????? * @DRM_GPUVM_USERBITS: user defined bits
> ???????? */
> -???????DRM_GPUVM_USERBITS = BIT(0),
> +???????DRM_GPUVM_USERBITS = BIT(1),
> ?};
> ?
> ?/**
> @@ -280,6 +294,19 @@ bool drm_gpuvm_interval_empty(struct drm_gpuvm
> *gpuvm, u64 addr, u64 range);
> ?struct drm_gem_object *
> ?drm_gpuvm_resv_object_alloc(struct drm_device *drm);
> ?
> +/**
> + * drm_gpuvm_resv_protected() - indicates whether
> &DRM_GPUVM_RESV_PROTECTED is
> + * set
> + * @gpuvm: the &drm_gpuvm
> + *
> + * Returns: true if &DRM_GPUVM_RESV_PROTECTED is set, false
> otherwise.
> + */
> +static inline bool
> +drm_gpuvm_resv_protected(struct drm_gpuvm *gpuvm)
> +{
> +???????return gpuvm->flags & DRM_GPUVM_RESV_PROTECTED;
> +}
> +
> ?/**
> ? * drm_gpuvm_resv() - returns the &drm_gpuvm's &dma_resv
> ? * @gpuvm__: the &drm_gpuvm
> @@ -298,6 +325,12 @@ drm_gpuvm_resv_object_alloc(struct drm_device
> *drm);
> ? */
> ?#define drm_gpuvm_resv_obj(gpuvm__) ((gpuvm__)->r_obj)
> ?
> +#define drm_gpuvm_resv_held(gpuvm__) \
> +???????dma_resv_held(drm_gpuvm_resv(gpuvm__))
> +
> +#define drm_gpuvm_resv_assert_held(gpuvm__) \
> +???????dma_resv_assert_held(drm_gpuvm_resv(gpuvm__))
> +
> ?#define drm_gpuvm_resv_held(gpuvm__) \
> ????????dma_resv_held(drm_gpuvm_resv(gpuvm__))
> ?
> @@ -382,6 +415,128 @@ __drm_gpuva_next(struct drm_gpuva *va)
> ?#define drm_gpuvm_for_each_va_safe(va__, next__, gpuvm__) \
> ????????list_for_each_entry_safe(va__, next__, &(gpuvm__)->rb.list,
> rb.entry)
> ?
> +/**
> + * struct drm_gpuvm_bo - structure representing a &drm_gpuvm and
> + * &drm_gem_object combination
> + *
> + * This structure is an abstraction representing a &drm_gpuvm and
> + * &drm_gem_object combination. It serves as an indirection to
> accelerate
> + * iterating all &drm_gpuvas within a &drm_gpuvm backed by the
same
> + * &drm_gem_object.
> + *
> + * Furthermore it is used cache evicted GEM objects for a certain
> GPU-VM to
> + * accelerate validation.
> + *
> + * Typically, drivers want to create an instance of a struct
> drm_gpuvm_bo once
> + * a GEM object is mapped first in a GPU-VM and release the instance
> once the
> + * last mapping of the GEM object in this GPU-VM is unmapped.
> + */
> +struct drm_gpuvm_bo {
> +???????/**
> +??????? * @vm: The &drm_gpuvm the @obj is mapped in. This pointer is
> not
> +??????? * reference counted.
> +??????? *
> +??????? * A struct drm_gpuvm_bo is not allowed to out-live its
> &drm_gpuvm
> +??????? * context. Implicitly, this is ensured by the fact that the
> driver is
> +??????? * responsible to ensure the VM doesn't contain mappings once
> it's
> +??????? * freed, since a struct drm_gpuvm_bo should be freed once
> the last
> +??????? * mapping being backed by the corresponding buffer object is
> unmapped.
> +??????? */

I don't think the above is completely true. Let's assume in the
!RESV_PROTECTED case that a reference is grabbed on the drm_gpuvm_bo
during an iteration over a list. Then user-space closes the vm and all
vmas are unlinked, but this reference remains but the vm pointer
becomes stale. In the RESV_PROTECTED case this is ensured not to happen
if by the vm->resv being grabbed during unlink, but in the
!RESV_PROTECTED case, the above wording isn't sufficient. The caller
needs to ensure the vm stays alive using some sort of similar rule or
use kref_get_unless_zero() on the vm under the spinlock if
dereferenced.
> +???????struct drm_gpuvm *vm;
> +
> +???????/**
> +??????? * @obj: The &drm_gem_object being mapped in @vm. This is a
> reference
> +??????? * counted pointer.
> +??????? */
> +???????struct drm_gem_object *obj;
> +
> +???????/**
> +??????? * @kref: The reference count for this &drm_gpuvm_bo.
> +??????? */
> +???????struct kref kref;
> +
> +???????/**
> +??????? * @list: Structure containing all &list_heads.
> +??????? */
> +???????struct {
> +???????????????/**
> +??????????????? * @gpuva: The list of linked &drm_gpuvas.
> +??????????????? */
> +???????????????struct list_head gpuva;
Still missing doc on how the @gpuva stays alive during iteration over
the list?



8<-------------------------------------------------------------

Thanks,
Thomas

On Mon, 2023-10-23 at 22:16 +0200, Danilo Krummrich
wrote:
> Add an abstraction layer between the drm_gpuva mappings of a
> particular
> drm_gem_object and this GEM object itself. The abstraction represents
> a
> combination of a drm_gem_object and drm_gpuvm. The drm_gem_object
> holds
> a list of drm_gpuvm_bo structures (the structure representing this
> abstraction), while each drm_gpuvm_bo contains list of mappings of
> this
> GEM object.
> 
> This has multiple advantages:
> 
> 1) We can use the drm_gpuvm_bo structure to attach it to various
> lists
> ?? of the drm_gpuvm. This is useful for tracking external and evicted
> ?? objects per VM, which is introduced in subsequent patches.
> 
> 2) Finding mappings of a certain drm_gem_object mapped in a certain
> ?? drm_gpuvm becomes much cheaper.
> 
> 3) Drivers can derive and extend the structure to easily represent
> ?? driver specific states of a BO for a certain GPUVM.
> 
> The idea of this abstraction was taken from amdgpu, hence the credit
> for
> this idea goes to the developers of amdgpu.
> 
> Cc: Christian K?nig <christian.koenig at amd.com>
> Signed-off-by: Danilo Krummrich <dakr at redhat.com>
> ---
> ?drivers/gpu/drm/drm_gpuvm.c??????????? | 335 +++++++++++++++++++++--
> --
> ?drivers/gpu/drm/nouveau/nouveau_uvmm.c |? 64 +++--
> ?include/drm/drm_gem.h????????????????? |? 32 +--
> ?include/drm/drm_gpuvm.h??????????????? | 188 +++++++++++++-
> ?4 files changed, 533 insertions(+), 86 deletions(-)
That checkpatch.pl error still remains as well.

Thanks,
Thomas