drm/nouveau: slowpath for pushbuf ioctl

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.

Cc: Ilia Mirkin <imirkin@alum.mit.edu>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Ben Skeggs <bskeggs@redhat.com>
Cc: nouveau@lists.freedesktop.org
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Acked-by: Dave Airlie <airlied@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20191104173801.2972-2-daniel.vetter@ffwll.ch
This commit is contained in:
Daniel Vetter 2019-11-04 18:38:00 +01:00
parent b2a8116e25
commit 03e0d26fcf

View File

@ -484,12 +484,9 @@ retry:
static int static int
validate_list(struct nouveau_channel *chan, struct nouveau_cli *cli, validate_list(struct nouveau_channel *chan, struct nouveau_cli *cli,
struct list_head *list, struct drm_nouveau_gem_pushbuf_bo *pbbo, struct list_head *list, struct drm_nouveau_gem_pushbuf_bo *pbbo)
uint64_t user_pbbo_ptr)
{ {
struct nouveau_drm *drm = chan->drm; 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; struct nouveau_bo *nvbo;
int ret, relocs = 0; int ret, relocs = 0;
@ -533,10 +530,6 @@ validate_list(struct nouveau_channel *chan, struct nouveau_cli *cli,
b->presumed.offset = nvbo->bo.offset; b->presumed.offset = nvbo->bo.offset;
b->presumed.valid = 0; b->presumed.valid = 0;
relocs++; relocs++;
if (copy_to_user(&upbbo[nvbo->pbbo_index].presumed,
&b->presumed, sizeof(b->presumed)))
return -EFAULT;
} }
} }
@ -547,8 +540,8 @@ static int
nouveau_gem_pushbuf_validate(struct nouveau_channel *chan, nouveau_gem_pushbuf_validate(struct nouveau_channel *chan,
struct drm_file *file_priv, struct drm_file *file_priv,
struct drm_nouveau_gem_pushbuf_bo *pbbo, struct drm_nouveau_gem_pushbuf_bo *pbbo,
uint64_t user_buffers, int nr_buffers, int nr_buffers,
struct validate_op *op, int *apply_relocs) struct validate_op *op, bool *apply_relocs)
{ {
struct nouveau_cli *cli = nouveau_cli(file_priv); struct nouveau_cli *cli = nouveau_cli(file_priv);
int ret; int ret;
@ -565,7 +558,7 @@ nouveau_gem_pushbuf_validate(struct nouveau_channel *chan,
return ret; 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 (unlikely(ret < 0)) {
if (ret != -ERESTARTSYS) if (ret != -ERESTARTSYS)
NV_PRINTK(err, cli, "validating bo list\n"); NV_PRINTK(err, cli, "validating bo list\n");
@ -605,16 +598,12 @@ u_memcpya(uint64_t user, unsigned nmemb, unsigned size)
static int static int
nouveau_gem_pushbuf_reloc_apply(struct nouveau_cli *cli, nouveau_gem_pushbuf_reloc_apply(struct nouveau_cli *cli,
struct drm_nouveau_gem_pushbuf *req, 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_bo *bo)
{ {
struct drm_nouveau_gem_pushbuf_reloc *reloc = NULL;
int ret = 0; int ret = 0;
unsigned i; 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++) { for (i = 0; i < req->nr_relocs; i++) {
struct drm_nouveau_gem_pushbuf_reloc *r = &reloc[i]; struct drm_nouveau_gem_pushbuf_reloc *r = &reloc[i];
struct drm_nouveau_gem_pushbuf_bo *b; struct drm_nouveau_gem_pushbuf_bo *b;
@ -693,11 +682,13 @@ nouveau_gem_ioctl_pushbuf(struct drm_device *dev, void *data,
struct nouveau_drm *drm = nouveau_drm(dev); struct nouveau_drm *drm = nouveau_drm(dev);
struct drm_nouveau_gem_pushbuf *req = data; struct drm_nouveau_gem_pushbuf *req = data;
struct drm_nouveau_gem_pushbuf_push *push; struct drm_nouveau_gem_pushbuf_push *push;
struct drm_nouveau_gem_pushbuf_reloc *reloc = NULL;
struct drm_nouveau_gem_pushbuf_bo *bo; struct drm_nouveau_gem_pushbuf_bo *bo;
struct nouveau_channel *chan = NULL; struct nouveau_channel *chan = NULL;
struct validate_op op; struct validate_op op;
struct nouveau_fence *fence = NULL; 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)) if (unlikely(!abi16))
return -ENOMEM; return -ENOMEM;
@ -755,7 +746,8 @@ nouveau_gem_ioctl_pushbuf(struct drm_device *dev, void *data,
} }
/* Validate buffer list */ /* 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); req->nr_buffers, &op, &do_reloc);
if (ret) { if (ret) {
if (ret != -ERESTARTSYS) if (ret != -ERESTARTSYS)
@ -765,7 +757,18 @@ nouveau_gem_ioctl_pushbuf(struct drm_device *dev, void *data,
/* Apply any relocations that are required */ /* Apply any relocations that are required */
if (do_reloc) { 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) { if (ret) {
NV_PRINTK(err, cli, "reloc apply: %d\n", ret); NV_PRINTK(err, cli, "reloc apply: %d\n", ret);
goto out; goto out;
@ -851,6 +854,22 @@ out:
validate_fini(&op, chan, fence, bo); validate_fini(&op, chan, fence, bo);
nouveau_fence_unref(&fence); 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: out_prevalid:
u_free(bo); u_free(bo);
u_free(push); u_free(push);