[igt-dev] [PATCH i-g-t v6 06/14] lib/intel_batchbuffer: use canonical addresses for 48bit ppgtt

["Zbigniew Kempczyński" <zbigniew.kempczynski@intel.com>]

For all EXEC_OBJECT_PINNED objects we need to be sure address
passed must be in canonical form.

Until IGT allocator will be written just limit 48 and 47 bit
gtt tables to 46 bit only. We don't want to play with canonical
addresses with 47-bit set to 1 (and then 63:48).

Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski@intel.com>
Cc: Dominik Grzegorzek <dominik.grzegorzek@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 lib/intel_batchbuffer.c   | 37 +++++++++++++++++++++++++++-----
 tests/i915/api_intel_bb.c | 44 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 76 insertions(+), 5 deletions(-)

diff --git a/lib/intel_batchbuffer.c b/lib/intel_batchbuffer.c
index bf84d5f0..33e5f123 100644
--- a/lib/intel_batchbuffer.c
+++ b/lib/intel_batchbuffer.c
@@ -1204,6 +1204,20 @@ static void __reallocate_objects(struct intel_bb *ibb)
 	}
 }
 
+/*
+ * gen8_canonical_addr
+ * Used to convert any address into canonical form, i.e. [63:48] == [47].
+ * Based on kernel's sign_extend64 implementation.
+ * @address - a virtual address
+ */
+#define GEN8_HIGH_ADDRESS_BIT 47
+static uint64_t gen8_canonical_addr(uint64_t address)
+{
+	__u8 shift = 63 - GEN8_HIGH_ADDRESS_BIT;
+
+	return (__s64)(address << shift) >> shift;
+}
+
 static inline uint64_t __intel_bb_propose_offset(struct intel_bb *ibb)
 {
 	uint64_t offset;
@@ -1216,6 +1230,7 @@ static inline uint64_t __intel_bb_propose_offset(struct intel_bb *ibb)
 	offset += 256 << 10; /* Keep the low 256k clear, for negative deltas */
 	offset &= ibb->gtt_size - 1;
 	offset &= ~(ibb->alignment - 1);
+	offset = gen8_canonical_addr(offset);
 
 	return offset;
 }
@@ -1253,9 +1268,21 @@ __intel_bb_create(int i915, uint32_t size, bool do_relocs)
 	gtt_size = gem_aperture_size(i915);
 	if (!gem_uses_full_ppgtt(i915))
 		gtt_size /= 2;
-	if ((gtt_size - 1) >> 32)
+	if ((gtt_size - 1) >> 32) {
 		ibb->supports_48b_address = true;
 
+		/*
+		 * Until we develop IGT address allocator we workaround
+		 * playing with canonical addresses with 47-bit set to 1
+		 * just by limiting gtt size to 46-bit when gtt is 47 or 48
+		 * bit size. Current interface doesn't pass bo size, so
+		 * limiting to 46 bit make us sure we won't enter to
+		 * addresses with 47-bit set (we use 32-bit size now so
+		 * still we fit 47-bit address space).
+		 */
+		if (gtt_size & (3ull << 47))
+			gtt_size = (1ull << 46);
+	}
 	ibb->gtt_size = gtt_size;
 
 	__reallocate_objects(ibb);
@@ -1499,7 +1526,11 @@ intel_bb_add_object(struct intel_bb *ibb, uint32_t handle,
 	i = ibb->num_objects;
 	object = &ibb->objects[i];
 	object->handle = handle;
+
+	/* Limit current offset to gtt size */
 	object->offset = offset;
+	if (offset != INTEL_BUF_INVALID_ADDRESS)
+		object->offset = gen8_canonical_addr(offset & (ibb->gtt_size - 1));
 	object->alignment = ibb->alignment;
 
 	found = tsearch((void *) object, &ibb->root, __compare_objects);
@@ -1809,8 +1840,6 @@ static void intel_bb_dump_execbuf(struct intel_bb *ibb,
 			    from_user_pointer(execbuf->buffers_ptr))[i];
 		relocs = from_user_pointer(objects->relocs_ptr);
 		address = objects->offset;
-		if (address != INTEL_BUF_INVALID_ADDRESS)
-			address = address & (ibb->gtt_size - 1);
 		igt_info(" [%d] handle: %u, reloc_count: %d, reloc_ptr: %p, "
 			 "align: 0x%llx, offset: 0x%" PRIx64 ", flags: 0x%llx, "
 			 "rsvd1: 0x%llx, rsvd2: 0x%llx\n",
@@ -1825,8 +1854,6 @@ static void intel_bb_dump_execbuf(struct intel_bb *ibb,
 			for (j = 0; j < objects->relocation_count; j++) {
 				reloc = &relocs[j];
 				address = reloc->presumed_offset;
-				if (address != INTEL_BUF_INVALID_ADDRESS)
-					address = address & (ibb->gtt_size - 1);
 				igt_info("\t [%d] target handle: %u, "
 					 "offset: 0x%llx, delta: 0x%x, "
 					 "presumed_offset: 0x%" PRIx64 ", "
diff --git a/tests/i915/api_intel_bb.c b/tests/i915/api_intel_bb.c
index df0eb933..ca4625d5 100644
--- a/tests/i915/api_intel_bb.c
+++ b/tests/i915/api_intel_bb.c
@@ -165,6 +165,47 @@ static void simple_bb(struct buf_ops *bops, bool use_context)
 		gem_context_destroy(i915, ctx);
 }
 
+/*
+ * Make sure intel-bb space allocator currently doesn't enter 47-48 bit
+ * gtt sizes.
+ */
+static void check_canonical(struct buf_ops *bops)
+{
+	int i915 = buf_ops_get_fd(bops);
+	struct intel_bb *ibb;
+	struct intel_buf *buf;
+	uint32_t offset;
+	uint64_t address;
+	bool supports_48bit;
+
+	ibb = intel_bb_create(i915, PAGE_SIZE);
+	supports_48bit = ibb->supports_48b_address;
+	if (!supports_48bit)
+		intel_bb_destroy(ibb);
+	igt_require_f(supports_48bit, "We need 48bit ppgtt for testing\n");
+
+	address = 0xc00000000000;
+	if (debug_bb)
+		intel_bb_set_debug(ibb, true);
+
+	offset = intel_bb_emit_bbe(ibb);
+
+	buf = intel_buf_create(bops, 512, 512, 32, 0,
+			       I915_TILING_NONE, I915_COMPRESSION_NONE);
+
+	buf->addr.offset = address;
+	intel_bb_add_intel_buf(ibb, buf, true);
+	intel_bb_object_set_flag(ibb, buf->handle, EXEC_OBJECT_PINNED);
+
+	igt_assert(buf->addr.offset == 0);
+
+	intel_bb_exec(ibb, offset,
+		      I915_EXEC_DEFAULT | I915_EXEC_NO_RELOC, true);
+
+	intel_buf_destroy(buf);
+	intel_bb_destroy(ibb);
+}
+
 #define MI_FLUSH_DW (0x26<<23)
 #define BCS_SWCTRL  0x22200
 #define BCS_SRC_Y   (1 << 0)
@@ -829,6 +870,9 @@ igt_main_args("dpi", NULL, help_str, opt_handler, NULL)
 	igt_subtest("simple-bb-ctx")
 		simple_bb(bops, true);
 
+	igt_subtest("check-canonical")
+		check_canonical(bops);
+
 	igt_subtest("blit-noreloc-keep-cache")
 		blit(bops, NORELOC, KEEP_CACHE);
 
-- 
2.26.0

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev