[Intel-gfx] [PATCH 1/8] drm/i915: Fix cmdparser drm.debug

[Intel-gfx] [PATCH 1/8] drm/i915: Fix cmdparser drm.debug

[Chris Wilson]

The cmdparser rejection debug is not for driver development, but for the
user, for which we use a plain DRM_DEBUG().

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_cmd_parser.c | 55 +++++++++++++-------------
 1 file changed, 27 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c
index 2ed497e7c9fd..7b7061973c5e 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -731,7 +731,7 @@ static u32 gen7_render_get_cmd_length_mask(u32 cmd_header)
 			return 0xFF;
 	}
 
-	DRM_DEBUG_DRIVER("CMD: Abnormal rcs cmd length! 0x%08X\n", cmd_header);
+	DRM_DEBUG("CMD: Abnormal rcs cmd length! 0x%08X\n", cmd_header);
 	return 0;
 }
 
@@ -754,7 +754,7 @@ static u32 gen7_bsd_get_cmd_length_mask(u32 cmd_header)
 			return 0xFF;
 	}
 
-	DRM_DEBUG_DRIVER("CMD: Abnormal bsd cmd length! 0x%08X\n", cmd_header);
+	DRM_DEBUG("CMD: Abnormal bsd cmd length! 0x%08X\n", cmd_header);
 	return 0;
 }
 
@@ -767,7 +767,7 @@ static u32 gen7_blt_get_cmd_length_mask(u32 cmd_header)
 	else if (client == INSTR_BC_CLIENT)
 		return 0xFF;
 
-	DRM_DEBUG_DRIVER("CMD: Abnormal blt cmd length! 0x%08X\n", cmd_header);
+	DRM_DEBUG("CMD: Abnormal blt cmd length! 0x%08X\n", cmd_header);
 	return 0;
 }
 
@@ -778,7 +778,7 @@ static u32 gen9_blt_get_cmd_length_mask(u32 cmd_header)
 	if (client == INSTR_MI_CLIENT || client == INSTR_BC_CLIENT)
 		return 0xFF;
 
-	DRM_DEBUG_DRIVER("CMD: Abnormal blt cmd length! 0x%08X\n", cmd_header);
+	DRM_DEBUG("CMD: Abnormal blt cmd length! 0x%08X\n", cmd_header);
 	return 0;
 }
 
@@ -1211,7 +1211,7 @@ static bool check_cmd(const struct intel_engine_cs *engine,
 		return true;
 
 	if (desc->flags & CMD_DESC_REJECT) {
-		DRM_DEBUG_DRIVER("CMD: Rejected command: 0x%08X\n", *cmd);
+		DRM_DEBUG("CMD: Rejected command: 0x%08X\n", *cmd);
 		return false;
 	}
 
@@ -1231,8 +1231,8 @@ static bool check_cmd(const struct intel_engine_cs *engine,
 				find_reg(engine, reg_addr);
 
 			if (!reg) {
-				DRM_DEBUG_DRIVER("CMD: Rejected register 0x%08X in command: 0x%08X (%s)\n",
-						 reg_addr, *cmd, engine->name);
+				DRM_DEBUG("CMD: Rejected register 0x%08X in command: 0x%08X (%s)\n",
+					  reg_addr, *cmd, engine->name);
 				return false;
 			}
 
@@ -1242,22 +1242,22 @@ static bool check_cmd(const struct intel_engine_cs *engine,
 			 */
 			if (reg->mask) {
 				if (desc->cmd.value == MI_LOAD_REGISTER_MEM) {
-					DRM_DEBUG_DRIVER("CMD: Rejected LRM to masked register 0x%08X\n",
-							 reg_addr);
+					DRM_DEBUG("CMD: Rejected LRM to masked register 0x%08X\n",
+						  reg_addr);
 					return false;
 				}
 
 				if (desc->cmd.value == MI_LOAD_REGISTER_REG) {
-					DRM_DEBUG_DRIVER("CMD: Rejected LRR to masked register 0x%08X\n",
-							 reg_addr);
+					DRM_DEBUG("CMD: Rejected LRR to masked register 0x%08X\n",
+						  reg_addr);
 					return false;
 				}
 
 				if (desc->cmd.value == MI_LOAD_REGISTER_IMM(1) &&
 				    (offset + 2 > length ||
 				     (cmd[offset + 1] & reg->mask) != reg->value)) {
-					DRM_DEBUG_DRIVER("CMD: Rejected LRI to masked register 0x%08X\n",
-							 reg_addr);
+					DRM_DEBUG("CMD: Rejected LRI to masked register 0x%08X\n",
+						  reg_addr);
 					return false;
 				}
 			}
@@ -1284,8 +1284,8 @@ static bool check_cmd(const struct intel_engine_cs *engine,
 			}
 
 			if (desc->bits[i].offset >= length) {
-				DRM_DEBUG_DRIVER("CMD: Rejected command 0x%08X, too short to check bitmask (%s)\n",
-						 *cmd, engine->name);
+				DRM_DEBUG("CMD: Rejected command 0x%08X, too short to check bitmask (%s)\n",
+					  *cmd, engine->name);
 				return false;
 			}
 
@@ -1293,11 +1293,11 @@ static bool check_cmd(const struct intel_engine_cs *engine,
 				desc->bits[i].mask;
 
 			if (dword != desc->bits[i].expected) {
-				DRM_DEBUG_DRIVER("CMD: Rejected command 0x%08X for bitmask 0x%08X (exp=0x%08X act=0x%08X) (%s)\n",
-						 *cmd,
-						 desc->bits[i].mask,
-						 desc->bits[i].expected,
-						 dword, engine->name);
+				DRM_DEBUG("CMD: Rejected command 0x%08X for bitmask 0x%08X (exp=0x%08X act=0x%08X) (%s)\n",
+					  *cmd,
+					  desc->bits[i].mask,
+					  desc->bits[i].expected,
+					  dword, engine->name);
 				return false;
 			}
 		}
@@ -1425,7 +1425,7 @@ int intel_engine_cmd_parser(struct intel_engine_cs *engine,
 			 batch_start_offset, batch_len,
 			 &needs_clflush_after);
 	if (IS_ERR(cmd)) {
-		DRM_DEBUG_DRIVER("CMD: Failed to copy batch\n");
+		DRM_DEBUG("CMD: Failed to copy batch\n");
 		return PTR_ERR(cmd);
 	}
 
@@ -1446,8 +1446,7 @@ int intel_engine_cmd_parser(struct intel_engine_cs *engine,
 
 		desc = find_cmd(engine, *cmd, desc, &default_desc);
 		if (!desc) {
-			DRM_DEBUG_DRIVER("CMD: Unrecognized command: 0x%08X\n",
-					 *cmd);
+			DRM_DEBUG("CMD: Unrecognized command: 0x%08X\n", *cmd);
 			ret = -EINVAL;
 			goto err;
 		}
@@ -1458,10 +1457,10 @@ int intel_engine_cmd_parser(struct intel_engine_cs *engine,
 			length = ((*cmd & desc->length.mask) + LENGTH_BIAS);
 
 		if ((batch_end - cmd) < length) {
-			DRM_DEBUG_DRIVER("CMD: Command length exceeds batch length: 0x%08X length=%u batchlen=%td\n",
-					 *cmd,
-					 length,
-					 batch_end - cmd);
+			DRM_DEBUG("CMD: Command length exceeds batch length: 0x%08X length=%u batchlen=%td\n",
+				  *cmd,
+				  length,
+				  batch_end - cmd);
 			ret = -EINVAL;
 			goto err;
 		}
@@ -1488,7 +1487,7 @@ int intel_engine_cmd_parser(struct intel_engine_cs *engine,
 		cmd += length;
 		offset += length;
 		if  (cmd >= batch_end) {
-			DRM_DEBUG_DRIVER("CMD: Got to the end of the buffer w/o a BBE cmd!\n");
+			DRM_DEBUG("CMD: Got to the end of the buffer w/o a BBE cmd!\n");
 			ret = -EINVAL;
 			goto err;
 		}
-- 
2.24.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 2/8] drm/i915: Remove redundant parameters from intel_engine_cmd_parser

[Chris Wilson]

Declutter the calling interface by reducing the parameters to the
i915_vma and associated offsets.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 .../gpu/drm/i915/gem/i915_gem_execbuffer.c    |  33 +-----
 drivers/gpu/drm/i915/gt/intel_gpu_commands.h  |  21 ++++
 drivers/gpu/drm/i915/i915_cmd_parser.c        | 104 +++++++++---------
 drivers/gpu/drm/i915/i915_drv.h               |  10 +-
 4 files changed, 81 insertions(+), 87 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 34044c6203a5..acf71466f8ea 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -276,25 +276,6 @@ struct i915_execbuffer {
 
 #define exec_entry(EB, VMA) (&(EB)->exec[(VMA)->exec_flags - (EB)->flags])
 
-/*
- * 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 bool eb_use_cmdparser(const struct i915_execbuffer *eb)
 {
 	return intel_engine_requires_cmd_parser(eb->engine) ||
@@ -2005,8 +1986,6 @@ static struct i915_vma *eb_parse(struct i915_execbuffer *eb)
 {
 	struct intel_engine_pool_node *pool;
 	struct i915_vma *vma;
-	u64 batch_start;
-	u64 shadow_batch_start;
 	int err;
 
 	pool = intel_engine_get_pool(eb->engine, eb->batch_len);
@@ -2017,19 +1996,11 @@ static struct i915_vma *eb_parse(struct i915_execbuffer *eb)
 	if (IS_ERR(vma))
 		goto err;
 
-	batch_start = gen8_canonical_addr(eb->batch->node.start) +
-		      eb->batch_start_offset;
-
-	shadow_batch_start = gen8_canonical_addr(vma->node.start);
-
 	err = intel_engine_cmd_parser(eb->engine,
-				      eb->batch->obj,
-				      batch_start,
+				      eb->batch,
 				      eb->batch_start_offset,
 				      eb->batch_len,
-				      pool->obj,
-				      shadow_batch_start);
-
+				      vma);
 	if (err) {
 		i915_vma_unpin(vma);
 
diff --git a/drivers/gpu/drm/i915/gt/intel_gpu_commands.h b/drivers/gpu/drm/i915/gt/intel_gpu_commands.h
index 4294f146f13c..c68c0e033f30 100644
--- a/drivers/gpu/drm/i915/gt/intel_gpu_commands.h
+++ b/drivers/gpu/drm/i915/gt/intel_gpu_commands.h
@@ -7,6 +7,8 @@
 #ifndef _INTEL_GPU_COMMANDS_H_
 #define _INTEL_GPU_COMMANDS_H_
 
+#include <linux/bitops.h>
+
 /*
  * Target address alignments required for GPU access e.g.
  * MI_STORE_DWORD_IMM.
@@ -319,4 +321,23 @@
 #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);
+}
+
 #endif /* _INTEL_GPU_COMMANDS_H_ */
diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c
index 7b7061973c5e..74fd0ea05f02 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -1127,8 +1127,7 @@ find_reg(const struct intel_engine_cs *engine, u32 addr)
 /* Returns a vmap'd pointer to dst_obj, which the caller must unmap */
 static u32 *copy_batch(struct drm_i915_gem_object *dst_obj,
 		       struct drm_i915_gem_object *src_obj,
-		       u32 batch_start_offset,
-		       u32 batch_len,
+		       u32 offset, u32 length,
 		       bool *needs_clflush_after)
 {
 	unsigned int src_needs_clflush;
@@ -1153,22 +1152,21 @@ static u32 *copy_batch(struct drm_i915_gem_object *dst_obj,
 
 	src = ERR_PTR(-ENODEV);
 	if (src_needs_clflush &&
-	    i915_can_memcpy_from_wc(NULL, batch_start_offset, 0)) {
+	    i915_can_memcpy_from_wc(NULL, offset, 0)) {
 		src = i915_gem_object_pin_map(src_obj, I915_MAP_WC);
 		if (!IS_ERR(src)) {
 			i915_memcpy_from_wc(dst,
-					    src + batch_start_offset,
-					    ALIGN(batch_len, 16));
+					    src + offset,
+					    ALIGN(length, 16));
 			i915_gem_object_unpin_map(src_obj);
 		}
 	}
 	if (IS_ERR(src)) {
 		void *ptr;
-		int offset, n;
+		int x, n;
 
-		offset = offset_in_page(batch_start_offset);
-
-		/* We can avoid clflushing partial cachelines before the write
+		/*
+		 * We can avoid clflushing partial cachelines before the write
 		 * if we only every write full cache-lines. Since we know that
 		 * both the source and destination are in multiples of
 		 * PAGE_SIZE, we can simply round up to the next cacheline.
@@ -1176,22 +1174,23 @@ static u32 *copy_batch(struct drm_i915_gem_object *dst_obj,
 		 * validate up to the end of the batch.
 		 */
 		if (dst_needs_clflush & CLFLUSH_BEFORE)
-			batch_len = roundup(batch_len,
-					    boot_cpu_data.x86_clflush_size);
+			length = round_up(length,
+					  boot_cpu_data.x86_clflush_size);
 
 		ptr = dst;
-		for (n = batch_start_offset >> PAGE_SHIFT; batch_len; n++) {
-			int len = min_t(int, batch_len, PAGE_SIZE - offset);
+		x = offset_in_page(offset);
+		for (n = offset >> PAGE_SHIFT; length; n++) {
+			int len = min_t(int, length, PAGE_SIZE - x);
 
 			src = kmap_atomic(i915_gem_object_get_page(src_obj, n));
 			if (src_needs_clflush)
-				drm_clflush_virt_range(src + offset, len);
-			memcpy(ptr, src + offset, len);
+				drm_clflush_virt_range(src + x, len);
+			memcpy(ptr, src + x, len);
 			kunmap_atomic(src);
 
 			ptr += len;
-			batch_len -= len;
-			offset = 0;
+			length -= len;
+			x = 0;
 		}
 	}
 
@@ -1307,9 +1306,9 @@ static bool check_cmd(const struct intel_engine_cs *engine,
 }
 
 static int check_bbstart(u32 *cmd, u32 offset, u32 length,
-			 u32 batch_len,
-			 u64 batch_start,
-			 u64 shadow_batch_start,
+			 u32 batch_length,
+			 u64 batch_addr,
+			 u64 shadow_addr,
 			 const unsigned long *jump_whitelist)
 {
 	u64 jump_offset, jump_target;
@@ -1328,13 +1327,13 @@ static int check_bbstart(u32 *cmd, u32 offset, u32 length,
 	}
 
 	jump_target = *(u64*)(cmd+1);
-	jump_offset = jump_target - batch_start;
+	jump_offset = jump_target - batch_addr;
 
 	/*
 	 * Any underflow of jump_target is guaranteed to be outside the range
 	 * of a u32, so >= test catches both too large and too small
 	 */
-	if (jump_offset >= batch_len) {
+	if (jump_offset >= batch_length) {
 		DRM_DEBUG("CMD: BB_START to 0x%llx jumps out of BB\n",
 			  jump_target);
 		return -EINVAL;
@@ -1342,12 +1341,12 @@ static int check_bbstart(u32 *cmd, u32 offset, u32 length,
 
 	/*
 	 * This cannot overflow a u32 because we already checked jump_offset
-	 * is within the BB, and the batch_len is a u32
+	 * is within the BB, and the batch_length is a u32
 	 */
 	target_cmd_offset = lower_32_bits(jump_offset);
 	target_cmd_index = target_cmd_offset / sizeof(u32);
 
-	*(u64*)(cmd + 1) = shadow_batch_start + target_cmd_offset;
+	*(u64*)(cmd + 1) = shadow_addr + target_cmd_offset;
 
 	if (target_cmd_index == offset)
 		return 0;
@@ -1365,12 +1364,12 @@ static int check_bbstart(u32 *cmd, u32 offset, u32 length,
 }
 
 static unsigned long *
-alloc_whitelist(struct drm_i915_private *i915, u32 batch_len)
+alloc_whitelist(struct drm_i915_private *i915, u32 batch_length)
 {
 	unsigned long *jmp;
 
 	/*
-	 * We expect batch_len to be less than 256KiB for known users,
+	 * We expect batch_length to be less than 256KiB for known users,
 	 * i.e. we need at most an 8KiB bitmap allocation which should be
 	 * reasonably cheap due to kmalloc caches.
 	 */
@@ -1379,7 +1378,7 @@ alloc_whitelist(struct drm_i915_private *i915, u32 batch_len)
 		return NULL;
 
 	/* Prefer to report transient allocation failure rather than hit oom */
-	jmp = bitmap_zalloc(DIV_ROUND_UP(batch_len, sizeof(u32)),
+	jmp = bitmap_zalloc(DIV_ROUND_UP(batch_length, sizeof(u32)),
 			    GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN);
 	if (!jmp)
 		return ERR_PTR(-ENOMEM);
@@ -1390,14 +1389,12 @@ alloc_whitelist(struct drm_i915_private *i915, u32 batch_len)
 #define LENGTH_BIAS 2
 
 /**
- * i915_parse_cmds() - parse a submitted batch buffer for privilege violations
+ * intel_engine_cmd_parser() - parse a submitted batch buffer for privilege violations
  * @engine: the engine on which the batch is to execute
- * @batch_obj: the batch buffer in question
- * @batch_start: Canonical base address of batch
- * @batch_start_offset: byte offset in the batch at which execution starts
- * @batch_len: length of the commands in batch_obj
- * @shadow_batch_obj: copy of the batch buffer in question
- * @shadow_batch_start: Canonical base address of shadow_batch_obj
+ * @batch: the batch buffer in question
+ * @batch_offset: byte offset in the batch at which execution starts
+ * @batch_length: length of the commands in batch_obj
+ * @shadow: validated copy of the batch buffer in question
  *
  * Parses the specified batch buffer looking for privilege violations as
  * described in the overview.
@@ -1407,22 +1404,27 @@ alloc_whitelist(struct drm_i915_private *i915, u32 batch_len)
  */
 
 int intel_engine_cmd_parser(struct intel_engine_cs *engine,
-			    struct drm_i915_gem_object *batch_obj,
-			    u64 batch_start,
-			    u32 batch_start_offset,
-			    u32 batch_len,
-			    struct drm_i915_gem_object *shadow_batch_obj,
-			    u64 shadow_batch_start)
+			    struct i915_vma *batch,
+			    u32 batch_offset,
+			    u32 batch_length,
+			    struct i915_vma *shadow)
 {
 	u32 *cmd, *batch_end, offset = 0;
 	struct drm_i915_cmd_descriptor default_desc = noop_desc;
 	const struct drm_i915_cmd_descriptor *desc = &default_desc;
 	bool needs_clflush_after = false;
 	unsigned long *jump_whitelist;
+	u64 batch_addr, shadow_addr;
 	int ret = 0;
 
-	cmd = copy_batch(shadow_batch_obj, batch_obj,
-			 batch_start_offset, batch_len,
+	GEM_BUG_ON(!IS_ALIGNED(batch_offset, sizeof(*cmd)));
+	GEM_BUG_ON(!IS_ALIGNED(batch_length, sizeof(*cmd)));
+	GEM_BUG_ON(range_overflows_t(u64, batch_offset, batch_length,
+				     batch->size));
+	GEM_BUG_ON(!batch_length);
+
+	cmd = copy_batch(shadow->obj, batch->obj,
+			 batch_offset, batch_length,
 			 &needs_clflush_after);
 	if (IS_ERR(cmd)) {
 		DRM_DEBUG("CMD: Failed to copy batch\n");
@@ -1430,14 +1432,17 @@ int intel_engine_cmd_parser(struct intel_engine_cs *engine,
 	}
 
 	/* Defer failure until attempted use */
-	jump_whitelist = alloc_whitelist(engine->i915, batch_len);
+	jump_whitelist = alloc_whitelist(engine->i915, batch_length);
+
+	shadow_addr = gen8_canonical_addr(shadow->node.start);
+	batch_addr = gen8_canonical_addr(batch->node.start + batch_offset);
 
 	/*
 	 * We use the batch length as size because the shadow object is as
 	 * large or larger and copy_batch() will write MI_NOPs to the extra
 	 * space. Parsing should be faster in some cases this way.
 	 */
-	batch_end = cmd + (batch_len / sizeof(*batch_end));
+	batch_end = cmd + batch_length / sizeof(*batch_end);
 	do {
 		u32 length;
 
@@ -1454,7 +1459,7 @@ int intel_engine_cmd_parser(struct intel_engine_cs *engine,
 		if (desc->flags & CMD_DESC_FIXED)
 			length = desc->length.fixed;
 		else
-			length = ((*cmd & desc->length.mask) + LENGTH_BIAS);
+			length = (*cmd & desc->length.mask) + LENGTH_BIAS;
 
 		if ((batch_end - cmd) < length) {
 			DRM_DEBUG("CMD: Command length exceeds batch length: 0x%08X length=%u batchlen=%td\n",
@@ -1471,9 +1476,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_len, batch_start,
-					    shadow_batch_start,
+			ret = check_bbstart(cmd, offset, length, batch_length,
+					    batch_addr, shadow_addr,
 					    jump_whitelist);
 
 			if (ret)
@@ -1494,7 +1498,7 @@ int intel_engine_cmd_parser(struct intel_engine_cs *engine,
 	} while (1);
 
 	if (needs_clflush_after) {
-		void *ptr = page_mask_bits(shadow_batch_obj->mm.mapping);
+		void *ptr = page_mask_bits(shadow->obj->mm.mapping);
 
 		drm_clflush_virt_range(ptr, (void *)(cmd + 1) - ptr);
 	}
@@ -1502,7 +1506,7 @@ int intel_engine_cmd_parser(struct intel_engine_cs *engine,
 err:
 	if (!IS_ERR_OR_NULL(jump_whitelist))
 		kfree(jump_whitelist);
-	i915_gem_object_unpin_map(shadow_batch_obj);
+	i915_gem_object_unpin_map(shadow->obj);
 	return ret;
 }
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c3d8af28bfc1..9805bc434d26 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1948,12 +1948,10 @@ int i915_cmd_parser_get_version(struct drm_i915_private *dev_priv);
 void intel_engine_init_cmd_parser(struct intel_engine_cs *engine);
 void intel_engine_cleanup_cmd_parser(struct intel_engine_cs *engine);
 int intel_engine_cmd_parser(struct intel_engine_cs *engine,
-			    struct drm_i915_gem_object *batch_obj,
-			    u64 user_batch_start,
-			    u32 batch_start_offset,
-			    u32 batch_len,
-			    struct drm_i915_gem_object *shadow_batch_obj,
-			    u64 shadow_batch_start);
+			    struct i915_vma *batch,
+			    u32 batch_offset,
+			    u32 batch_length,
+			    struct i915_vma *shadow);
 
 /* intel_device_info.c */
 static inline struct intel_device_info *
-- 
2.24.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 3/8] drm/i915: Simplify error escape from cmdparser

[Chris Wilson]

We need to flush the destination buffer, even on error, to maintain
consistent cache state. Thereby removing the jump on error past the
clear, and reducing the loop-escape mechanism to a mere break.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_cmd_parser.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c
index 74fd0ea05f02..6cf4e336461b 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -1453,7 +1453,7 @@ int intel_engine_cmd_parser(struct intel_engine_cs *engine,
 		if (!desc) {
 			DRM_DEBUG("CMD: Unrecognized command: 0x%08X\n", *cmd);
 			ret = -EINVAL;
-			goto err;
+			break;
 		}
 
 		if (desc->flags & CMD_DESC_FIXED)
@@ -1467,21 +1467,18 @@ int intel_engine_cmd_parser(struct intel_engine_cs *engine,
 				  length,
 				  batch_end - cmd);
 			ret = -EINVAL;
-			goto err;
+			break;
 		}
 
 		if (!check_cmd(engine, desc, cmd, length)) {
 			ret = -EACCES;
-			goto err;
+			break;
 		}
 
 		if (desc->cmd.value == MI_BATCH_BUFFER_START) {
 			ret = check_bbstart(cmd, offset, length, batch_length,
 					    batch_addr, shadow_addr,
 					    jump_whitelist);
-
-			if (ret)
-				goto err;
 			break;
 		}
 
@@ -1493,7 +1490,7 @@ int intel_engine_cmd_parser(struct intel_engine_cs *engine,
 		if  (cmd >= batch_end) {
 			DRM_DEBUG("CMD: Got to the end of the buffer w/o a BBE cmd!\n");
 			ret = -EINVAL;
-			goto err;
+			break;
 		}
 	} while (1);
 
@@ -1503,7 +1500,6 @@ int intel_engine_cmd_parser(struct intel_engine_cs *engine,
 		drm_clflush_virt_range(ptr, (void *)(cmd + 1) - ptr);
 	}
 
-err:
 	if (!IS_ERR_OR_NULL(jump_whitelist))
 		kfree(jump_whitelist);
 	i915_gem_object_unpin_map(shadow->obj);
-- 
2.24.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 4/8] drm/i915/gem: Tidy up error handling for eb_parse()

[Chris Wilson]

As the caller no longer uses the i915_vma result, stop returning it and
just return the error code instead.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 .../gpu/drm/i915/gem/i915_gem_execbuffer.c    | 37 +++++++++----------
 1 file changed, 17 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index acf71466f8ea..690a3670ed08 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -1982,19 +1982,24 @@ shadow_batch_pin(struct i915_execbuffer *eb, struct drm_i915_gem_object *obj)
 	return vma;
 }
 
-static struct i915_vma *eb_parse(struct i915_execbuffer *eb)
+static int eb_parse(struct i915_execbuffer *eb)
 {
 	struct intel_engine_pool_node *pool;
 	struct i915_vma *vma;
 	int err;
 
+	if (!eb_use_cmdparser(eb))
+		return 0;
+
 	pool = intel_engine_get_pool(eb->engine, eb->batch_len);
 	if (IS_ERR(pool))
-		return ERR_CAST(pool);
+		return PTR_ERR(pool);
 
 	vma = shadow_batch_pin(eb, pool->obj);
-	if (IS_ERR(vma))
+	if (IS_ERR(vma)) {
+		err = PTR_ERR(vma);
 		goto err;
+	}
 
 	err = intel_engine_cmd_parser(eb->engine,
 				      eb->batch,
@@ -2002,8 +2007,6 @@ static struct i915_vma *eb_parse(struct i915_execbuffer *eb)
 				      eb->batch_len,
 				      vma);
 	if (err) {
-		i915_vma_unpin(vma);
-
 		/*
 		 * Unsafe GGTT-backed buffers can still be submitted safely
 		 * as non-secure.
@@ -2012,10 +2015,8 @@ static struct i915_vma *eb_parse(struct i915_execbuffer *eb)
 		 */
 		if (i915_vma_is_ggtt(vma) && err == -EACCES)
 			/* Execute original buffer non-secure */
-			vma = NULL;
-		else
-			vma = ERR_PTR(err);
-		goto err;
+			err = 0;
+		goto err_unpin;
 	}
 
 	eb->vma[eb->buffer_count] = i915_vma_get(vma);
@@ -2033,11 +2034,13 @@ static struct i915_vma *eb_parse(struct i915_execbuffer *eb)
 	/* eb->batch_len unchanged */
 
 	vma->private = pool;
-	return vma;
+	return 0;
 
+err_unpin:
+	i915_vma_unpin(vma);
 err:
 	intel_engine_pool_put(pool);
-	return vma;
+	return err;
 }
 
 static void
@@ -2558,15 +2561,9 @@ i915_gem_do_execbuffer(struct drm_device *dev,
 	if (eb.batch_len == 0)
 		eb.batch_len = eb.batch->size - eb.batch_start_offset;
 
-	if (eb_use_cmdparser(&eb)) {
-		struct i915_vma *vma;
-
-		vma = eb_parse(&eb);
-		if (IS_ERR(vma)) {
-			err = PTR_ERR(vma);
-			goto err_vma;
-		}
-	}
+	err = eb_parse(&eb);
+	if (err)
+		goto err_vma;
 
 	/*
 	 * snb/ivb/vlv conflate the "batch in ppgtt" bit with the "non-secure
-- 
2.24.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 5/8] drm/i915: Align start for memcpy_from_wc

[Chris Wilson]

The movntqda requires 16-byte alignment for the source pointer. Avoid
falling back to clflush if the source pointer is misaligned by doing the
doing a small uncached memcpy to fixup the alignments.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_cmd_parser.c | 30 +++++++++++++++++---------
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c
index 6cf4e336461b..2977316d64ae 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -1132,8 +1132,8 @@ static u32 *copy_batch(struct drm_i915_gem_object *dst_obj,
 {
 	unsigned int src_needs_clflush;
 	unsigned int dst_needs_clflush;
-	void *dst, *src;
-	int ret;
+	void *dst, *src, *ptr;
+	int ret, len;
 
 	ret = i915_gem_object_prepare_write(dst_obj, &dst_needs_clflush);
 	if (ret)
@@ -1150,19 +1150,30 @@ static u32 *copy_batch(struct drm_i915_gem_object *dst_obj,
 		return ERR_PTR(ret);
 	}
 
+	ptr = dst;
 	src = ERR_PTR(-ENODEV);
-	if (src_needs_clflush &&
-	    i915_can_memcpy_from_wc(NULL, offset, 0)) {
+	if (src_needs_clflush && i915_has_memcpy_from_wc()) {
 		src = i915_gem_object_pin_map(src_obj, I915_MAP_WC);
 		if (!IS_ERR(src)) {
-			i915_memcpy_from_wc(dst,
-					    src + offset,
-					    ALIGN(length, 16));
+			src += offset;
+
+			if (!IS_ALIGNED(offset, 16)) {
+				len = min(ALIGN(offset, 16) - offset, length);
+
+				memcpy(ptr, src, len);
+
+				offset += len;
+				length -= len;
+				ptr += len;
+				src += len;
+			}
+			GEM_BUG_ON(!IS_ALIGNED((unsigned long)src, 16));
+
+			i915_memcpy_from_wc(ptr, src, ALIGN(length, 16));
 			i915_gem_object_unpin_map(src_obj);
 		}
 	}
 	if (IS_ERR(src)) {
-		void *ptr;
 		int x, n;
 
 		/*
@@ -1177,10 +1188,9 @@ static u32 *copy_batch(struct drm_i915_gem_object *dst_obj,
 			length = round_up(length,
 					  boot_cpu_data.x86_clflush_size);
 
-		ptr = dst;
 		x = offset_in_page(offset);
 		for (n = offset >> PAGE_SHIFT; length; n++) {
-			int len = min_t(int, length, PAGE_SIZE - x);
+			len = min_t(int, length, PAGE_SIZE - x);
 
 			src = kmap_atomic(i915_gem_object_get_page(src_obj, n));
 			if (src_needs_clflush)
-- 
2.24.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 6/8] drm/i915: Prepare gen7 cmdparser for async execution

[Chris Wilson]

The gen7 cmdparser is primarily a promotion-based system to allow access
to additional registers beyond the HW validation, and allows fallback to
normal execution of the user batch buffer if valid and requires
chaining. In the next patch, we will do the cmdparser validation in the
pipeline asynchronously and so at the point of request construction we
will not know if we want to execute the privileged and validated batch,
or the original user batch. The solution employed here is to execute
both batches, one with raised privileges and one as normal. This is
because the gen7 MI_BATCH_BUFFER_START command cannot change privilege
level within a batch and must strictly use the current privilege level
(or undefined behaviour kills the GPU). So in order to execute the
original batch, we need a second non-priviledged batch buffer chain from
the ring, i.e. we need to emit two batches for each user batch. Inside
the two batches we determine which one should actually execute, we
provide a conditional trampoline to call the original batch.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 .../gpu/drm/i915/gem/i915_gem_execbuffer.c    | 127 ++++++++++--------
 drivers/gpu/drm/i915/i915_cmd_parser.c        |  27 ++++
 2 files changed, 100 insertions(+), 54 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 690a3670ed08..5f4e460701ca 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -47,6 +47,8 @@ enum {
 #define __EXEC_INTERNAL_FLAGS	(~0u << 30)
 #define UPDATE			PIN_OFFSET_FIXED
 
+#define NUM_EXTRA 2
+
 #define BATCH_OFFSET_BIAS (256*1024)
 
 #define __I915_EXEC_ILLEGAL_FLAGS \
@@ -228,6 +230,7 @@ struct i915_execbuffer {
 
 	struct i915_request *request; /** our request to build */
 	struct i915_vma *batch; /** identity of the batch obj/vma */
+	struct i915_vma *trampoline; /** trampoline used for chaining */
 
 	/** actual size of execobj[] as we may extend it for the cmdparser */
 	unsigned int buffer_count;
@@ -1946,31 +1949,13 @@ static int i915_reset_gen7_sol_offsets(struct i915_request *rq)
 }
 
 static struct i915_vma *
-shadow_batch_pin(struct i915_execbuffer *eb, struct drm_i915_gem_object *obj)
+shadow_batch_pin(struct drm_i915_gem_object *obj,
+		 struct i915_address_space *vm,
+		 unsigned int flags)
 {
-	struct i915_address_space *vm;
 	struct i915_vma *vma;
-	u64 flags;
 	int err;
 
-	/*
-	 * PPGTT backed shadow buffers must be mapped RO, to prevent
-	 * post-scan tampering
-	 */
-	if (CMDPARSER_USES_GGTT(eb->i915)) {
-		vm = &eb->engine->gt->ggtt->vm;
-		flags = PIN_GLOBAL;
-	} else {
-		vm = eb->context->vm;
-		if (!vm->has_read_only) {
-			DRM_DEBUG("Cannot prevent post-scan tampering without RO capable vm\n");
-			return ERR_PTR(-EINVAL);
-		}
-
-		i915_gem_object_set_readonly(obj);
-		flags = PIN_USER;
-	}
-
 	vma = i915_vma_instance(obj, vm, NULL);
 	if (IS_ERR(vma))
 		return vma;
@@ -1985,59 +1970,80 @@ shadow_batch_pin(struct i915_execbuffer *eb, struct drm_i915_gem_object *obj)
 static int eb_parse(struct i915_execbuffer *eb)
 {
 	struct intel_engine_pool_node *pool;
-	struct i915_vma *vma;
+	struct i915_vma *shadow, *trampoline;
+	unsigned int len;
 	int err;
 
 	if (!eb_use_cmdparser(eb))
 		return 0;
 
-	pool = intel_engine_get_pool(eb->engine, eb->batch_len);
+	len = eb->batch_len;
+	if (!CMDPARSER_USES_GGTT(eb->i915)) {
+		/*
+		 * PPGTT backed shadow buffers must be mapped RO, to prevent
+		 * post-scan tampering
+		 */
+		if (!eb->context->vm->has_read_only) {
+			DRM_DEBUG("Cannot prevent post-scan tampering without RO capable vm\n");
+			return -EINVAL;
+		}
+	} else {
+		len += 8;
+	}
+
+	pool = intel_engine_get_pool(eb->engine, len);
 	if (IS_ERR(pool))
 		return PTR_ERR(pool);
 
-	vma = shadow_batch_pin(eb, pool->obj);
-	if (IS_ERR(vma)) {
-		err = PTR_ERR(vma);
+	shadow = shadow_batch_pin(pool->obj, eb->context->vm, PIN_USER);
+	if (IS_ERR(shadow)) {
+		err = PTR_ERR(shadow);
 		goto err;
 	}
+	i915_gem_object_set_readonly(shadow->obj);
+
+	trampoline = NULL;
+	if (CMDPARSER_USES_GGTT(eb->i915)) {
+		trampoline = shadow;
+
+		shadow = shadow_batch_pin(pool->obj,
+					  &eb->engine->gt->ggtt->vm,
+					  PIN_GLOBAL);
+		if (IS_ERR(shadow)) {
+			err = PTR_ERR(shadow);
+			shadow = trampoline;
+			goto err_shadow;
+		}
+
+		eb->batch_flags |= I915_DISPATCH_SECURE;
+	}
 
 	err = intel_engine_cmd_parser(eb->engine,
 				      eb->batch,
 				      eb->batch_start_offset,
 				      eb->batch_len,
-				      vma);
-	if (err) {
-		/*
-		 * Unsafe GGTT-backed buffers can still be submitted safely
-		 * as non-secure.
-		 * For PPGTT backing however, we have no choice but to forcibly
-		 * reject unsafe buffers
-		 */
-		if (i915_vma_is_ggtt(vma) && err == -EACCES)
-			/* Execute original buffer non-secure */
-			err = 0;
-		goto err_unpin;
-	}
+				      shadow);
+	if (err)
+		goto err_trampoline;
 
-	eb->vma[eb->buffer_count] = i915_vma_get(vma);
+	eb->vma[eb->buffer_count] = i915_vma_get(shadow);
 	eb->flags[eb->buffer_count] =
 		__EXEC_OBJECT_HAS_PIN | __EXEC_OBJECT_HAS_REF;
-	vma->exec_flags = &eb->flags[eb->buffer_count];
+	shadow->exec_flags = &eb->flags[eb->buffer_count];
 	eb->buffer_count++;
 
+	eb->trampoline = trampoline;
 	eb->batch_start_offset = 0;
-	eb->batch = vma;
-
-	if (i915_vma_is_ggtt(vma))
-		eb->batch_flags |= I915_DISPATCH_SECURE;
-
-	/* eb->batch_len unchanged */
+	eb->batch = shadow;
 
-	vma->private = pool;
+	shadow->private = pool;
 	return 0;
 
-err_unpin:
-	i915_vma_unpin(vma);
+err_trampoline:
+	if (trampoline)
+		i915_vma_unpin(trampoline);
+err_shadow:
+	i915_vma_unpin(shadow);
 err:
 	intel_engine_pool_put(pool);
 	return err;
@@ -2089,6 +2095,16 @@ static int eb_submit(struct i915_execbuffer *eb)
 	if (err)
 		return err;
 
+	if (eb->trampoline) {
+		GEM_BUG_ON(eb->batch_start_offset);
+		err = eb->engine->emit_bb_start(eb->request,
+						eb->trampoline->node.start +
+						eb->batch_len,
+						8, 0);
+		if (err)
+			return err;
+	}
+
 	if (i915_gem_context_nopreempt(eb->gem_context))
 		eb->request->flags |= I915_REQUEST_NOPREEMPT;
 
@@ -2460,9 +2476,9 @@ i915_gem_do_execbuffer(struct drm_device *dev,
 		args->flags |= __EXEC_HAS_RELOC;
 
 	eb.exec = exec;
-	eb.vma = (struct i915_vma **)(exec + args->buffer_count + 1);
+	eb.vma = (struct i915_vma **)(exec + args->buffer_count + NUM_EXTRA);
 	eb.vma[0] = NULL;
-	eb.flags = (unsigned int *)(eb.vma + args->buffer_count + 1);
+	eb.flags = (unsigned int *)(eb.vma + args->buffer_count + NUM_EXTRA);
 
 	eb.invalid_flags = __EXEC_OBJECT_UNKNOWN_FLAGS;
 	reloc_cache_init(&eb.reloc_cache, eb.i915);
@@ -2470,6 +2486,7 @@ i915_gem_do_execbuffer(struct drm_device *dev,
 	eb.buffer_count = args->buffer_count;
 	eb.batch_start_offset = args->batch_start_offset;
 	eb.batch_len = args->batch_len;
+	eb.trampoline = NULL;
 
 	eb.batch_flags = 0;
 	if (args->flags & I915_EXEC_SECURE) {
@@ -2667,6 +2684,8 @@ i915_gem_do_execbuffer(struct drm_device *dev,
 err_vma:
 	if (eb.exec)
 		eb_release_vmas(&eb);
+	if (eb.trampoline)
+		i915_vma_unpin(eb.trampoline);
 	mutex_unlock(&dev->struct_mutex);
 err_engine:
 	eb_unpin_engine(&eb);
@@ -2742,7 +2761,7 @@ i915_gem_execbuffer_ioctl(struct drm_device *dev, void *data,
 	/* Copy in the exec list from userland */
 	exec_list = kvmalloc_array(count, sizeof(*exec_list),
 				   __GFP_NOWARN | GFP_KERNEL);
-	exec2_list = kvmalloc_array(count + 1, eb_element_size(),
+	exec2_list = kvmalloc_array(count + NUM_EXTRA, eb_element_size(),
 				    __GFP_NOWARN | GFP_KERNEL);
 	if (exec_list == NULL || exec2_list == NULL) {
 		DRM_DEBUG("Failed to allocate exec list for %d buffers\n",
@@ -2818,7 +2837,7 @@ i915_gem_execbuffer2_ioctl(struct drm_device *dev, void *data,
 		return -EINVAL;
 
 	/* Allocate an extra slot for use by the command parser */
-	exec2_list = kvmalloc_array(count + 1, eb_element_size(),
+	exec2_list = kvmalloc_array(count + NUM_EXTRA, eb_element_size(),
 				    __GFP_NOWARN | GFP_KERNEL);
 	if (exec2_list == NULL) {
 		DRM_DEBUG("Failed to allocate exec list for %zd buffers\n",
diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c
index 2977316d64ae..5c942a582b06 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -1504,6 +1504,33 @@ int intel_engine_cmd_parser(struct intel_engine_cs *engine,
 		}
 	} while (1);
 
+	if (!jump_whitelist) { /* setup up the trampoline for chaining */
+		cmd = page_mask_bits(shadow->obj->mm.mapping);
+		if (!ret) {
+			cmd += batch_length / sizeof(*cmd);
+			*cmd = MI_BATCH_BUFFER_END;
+		} else {
+			*cmd = MI_BATCH_BUFFER_END;
+			cmd += batch_length / sizeof(*cmd);
+
+			if (ret == -EACCES) {
+				u32 bbs;
+
+				bbs = MI_BATCH_BUFFER_START;
+				bbs |= MI_BATCH_NON_SECURE_I965;
+				if (IS_HASWELL(engine->i915))
+					bbs |= MI_BATCH_NON_SECURE_HSW;
+
+				cmd[0] = bbs;
+				cmd[1] = batch_addr;
+
+				ret = 0;
+			} else {
+				*cmd = MI_BATCH_BUFFER_END;
+			}
+		}
+	}
+
 	if (needs_clflush_after) {
 		void *ptr = page_mask_bits(shadow->obj->mm.mapping);
 
-- 
2.24.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 7/8] drm/i915: Asynchronous cmdparser

[Chris Wilson]

Execute the cmdparser asynchronously as part of the submission pipeline.
Using our dma-fences, we can schedule execution after an asynchronous
piece of work, so we move the cmdparser out from under the struct_mutex
inside execbuf as run it as part of the submission pipeline. The same
security rules apply, we copy the user batch before validation and
userspace cannot touch the validation shadow. The only caveat is that we
will do request construction before we complete cmdparsing and so we
cannot know the outcome of the validation step until later -- so the
execbuf ioctl does not report -EINVAL directly, but we must cancel
execution of the request and flag the error on the out-fence.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 .../gpu/drm/i915/gem/i915_gem_execbuffer.c    | 82 +++++++++++++++++--
 drivers/gpu/drm/i915/i915_cmd_parser.c        | 33 +++-----
 2 files changed, 85 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 5f4e460701ca..6efca2bcf46a 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -25,6 +25,7 @@
 #include "i915_gem_clflush.h"
 #include "i915_gem_context.h"
 #include "i915_gem_ioctls.h"
+#include "i915_sw_fence_work.h"
 #include "i915_trace.h"
 
 enum {
@@ -1225,10 +1226,6 @@ static u32 *reloc_gpu(struct i915_execbuffer *eb,
 	if (unlikely(!cache->rq)) {
 		int err;
 
-		/* If we need to copy for the cmdparser, we will stall anyway */
-		if (eb_use_cmdparser(eb))
-			return ERR_PTR(-EWOULDBLOCK);
-
 		if (!intel_engine_can_store_dword(eb->engine))
 			return ERR_PTR(-ENODEV);
 
@@ -1967,6 +1964,77 @@ shadow_batch_pin(struct drm_i915_gem_object *obj,
 	return vma;
 }
 
+struct eb_parse_work {
+	struct dma_fence_work base;
+	struct intel_engine_cs *engine;
+	struct i915_vma *batch;
+	struct i915_vma *shadow;
+	unsigned int batch_offset;
+	unsigned int batch_length;
+};
+
+static int __eb_parse(struct dma_fence_work *work)
+{
+	struct eb_parse_work *pw = container_of(work, typeof(*pw), base);
+
+	return intel_engine_cmd_parser(pw->engine,
+				       pw->batch,
+				       pw->batch_offset,
+				       pw->batch_length,
+				       pw->shadow);
+}
+
+static const struct dma_fence_work_ops eb_parse_ops = {
+	.name = "parse",
+	.work = __eb_parse,
+};
+
+static int eb_parse_pipeline(struct i915_execbuffer *eb,
+			     struct i915_vma *shadow)
+{
+	struct eb_parse_work *pw;
+	int err;
+
+	pw = kzalloc(sizeof(*pw), GFP_KERNEL);
+	if (!pw)
+		return -ENOMEM;
+
+	dma_fence_work_init(&pw->base, &eb_parse_ops);
+
+	pw->engine = eb->engine;
+	pw->batch = eb->batch;
+	pw->batch_offset = eb->batch_start_offset;
+	pw->batch_length = eb->batch_len;
+	pw->shadow = shadow;
+
+	dma_resv_lock(pw->batch->resv, NULL);
+	err = dma_resv_reserve_shared(pw->batch->resv, 1);
+	if (err) {
+		dma_resv_unlock(pw->batch->resv);
+		kfree(pw);
+		return err;
+	}
+
+	err = i915_sw_fence_await_reservation(&pw->base.chain,
+					      pw->batch->resv, NULL, false,
+					      0, I915_FENCE_GFP);
+	if (err < 0) {
+		dma_resv_unlock(pw->batch->resv);
+		kfree(pw);
+		return err;
+	}
+
+	dma_resv_add_shared_fence(pw->batch->resv, &pw->base.dma);
+	dma_resv_unlock(pw->batch->resv);
+
+	dma_resv_lock(shadow->resv, NULL);
+	dma_resv_add_excl_fence(shadow->resv, &pw->base.dma);
+	dma_resv_unlock(shadow->resv);
+
+	dma_fence_work_commit(&pw->base);
+	return 0;
+}
+
 static int eb_parse(struct i915_execbuffer *eb)
 {
 	struct intel_engine_pool_node *pool;
@@ -2018,11 +2086,7 @@ static int eb_parse(struct i915_execbuffer *eb)
 		eb->batch_flags |= I915_DISPATCH_SECURE;
 	}
 
-	err = intel_engine_cmd_parser(eb->engine,
-				      eb->batch,
-				      eb->batch_start_offset,
-				      eb->batch_len,
-				      shadow);
+	err = eb_parse_pipeline(eb, shadow);
 	if (err)
 		goto err_trampoline;
 
diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c
index 5c942a582b06..9845182ce587 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -1127,32 +1127,28 @@ find_reg(const struct intel_engine_cs *engine, u32 addr)
 /* Returns a vmap'd pointer to dst_obj, which the caller must unmap */
 static u32 *copy_batch(struct drm_i915_gem_object *dst_obj,
 		       struct drm_i915_gem_object *src_obj,
-		       u32 offset, u32 length,
-		       bool *needs_clflush_after)
+		       u32 offset, u32 length)
 {
-	unsigned int src_needs_clflush;
-	unsigned int dst_needs_clflush;
 	void *dst, *src, *ptr;
+	bool needs_clflush;
 	int ret, len;
 
-	ret = i915_gem_object_prepare_write(dst_obj, &dst_needs_clflush);
-	if (ret)
-		return ERR_PTR(ret);
-
 	dst = i915_gem_object_pin_map(dst_obj, I915_MAP_FORCE_WB);
-	i915_gem_object_finish_access(dst_obj);
 	if (IS_ERR(dst))
 		return dst;
 
-	ret = i915_gem_object_prepare_read(src_obj, &src_needs_clflush);
+	ret = i915_gem_object_pin_pages(src_obj);
 	if (ret) {
 		i915_gem_object_unpin_map(dst_obj);
 		return ERR_PTR(ret);
 	}
 
+	needs_clflush =
+		!(src_obj->cache_coherent & I915_BO_CACHE_COHERENT_FOR_READ);
+
 	ptr = dst;
 	src = ERR_PTR(-ENODEV);
-	if (src_needs_clflush && i915_has_memcpy_from_wc()) {
+	if (needs_clflush && i915_has_memcpy_from_wc()) {
 		src = i915_gem_object_pin_map(src_obj, I915_MAP_WC);
 		if (!IS_ERR(src)) {
 			src += offset;
@@ -1184,7 +1180,7 @@ static u32 *copy_batch(struct drm_i915_gem_object *dst_obj,
 		 * We don't care about copying too much here as we only
 		 * validate up to the end of the batch.
 		 */
-		if (dst_needs_clflush & CLFLUSH_BEFORE)
+		if (!(dst_obj->cache_coherent & I915_BO_CACHE_COHERENT_FOR_READ))
 			length = round_up(length,
 					  boot_cpu_data.x86_clflush_size);
 
@@ -1193,7 +1189,7 @@ static u32 *copy_batch(struct drm_i915_gem_object *dst_obj,
 			len = min_t(int, length, PAGE_SIZE - x);
 
 			src = kmap_atomic(i915_gem_object_get_page(src_obj, n));
-			if (src_needs_clflush)
+			if (needs_clflush)
 				drm_clflush_virt_range(src + x, len);
 			memcpy(ptr, src + x, len);
 			kunmap_atomic(src);
@@ -1204,11 +1200,9 @@ static u32 *copy_batch(struct drm_i915_gem_object *dst_obj,
 		}
 	}
 
-	i915_gem_object_finish_access(src_obj);
+	i915_gem_object_unpin_pages(src_obj);
 
 	/* dst_obj is returned with vmap pinned */
-	*needs_clflush_after = dst_needs_clflush & CLFLUSH_AFTER;
-
 	return dst;
 }
 
@@ -1422,7 +1416,6 @@ int intel_engine_cmd_parser(struct intel_engine_cs *engine,
 	u32 *cmd, *batch_end, offset = 0;
 	struct drm_i915_cmd_descriptor default_desc = noop_desc;
 	const struct drm_i915_cmd_descriptor *desc = &default_desc;
-	bool needs_clflush_after = false;
 	unsigned long *jump_whitelist;
 	u64 batch_addr, shadow_addr;
 	int ret = 0;
@@ -1433,9 +1426,7 @@ int intel_engine_cmd_parser(struct intel_engine_cs *engine,
 				     batch->size));
 	GEM_BUG_ON(!batch_length);
 
-	cmd = copy_batch(shadow->obj, batch->obj,
-			 batch_offset, batch_length,
-			 &needs_clflush_after);
+	cmd = copy_batch(shadow->obj, batch->obj, batch_offset, batch_length);
 	if (IS_ERR(cmd)) {
 		DRM_DEBUG("CMD: Failed to copy batch\n");
 		return PTR_ERR(cmd);
@@ -1531,7 +1522,7 @@ int intel_engine_cmd_parser(struct intel_engine_cs *engine,
 		}
 	}
 
-	if (needs_clflush_after) {
+	if (!(shadow->obj->cache_coherent & I915_BO_CACHE_COHERENT_FOR_WRITE)) {
 		void *ptr = page_mask_bits(shadow->obj->mm.mapping);
 
 		drm_clflush_virt_range(ptr, (void *)(cmd + 1) - ptr);
-- 
2.24.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 8/8] HAX: Use aliasing-ppgtt for gen7

[Chris Wilson]

---
 drivers/gpu/drm/i915/i915_pci.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index bba6b50e6beb..da3e9b5752ac 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -436,7 +436,7 @@ static const struct intel_device_info intel_sandybridge_m_gt2_info = {
 	.has_rc6 = 1, \
 	.has_rc6p = 1, \
 	.has_rps = true, \
-	.ppgtt_type = INTEL_PPGTT_FULL, \
+	.ppgtt_type = INTEL_PPGTT_ALIASING, \
 	.ppgtt_size = 31, \
 	IVB_PIPE_OFFSETS, \
 	IVB_CURSOR_OFFSETS, \
@@ -493,7 +493,7 @@ static const struct intel_device_info intel_valleyview_info = {
 	.has_rps = true,
 	.display.has_gmch = 1,
 	.display.has_hotplug = 1,
-	.ppgtt_type = INTEL_PPGTT_FULL,
+	.ppgtt_type = INTEL_PPGTT_ALIASING,
 	.ppgtt_size = 31,
 	.has_snoop = true,
 	.has_coherent_ggtt = false,
-- 
2.24.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 7/8] drm/i915: Asynchronous cmdparser

[Chris Wilson]

Quoting Chris Wilson (2019-12-07 17:01:09)
> Execute the cmdparser asynchronously as part of the submission pipeline.
> Using our dma-fences, we can schedule execution after an asynchronous
> piece of work, so we move the cmdparser out from under the struct_mutex
> inside execbuf as run it as part of the submission pipeline. The same
> security rules apply, we copy the user batch before validation and
> userspace cannot touch the validation shadow. The only caveat is that we
> will do request construction before we complete cmdparsing and so we
> cannot know the outcome of the validation step until later -- so the
> execbuf ioctl does not report -EINVAL directly, but we must cancel
> execution of the request and flag the error on the out-fence.
 
Closes: https://gitlab.freedesktop.org/drm/intel/issues/611
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 7/8] drm/i915: Asynchronous cmdparser

[Chris Wilson]

Quoting Chris Wilson (2019-12-07 17:17:31)
> Quoting Chris Wilson (2019-12-07 17:01:09)
> > Execute the cmdparser asynchronously as part of the submission pipeline.
> > Using our dma-fences, we can schedule execution after an asynchronous
> > piece of work, so we move the cmdparser out from under the struct_mutex
> > inside execbuf as run it as part of the submission pipeline. The same
> > security rules apply, we copy the user batch before validation and
> > userspace cannot touch the validation shadow. The only caveat is that we
> > will do request construction before we complete cmdparsing and so we
> > cannot know the outcome of the validation step until later -- so the
> > execbuf ioctl does not report -EINVAL directly, but we must cancel
> > execution of the request and flag the error on the out-fence.
>  
> Closes: https://gitlab.freedesktop.org/drm/intel/issues/611

Also
Closes: https://gitlab.freedesktop.org/drm/intel/issues/412

> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/8] drm/i915: Fix cmdparser drm.debug

[Patchwork]

== Series Details ==

Series: series starting with [1/8] drm/i915: Fix cmdparser drm.debug
URL   : https://patchwork.freedesktop.org/series/70586/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
d02707c0b2f8 drm/i915: Fix cmdparser drm.debug
46e41bd43545 drm/i915: Remove redundant parameters from intel_engine_cmd_parser
-:225: ERROR:POINTER_LOCATION: "(foo*)" should be "(foo *)"
#225: FILE: drivers/gpu/drm/i915/i915_cmd_parser.c:1349:
+	*(u64*)(cmd + 1) = shadow_addr + target_cmd_offset;

total: 1 errors, 0 warnings, 0 checks, 340 lines checked
4dccd0538651 drm/i915: Simplify error escape from cmdparser
10edc2cb9e3d drm/i915/gem: Tidy up error handling for eb_parse()
6a96f1a554d2 drm/i915: Align start for memcpy_from_wc
f038f5af17cf drm/i915: Prepare gen7 cmdparser for async execution
a2130d01ad95 drm/i915: Asynchronous cmdparser
fb658d0dd6b0 HAX: Use aliasing-ppgtt for gen7
-:8: WARNING:COMMIT_MESSAGE: Missing commit description - Add an appropriate one

-:28: ERROR:MISSING_SIGN_OFF: Missing Signed-off-by: line(s)

total: 1 errors, 1 warnings, 0 checks, 16 lines checked

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/8] drm/i915: Fix cmdparser drm.debug

[Patchwork]

== Series Details ==

Series: series starting with [1/8] drm/i915: Fix cmdparser drm.debug
URL   : https://patchwork.freedesktop.org/series/70586/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_7507 -> Patchwork_15637
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/index.html

Known issues
------------

  Here are the changes found in Patchwork_15637 that come from known issues:

### IGT changes ###

#### Possible fixes ####

  * igt@i915_selftest@live_blt:
    - fi-hsw-4770r:       [DMESG-FAIL][1] ([i915#553] / [i915#725]) -> [PASS][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/fi-hsw-4770r/igt@i915_selftest@live_blt.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/fi-hsw-4770r/igt@i915_selftest@live_blt.html

  * igt@kms_frontbuffer_tracking@basic:
    - fi-hsw-peppy:       [DMESG-WARN][3] ([i915#44]) -> [PASS][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html

  
  [i915#44]: https://gitlab.freedesktop.org/drm/intel/issues/44
  [i915#553]: https://gitlab.freedesktop.org/drm/intel/issues/553
  [i915#725]: https://gitlab.freedesktop.org/drm/intel/issues/725


Participating hosts (34 -> 27)
------------------------------

  Missing    (7): fi-kbl-soraka fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7507 -> Patchwork_15637

  CI-20190529: 20190529
  CI_DRM_7507: e7dc199b8e4e9a856000d26b346c193f383fc0c3 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5335: 06aa2c377ed40df1e246fca009c441fa18e53825 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_15637: fb658d0dd6b03475c666112dab50e5de88a3fac7 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

fb658d0dd6b0 HAX: Use aliasing-ppgtt for gen7
a2130d01ad95 drm/i915: Asynchronous cmdparser
f038f5af17cf drm/i915: Prepare gen7 cmdparser for async execution
6a96f1a554d2 drm/i915: Align start for memcpy_from_wc
10edc2cb9e3d drm/i915/gem: Tidy up error handling for eb_parse()
4dccd0538651 drm/i915: Simplify error escape from cmdparser
46e41bd43545 drm/i915: Remove redundant parameters from intel_engine_cmd_parser
d02707c0b2f8 drm/i915: Fix cmdparser drm.debug

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✗ Fi.CI.IGT: failure for series starting with [1/8] drm/i915: Fix cmdparser drm.debug

[Patchwork]

== Series Details ==

Series: series starting with [1/8] drm/i915: Fix cmdparser drm.debug
URL   : https://patchwork.freedesktop.org/series/70586/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_7507_full -> Patchwork_15637_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_15637_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_15637_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_15637_full:

### IGT changes ###

#### Possible regressions ####

  * igt@gem_ctx_isolation@vcs0-nonpriv-switch:
    - shard-iclb:         [PASS][1] -> [FAIL][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-iclb2/igt@gem_ctx_isolation@vcs0-nonpriv-switch.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-iclb4/igt@gem_ctx_isolation@vcs0-nonpriv-switch.html
    - shard-tglb:         NOTRUN -> [FAIL][3]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-tglb4/igt@gem_ctx_isolation@vcs0-nonpriv-switch.html

  * igt@gem_exec_parse_blt@allowed-single:
    - shard-glk:          [PASS][4] -> [DMESG-WARN][5]
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-glk1/igt@gem_exec_parse_blt@allowed-single.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-glk4/igt@gem_exec_parse_blt@allowed-single.html

  * igt@gem_exec_parse_blt@cmd-crossing-page:
    - shard-glk:          [PASS][6] -> [FAIL][7]
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-glk8/igt@gem_exec_parse_blt@cmd-crossing-page.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-glk8/igt@gem_exec_parse_blt@cmd-crossing-page.html
    - shard-apl:          [PASS][8] -> [FAIL][9]
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-apl7/igt@gem_exec_parse_blt@cmd-crossing-page.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-apl4/igt@gem_exec_parse_blt@cmd-crossing-page.html

  * igt@kms_big_fb@x-tiled-8bpp-rotate-180:
    - shard-skl:          [PASS][10] -> [DMESG-WARN][11]
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-skl7/igt@kms_big_fb@x-tiled-8bpp-rotate-180.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-skl9/igt@kms_big_fb@x-tiled-8bpp-rotate-180.html

  
#### Warnings ####

  * igt@kms_big_fb@y-tiled-16bpp-rotate-180:
    - shard-tglb:         [DMESG-WARN][12] ([i915#728]) -> [DMESG-WARN][13]
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-tglb4/igt@kms_big_fb@y-tiled-16bpp-rotate-180.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-tglb5/igt@kms_big_fb@y-tiled-16bpp-rotate-180.html

  
Known issues
------------

  Here are the changes found in Patchwork_15637_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_isolation@bcs0-s3:
    - shard-tglb:         [PASS][14] -> [INCOMPLETE][15] ([i915#456])
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-tglb2/igt@gem_ctx_isolation@bcs0-s3.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-tglb2/igt@gem_ctx_isolation@bcs0-s3.html

  * igt@gem_ctx_isolation@vcs0-s3:
    - shard-kbl:          [PASS][16] -> [INCOMPLETE][17] ([fdo#103665])
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-kbl3/igt@gem_ctx_isolation@vcs0-s3.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-kbl3/igt@gem_ctx_isolation@vcs0-s3.html

  * igt@gem_exec_gttfill@basic:
    - shard-tglb:         [PASS][18] -> [INCOMPLETE][19] ([fdo#111593])
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-tglb3/igt@gem_exec_gttfill@basic.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-tglb8/igt@gem_exec_gttfill@basic.html

  * igt@gem_exec_parallel@vecs0:
    - shard-tglb:         [PASS][20] -> [INCOMPLETE][21] ([fdo#111736])
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-tglb5/igt@gem_exec_parallel@vecs0.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-tglb4/igt@gem_exec_parallel@vecs0.html

  * igt@gem_exec_schedule@out-order-bsd:
    - shard-iclb:         [PASS][22] -> [SKIP][23] ([fdo#112146])
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-iclb6/igt@gem_exec_schedule@out-order-bsd.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-iclb2/igt@gem_exec_schedule@out-order-bsd.html

  * igt@gem_exec_schedule@smoketest-bsd2:
    - shard-tglb:         [PASS][24] -> [INCOMPLETE][25] ([i915#707]) +1 similar issue
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-tglb2/igt@gem_exec_schedule@smoketest-bsd2.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-tglb3/igt@gem_exec_schedule@smoketest-bsd2.html

  * igt@gem_persistent_relocs@forked-interruptible-thrashing:
    - shard-kbl:          [PASS][26] -> [FAIL][27] ([i915#520])
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-kbl7/igt@gem_persistent_relocs@forked-interruptible-thrashing.html
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-kbl6/igt@gem_persistent_relocs@forked-interruptible-thrashing.html

  * igt@gem_ppgtt@flink-and-close-vma-leak:
    - shard-glk:          [PASS][28] -> [FAIL][29] ([i915#644])
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-glk5/igt@gem_ppgtt@flink-and-close-vma-leak.html
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-glk2/igt@gem_ppgtt@flink-and-close-vma-leak.html

  * igt@gem_userptr_blits@dmabuf-sync:
    - shard-snb:          [PASS][30] -> [DMESG-WARN][31] ([fdo#111870]) +1 similar issue
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-snb1/igt@gem_userptr_blits@dmabuf-sync.html
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-snb2/igt@gem_userptr_blits@dmabuf-sync.html

  * igt@gem_userptr_blits@sync-unmap:
    - shard-snb:          [PASS][32] -> [DMESG-WARN][33] ([fdo#110789] / [fdo#111870])
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-snb4/igt@gem_userptr_blits@sync-unmap.html
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-snb5/igt@gem_userptr_blits@sync-unmap.html

  * igt@gem_userptr_blits@sync-unmap-after-close:
    - shard-hsw:          [PASS][34] -> [DMESG-WARN][35] ([fdo#111870]) +4 similar issues
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-hsw1/igt@gem_userptr_blits@sync-unmap-after-close.html
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-hsw1/igt@gem_userptr_blits@sync-unmap-after-close.html

  * igt@gem_vm_create@execbuf:
    - shard-hsw:          [PASS][36] -> [SKIP][37] ([fdo#109271]) +8 similar issues
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-hsw2/igt@gem_vm_create@execbuf.html
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-hsw5/igt@gem_vm_create@execbuf.html

  * igt@gem_workarounds@suspend-resume:
    - shard-tglb:         [PASS][38] -> [INCOMPLETE][39] ([i915#456] / [i915#460])
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-tglb3/igt@gem_workarounds@suspend-resume.html
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-tglb5/igt@gem_workarounds@suspend-resume.html

  * igt@i915_selftest@live_hangcheck:
    - shard-tglb:         [PASS][40] -> [INCOMPLETE][41] ([i915#435])
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-tglb4/igt@i915_selftest@live_hangcheck.html
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-tglb6/igt@i915_selftest@live_hangcheck.html

  * igt@kms_concurrent@pipe-c:
    - shard-hsw:          [PASS][42] -> [DMESG-WARN][43] ([i915#44])
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-hsw1/igt@kms_concurrent@pipe-c.html
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-hsw5/igt@kms_concurrent@pipe-c.html

  * igt@kms_cursor_crc@pipe-a-cursor-suspend:
    - shard-kbl:          [PASS][44] -> [DMESG-WARN][45] ([i915#180]) +4 similar issues
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-kbl6/igt@kms_cursor_crc@pipe-a-cursor-suspend.html
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-kbl2/igt@kms_cursor_crc@pipe-a-cursor-suspend.html

  * igt@kms_cursor_crc@pipe-b-cursor-128x42-sliding:
    - shard-skl:          [PASS][46] -> [FAIL][47] ([i915#54]) +1 similar issue
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-skl5/igt@kms_cursor_crc@pipe-b-cursor-128x42-sliding.html
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-skl10/igt@kms_cursor_crc@pipe-b-cursor-128x42-sliding.html

  * igt@kms_cursor_crc@pipe-c-cursor-256x85-random:
    - shard-kbl:          [PASS][48] -> [FAIL][49] ([i915#54])
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-kbl3/igt@kms_cursor_crc@pipe-c-cursor-256x85-random.html
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-kbl2/igt@kms_cursor_crc@pipe-c-cursor-256x85-random.html

  * igt@kms_draw_crc@draw-method-xrgb8888-pwrite-xtiled:
    - shard-skl:          [PASS][50] -> [FAIL][51] ([i915#52] / [i915#54])
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-skl8/igt@kms_draw_crc@draw-method-xrgb8888-pwrite-xtiled.html
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-skl10/igt@kms_draw_crc@draw-method-xrgb8888-pwrite-xtiled.html

  * igt@kms_flip@2x-flip-vs-suspend-interruptible:
    - shard-hsw:          [PASS][52] -> [INCOMPLETE][53] ([i915#61]) +1 similar issue
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-hsw5/igt@kms_flip@2x-flip-vs-suspend-interruptible.html
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-hsw5/igt@kms_flip@2x-flip-vs-suspend-interruptible.html

  * igt@kms_flip@2x-plain-flip-fb-recreate:
    - shard-glk:          [PASS][54] -> [FAIL][55] ([i915#34])
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-glk6/igt@kms_flip@2x-plain-flip-fb-recreate.html
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-glk3/igt@kms_flip@2x-plain-flip-fb-recreate.html

  * igt@kms_flip@flip-vs-suspend:
    - shard-tglb:         [PASS][56] -> [INCOMPLETE][57] ([i915#460] / [i915#516])
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-tglb6/igt@kms_flip@flip-vs-suspend.html
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-tglb1/igt@kms_flip@flip-vs-suspend.html

  * igt@kms_flip@flip-vs-suspend-interruptible:
    - shard-apl:          [PASS][58] -> [DMESG-WARN][59] ([i915#180]) +2 similar issues
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-apl2/igt@kms_flip@flip-vs-suspend-interruptible.html
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-apl8/igt@kms_flip@flip-vs-suspend-interruptible.html

  * igt@kms_flip@modeset-vs-vblank-race:
    - shard-hsw:          [PASS][60] -> [DMESG-FAIL][61] ([i915#407] / [i915#44])
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-hsw2/igt@kms_flip@modeset-vs-vblank-race.html
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-hsw5/igt@kms_flip@modeset-vs-vblank-race.html

  * igt@kms_frontbuffer_tracking@basic:
    - shard-iclb:         [PASS][62] -> [FAIL][63] ([i915#49]) +1 similar issue
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-iclb2/igt@kms_frontbuffer_tracking@basic.html
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-iclb4/igt@kms_frontbuffer_tracking@basic.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-shrfb-pgflip-blt:
    - shard-tglb:         [PASS][64] -> [FAIL][65] ([i915#49])
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-tglb4/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-shrfb-pgflip-blt.html
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-tglb2/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-shrfb-pgflip-blt.html

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-cur-indfb-draw-pwrite:
    - shard-skl:          [PASS][66] -> [DMESG-WARN][67] ([i915#728]) +4 similar issues
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-skl5/igt@kms_frontbuffer_tracking@psr-1p-primscrn-cur-indfb-draw-pwrite.html
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-skl5/igt@kms_frontbuffer_tracking@psr-1p-primscrn-cur-indfb-draw-pwrite.html

  * igt@kms_plane@pixel-format-pipe-a-planes:
    - shard-kbl:          [PASS][68] -> [DMESG-WARN][69] ([i915#728]) +3 similar issues
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-kbl7/igt@kms_plane@pixel-format-pipe-a-planes.html
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-kbl4/igt@kms_plane@pixel-format-pipe-a-planes.html

  * igt@kms_psr@psr2_cursor_blt:
    - shard-iclb:         [PASS][70] -> [SKIP][71] ([fdo#109441]) +1 similar issue
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-iclb2/igt@kms_psr@psr2_cursor_blt.html
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-iclb4/igt@kms_psr@psr2_cursor_blt.html

  * igt@perf_pmu@multi-client-vcs1:
    - shard-iclb:         [PASS][72] -> [DMESG-WARN][73] ([i915#728]) +1 similar issue
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-iclb2/igt@perf_pmu@multi-client-vcs1.html
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-iclb4/igt@perf_pmu@multi-client-vcs1.html

  
#### Possible fixes ####

  * igt@gem_ctx_isolation@rcs0-s3:
    - shard-tglb:         [INCOMPLETE][74] ([i915#456]) -> [PASS][75]
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-tglb1/igt@gem_ctx_isolation@rcs0-s3.html
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-tglb2/igt@gem_ctx_isolation@rcs0-s3.html

  * igt@gem_ctx_isolation@vcs1-reset:
    - shard-kbl:          [FAIL][76] -> [PASS][77] +7 similar issues
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-kbl2/igt@gem_ctx_isolation@vcs1-reset.html
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-kbl6/igt@gem_ctx_isolation@vcs1-reset.html

  * igt@gem_ctx_isolation@vecs0-clean:
    - shard-apl:          [FAIL][78] -> [PASS][79] +5 similar issues
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-apl6/igt@gem_ctx_isolation@vecs0-clean.html
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-apl8/igt@gem_ctx_isolation@vecs0-clean.html
    - shard-skl:          [FAIL][80] -> [PASS][81] +3 similar issues
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-skl3/igt@gem_ctx_isolation@vecs0-clean.html
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-skl9/igt@gem_ctx_isolation@vecs0-clean.html
    - shard-glk:          [FAIL][82] -> [PASS][83] +4 similar issues
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-glk7/igt@gem_ctx_isolation@vecs0-clean.html
   [83]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-glk6/igt@gem_ctx_isolation@vecs0-clean.html

  * igt@gem_ctx_shared@exec-shared-gtt-bsd2:
    - shard-iclb:         [SKIP][84] ([fdo#109276]) -> [PASS][85]
   [84]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-iclb6/igt@gem_ctx_shared@exec-shared-gtt-bsd2.html
   [85]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-iclb2/igt@gem_ctx_shared@exec-shared-gtt-bsd2.html

  * igt@gem_exec_parse_blt@allowed-single:
    - shard-skl:          [DMESG-WARN][86] -> [PASS][87] +1 similar issue
   [86]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-skl1/igt@gem_exec_parse_blt@allowed-single.html
   [87]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-skl3/igt@gem_exec_parse_blt@allowed-single.html

  * igt@gem_exec_reloc@basic-spin-blt:
    - shard-glk:          [FAIL][88] ([i915#611]) -> [PASS][89]
   [88]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-glk9/igt@gem_exec_reloc@basic-spin-blt.html
   [89]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-glk1/igt@gem_exec_reloc@basic-spin-blt.html
    - shard-apl:          [FAIL][90] ([i915#611]) -> [PASS][91]
   [90]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-apl4/igt@gem_exec_reloc@basic-spin-blt.html
   [91]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-apl6/igt@gem_exec_reloc@basic-spin-blt.html
    - shard-kbl:          [FAIL][92] ([i915#611]) -> [PASS][93]
   [92]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-kbl2/igt@gem_exec_reloc@basic-spin-blt.html
   [93]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-kbl7/igt@gem_exec_reloc@basic-spin-blt.html

  * igt@gem_exec_schedule@deep-blt:
    - shard-apl:          [FAIL][94] ([i915#412]) -> [PASS][95]
   [94]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-apl1/igt@gem_exec_schedule@deep-blt.html
   [95]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-apl7/igt@gem_exec_schedule@deep-blt.html
    - shard-kbl:          [FAIL][96] ([i915#412]) -> [PASS][97]
   [96]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-kbl2/igt@gem_exec_schedule@deep-blt.html
   [97]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-kbl6/igt@gem_exec_schedule@deep-blt.html

  * igt@gem_exec_schedule@preempt-queue-bsd1:
    - shard-tglb:         [INCOMPLETE][98] ([fdo#111677]) -> [PASS][99] +1 similar issue
   [98]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-tglb6/igt@gem_exec_schedule@preempt-queue-bsd1.html
   [99]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-tglb4/igt@gem_exec_schedule@preempt-queue-bsd1.html

  * igt@gem_persistent_relocs@forked-thrash-inactive:
    - shard-hsw:          [INCOMPLETE][100] ([i915#530] / [i915#61]) -> [PASS][101]
   [100]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-hsw6/igt@gem_persistent_relocs@forked-thrash-inactive.html
   [101]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-hsw2/igt@gem_persistent_relocs@forked-thrash-inactive.html

  * igt@gem_ppgtt@flink-and-close-vma-leak:
    - shard-skl:          [FAIL][102] ([i915#644]) -> [PASS][103]
   [102]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-skl2/igt@gem_ppgtt@flink-and-close-vma-leak.html
   [103]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-skl7/igt@gem_ppgtt@flink-and-close-vma-leak.html

  * igt@gem_userptr_blits@sync-unmap-cycles:
    - shard-snb:          [DMESG-WARN][104] ([fdo#111870]) -> [PASS][105] +1 similar issue
   [104]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-snb6/igt@gem_userptr_blits@sync-unmap-cycles.html
   [105]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-snb1/igt@gem_userptr_blits@sync-unmap-cycles.html

  * igt@kms_cursor_crc@pipe-b-cursor-64x21-sliding:
    - shard-skl:          [FAIL][106] ([i915#54]) -> [PASS][107] +5 similar issues
   [106]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-skl8/igt@kms_cursor_crc@pipe-b-cursor-64x21-sliding.html
   [107]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-skl1/igt@kms_cursor_crc@pipe-b-cursor-64x21-sliding.html

  * igt@kms_cursor_crc@pipe-c-cursor-suspend:
    - shard-skl:          [INCOMPLETE][108] ([i915#300]) -> [PASS][109]
   [108]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-skl7/igt@kms_cursor_crc@pipe-c-cursor-suspend.html
   [109]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-skl1/igt@kms_cursor_crc@pipe-c-cursor-suspend.html

  * igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy:
    - shard-glk:          [FAIL][110] ([i915#72]) -> [PASS][111]
   [110]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-glk9/igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy.html
   [111]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-glk1/igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy.html

  * igt@kms_cursor_legacy@cursor-vs-flip-atomic:
    - shard-hsw:          [FAIL][112] ([i915#57]) -> [PASS][113]
   [112]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-hsw6/igt@kms_cursor_legacy@cursor-vs-flip-atomic.html
   [113]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-hsw6/igt@kms_cursor_legacy@cursor-vs-flip-atomic.html

  * igt@kms_draw_crc@draw-method-xrgb2101010-render-ytiled:
    - shard-skl:          [INCOMPLETE][114] ([i915#646]) -> [PASS][115] +1 similar issue
   [114]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-skl7/igt@kms_draw_crc@draw-method-xrgb2101010-render-ytiled.html
   [115]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-skl9/igt@kms_draw_crc@draw-method-xrgb2101010-render-ytiled.html

  * igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-indfb-draw-render:
    - shard-tglb:         [FAIL][116] ([i915#49]) -> [PASS][117] +2 similar issues
   [116]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-tglb3/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-indfb-draw-render.html
   [117]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-tglb8/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-indfb-draw-render.html

  * igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-blt:
    - shard-kbl:          [DMESG-WARN][118] ([i915#728]) -> [PASS][119] +2 similar issues
   [118]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-kbl3/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-blt.html
   [119]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-kbl7/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-blt.html

  * igt@kms_frontbuffer_tracking@fbc-rgb565-draw-pwrite:
    - shard-iclb:         [FAIL][120] ([i915#49]) -> [PASS][121] +2 similar issues
   [120]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-iclb1/igt@kms_frontbuffer_tracking@fbc-rgb565-draw-pwrite.html
   [121]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-iclb4/igt@kms_frontbuffer_tracking@fbc-rgb565-draw-pwrite.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-mmap-wc:
    - shard-tglb:         [INCOMPLETE][122] ([i915#474]) -> [PASS][123]
   [122]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-tglb7/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-mmap-wc.html
   [123]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-tglb6/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-mmap-wc.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-shrfb-plflip-blt:
    - shard-iclb:         [DMESG-WARN][124] ([i915#728]) -> [PASS][125] +1 similar issue
   [124]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-iclb1/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-shrfb-plflip-blt.html
   [125]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-iclb4/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-shrfb-plflip-blt.html

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-indfb-plflip-blt:
    - shard-skl:          [INCOMPLETE][126] ([i915#123]) -> [PASS][127]
   [126]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-skl1/igt@kms_frontbuffer_tracking@psr-1p-primscrn-indfb-plflip-blt.html
   [127]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-skl2/igt@kms_frontbuffer_tracking@psr-1p-primscrn-indfb-plflip-blt.html

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-draw-render:
    - shard-skl:          [DMESG-WARN][128] ([i915#728]) -> [PASS][129]
   [128]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-skl10/igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-draw-render.html
   [129]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-skl1/igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-draw-render.html

  * igt@kms_plane@pixel-format-pipe-b-planes-source-clamping:
    - shard-skl:          [INCOMPLETE][130] ([fdo#112347] / [fdo#112391] / [i915#648]) -> [PASS][131] +1 similar issue
   [130]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-skl6/igt@kms_plane@pixel-format-pipe-b-planes-source-clamping.html
   [131]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-skl7/igt@kms_plane@pixel-format-pipe-b-planes-source-clamping.html

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes:
    - shard-apl:          [DMESG-WARN][132] ([i915#180]) -> [PASS][133]
   [132]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-apl4/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html
   [133]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-apl6/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html

  * igt@kms_psr@psr2_no_drrs:
    - shard-iclb:         [SKIP][134] ([fdo#109441]) -> [PASS][135]
   [134]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-iclb6/igt@kms_psr@psr2_no_drrs.html
   [135]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-iclb2/igt@kms_psr@psr2_no_drrs.html

  * igt@kms_psr@psr2_suspend:
    - shard-tglb:         [DMESG-WARN][136] ([i915#402]) -> [PASS][137]
   [136]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-tglb2/igt@kms_psr@psr2_suspend.html
   [137]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-tglb6/igt@kms_psr@psr2_suspend.html

  * igt@kms_psr@suspend:
    - shard-tglb:         [INCOMPLETE][138] ([i915#456] / [i915#460]) -> [PASS][139] +2 similar issues
   [138]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-tglb8/igt@kms_psr@suspend.html
   [139]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-tglb1/igt@kms_psr@suspend.html

  * igt@kms_vblank@pipe-a-ts-continuation-suspend:
    - shard-kbl:          [DMESG-WARN][140] ([i915#180]) -> [PASS][141] +3 similar issues
   [140]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-kbl2/igt@kms_vblank@pipe-a-ts-continuation-suspend.html
   [141]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-kbl7/igt@kms_vblank@pipe-a-ts-continuation-suspend.html

  * igt@perf_pmu@frequency-idle:
    - shard-kbl:          [DMESG-WARN][142] -> [PASS][143]
   [142]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-kbl7/igt@perf_pmu@frequency-idle.html
   [143]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-kbl4/igt@perf_pmu@frequency-idle.html

  * igt@perf_pmu@render-node-busy-idle-vcs1:
    - shard-iclb:         [SKIP][144] ([fdo#112080]) -> [PASS][145] +1 similar issue
   [144]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-iclb6/igt@perf_pmu@render-node-busy-idle-vcs1.html
   [145]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-iclb2/igt@perf_pmu@render-node-busy-idle-vcs1.html

  * igt@perf_pmu@render-node-busy-idle-vecs0:
    - shard-tglb:         [DMESG-WARN][146] ([i915#728]) -> [PASS][147] +4 similar issues
   [146]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-tglb2/igt@perf_pmu@render-node-busy-idle-vecs0.html
   [147]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-tglb3/igt@perf_pmu@render-node-busy-idle-vecs0.html

  
#### Warnings ####

  * igt@gem_eio@kms:
    - shard-snb:          [INCOMPLETE][148] ([i915#82]) -> [DMESG-WARN][149] ([i915#444] / [i915#502])
   [148]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-snb7/igt@gem_eio@kms.html
   [149]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-snb6/igt@gem_eio@kms.html

  * igt@kms_flip@flip-vs-suspend:
    - shard-kbl:          [DMESG-WARN][150] ([i915#180]) -> [INCOMPLETE][151] ([fdo#103665] / [i915#600])
   [150]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-kbl2/igt@kms_flip@flip-vs-suspend.html
   [151]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-kbl7/igt@kms_flip@flip-vs-suspend.html

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-shrfb-plflip-blt:
    - shard-skl:          [DMESG-WARN][152] -> [DMESG-WARN][153] ([i915#728])
   [152]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7507/shard-skl4/igt@kms_frontbuffer_tracking@psr-1p-primscrn-shrfb-plflip-blt.html
   [153]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/shard-skl6/igt@kms_frontbuffer_tracking@psr-1p-primscrn-shrfb-plflip-blt.html

  
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#110789]: https://bugs.freedesktop.org/show_bug.cgi?id=110789
  [fdo#111593]: https://bugs.freedesktop.org/show_bug.cgi?id=111593
  [fdo#111677]: https://bugs.freedesktop.org/show_bug.cgi?id=111677
  [fdo#111736]: https://bugs.freedesktop.org/show_bug.cgi?id=111736
  [fdo#111870]: https://bugs.freedesktop.org/show_bug.cgi?id=111870
  [fdo#112080]: https://bugs.freedesktop.org/show_bug.cgi?id=112080
  [fdo#112146]: https://bugs.freedesktop.org/show_bug.cgi?id=112146
  [fdo#112347]: https://bugs.freedesktop.org/show_bug.cgi?id=112347
  [fdo#112391]: https://bugs.freedesktop.org/show_bug.cgi?id=112391
  [i915#123]: https://gitlab.freedesktop.org/drm/intel/issues/123
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#300]: https://gitlab.freedesktop.org/drm/intel/issues/300
  [i915#34]: https://gitlab.freedesktop.org/drm/intel/issues/34
  [i915#402]: https://gitlab.freedesktop.org/drm/intel/issues/402
  [i915#407]: https://gitlab.freedesktop.org/drm/intel/issues/407
  [i915#412]: https://gi

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15637/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/8] drm/i915: Fix cmdparser drm.debug

[Joonas Lahtinen]

Quoting Chris Wilson (2019-12-07 19:01:03)
> The cmdparser rejection debug is not for driver development, but for the
> user, for which we use a plain DRM_DEBUG().
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Regards, Joonas
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/8] drm/i915: Remove redundant parameters from intel_engine_cmd_parser

[Joonas Lahtinen]

Quoting Chris Wilson (2019-12-07 19:01:04)
> Declutter the calling interface by reducing the parameters to the
> i915_vma and associated offsets.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Regards, Joonas
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 3/8] drm/i915: Simplify error escape from cmdparser

[Joonas Lahtinen]

Quoting Chris Wilson (2019-12-07 19:01:05)
> We need to flush the destination buffer, even on error, to maintain
> consistent cache state. Thereby removing the jump on error past the
> clear, and reducing the loop-escape mechanism to a mere break.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Regards, Joonas
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 4/8] drm/i915/gem: Tidy up error handling for eb_parse()

[Joonas Lahtinen]

Quoting Chris Wilson (2019-12-07 19:01:06)
> As the caller no longer uses the i915_vma result, stop returning it and
> just return the error code instead.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

<SNIP>

> @@ -2002,8 +2007,6 @@ static struct i915_vma *eb_parse(struct i915_execbuffer *eb)
>                                       eb->batch_len,
>                                       vma);
>         if (err) {
> -               i915_vma_unpin(vma);
> -
>                 /*
>                  * Unsafe GGTT-backed buffers can still be submitted safely
>                  * as non-secure.
> @@ -2012,10 +2015,8 @@ static struct i915_vma *eb_parse(struct i915_execbuffer *eb)
>                  */
>                 if (i915_vma_is_ggtt(vma) && err == -EACCES)
>                         /* Execute original buffer non-secure */

This second comment is bit of a repetition, especially after the code
flow is simplified.

Either way;

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Regards, Joonas
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 5/8] drm/i915: Align start for memcpy_from_wc

[Joonas Lahtinen]

Quoting Chris Wilson (2019-12-07 19:01:07)
> The movntqda requires 16-byte alignment for the source pointer. Avoid
> falling back to clflush if the source pointer is misaligned by doing the
> doing a small uncached memcpy to fixup the alignments.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

<SNIP>

> @@ -1150,19 +1150,30 @@ static u32 *copy_batch(struct drm_i915_gem_object *dst_obj,
>                 return ERR_PTR(ret);
>         }
>  
> +       ptr = dst;

I'd add a newline here as this will propagate to the later branch if WC
map fails.

>         src = ERR_PTR(-ENODEV);
> -       if (src_needs_clflush &&
> -           i915_can_memcpy_from_wc(NULL, offset, 0)) {
> +       if (src_needs_clflush && i915_has_memcpy_from_wc()) {
>                 src = i915_gem_object_pin_map(src_obj, I915_MAP_WC);
>                 if (!IS_ERR(src)) {
> -                       i915_memcpy_from_wc(dst,
> -                                           src + offset,
> -                                           ALIGN(length, 16));
> +                       src += offset;
> +
> +                       if (!IS_ALIGNED(offset, 16)) {
> +                               len = min(ALIGN(offset, 16) - offset, length);
> +
> +                               memcpy(ptr, src, len);
> +
> +                               offset += len;
> +                               length -= len;
> +                               ptr += len;
> +                               src += len;
> +                       }
> +                       GEM_BUG_ON(!IS_ALIGNED((unsigned long)src, 16));
> +
> +                       i915_memcpy_from_wc(ptr, src, ALIGN(length, 16));

Could be a helper function.

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Regards, Joonas
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 6/8] drm/i915: Prepare gen7 cmdparser for async execution

[Joonas Lahtinen]

Quoting Chris Wilson (2019-12-07 19:01:08)
> The gen7 cmdparser is primarily a promotion-based system to allow access
> to additional registers beyond the HW validation, and allows fallback to
> normal execution of the user batch buffer if valid and requires
> chaining. In the next patch, we will do the cmdparser validation in the
> pipeline asynchronously and so at the point of request construction we
> will not know if we want to execute the privileged and validated batch,
> or the original user batch. The solution employed here is to execute
> both batches, one with raised privileges and one as normal. This is
> because the gen7 MI_BATCH_BUFFER_START command cannot change privilege
> level within a batch and must strictly use the current privilege level
> (or undefined behaviour kills the GPU). So in order to execute the
> original batch, we need a second non-priviledged batch buffer chain from
> the ring, i.e. we need to emit two batches for each user batch. Inside
> the two batches we determine which one should actually execute, we
> provide a conditional trampoline to call the original batch.

It's only a single batch executed twice from different offsets. I would
rephrase the commit message to reflect that.

> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

<SNIP>

> #define NUM_EXTRA 2

Looks like BOs, we should have a more descriptive name.

#define NUM_KERNEL_BUFFERS?

> @@ -1985,59 +1970,80 @@ shadow_batch_pin(struct i915_execbuffer *eb, struct drm_i915_gem_object *obj)
>  static int eb_parse(struct i915_execbuffer *eb)
>  {
>         struct intel_engine_pool_node *pool;
> -       struct i915_vma *vma;
> +       struct i915_vma *shadow, *trampoline;
> +       unsigned int len;
>         int err;
>  
>         if (!eb_use_cmdparser(eb))
>                 return 0;
>  
> -       pool = intel_engine_get_pool(eb->engine, eb->batch_len);
> +       len = eb->batch_len;
> +       if (!CMDPARSER_USES_GGTT(eb->i915)) {
> +               /*
> +                * PPGTT backed shadow buffers must be mapped RO, to prevent
> +                * post-scan tampering
> +                */
> +               if (!eb->context->vm->has_read_only) {
> +                       DRM_DEBUG("Cannot prevent post-scan tampering without RO capable vm\n");
> +                       return -EINVAL;
> +               }
> +       } else {
> +               len += 8;

Magic number. #define TRAMPOLINE_SOMETHING ?

> @@ -2089,6 +2095,16 @@ static int eb_submit(struct i915_execbuffer *eb)
>         if (err)
>                 return err;
>  
> +       if (eb->trampoline) {
> +               GEM_BUG_ON(eb->batch_start_offset);
> +               err = eb->engine->emit_bb_start(eb->request,
> +                                               eb->trampoline->node.start +
> +                                               eb->batch_len,
> +                                               8, 0);

Magic 8 detected.

I'd emphasis that we're jumping to the end, either by computing start +
batch_len separately or bringing them to same line.

> @@ -1504,6 +1504,33 @@ int intel_engine_cmd_parser(struct intel_engine_cs *engine,
>                 }
>         } while (1);
>  
> +       if (!jump_whitelist) { /* setup up the trampoline for chaining */

I think we should hoist the CMDPARSER_USES_GGTT check from
alloc_whitelist function. It's quite misleading now, and I
spent quite some time wondering why we would only do this on
out of memory.

Especially as there is a comment "defer failure until attempted use".

> +               cmd = page_mask_bits(shadow->obj->mm.mapping);
> +               if (!ret) {
> +                       cmd += batch_length / sizeof(*cmd);

This could use sharing the offset through variable/helper function
to tie this to be overwriting the trampoline jump.

Helper func maybe to compute the offset of trampoline, even if it
happens to be right at the end.

> +                       *cmd = MI_BATCH_BUFFER_END;
> +               } else {
> +                       *cmd = MI_BATCH_BUFFER_END;
> +                       cmd += batch_length / sizeof(*cmd);

Again using the helper function would help tracing that each BB
is jumped to twice, and this is about the second jump.

> +
> +                       if (ret == -EACCES) {
> +                               u32 bbs;
> +
> +                               bbs = MI_BATCH_BUFFER_START;
> +                               bbs |= MI_BATCH_NON_SECURE_I965;
> +                               if (IS_HASWELL(engine->i915))
> +                                       bbs |= MI_BATCH_NON_SECURE_HSW;
> +
> +                               cmd[0] = bbs;
> +                               cmd[1] = batch_addr;

__{gen6,hsw}_bb_start helper?

With the magics removed this is;

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Regards, Joonas
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 6/8] drm/i915: Prepare gen7 cmdparser for async execution

[Chris Wilson]

Quoting Joonas Lahtinen (2019-12-11 11:27:17)
> Quoting Chris Wilson (2019-12-07 19:01:08)
> > The gen7 cmdparser is primarily a promotion-based system to allow access
> > to additional registers beyond the HW validation, and allows fallback to
> > normal execution of the user batch buffer if valid and requires
> > chaining. In the next patch, we will do the cmdparser validation in the
> > pipeline asynchronously and so at the point of request construction we
> > will not know if we want to execute the privileged and validated batch,
> > or the original user batch. The solution employed here is to execute
> > both batches, one with raised privileges and one as normal. This is
> > because the gen7 MI_BATCH_BUFFER_START command cannot change privilege
> > level within a batch and must strictly use the current privilege level
> > (or undefined behaviour kills the GPU). So in order to execute the
> > original batch, we need a second non-priviledged batch buffer chain from
> > the ring, i.e. we need to emit two batches for each user batch. Inside
> > the two batches we determine which one should actually execute, we
> > provide a conditional trampoline to call the original batch.
> 
> It's only a single batch executed twice from different offsets. I would
> rephrase the commit message to reflect that.
> 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> <SNIP>
> 
> > #define NUM_EXTRA 2
> 
> Looks like BOs, we should have a more descriptive name.

It's not just bo, it's the execbuf state.
 
> #define NUM_KERNEL_BUFFERS?

I'll go one better, and drop it since it ended up not being used.

> > @@ -1985,59 +1970,80 @@ shadow_batch_pin(struct i915_execbuffer *eb, struct drm_i915_gem_object *obj)
> >  static int eb_parse(struct i915_execbuffer *eb)
> >  {
> >         struct intel_engine_pool_node *pool;
> > -       struct i915_vma *vma;
> > +       struct i915_vma *shadow, *trampoline;
> > +       unsigned int len;
> >         int err;
> >  
> >         if (!eb_use_cmdparser(eb))
> >                 return 0;
> >  
> > -       pool = intel_engine_get_pool(eb->engine, eb->batch_len);
> > +       len = eb->batch_len;
> > +       if (!CMDPARSER_USES_GGTT(eb->i915)) {
> > +               /*
> > +                * PPGTT backed shadow buffers must be mapped RO, to prevent
> > +                * post-scan tampering
> > +                */
> > +               if (!eb->context->vm->has_read_only) {
> > +                       DRM_DEBUG("Cannot prevent post-scan tampering without RO capable vm\n");
> > +                       return -EINVAL;
> > +               }
> > +       } else {
> > +               len += 8;
> 
> Magic number. #define TRAMPOLINE_SOMETHING ?
> 
> > @@ -2089,6 +2095,16 @@ static int eb_submit(struct i915_execbuffer *eb)
> >         if (err)
> >                 return err;
> >  
> > +       if (eb->trampoline) {
> > +               GEM_BUG_ON(eb->batch_start_offset);
> > +               err = eb->engine->emit_bb_start(eb->request,
> > +                                               eb->trampoline->node.start +
> > +                                               eb->batch_len,
> > +                                               8, 0);
> 
> Magic 8 detected.
> 
> I'd emphasis that we're jumping to the end, either by computing start +
> batch_len separately or bringing them to same line.

Fwiw, I kept the line split to match the original eb->engine->emit_bb_start() call.
You can't see that in the diff

I'll replace the magic 8 with the even more magic 0 :-p

The rest will take some time to polish up.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 7/8] drm/i915: Asynchronous cmdparser

[Joonas Lahtinen]

+ Daniel/Maarten for the dma_resv

Quoting Chris Wilson (2019-12-07 19:01:09)
> Execute the cmdparser asynchronously as part of the submission pipeline.
> Using our dma-fences, we can schedule execution after an asynchronous
> piece of work, so we move the cmdparser out from under the struct_mutex
> inside execbuf as run it as part of the submission pipeline. The same
> security rules apply, we copy the user batch before validation and
> userspace cannot touch the validation shadow. The only caveat is that we
> will do request construction before we complete cmdparsing and so we
> cannot know the outcome of the validation step until later -- so the
> execbuf ioctl does not report -EINVAL directly, but we must cancel
> execution of the request and flag the error on the out-fence.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

<SNIP>

> +static const struct dma_fence_work_ops eb_parse_ops = {
> +       .name = "parse",

I'm noticing all our dma_fence_work_ops are named very briefly.

.name = "eb_parse" might be in order.

> +       .work = __eb_parse,
> +};
> +
> +static int eb_parse_pipeline(struct i915_execbuffer *eb,
> +                            struct i915_vma *shadow)
> +{
> +       struct eb_parse_work *pw;
> +       int err;
> +
> +       pw = kzalloc(sizeof(*pw), GFP_KERNEL);
> +       if (!pw)
> +               return -ENOMEM;
> +
> +       dma_fence_work_init(&pw->base, &eb_parse_ops);
> +
> +       pw->engine = eb->engine;
> +       pw->batch = eb->batch;
> +       pw->batch_offset = eb->batch_start_offset;
> +       pw->batch_length = eb->batch_len;
> +       pw->shadow = shadow;
> +
> +       dma_resv_lock(pw->batch->resv, NULL);
> +       err = dma_resv_reserve_shared(pw->batch->resv, 1);
> +       if (err) {
> +               dma_resv_unlock(pw->batch->resv);
> +               kfree(pw);
> +               return err;
> +       }
> +
> +       err = i915_sw_fence_await_reservation(&pw->base.chain,
> +                                             pw->batch->resv, NULL, false,
> +                                             0, I915_FENCE_GFP);
> +       if (err < 0) {

Onion teardown to dedupe code.

> +               dma_resv_unlock(pw->batch->resv);
> +               kfree(pw);
> +               return err;
> +       }
> +
> +       dma_resv_add_shared_fence(pw->batch->resv, &pw->base.dma);
> +       dma_resv_unlock(pw->batch->resv);
> +
> +       dma_resv_lock(shadow->resv, NULL);
> +       dma_resv_add_excl_fence(shadow->resv, &pw->base.dma);
> +       dma_resv_unlock(shadow->resv);
> +
> +       dma_fence_work_commit(&pw->base);
> +       return 0;
> +}

After de-duping, I think this is just fine as far as the fences come.

Kernel wouldn't initiate any requests in need of cmd parsing and some
work needs to be waited upon to free memory, the cmdparser will fail
gracefully as the only allocation is __GFP_RETRY_MAYFAIL.

The rest looks fine to me, too. We probably want another set of eyes
to also ack the clflushing correctness.

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Regards, Joonas
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/8] drm/i915: Fix cmdparser drm.debug

[Jani Nikula]

On Sat, 07 Dec 2019, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> The cmdparser rejection debug is not for driver development, but for the
> user, for which we use a plain DRM_DEBUG().

...

> -	DRM_DEBUG_DRIVER("CMD: Abnormal rcs cmd length! 0x%08X\n", cmd_header);
> +	DRM_DEBUG("CMD: Abnormal rcs cmd length! 0x%08X\n", cmd_header);

That's not what the documentation says about the difference between
DRM_DEBUG and DRM_DEBUG_DRIVER.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/8] drm/i915: Fix cmdparser drm.debug

[Chris Wilson]

Quoting Jani Nikula (2020-01-02 09:56:05)
> On Sat, 07 Dec 2019, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > The cmdparser rejection debug is not for driver development, but for the
> > user, for which we use a plain DRM_DEBUG().
> 
> ...
> 
> > -     DRM_DEBUG_DRIVER("CMD: Abnormal rcs cmd length! 0x%08X\n", cmd_header);
> > +     DRM_DEBUG("CMD: Abnormal rcs cmd length! 0x%08X\n", cmd_header);
> 
> That's not what the documentation says about the difference between
> DRM_DEBUG and DRM_DEBUG_DRIVER.

The documentation seems to be a misconception.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/8] drm/i915: Fix cmdparser drm.debug

[Chris Wilson]

Quoting Jani Nikula (2020-01-02 09:56:05)
> On Sat, 07 Dec 2019, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > The cmdparser rejection debug is not for driver development, but for the
> > user, for which we use a plain DRM_DEBUG().
> 
> ...
> 
> > -     DRM_DEBUG_DRIVER("CMD: Abnormal rcs cmd length! 0x%08X\n", cmd_header);
> > +     DRM_DEBUG("CMD: Abnormal rcs cmd length! 0x%08X\n", cmd_header);
> 
> That's not what the documentation says about the difference between
> DRM_DEBUG and DRM_DEBUG_DRIVER.

Please note these have nothing to do with debugging the driver in any
form whatsoever.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/8] drm/i915: Fix cmdparser drm.debug

[Jani Nikula]

On Thu, 02 Jan 2020, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Quoting Jani Nikula (2020-01-02 09:56:05)
>> On Sat, 07 Dec 2019, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>> > The cmdparser rejection debug is not for driver development, but for the
>> > user, for which we use a plain DRM_DEBUG().
>> 
>> ...
>> 
>> > -     DRM_DEBUG_DRIVER("CMD: Abnormal rcs cmd length! 0x%08X\n", cmd_header);
>> > +     DRM_DEBUG("CMD: Abnormal rcs cmd length! 0x%08X\n", cmd_header);
>> 
>> That's not what the documentation says about the difference between
>> DRM_DEBUG and DRM_DEBUG_DRIVER.
>
> The documentation seems to be a misconception.

How so? DRM_DEBUG() translates to DRM_UT_CORE category, which has been
intended for "generic drm code" since the beginning:

4fefcb27050b ("drm: add separate drm debugging levels")
87fdff81cd2d ("DRM: Add the explanation about DRM debug level")

Because there's so much DRM_DEBUG() usage across drivers, I've named the
new drm_device specific logging macros drm_dbg_core() for DRM_UT_CORE
and drm_dbg() for DRM_UT_DRIVER, with the idea that drm_dbg_core() would
be used exclusively for drivers/gpu/drm/drm_*.[ch].

BR,
Jani.



-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx