Danilo Krummrich
2023-Sep-09 15:31 UTC
[Nouveau] [PATCH drm-misc-next v3 6/7] drm/gpuvm: generalize dma_resv/extobj handling and GEM validation
So far the DRM GPUVA manager offers common infrastructure to track GPU VA allocations and mappings, generically connect GPU VA mappings to their backing buffers and perform more complex mapping operations on the GPU VA space. However, there are more design patterns commonly used by drivers, which can potentially be generalized in order to make the DRM GPUVA manager represent a basic GPU-VM implementation. In this context, this patch aims at generalizing the following elements. 1) Provide a common dma-resv for GEM objects not being used outside of this GPU-VM. 2) Provide tracking of external GEM objects (GEM objects which are shared with other GPU-VMs). 3) Provide functions to efficiently lock all GEM objects dma-resv the GPU-VM contains mappings of. 4) Provide tracking of evicted GEM objects the GPU-VM contains mappings of, such that validation of evicted GEM objects is accelerated. 5) Provide some convinience functions for common patterns. Rather than being designed as a "framework", the target is to make all features appear as a collection of optional helper functions, such that drivers are free to make use of the DRM GPUVA managers basic functionality and opt-in for other features without setting any feature flags, just by making use of the corresponding functions. Big kudos to Boris Brezillon for his help to figure out locking for drivers updating the GPU VA space within the fence signalling path. Suggested-by: Matthew Brost <matthew.brost at intel.com> Signed-off-by: Danilo Krummrich <dakr at redhat.com> --- drivers/gpu/drm/drm_gpuvm.c | 516 ++++++++++++++++++++++++++++++++++++ include/drm/drm_gpuvm.h | 197 ++++++++++++++ 2 files changed, 713 insertions(+) diff --git a/drivers/gpu/drm/drm_gpuvm.c b/drivers/gpu/drm/drm_gpuvm.c index f4411047dbb3..8e62a043f719 100644 --- a/drivers/gpu/drm/drm_gpuvm.c +++ b/drivers/gpu/drm/drm_gpuvm.c @@ -73,6 +73,21 @@ * &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. + * + * &drm_gpuvm_bo structures, since unique for a given &drm_gpuvm, are also used + * as entry for the &drm_gpuvm's lists of external and evicted objects. Those + * list are maintained in order to accelerate locking of dma-resv locks and + * validation of evicted objects bound in a &drm_gpuvm. For instance the all + * &drm_gem_object's &dma_resv of a given &drm_gpuvm can be locked by calling + * drm_gpuvm_exec_lock(). Once locked drivers can call drm_gpuvm_validate() in + * order to validate all evicted &drm_gem_objects. It is also possible to lock + * additional &drm_gem_objects by providing the corresponding parameters to + * drm_gpuvm_exec_lock() as well as open code the &drm_exec loop while making + * use of helper functions such as drm_gpuvm_prepare_range() or + * drm_gpuvm_prepare_objects(). + * + * Every bound &drm_gem_object is treated as external object when its &dma_resv + * structure is different than the &drm_gpuvm's common &dma_resv structure. */ /** @@ -420,6 +435,20 @@ * 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. + * + * The &drm_gpuvm's lists for keeping track of external and evicted objects are + * protected against concurrent insertion / removal and iteration internally. + * + * However, drivers still need ensure to protect concurrent calls to functions + * iterating those lists, such as drm_gpuvm_validate() and + * drm_gpuvm_prepare_objects(). Every such function contains a particular + * comment and lockdep checks if possible. + * + * Functions adding or removing entries from those lists, such as + * drm_gpuvm_bo_evict() or drm_gpuvm_bo_extobj_add() may be called with external + * locks being held, e.g. in order to avoid the corresponding list to be + * (safely) modified while potentially being iternated by other API functions. + * However, this is entirely optional. */ /** @@ -632,6 +661,131 @@ * } */ +/** + * get_next_vm_bo_from_list() - get the next vm_bo element + * @__gpuvm: The GPU VM + * @__list_name: The name of the list we're iterating on + * @__local_list: A pointer to the local list used to store already iterated items + * @__prev_vm_bo: The previous element we got from drm_gpuvm_get_next_cached_vm_bo() + * + * This helper is here to provide lockless list iteration. Lockless as in, the + * iterator releases the lock immediately after picking the first element from + * the list, so list insertion deletion can happen concurrently. + * + * Elements popped from the original list are kept in a local list, so removal + * and is_empty checks can still happen while we're iterating the list. + */ +#define get_next_vm_bo_from_list(__gpuvm, __list_name, __local_list, __prev_vm_bo) \ + ({ \ + struct drm_gpuvm_bo *__vm_bo; \ + \ + drm_gpuvm_bo_put(__prev_vm_bo); \ + \ + spin_lock(&(__gpuvm)->__list_name.lock); \ + while (!list_empty(&(__gpuvm)->__list_name.list)) { \ + __vm_bo = list_first_entry(&(__gpuvm)->__list_name.list, \ + struct drm_gpuvm_bo, \ + list.entry.__list_name); \ + if (drm_gpuvm_bo_get_unless_zero(__vm_bo)) { \ + list_move_tail(&(__vm_bo)->list.entry.__list_name, \ + __local_list); \ + break; \ + } else { \ + list_del_init(&(__vm_bo)->list.entry.__list_name); \ + __vm_bo = NULL; \ + } \ + } \ + spin_unlock(&(__gpuvm)->__list_name.lock); \ + \ + __vm_bo; \ + }) + +/** + * for_each_vm_bo_in_list() - internal vm_bo list iterator + * + * This helper is here to provide lockless list iteration. Lockless as in, the + * iterator releases the lock immediately after picking the first element from the + * list, so list insertion and deletion can happen concurrently. + * + * Typical use: + * + * struct drm_gpuvm_bo *vm_bo; + * LIST_HEAD(my_local_list); + * + * ret = 0; + * drm_gpuvm_for_each_vm_bo(gpuvm, <list_name>, &my_local_list, vm_bo) { + * ret = do_something_with_vm_bo(..., vm_bo); + * if (ret) + * break; + * } + * drm_gpuvm_bo_put(vm_bo); + * drm_gpuvm_restore_vm_bo_list(gpuvm, <list_name>, &my_local_list); + * + * + * Only used for internal list iterations, not meant to be exposed to the outside + * world. + */ +#define for_each_vm_bo_in_list(__gpuvm, __list_name, __local_list, __vm_bo) \ + for (__vm_bo = get_next_vm_bo_from_list(__gpuvm, __list_name, \ + __local_list, NULL); \ + __vm_bo; \ + __vm_bo = get_next_vm_bo_from_list(__gpuvm, __list_name, \ + __local_list, __vm_bo)) \ + +/** + * restore_vm_bo_list() - move vm_bo elements back to their original list + * @__gpuvm: The GPU VM + * @__list_name: The name of the list we're iterating on + * @__local_list: A pointer to the local list used to store already iterated items + * + * When we're done iterating a vm_bo list, we should call restore_vm_bo_list() + * to restore the original state and let new iterations take place. + */ +#define restore_vm_bo_list(__gpuvm, __list_name, __local_list) \ + do { \ + /* Merge back the two lists, moving local list elements to the \ + * head to preserve previous ordering, in case it matters. \ + */ \ + spin_lock(&(__gpuvm)->__list_name.lock); \ + list_splice(__local_list, &(__gpuvm)->__list_name.list); \ + spin_unlock(&(__gpuvm)->__list_name.lock); \ + } while (0) +/** + * drm_gpuvm_bo_list_add() - insert a vm_bo into the given list + * @__vm_bo: the &drm_gpuvm_bo + * @__list_name: the name of the list to insert into + * + * Inserts the given @__vm_bo into the list specified by @__list_name and + * increases the vm_bo's reference count. + */ +#define drm_gpuvm_bo_list_add(__vm_bo, __list_name) \ + do { \ + spin_lock(&(__vm_bo)->vm->__list_name.lock); \ + if (list_empty(&(__vm_bo)->list.entry.__list_name)) \ + list_add_tail(&(__vm_bo)->list.entry.__list_name, \ + &(__vm_bo)->vm->__list_name.list); \ + spin_unlock(&(__vm_bo)->vm->__list_name.lock); \ + } while (0) + +/** + * drm_gpuvm_bo_list_del() - remove a vm_bo from the given list + * @__vm_bo: the &drm_gpuvm_bo + * @__list_name: the name of the list to insert into + * + * Removes the given @__vm_bo from the list specified by @__list_name and + * decreases the vm_bo's reference count. + */ +#define drm_gpuvm_bo_list_del(__vm_bo, __list_name) \ + do { \ + spin_lock(&(__vm_bo)->vm->__list_name.lock); \ + if (!list_empty(&(__vm_bo)->list.entry.__list_name)) \ + list_del_init(&(__vm_bo)->list.entry.__list_name); \ + spin_unlock(&(__vm_bo)->vm->__list_name.lock); \ + } while (0) + +static int __must_check +drm_gpuvm_bo_get_unless_zero(struct drm_gpuvm_bo *vm_bo); + #define to_drm_gpuva(__node) container_of((__node), struct drm_gpuva, rb.node) #define GPUVA_START(node) ((node)->va.addr) @@ -713,6 +867,12 @@ drm_gpuvm_init(struct drm_gpuvm *gpuvm, struct drm_device *drm, gpuvm->rb.tree = RB_ROOT_CACHED; INIT_LIST_HEAD(&gpuvm->rb.list); + INIT_LIST_HEAD(&gpuvm->extobj.list); + spin_lock_init(&gpuvm->extobj.lock); + + INIT_LIST_HEAD(&gpuvm->evict.list); + spin_lock_init(&gpuvm->evict.lock); + drm_gpuva_check_overflow(start_offset, range); gpuvm->mm_start = start_offset; gpuvm->mm_range = range; @@ -754,10 +914,302 @@ drm_gpuvm_destroy(struct drm_gpuvm *gpuvm) WARN(!RB_EMPTY_ROOT(&gpuvm->rb.tree.rb_root), "GPUVA tree is not empty, potentially leaking memory.\n"); + WARN(!list_empty(&gpuvm->extobj.list), "Extobj list should be empty.\n"); + WARN(!list_empty(&gpuvm->evict.list), "Evict list should be empty.\n"); + drm_gem_private_object_fini(&gpuvm->d_obj); } EXPORT_SYMBOL_GPL(drm_gpuvm_destroy); +/** + * drm_gpuvm_prepare_objects() - prepare all assoiciated BOs + * @gpuvm: the &drm_gpuvm + * @exec: the &drm_exec locking context + * @num_fences: the amount of &dma_fences to reserve + * + * Calls drm_exec_prepare_obj() for all &drm_gem_objects the given + * &drm_gpuvm contains mappings of. + * + * Using this function directly, it is the drivers responsibility to call + * drm_exec_init() and drm_exec_fini() accordingly. + * + * Note: This function is safe against concurrent insertion and removal of + * external objects, however it is not safe against concurrent usage itself. + * + * Drivers need to make sure to protect this case with either an outer VM lock + * or by calling drm_gpuvm_prepare_vm() before this function within the + * drm_exec_until_all_locked() loop, such that the GPUVM's dma-resv lock ensures + * mutual exclusion. + * + * Returns: 0 on success, negative error code on failure. + */ +int +drm_gpuvm_prepare_objects(struct drm_gpuvm *gpuvm, + struct drm_exec *exec, + unsigned int num_fences) +{ + struct drm_gpuvm_bo *vm_bo; + LIST_HEAD(extobjs); + int ret = 0; + + for_each_vm_bo_in_list(gpuvm, extobj, &extobjs, vm_bo) { + ret = drm_exec_prepare_obj(exec, vm_bo->obj, num_fences); + if (ret) + break; + } + /* Drop ref in case we break out of the loop. */ + drm_gpuvm_bo_put(vm_bo); + restore_vm_bo_list(gpuvm, extobj, &extobjs); + + return ret; +} +EXPORT_SYMBOL_GPL(drm_gpuvm_prepare_objects); + +/** + * drm_gpuvm_prepare_range() - prepare all BOs mapped within a given range + * @gpuvm: the &drm_gpuvm + * @exec: the &drm_exec locking context + * @addr: the start address within the VA space + * @range: the range to iterate within the VA space + * @num_fences: the amount of &dma_fences to reserve + * + * Calls drm_exec_prepare_obj() for all &drm_gem_objects mapped between @addr + * and @addr + @range. + * + * Returns: 0 on success, negative error code on failure. + */ +int +drm_gpuvm_prepare_range(struct drm_gpuvm *gpuvm, struct drm_exec *exec, + u64 addr, u64 range, unsigned int num_fences) +{ + struct drm_gpuva *va; + u64 end = addr + range; + int ret; + + drm_gpuvm_for_each_va_range(va, gpuvm, addr, end) { + struct drm_gem_object *obj = va->gem.obj; + + ret = drm_exec_prepare_obj(exec, obj, num_fences); + if (ret) + return ret; + } + + return 0; +} +EXPORT_SYMBOL_GPL(drm_gpuvm_prepare_range); + +/** + * drm_gpuvm_exec_lock() - lock all dma-resv of all assoiciated BOs + * @vm_exec: the &drm_gpuvm_exec abstraction + * @num_fences: the amount of &dma_fences to reserve + * @interruptible: sleep interruptible if waiting + * + * Acquires all dma-resv locks of all &drm_gem_objects the given + * &drm_gpuvm contains mappings of. + * + * Addionally, when calling this function with struct drm_gpuvm_exec::extra + * being set the driver receives the given @fn callback to lock additional + * dma-resv in the context of the &drm_gpuvm_exec instance. Typically, drivers + * would call drm_exec_prepare_obj() from within this callback. + * + * Returns: 0 on success, negative error code on failure. + */ +int +drm_gpuvm_exec_lock(struct drm_gpuvm_exec *vm_exec, + unsigned int num_fences, + bool interruptible) +{ + struct drm_gpuvm *gpuvm = vm_exec->vm; + struct drm_exec *exec = &vm_exec->exec; + uint32_t flags; + int ret; + + flags = interruptible ? DRM_EXEC_INTERRUPTIBLE_WAIT : 0 | + DRM_EXEC_IGNORE_DUPLICATES; + + drm_exec_init(exec, flags); + + drm_exec_until_all_locked(exec) { + ret = drm_gpuvm_prepare_vm(gpuvm, exec, num_fences); + drm_exec_retry_on_contention(exec); + if (ret) + goto err; + + ret = drm_gpuvm_prepare_objects(gpuvm, exec, num_fences); + drm_exec_retry_on_contention(exec); + if (ret) + goto err; + + if (vm_exec->extra.fn) { + ret = vm_exec->extra.fn(vm_exec, num_fences); + drm_exec_retry_on_contention(exec); + if (ret) + goto err; + } + } + + return 0; + +err: + drm_exec_fini(exec); + return ret; +} +EXPORT_SYMBOL_GPL(drm_gpuvm_exec_lock); + +static int +fn_lock_array(struct drm_gpuvm_exec *vm_exec, unsigned int num_fences) +{ + struct { + struct drm_gem_object **objs; + unsigned int num_objs; + } *args = vm_exec->extra.priv; + + return drm_exec_prepare_array(&vm_exec->exec, args->objs, + args->num_objs, num_fences); +} + +/** + * drm_gpuvm_exec_lock_array() - lock all dma-resv of all assoiciated BOs + * @vm_exec: the &drm_gpuvm_exec abstraction + * @objs: additional &drm_gem_objects to lock + * @num_objs: the number of additional &drm_gem_objects to lock + * @num_fences: the amount of &dma_fences to reserve + * @interruptible: sleep interruptible if waiting + * + * Acquires all dma-resv locks of all &drm_gem_objects the given &drm_gpuvm + * contains mappings of, plus the ones given through @objs. + * + * Returns: 0 on success, negative error code on failure. + */ +int +drm_gpuvm_exec_lock_array(struct drm_gpuvm_exec *vm_exec, + struct drm_gem_object **objs, + unsigned int num_objs, + unsigned int num_fences, + bool interruptible) +{ + struct { + struct drm_gem_object **objs; + unsigned int num_objs; + } args; + + args.objs = objs; + args.num_objs = num_objs; + + vm_exec->extra.fn = fn_lock_array; + vm_exec->extra.priv = &args; + + return drm_gpuvm_exec_lock(vm_exec, num_fences, interruptible); +} +EXPORT_SYMBOL_GPL(drm_gpuvm_exec_lock_array); + +/** + * drm_gpuvm_exec_lock_range() - prepare all BOs mapped within a given range + * @vm_exec: the &drm_gpuvm_exec abstraction + * @addr: the start address within the VA space + * @range: the range to iterate within the VA space + * @num_fences: the amount of &dma_fences to reserve + * @interruptible: sleep interruptible if waiting + * + * Acquires all dma-resv locks of all &drm_gem_objects mapped between @addr and + * @addr + @range. + * + * Returns: 0 on success, negative error code on failure. + */ +int +drm_gpuvm_exec_lock_range(struct drm_gpuvm_exec *vm_exec, + u64 addr, u64 range, + unsigned int num_fences, + bool interruptible) +{ + struct drm_gpuvm *gpuvm = vm_exec->vm; + struct drm_exec *exec = &vm_exec->exec; + uint32_t flags; + int ret; + + flags = interruptible ? DRM_EXEC_INTERRUPTIBLE_WAIT : 0 | + DRM_EXEC_IGNORE_DUPLICATES; + + drm_exec_init(exec, flags); + + drm_exec_until_all_locked(exec) { + ret = drm_gpuvm_prepare_range(gpuvm, exec, addr, range, + num_fences); + drm_exec_retry_on_contention(exec); + if (ret) + goto err; + } + + return ret; + +err: + drm_exec_fini(exec); + return ret; +} +EXPORT_SYMBOL_GPL(drm_gpuvm_exec_lock_range); + +/** + * drm_gpuvm_validate() - validate all BOs marked as evicted + * @gpuvm: the &drm_gpuvm to validate evicted BOs + * + * Calls the &drm_gpuvm_ops.bo_validate callback for all evicted buffer + * objects being mapped in the given &drm_gpuvm. + * + * Returns: 0 on success, negative error code on failure. + */ +int +drm_gpuvm_validate(struct drm_gpuvm *gpuvm) +{ + const struct drm_gpuvm_ops *ops = gpuvm->ops; + struct drm_gpuvm_bo *vm_bo; + LIST_HEAD(evict); + int ret = 0; + + if (unlikely(!ops || !ops->bo_validate)) + return -ENOTSUPP; + + for_each_vm_bo_in_list(gpuvm, evict, &evict, vm_bo) { + dma_resv_assert_held(vm_bo->obj->resv); + ret = ops->bo_validate(vm_bo->obj); + if (ret) + break; + } + /* Drop ref in case we break out of the loop. */ + drm_gpuvm_bo_put(vm_bo); + restore_vm_bo_list(gpuvm, evict, &evict); + + return ret; +} +EXPORT_SYMBOL_GPL(drm_gpuvm_validate); + +/** + * drm_gpuvm_resv_add_fence - add fence to private and all extobj + * dma-resv + * @gpuvm: the &drm_gpuvm to add a fence to + * @exec: the &drm_exec locking context + * @fence: fence to add + * @private_usage: private dma-resv usage + * @extobj_usage: extobj dma-resv usage + */ +void +drm_gpuvm_resv_add_fence(struct drm_gpuvm *gpuvm, + struct drm_exec *exec, + struct dma_fence *fence, + enum dma_resv_usage private_usage, + enum dma_resv_usage extobj_usage) +{ + struct drm_gem_object *obj; + unsigned long index; + + drm_exec_for_each_locked_object(exec, index, obj) { + dma_resv_assert_held(obj->resv); + dma_resv_add_fence(obj->resv, fence, + drm_gpuvm_is_extobj(gpuvm, obj) ? + private_usage : extobj_usage); + } +} +EXPORT_SYMBOL_GPL(drm_gpuvm_resv_add_fence); + /** * drm_gpuvm_bo_create() - create a new instance of struct drm_gpuvm_bo * @gpuvm: The &drm_gpuvm the @obj is mapped in. @@ -790,6 +1242,9 @@ drm_gpuvm_bo_create(struct drm_gpuvm *gpuvm, INIT_LIST_HEAD(&vm_bo->list.gpuva); INIT_LIST_HEAD(&vm_bo->list.entry.gem); + INIT_LIST_HEAD(&vm_bo->list.entry.extobj); + INIT_LIST_HEAD(&vm_bo->list.entry.evict); + drm_gem_object_get(obj); return vm_bo; @@ -807,6 +1262,14 @@ drm_gpuvm_bo_destroy(struct kref *kref) drm_gem_gpuva_assert_lock_held(vm_bo->obj); + spin_lock(&gpuvm->extobj.lock); + list_del(&vm_bo->list.entry.extobj); + spin_unlock(&gpuvm->extobj.lock); + + spin_lock(&gpuvm->evict.lock); + list_del(&vm_bo->list.entry.evict); + spin_unlock(&gpuvm->evict.lock); + list_del(&vm_bo->list.entry.gem); drm_gem_object_put(obj); @@ -822,6 +1285,11 @@ drm_gpuvm_bo_destroy(struct kref *kref) * @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) @@ -831,6 +1299,12 @@ drm_gpuvm_bo_put(struct drm_gpuvm_bo *vm_bo) } EXPORT_SYMBOL_GPL(drm_gpuvm_bo_put); +static int __must_check +drm_gpuvm_bo_get_unless_zero(struct drm_gpuvm_bo *vm_bo) +{ + return kref_get_unless_zero(&vm_bo->kref); +} + static struct drm_gpuvm_bo * __drm_gpuvm_bo_find(struct drm_gpuvm *gpuvm, struct drm_gem_object *obj) @@ -938,6 +1412,48 @@ drm_gpuvm_bo_obtain_prealloc(struct drm_gpuvm_bo *__vm_bo) } EXPORT_SYMBOL_GPL(drm_gpuvm_bo_obtain_prealloc); +/** + * drm_gpuvm_bo_extobj_add() - adds the &drm_gpuvm_bo to its &drm_gpuvm's + * extobj list + * @vm_bo: The &drm_gpuvm_bo to add to its &drm_gpuvm's the extobj list. + * + * Adds the given @vm_bo to its &drm_gpuvm's extobj list if not on the list + * already and if the corresponding &drm_gem_object is an external object, + * actually. + */ +void +drm_gpuvm_bo_extobj_add(struct drm_gpuvm_bo *vm_bo) +{ + struct drm_gpuvm *gpuvm = vm_bo->vm; + + if (drm_gpuvm_is_extobj(gpuvm, vm_bo->obj)) + drm_gpuvm_bo_list_add(vm_bo, extobj); +} +EXPORT_SYMBOL_GPL(drm_gpuvm_bo_extobj_add); + +/** + * drm_gpuvm_bo_evict() - add / remove a &drm_gem_object to / from a + * &drm_gpuvms evicted list + * @obj: the &drm_gem_object to add or remove + * @evict: indicates whether the object is evicted + * + * Adds a &drm_gem_object to or removes it from all &drm_gpuvms evicted + * list containing a mapping of this &drm_gem_object. + */ +void +drm_gpuvm_bo_evict(struct drm_gem_object *obj, bool evict) +{ + struct drm_gpuvm_bo *vm_bo; + + drm_gem_for_each_gpuvm_bo(vm_bo, obj) { + if (evict) + drm_gpuvm_bo_list_add(vm_bo, evict); + else + drm_gpuvm_bo_list_del(vm_bo, evict); + } +} +EXPORT_SYMBOL_GPL(drm_gpuvm_bo_evict); + static int __drm_gpuva_insert(struct drm_gpuvm *gpuvm, struct drm_gpuva *va) diff --git a/include/drm/drm_gpuvm.h b/include/drm/drm_gpuvm.h index afa50b9059a2..834bb6d6617e 100644 --- a/include/drm/drm_gpuvm.h +++ b/include/drm/drm_gpuvm.h @@ -26,10 +26,12 @@ */ #include <linux/list.h> +#include <linux/dma-resv.h> #include <linux/rbtree.h> #include <linux/types.h> #include <drm/drm_gem.h> +#include <drm/drm_exec.h> struct drm_gpuvm; struct drm_gpuvm_bo; @@ -259,6 +261,38 @@ struct drm_gpuvm { * space */ struct dma_resv *resv; + + /** + * @extobj: structure holding the extobj list + */ + struct { + /** + * @list: &list_head storing &drm_gpuvm_bos serving as + * external object + */ + struct list_head list; + + /** + * @lock: spinlock to protect the extobj list + */ + spinlock_t lock; + } extobj; + + /** + * @evict: structure holding the evict list and evict list lock + */ + struct { + /** + * @list: &list_head storing &drm_gpuvm_bos currently being + * evicted + */ + struct list_head list; + + /** + * @lock: spinlock to protect the evict list + */ + spinlock_t lock; + } evict; }; void drm_gpuvm_init(struct drm_gpuvm *gpuvm, struct drm_device *drm, @@ -268,6 +302,21 @@ void drm_gpuvm_init(struct drm_gpuvm *gpuvm, struct drm_device *drm, const struct drm_gpuvm_ops *ops); void drm_gpuvm_destroy(struct drm_gpuvm *gpuvm); +/** + * drm_gpuvm_is_extobj() - indicates whether the given &drm_gem_object is an + * external object + * @gpuvm: the &drm_gpuvm to check + * @obj: the &drm_gem_object to check + * + * Returns: true if the &drm_gem_object &dma_resv differs from the + * &drm_gpuvms &dma_resv, false otherwise + */ +static inline bool drm_gpuvm_is_extobj(struct drm_gpuvm *gpuvm, + struct drm_gem_object *obj) +{ + return obj && obj->resv != gpuvm->resv; +} + static inline struct drm_gpuva * __drm_gpuva_next(struct drm_gpuva *va) { @@ -346,6 +395,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_exec - &drm_gpuvm abstraction of &drm_exec + * + * This structure should be created on the stack as &drm_exec should be. + * + * Optionally, @extra can be set in order to lock additional &drm_gem_objects. + */ +struct drm_gpuvm_exec { + /** + * @exec: the &drm_exec structure + */ + struct drm_exec exec; + + /** + * @vm: the &drm_gpuvm to lock its DMA reservations + */ + struct drm_gpuvm *vm; + + /** + * @extra: Callback and corresponding private data for the driver to + * lock arbitrary additional &drm_gem_objects. + */ + struct { + /** + * @fn: The driver callback to lock additional &drm_gem_objects. + */ + int (*fn)(struct drm_gpuvm_exec *vm_exec, + unsigned int num_fences); + + /** + * @priv: driver private data for the @fn callback + */ + void *priv; + } extra; +}; + +/** + * drm_gpuvm_prepare_vm() - prepare the GPUVMs common dma-resv + * @gpuvm: the &drm_gpuvm + * @exec: the &drm_exec context + * @num_fences: the amount of &dma_fences to reserve + * + * Calls drm_exec_prepare_obj() for the GPUVMs dummy &drm_gem_object. + * + * Using this function directly, it is the drivers responsibility to call + * drm_exec_init() and drm_exec_fini() accordingly. + * + * Returns: 0 on success, negative error code on failure. + */ +static inline int +drm_gpuvm_prepare_vm(struct drm_gpuvm *gpuvm, + struct drm_exec *exec, + unsigned int num_fences) +{ + return drm_exec_prepare_obj(exec, &gpuvm->d_obj, num_fences); +} + +int drm_gpuvm_prepare_objects(struct drm_gpuvm *gpuvm, + struct drm_exec *exec, + unsigned int num_fences); + +int drm_gpuvm_prepare_range(struct drm_gpuvm *gpuvm, + struct drm_exec *exec, + u64 addr, u64 range, + unsigned int num_fences); + +int drm_gpuvm_exec_lock(struct drm_gpuvm_exec *vm_exec, + unsigned int num_fences, + bool interruptible); + +int drm_gpuvm_exec_lock_array(struct drm_gpuvm_exec *vm_exec, + struct drm_gem_object **objs, + unsigned int num_objs, + unsigned int num_fences, + bool interruptible); + +int drm_gpuvm_exec_lock_range(struct drm_gpuvm_exec *vm_exec, + u64 addr, u64 range, + unsigned int num_fences, + bool interruptible); + +/** + * drm_gpuvm_lock() - lock all dma-resv of all assoiciated BOs + * @gpuvm: the &drm_gpuvm + * + * Releases all dma-resv locks of all &drm_gem_objects previously acquired + * through drm_gpuvm_lock() or its variants. + * + * Returns: 0 on success, negative error code on failure. + */ +static inline void +drm_gpuvm_exec_unlock(struct drm_gpuvm_exec *vm_exec) +{ + drm_exec_fini(&vm_exec->exec); +} + +int drm_gpuvm_validate(struct drm_gpuvm *gpuvm); +void drm_gpuvm_resv_add_fence(struct drm_gpuvm *gpuvm, + struct drm_exec *exec, + struct dma_fence *fence, + enum dma_resv_usage private_usage, + enum dma_resv_usage extobj_usage); + +/** + * drm_gpuvm_exec_resv_add_fence() + * @vm_exec: the &drm_gpuvm_exec abstraction + * @fence: fence to add + * @private_usage: private dma-resv usage + * @extobj_usage: extobj dma-resv usage + * + * See drm_gpuvm_resv_add_fence(). + */ +static inline void +drm_gpuvm_exec_resv_add_fence(struct drm_gpuvm_exec *vm_exec, + struct dma_fence *fence, + enum dma_resv_usage private_usage, + enum dma_resv_usage extobj_usage) +{ + drm_gpuvm_resv_add_fence(vm_exec->vm, &vm_exec->exec, fence, + private_usage, extobj_usage); +} + /** * struct drm_gpuvm_bo - structure representing a &drm_gpuvm and * &drm_gem_object combination @@ -398,6 +569,18 @@ struct drm_gpuvm_bo { * gpuva list. */ struct list_head gem; + + /** + * @evict: List entry to attach to the &drm_gpuvms + * extobj list. + */ + struct list_head extobj; + + /** + * @evict: List entry to attach to the &drm_gpuvms evict + * list. + */ + struct list_head evict; } entry; } list; }; @@ -432,6 +615,9 @@ struct drm_gpuvm_bo * drm_gpuvm_bo_find(struct drm_gpuvm *gpuvm, struct drm_gem_object *obj); +void drm_gpuvm_bo_evict(struct drm_gem_object *obj, bool evict); +void drm_gpuvm_bo_extobj_add(struct drm_gpuvm_bo *vm_bo); + /** * 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 @@ -837,6 +1023,17 @@ struct drm_gpuvm_ops { * used. */ int (*sm_step_unmap)(struct drm_gpuva_op *op, void *priv); + + /** + * @bo_validate: called from drm_gpuvm_validate() + * + * Drivers receive this callback for every evicted &drm_gem_object being + * mapped in the corresponding &drm_gpuvm. + * + * Typically, drivers would call their driver specific variant of + * ttm_bo_validate() from within this callback. + */ + int (*bo_validate)(struct drm_gem_object *obj); }; int drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm, void *priv, -- 2.41.0
kernel test robot
2023-Sep-09 20:16 UTC
[Nouveau] [PATCH drm-misc-next v3 6/7] drm/gpuvm: generalize dma_resv/extobj handling and GEM validation
Hi Danilo, kernel test robot noticed the following build warnings: [auto build test WARNING on 6bd3d8da51ca1ec97c724016466606aec7739b9f] url: https://github.com/intel-lab-lkp/linux/commits/Danilo-Krummrich/drm-gpuvm-rename-struct-drm_gpuva_manager-to-struct-drm_gpuvm/20230909-233346 base: 6bd3d8da51ca1ec97c724016466606aec7739b9f patch link: https://lore.kernel.org/r/20230909153125.30032-7-dakr%40redhat.com patch subject: [PATCH drm-misc-next v3 6/7] drm/gpuvm: generalize dma_resv/extobj handling and GEM validation config: riscv-defconfig (https://download.01.org/0day-ci/archive/20230910/202309100424.uNXGR9d4-lkp at intel.com/config) compiler: riscv64-linux-gcc (GCC) 13.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230910/202309100424.uNXGR9d4-lkp at intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp at intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202309100424.uNXGR9d4-lkp at intel.com/ All warnings (new ones prefixed by >>):>> drivers/gpu/drm/drm_gpuvm.c:734: warning: Function parameter or member '__gpuvm' not described in 'for_each_vm_bo_in_list' >> drivers/gpu/drm/drm_gpuvm.c:734: warning: Function parameter or member '__list_name' not described in 'for_each_vm_bo_in_list' >> drivers/gpu/drm/drm_gpuvm.c:734: warning: Function parameter or member '__local_list' not described in 'for_each_vm_bo_in_list' >> drivers/gpu/drm/drm_gpuvm.c:734: warning: Function parameter or member '__vm_bo' not described in 'for_each_vm_bo_in_list'vim +734 drivers/gpu/drm/drm_gpuvm.c 32 33 /** 34 * DOC: Overview 35 * 36 * The DRM GPU VA Manager, represented by struct drm_gpuvm keeps track of a 37 * GPU's virtual address (VA) space and manages the corresponding virtual 38 * mappings represented by &drm_gpuva objects. It also keeps track of the 39 * mapping's backing &drm_gem_object buffers. 40 * 41 * &drm_gem_object buffers maintain a list of &drm_gpuva objects representing 42 * all existent GPU VA mappings using this &drm_gem_object as backing buffer. 43 * 44 * GPU VAs can be flagged as sparse, such that drivers may use GPU VAs to also 45 * keep track of sparse PTEs in order to support Vulkan 'Sparse Resources'. 46 * 47 * The GPU VA manager internally uses a rb-tree to manage the 48 * &drm_gpuva mappings within a GPU's virtual address space. 49 * 50 * The &drm_gpuvm structure contains a special &drm_gpuva representing the 51 * portion of VA space reserved by the kernel. This node is initialized together 52 * with the GPU VA manager instance and removed when the GPU VA manager is 53 * destroyed. 54 * 55 * In a typical application drivers would embed struct drm_gpuvm and 56 * struct drm_gpuva within their own driver specific structures, there won't be 57 * any memory allocations of its own nor memory allocations of &drm_gpuva 58 * entries. 59 * 60 * The data structures needed to store &drm_gpuvas within the &drm_gpuvm are 61 * contained within struct drm_gpuva already. Hence, for inserting &drm_gpuva 62 * entries from within dma-fence signalling critical sections it is enough to 63 * pre-allocate the &drm_gpuva structures. 64 * 65 * In order to connect a struct drm_gpuva its backing &drm_gem_object each 66 * &drm_gem_object maintains a list of &drm_gpuvm_bo structures, and each 67 * &drm_gpuvm_bo contains a list of &&drm_gpuva structures. 68 * 69 * A &drm_gpuvm_bo is an abstraction that represents a combination of a 70 * &drm_gpuvm and a &drm_gem_object. Every such combination should be unique. 71 * This is ensured by the API through drm_gpuvm_bo_obtain() and 72 * drm_gpuvm_bo_obtain_prealloc() which first look into the corresponding 73 * &drm_gem_object list of &drm_gpuvm_bos for an existing instance of this 74 * particular combination. If not existent a new instance is created and linked 75 * to the &drm_gem_object. 76 * 77 * &drm_gpuvm_bo structures, since unique for a given &drm_gpuvm, are also used 78 * as entry for the &drm_gpuvm's lists of external and evicted objects. Those 79 * list are maintained in order to accelerate locking of dma-resv locks and 80 * validation of evicted objects bound in a &drm_gpuvm. For instance the all 81 * &drm_gem_object's &dma_resv of a given &drm_gpuvm can be locked by calling 82 * drm_gpuvm_exec_lock(). Once locked drivers can call drm_gpuvm_validate() in 83 * order to validate all evicted &drm_gem_objects. It is also possible to lock 84 * additional &drm_gem_objects by providing the corresponding parameters to 85 * drm_gpuvm_exec_lock() as well as open code the &drm_exec loop while making 86 * use of helper functions such as drm_gpuvm_prepare_range() or 87 * drm_gpuvm_prepare_objects(). 88 * 89 * Every bound &drm_gem_object is treated as external object when its &dma_resv 90 * structure is different than the &drm_gpuvm's common &dma_resv structure. 91 */ 92 93 /** 94 * DOC: Split and Merge 95 * 96 * Besides its capability to manage and represent a GPU VA space, the 97 * GPU VA manager also provides functions to let the &drm_gpuvm calculate a 98 * sequence of operations to satisfy a given map or unmap request. 99 * 100 * Therefore the DRM GPU VA manager provides an algorithm implementing splitting 101 * and merging of existent GPU VA mappings with the ones that are requested to 102 * be mapped or unmapped. This feature is required by the Vulkan API to 103 * implement Vulkan 'Sparse Memory Bindings' - drivers UAPIs often refer to this 104 * as VM BIND. 105 * 106 * Drivers can call drm_gpuvm_sm_map() to receive a sequence of callbacks 107 * containing map, unmap and remap operations for a given newly requested 108 * mapping. The sequence of callbacks represents the set of operations to 109 * execute in order to integrate the new mapping cleanly into the current state 110 * of the GPU VA space. 111 * 112 * Depending on how the new GPU VA mapping intersects with the existent mappings 113 * of the GPU VA space the &drm_gpuvm_ops callbacks contain an arbitrary amount 114 * of unmap operations, a maximum of two remap operations and a single map 115 * operation. The caller might receive no callback at all if no operation is 116 * required, e.g. if the requested mapping already exists in the exact same way. 117 * 118 * The single map operation represents the original map operation requested by 119 * the caller. 120 * 121 * &drm_gpuva_op_unmap contains a 'keep' field, which indicates whether the 122 * &drm_gpuva to unmap is physically contiguous with the original mapping 123 * request. Optionally, if 'keep' is set, drivers may keep the actual page table 124 * entries for this &drm_gpuva, adding the missing page table entries only and 125 * update the &drm_gpuvm's view of things accordingly. 126 * 127 * Drivers may do the same optimization, namely delta page table updates, also 128 * for remap operations. This is possible since &drm_gpuva_op_remap consists of 129 * one unmap operation and one or two map operations, such that drivers can 130 * derive the page table update delta accordingly. 131 * 132 * Note that there can't be more than two existent mappings to split up, one at 133 * the beginning and one at the end of the new mapping, hence there is a 134 * maximum of two remap operations. 135 * 136 * Analogous to drm_gpuvm_sm_map() drm_gpuvm_sm_unmap() uses &drm_gpuvm_ops to 137 * call back into the driver in order to unmap a range of GPU VA space. The 138 * logic behind this function is way simpler though: For all existent mappings 139 * enclosed by the given range unmap operations are created. For mappings which 140 * are only partically located within the given range, remap operations are 141 * created such that those mappings are split up and re-mapped partically. 142 * 143 * As an alternative to drm_gpuvm_sm_map() and drm_gpuvm_sm_unmap(), 144 * drm_gpuvm_sm_map_ops_create() and drm_gpuvm_sm_unmap_ops_create() can be used 145 * to directly obtain an instance of struct drm_gpuva_ops containing a list of 146 * &drm_gpuva_op, which can be iterated with drm_gpuva_for_each_op(). This list 147 * contains the &drm_gpuva_ops analogous to the callbacks one would receive when 148 * calling drm_gpuvm_sm_map() or drm_gpuvm_sm_unmap(). While this way requires 149 * more memory (to allocate the &drm_gpuva_ops), it provides drivers a way to 150 * iterate the &drm_gpuva_op multiple times, e.g. once in a context where memory 151 * allocations are possible (e.g. to allocate GPU page tables) and once in the 152 * dma-fence signalling critical path. 153 * 154 * To update the &drm_gpuvm's view of the GPU VA space drm_gpuva_insert() and 155 * drm_gpuva_remove() may be used. These functions can safely be used from 156 * &drm_gpuvm_ops callbacks originating from drm_gpuvm_sm_map() or 157 * drm_gpuvm_sm_unmap(). However, it might be more convenient to use the 158 * provided helper functions drm_gpuva_map(), drm_gpuva_remap() and 159 * drm_gpuva_unmap() instead. 160 * 161 * The following diagram depicts the basic relationships of existent GPU VA 162 * mappings, a newly requested mapping and the resulting mappings as implemented 163 * by drm_gpuvm_sm_map() - it doesn't cover any arbitrary combinations of these. 164 * 165 * 1) Requested mapping is identical. Replace it, but indicate the backing PTEs 166 * could be kept. 167 * 168 * :: 169 * 170 * 0 a 1 171 * old: |-----------| (bo_offset=n) 172 * 173 * 0 a 1 174 * req: |-----------| (bo_offset=n) 175 * 176 * 0 a 1 177 * new: |-----------| (bo_offset=n) 178 * 179 * 180 * 2) Requested mapping is identical, except for the BO offset, hence replace 181 * the mapping. 182 * 183 * :: 184 * 185 * 0 a 1 186 * old: |-----------| (bo_offset=n) 187 * 188 * 0 a 1 189 * req: |-----------| (bo_offset=m) 190 * 191 * 0 a 1 192 * new: |-----------| (bo_offset=m) 193 * 194 * 195 * 3) Requested mapping is identical, except for the backing BO, hence replace 196 * the mapping. 197 * 198 * :: 199 * 200 * 0 a 1 201 * old: |-----------| (bo_offset=n) 202 * 203 * 0 b 1 204 * req: |-----------| (bo_offset=n) 205 * 206 * 0 b 1 207 * new: |-----------| (bo_offset=n) 208 * 209 * 210 * 4) Existent mapping is a left aligned subset of the requested one, hence 211 * replace the existent one. 212 * 213 * :: 214 * 215 * 0 a 1 216 * old: |-----| (bo_offset=n) 217 * 218 * 0 a 2 219 * req: |-----------| (bo_offset=n) 220 * 221 * 0 a 2 222 * new: |-----------| (bo_offset=n) 223 * 224 * .. note:: 225 * We expect to see the same result for a request with a different BO 226 * and/or non-contiguous BO offset. 227 * 228 * 229 * 5) Requested mapping's range is a left aligned subset of the existent one, 230 * but backed by a different BO. Hence, map the requested mapping and split 231 * the existent one adjusting its BO offset. 232 * 233 * :: 234 * 235 * 0 a 2 236 * old: |-----------| (bo_offset=n) 237 * 238 * 0 b 1 239 * req: |-----| (bo_offset=n) 240 * 241 * 0 b 1 a' 2 242 * new: |-----|-----| (b.bo_offset=n, a.bo_offset=n+1) 243 * 244 * .. note:: 245 * We expect to see the same result for a request with a different BO 246 * and/or non-contiguous BO offset. 247 * 248 * 249 * 6) Existent mapping is a superset of the requested mapping. Split it up, but 250 * indicate that the backing PTEs could be kept. 251 * 252 * :: 253 * 254 * 0 a 2 255 * old: |-----------| (bo_offset=n) 256 * 257 * 0 a 1 258 * req: |-----| (bo_offset=n) 259 * 260 * 0 a 1 a' 2 261 * new: |-----|-----| (a.bo_offset=n, a'.bo_offset=n+1) 262 * 263 * 264 * 7) Requested mapping's range is a right aligned subset of the existent one, 265 * but backed by a different BO. Hence, map the requested mapping and split 266 * the existent one, without adjusting the BO offset. 267 * 268 * :: 269 * 270 * 0 a 2 271 * old: |-----------| (bo_offset=n) 272 * 273 * 1 b 2 274 * req: |-----| (bo_offset=m) 275 * 276 * 0 a 1 b 2 277 * new: |-----|-----| (a.bo_offset=n,b.bo_offset=m) 278 * 279 * 280 * 8) Existent mapping is a superset of the requested mapping. Split it up, but 281 * indicate that the backing PTEs could be kept. 282 * 283 * :: 284 * 285 * 0 a 2 286 * old: |-----------| (bo_offset=n) 287 * 288 * 1 a 2 289 * req: |-----| (bo_offset=n+1) 290 * 291 * 0 a' 1 a 2 292 * new: |-----|-----| (a'.bo_offset=n, a.bo_offset=n+1) 293 * 294 * 295 * 9) Existent mapping is overlapped at the end by the requested mapping backed 296 * by a different BO. Hence, map the requested mapping and split up the 297 * existent one, without adjusting the BO offset. 298 * 299 * :: 300 * 301 * 0 a 2 302 * old: |-----------| (bo_offset=n) 303 * 304 * 1 b 3 305 * req: |-----------| (bo_offset=m) 306 * 307 * 0 a 1 b 3 308 * new: |-----|-----------| (a.bo_offset=n,b.bo_offset=m) 309 * 310 * 311 * 10) Existent mapping is overlapped by the requested mapping, both having the 312 * same backing BO with a contiguous offset. Indicate the backing PTEs of 313 * the old mapping could be kept. 314 * 315 * :: 316 * 317 * 0 a 2 318 * old: |-----------| (bo_offset=n) 319 * 320 * 1 a 3 321 * req: |-----------| (bo_offset=n+1) 322 * 323 * 0 a' 1 a 3 324 * new: |-----|-----------| (a'.bo_offset=n, a.bo_offset=n+1) 325 * 326 * 327 * 11) Requested mapping's range is a centered subset of the existent one 328 * having a different backing BO. Hence, map the requested mapping and split 329 * up the existent one in two mappings, adjusting the BO offset of the right 330 * one accordingly. 331 * 332 * :: 333 * 334 * 0 a 3 335 * old: |-----------------| (bo_offset=n) 336 * 337 * 1 b 2 338 * req: |-----| (bo_offset=m) 339 * 340 * 0 a 1 b 2 a' 3 341 * new: |-----|-----|-----| (a.bo_offset=n,b.bo_offset=m,a'.bo_offset=n+2) 342 * 343 * 344 * 12) Requested mapping is a contiguous subset of the existent one. Split it 345 * up, but indicate that the backing PTEs could be kept. 346 * 347 * :: 348 * 349 * 0 a 3 350 * old: |-----------------| (bo_offset=n) 351 * 352 * 1 a 2 353 * req: |-----| (bo_offset=n+1) 354 * 355 * 0 a' 1 a 2 a'' 3 356 * old: |-----|-----|-----| (a'.bo_offset=n, a.bo_offset=n+1, a''.bo_offset=n+2) 357 * 358 * 359 * 13) Existent mapping is a right aligned subset of the requested one, hence 360 * replace the existent one. 361 * 362 * :: 363 * 364 * 1 a 2 365 * old: |-----| (bo_offset=n+1) 366 * 367 * 0 a 2 368 * req: |-----------| (bo_offset=n) 369 * 370 * 0 a 2 371 * new: |-----------| (bo_offset=n) 372 * 373 * .. note:: 374 * We expect to see the same result for a request with a different bo 375 * and/or non-contiguous bo_offset. 376 * 377 * 378 * 14) Existent mapping is a centered subset of the requested one, hence 379 * replace the existent one. 380 * 381 * :: 382 * 383 * 1 a 2 384 * old: |-----| (bo_offset=n+1) 385 * 386 * 0 a 3 387 * req: |----------------| (bo_offset=n) 388 * 389 * 0 a 3 390 * new: |----------------| (bo_offset=n) 391 * 392 * .. note:: 393 * We expect to see the same result for a request with a different bo 394 * and/or non-contiguous bo_offset. 395 * 396 * 397 * 15) Existent mappings is overlapped at the beginning by the requested mapping 398 * backed by a different BO. Hence, map the requested mapping and split up 399 * the existent one, adjusting its BO offset accordingly. 400 * 401 * :: 402 * 403 * 1 a 3 404 * old: |-----------| (bo_offset=n) 405 * 406 * 0 b 2 407 * req: |-----------| (bo_offset=m) 408 * 409 * 0 b 2 a' 3 410 * new: |-----------|-----| (b.bo_offset=m,a.bo_offset=n+2) 411 */ 412 413 /** 414 * DOC: Locking 415 * 416 * Generally, the GPU VA manager does not take care of locking itself, it is 417 * the drivers responsibility to take care about locking. Drivers might want to 418 * protect the following operations: inserting, removing and iterating 419 * &drm_gpuva objects as well as generating all kinds of operations, such as 420 * split / merge or prefetch. 421 * 422 * The GPU VA manager also does not take care of the locking of the backing 423 * &drm_gem_object buffers GPU VA lists and &drm_gpuvm_bo abstractions by 424 * itself; drivers are responsible to enforce mutual exclusion using either the 425 * GEMs dma_resv lock or alternatively a driver specific external lock. For the 426 * latter see also drm_gem_gpuva_set_lock(). 427 * 428 * However, the GPU VA manager contains lockdep checks to ensure callers of its 429 * API hold the corresponding lock whenever the &drm_gem_objects GPU VA list is 430 * accessed by functions such as drm_gpuva_link() or drm_gpuva_unlink(), but 431 * also drm_gpuvm_bo_obtain() and drm_gpuvm_bo_put(). 432 * 433 * The latter is required since on creation and destruction of a &drm_gpuvm_bo 434 * the &drm_gpuvm_bo is attached / removed from the &drm_gem_objects gpuva list. 435 * Subsequent calls to drm_gpuvm_bo_obtain() for the same &drm_gpuvm and 436 * &drm_gem_object must be able to observe previous creations and destructions 437 * of &drm_gpuvm_bos in order to keep instances unique. 438 * 439 * The &drm_gpuvm's lists for keeping track of external and evicted objects are 440 * protected against concurrent insertion / removal and iteration internally. 441 * 442 * However, drivers still need ensure to protect concurrent calls to functions 443 * iterating those lists, such as drm_gpuvm_validate() and 444 * drm_gpuvm_prepare_objects(). Every such function contains a particular 445 * comment and lockdep checks if possible. 446 * 447 * Functions adding or removing entries from those lists, such as 448 * drm_gpuvm_bo_evict() or drm_gpuvm_bo_extobj_add() may be called with external 449 * locks being held, e.g. in order to avoid the corresponding list to be 450 * (safely) modified while potentially being iternated by other API functions. 451 * However, this is entirely optional. 452 */ 453 454 /** 455 * DOC: Examples 456 * 457 * This section gives two examples on how to let the DRM GPUVA Manager generate 458 * &drm_gpuva_op in order to satisfy a given map or unmap request and how to 459 * make use of them. 460 * 461 * The below code is strictly limited to illustrate the generic usage pattern. 462 * To maintain simplicitly, it doesn't make use of any abstractions for common 463 * code, different (asyncronous) stages with fence signalling critical paths, 464 * any other helpers or error handling in terms of freeing memory and dropping 465 * previously taken locks. 466 * 467 * 1) Obtain a list of &drm_gpuva_op to create a new mapping:: 468 * 469 * // Allocates a new &drm_gpuva. 470 * struct drm_gpuva * driver_gpuva_alloc(void); 471 * 472 * // Typically drivers would embedd the &drm_gpuvm and &drm_gpuva 473 * // structure in individual driver structures and lock the dma-resv with 474 * // drm_exec or similar helpers. 475 * int driver_mapping_create(struct drm_gpuvm *gpuvm, 476 * u64 addr, u64 range, 477 * struct drm_gem_object *obj, u64 offset) 478 * { 479 * struct drm_gpuva_ops *ops; 480 * struct drm_gpuva_op *op 481 * struct drm_gpuvm_bo *vm_bo; 482 * 483 * driver_lock_va_space(); 484 * ops = drm_gpuvm_sm_map_ops_create(gpuvm, addr, range, 485 * obj, offset); 486 * if (IS_ERR(ops)) 487 * return PTR_ERR(ops); 488 * 489 * vm_bo = drm_gpuvm_bo_obtain(gpuvm, obj); 490 * if (IS_ERR(vm_bo)) 491 * return PTR_ERR(vm_bo); 492 * 493 * drm_gpuva_for_each_op(op, ops) { 494 * struct drm_gpuva *va; 495 * 496 * switch (op->op) { 497 * case DRM_GPUVA_OP_MAP: 498 * va = driver_gpuva_alloc(); 499 * if (!va) 500 * ; // unwind previous VA space updates, 501 * // free memory and unlock 502 * 503 * driver_vm_map(); 504 * drm_gpuva_map(gpuvm, va, &op->map); 505 * drm_gpuva_link(va, vm_bo); 506 * 507 * break; 508 * case DRM_GPUVA_OP_REMAP: { 509 * struct drm_gpuva *prev = NULL, *next = NULL; 510 * 511 * va = op->remap.unmap->va; 512 * 513 * if (op->remap.prev) { 514 * prev = driver_gpuva_alloc(); 515 * if (!prev) 516 * ; // unwind previous VA space 517 * // updates, free memory and 518 * // unlock 519 * } 520 * 521 * if (op->remap.next) { 522 * next = driver_gpuva_alloc(); 523 * if (!next) 524 * ; // unwind previous VA space 525 * // updates, free memory and 526 * // unlock 527 * } 528 * 529 * driver_vm_remap(); 530 * drm_gpuva_remap(prev, next, &op->remap); 531 * 532 * if (prev) 533 * drm_gpuva_link(prev, va->vm_bo); 534 * if (next) 535 * drm_gpuva_link(next, va->vm_bo); 536 * drm_gpuva_unlink(va); 537 * 538 * break; 539 * } 540 * case DRM_GPUVA_OP_UNMAP: 541 * va = op->unmap->va; 542 * 543 * driver_vm_unmap(); 544 * drm_gpuva_unlink(va); 545 * drm_gpuva_unmap(&op->unmap); 546 * 547 * break; 548 * default: 549 * break; 550 * } 551 * } 552 * drm_gpuvm_bo_put(vm_bo); 553 * driver_unlock_va_space(); 554 * 555 * return 0; 556 * } 557 * 558 * 2) Receive a callback for each &drm_gpuva_op to create a new mapping:: 559 * 560 * struct driver_context { 561 * struct drm_gpuvm *gpuvm; 562 * struct drm_gpuvm_bo *vm_bo; 563 * struct drm_gpuva *new_va; 564 * struct drm_gpuva *prev_va; 565 * struct drm_gpuva *next_va; 566 * }; 567 * 568 * // ops to pass to drm_gpuvm_init() 569 * static const struct drm_gpuvm_ops driver_gpuvm_ops = { 570 * .sm_step_map = driver_gpuva_map, 571 * .sm_step_remap = driver_gpuva_remap, 572 * .sm_step_unmap = driver_gpuva_unmap, 573 * }; 574 * 575 * // Typically drivers would embedd the &drm_gpuvm and &drm_gpuva 576 * // structure in individual driver structures and lock the dma-resv with 577 * // drm_exec or similar helpers. 578 * int driver_mapping_create(struct drm_gpuvm *gpuvm, 579 * u64 addr, u64 range, 580 * struct drm_gem_object *obj, u64 offset) 581 * { 582 * struct driver_context ctx; 583 * struct drm_gpuvm_bo *vm_bo; 584 * struct drm_gpuva_ops *ops; 585 * struct drm_gpuva_op *op; 586 * int ret = 0; 587 * 588 * ctx.gpuvm = gpuvm; 589 * 590 * ctx.new_va = kzalloc(sizeof(*ctx.new_va), GFP_KERNEL); 591 * ctx.prev_va = kzalloc(sizeof(*ctx.prev_va), GFP_KERNEL); 592 * ctx.next_va = kzalloc(sizeof(*ctx.next_va), GFP_KERNEL); 593 * ctx.vm_bo = drm_gpuvm_bo_create(gpuvm, obj); 594 * if (!ctx.new_va || !ctx.prev_va || !ctx.next_va || !vm_bo) { 595 * ret = -ENOMEM; 596 * goto out; 597 * } 598 * 599 * // Typically protected with a driver specific GEM gpuva lock 600 * // used in the fence signaling path for drm_gpuva_link() and 601 * // drm_gpuva_unlink(), hence pre-allocate. 602 * ctx.vm_bo = drm_gpuvm_bo_obtain_prealloc(ctx.vm_bo); 603 * 604 * driver_lock_va_space(); 605 * ret = drm_gpuvm_sm_map(gpuvm, &ctx, addr, range, obj, offset); 606 * driver_unlock_va_space(); 607 * 608 * out: 609 * drm_gpuvm_bo_put(ctx.vm_bo); 610 * kfree(ctx.new_va); 611 * kfree(ctx.prev_va); 612 * kfree(ctx.next_va); 613 * return ret; 614 * } 615 * 616 * int driver_gpuva_map(struct drm_gpuva_op *op, void *__ctx) 617 * { 618 * struct driver_context *ctx = __ctx; 619 * 620 * drm_gpuva_map(ctx->vm, ctx->new_va, &op->map); 621 * 622 * drm_gpuva_link(ctx->new_va, ctx->vm_bo); 623 * 624 * // prevent the new GPUVA from being freed in 625 * // driver_mapping_create() 626 * ctx->new_va = NULL; 627 * 628 * return 0; 629 * } 630 * 631 * int driver_gpuva_remap(struct drm_gpuva_op *op, void *__ctx) 632 * { 633 * struct driver_context *ctx = __ctx; 634 * struct drm_gpuva *va = op->remap.unmap->va; 635 * 636 * drm_gpuva_remap(ctx->prev_va, ctx->next_va, &op->remap); 637 * 638 * if (op->remap.prev) { 639 * drm_gpuva_link(ctx->prev_va, va->vm_bo); 640 * ctx->prev_va = NULL; 641 * } 642 * 643 * if (op->remap.next) { 644 * drm_gpuva_link(ctx->next_va, va->vm_bo); 645 * ctx->next_va = NULL; 646 * } 647 * 648 * drm_gpuva_unlink(va); 649 * kfree(va); 650 * 651 * return 0; 652 * } 653 * 654 * int driver_gpuva_unmap(struct drm_gpuva_op *op, void *__ctx) 655 * { 656 * drm_gpuva_unlink(op->unmap.va); 657 * drm_gpuva_unmap(&op->unmap); 658 * kfree(op->unmap.va); 659 * 660 * return 0; 661 * } 662 */ 663 664 /** 665 * get_next_vm_bo_from_list() - get the next vm_bo element 666 * @__gpuvm: The GPU VM 667 * @__list_name: The name of the list we're iterating on 668 * @__local_list: A pointer to the local list used to store already iterated items 669 * @__prev_vm_bo: The previous element we got from drm_gpuvm_get_next_cached_vm_bo() 670 * 671 * This helper is here to provide lockless list iteration. Lockless as in, the 672 * iterator releases the lock immediately after picking the first element from 673 * the list, so list insertion deletion can happen concurrently. 674 * 675 * Elements popped from the original list are kept in a local list, so removal 676 * and is_empty checks can still happen while we're iterating the list. 677 */ 678 #define get_next_vm_bo_from_list(__gpuvm, __list_name, __local_list, __prev_vm_bo) \ 679 ({ \ 680 struct drm_gpuvm_bo *__vm_bo; \ 681 \ 682 drm_gpuvm_bo_put(__prev_vm_bo); \ 683 \ 684 spin_lock(&(__gpuvm)->__list_name.lock); \ 685 while (!list_empty(&(__gpuvm)->__list_name.list)) { \ 686 __vm_bo = list_first_entry(&(__gpuvm)->__list_name.list, \ 687 struct drm_gpuvm_bo, \ 688 list.entry.__list_name); \ 689 if (drm_gpuvm_bo_get_unless_zero(__vm_bo)) { \ 690 list_move_tail(&(__vm_bo)->list.entry.__list_name, \ 691 __local_list); \ 692 break; \ 693 } else { \ 694 list_del_init(&(__vm_bo)->list.entry.__list_name); \ 695 __vm_bo = NULL; \ 696 } \ 697 } \ 698 spin_unlock(&(__gpuvm)->__list_name.lock); \ 699 \ 700 __vm_bo; \ 701 }) 702 703 /** 704 * for_each_vm_bo_in_list() - internal vm_bo list iterator 705 * 706 * This helper is here to provide lockless list iteration. Lockless as in, the 707 * iterator releases the lock immediately after picking the first element from the 708 * list, so list insertion and deletion can happen concurrently. 709 * 710 * Typical use: 711 * 712 * struct drm_gpuvm_bo *vm_bo; 713 * LIST_HEAD(my_local_list); 714 * 715 * ret = 0; 716 * drm_gpuvm_for_each_vm_bo(gpuvm, <list_name>, &my_local_list, vm_bo) { 717 * ret = do_something_with_vm_bo(..., vm_bo); 718 * if (ret) 719 * break; 720 * } 721 * drm_gpuvm_bo_put(vm_bo); 722 * drm_gpuvm_restore_vm_bo_list(gpuvm, <list_name>, &my_local_list); 723 * 724 * 725 * Only used for internal list iterations, not meant to be exposed to the outside 726 * world. 727 */ 728 #define for_each_vm_bo_in_list(__gpuvm, __list_name, __local_list, __vm_bo) \ 729 for (__vm_bo = get_next_vm_bo_from_list(__gpuvm, __list_name, \ 730 __local_list, NULL); \ 731 __vm_bo; \ 732 __vm_bo = get_next_vm_bo_from_list(__gpuvm, __list_name, \ 733 __local_list, __vm_bo)) \ > 734 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
Thomas Hellström
2023-Sep-12 16:20 UTC
[Nouveau] [PATCH drm-misc-next v3 6/7] drm/gpuvm: generalize dma_resv/extobj handling and GEM validation
Hi, Danilo, On 9/9/23 17:31, Danilo Krummrich wrote:> So far the DRM GPUVA manager offers common infrastructure to track GPU VA > allocations and mappings, generically connect GPU VA mappings to their > backing buffers and perform more complex mapping operations on the GPU VA > space. > > However, there are more design patterns commonly used by drivers, which > can potentially be generalized in order to make the DRM GPUVA manager > represent a basic GPU-VM implementation. In this context, this patch aims > at generalizing the following elements. > > 1) Provide a common dma-resv for GEM objects not being used outside of > this GPU-VM. > > 2) Provide tracking of external GEM objects (GEM objects which are > shared with other GPU-VMs). > > 3) Provide functions to efficiently lock all GEM objects dma-resv the > GPU-VM contains mappings of. > > 4) Provide tracking of evicted GEM objects the GPU-VM contains mappings > of, such that validation of evicted GEM objects is accelerated. > > 5) Provide some convinience functions for common patterns. > > Rather than being designed as a "framework", the target is to make all > features appear as a collection of optional helper functions, such that > drivers are free to make use of the DRM GPUVA managers basic > functionality and opt-in for other features without setting any feature > flags, just by making use of the corresponding functions. > > Big kudos to Boris Brezillon for his help to figure out locking for drivers > updating the GPU VA space within the fence signalling path. > > Suggested-by: Matthew Brost <matthew.brost at intel.com> > Signed-off-by: Danilo Krummrich <dakr at redhat.com> > --- > drivers/gpu/drm/drm_gpuvm.c | 516 ++++++++++++++++++++++++++++++++++++ > include/drm/drm_gpuvm.h | 197 ++++++++++++++ > 2 files changed, 713 insertions(+) > > diff --git a/drivers/gpu/drm/drm_gpuvm.c b/drivers/gpu/drm/drm_gpuvm.c > index f4411047dbb3..8e62a043f719 100644 > --- a/drivers/gpu/drm/drm_gpuvm.c > +++ b/drivers/gpu/drm/drm_gpuvm.c > @@ -73,6 +73,21 @@ > * &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. > + * > + * &drm_gpuvm_bo structures, since unique for a given &drm_gpuvm, are also used > + * as entry for the &drm_gpuvm's lists of external and evicted objects. Those > + * list are maintained in order to accelerate locking of dma-resv locks and > + * validation of evicted objects bound in a &drm_gpuvm. For instance the all > + * &drm_gem_object's &dma_resv of a given &drm_gpuvm can be locked by calling > + * drm_gpuvm_exec_lock(). Once locked drivers can call drm_gpuvm_validate() in > + * order to validate all evicted &drm_gem_objects. It is also possible to lock > + * additional &drm_gem_objects by providing the corresponding parameters to > + * drm_gpuvm_exec_lock() as well as open code the &drm_exec loop while making > + * use of helper functions such as drm_gpuvm_prepare_range() or > + * drm_gpuvm_prepare_objects(). > + * > + * Every bound &drm_gem_object is treated as external object when its &dma_resv > + * structure is different than the &drm_gpuvm's common &dma_resv structure. > */ > > /** > @@ -420,6 +435,20 @@ > * 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. > + * > + * The &drm_gpuvm's lists for keeping track of external and evicted objects are > + * protected against concurrent insertion / removal and iteration internally. > + * > + * However, drivers still need ensure to protect concurrent calls to functions > + * iterating those lists, such as drm_gpuvm_validate() and > + * drm_gpuvm_prepare_objects(). Every such function contains a particular > + * comment and lockdep checks if possible. > + * > + * Functions adding or removing entries from those lists, such as > + * drm_gpuvm_bo_evict() or drm_gpuvm_bo_extobj_add() may be called with external > + * locks being held, e.g. in order to avoid the corresponding list to be > + * (safely) modified while potentially being iternated by other API functions. > + * However, this is entirely optional. > */ > > /** > @@ -632,6 +661,131 @@ > * } > */ > > +/** > + * get_next_vm_bo_from_list() - get the next vm_bo element > + * @__gpuvm: The GPU VM > + * @__list_name: The name of the list we're iterating on > + * @__local_list: A pointer to the local list used to store already iterated items > + * @__prev_vm_bo: The previous element we got from drm_gpuvm_get_next_cached_vm_bo() > + * > + * This helper is here to provide lockless list iteration. Lockless as in, the > + * iterator releases the lock immediately after picking the first element from > + * the list, so list insertion deletion can happen concurrently.Are the list spinlocks needed for that async state update from within the dma-fence critical section we've discussed previously? Otherwise it should be sufficient to protect the lists with the gpuvm's resv (or for the extobj list with an outer lock). If those spinlocks are still needed in some situations, perhaps could we have an option to set them to NULL (Like IIRC the maple tree allows for)? For such drivers, that would require anybody calling unlink to hold the vm's resv, though. It seems that with that also the refcount could be make non-atomic. All in the spirit of the drm locking guidelines "use big locks when possible". Lower level locks only when necessary for performance or locking inversion? /Thomas> + * > + * Elements popped from the original list are kept in a local list, so removal > + * and is_empty checks can still happen while we're iterating the list. > + */ > +#define get_next_vm_bo_from_list(__gpuvm, __list_name, __local_list, __prev_vm_bo) \ > + ({ \ > + struct drm_gpuvm_bo *__vm_bo; \ > + \ > + drm_gpuvm_bo_put(__prev_vm_bo); \ > + \ > + spin_lock(&(__gpuvm)->__list_name.lock); \ > + while (!list_empty(&(__gpuvm)->__list_name.list)) { \ > + __vm_bo = list_first_entry(&(__gpuvm)->__list_name.list, \ > + struct drm_gpuvm_bo, \ > + list.entry.__list_name); \ > + if (drm_gpuvm_bo_get_unless_zero(__vm_bo)) { \ > + list_move_tail(&(__vm_bo)->list.entry.__list_name, \ > + __local_list); \ > + break; \ > + } else { \ > + list_del_init(&(__vm_bo)->list.entry.__list_name); \ > + __vm_bo = NULL; \ > + } \ > + } \ > + spin_unlock(&(__gpuvm)->__list_name.lock); \ > + \ > + __vm_bo; \ > + }) > + > +/** > + * for_each_vm_bo_in_list() - internal vm_bo list iterator > + * > + * This helper is here to provide lockless list iteration. Lockless as in, the > + * iterator releases the lock immediately after picking the first element from the > + * list, so list insertion and deletion can happen concurrently. > + * > + * Typical use: > + * > + * struct drm_gpuvm_bo *vm_bo; > + * LIST_HEAD(my_local_list); > + * > + * ret = 0; > + * drm_gpuvm_for_each_vm_bo(gpuvm, <list_name>, &my_local_list, vm_bo) { > + * ret = do_something_with_vm_bo(..., vm_bo); > + * if (ret) > + * break; > + * } > + * drm_gpuvm_bo_put(vm_bo); > + * drm_gpuvm_restore_vm_bo_list(gpuvm, <list_name>, &my_local_list); > + * > + * > + * Only used for internal list iterations, not meant to be exposed to the outside > + * world. > + */ > +#define for_each_vm_bo_in_list(__gpuvm, __list_name, __local_list, __vm_bo) \ > + for (__vm_bo = get_next_vm_bo_from_list(__gpuvm, __list_name, \ > + __local_list, NULL); \ > + __vm_bo; \ > + __vm_bo = get_next_vm_bo_from_list(__gpuvm, __list_name, \ > + __local_list, __vm_bo)) \ > + > +/** > + * restore_vm_bo_list() - move vm_bo elements back to their original list > + * @__gpuvm: The GPU VM > + * @__list_name: The name of the list we're iterating on > + * @__local_list: A pointer to the local list used to store already iterated items > + * > + * When we're done iterating a vm_bo list, we should call restore_vm_bo_list() > + * to restore the original state and let new iterations take place. > + */ > +#define restore_vm_bo_list(__gpuvm, __list_name, __local_list) \ > + do { \ > + /* Merge back the two lists, moving local list elements to the \ > + * head to preserve previous ordering, in case it matters. \ > + */ \ > + spin_lock(&(__gpuvm)->__list_name.lock); \ > + list_splice(__local_list, &(__gpuvm)->__list_name.list); \ > + spin_unlock(&(__gpuvm)->__list_name.lock); \ > + } while (0) > +/** > + * drm_gpuvm_bo_list_add() - insert a vm_bo into the given list > + * @__vm_bo: the &drm_gpuvm_bo > + * @__list_name: the name of the list to insert into > + * > + * Inserts the given @__vm_bo into the list specified by @__list_name and > + * increases the vm_bo's reference count. > + */ > +#define drm_gpuvm_bo_list_add(__vm_bo, __list_name) \ > + do { \ > + spin_lock(&(__vm_bo)->vm->__list_name.lock); \ > + if (list_empty(&(__vm_bo)->list.entry.__list_name)) \ > + list_add_tail(&(__vm_bo)->list.entry.__list_name, \ > + &(__vm_bo)->vm->__list_name.list); \ > + spin_unlock(&(__vm_bo)->vm->__list_name.lock); \ > + } while (0) > + > +/** > + * drm_gpuvm_bo_list_del() - remove a vm_bo from the given list > + * @__vm_bo: the &drm_gpuvm_bo > + * @__list_name: the name of the list to insert into > + * > + * Removes the given @__vm_bo from the list specified by @__list_name and > + * decreases the vm_bo's reference count. > + */ > +#define drm_gpuvm_bo_list_del(__vm_bo, __list_name) \ > + do { \ > + spin_lock(&(__vm_bo)->vm->__list_name.lock); \ > + if (!list_empty(&(__vm_bo)->list.entry.__list_name)) \ > + list_del_init(&(__vm_bo)->list.entry.__list_name); \ > + spin_unlock(&(__vm_bo)->vm->__list_name.lock); \ > + } while (0) > + > +static int __must_check > +drm_gpuvm_bo_get_unless_zero(struct drm_gpuvm_bo *vm_bo); > + > #define to_drm_gpuva(__node) container_of((__node), struct drm_gpuva, rb.node) > > #define GPUVA_START(node) ((node)->va.addr) > @@ -713,6 +867,12 @@ drm_gpuvm_init(struct drm_gpuvm *gpuvm, struct drm_device *drm, > gpuvm->rb.tree = RB_ROOT_CACHED; > INIT_LIST_HEAD(&gpuvm->rb.list); > > + INIT_LIST_HEAD(&gpuvm->extobj.list); > + spin_lock_init(&gpuvm->extobj.lock); > + > + INIT_LIST_HEAD(&gpuvm->evict.list); > + spin_lock_init(&gpuvm->evict.lock); > + > drm_gpuva_check_overflow(start_offset, range); > gpuvm->mm_start = start_offset; > gpuvm->mm_range = range; > @@ -754,10 +914,302 @@ drm_gpuvm_destroy(struct drm_gpuvm *gpuvm) > WARN(!RB_EMPTY_ROOT(&gpuvm->rb.tree.rb_root), > "GPUVA tree is not empty, potentially leaking memory.\n"); > > + WARN(!list_empty(&gpuvm->extobj.list), "Extobj list should be empty.\n"); > + WARN(!list_empty(&gpuvm->evict.list), "Evict list should be empty.\n"); > + > drm_gem_private_object_fini(&gpuvm->d_obj); > } > EXPORT_SYMBOL_GPL(drm_gpuvm_destroy); > > +/** > + * drm_gpuvm_prepare_objects() - prepare all assoiciated BOs > + * @gpuvm: the &drm_gpuvm > + * @exec: the &drm_exec locking context > + * @num_fences: the amount of &dma_fences to reserve > + * > + * Calls drm_exec_prepare_obj() for all &drm_gem_objects the given > + * &drm_gpuvm contains mappings of. > + * > + * Using this function directly, it is the drivers responsibility to call > + * drm_exec_init() and drm_exec_fini() accordingly. > + * > + * Note: This function is safe against concurrent insertion and removal of > + * external objects, however it is not safe against concurrent usage itself. > + * > + * Drivers need to make sure to protect this case with either an outer VM lock > + * or by calling drm_gpuvm_prepare_vm() before this function within the > + * drm_exec_until_all_locked() loop, such that the GPUVM's dma-resv lock ensures > + * mutual exclusion. > + * > + * Returns: 0 on success, negative error code on failure. > + */ > +int > +drm_gpuvm_prepare_objects(struct drm_gpuvm *gpuvm, > + struct drm_exec *exec, > + unsigned int num_fences) > +{ > + struct drm_gpuvm_bo *vm_bo; > + LIST_HEAD(extobjs); > + int ret = 0; > + > + for_each_vm_bo_in_list(gpuvm, extobj, &extobjs, vm_bo) { > + ret = drm_exec_prepare_obj(exec, vm_bo->obj, num_fences); > + if (ret) > + break; > + } > + /* Drop ref in case we break out of the loop. */ > + drm_gpuvm_bo_put(vm_bo); > + restore_vm_bo_list(gpuvm, extobj, &extobjs); > + > + return ret; > +} > +EXPORT_SYMBOL_GPL(drm_gpuvm_prepare_objects); > + > +/** > + * drm_gpuvm_prepare_range() - prepare all BOs mapped within a given range > + * @gpuvm: the &drm_gpuvm > + * @exec: the &drm_exec locking context > + * @addr: the start address within the VA space > + * @range: the range to iterate within the VA space > + * @num_fences: the amount of &dma_fences to reserve > + * > + * Calls drm_exec_prepare_obj() for all &drm_gem_objects mapped between @addr > + * and @addr + @range. > + * > + * Returns: 0 on success, negative error code on failure. > + */ > +int > +drm_gpuvm_prepare_range(struct drm_gpuvm *gpuvm, struct drm_exec *exec, > + u64 addr, u64 range, unsigned int num_fences) > +{ > + struct drm_gpuva *va; > + u64 end = addr + range; > + int ret; > + > + drm_gpuvm_for_each_va_range(va, gpuvm, addr, end) { > + struct drm_gem_object *obj = va->gem.obj; > + > + ret = drm_exec_prepare_obj(exec, obj, num_fences); > + if (ret) > + return ret; > + } > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(drm_gpuvm_prepare_range); > + > +/** > + * drm_gpuvm_exec_lock() - lock all dma-resv of all assoiciated BOs > + * @vm_exec: the &drm_gpuvm_exec abstraction > + * @num_fences: the amount of &dma_fences to reserve > + * @interruptible: sleep interruptible if waiting > + * > + * Acquires all dma-resv locks of all &drm_gem_objects the given > + * &drm_gpuvm contains mappings of. > + * > + * Addionally, when calling this function with struct drm_gpuvm_exec::extra > + * being set the driver receives the given @fn callback to lock additional > + * dma-resv in the context of the &drm_gpuvm_exec instance. Typically, drivers > + * would call drm_exec_prepare_obj() from within this callback. > + * > + * Returns: 0 on success, negative error code on failure. > + */ > +int > +drm_gpuvm_exec_lock(struct drm_gpuvm_exec *vm_exec, > + unsigned int num_fences, > + bool interruptible) > +{ > + struct drm_gpuvm *gpuvm = vm_exec->vm; > + struct drm_exec *exec = &vm_exec->exec; > + uint32_t flags; > + int ret; > + > + flags = interruptible ? DRM_EXEC_INTERRUPTIBLE_WAIT : 0 | > + DRM_EXEC_IGNORE_DUPLICATES; > + > + drm_exec_init(exec, flags); > + > + drm_exec_until_all_locked(exec) { > + ret = drm_gpuvm_prepare_vm(gpuvm, exec, num_fences); > + drm_exec_retry_on_contention(exec); > + if (ret) > + goto err; > + > + ret = drm_gpuvm_prepare_objects(gpuvm, exec, num_fences); > + drm_exec_retry_on_contention(exec); > + if (ret) > + goto err; > + > + if (vm_exec->extra.fn) { > + ret = vm_exec->extra.fn(vm_exec, num_fences); > + drm_exec_retry_on_contention(exec); > + if (ret) > + goto err; > + } > + } > + > + return 0; > + > +err: > + drm_exec_fini(exec); > + return ret; > +} > +EXPORT_SYMBOL_GPL(drm_gpuvm_exec_lock); > + > +static int > +fn_lock_array(struct drm_gpuvm_exec *vm_exec, unsigned int num_fences) > +{ > + struct { > + struct drm_gem_object **objs; > + unsigned int num_objs; > + } *args = vm_exec->extra.priv; > + > + return drm_exec_prepare_array(&vm_exec->exec, args->objs, > + args->num_objs, num_fences); > +} > + > +/** > + * drm_gpuvm_exec_lock_array() - lock all dma-resv of all assoiciated BOs > + * @vm_exec: the &drm_gpuvm_exec abstraction > + * @objs: additional &drm_gem_objects to lock > + * @num_objs: the number of additional &drm_gem_objects to lock > + * @num_fences: the amount of &dma_fences to reserve > + * @interruptible: sleep interruptible if waiting > + * > + * Acquires all dma-resv locks of all &drm_gem_objects the given &drm_gpuvm > + * contains mappings of, plus the ones given through @objs. > + * > + * Returns: 0 on success, negative error code on failure. > + */ > +int > +drm_gpuvm_exec_lock_array(struct drm_gpuvm_exec *vm_exec, > + struct drm_gem_object **objs, > + unsigned int num_objs, > + unsigned int num_fences, > + bool interruptible) > +{ > + struct { > + struct drm_gem_object **objs; > + unsigned int num_objs; > + } args; > + > + args.objs = objs; > + args.num_objs = num_objs; > + > + vm_exec->extra.fn = fn_lock_array; > + vm_exec->extra.priv = &args; > + > + return drm_gpuvm_exec_lock(vm_exec, num_fences, interruptible); > +} > +EXPORT_SYMBOL_GPL(drm_gpuvm_exec_lock_array); > + > +/** > + * drm_gpuvm_exec_lock_range() - prepare all BOs mapped within a given range > + * @vm_exec: the &drm_gpuvm_exec abstraction > + * @addr: the start address within the VA space > + * @range: the range to iterate within the VA space > + * @num_fences: the amount of &dma_fences to reserve > + * @interruptible: sleep interruptible if waiting > + * > + * Acquires all dma-resv locks of all &drm_gem_objects mapped between @addr and > + * @addr + @range. > + * > + * Returns: 0 on success, negative error code on failure. > + */ > +int > +drm_gpuvm_exec_lock_range(struct drm_gpuvm_exec *vm_exec, > + u64 addr, u64 range, > + unsigned int num_fences, > + bool interruptible) > +{ > + struct drm_gpuvm *gpuvm = vm_exec->vm; > + struct drm_exec *exec = &vm_exec->exec; > + uint32_t flags; > + int ret; > + > + flags = interruptible ? DRM_EXEC_INTERRUPTIBLE_WAIT : 0 | > + DRM_EXEC_IGNORE_DUPLICATES; > + > + drm_exec_init(exec, flags); > + > + drm_exec_until_all_locked(exec) { > + ret = drm_gpuvm_prepare_range(gpuvm, exec, addr, range, > + num_fences); > + drm_exec_retry_on_contention(exec); > + if (ret) > + goto err; > + } > + > + return ret; > + > +err: > + drm_exec_fini(exec); > + return ret; > +} > +EXPORT_SYMBOL_GPL(drm_gpuvm_exec_lock_range); > + > +/** > + * drm_gpuvm_validate() - validate all BOs marked as evicted > + * @gpuvm: the &drm_gpuvm to validate evicted BOs > + * > + * Calls the &drm_gpuvm_ops.bo_validate callback for all evicted buffer > + * objects being mapped in the given &drm_gpuvm. > + * > + * Returns: 0 on success, negative error code on failure. > + */ > +int > +drm_gpuvm_validate(struct drm_gpuvm *gpuvm) > +{ > + const struct drm_gpuvm_ops *ops = gpuvm->ops; > + struct drm_gpuvm_bo *vm_bo; > + LIST_HEAD(evict); > + int ret = 0; > + > + if (unlikely(!ops || !ops->bo_validate)) > + return -ENOTSUPP; > + > + for_each_vm_bo_in_list(gpuvm, evict, &evict, vm_bo) { > + dma_resv_assert_held(vm_bo->obj->resv); > + ret = ops->bo_validate(vm_bo->obj); > + if (ret) > + break; > + } > + /* Drop ref in case we break out of the loop. */ > + drm_gpuvm_bo_put(vm_bo); > + restore_vm_bo_list(gpuvm, evict, &evict); > + > + return ret; > +} > +EXPORT_SYMBOL_GPL(drm_gpuvm_validate); > + > +/** > + * drm_gpuvm_resv_add_fence - add fence to private and all extobj > + * dma-resv > + * @gpuvm: the &drm_gpuvm to add a fence to > + * @exec: the &drm_exec locking context > + * @fence: fence to add > + * @private_usage: private dma-resv usage > + * @extobj_usage: extobj dma-resv usage > + */ > +void > +drm_gpuvm_resv_add_fence(struct drm_gpuvm *gpuvm, > + struct drm_exec *exec, > + struct dma_fence *fence, > + enum dma_resv_usage private_usage, > + enum dma_resv_usage extobj_usage) > +{ > + struct drm_gem_object *obj; > + unsigned long index; > + > + drm_exec_for_each_locked_object(exec, index, obj) { > + dma_resv_assert_held(obj->resv); > + dma_resv_add_fence(obj->resv, fence, > + drm_gpuvm_is_extobj(gpuvm, obj) ? > + private_usage : extobj_usage); > + } > +} > +EXPORT_SYMBOL_GPL(drm_gpuvm_resv_add_fence); > + > /** > * drm_gpuvm_bo_create() - create a new instance of struct drm_gpuvm_bo > * @gpuvm: The &drm_gpuvm the @obj is mapped in. > @@ -790,6 +1242,9 @@ drm_gpuvm_bo_create(struct drm_gpuvm *gpuvm, > INIT_LIST_HEAD(&vm_bo->list.gpuva); > INIT_LIST_HEAD(&vm_bo->list.entry.gem); > > + INIT_LIST_HEAD(&vm_bo->list.entry.extobj); > + INIT_LIST_HEAD(&vm_bo->list.entry.evict); > + > drm_gem_object_get(obj); > > return vm_bo; > @@ -807,6 +1262,14 @@ drm_gpuvm_bo_destroy(struct kref *kref) > > drm_gem_gpuva_assert_lock_held(vm_bo->obj); > > + spin_lock(&gpuvm->extobj.lock); > + list_del(&vm_bo->list.entry.extobj); > + spin_unlock(&gpuvm->extobj.lock); > + > + spin_lock(&gpuvm->evict.lock); > + list_del(&vm_bo->list.entry.evict); > + spin_unlock(&gpuvm->evict.lock); > + > list_del(&vm_bo->list.entry.gem); > > drm_gem_object_put(obj); > @@ -822,6 +1285,11 @@ drm_gpuvm_bo_destroy(struct kref *kref) > * @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) > @@ -831,6 +1299,12 @@ drm_gpuvm_bo_put(struct drm_gpuvm_bo *vm_bo) > } > EXPORT_SYMBOL_GPL(drm_gpuvm_bo_put); > > +static int __must_check > +drm_gpuvm_bo_get_unless_zero(struct drm_gpuvm_bo *vm_bo) > +{ > + return kref_get_unless_zero(&vm_bo->kref); > +} > + > static struct drm_gpuvm_bo * > __drm_gpuvm_bo_find(struct drm_gpuvm *gpuvm, > struct drm_gem_object *obj) > @@ -938,6 +1412,48 @@ drm_gpuvm_bo_obtain_prealloc(struct drm_gpuvm_bo *__vm_bo) > } > EXPORT_SYMBOL_GPL(drm_gpuvm_bo_obtain_prealloc); > > +/** > + * drm_gpuvm_bo_extobj_add() - adds the &drm_gpuvm_bo to its &drm_gpuvm's > + * extobj list > + * @vm_bo: The &drm_gpuvm_bo to add to its &drm_gpuvm's the extobj list. > + * > + * Adds the given @vm_bo to its &drm_gpuvm's extobj list if not on the list > + * already and if the corresponding &drm_gem_object is an external object, > + * actually. > + */ > +void > +drm_gpuvm_bo_extobj_add(struct drm_gpuvm_bo *vm_bo) > +{ > + struct drm_gpuvm *gpuvm = vm_bo->vm; > + > + if (drm_gpuvm_is_extobj(gpuvm, vm_bo->obj)) > + drm_gpuvm_bo_list_add(vm_bo, extobj); > +} > +EXPORT_SYMBOL_GPL(drm_gpuvm_bo_extobj_add); > + > +/** > + * drm_gpuvm_bo_evict() - add / remove a &drm_gem_object to / from a > + * &drm_gpuvms evicted list > + * @obj: the &drm_gem_object to add or remove > + * @evict: indicates whether the object is evicted > + * > + * Adds a &drm_gem_object to or removes it from all &drm_gpuvms evicted > + * list containing a mapping of this &drm_gem_object. > + */ > +void > +drm_gpuvm_bo_evict(struct drm_gem_object *obj, bool evict) > +{ > + struct drm_gpuvm_bo *vm_bo; > + > + drm_gem_for_each_gpuvm_bo(vm_bo, obj) { > + if (evict) > + drm_gpuvm_bo_list_add(vm_bo, evict); > + else > + drm_gpuvm_bo_list_del(vm_bo, evict); > + } > +} > +EXPORT_SYMBOL_GPL(drm_gpuvm_bo_evict); > + > static int > __drm_gpuva_insert(struct drm_gpuvm *gpuvm, > struct drm_gpuva *va) > diff --git a/include/drm/drm_gpuvm.h b/include/drm/drm_gpuvm.h > index afa50b9059a2..834bb6d6617e 100644 > --- a/include/drm/drm_gpuvm.h > +++ b/include/drm/drm_gpuvm.h > @@ -26,10 +26,12 @@ > */ > > #include <linux/list.h> > +#include <linux/dma-resv.h> > #include <linux/rbtree.h> > #include <linux/types.h> > > #include <drm/drm_gem.h> > +#include <drm/drm_exec.h> > > struct drm_gpuvm; > struct drm_gpuvm_bo; > @@ -259,6 +261,38 @@ struct drm_gpuvm { > * space > */ > struct dma_resv *resv; > + > + /** > + * @extobj: structure holding the extobj list > + */ > + struct { > + /** > + * @list: &list_head storing &drm_gpuvm_bos serving as > + * external object > + */ > + struct list_head list; > + > + /** > + * @lock: spinlock to protect the extobj list > + */ > + spinlock_t lock; > + } extobj; > + > + /** > + * @evict: structure holding the evict list and evict list lock > + */ > + struct { > + /** > + * @list: &list_head storing &drm_gpuvm_bos currently being > + * evicted > + */ > + struct list_head list; > + > + /** > + * @lock: spinlock to protect the evict list > + */ > + spinlock_t lock; > + } evict; > }; > > void drm_gpuvm_init(struct drm_gpuvm *gpuvm, struct drm_device *drm, > @@ -268,6 +302,21 @@ void drm_gpuvm_init(struct drm_gpuvm *gpuvm, struct drm_device *drm, > const struct drm_gpuvm_ops *ops); > void drm_gpuvm_destroy(struct drm_gpuvm *gpuvm); > > +/** > + * drm_gpuvm_is_extobj() - indicates whether the given &drm_gem_object is an > + * external object > + * @gpuvm: the &drm_gpuvm to check > + * @obj: the &drm_gem_object to check > + * > + * Returns: true if the &drm_gem_object &dma_resv differs from the > + * &drm_gpuvms &dma_resv, false otherwise > + */ > +static inline bool drm_gpuvm_is_extobj(struct drm_gpuvm *gpuvm, > + struct drm_gem_object *obj) > +{ > + return obj && obj->resv != gpuvm->resv; > +} > + > static inline struct drm_gpuva * > __drm_gpuva_next(struct drm_gpuva *va) > { > @@ -346,6 +395,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_exec - &drm_gpuvm abstraction of &drm_exec > + * > + * This structure should be created on the stack as &drm_exec should be. > + * > + * Optionally, @extra can be set in order to lock additional &drm_gem_objects. > + */ > +struct drm_gpuvm_exec { > + /** > + * @exec: the &drm_exec structure > + */ > + struct drm_exec exec; > + > + /** > + * @vm: the &drm_gpuvm to lock its DMA reservations > + */ > + struct drm_gpuvm *vm; > + > + /** > + * @extra: Callback and corresponding private data for the driver to > + * lock arbitrary additional &drm_gem_objects. > + */ > + struct { > + /** > + * @fn: The driver callback to lock additional &drm_gem_objects. > + */ > + int (*fn)(struct drm_gpuvm_exec *vm_exec, > + unsigned int num_fences); > + > + /** > + * @priv: driver private data for the @fn callback > + */ > + void *priv; > + } extra; > +}; > + > +/** > + * drm_gpuvm_prepare_vm() - prepare the GPUVMs common dma-resv > + * @gpuvm: the &drm_gpuvm > + * @exec: the &drm_exec context > + * @num_fences: the amount of &dma_fences to reserve > + * > + * Calls drm_exec_prepare_obj() for the GPUVMs dummy &drm_gem_object. > + * > + * Using this function directly, it is the drivers responsibility to call > + * drm_exec_init() and drm_exec_fini() accordingly. > + * > + * Returns: 0 on success, negative error code on failure. > + */ > +static inline int > +drm_gpuvm_prepare_vm(struct drm_gpuvm *gpuvm, > + struct drm_exec *exec, > + unsigned int num_fences) > +{ > + return drm_exec_prepare_obj(exec, &gpuvm->d_obj, num_fences); > +} > + > +int drm_gpuvm_prepare_objects(struct drm_gpuvm *gpuvm, > + struct drm_exec *exec, > + unsigned int num_fences); > + > +int drm_gpuvm_prepare_range(struct drm_gpuvm *gpuvm, > + struct drm_exec *exec, > + u64 addr, u64 range, > + unsigned int num_fences); > + > +int drm_gpuvm_exec_lock(struct drm_gpuvm_exec *vm_exec, > + unsigned int num_fences, > + bool interruptible); > + > +int drm_gpuvm_exec_lock_array(struct drm_gpuvm_exec *vm_exec, > + struct drm_gem_object **objs, > + unsigned int num_objs, > + unsigned int num_fences, > + bool interruptible); > + > +int drm_gpuvm_exec_lock_range(struct drm_gpuvm_exec *vm_exec, > + u64 addr, u64 range, > + unsigned int num_fences, > + bool interruptible); > + > +/** > + * drm_gpuvm_lock() - lock all dma-resv of all assoiciated BOs > + * @gpuvm: the &drm_gpuvm > + * > + * Releases all dma-resv locks of all &drm_gem_objects previously acquired > + * through drm_gpuvm_lock() or its variants. > + * > + * Returns: 0 on success, negative error code on failure. > + */ > +static inline void > +drm_gpuvm_exec_unlock(struct drm_gpuvm_exec *vm_exec) > +{ > + drm_exec_fini(&vm_exec->exec); > +} > + > +int drm_gpuvm_validate(struct drm_gpuvm *gpuvm); > +void drm_gpuvm_resv_add_fence(struct drm_gpuvm *gpuvm, > + struct drm_exec *exec, > + struct dma_fence *fence, > + enum dma_resv_usage private_usage, > + enum dma_resv_usage extobj_usage); > + > +/** > + * drm_gpuvm_exec_resv_add_fence() > + * @vm_exec: the &drm_gpuvm_exec abstraction > + * @fence: fence to add > + * @private_usage: private dma-resv usage > + * @extobj_usage: extobj dma-resv usage > + * > + * See drm_gpuvm_resv_add_fence(). > + */ > +static inline void > +drm_gpuvm_exec_resv_add_fence(struct drm_gpuvm_exec *vm_exec, > + struct dma_fence *fence, > + enum dma_resv_usage private_usage, > + enum dma_resv_usage extobj_usage) > +{ > + drm_gpuvm_resv_add_fence(vm_exec->vm, &vm_exec->exec, fence, > + private_usage, extobj_usage); > +} > + > /** > * struct drm_gpuvm_bo - structure representing a &drm_gpuvm and > * &drm_gem_object combination > @@ -398,6 +569,18 @@ struct drm_gpuvm_bo { > * gpuva list. > */ > struct list_head gem; > + > + /** > + * @evict: List entry to attach to the &drm_gpuvms > + * extobj list. > + */ > + struct list_head extobj; > + > + /** > + * @evict: List entry to attach to the &drm_gpuvms evict > + * list. > + */ > + struct list_head evict; > } entry; > } list; > }; > @@ -432,6 +615,9 @@ struct drm_gpuvm_bo * > drm_gpuvm_bo_find(struct drm_gpuvm *gpuvm, > struct drm_gem_object *obj); > > +void drm_gpuvm_bo_evict(struct drm_gem_object *obj, bool evict); > +void drm_gpuvm_bo_extobj_add(struct drm_gpuvm_bo *vm_bo); > + > /** > * 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 > @@ -837,6 +1023,17 @@ struct drm_gpuvm_ops { > * used. > */ > int (*sm_step_unmap)(struct drm_gpuva_op *op, void *priv); > + > + /** > + * @bo_validate: called from drm_gpuvm_validate() > + * > + * Drivers receive this callback for every evicted &drm_gem_object being > + * mapped in the corresponding &drm_gpuvm. > + * > + * Typically, drivers would call their driver specific variant of > + * ttm_bo_validate() from within this callback. > + */ > + int (*bo_validate)(struct drm_gem_object *obj); > }; > > int drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm, void *priv,
Thomas Hellström
2023-Sep-14 13:48 UTC
[Nouveau] [PATCH drm-misc-next v3 6/7] drm/gpuvm: generalize dma_resv/extobj handling and GEM validation
Hi, Danilo Some additional minor comments as xe conversion progresses. On 9/9/23 17:31, Danilo Krummrich wrote:> So far the DRM GPUVA manager offers common infrastructure to track GPU VA > allocations and mappings, generically connect GPU VA mappings to their > backing buffers and perform more complex mapping operations on the GPU VA > space. > > However, there are more design patterns commonly used by drivers, which > can potentially be generalized in order to make the DRM GPUVA manager > represent a basic GPU-VM implementation. In this context, this patch aims > at generalizing the following elements. > > 1) Provide a common dma-resv for GEM objects not being used outside of > this GPU-VM. > > 2) Provide tracking of external GEM objects (GEM objects which are > shared with other GPU-VMs). > > 3) Provide functions to efficiently lock all GEM objects dma-resv the > GPU-VM contains mappings of. > > 4) Provide tracking of evicted GEM objects the GPU-VM contains mappings > of, such that validation of evicted GEM objects is accelerated. > > 5) Provide some convinience functions for common patterns. > > Rather than being designed as a "framework", the target is to make all > features appear as a collection of optional helper functions, such that > drivers are free to make use of the DRM GPUVA managers basic > functionality and opt-in for other features without setting any feature > flags, just by making use of the corresponding functions. > > Big kudos to Boris Brezillon for his help to figure out locking for drivers > updating the GPU VA space within the fence signalling path. > > Suggested-by: Matthew Brost <matthew.brost at intel.com> > Signed-off-by: Danilo Krummrich <dakr at redhat.com> > --- > > +/** > + * drm_gpuvm_bo_evict() - add / remove a &drm_gem_object to / from a > + * &drm_gpuvms evicted list > + * @obj: the &drm_gem_object to add or remove > + * @evict: indicates whether the object is evicted > + * > + * Adds a &drm_gem_object to or removes it from all &drm_gpuvms evicted > + * list containing a mapping of this &drm_gem_object. > + */ > +void > +drm_gpuvm_bo_evict(struct drm_gem_object *obj, bool evict) > +{ > + struct drm_gpuvm_bo *vm_bo; > + > + drm_gem_for_each_gpuvm_bo(vm_bo, obj) { > + if (evict) > + drm_gpuvm_bo_list_add(vm_bo, evict); > + else > + drm_gpuvm_bo_list_del(vm_bo, evict); > + } > +} > +EXPORT_SYMBOL_GPL(drm_gpuvm_bo_evict); > +We need a drm_gpuvm_bo_evict(struct drm_gpuvm_bo *vm_bo, ...) that puts a single gpuvm_bo on the list, the above function could perhaps be renamed as drm_gpuvm_gem_obj_evict(obj, ....). Reason is some vm's are faulting vms which don't have an evict list, but validate from the pagefault handler. Also evict == false is dangerous because if called from within an exec, it might remove the obj from other vm's evict list before they've had a chance to rebind their VMAs.> static int > __drm_gpuva_insert(struct drm_gpuvm *gpuvm, > struct drm_gpuva *va) > diff --git a/include/drm/drm_gpuvm.h b/include/drm/drm_gpuvm.h > index afa50b9059a2..834bb6d6617e 100644 > --- a/include/drm/drm_gpuvm.h > +++ b/include/drm/drm_gpuvm.h > @@ -26,10 +26,12 @@ > */ > > #include <linux/list.h> > +#include <linux/dma-resv.h> > #include <linux/rbtree.h> > #include <linux/types.h> > > #include <drm/drm_gem.h> > +#include <drm/drm_exec.h> > > struct drm_gpuvm; > struct drm_gpuvm_bo; > @@ -259,6 +261,38 @@ struct drm_gpuvm { > * space > */ > struct dma_resv *resv; > + > + /** > + * @extobj: structure holding the extobj list > + */ > + struct { > + /** > + * @list: &list_head storing &drm_gpuvm_bos serving as > + * external object > + */ > + struct list_head list; > + > + /** > + * @lock: spinlock to protect the extobj list > + */ > + spinlock_t lock; > + } extobj; > + > + /** > + * @evict: structure holding the evict list and evict list lock > + */ > + struct { > + /** > + * @list: &list_head storing &drm_gpuvm_bos currently being > + * evicted > + */ > + struct list_head list; > + > + /** > + * @lock: spinlock to protect the evict list > + */ > + spinlock_t lock; > + } evict; > }; > > void drm_gpuvm_init(struct drm_gpuvm *gpuvm, struct drm_device *drm, > @@ -268,6 +302,21 @@ void drm_gpuvm_init(struct drm_gpuvm *gpuvm, struct drm_device *drm, > const struct drm_gpuvm_ops *ops); > void drm_gpuvm_destroy(struct drm_gpuvm *gpuvm); > > +/** > + * drm_gpuvm_is_extobj() - indicates whether the given &drm_gem_object is an > + * external object > + * @gpuvm: the &drm_gpuvm to check > + * @obj: the &drm_gem_object to check > + * > + * Returns: true if the &drm_gem_object &dma_resv differs from the > + * &drm_gpuvms &dma_resv, false otherwise > + */ > +static inline bool drm_gpuvm_is_extobj(struct drm_gpuvm *gpuvm, > + struct drm_gem_object *obj) > +{ > + return obj && obj->resv != gpuvm->resv; > +} > + > static inline struct drm_gpuva * > __drm_gpuva_next(struct drm_gpuva *va) > { > @@ -346,6 +395,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_exec - &drm_gpuvm abstraction of &drm_exec > + * > + * This structure should be created on the stack as &drm_exec should be. > + * > + * Optionally, @extra can be set in order to lock additional &drm_gem_objects. > + */ > +struct drm_gpuvm_exec { > + /** > + * @exec: the &drm_exec structure > + */ > + struct drm_exec exec; > + > + /** > + * @vm: the &drm_gpuvm to lock its DMA reservations > + */ > + struct drm_gpuvm *vm; > + > + /** > + * @extra: Callback and corresponding private data for the driver to > + * lock arbitrary additional &drm_gem_objects. > + */ > + struct { > + /** > + * @fn: The driver callback to lock additional &drm_gem_objects. > + */ > + int (*fn)(struct drm_gpuvm_exec *vm_exec, > + unsigned int num_fences); > + > + /** > + * @priv: driver private data for the @fn callback > + */ > + void *priv; > + } extra; > +}; > + > +/** > + * drm_gpuvm_prepare_vm() - prepare the GPUVMs common dma-resv > + * @gpuvm: the &drm_gpuvm > + * @exec: the &drm_exec context > + * @num_fences: the amount of &dma_fences to reserve > + * > + * Calls drm_exec_prepare_obj() for the GPUVMs dummy &drm_gem_object. > + * > + * Using this function directly, it is the drivers responsibility to call > + * drm_exec_init() and drm_exec_fini() accordingly. > + * > + * Returns: 0 on success, negative error code on failure. > + */ > +static inline int > +drm_gpuvm_prepare_vm(struct drm_gpuvm *gpuvm, > + struct drm_exec *exec, > + unsigned int num_fences) > +{ > + return drm_exec_prepare_obj(exec, &gpuvm->d_obj, num_fences); > +} > + > +int drm_gpuvm_prepare_objects(struct drm_gpuvm *gpuvm, > + struct drm_exec *exec, > + unsigned int num_fences); > + > +int drm_gpuvm_prepare_range(struct drm_gpuvm *gpuvm, > + struct drm_exec *exec, > + u64 addr, u64 range, > + unsigned int num_fences); > + > +int drm_gpuvm_exec_lock(struct drm_gpuvm_exec *vm_exec, > + unsigned int num_fences, > + bool interruptible); > + > +int drm_gpuvm_exec_lock_array(struct drm_gpuvm_exec *vm_exec, > + struct drm_gem_object **objs, > + unsigned int num_objs, > + unsigned int num_fences, > + bool interruptible); > + > +int drm_gpuvm_exec_lock_range(struct drm_gpuvm_exec *vm_exec, > + u64 addr, u64 range, > + unsigned int num_fences, > + bool interruptible); > + > +/** > + * drm_gpuvm_lock() - lock all dma-resv of all assoiciated BOs > + * @gpuvm: the &drm_gpuvm > + * > + * Releases all dma-resv locks of all &drm_gem_objects previously acquired > + * through drm_gpuvm_lock() or its variants. > + * > + * Returns: 0 on success, negative error code on failure. > + */ > +static inline void > +drm_gpuvm_exec_unlock(struct drm_gpuvm_exec *vm_exec) > +{ > + drm_exec_fini(&vm_exec->exec); > +} > + > +int drm_gpuvm_validate(struct drm_gpuvm *gpuvm); > +void drm_gpuvm_resv_add_fence(struct drm_gpuvm *gpuvm, > + struct drm_exec *exec, > + struct dma_fence *fence, > + enum dma_resv_usage private_usage, > + enum dma_resv_usage extobj_usage); > + > +/** > + * drm_gpuvm_exec_resv_add_fence() > + * @vm_exec: the &drm_gpuvm_exec abstraction > + * @fence: fence to add > + * @private_usage: private dma-resv usage > + * @extobj_usage: extobj dma-resv usage > + * > + * See drm_gpuvm_resv_add_fence(). > + */ > +static inline void > +drm_gpuvm_exec_resv_add_fence(struct drm_gpuvm_exec *vm_exec, > + struct dma_fence *fence, > + enum dma_resv_usage private_usage, > + enum dma_resv_usage extobj_usage) > +{ > + drm_gpuvm_resv_add_fence(vm_exec->vm, &vm_exec->exec, fence, > + private_usage, extobj_usage); > +} > + > /** > * struct drm_gpuvm_bo - structure representing a &drm_gpuvm and > * &drm_gem_object combination > @@ -398,6 +569,18 @@ struct drm_gpuvm_bo { > * gpuva list. > */ > struct list_head gem; > + > + /** > + * @evict: List entry to attach to the &drm_gpuvms > + * extobj list. > + */ > + struct list_head extobj; > + > + /** > + * @evict: List entry to attach to the &drm_gpuvms evict > + * list. > + */ > + struct list_head evict; > } entry; > } list; > }; > @@ -432,6 +615,9 @@ struct drm_gpuvm_bo * > drm_gpuvm_bo_find(struct drm_gpuvm *gpuvm, > struct drm_gem_object *obj); > > +void drm_gpuvm_bo_evict(struct drm_gem_object *obj, bool evict); > +void drm_gpuvm_bo_extobj_add(struct drm_gpuvm_bo *vm_bo); > + > /** > * 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 > @@ -837,6 +1023,17 @@ struct drm_gpuvm_ops { > * used. > */ > int (*sm_step_unmap)(struct drm_gpuva_op *op, void *priv); > + > + /** > + * @bo_validate: called from drm_gpuvm_validate() > + * > + * Drivers receive this callback for every evicted &drm_gem_object being > + * mapped in the corresponding &drm_gpuvm. > + * > + * Typically, drivers would call their driver specific variant of > + * ttm_bo_validate() from within this callback. > + */ > + int (*bo_validate)(struct drm_gem_object *obj);Same here. Could we have a vm_bo as an argument instead, so that the callback knows what gpuvm we're targeting and can mark all its gpu_vas for revalidation? Or is that intended to be done elsewhere?> }; > > int drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm, void *priv,Thanks, Thomas