diff options
author | Daniel Vetter <daniel.vetter@ffwll.ch> | 2021-06-16 14:52:12 +0200 |
---|---|---|
committer | Daniel Vetter <daniel.vetter@ffwll.ch> | 2024-01-08 14:01:49 +0100 |
commit | bdcf7226b3abaac8043f2c025f6e2fdc3cce6c9d (patch) | |
tree | f94d9327748b31bc5056d8fcd9853a4d9b0828c9 | |
parent | ce1bfa04eb36f1fd645b9599f3350ca61538a8f0 (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.c | 25 |
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 |