diff mbox

drm/i915: Always convert incoming exec offsets to non-canonical

Message ID 20170207195559.18798-1-michal.winiarski@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Michał Winiarski Feb. 7, 2017, 7:55 p.m. UTC
We're using non-canonical addresses in drm_mm, and we're making sure that
userspace is using canonical addressing - both in case of softpin
(verifying incoming offset) and when relocating (converting to canonical
when updating offset returned to userspace).
Unfortunately when considering the need for relocations, we're comparing
offset from userspace (in canonical form) with drm_mm node (in
non-canonical form), and as a result, we end up always relocating if our
offsets are in the "problematic" range.
Let's always convert the offsets to avoid the performance impact of
relocations.

Fixes: a5f0edf63bdf ("drm/i915: Avoid writing relocs with addresses in non-canonical form")
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michel Thierry <michel.thierry@intel.com>
Reported-by: Michał Pyrzowski <michal.pyrzowski@intel.com>
Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Comments

Chris Wilson Feb. 7, 2017, 8:42 p.m. UTC | #1
On Tue, Feb 07, 2017 at 08:55:59PM +0100, Michał Winiarski wrote:
> We're using non-canonical addresses in drm_mm, and we're making sure that
> userspace is using canonical addressing - both in case of softpin
> (verifying incoming offset) and when relocating (converting to canonical
> when updating offset returned to userspace).
> Unfortunately when considering the need for relocations, we're comparing
> offset from userspace (in canonical form) with drm_mm node (in
> non-canonical form), and as a result, we end up always relocating if our
> offsets are in the "problematic" range.
> Let's always convert the offsets to avoid the performance impact of
> relocations.
> 
> Fixes: a5f0edf63bdf ("drm/i915: Avoid writing relocs with addresses in non-canonical form")
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Michel Thierry <michel.thierry@intel.com>
> Reported-by: Michał Pyrzowski <michal.pyrzowski@intel.com>
> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>

Matches what I have in my tree. Continual hint for review.
-Chris
Chris Wilson Feb. 8, 2017, 9:27 a.m. UTC | #2
On Tue, Feb 07, 2017 at 08:55:59PM +0100, Michał Winiarski wrote:
> We're using non-canonical addresses in drm_mm, and we're making sure that
> userspace is using canonical addressing - both in case of softpin
> (verifying incoming offset) and when relocating (converting to canonical
> when updating offset returned to userspace).
> Unfortunately when considering the need for relocations, we're comparing
> offset from userspace (in canonical form) with drm_mm node (in
> non-canonical form), and as a result, we end up always relocating if our
> offsets are in the "problematic" range.
> Let's always convert the offsets to avoid the performance impact of
> relocations.
> 
> Fixes: a5f0edf63bdf ("drm/i915: Avoid writing relocs with addresses in non-canonical form")
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Michel Thierry <michel.thierry@intel.com>
> Reported-by: Michał Pyrzowski <michal.pyrzowski@intel.com>
> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>

Pushed, thanks for the path.
-Chris
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 1298357..111b7f8 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1184,14 +1184,14 @@  validate_exec_list(struct drm_device *dev,
 			if (exec[i].offset !=
 			    gen8_canonical_addr(exec[i].offset & PAGE_MASK))
 				return -EINVAL;
-
-			/* From drm_mm perspective address space is continuous,
-			 * so from this point we're always using non-canonical
-			 * form internally.
-			 */
-			exec[i].offset = gen8_noncanonical_addr(exec[i].offset);
 		}
 
+		/* From drm_mm perspective address space is continuous,
+		 * so from this point we're always using non-canonical
+		 * form internally.
+		 */
+		exec[i].offset = gen8_noncanonical_addr(exec[i].offset);
+
 		if (exec[i].alignment && !is_power_of_2(exec[i].alignment))
 			return -EINVAL;