We can't copy_*_user while holding reservations, that will (soon even
for nouveau) lead to deadlocks. And it breaks the cross-driver
contract around dma_resv.

Fix this by adding a slowpath for when we need relocations, and by
pushing the writeback of the new presumed offsets to the very end.

Aside from "it compiles" entirely untested unfortunately.

Signed-off-by: Daniel Vetter <daniel.vet...@intel.com>
Cc: Ben Skeggs <bske...@redhat.com>
Cc: nouv...@lists.freedesktop.org
---
 drivers/gpu/drm/nouveau/nouveau_gem.c | 57 ++++++++++++++++++---------
 1 file changed, 38 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c 
b/drivers/gpu/drm/nouveau/nouveau_gem.c
index c77302f969e8..60309b997951 100644
--- a/drivers/gpu/drm/nouveau/nouveau_gem.c
+++ b/drivers/gpu/drm/nouveau/nouveau_gem.c
@@ -482,12 +482,9 @@ validate_init(struct nouveau_channel *chan, struct 
drm_file *file_priv,
 
 static int
 validate_list(struct nouveau_channel *chan, struct nouveau_cli *cli,
-             struct list_head *list, struct drm_nouveau_gem_pushbuf_bo *pbbo,
-             uint64_t user_pbbo_ptr)
+             struct list_head *list, struct drm_nouveau_gem_pushbuf_bo *pbbo)
 {
        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;
 
@@ -531,10 +528,6 @@ validate_list(struct nouveau_channel *chan, struct 
nouveau_cli *cli,
                        b->presumed.offset = nvbo->bo.offset;
                        b->presumed.valid = 0;
                        relocs++;
-
-                       if (copy_to_user(&upbbo[nvbo->pbbo_index].presumed,
-                                            &b->presumed, sizeof(b->presumed)))
-                               return -EFAULT;
                }
        }
 
@@ -545,8 +538,8 @@ static int
 nouveau_gem_pushbuf_validate(struct nouveau_channel *chan,
                             struct drm_file *file_priv,
                             struct drm_nouveau_gem_pushbuf_bo *pbbo,
-                            uint64_t user_buffers, int nr_buffers,
-                            struct validate_op *op, int *apply_relocs)
+                            int nr_buffers,
+                            struct validate_op *op, bool *apply_relocs)
 {
        struct nouveau_cli *cli = nouveau_cli(file_priv);
        int ret;
@@ -563,7 +556,7 @@ nouveau_gem_pushbuf_validate(struct nouveau_channel *chan,
                return ret;
        }
 
-       ret = validate_list(chan, cli, &op->list, pbbo, user_buffers);
+       ret = validate_list(chan, cli, &op->list, pbbo);
        if (unlikely(ret < 0)) {
                if (ret != -ERESTARTSYS)
                        NV_PRINTK(err, cli, "validating bo list\n");
@@ -603,16 +596,12 @@ u_memcpya(uint64_t user, unsigned nmemb, unsigned size)
 static int
 nouveau_gem_pushbuf_reloc_apply(struct nouveau_cli *cli,
                                struct drm_nouveau_gem_pushbuf *req,
+                               struct drm_nouveau_gem_pushbuf_reloc *reloc,
                                struct drm_nouveau_gem_pushbuf_bo *bo)
 {
-       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_relocs; i++) {
                struct drm_nouveau_gem_pushbuf_reloc *r = &reloc[i];
                struct drm_nouveau_gem_pushbuf_bo *b;
@@ -691,11 +680,13 @@ nouveau_gem_ioctl_pushbuf(struct drm_device *dev, void 
*data,
        struct nouveau_drm *drm = nouveau_drm(dev);
        struct drm_nouveau_gem_pushbuf *req = data;
        struct drm_nouveau_gem_pushbuf_push *push;
+       struct drm_nouveau_gem_pushbuf_reloc *reloc = NULL;
        struct drm_nouveau_gem_pushbuf_bo *bo;
        struct nouveau_channel *chan = NULL;
        struct validate_op op;
        struct nouveau_fence *fence = NULL;
-       int i, j, ret = 0, do_reloc = 0;
+       int i, j, ret = 0;
+       bool do_reloc = false;
 
        if (unlikely(!abi16))
                return -ENOMEM;
@@ -753,7 +744,8 @@ nouveau_gem_ioctl_pushbuf(struct drm_device *dev, void 
*data,
        }
 
        /* Validate buffer list */
-       ret = nouveau_gem_pushbuf_validate(chan, file_priv, bo, req->buffers,
+revalidate:
+       ret = nouveau_gem_pushbuf_validate(chan, file_priv, bo,
                                           req->nr_buffers, &op, &do_reloc);
        if (ret) {
                if (ret != -ERESTARTSYS)
@@ -763,7 +755,18 @@ nouveau_gem_ioctl_pushbuf(struct drm_device *dev, void 
*data,
 
        /* Apply any relocations that are required */
        if (do_reloc) {
-               ret = nouveau_gem_pushbuf_reloc_apply(cli, req, bo);
+               if (!reloc) {
+                       validate_fini(&op, chan, NULL, bo);
+                       reloc = u_memcpya(req->relocs, req->nr_relocs, 
sizeof(*reloc));
+                       if (IS_ERR(reloc)) {
+                               ret = PTR_ERR(reloc);
+                               goto out_prevalid;
+                       }
+
+                       goto revalidate;
+               }
+
+               ret = nouveau_gem_pushbuf_reloc_apply(cli, req, reloc, bo);
                if (ret) {
                        NV_PRINTK(err, cli, "reloc apply: %d\n", ret);
                        goto out;
@@ -849,6 +852,22 @@ nouveau_gem_ioctl_pushbuf(struct drm_device *dev, void 
*data,
        validate_fini(&op, chan, fence, bo);
        nouveau_fence_unref(&fence);
 
+       if (do_reloc) {
+               struct drm_nouveau_gem_pushbuf_bo __user *upbbo =
+                       u64_to_user_ptr(req->buffers);
+
+               for (i = 0; i < req->nr_buffers; i++) {
+                       if (bo[i].presumed.valid)
+                               continue;
+
+                       if (copy_to_user(&upbbo[i].presumed, &bo[i].presumed,
+                                        sizeof(bo[i].presumed))) {
+                               ret = -EFAULT;
+                               break;
+                       }
+               }
+               u_free(reloc);
+       }
 out_prevalid:
        u_free(bo);
        u_free(push);
-- 
2.23.0.rc1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to