Commit 03e0d26f authored by Daniel Vetter's avatar Daniel Vetter
Browse files

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: default avatarMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
Acked-by: default avatarDave Airlie <airlied@gmail.com>
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20191104173801.2972-2-daniel.vetter@ffwll.ch
parent b2a8116e

drivers/gpu/drm/nouveau/nouveau_gem.c

+38 −19
Original line number Diff line number Diff line
@@ -484,12 +484,9 @@ retry: 


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;
@@ -533,10 +530,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;

		}

	}
@@ -547,8 +540,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;
@@ -565,7 +558,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");
@@ -605,16 +598,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;
@@ -693,11 +682,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;
@@ -755,7 +746,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)
@@ -765,7 +757,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;
@@ -851,6 +854,22 @@ out: 
	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);

CRA Git | Maintained and supported by SUSTech CRA and CCSE