summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDaniel Vetter <daniel.vetter@ffwll.ch>2021-06-16 14:52:12 +0200
committerDaniel Vetter <daniel.vetter@ffwll.ch>2024-01-08 14:01:49 +0100
commitbdcf7226b3abaac8043f2c025f6e2fdc3cce6c9d (patch)
treef94d9327748b31bc5056d8fcd9853a4d9b0828c9
parentce1bfa04eb36f1fd645b9599f3350ca61538a8f0 (diff)
drm/i915/eb: Don't support 4GiB of relocations
The most relocation-affine userspace we have is SNA, which limits it's batches to roughly 256KB. Max relocation density is a relocation every 8 bytes (one dword instruction, one dword address), which means at most 32kb relocations. Which with 32 bytes per relocation entry means at most 1MiB. This uapi change was introduced in commit 2889caa9232109afc8881f29a2205abeb5709d0c Author: Chris Wilson <chris@chris-wilson.co.uk> Date: Fri Jun 16 15:05:19 2017 +0100 drm/i915: Eliminate lots of iterations over the execobjects array and the commit doesn't explain why this was done. The best fit explanation I could come up with is that as part of making relocations really, really fast, even for the cold relocation case an igt testcase was added with ridiculous amounts of relocations: commit 506d683da138a7b90f5e338d522012f00d3145e9 Author: Chris Wilson <chris@chris-wilson.co.uk> Date: Thu Jan 28 11:46:21 2016 +0000 tests: Add gem_exec_reloc Which I guess then motivated working on this and resulted in later igt work to limit the overflow checks, which have been quite a bit strictker than the above testcase. commit 2d2b61e1608433733de3d04a492d0d85a93933cd Author: Chris Wilson <chris@chris-wilson.co.uk> Date: Mon Mar 7 15:34:02 2016 +0000 igt/gem_reloc_overflow: Fix errno tests for "overflow" Which also means that both of these igt commits introduced new tests which failed from the start, which I guess was then justified to push for this change (hidden away in a larger patch that did install a new kitchen sink, new kitchen, new house and new village because). Or something like that. Note that the main reloc optimization patches are commit 31a39207f04a37324d70ff5665fd19d3a75b39c1 Author: Chris Wilson <chris@chris-wilson.co.uk> Date: Thu Aug 18 17:16:46 2016 +0100 drm/i915: Cache kmap between relocations and commit d50415cc6c8395602052b39a1a39290fba3d313e Author: Chris Wilson <chris@chris-wilson.co.uk> Date: Thu Aug 18 17:16:52 2016 +0100 drm/i915: Refactor execbuffer relocation writing so actually landed substantially after all the igt tests. Which means we have a pretty epic case of igt-driven uapi development here. Anyway, long story short: Burn it to the ground. Note that this doesn't fully go back to the old limit, because the old code had an overflow limit of UINT_MAX across all relocations of all buffers, due to how we only allocated a single array for all of them in the slow path. Since the main push of 2889caa92321 ("drm/i915: Eliminate lots of iterations over the execobjects array") is to lift iterations over the objects we don't absolutely have to do, keeping the per-object limit seems reasonable. But that means that we can't just revert the igt changes, but have to adapt the original old tests to the adjusted limitations. Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Cc: Jon Bloomfield <jon.bloomfield@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: "Thomas Hellström" <thomas.hellstrom@linux.intel.com> Cc: Matthew Auld <matthew.auld@intel.com> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com> Cc: Dave Airlie <airlied@redhat.com> Cc: Jason Ekstrand <jason@jlekstrand.net>
-rw-r--r--drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c25
1 files changed, 7 insertions, 18 deletions
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index dc2daf780ffd..23b5424c0d67 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -1529,9 +1529,9 @@ static int eb_relocate_vma(struct i915_execbuffer *eb, struct eb_vma *ev)
const struct drm_i915_gem_exec_object2 *entry = ev->exec;
struct drm_i915_gem_relocation_entry __user *urelocs =
u64_to_user_ptr(entry->relocs_ptr);
- unsigned long remain = entry->relocation_count;
+ unsigned int remain = entry->relocation_count;
- if (unlikely(remain > N_RELOC(ULONG_MAX)))
+ if (unlikely(remain > N_RELOC(UINT_MAX)))
return -EINVAL;
/*
@@ -1545,7 +1545,7 @@ static int eb_relocate_vma(struct i915_execbuffer *eb, struct eb_vma *ev)
do {
struct drm_i915_gem_relocation_entry *r = stack;
unsigned int count =
- min_t(unsigned long, remain, ARRAY_SIZE(stack));
+ min_t(unsigned int, remain, ARRAY_SIZE(stack));
unsigned int copied;
/*
@@ -1639,7 +1639,7 @@ static int check_relocations(const struct drm_i915_gem_exec_object2 *entry)
if (size == 0)
return 0;
- if (size > N_RELOC(ULONG_MAX))
+ if (size > N_RELOC(UINT_MAX))
return -EINVAL;
addr = u64_to_user_ptr(entry->relocs_ptr);
@@ -1667,7 +1667,7 @@ static int eb_copy_relocations(const struct i915_execbuffer *eb)
const unsigned int nreloc = eb->exec[i].relocation_count;
struct drm_i915_gem_relocation_entry __user *urelocs;
unsigned long size;
- unsigned long copied;
+ unsigned int copied;
if (nreloc == 0)
continue;
@@ -1685,19 +1685,8 @@ static int eb_copy_relocations(const struct i915_execbuffer *eb)
goto err;
}
- /* copy_from_user is limited to < 4GiB */
- copied = 0;
- do {
- unsigned int len =
- min_t(u64, BIT_ULL(31), size - copied);
-
- if (__copy_from_user((char *)relocs + copied,
- (char __user *)urelocs + copied,
- len))
- goto end;
-
- copied += len;
- } while (copied < size);
+ if (__copy_from_user(relocs, urelocs, size))
+ goto end;
/*
* As we do not update the known relocation offsets after