[PATCH] drm/i915: Stop constraining relocation targets to canonical form

Chris Wilson chris at chris-wilson.co.uk
Tue Feb 4 23:43:42 UTC 2020


Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
---
 .../gpu/drm/i915/gem/i915_gem_execbuffer.c    | 21 ++++++++-----------
 drivers/gpu/drm/i915/gt/intel_gpu_commands.h  | 19 -----------------
 drivers/gpu/drm/i915/i915_cmd_parser.c        | 11 ++++++----
 drivers/gpu/drm/i915/i915_drv.h               | 15 +++++++++++++
 4 files changed, 31 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 87fa5f42c39a..10c9f03bb7fc 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -429,10 +429,10 @@ eb_validate_vma(struct i915_execbuffer *eb,
 
 	/*
 	 * Offset can be used as input (EXEC_OBJECT_PINNED), reject
-	 * any non-page-aligned or non-canonical addresses.
+	 * any non-page-aligned.
 	 */
 	if (unlikely(entry->flags & EXEC_OBJECT_PINNED &&
-		     entry->offset != gen8_canonical_addr(entry->offset & I915_GTT_PAGE_MASK)))
+		     entry->offset & ~I915_GTT_PAGE_MASK))
 		return -EINVAL;
 
 	/* pad_to_size was once a reserved field, so sanitize it */
@@ -455,7 +455,7 @@ eb_validate_vma(struct i915_execbuffer *eb,
 	 * so from this point we're always using non-canonical
 	 * form internally.
 	 */
-	entry->offset = gen8_noncanonical_addr(entry->offset);
+	entry->offset = gen8_noncanonical_addr(eb->i915, entry->offset);
 
 	if (!eb->reloc_cache.has_fence) {
 		entry->flags &= ~EXEC_OBJECT_NEEDS_FENCE;
@@ -875,7 +875,7 @@ static inline u64
 relocation_target(const struct drm_i915_gem_relocation_entry *reloc,
 		  const struct i915_vma *target)
 {
-	return gen8_canonical_addr((int)reloc->delta + target->node.start);
+	return (int)reloc->delta + target->node.start;
 }
 
 static void reloc_cache_init(struct reloc_cache *cache,
@@ -1267,7 +1267,7 @@ relocate_entry(struct i915_vma *vma,
 		if (IS_ERR(batch))
 			goto repeat;
 
-		addr = gen8_canonical_addr(vma->node.start + offset);
+		addr = vma->node.start + offset;
 		if (wide) {
 			if (offset & 7) {
 				*batch++ = MI_STORE_DWORD_IMM_GEN4;
@@ -1275,7 +1275,7 @@ relocate_entry(struct i915_vma *vma,
 				*batch++ = upper_32_bits(addr);
 				*batch++ = lower_32_bits(target_offset);
 
-				addr = gen8_canonical_addr(addr + 4);
+				addr += 4;
 
 				*batch++ = MI_STORE_DWORD_IMM_GEN4;
 				*batch++ = lower_32_bits(addr);
@@ -1388,7 +1388,7 @@ eb_relocate_entry(struct i915_execbuffer *eb,
 	 * more work needs to be done.
 	 */
 	if (!DBG_FORCE_RELOC &&
-	    gen8_canonical_addr(target->node.start) == reloc->presumed_offset)
+	    target->node.start == gen8_noncanonical_addr(eb->i915, reloc->presumed_offset))
 		return 0;
 
 	/* Check that the relocation address is valid... */
@@ -1496,7 +1496,7 @@ static int eb_relocate_vma(struct i915_execbuffer *eb, struct i915_vma *vma)
 				 * having already demonstrated that we
 				 * can read from this userspace address.
 				 */
-				offset = gen8_canonical_addr(offset & ~UPDATE);
+				offset &= ~UPDATE;
 				if (unlikely(__put_user(offset, &urelocs[r-stack].presumed_offset))) {
 					remain = -EFAULT;
 					goto out;
@@ -2919,8 +2919,6 @@ i915_gem_execbuffer_ioctl(struct drm_device *dev, void *data,
 			if (!(exec2_list[i].offset & UPDATE))
 				continue;
 
-			exec2_list[i].offset =
-				gen8_canonical_addr(exec2_list[i].offset & PIN_OFFSET_MASK);
 			exec2_list[i].offset &= PIN_OFFSET_MASK;
 			if (__copy_to_user(&user_exec_list[i].offset,
 					   &exec2_list[i].offset,
@@ -3006,8 +3004,7 @@ i915_gem_execbuffer2_ioctl(struct drm_device *dev, void *data,
 			if (!(exec2_list[i].offset & UPDATE))
 				continue;
 
-			exec2_list[i].offset =
-				gen8_canonical_addr(exec2_list[i].offset & PIN_OFFSET_MASK);
+			exec2_list[i].offset &= PIN_OFFSET_MASK;
 			unsafe_put_user(exec2_list[i].offset,
 					&user_exec_list[i].offset,
 					end_user);
diff --git a/drivers/gpu/drm/i915/gt/intel_gpu_commands.h b/drivers/gpu/drm/i915/gt/intel_gpu_commands.h
index 51b8718513bc..d5aaa599e2a4 100644
--- a/drivers/gpu/drm/i915/gt/intel_gpu_commands.h
+++ b/drivers/gpu/drm/i915/gt/intel_gpu_commands.h
@@ -321,25 +321,6 @@
 #define COLOR_BLT     ((0x2<<29)|(0x40<<22))
 #define SRC_COPY_BLT  ((0x2<<29)|(0x43<<22))
 
-/*
- * Used to convert any address to canonical form.
- * Starting from gen8, some commands (e.g. STATE_BASE_ADDRESS,
- * MI_LOAD_REGISTER_MEM and others, see Broadwell PRM Vol2a) require the
- * addresses to be in a canonical form:
- * "GraphicsAddress[63:48] are ignored by the HW and assumed to be in correct
- * canonical form [63:48] == [47]."
- */
-#define GEN8_HIGH_ADDRESS_BIT 47
-static inline u64 gen8_canonical_addr(u64 address)
-{
-	return sign_extend64(address, GEN8_HIGH_ADDRESS_BIT);
-}
-
-static inline u64 gen8_noncanonical_addr(u64 address)
-{
-	return address & GENMASK_ULL(GEN8_HIGH_ADDRESS_BIT, 0);
-}
-
 static inline u32 *__gen6_emit_bb_start(u32 *cs, u32 addr, unsigned int flags)
 {
 	*cs++ = MI_BATCH_BUFFER_START | flags;
diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c
index 189b573d02be..27bd696a5ed9 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -1303,7 +1303,8 @@ static bool check_cmd(const struct intel_engine_cs *engine,
 	return true;
 }
 
-static int check_bbstart(u32 *cmd, u32 offset, u32 length,
+static int check_bbstart(const struct intel_engine_cs *engine,
+			 u32 *cmd, u32 offset, u32 length,
 			 u32 batch_length,
 			 u64 batch_addr,
 			 u64 shadow_addr,
@@ -1325,6 +1326,7 @@ static int check_bbstart(u32 *cmd, u32 offset, u32 length,
 	}
 
 	jump_target = *(u64 *)(cmd + 1);
+	jump_target = gen8_noncanonical_addr(engine->i915, jump_target);
 	jump_offset = jump_target - batch_addr;
 
 	/*
@@ -1433,8 +1435,8 @@ int intel_engine_cmd_parser(struct intel_engine_cs *engine,
 		/* Defer failure until attempted use */
 		jump_whitelist = alloc_whitelist(batch_length);
 
-	shadow_addr = gen8_canonical_addr(shadow->node.start);
-	batch_addr = gen8_canonical_addr(batch->node.start + batch_offset);
+	shadow_addr = shadow->node.start;
+	batch_addr = batch->node.start + batch_offset;
 
 	/*
 	 * We use the batch length as size because the shadow object is as
@@ -1475,7 +1477,8 @@ int intel_engine_cmd_parser(struct intel_engine_cs *engine,
 		}
 
 		if (desc->cmd.value == MI_BATCH_BUFFER_START) {
-			ret = check_bbstart(cmd, offset, length, batch_length,
+			ret = check_bbstart(engine,
+					    cmd, offset, length, batch_length,
 					    batch_addr, shadow_addr,
 					    jump_whitelist);
 			break;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a71ff233cc55..46da9996d071 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2020,4 +2020,19 @@ static inline bool intel_guc_submission_is_enabled(struct intel_guc *guc)
 	       intel_guc_is_ready(guc);
 }
 
+/*
+ * Used to convert any address to canonical form.
+ * Starting from gen8, some commands (e.g. STATE_BASE_ADDRESS,
+ * MI_LOAD_REGISTER_MEM and others, see Broadwell PRM Vol2a) require the
+ * addresses to be in a canonical form:
+ * "GraphicsAddress[63:48] are ignored by the HW and assumed to be in correct
+ * canonical form [63:48] == [47]."
+ */
+
+static inline u64
+gen8_noncanonical_addr(const struct drm_i915_private *i915, u64 address)
+{
+	return address & GENMASK_ULL(INTEL_INFO(i915)->ppgtt_size - 1, 0);
+}
+
 #endif
-- 
2.25.0



More information about the Intel-gfx-trybot mailing list