From: Rob Clark <robdcl...@chromium.org>

Hook up the map/remap/unmap ops to apply MAP/UNMAP operations.  The
MAP/UNMAP operations are split up by drm_gpuvm into a series of map/
remap/unmap ops, for example an UNMAP operation which spans multiple
vmas will get split up into a sequence of unmap (and possibly remap)
ops which each apply to a single vma.

Signed-off-by: Rob Clark <robdcl...@chromium.org>
---
 drivers/gpu/drm/msm/msm_gem.h     |  12 ++
 drivers/gpu/drm/msm/msm_gem_vma.c | 329 ++++++++++++++++++++++++++++--
 2 files changed, 320 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h
index 117f0e35e628..7f6315a66751 100644
--- a/drivers/gpu/drm/msm/msm_gem.h
+++ b/drivers/gpu/drm/msm/msm_gem.h
@@ -76,6 +76,16 @@ struct msm_gem_vm {
        /** @vm_lock: protects gpuvm insert/remove/traverse */
        struct mutex vm_lock;
 
+       /**
+        * @op_lock:
+        *
+        * Serializes VM operations.  Typically operations are serialized
+        * by virtue of running on the VM_BIND queue, but in the cleanup
+        * path (or if multiple VM_BIND queues) the @op_lock provides the
+        * needed serialization.
+        */
+       struct mutex op_lock;
+
        /** @mmu: The mmu object which manages the pgtables */
        struct msm_mmu *mmu;
 
@@ -121,6 +131,8 @@ void msm_vma_job_cleanup(struct msm_gem_submit *submit);
 
 struct msm_fence_context;
 
+#define MSM_VMA_DUMP (DRM_GPUVA_USERBITS << 0)
+
 /**
  * struct msm_gem_vma - a VMA mapping
  *
diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c 
b/drivers/gpu/drm/msm/msm_gem_vma.c
index 554ec93456a0..09d4746248c2 100644
--- a/drivers/gpu/drm/msm/msm_gem_vma.c
+++ b/drivers/gpu/drm/msm/msm_gem_vma.c
@@ -8,6 +8,8 @@
 #include "msm_gem.h"
 #include "msm_mmu.h"
 
+#define vm_dbg(fmt, ...) pr_debug("%s:%d: "fmt"\n", __func__, __LINE__, 
##__VA_ARGS__)
+
 static void
 msm_gem_vm_free(struct drm_gpuvm *gpuvm)
 {
@@ -20,18 +22,29 @@ msm_gem_vm_free(struct drm_gpuvm *gpuvm)
        kfree(vm);
 }
 
+static void
+msm_gem_vma_unmap_range(struct drm_gpuva *vma, uint64_t unmap_start, uint64_t 
unmap_range)
+{
+       struct msm_gem_vm *vm = to_msm_vm(vma->vm);
+
+       vm_dbg("%p:%p: %016llx %016llx", vma->vm, vma, unmap_start, unmap_start 
+ unmap_range);
+
+       if (vma->gem.obj)
+               msm_gem_assert_locked(vma->gem.obj);
+
+       vm->mmu->funcs->unmap(vm->mmu, unmap_start, unmap_range);
+}
+
 /* Actually unmap memory for the vma */
 void msm_gem_vma_unmap(struct drm_gpuva *vma)
 {
        struct msm_gem_vma *msm_vma = to_msm_vma(vma);
-       struct msm_gem_vm *vm = to_msm_vm(vma->vm);
-       unsigned size = vma->va.range;
 
        /* Don't do anything if the memory isn't mapped */
        if (!msm_vma->mapped)
                return;
 
-       vm->mmu->funcs->unmap(vm->mmu, vma->va.addr, size);
+       msm_gem_vma_unmap_range(vma, vma->va.addr, vma->va.range);
 
        msm_vma->mapped = false;
 }
@@ -52,6 +65,11 @@ msm_gem_vma_map(struct drm_gpuva *vma, int prot, struct 
sg_table *sgt)
 
        msm_vma->mapped = true;
 
+       vm_dbg("%p: %016llx %016llx", vma, vma->va.addr, vma->va.range);
+
+       if (vma->gem.obj)
+               msm_gem_assert_locked(vma->gem.obj);
+
        /*
         * NOTE: iommu/io-pgtable can allocate pages, so we cannot hold
         * a lock across map/unmap which is also used in the job_run()
@@ -79,10 +97,11 @@ static void __vma_close(struct drm_gpuva *vma)
        GEM_WARN_ON(msm_vma->mapped);
        GEM_WARN_ON(!mutex_is_locked(&vm->vm_lock));
 
-       spin_lock(&vm->mm_lock);
-       if (vma->va.addr && vm->managed)
+       if (vma->va.addr && vm->managed) {
+               spin_lock(&vm->mm_lock);
                drm_mm_remove_node(&msm_vma->node);
-       spin_unlock(&vm->mm_lock);
+               spin_unlock(&vm->mm_lock);
+       }
 
        drm_gpuva_remove(vma);
        drm_gpuva_unlink(vma);
@@ -101,11 +120,9 @@ void msm_gem_vma_close(struct drm_gpuva *vma)
         */
        GEM_WARN_ON(!vm->managed);
 
-       dma_resv_lock(drm_gpuvm_resv(vma->vm), NULL);
        mutex_lock(&vm->vm_lock);
        __vma_close(vma);
        mutex_unlock(&vm->vm_lock);
-       dma_resv_unlock(drm_gpuvm_resv(vma->vm));
 }
 
 static struct drm_gpuva *
@@ -124,6 +141,7 @@ __vma_init(struct msm_gem_vma *vma, struct drm_gpuvm *_vm,
 
        if (vm->managed) {
                BUG_ON(offset != 0);
+               BUG_ON(!obj);  /* NULL mappings not valid for kernel managed VM 
*/
                spin_lock(&vm->mm_lock);
                ret = drm_mm_insert_node_in_range(&vm->mm, &vma->node,
                                                obj->size, PAGE_SIZE, 0,
@@ -137,7 +155,8 @@ __vma_init(struct msm_gem_vma *vma, struct drm_gpuvm *_vm,
                range_end   = range_start + obj->size;
        }
 
-       GEM_WARN_ON((range_end - range_start) > obj->size);
+       if (obj)
+               GEM_WARN_ON((range_end - range_start) > obj->size);
 
        drm_gpuva_init(&vma->base, range_start, range_end - range_start, obj, 
offset);
        vma->mapped = false;
@@ -146,6 +165,9 @@ __vma_init(struct msm_gem_vma *vma, struct drm_gpuvm *_vm,
        if (ret)
                goto err_free_range;
 
+       if (!obj)
+               return &vma->base;
+
        vm_bo = drm_gpuvm_bo_obtain(&vm->base, obj);
        if (IS_ERR(vm_bo)) {
                ret = PTR_ERR(vm_bo);
@@ -190,39 +212,289 @@ msm_gem_vma_new(struct drm_gpuvm *_vm, struct 
drm_gem_object *obj,
        return &vma->base;
 }
 
+static int
+msm_gem_vm_bo_validate(struct drm_gpuvm_bo *vm_bo, struct drm_exec *exec)
+{
+       struct drm_gem_object *obj = vm_bo->obj;
+       struct drm_gpuva *vma;
+       int ret;
+
+       msm_gem_assert_locked(obj);
+
+       drm_gpuvm_bo_for_each_va (vma, vm_bo) {
+               ret = msm_gem_pin_vma_locked(obj, vma);
+               if (ret)
+                       return ret;
+       }
+
+       return 0;
+}
+
+struct op_arg {
+       unsigned flags;
+       struct drm_gpuvm *vm;
+       struct msm_gem_submit *submit;
+};
+
+static struct drm_gpuva *
+vma_from_op(struct op_arg *arg, struct drm_gpuva_op_map *op)
+{
+       struct msm_gem_vma *vma;
+
+       if (WARN_ON(list_empty(&arg->submit->preallocated_vmas)))
+               return NULL;
+
+       vma = list_first_entry(&arg->submit->preallocated_vmas,
+                              struct msm_gem_vma, base.rb.entry);
+
+       list_del(&vma->base.rb.entry);
+
+       return __vma_init(vma, arg->vm, op->gem.obj, op->gem.offset,
+                         op->va.addr, op->va.addr + op->va.range);
+}
+
+static int
+msm_gem_vm_sm_step_map(struct drm_gpuva_op *op, void *arg)
+{
+       struct drm_gem_object *obj = op->map.gem.obj;
+       struct drm_gpuva *vma;
+       struct sg_table *sgt;
+       unsigned prot;
+
+       vma = vma_from_op(arg, &op->map);
+
+       vm_dbg("%p:%p: %016llx %016llx", vma->vm, vma, vma->va.addr, 
vma->va.range);
+
+       if (obj) {
+               sgt = to_msm_bo(obj)->sgt;
+               prot = msm_gem_prot(obj);
+       } else {
+               sgt = NULL;
+               prot = IOMMU_READ | IOMMU_WRITE;
+       }
+
+       if (WARN_ON(IS_ERR(vma)))
+               return PTR_ERR(vma);
+
+       vma->flags = ((struct op_arg *)arg)->flags;
+
+       return msm_gem_vma_map(vma, prot, sgt);
+}
+
+static int
+msm_gem_vm_sm_step_remap(struct drm_gpuva_op *op, void *arg)
+{
+       struct drm_gpuvm *vm = ((struct op_arg *)arg)->vm;
+       struct drm_gpuva *orig_vma = op->remap.unmap->va;
+       struct drm_gpuva *prev_vma = NULL, *next_vma = NULL;
+       uint64_t unmap_start, unmap_range;
+       unsigned flags;
+
+       vm_dbg("orig_vma: %p:%p: %016llx %016llx", vm, orig_vma, 
orig_vma->va.addr, orig_vma->va.range);
+
+       drm_gpuva_op_remap_to_unmap_range(&op->remap, &unmap_start, 
&unmap_range);
+
+       msm_gem_vma_unmap_range(op->remap.unmap->va, unmap_start, unmap_range);
+
+       /*
+        * Part of this GEM obj is still mapped, but we're going to kill the
+        * existing VMA and replace it with one or two new ones (ie. two if)
+        * the unmapped range is in the middle of the existing (unmap) VMA.
+        * So just set the state to unmapped:
+        */
+       to_msm_vma(orig_vma)->mapped = false;
+
+       /*
+        * The prev_vma and/or next_vma are replacing the unmapped vma, and
+        * therefore should preserve it's flags:
+        */
+       flags = orig_vma->flags;
+
+       __vma_close(orig_vma);
+
+       if (op->remap.prev) {
+               prev_vma = vma_from_op(arg, op->remap.prev);
+               if (WARN_ON(IS_ERR(prev_vma)))
+                       return PTR_ERR(prev_vma);
+
+               vm_dbg("prev_vma: %p:%p: %016llx %016llx", vm, prev_vma, 
prev_vma->va.addr, prev_vma->va.range);
+               to_msm_vma(prev_vma)->mapped = true;
+               prev_vma->flags = flags;
+       }
+
+       if (op->remap.next) {
+               next_vma = vma_from_op(arg, op->remap.next);
+               if (WARN_ON(IS_ERR(next_vma)))
+                       return PTR_ERR(next_vma);
+
+               vm_dbg("next_vma: %p:%p: %016llx %016llx", vm, next_vma, 
next_vma->va.addr, next_vma->va.range);
+               to_msm_vma(next_vma)->mapped = true;
+               next_vma->flags = flags;
+       }
+
+       return 0;
+}
+
+static int
+msm_gem_vm_sm_step_unmap(struct drm_gpuva_op *op, void *priv)
+{
+       struct drm_gpuva *vma = op->unmap.va;
+
+       vm_dbg("%p:%p: %016llx %016llx", vma->vm, vma, vma->va.addr, 
vma->va.range);
+
+       msm_gem_vma_unmap(vma);
+       __vma_close(vma);
+
+       return 0;
+}
+
 static const struct drm_gpuvm_ops msm_gpuvm_ops = {
        .vm_free = msm_gem_vm_free,
+       .vm_bo_validate = msm_gem_vm_bo_validate,
+       .sm_step_map = msm_gem_vm_sm_step_map,
+       .sm_step_remap = msm_gem_vm_sm_step_remap,
+       .sm_step_unmap = msm_gem_vm_sm_step_unmap,
 };
 
+static void
+cond_lock(struct drm_gem_object *obj)
+{
+       if (!obj)
+               return;
+
+       /*
+        * Hold a ref while we have the obj locked, so drm_gpuvm doesn't
+        * manage to drop the last ref to the obj while it is locked:
+        */
+       drm_gem_object_get(obj);
+       msm_gem_lock(obj);
+}
+
+static void
+cond_unlock(struct drm_gem_object *obj)
+{
+       if (!obj)
+               return;
+
+       msm_gem_unlock(obj);
+       /* Drop the ref obtained in cond_lock(): */
+       drm_gem_object_put(obj);
+}
+
+static int
+run_bo_unmap(struct op_arg *arg, u64 req_addr, u64 req_range)
+{
+       struct drm_gpuva *vma, *next;
+       struct drm_gpuvm *vm = arg->vm;
+       int ret;
+       u64 req_end = req_addr + req_range;
+
+       GEM_WARN_ON(!mutex_is_locked(&to_msm_vm(vm)->op_lock));
+
+       /*
+        * There are two locks at play when it comes to inserting/
+        * removing objs into a VM.  There is vm->vm_lock and the
+        * obj resv lock.  Because there are N objs and M VMs, and
+        * an obj can be attached to an arbitrary # of VMs, the only
+        * alternative is a single global lock.
+        *
+        * With two locks at play, there are two possible orderings
+        * of locking:
+        *
+        * 1) locking obj first, and then vm_lock.  The problem with
+        *    this ordering is that OP_UNMAP can be touching an
+        *    arbitrary # of VMAs, with their own objs.  So we have
+        *    no way to know which objs to lock without traversing
+        *    the VM.  Which requires vm_lock!
+        *
+        * 2) vm_lock first, and then obj lock.  The problem with this
+        *    ordering is that non-VM_BIND (ie. kernel managed VM)
+        *    userspace relies on implicit VMA teardown when a BO is
+        *    freed.  Which means there are paths where we need to
+        *    traverse the obj's gpuva list to find the VM(s).  Which
+        *    requires the obj lock!
+        *
+        * To resolve this we rely on the fact that, with a VM_BIND
+        * userspace the legacy paths are disallowed.  And all paths
+        * which modify the VM also hold op_lock.  So we can savely
+        * reach into the VM to find the first object to lock.  But
+        * it means having to bypass drm_gpuvm_sm_unmap(), and pull
+        * out the impacted VMAs ourself.
+        *
+        * (The other option would be to use drm_gpuvm_sm_unmap_ops_create(),
+        * but that requires memory allocation.. which we can't do
+        * here.)
+        */
+       drm_gpuvm_for_each_va_range_safe(vma, next, vm, req_addr, req_end) {
+               struct drm_gem_object *obj = vma->gem.obj;
+
+               cond_lock(obj);
+               mutex_lock(&to_msm_vm(vm)->vm_lock);
+
+               ret = drm_gpuvm_sm_unmap_va(vma, arg,
+                                           vma->va.addr,
+                                           vma->va.range);
+
+               mutex_unlock(&to_msm_vm(vm)->vm_lock);
+               cond_unlock(obj);
+
+               if (ret)
+                       break;
+       }
+
+       return ret;
+}
+
 static int
 run_bo_op(struct msm_gem_submit *submit, const struct msm_gem_submit_bo *bo)
 {
+       struct msm_gem_vm *vm = to_msm_vm(submit->vm);
+       struct op_arg arg = {
+               .vm = submit->vm,
+               .submit = submit,
+       };
        unsigned op = bo->flags & MSM_SUBMIT_BO_OP_MASK;
+       int ret;
 
-       switch (op) {
-       case MSM_SUBMIT_BO_OP_MAP:
-       case MSM_SUBMIT_BO_OP_MAP_NULL:
-               return drm_gpuvm_sm_map(submit->vm, submit->vm, bo->iova,
-                                       bo->range, bo->obj, bo->bo_offset);
-               break;
-       case MSM_SUBMIT_BO_OP_UNMAP:
-               return drm_gpuvm_sm_unmap(submit->vm, submit->vm, bo->iova,
-                                         bo->bo_offset);
+       if (op == MSM_SUBMIT_BO_OP_UNMAP) {
+               vm_dbg("UNMAP: %p: %016llx %016llx", vm, bo->iova, bo->range);
+               ret = run_bo_unmap(&arg, bo->iova, bo->range);
+       } else {
+               if ((op == MSM_SUBMIT_BO_OP_MAP) &&
+                   (bo->flags & MSM_SUBMIT_BO_DUMP))
+                       arg.flags |= MSM_VMA_DUMP;
+
+               cond_lock(bo->obj);
+               mutex_lock(&vm->vm_lock);
+
+               vm_dbg("MAP: %p: %016llx %016llx", vm, bo->iova, bo->range);
+               ret = drm_gpuvm_sm_map(submit->vm, &arg,
+                                      bo->iova, bo->range,
+                                      bo->obj, bo->bo_offset);
+
+               mutex_unlock(&vm->vm_lock);
+               cond_unlock(bo->obj);
        }
 
-       return -EINVAL;
+       return ret;
 }
 
 static struct dma_fence *
 msm_vma_job_run(struct drm_sched_job *job)
 {
        struct msm_gem_submit *submit = to_msm_submit(job);
+       struct msm_gem_vm *vm = to_msm_vm(submit->vm);
+       int ret = 0;
+
+       vm_dbg("");
+
+       mutex_lock(&vm->op_lock);
+       vm->mmu->prealloc = &submit->prealloc;
 
        for (unsigned i = 0; i < submit->nr_bos; i++) {
                int ret = run_bo_op(submit, &submit->bos[i]);
                if (ret) {
                        to_msm_vm(submit->vm)->unusable = true;
-                       return ERR_PTR(ret);
                }
        }
 
@@ -240,8 +512,11 @@ msm_vma_job_run(struct drm_sched_job *job)
                msm_gem_unlock(obj);
        }
 
+       vm->mmu->prealloc = NULL;
+       mutex_unlock(&vm->op_lock);
+
        /* VM_BIND ops are synchronous, so no fence to wait on: */
-       return NULL;
+       return ERR_PTR(ret);
 }
 
 static void
@@ -404,6 +679,7 @@ msm_gem_vm_create(struct drm_device *drm, struct msm_mmu 
*mmu, const char *name,
 
        spin_lock_init(&vm->mm_lock);
        mutex_init(&vm->vm_lock);
+       mutex_init(&vm->op_lock);
 
        vm->mmu = mmu;
        vm->managed = managed;
@@ -433,6 +709,9 @@ void
 msm_gem_vm_close(struct drm_gpuvm *gpuvm)
 {
        struct msm_gem_vm *vm = to_msm_vm(gpuvm);
+       struct op_arg arg = {
+               .vm = gpuvm,
+       };
 
        /*
         * For kernel managed VMs, the VMAs are torn down when the handle is
@@ -444,4 +723,12 @@ msm_gem_vm_close(struct drm_gpuvm *gpuvm)
        /* Kill the scheduler now, so we aren't racing with it for cleanup: */
        drm_sched_stop(&vm->sched, NULL);
        drm_sched_fini(&vm->sched);
+
+       /* Serialize against vm scheduler thread: */
+       mutex_lock(&vm->op_lock);
+
+       /* Tear down any remaining mappings: */
+       run_bo_unmap(&arg, gpuvm->mm_start, gpuvm->mm_range);
+
+       mutex_unlock(&vm->op_lock);
 }
-- 
2.48.1

Reply via email to