From: Maarten Lankhorst <maarten.lankho...@canonical.com>

Allocate and copy all kernel memory before doing reservations. This prevents a 
locking
inversion between mmap_sem and reservation_class, and allows us to drop the 
trylocking
in ttm_bo_vm_fault without upsetting lockdep.

Relocations are handled by trying with __copy_from_user_inatomic first. If that 
fails
all validation will be undone, memory copied from userspace and all validations 
retried.

Signed-off-by: Maarten Lankhorst <maarten.lankho...@canonical.com>
---
 drivers/gpu/drm/nouveau/nouveau_gem.c | 188 +++++++++++++++++++++-------------
 1 file changed, 119 insertions(+), 69 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c 
b/drivers/gpu/drm/nouveau/nouveau_gem.c
index f32b712..41a4bf6 100644
--- a/drivers/gpu/drm/nouveau/nouveau_gem.c
+++ b/drivers/gpu/drm/nouveau/nouveau_gem.c
@@ -464,8 +464,6 @@ validate_list(struct nouveau_channel *chan, struct 
nouveau_cli *cli,
              uint64_t user_pbbo_ptr)
 {
        struct nouveau_drm *drm = chan->drm;
-       struct drm_nouveau_gem_pushbuf_bo __user *upbbo =
-                               (void __force __user *)(uintptr_t)user_pbbo_ptr;
        struct nouveau_bo *nvbo;
        int ret, relocs = 0;
 
@@ -499,7 +497,7 @@ validate_list(struct nouveau_channel *chan, struct 
nouveau_cli *cli,
                        return ret;
                }
 
-               if (nv_device(drm->device)->card_type < NV_50) {
+               if (nv_device(drm->device)->card_type < NV_50 && !relocs) {
                        if (nvbo->bo.offset == b->presumed.offset &&
                            ((nvbo->bo.mem.mem_type == TTM_PL_VRAM &&
                              b->presumed.domain & NOUVEAU_GEM_DOMAIN_VRAM) ||
@@ -507,32 +505,53 @@ validate_list(struct nouveau_channel *chan, struct 
nouveau_cli *cli,
                              b->presumed.domain & NOUVEAU_GEM_DOMAIN_GART)))
                                continue;
 
-                       if (nvbo->bo.mem.mem_type == TTM_PL_TT)
-                               b->presumed.domain = NOUVEAU_GEM_DOMAIN_GART;
-                       else
-                               b->presumed.domain = NOUVEAU_GEM_DOMAIN_VRAM;
-                       b->presumed.offset = nvbo->bo.offset;
-                       b->presumed.valid = 0;
-                       relocs++;
-
-                       if (DRM_COPY_TO_USER(&upbbo[nvbo->pbbo_index].presumed,
-                                            &b->presumed, sizeof(b->presumed)))
-                               return -EFAULT;
+                       relocs = 1;
                }
        }
 
        return relocs;
 }
 
+static inline void *
+u_memcpya(uint64_t user, unsigned nmemb, unsigned size, unsigned inatomic)
+{
+       void *mem;
+       void __user *userptr = (void __force __user *)(uintptr_t)user;
+
+       mem = drm_malloc_ab(size, nmemb);
+       if (!mem)
+               return ERR_PTR(-ENOMEM);
+       size *= nmemb;
+
+       if (inatomic && (!access_ok(VERIFY_READ, userptr, size) ||
+           __copy_from_user_inatomic(mem, userptr, size))) {
+               drm_free_large(mem);
+               return ERR_PTR(-EFAULT);
+       } else if (!inatomic && copy_from_user(mem, userptr, size)) {
+               drm_free_large(mem);
+               return ERR_PTR(-EFAULT);
+       }
+
+       return mem;
+}
+
+static int
+nouveau_gem_pushbuf_reloc_apply(struct nouveau_cli *cli,
+                               struct drm_nouveau_gem_pushbuf *req,
+                               struct drm_nouveau_gem_pushbuf_bo *bo,
+                               struct drm_nouveau_gem_pushbuf_reloc *reloc);
+
 static int
 nouveau_gem_pushbuf_validate(struct nouveau_channel *chan,
                             struct drm_file *file_priv,
                             struct drm_nouveau_gem_pushbuf_bo *pbbo,
+                            struct drm_nouveau_gem_pushbuf *req,
                             uint64_t user_buffers, int nr_buffers,
-                            struct validate_op *op, int *apply_relocs)
+                            struct validate_op *op, int *do_reloc)
 {
        struct nouveau_cli *cli = nouveau_cli(file_priv);
        int ret, relocs = 0;
+       struct drm_nouveau_gem_pushbuf_reloc *reloc = NULL;
 
        INIT_LIST_HEAD(&op->vram_list);
        INIT_LIST_HEAD(&op->gart_list);
@@ -541,19 +560,19 @@ nouveau_gem_pushbuf_validate(struct nouveau_channel *chan,
        if (nr_buffers == 0)
                return 0;
 
+restart:
        ret = validate_init(chan, file_priv, pbbo, nr_buffers, op);
        if (unlikely(ret)) {
                if (ret != -ERESTARTSYS)
                        NV_ERROR(cli, "validate_init\n");
-               return ret;
+               goto err;
        }
 
        ret = validate_list(chan, cli, &op->vram_list, pbbo, user_buffers);
        if (unlikely(ret < 0)) {
                if (ret != -ERESTARTSYS)
                        NV_ERROR(cli, "validate vram_list\n");
-               validate_fini(op, NULL);
-               return ret;
+               goto err_fini;
        }
        relocs += ret;
 
@@ -561,8 +580,7 @@ nouveau_gem_pushbuf_validate(struct nouveau_channel *chan,
        if (unlikely(ret < 0)) {
                if (ret != -ERESTARTSYS)
                        NV_ERROR(cli, "validate gart_list\n");
-               validate_fini(op, NULL);
-               return ret;
+               goto err_fini;
        }
        relocs += ret;
 
@@ -570,58 +588,90 @@ nouveau_gem_pushbuf_validate(struct nouveau_channel *chan,
        if (unlikely(ret < 0)) {
                if (ret != -ERESTARTSYS)
                        NV_ERROR(cli, "validate both_list\n");
-               validate_fini(op, NULL);
-               return ret;
+               goto err_fini;
        }
        relocs += ret;
+       if (relocs) {
+               if (!reloc)
+                       reloc = u_memcpya(req->relocs, req->nr_relocs, 
sizeof(*reloc), 1);
+               if (IS_ERR(reloc)) {
+                       validate_fini(op, NULL);
+
+                       if (PTR_ERR(reloc) == -EFAULT) {
+                               reloc = u_memcpya(req->relocs, req->nr_relocs, 
sizeof(*reloc), 0);
+                               if (!IS_ERR(reloc))
+                                       goto restart;
+                       }
+
+                       return PTR_ERR(reloc);
+               }
+
+               ret = nouveau_gem_pushbuf_reloc_apply(cli, req, pbbo, reloc);
+               if (ret) {
+                       NV_ERROR(cli, "reloc apply: %d\n", ret);
+                       goto err_fini;
+               }
+               drm_free_large(reloc);
+               *do_reloc = 1;
+       }
 
-       *apply_relocs = relocs;
        return 0;
-}
 
-static inline void
-u_free(void *addr)
-{
-       if (!is_vmalloc_addr(addr))
-               kfree(addr);
-       else
-               vfree(addr);
+err_fini:
+       validate_fini(op, NULL);
+err:
+       drm_free_large(reloc);
+       return ret;
 }
 
-static inline void *
-u_memcpya(uint64_t user, unsigned nmemb, unsigned size)
+static int
+nouveau_gem_pushbuf_reloc_copy_to_user(struct drm_nouveau_gem_pushbuf *req,
+                                      struct drm_nouveau_gem_pushbuf_bo *bo)
 {
-       void *mem;
-       void __user *userptr = (void __force __user *)(uintptr_t)user;
-
-       size *= nmemb;
+       struct drm_nouveau_gem_pushbuf_bo __user *upbbo =
+                                (void __force __user *)(uintptr_t)req->buffers;
+       unsigned i;
 
-       mem = kmalloc(size, GFP_KERNEL | __GFP_NOWARN);
-       if (!mem)
-               mem = vmalloc(size);
-       if (!mem)
-               return ERR_PTR(-ENOMEM);
+       for (i = 0; i < req->nr_buffers; ++i) {
+               struct drm_nouveau_gem_pushbuf_bo *b = &bo[i];
 
-       if (DRM_COPY_FROM_USER(mem, userptr, size)) {
-               u_free(mem);
-               return ERR_PTR(-EFAULT);
+               if (!b->presumed.valid &&
+                   copy_to_user(&upbbo[i].presumed, &b->presumed, 
sizeof(b->presumed)))
+                       return -EFAULT;
        }
-
-       return mem;
+       return 0;
 }
 
 static int
 nouveau_gem_pushbuf_reloc_apply(struct nouveau_cli *cli,
                                struct drm_nouveau_gem_pushbuf *req,
-                               struct drm_nouveau_gem_pushbuf_bo *bo)
+                               struct drm_nouveau_gem_pushbuf_bo *bo,
+                               struct drm_nouveau_gem_pushbuf_reloc *reloc)
 {
-       struct drm_nouveau_gem_pushbuf_reloc *reloc = NULL;
        int ret = 0;
        unsigned i;
 
-       reloc = u_memcpya(req->relocs, req->nr_relocs, sizeof(*reloc));
-       if (IS_ERR(reloc))
-               return PTR_ERR(reloc);
+       for (i = 0; i < req->nr_buffers; ++i) {
+               struct drm_nouveau_gem_pushbuf_bo *b = &bo[i];
+               struct nouveau_bo *nvbo = (void *)(unsigned long)
+                       bo[i].user_priv;
+
+               if (nvbo->bo.offset == b->presumed.offset &&
+                   ((nvbo->bo.mem.mem_type == TTM_PL_VRAM &&
+                     b->presumed.domain & NOUVEAU_GEM_DOMAIN_VRAM) ||
+                    (nvbo->bo.mem.mem_type == TTM_PL_TT &&
+                     b->presumed.domain & NOUVEAU_GEM_DOMAIN_GART))) {
+                       b->presumed.valid = 1;
+                       continue;
+               }
+
+               if (nvbo->bo.mem.mem_type == TTM_PL_TT)
+                       b->presumed.domain = NOUVEAU_GEM_DOMAIN_GART;
+               else
+                       b->presumed.domain = NOUVEAU_GEM_DOMAIN_VRAM;
+               b->presumed.offset = nvbo->bo.offset;
+               b->presumed.valid = 0;
+       }
 
        for (i = 0; i < req->nr_relocs; i++) {
                struct drm_nouveau_gem_pushbuf_reloc *r = &reloc[i];
@@ -688,8 +738,6 @@ nouveau_gem_pushbuf_reloc_apply(struct nouveau_cli *cli,
 
                nouveau_bo_wr32(nvbo, r->reloc_bo_offset >> 2, data);
        }
-
-       u_free(reloc);
        return ret;
 }
 
@@ -745,13 +793,13 @@ nouveau_gem_ioctl_pushbuf(struct drm_device *dev, void 
*data,
                return nouveau_abi16_put(abi16, -EINVAL);
        }
 
-       push = u_memcpya(req->push, req->nr_push, sizeof(*push));
+       push = u_memcpya(req->push, req->nr_push, sizeof(*push), 0);
        if (IS_ERR(push))
                return nouveau_abi16_put(abi16, PTR_ERR(push));
 
-       bo = u_memcpya(req->buffers, req->nr_buffers, sizeof(*bo));
+       bo = u_memcpya(req->buffers, req->nr_buffers, sizeof(*bo), 0);
        if (IS_ERR(bo)) {
-               u_free(push);
+               drm_free_large(push);
                return nouveau_abi16_put(abi16, PTR_ERR(bo));
        }
 
@@ -765,7 +813,7 @@ nouveau_gem_ioctl_pushbuf(struct drm_device *dev, void 
*data,
        }
 
        /* Validate buffer list */
-       ret = nouveau_gem_pushbuf_validate(chan, file_priv, bo, req->buffers,
+       ret = nouveau_gem_pushbuf_validate(chan, file_priv, bo, req, 
req->buffers,
                                           req->nr_buffers, &op, &do_reloc);
        if (ret) {
                if (ret != -ERESTARTSYS)
@@ -773,15 +821,6 @@ nouveau_gem_ioctl_pushbuf(struct drm_device *dev, void 
*data,
                goto out_prevalid;
        }
 
-       /* Apply any relocations that are required */
-       if (do_reloc) {
-               ret = nouveau_gem_pushbuf_reloc_apply(cli, req, bo);
-               if (ret) {
-                       NV_ERROR(cli, "reloc apply: %d\n", ret);
-                       goto out;
-               }
-       }
-
        if (chan->dma.ib_max) {
                ret = nouveau_dma_wait(chan, req->nr_push + 1, 16);
                if (ret) {
@@ -861,9 +900,20 @@ out:
        validate_fini(&op, fence);
        nouveau_fence_unref(&fence);
 
+       if (do_reloc && !ret) {
+               ret = nouveau_gem_pushbuf_reloc_copy_to_user(req, bo);
+               if (ret) {
+                       NV_ERROR(cli, "error copying presumed back to 
userspace: %d\n", ret);
+                       /*
+                        * XXX: We return -EFAULT, but command submission
+                        * has already been completed.
+                        */
+               }
+       }
+
 out_prevalid:
-       u_free(bo);
-       u_free(push);
+       drm_free_large(bo);
+       drm_free_large(push);
 
 out_next:
        if (chan->dma.ib_max) {
-- 
1.8.4

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

Reply via email to