[CI resend 1/2] drm/i915: compile-time consistency check on __EXEC_OBJECT flags

[CI resend 1/2] drm/i915: compile-time consistency check on __EXEC_OBJECT flags

[Dave Gordon]

Two different sets of flag bits are stored in the 'flags' member of a
'struct drm_i915_gem_exec_object2', and they're defined in two different
source files, increasing the risk of an accidental clash.

Some flags in this field are supplied by the user; these are defined in
i915_drm.h, and they start from the LSB and work up.

Other flags are defined in i915_gem_execbuffer, for internal use within
that file only; they start from the MSB and work down.

So here we add a compile-time check that the two sets of flags do not
overlap, which would cause all sorts of confusion.

Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c | 12 ++++++++----
 include/uapi/drm/i915_drm.h                | 11 ++++++-----
 2 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 1978633..1bb1f25 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -34,10 +34,11 @@
 #include <linux/dma_remapping.h>
 #include <linux/uaccess.h>
 
-#define  __EXEC_OBJECT_HAS_PIN (1<<31)
-#define  __EXEC_OBJECT_HAS_FENCE (1<<30)
-#define  __EXEC_OBJECT_NEEDS_MAP (1<<29)
-#define  __EXEC_OBJECT_NEEDS_BIAS (1<<28)
+#define  __EXEC_OBJECT_HAS_PIN		(1<<31)
+#define  __EXEC_OBJECT_HAS_FENCE	(1<<30)
+#define  __EXEC_OBJECT_NEEDS_MAP	(1<<29)
+#define  __EXEC_OBJECT_NEEDS_BIAS	(1<<28)
+#define  __EXEC_OBJECT_INTERNAL_FLAGS (0xf<<28) /* all of the above */
 
 #define BATCH_OFFSET_BIAS (256*1024)
 
@@ -1007,6 +1008,9 @@ static bool only_mappable_for_reloc(unsigned int flags)
 	unsigned invalid_flags;
 	int i;
 
+	/* INTERNAL flags must not overlap with external ones */
+	BUILD_BUG_ON(__EXEC_OBJECT_INTERNAL_FLAGS & ~__EXEC_OBJECT_UNKNOWN_FLAGS);
+
 	invalid_flags = __EXEC_OBJECT_UNKNOWN_FLAGS;
 	if (USES_FULL_PPGTT(dev))
 		invalid_flags |= EXEC_OBJECT_NEEDS_GTT;
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index d7e81a3..51b9360 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -698,12 +698,13 @@ struct drm_i915_gem_exec_object2 {
 	 */
 	__u64 offset;
 
-#define EXEC_OBJECT_NEEDS_FENCE (1<<0)
-#define EXEC_OBJECT_NEEDS_GTT	(1<<1)
-#define EXEC_OBJECT_WRITE	(1<<2)
+#define EXEC_OBJECT_NEEDS_FENCE		 (1<<0)
+#define EXEC_OBJECT_NEEDS_GTT		 (1<<1)
+#define EXEC_OBJECT_WRITE		 (1<<2)
 #define EXEC_OBJECT_SUPPORTS_48B_ADDRESS (1<<3)
-#define EXEC_OBJECT_PINNED	(1<<4)
-#define __EXEC_OBJECT_UNKNOWN_FLAGS -(EXEC_OBJECT_PINNED<<1)
+#define EXEC_OBJECT_PINNED		 (1<<4)
+/* All remaining bits are MBZ and RESERVED FOR FUTURE USE */
+#define __EXEC_OBJECT_UNKNOWN_FLAGS	(-(EXEC_OBJECT_PINNED<<1))
 	__u64 flags;
 
 	__u64 rsvd1;
-- 
1.9.1

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

[CI resend 2/2] drm/i915: refactor eb_get_batch()

[Dave Gordon]

Precursor for fix to secure batch execution. We will need to be able to
retrieve the batch VMA (as well as the batch itself) from the eb list,
so this patch extracts that part of eb_get_batch() into a separate
function, and moves both parts to a more logical place in the file, near
where the eb list is created.

Also, it may not be obvious, but the current execbuffer2 ioctl interface
requires that the buffer object containing the batch-to-be-executed be
the LAST entry in the exec2_list[] array (I expected it to be the
first!).

To clarify this, we can replace the rather obscure construct
	"list_entry(eb->vmas.prev, ...)"
in the old version of eb_get_batch() with the equivalent but more
explicit
	"list_last_entry(&eb->vmas,...)"
in the new eb_get_batch_vma() and of course add an explanatory comment.

Signed-off-by: Dave Gordon <david.s.gordon@intel.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c | 49 ++++++++++++++++++------------
 1 file changed, 29 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 1bb1f25..f6724ae 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -186,6 +186,35 @@ struct eb_vmas {
 	return ret;
 }
 
+static inline struct i915_vma *
+eb_get_batch_vma(struct eb_vmas *eb)
+{
+	/* The batch is always the LAST item in the VMA list */
+	struct i915_vma *vma = list_last_entry(&eb->vmas, typeof(*vma), exec_list);
+
+	return vma;
+}
+
+static struct drm_i915_gem_object *
+eb_get_batch(struct eb_vmas *eb)
+{
+	struct i915_vma *vma = eb_get_batch_vma(eb);
+
+	/*
+	 * SNA is doing fancy tricks with compressing batch buffers, which leads
+	 * to negative relocation deltas. Usually that works out ok since the
+	 * relocate address is still positive, except when the batch is placed
+	 * very low in the GTT. Ensure this doesn't happen.
+	 *
+	 * Note that actual hangs have only been observed on gen7, but for
+	 * paranoia do it everywhere.
+	 */
+	if ((vma->exec_entry->flags & EXEC_OBJECT_PINNED) == 0)
+		vma->exec_entry->flags |= __EXEC_OBJECT_NEEDS_BIAS;
+
+	return vma->obj;
+}
+
 static struct i915_vma *eb_get_vma(struct eb_vmas *eb, unsigned long handle)
 {
 	if (eb->and < 0) {
@@ -1341,26 +1370,6 @@ static bool only_mappable_for_reloc(unsigned int flags)
 	return file_priv->bsd_ring;
 }
 
-static struct drm_i915_gem_object *
-eb_get_batch(struct eb_vmas *eb)
-{
-	struct i915_vma *vma = list_entry(eb->vmas.prev, typeof(*vma), exec_list);
-
-	/*
-	 * SNA is doing fancy tricks with compressing batch buffers, which leads
-	 * to negative relocation deltas. Usually that works out ok since the
-	 * relocate address is still positive, except when the batch is placed
-	 * very low in the GTT. Ensure this doesn't happen.
-	 *
-	 * Note that actual hangs have only been observed on gen7, but for
-	 * paranoia do it everywhere.
-	 */
-	if ((vma->exec_entry->flags & EXEC_OBJECT_PINNED) == 0)
-		vma->exec_entry->flags |= __EXEC_OBJECT_NEEDS_BIAS;
-
-	return vma->obj;
-}
-
 #define I915_USER_RINGS (4)
 
 static const enum intel_engine_id user_ring_map[I915_USER_RINGS + 1] = {
-- 
1.9.1

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

✗ Ro.CI.BAT: failure for series starting with [CI,resend,1/2] drm/i915: compile-time consistency check on __EXEC_OBJECT flags

[Patchwork]

== Series Details ==

Series: series starting with [CI,resend,1/2] drm/i915: compile-time consistency check on __EXEC_OBJECT flags
URL   : https://patchwork.freedesktop.org/series/9876/
State : failure

== Summary ==

Series 9876v1 Series without cover letter
http://patchwork.freedesktop.org/api/1.0/series/9876/revisions/1/mbox

Test drv_module_reload_basic:
                skip       -> PASS       (ro-ivb-i7-3770)
Test kms_cursor_legacy:
        Subgroup basic-flip-vs-cursor:
                dmesg-warn -> PASS       (ro-byt-n2820)
Test kms_pipe_crc_basic:
        Subgroup nonblocking-crc-pipe-b:
                skip       -> PASS       (fi-skl-i5-6260u)
        Subgroup read-crc-pipe-c:
                pass       -> SKIP       (fi-skl-i5-6260u)
        Subgroup suspend-read-crc-pipe-a:
                pass       -> INCOMPLETE (fi-skl-i7-6700k)
Test vgem_basic:
        Subgroup debugfs:
                incomplete -> PASS       (ro-snb-i7-2620M)

fi-kbl-qkkr      total:241  pass:174  dwarn:29  dfail:1   fail:6   skip:31 
fi-skl-i5-6260u  total:241  pass:217  dwarn:0   dfail:0   fail:7   skip:17 
fi-skl-i7-6700k  total:195  pass:170  dwarn:0   dfail:0   fail:0   skip:24 
fi-snb-i7-2600   total:241  pass:190  dwarn:0   dfail:0   fail:7   skip:44 
ro-bdw-i5-5250u  total:241  pass:213  dwarn:4   dfail:0   fail:7   skip:17 
ro-bdw-i7-5557U  total:241  pass:213  dwarn:1   dfail:0   fail:7   skip:20 
ro-bdw-i7-5600u  total:241  pass:199  dwarn:0   dfail:0   fail:7   skip:35 
ro-byt-n2820     total:241  pass:191  dwarn:0   dfail:0   fail:8   skip:42 
ro-hsw-i3-4010u  total:241  pass:206  dwarn:0   dfail:0   fail:7   skip:28 
ro-hsw-i7-4770r  total:241  pass:206  dwarn:0   dfail:0   fail:7   skip:28 
ro-ilk-i7-620lm  total:241  pass:166  dwarn:0   dfail:0   fail:8   skip:67 
ro-ilk1-i5-650   total:236  pass:166  dwarn:0   dfail:0   fail:8   skip:62 
ro-ivb-i7-3770   total:241  pass:197  dwarn:0   dfail:0   fail:7   skip:37 
ro-skl3-i5-6260u total:241  pass:217  dwarn:1   dfail:0   fail:7   skip:16 
ro-snb-i7-2620M  total:241  pass:188  dwarn:0   dfail:0   fail:8   skip:45 

Results at /archive/results/CI_IGT_test/RO_Patchwork_1493/

c01b445 drm-intel-nightly: 2016y-07m-15d-07h-02m-07s UTC integration manifest
ce44917 drm/i915: refactor eb_get_batch()
46acffb drm/i915: compile-time consistency check on __EXEC_OBJECT flags

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

Re: ✗ Ro.CI.BAT: failure for series starting with [CI,resend,1/2] drm/i915: compile-time consistency check on __EXEC_OBJECT flags

[Dave Gordon]

On 15/07/16 09:15, Patchwork wrote:
> == Series Details ==
>
> Series: series starting with [CI,resend,1/2] drm/i915: compile-time consistency check on __EXEC_OBJECT flags
> URL   : https://patchwork.freedesktop.org/series/9876/
> State : failure
>
> == Summary ==
>
> Series 9876v1 Series without cover letter
> http://patchwork.freedesktop.org/api/1.0/series/9876/revisions/1/mbox
>
> Test drv_module_reload_basic:
>                  skip       -> PASS       (ro-ivb-i7-3770)
> Test kms_cursor_legacy:
>          Subgroup basic-flip-vs-cursor:
>                  dmesg-warn -> PASS       (ro-byt-n2820)
> Test kms_pipe_crc_basic:
>          Subgroup nonblocking-crc-pipe-b:
>                  skip       -> PASS       (fi-skl-i5-6260u)
>          Subgroup read-crc-pipe-c:
>                  pass       -> SKIP       (fi-skl-i5-6260u)

"No connector found for pipe 2"
See https://bugs.freedesktop.org/show_bug.cgi?id=93769

.Dave.

>          Subgroup suspend-read-crc-pipe-a:
>                  pass       -> INCOMPLETE (fi-skl-i7-6700k)
> Test vgem_basic:
>          Subgroup debugfs:
>                  incomplete -> PASS       (ro-snb-i7-2620M)
>
> fi-kbl-qkkr      total:241  pass:174  dwarn:29  dfail:1   fail:6   skip:31
> fi-skl-i5-6260u  total:241  pass:217  dwarn:0   dfail:0   fail:7   skip:17
> fi-skl-i7-6700k  total:195  pass:170  dwarn:0   dfail:0   fail:0   skip:24
> fi-snb-i7-2600   total:241  pass:190  dwarn:0   dfail:0   fail:7   skip:44
> ro-bdw-i5-5250u  total:241  pass:213  dwarn:4   dfail:0   fail:7   skip:17
> ro-bdw-i7-5557U  total:241  pass:213  dwarn:1   dfail:0   fail:7   skip:20
> ro-bdw-i7-5600u  total:241  pass:199  dwarn:0   dfail:0   fail:7   skip:35
> ro-byt-n2820     total:241  pass:191  dwarn:0   dfail:0   fail:8   skip:42
> ro-hsw-i3-4010u  total:241  pass:206  dwarn:0   dfail:0   fail:7   skip:28
> ro-hsw-i7-4770r  total:241  pass:206  dwarn:0   dfail:0   fail:7   skip:28
> ro-ilk-i7-620lm  total:241  pass:166  dwarn:0   dfail:0   fail:8   skip:67
> ro-ilk1-i5-650   total:236  pass:166  dwarn:0   dfail:0   fail:8   skip:62
> ro-ivb-i7-3770   total:241  pass:197  dwarn:0   dfail:0   fail:7   skip:37
> ro-skl3-i5-6260u total:241  pass:217  dwarn:1   dfail:0   fail:7   skip:16
> ro-snb-i7-2620M  total:241  pass:188  dwarn:0   dfail:0   fail:8   skip:45
>
> Results at /archive/results/CI_IGT_test/RO_Patchwork_1493/
>
> c01b445 drm-intel-nightly: 2016y-07m-15d-07h-02m-07s UTC integration manifest
> ce44917 drm/i915: refactor eb_get_batch()
> 46acffb drm/i915: compile-time consistency check on __EXEC_OBJECT flags
>

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

Re: ✗ Ro.CI.BAT: failure for series starting with [CI,resend,1/2] drm/i915: compile-time consistency check on __EXEC_OBJECT flags

[Daniel Vetter]

On Fri, Jul 15, 2016 at 09:52:27AM +0100, Dave Gordon wrote:
> On 15/07/16 09:15, Patchwork wrote:
> > == Series Details ==
> > 
> > Series: series starting with [CI,resend,1/2] drm/i915: compile-time consistency check on __EXEC_OBJECT flags
> > URL   : https://patchwork.freedesktop.org/series/9876/
> > State : failure
> > 
> > == Summary ==
> > 
> > Series 9876v1 Series without cover letter
> > http://patchwork.freedesktop.org/api/1.0/series/9876/revisions/1/mbox
> > 
> > Test drv_module_reload_basic:
> >                  skip       -> PASS       (ro-ivb-i7-3770)
> > Test kms_cursor_legacy:
> >          Subgroup basic-flip-vs-cursor:
> >                  dmesg-warn -> PASS       (ro-byt-n2820)
> > Test kms_pipe_crc_basic:
> >          Subgroup nonblocking-crc-pipe-b:
> >                  skip       -> PASS       (fi-skl-i5-6260u)
> >          Subgroup read-crc-pipe-c:
> >                  pass       -> SKIP       (fi-skl-i5-6260u)
> 
> "No connector found for pipe 2"
> See https://bugs.freedesktop.org/show_bug.cgi?id=93769

Applied, thanks for the patches.
-Daniel

> 
> .Dave.
> 
> >          Subgroup suspend-read-crc-pipe-a:
> >                  pass       -> INCOMPLETE (fi-skl-i7-6700k)
> > Test vgem_basic:
> >          Subgroup debugfs:
> >                  incomplete -> PASS       (ro-snb-i7-2620M)
> > 
> > fi-kbl-qkkr      total:241  pass:174  dwarn:29  dfail:1   fail:6   skip:31
> > fi-skl-i5-6260u  total:241  pass:217  dwarn:0   dfail:0   fail:7   skip:17
> > fi-skl-i7-6700k  total:195  pass:170  dwarn:0   dfail:0   fail:0   skip:24
> > fi-snb-i7-2600   total:241  pass:190  dwarn:0   dfail:0   fail:7   skip:44
> > ro-bdw-i5-5250u  total:241  pass:213  dwarn:4   dfail:0   fail:7   skip:17
> > ro-bdw-i7-5557U  total:241  pass:213  dwarn:1   dfail:0   fail:7   skip:20
> > ro-bdw-i7-5600u  total:241  pass:199  dwarn:0   dfail:0   fail:7   skip:35
> > ro-byt-n2820     total:241  pass:191  dwarn:0   dfail:0   fail:8   skip:42
> > ro-hsw-i3-4010u  total:241  pass:206  dwarn:0   dfail:0   fail:7   skip:28
> > ro-hsw-i7-4770r  total:241  pass:206  dwarn:0   dfail:0   fail:7   skip:28
> > ro-ilk-i7-620lm  total:241  pass:166  dwarn:0   dfail:0   fail:8   skip:67
> > ro-ilk1-i5-650   total:236  pass:166  dwarn:0   dfail:0   fail:8   skip:62
> > ro-ivb-i7-3770   total:241  pass:197  dwarn:0   dfail:0   fail:7   skip:37
> > ro-skl3-i5-6260u total:241  pass:217  dwarn:1   dfail:0   fail:7   skip:16
> > ro-snb-i7-2620M  total:241  pass:188  dwarn:0   dfail:0   fail:8   skip:45
> > 
> > Results at /archive/results/CI_IGT_test/RO_Patchwork_1493/
> > 
> > c01b445 drm-intel-nightly: 2016y-07m-15d-07h-02m-07s UTC integration manifest
> > ce44917 drm/i915: refactor eb_get_batch()
> > 46acffb drm/i915: compile-time consistency check on __EXEC_OBJECT flags
> > 
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx