[Intel-gfx] [PATCH 05/16] drm/i915/vm_bind: Implement bind and unbind of object

Niranjana Vishwanathapura niranjana.vishwanathapura at intel.com
Thu Sep 29 14:24:19 UTC 2022


On Thu, Sep 29, 2022 at 11:51:51AM +0100, Matthew Auld wrote:
>On 29/09/2022 10:03, Matthew Auld wrote:
>>On 29/09/2022 06:24, Niranjana Vishwanathapura wrote:
>>>On Wed, Sep 28, 2022 at 06:52:21PM +0100, Matthew Auld wrote:
>>>>On 28/09/2022 07:19, Niranjana Vishwanathapura wrote:
>>>>>Add uapi and implement support for bind and unbind of an
>>>>>object at the specified GPU virtual addresses.
>>>>>
>>>>>The vm_bind mode is not supported in legacy execbuf2 ioctl.
>>>>>It will be supported only in the newer execbuf3 ioctl.
>>>>>
>>>>>Signed-off-by: Niranjana Vishwanathapura 
>>>>><niranjana.vishwanathapura at intel.com>
>>>>>Signed-off-by: Prathap Kumar Valsan <prathap.kumar.valsan at intel.com>
>>>>>Signed-off-by: Andi Shyti <andi.shyti at linux.intel.com>
>>>>>---
>>>>> drivers/gpu/drm/i915/Makefile                 |   1 +
>>>>> .../gpu/drm/i915/gem/i915_gem_execbuffer.c    |   5 +
>>>>> drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h   |  26 ++
>>>>> .../drm/i915/gem/i915_gem_vm_bind_object.c    | 306 ++++++++++++++++++
>>>>> drivers/gpu/drm/i915/gt/intel_gtt.c           |  10 +
>>>>> drivers/gpu/drm/i915/gt/intel_gtt.h           |  17 +
>>>>> drivers/gpu/drm/i915/i915_driver.c            |   3 +
>>>>> drivers/gpu/drm/i915/i915_vma.c               |   1 +
>>>>> drivers/gpu/drm/i915/i915_vma_types.h         |  14 +
>>>>> include/uapi/drm/i915_drm.h                   | 112 +++++++
>>>>> 10 files changed, 495 insertions(+)
>>>>> create mode 100644 drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h
>>>>> create mode 100644 drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c
>>>>>
>>>>>diff --git a/drivers/gpu/drm/i915/Makefile 
>>>>>b/drivers/gpu/drm/i915/Makefile
>>>>>index a26edcdadc21..9bf939ef18ea 100644
>>>>>--- a/drivers/gpu/drm/i915/Makefile
>>>>>+++ b/drivers/gpu/drm/i915/Makefile
>>>>>@@ -166,6 +166,7 @@ gem-y += \
>>>>>     gem/i915_gem_ttm_move.o \
>>>>>     gem/i915_gem_ttm_pm.o \
>>>>>     gem/i915_gem_userptr.o \
>>>>>+    gem/i915_gem_vm_bind_object.o \
>>>>>     gem/i915_gem_wait.o \
>>>>>     gem/i915_gemfs.o
>>>>> i915-y += \
>>>>>diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c 
>>>>>b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
>>>>>index cd75b0ca2555..f85f10cf9c34 100644
>>>>>--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
>>>>>+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
>>>>>@@ -781,6 +781,11 @@ static int eb_select_context(struct 
>>>>>i915_execbuffer *eb)
>>>>>     if (unlikely(IS_ERR(ctx)))
>>>>>         return PTR_ERR(ctx);
>>>>>+    if (ctx->vm->vm_bind_mode) {
>>>>>+        i915_gem_context_put(ctx);
>>>>>+        return -EOPNOTSUPP;
>>>>>+    }
>>>>>+
>>>>>     eb->gem_context = ctx;
>>>>>     if (i915_gem_context_has_full_ppgtt(ctx))
>>>>>         eb->invalid_flags |= EXEC_OBJECT_NEEDS_GTT;
>>>>>diff --git a/drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h 
>>>>>b/drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h
>>>>>new file mode 100644
>>>>>index 000000000000..36262a6357b5
>>>>>--- /dev/null
>>>>>+++ b/drivers/gpu/drm/i915/gem/i915_gem_vm_bind.h
>>>>>@@ -0,0 +1,26 @@
>>>>>+/* SPDX-License-Identifier: MIT */
>>>>>+/*
>>>>>+ * Copyright © 2022 Intel Corporation
>>>>>+ */
>>>>>+
>>>>>+#ifndef __I915_GEM_VM_BIND_H
>>>>>+#define __I915_GEM_VM_BIND_H
>>>>>+
>>>>>+#include <linux/types.h>
>>>>>+
>>>>>+struct drm_device;
>>>>>+struct drm_file;
>>>>>+struct i915_address_space;
>>>>>+struct i915_vma;
>>>>>+
>>>>>+struct i915_vma *
>>>>>+i915_gem_vm_bind_lookup_vma(struct i915_address_space *vm, u64 va);
>>>>>+
>>>>>+int i915_gem_vm_bind_ioctl(struct drm_device *dev, void *data,
>>>>>+               struct drm_file *file);
>>>>>+int i915_gem_vm_unbind_ioctl(struct drm_device *dev, void *data,
>>>>>+                 struct drm_file *file);
>>>>>+
>>>>>+void i915_gem_vm_unbind_all(struct i915_address_space *vm);
>>>>>+
>>>>>+#endif /* __I915_GEM_VM_BIND_H */
>>>>>diff --git 
>>>>>a/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c 
>>>>>b/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c
>>>>>new file mode 100644
>>>>>index 000000000000..e529162abd2c
>>>>>--- /dev/null
>>>>>+++ b/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c
>>>>>@@ -0,0 +1,306 @@
>>>>>+// SPDX-License-Identifier: MIT
>>>>>+/*
>>>>>+ * Copyright © 2022 Intel Corporation
>>>>>+ */
>>>>>+
>>>>>+#include <uapi/drm/i915_drm.h>
>>>>>+
>>>>>+#include <linux/interval_tree_generic.h>
>>>>>+
>>>>>+#include "gem/i915_gem_context.h"
>>>>>+#include "gem/i915_gem_vm_bind.h"
>>>>>+
>>>>>+#include "gt/intel_gpu_commands.h"
>>>>>+
>>>>>+#define START(node) ((node)->start)
>>>>>+#define LAST(node) ((node)->last)
>>>>>+
>>>>>+INTERVAL_TREE_DEFINE(struct i915_vma, rb, u64, __subtree_last,
>>>>>+             START, LAST, static inline, i915_vm_bind_it)
>>>>>+
>>>>>+#undef START
>>>>>+#undef LAST
>>>>>+
>>>>>+/**
>>>>>+ * DOC: VM_BIND/UNBIND ioctls
>>>>>+ *
>>>>>+ * DRM_I915_GEM_VM_BIND/UNBIND ioctls allows UMD to 
>>>>>bind/unbind GEM buffer
>>>>>+ * objects (BOs) or sections of a BOs at specified GPU 
>>>>>virtual addresses on a
>>>>>+ * specified address space (VM). Multiple mappings can map to 
>>>>>the same physical
>>>>>+ * pages of an object (aliasing). These mappings (also 
>>>>>referred to as persistent
>>>>>+ * mappings) will be persistent across multiple GPU 
>>>>>submissions (execbuf calls)
>>>>>+ * issued by the UMD, without user having to provide a list 
>>>>>of all required
>>>>>+ * mappings during each submission (as required by older 
>>>>>execbuf mode).
>>>>>+ *
>>>>>+ * The VM_BIND/UNBIND calls allow UMDs to request a timeline 
>>>>>out fence for
>>>>>+ * signaling the completion of bind/unbind operation.
>>>>>+ *
>>>>>+ * VM_BIND feature is advertised to user via 
>>>>>I915_PARAM_VM_BIND_VERSION.
>>>>>+ * User has to opt-in for VM_BIND mode of binding for an 
>>>>>address space (VM)
>>>>>+ * during VM creation time via 
>>>>>I915_VM_CREATE_FLAGS_USE_VM_BIND extension.
>>>>>+ *
>>>>>+ * VM_BIND/UNBIND ioctl calls executed on different CPU 
>>>>>threads concurrently
>>>>>+ * are not ordered. Furthermore, parts of the VM_BIND/UNBIND 
>>>>>operations can be
>>>>>+ * done asynchronously, when valid out fence is specified.
>>>>>+ *
>>>>>+ * VM_BIND locking order is as below.
>>>>>+ *
>>>>>+ * 1) vm_bind_lock mutex will protect vm_bind lists. This 
>>>>>lock is taken in
>>>>>+ *    vm_bind/vm_unbind ioctl calls, in the execbuf path and 
>>>>>while releasing the
>>>>>+ *    mapping.
>>>>>+ *
>>>>>+ *    In future, when GPU page faults are supported, we can 
>>>>>potentially use a
>>>>>+ *    rwsem instead, so that multiple page fault handlers can 
>>>>>take the read
>>>>>+ *    side lock to lookup the mapping and hence can run in parallel.
>>>>>+ *    The older execbuf mode of binding do not need this lock.
>>>>>+ *
>>>>>+ * 2) The object's dma-resv lock will protect i915_vma state and needs
>>>>>+ *    to be held while binding/unbinding a vma in the async 
>>>>>worker and while
>>>>>+ *    updating dma-resv fence list of an object. Note that 
>>>>>private BOs of a VM
>>>>>+ *    will all share a dma-resv object.
>>>>>+ *
>>>>>+ * 3) Spinlock/s to protect some of the VM's lists like the list of
>>>>>+ *    invalidated vmas (due to eviction and userptr invalidation) etc.
>>>>>+ */
>>>>>+
>>>>>+/**
>>>>>+ * i915_gem_vm_bind_lookup_vma() - lookup for persistent vma 
>>>>>mapped at a
>>>>>+ * specified address
>>>>>+ * @vm: virtual address space to look for persistent vma
>>>>>+ * @va: starting address where vma is mapped
>>>>>+ *
>>>>>+ * Retrieves the persistent vma mapped address @va from the 
>>>>>@vm's vma tree.
>>>>>+ *
>>>>>+ * Returns vma pointer on success, NULL on failure.
>>>>>+ */
>>>>>+struct i915_vma *
>>>>>+i915_gem_vm_bind_lookup_vma(struct i915_address_space *vm, u64 va)
>>>>>+{
>>>>>+    lockdep_assert_held(&vm->vm_bind_lock);
>>>>>+
>>>>>+    return i915_vm_bind_it_iter_first(&vm->va, va, va);
>>>>>+}
>>>>>+
>>>>>+static void i915_gem_vm_bind_remove(struct i915_vma *vma, 
>>>>>bool release_obj)
>>>>>+{
>>>>>+    lockdep_assert_held(&vma->vm->vm_bind_lock);
>>>>>+
>>>>>+    list_del_init(&vma->vm_bind_link);
>>>>>+    i915_vm_bind_it_remove(vma, &vma->vm->va);
>>>>>+
>>>>>+    /* Release object */
>>>>>+    if (release_obj)
>>>>>+        i915_gem_object_put(vma->obj);
>>>>>+}
>>>>>+
>>>>>+static int i915_gem_vm_unbind_vma(struct i915_address_space *vm,
>>>>>+                  struct drm_i915_gem_vm_unbind *va)
>>>>>+{
>>>>>+    struct drm_i915_gem_object *obj;
>>>>>+    struct i915_vma *vma;
>>>>>+    int ret;
>>>>>+
>>>>>+    ret = mutex_lock_interruptible(&vm->vm_bind_lock);
>>>>>+    if (ret)
>>>>>+        return ret;
>>>>>+
>>>>>+    va->start = gen8_noncanonical_addr(va->start);
>>>>>+    vma = i915_gem_vm_bind_lookup_vma(vm, va->start);
>>>>>+
>>>>>+    if (!vma)
>>>>>+        ret = -ENOENT;
>>>>>+    else if (vma->size != va->length)
>>>>>+        ret = -EINVAL;
>>>>>+
>>>>>+    if (ret) {
>>>>>+        mutex_unlock(&vm->vm_bind_lock);
>>>>>+        return ret;
>>>>>+    }
>>>>>+
>>>>>+    i915_gem_vm_bind_remove(vma, false);
>>>>>+
>>>>>+    mutex_unlock(&vm->vm_bind_lock);
>>>>>+
>>>>>+    /* Destroy vma and then release object */
>>>>>+    obj = vma->obj;
>>>>>+    ret = i915_gem_object_lock(obj, NULL);
>>>>>+    if (ret)
>>>>>+        return ret;
>>>>>+
>>>>>+    i915_vma_destroy(vma);
>>>>>+    i915_gem_object_unlock(obj);
>>>>>+
>>>>>+    i915_gem_object_put(obj);
>>>>>+
>>>>>+    return 0;
>>>>>+}
>>>>>+
>>>>>+/**
>>>>>+ * i915_gem_vm_unbind_all() - unbind all persistent mappings from an
>>>>>+ * address space
>>>>>+ * @vm: Address spece to remove persistent mappings from
>>>>>+ *
>>>>>+ * Unbind all userspace requested vm_bind mappings from @vm.
>>>>>+ */
>>>>>+void i915_gem_vm_unbind_all(struct i915_address_space *vm)
>>>>>+{
>>>>>+    struct i915_vma *vma, *t;
>>>>>+
>>>>>+    mutex_lock(&vm->vm_bind_lock);
>>>>>+    list_for_each_entry_safe(vma, t, &vm->vm_bind_list, vm_bind_link)
>>>>>+        i915_gem_vm_bind_remove(vma, true);
>>>>>+    list_for_each_entry_safe(vma, t, &vm->vm_bound_list, vm_bind_link)
>>>>>+        i915_gem_vm_bind_remove(vma, true);
>>>>>+    mutex_unlock(&vm->vm_bind_lock);
>>>>>+}
>>>>>+
>>>>>+static struct i915_vma *vm_bind_get_vma(struct i915_address_space *vm,
>>>>>+                    struct drm_i915_gem_object *obj,
>>>>>+                    struct drm_i915_gem_vm_bind *va)
>>>>>+{
>>>>>+    struct i915_gtt_view view;
>>>>>+    struct i915_vma *vma;
>>>>>+
>>>>>+    va->start = gen8_noncanonical_addr(va->start);
>>>>>+    vma = i915_gem_vm_bind_lookup_vma(vm, va->start);
>>>>>+    if (vma)
>>>>>+        return ERR_PTR(-EEXIST);
>>>>>+
>>>>>+    view.type = I915_GTT_VIEW_PARTIAL;
>>>>>+    view.partial.offset = va->offset >> PAGE_SHIFT;
>>>>>+    view.partial.size = va->length >> PAGE_SHIFT;
>>>>>+    vma = i915_vma_create_persistent(obj, vm, &view);
>>>>>+    if (IS_ERR(vma))
>>>>>+        return vma;
>>>>>+
>>>>>+    vma->start = va->start;
>>>>>+    vma->last = va->start + va->length - 1;
>>>>>+
>>>>>+    return vma;
>>>>>+}
>>>>>+
>>>>>+static int i915_gem_vm_bind_obj(struct i915_address_space *vm,
>>>>>+                struct drm_i915_gem_vm_bind *va,
>>>>>+                struct drm_file *file)
>>>>>+{
>>>>>+    struct drm_i915_gem_object *obj;
>>>>>+    struct i915_vma *vma = NULL;
>>>>>+    struct i915_gem_ww_ctx ww;
>>>>>+    u64 pin_flags;
>>>>>+    int ret = 0;
>>>>>+
>>>>>+    if (!vm->vm_bind_mode)
>>>>>+        return -EOPNOTSUPP;
>>>>>+
>>>>>+    obj = i915_gem_object_lookup(file, va->handle);
>>>>>+    if (!obj)
>>>>>+        return -ENOENT;
>>>>>+
>>>>>+    if (!va->length ||
>>>>>+        !IS_ALIGNED(va->offset | va->length,
>>>>>+            i915_gem_object_max_page_size(obj->mm.placements,
>>>>>+                              obj->mm.n_placements)) ||
>>>>>+        range_overflows_t(u64, va->offset, va->length, 
>>>>>obj->base.size)) {
>>>>>+        ret = -EINVAL;
>>>>>+        goto put_obj;
>>>>>+    }
>>>>>+
>>>>>+    ret = mutex_lock_interruptible(&vm->vm_bind_lock);
>>>>>+    if (ret)
>>>>>+        goto put_obj;
>>>>>+
>>>>>+    vma = vm_bind_get_vma(vm, obj, va);
>>>>>+    if (IS_ERR(vma)) {
>>>>>+        ret = PTR_ERR(vma);
>>>>>+        goto unlock_vm;
>>>>>+    }
>>>>>+
>>>>>+    pin_flags = va->start | PIN_OFFSET_FIXED | PIN_USER;
>>>>>+
>>>>>+    for_i915_gem_ww(&ww, ret, true) {
>>>>>+        ret = i915_gem_object_lock(vma->obj, &ww);
>>>>>+        if (ret)
>>>>>+            continue;
>>>>>+
>>>>>+        ret = i915_vma_pin_ww(vma, &ww, 0, 0, pin_flags);
>>>>>+        if (ret)
>>>>>+            continue;
>>>>>+
>>>>>+        /* Make it evictable */
>>>>>+        __i915_vma_unpin(vma);
>>>>>+
>>>>>+        list_add_tail(&vma->vm_bind_link, &vm->vm_bound_list);
>>>>>+        i915_vm_bind_it_insert(vma, &vm->va);
>>>>>+
>>>>>+        /* Hold object reference until vm_unbind */
>>>>>+        i915_gem_object_get(vma->obj);
>>>>>+    }
>>>>>+
>>>>>+    if (ret)
>>>>>+        i915_vma_destroy(vma);
>>>>>+unlock_vm:
>>>>>+    mutex_unlock(&vm->vm_bind_lock);
>>>>>+put_obj:
>>>>>+    i915_gem_object_put(obj);
>>>>>+
>>>>>+    return ret;
>>>>>+}
>>>>>+
>>>>>+/**
>>>>>+ * i915_gem_vm_bind_ioctl() - ioctl function for binding a 
>>>>>section of object
>>>>>+ * at a specified virtual address
>>>>>+ * @dev: drm_device pointer
>>>>>+ * @data: ioctl data structure
>>>>>+ * @file: drm_file pointer
>>>>>+ *
>>>>>+ * Adds the specified persistent mapping (virtual address to 
>>>>>a section of an
>>>>>+ * object) and binds it in the device page table.
>>>>>+ *
>>>>>+ * Returns 0 on success, error code on failure.
>>>>>+ */
>>>>>+int i915_gem_vm_bind_ioctl(struct drm_device *dev, void *data,
>>>>>+               struct drm_file *file)
>>>>>+{
>>>>>+    struct drm_i915_gem_vm_bind *args = data;
>>>>>+    struct i915_address_space *vm;
>>>>>+    int ret;
>>>>>+
>>>>>+    vm = i915_gem_vm_lookup(file->driver_priv, args->vm_id);
>>>>>+    if (unlikely(!vm))
>>>>>+        return -ENOENT;
>>>>>+
>>>>>+    ret = i915_gem_vm_bind_obj(vm, args, file);
>>>>>+
>>>>>+    i915_vm_put(vm);
>>>>>+    return ret;
>>>>>+}
>>>>>+
>>>>>+/**
>>>>>+ * i915_gem_vm_unbind_ioctl() - ioctl function for unbinding 
>>>>>a mapping at a
>>>>>+ * specified virtual address
>>>>>+ * @dev: drm_device pointer
>>>>>+ * @data: ioctl data structure
>>>>>+ * @file: drm_file pointer
>>>>>+ *
>>>>>+ * Removes the persistent mapping at the specified address 
>>>>>and unbinds it
>>>>>+ * from the device page table.
>>>>>+ *
>>>>>+ * Returns 0 on success, error code on failure. -ENOENT is 
>>>>>returned if the
>>>>>+ * specified mapping is not found.
>>>>>+ */
>>>>>+int i915_gem_vm_unbind_ioctl(struct drm_device *dev, void *data,
>>>>>+                 struct drm_file *file)
>>>>>+{
>>>>>+    struct drm_i915_gem_vm_unbind *args = data;
>>>>>+    struct i915_address_space *vm;
>>>>>+    int ret;
>>>>>+
>>>>>+    vm = i915_gem_vm_lookup(file->driver_priv, args->vm_id);
>>>>>+    if (unlikely(!vm))
>>>>>+        return -ENOENT;
>>>>>+
>>>>>+    ret = i915_gem_vm_unbind_vma(vm, args);
>>>>>+
>>>>>+    i915_vm_put(vm);
>>>>>+    return ret;
>>>>>+}
>>>>>diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c 
>>>>>b/drivers/gpu/drm/i915/gt/intel_gtt.c
>>>>>index b67831833c9a..0daa70c6ed0d 100644
>>>>>--- a/drivers/gpu/drm/i915/gt/intel_gtt.c
>>>>>+++ b/drivers/gpu/drm/i915/gt/intel_gtt.c
>>>>>@@ -12,6 +12,7 @@
>>>>> #include "gem/i915_gem_internal.h"
>>>>> #include "gem/i915_gem_lmem.h"
>>>>>+#include "gem/i915_gem_vm_bind.h"
>>>>> #include "i915_trace.h"
>>>>> #include "i915_utils.h"
>>>>> #include "intel_gt.h"
>>>>>@@ -176,6 +177,8 @@ int i915_vm_lock_objects(struct 
>>>>>i915_address_space *vm,
>>>>> void i915_address_space_fini(struct i915_address_space *vm)
>>>>> {
>>>>>     drm_mm_takedown(&vm->mm);
>>>>>+    GEM_BUG_ON(!RB_EMPTY_ROOT(&vm->va.rb_root));
>>>>>+    mutex_destroy(&vm->vm_bind_lock);
>>>>> }
>>>>> /**
>>>>>@@ -202,6 +205,8 @@ static void __i915_vm_release(struct 
>>>>>work_struct *work)
>>>>>     struct i915_address_space *vm =
>>>>>         container_of(work, struct i915_address_space, release_work);
>>>>>+    i915_gem_vm_unbind_all(vm);
>>>>>+
>>>>>     __i915_vm_close(vm);
>>>>>     /* Synchronize async unbinds. */
>>>>>@@ -282,6 +287,11 @@ void i915_address_space_init(struct 
>>>>>i915_address_space *vm, int subclass)
>>>>>     INIT_LIST_HEAD(&vm->bound_list);
>>>>>     INIT_LIST_HEAD(&vm->unbound_list);
>>>>>+
>>>>>+    vm->va = RB_ROOT_CACHED;
>>>>>+    INIT_LIST_HEAD(&vm->vm_bind_list);
>>>>>+    INIT_LIST_HEAD(&vm->vm_bound_list);
>>>>>+    mutex_init(&vm->vm_bind_lock);
>>>>> }
>>>>> void *__px_vaddr(struct drm_i915_gem_object *p)
>>>>>diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.h 
>>>>>b/drivers/gpu/drm/i915/gt/intel_gtt.h
>>>>>index c0ca53cba9f0..b52061858161 100644
>>>>>--- a/drivers/gpu/drm/i915/gt/intel_gtt.h
>>>>>+++ b/drivers/gpu/drm/i915/gt/intel_gtt.h
>>>>>@@ -259,6 +259,23 @@ struct i915_address_space {
>>>>>      */
>>>>>     struct list_head unbound_list;
>>>>>+    /**
>>>>>+     * @vm_bind_mode: flag to indicate vm_bind method of binding
>>>>>+     *
>>>>>+     * True: allow only vm_bind method of binding.
>>>>>+     * False: allow only legacy execbuff method of binding.
>>>>>+     */
>>>>>+    bool vm_bind_mode:1;
>>>>>+
>>>>>+    /** @vm_bind_lock: Mutex to protect @vm_bind_list and 
>>>>>@vm_bound_list */
>>>>>+    struct mutex vm_bind_lock;
>>>>>+    /** @vm_bind_list: List of vm_binding in process */
>>>>>+    struct list_head vm_bind_list;
>>>>>+    /** @vm_bound_list: List of vm_binding completed */
>>>>>+    struct list_head vm_bound_list;
>>>>>+    /* @va: tree of persistent vmas */
>>>>>+    struct rb_root_cached va;
>>>>>+
>>>>>     /* Global GTT */
>>>>>     bool is_ggtt:1;
>>>>>diff --git a/drivers/gpu/drm/i915/i915_driver.c 
>>>>>b/drivers/gpu/drm/i915/i915_driver.c
>>>>>index fb3826dabe8b..c3a9a5031cdb 100644
>>>>>--- a/drivers/gpu/drm/i915/i915_driver.c
>>>>>+++ b/drivers/gpu/drm/i915/i915_driver.c
>>>>>@@ -69,6 +69,7 @@
>>>>> #include "gem/i915_gem_ioctls.h"
>>>>> #include "gem/i915_gem_mman.h"
>>>>> #include "gem/i915_gem_pm.h"
>>>>>+#include "gem/i915_gem_vm_bind.h"
>>>>> #include "gt/intel_gt.h"
>>>>> #include "gt/intel_gt_pm.h"
>>>>> #include "gt/intel_rc6.h"
>>>>>@@ -1893,6 +1894,8 @@ static const struct drm_ioctl_desc 
>>>>>i915_ioctls[] = {
>>>>>     DRM_IOCTL_DEF_DRV(I915_QUERY, i915_query_ioctl, DRM_RENDER_ALLOW),
>>>>>     DRM_IOCTL_DEF_DRV(I915_GEM_VM_CREATE, 
>>>>>i915_gem_vm_create_ioctl, DRM_RENDER_ALLOW),
>>>>>     DRM_IOCTL_DEF_DRV(I915_GEM_VM_DESTROY, 
>>>>>i915_gem_vm_destroy_ioctl, DRM_RENDER_ALLOW),
>>>>>+    DRM_IOCTL_DEF_DRV(I915_GEM_VM_BIND, 
>>>>>i915_gem_vm_bind_ioctl, DRM_RENDER_ALLOW),
>>>>>+    DRM_IOCTL_DEF_DRV(I915_GEM_VM_UNBIND, 
>>>>>i915_gem_vm_unbind_ioctl, DRM_RENDER_ALLOW),
>>>>> };
>>>>> /*
>>>>>diff --git a/drivers/gpu/drm/i915/i915_vma.c 
>>>>>b/drivers/gpu/drm/i915/i915_vma.c
>>>>>index 5839e1f55f00..33f910473263 100644
>>>>>--- a/drivers/gpu/drm/i915/i915_vma.c
>>>>>+++ b/drivers/gpu/drm/i915/i915_vma.c
>>>>>@@ -239,6 +239,7 @@ vma_create(struct drm_i915_gem_object *obj,
>>>>>     spin_unlock(&obj->vma.lock);
>>>>>     mutex_unlock(&vm->mutex);
>>>>>+    INIT_LIST_HEAD(&vma->vm_bind_link);
>>>>>     return vma;
>>>>> err_unlock:
>>>>>diff --git a/drivers/gpu/drm/i915/i915_vma_types.h 
>>>>>b/drivers/gpu/drm/i915/i915_vma_types.h
>>>>>index 2200f1f103ba..f56ac07c6cfa 100644
>>>>>--- a/drivers/gpu/drm/i915/i915_vma_types.h
>>>>>+++ b/drivers/gpu/drm/i915/i915_vma_types.h
>>>>>@@ -296,6 +296,20 @@ struct i915_vma {
>>>>>     /** This object's place on the active/inactive lists */
>>>>>     struct list_head vm_link;
>>>>>+    /** @vm_bind_link: node for the vm_bind related lists of vm */
>>>>>+    struct list_head vm_bind_link;
>>>>>+
>>>>>+    /** Interval tree structures for persistent vma */
>>>>>+
>>>>>+    /** @rb: node for the interval tree of vm for persistent vmas */
>>>>>+    struct rb_node rb;
>>>>>+    /** @start: start endpoint of the rb node */
>>>>>+    u64 start;
>>>>>+    /** @last: Last endpoint of the rb node */
>>>>>+    u64 last;
>>>>>+    /** @__subtree_last: last in subtree */
>>>>>+    u64 __subtree_last;
>>>>>+
>>>>>     struct list_head obj_link; /* Link in the object's VMA list */
>>>>>     struct rb_node obj_node;
>>>>>     struct hlist_node obj_hash;
>>>>>diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
>>>>>index 520ad2691a99..9760564b4693 100644
>>>>>--- a/include/uapi/drm/i915_drm.h
>>>>>+++ b/include/uapi/drm/i915_drm.h
>>>>>@@ -470,6 +470,8 @@ typedef struct _drm_i915_sarea {
>>>>> #define DRM_I915_GEM_VM_CREATE        0x3a
>>>>> #define DRM_I915_GEM_VM_DESTROY        0x3b
>>>>> #define DRM_I915_GEM_CREATE_EXT        0x3c
>>>>>+#define DRM_I915_GEM_VM_BIND        0x3d
>>>>>+#define DRM_I915_GEM_VM_UNBIND        0x3e
>>>>> /* Must be kept compact -- no holes */
>>>>> #define DRM_IOCTL_I915_INIT        DRM_IOW( DRM_COMMAND_BASE 
>>>>>+ DRM_I915_INIT, drm_i915_init_t)
>>>>>@@ -534,6 +536,8 @@ typedef struct _drm_i915_sarea {
>>>>> #define DRM_IOCTL_I915_QUERY            
>>>>>DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_QUERY, struct 
>>>>>drm_i915_query)
>>>>> #define DRM_IOCTL_I915_GEM_VM_CREATE    
>>>>>DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_VM_CREATE, struct 
>>>>>drm_i915_gem_vm_control)
>>>>> #define DRM_IOCTL_I915_GEM_VM_DESTROY    DRM_IOW 
>>>>>(DRM_COMMAND_BASE + DRM_I915_GEM_VM_DESTROY, struct 
>>>>>drm_i915_gem_vm_control)
>>>>>+#define DRM_IOCTL_I915_GEM_VM_BIND    
>>>>>DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_VM_BIND, struct 
>>>>>drm_i915_gem_vm_bind)
>>>>>+#define DRM_IOCTL_I915_GEM_VM_UNBIND    
>>>>>DRM_IOWR(DRM_COMMAND_BASE + DRM_I915_GEM_VM_UNBIND, struct 
>>>>>drm_i915_gem_vm_unbind)
>>>>> /* Allow drivers to submit batchbuffers directly to hardware, relying
>>>>>  * on the security mechanisms provided by hardware.
>>>>>@@ -3717,6 +3721,114 @@ struct 
>>>>>drm_i915_gem_create_ext_protected_content {
>>>>> /* ID of the protected content session managed by i915 when 
>>>>>PXP is active */
>>>>> #define I915_PROTECTED_CONTENT_DEFAULT_SESSION 0xf
>>>>>+/**
>>>>>+ * struct drm_i915_gem_vm_bind - VA to object mapping to bind.
>>>>>+ *
>>>>>+ * This structure is passed to VM_BIND ioctl and specifies 
>>>>>the mapping of GPU
>>>>>+ * virtual address (VA) range to the section of an object 
>>>>>that should be bound
>>>>>+ * in the device page table of the specified address space (VM).
>>>>>+ * The VA range specified must be unique (ie., not currently 
>>>>>bound) and can
>>>>>+ * be mapped to whole object or a section of the object 
>>>>>(partial binding).
>>>>>+ * Multiple VA mappings can be created to the same section of 
>>>>>the object
>>>>>+ * (aliasing).
>>>>>+ *
>>>>>+ * The @start, @offset and @length must be 4K page aligned. 
>>>>>However the DG2
>>>>>+ * and XEHPSDV has 64K page size for device local memory and 
>>>>>has compact page
>>>>>+ * table. On those platforms, for binding device local-memory 
>>>>>objects, the
>>>>>+ * @start, @offset and @length must be 64K aligned. Also, 
>>>>>UMDs should not mix
>>>>>+ * the local memory 64K page and the system memory 4K page 
>>>>>bindings in the same
>>>>>+ * 2M range.
>>>>
>>>>This is hopefully no longer the case if we land:
>>>>https://patchwork.freedesktop.org/series/109126/
>>>>
>>>>Should only need 64K alignment, and mixing should be fine now, 
>>>>which should be a lot nicer. Hopefully doesn't really impact 
>>>>your series, other than just updating the comment here?
>>>>
>>>
>>>Oh great. Will remove the last sentence then.
>>>Yah, we just need to remove the extra comment here.
>>>
>>>>>+ *
>>>>>+ * Error code -EINVAL will be returned if @start, @offset and 
>>>>>@length are not
>>>>>+ * properly aligned. In version 1 (See 
>>>>>I915_PARAM_VM_BIND_VERSION), error code
>>>>>+ * -ENOSPC will be returned if the VA range specified can't 
>>>>>be reserved.
>>>>>+ *
>>>>>+ * VM_BIND/UNBIND ioctl calls executed on different CPU 
>>>>>threads concurrently
>>>>>+ * are not ordered. Furthermore, parts of the VM_BIND 
>>>>>operation can be done
>>>>>+ * asynchronously, if valid @fence is specified.
>>>>>+ */
>>>>>+struct drm_i915_gem_vm_bind {
>>>>>+    /** @vm_id: VM (address space) id to bind */
>>>>>+    __u32 vm_id;
>>>>>+
>>>>>+    /** @handle: Object handle */
>>>>>+    __u32 handle;
>>>>>+
>>>>>+    /** @start: Virtual Address start to bind */
>>>>>+    __u64 start;
>>>>>+
>>>>>+    /** @offset: Offset in object to bind */
>>>>>+    __u64 offset;
>>>>>+
>>>>>+    /** @length: Length of mapping to bind */
>>>>>+    __u64 length;
>>>>>+
>>>>>+    /**
>>>>>+     * @flags: Currently reserved, MBZ.
>>>>>+     *
>>>>>+     * Note that @fence carries its own flags.
>>>>>+     */
>>>>>+    __u64 flags;
>>>>>+
>>>>>+    /** @rsvd: Reserved, MBZ */
>>>>>+    __u64 rsvd[2];
>>>>
>>>>There is lots of rsvd stuff here and below, but I don't see 
>>>>where we are verifying if it is actually MBZ?
>>>>
>>>
>>>These rsvd fields are temporary and is replaced by a later patch in
>>>the series. However, the rsvd field in vm_unbind structure below is
>>>needed and I will update the code to check for 0.
>>
>>Ideally each patch by itself should be functional, so here we should 
>>check the padding, and then later remove the checks for the 
>>additional functionally. Or just keep the ioctls(s) disabled (return 
>>-ENODEV or so) at this point in the series, and then have the last 
>>patch in the series turn on the vm_bind support by removing the 
>>-ENODEV guards.
>
>It looks like that is maybe already the case as per the final patch: 
>"Add uapi for user to enable vm_bind_mode"?
>

Yes that is correct. The last patch enables the ioctls.

Regards,
Niranjana

>>
>>>
>>>Regards,
>>>Niranjana
>>>
>>>>>+
>>>>>+    /**
>>>>>+     * @extensions: Zero-terminated chain of extensions.
>>>>>+     *
>>>>>+     * For future extensions. See struct i915_user_extension.
>>>>>+     */
>>>>>+    __u64 extensions;
>>>>>+};
>>>>>+
>>>>>+/**
>>>>>+ * struct drm_i915_gem_vm_unbind - VA to object mapping to unbind.
>>>>>+ *
>>>>>+ * This structure is passed to VM_UNBIND ioctl and specifies 
>>>>>the GPU virtual
>>>>>+ * address (VA) range that should be unbound from the device 
>>>>>page table of the
>>>>>+ * specified address space (VM). VM_UNBIND will force unbind 
>>>>>the specified
>>>>>+ * range from device page table without waiting for any GPU 
>>>>>job to complete.
>>>>>+ * It is UMDs responsibility to ensure the mapping is no 
>>>>>longer in use before
>>>>>+ * calling VM_UNBIND.
>>>>>+ *
>>>>>+ * If the specified mapping is not found, the ioctl will 
>>>>>simply return without
>>>>>+ * any error.
>>>>>+ *
>>>>>+ * VM_BIND/UNBIND ioctl calls executed on different CPU 
>>>>>threads concurrently
>>>>>+ * are not ordered. Furthermore, parts of the VM_UNBIND 
>>>>>operation can be done
>>>>>+ * asynchronously, if valid @fence is specified.
>>>>>+ */
>>>>>+struct drm_i915_gem_vm_unbind {
>>>>>+    /** @vm_id: VM (address space) id to bind */
>>>>>+    __u32 vm_id;
>>>>>+
>>>>>+    /** @rsvd: Reserved, MBZ */
>>>>>+    __u32 rsvd;
>>>>>+
>>>>>+    /** @start: Virtual Address start to unbind */
>>>>>+    __u64 start;
>>>>>+
>>>>>+    /** @length: Length of mapping to unbind */
>>>>>+    __u64 length;
>>>>>+
>>>>>+    /**
>>>>>+     * @flags: Currently reserved, MBZ.
>>>>>+     *
>>>>>+     * Note that @fence carries its own flags.
>>>>>+     */
>>>>>+    __u64 flags;
>>>>>+
>>>>>+    /** @rsvd2: Reserved, MBZ */
>>>>>+    __u64 rsvd2[2];
>>>>>+
>>>>>+    /**
>>>>>+     * @extensions: Zero-terminated chain of extensions.
>>>>>+     *
>>>>>+     * For future extensions. See struct i915_user_extension.
>>>>>+     */
>>>>>+    __u64 extensions;
>>>>>+};
>>>>>+
>>>>> #if defined(__cplusplus)
>>>>> }
>>>>> #endif


More information about the Intel-gfx mailing list