[Intel-gfx] [PATCH] igt/gem_exec_parse: update for version 8 changes
Robert Bragg
robert at sixbynine.org
Fri Oct 21 19:38:51 UTC 2016
Just wanted to show how I'd been looking at updating gem_exec_parse considering
my interface change to stop returning EINVAL to userspace. I think maybe it'd
be good to split the patch up since I moved things around a bit, but hopefully
it's not too bad to skim for now.
I wasn't quite sure what to make of the recentl stray_lri change, since it
seems to accept returning EINVAL to userspace which for OACONTROL specifically
is a condition we ultimately want to be sure we don't allow because it's liable
to cause Mesa applications to abort.
The reason stray_lri should currently see an acceptable EINVAL is because
OACONTROL is is a very special case register for the command parser and
OACONTROL needs to be disabled before the end of a batch, but that's checked
for in oacontrol-tracking.
Something else I was unsure of with the stray_lri test is that it's using
0xdeadbeef as a debug value, which seems a bit risky for OACONTROL as it will
enable the OA unit with periodic sampling.
--- >8 ---
This adapts the tests to account for the parser no longer reporting
privilege violations back to userspace as EINVAL errors (they are left
to the HW command parser to squash the commands to NOOPS).
The interface change isn't expected to affect userspace and in fact it
looks like the previous behaviour was liable to break userspace, such as
Mesa which explicitly tries to observe whether OACONTROL LRIs are
squashed to NOOPs but Mesa will abort for execbuffer errors.
Signed-off-by: Robert Bragg <robert at sixbynine.org>
---
tests/gem_exec_parse.c | 368 +++++++++++++++++++++++++++----------------------
1 file changed, 200 insertions(+), 168 deletions(-)
diff --git a/tests/gem_exec_parse.c b/tests/gem_exec_parse.c
index 36bf57d..2bccecd 100644
--- a/tests/gem_exec_parse.c
+++ b/tests/gem_exec_parse.c
@@ -34,7 +34,24 @@
#define I915_PARAM_CMD_PARSER_VERSION 28
#endif
+#define ARRAY_LEN(A) (sizeof(A) / sizeof(A[0]))
+
+#define OACONTROL 0x2360
#define DERRMR 0x44050
+#define SO_WRITE_OFFSET_0 0x5280
+#define HSW_CS_GPR(n) (0x2600 + 8*(n))
+#define HSW_CS_GPR0 HSW_CS_GPR(0)
+#define HSW_CS_GPR1 HSW_CS_GPR(1)
+
+#define MI_LOAD_REGISTER_REG (0x2a << 23)
+#define MI_STORE_REGISTER_MEM (0x24 << 23)
+#define MI_ARB_ON_OFF (0x8 << 23)
+#define MI_DISPLAY_FLIP ((0x14 << 23) | 1)
+
+#define GFX_OP_PIPE_CONTROL ((0x3<<29)|(0x3<<27)|(0x2<<24)|2)
+#define PIPE_CONTROL_QW_WRITE (1<<14)
+#define PIPE_CONTROL_LRI_POST_OP (1<<23)
+
static int command_parser_version(int fd)
{
@@ -50,101 +67,8 @@ static int command_parser_version(int fd)
return -1;
}
-#define HSW_CS_GPR(n) (0x2600 + 8*(n))
-#define HSW_CS_GPR0 HSW_CS_GPR(0)
-#define HSW_CS_GPR1 HSW_CS_GPR(1)
-
-#define MI_LOAD_REGISTER_REG (0x2a << 23)
-#define MI_STORE_REGISTER_MEM (0x24 << 23)
-static void hsw_load_register_reg(void)
-{
- uint32_t buf[16] = {
- MI_LOAD_REGISTER_IMM | (5 - 2),
- HSW_CS_GPR0,
- 0xabcdabcd,
- HSW_CS_GPR1,
- 0xdeadbeef,
-
- MI_STORE_REGISTER_MEM | (3 - 2),
- HSW_CS_GPR1,
- 0, /* address0 */
-
- MI_LOAD_REGISTER_REG | (3 - 2),
- HSW_CS_GPR0,
- HSW_CS_GPR1,
-
- MI_STORE_REGISTER_MEM | (3 - 2),
- HSW_CS_GPR1,
- 4, /* address1 */
-
- MI_BATCH_BUFFER_END,
- };
- struct drm_i915_gem_execbuffer2 execbuf;
- struct drm_i915_gem_exec_object2 obj[2];
- struct drm_i915_gem_relocation_entry reloc[2];
- int fd;
-
- /* Open again to get a non-master file descriptor */
- fd = drm_open_driver(DRIVER_INTEL);
-
- igt_require(IS_HASWELL(intel_get_drm_devid(fd)));
- igt_require(command_parser_version(fd) >= 7);
-
- memset(obj, 0, sizeof(obj));
- obj[0].handle = gem_create(fd, 4096);
- obj[1].handle = gem_create(fd, 4096);
- gem_write(fd, obj[1].handle, 0, buf, sizeof(buf));
-
- memset(reloc, 0, sizeof(reloc));
- reloc[0].offset = 7*sizeof(uint32_t);
- reloc[0].target_handle = obj[0].handle;
- reloc[0].delta = 0;
- reloc[0].read_domains = I915_GEM_DOMAIN_INSTRUCTION;
- reloc[0].write_domain = I915_GEM_DOMAIN_INSTRUCTION;
- reloc[1].offset = 13*sizeof(uint32_t);
- reloc[1].target_handle = obj[0].handle;
- reloc[1].delta = sizeof(uint32_t);
- reloc[1].read_domains = I915_GEM_DOMAIN_INSTRUCTION;
- reloc[1].write_domain = I915_GEM_DOMAIN_INSTRUCTION;
- obj[1].relocs_ptr = (uintptr_t)&reloc;
- obj[1].relocation_count = 2;
-
- memset(&execbuf, 0, sizeof(execbuf));
- execbuf.buffers_ptr = (uintptr_t)obj;
- execbuf.buffer_count = 2;
- execbuf.batch_len = sizeof(buf);
- execbuf.flags = I915_EXEC_RENDER;
- gem_execbuf(fd, &execbuf);
- gem_close(fd, obj[1].handle);
-
- gem_read(fd, obj[0].handle, 0, buf, 2*sizeof(buf[0]));
- igt_assert_eq_u32(buf[0], 0xdeadbeef); /* before copy */
- igt_assert_eq_u32(buf[1], 0xabcdabcd); /* after copy */
-
- /* Now a couple of negative tests that should be filtered */
- execbuf.buffer_count = 1;
- execbuf.batch_len = 4*sizeof(buf[0]);
-
- buf[0] = MI_LOAD_REGISTER_REG | (3 - 2);
- buf[1] = HSW_CS_GPR0;
- buf[2] = 0;
- buf[3] = MI_BATCH_BUFFER_END;
- gem_write(fd, obj[0].handle, 0, buf, execbuf.batch_len);
- igt_assert_eq(__gem_execbuf(fd, &execbuf), -EINVAL);
-
- buf[2] = DERRMR; /* master only */
- gem_write(fd, obj[0].handle, 0, buf, execbuf.batch_len);
- igt_assert_eq(__gem_execbuf(fd, &execbuf), -EINVAL);
-
- buf[2] = 0x2038; /* RING_START: invalid */
- gem_write(fd, obj[0].handle, 0, buf, execbuf.batch_len);
- igt_assert_eq(__gem_execbuf(fd, &execbuf), -EINVAL);
-
- close(fd);
-}
-
-static void exec_batch_patched(int fd, uint32_t cmd_bo, uint32_t *cmds,
- int size, int patch_offset, uint64_t expected_value)
+static uint64_t __exec_batch_patched(int fd, uint32_t cmd_bo, uint32_t *cmds,
+ int size, int patch_offset)
{
struct drm_i915_gem_execbuffer2 execbuf;
struct drm_i915_gem_exec_object2 objs[2];
@@ -155,50 +79,43 @@ static void exec_batch_patched(int fd, uint32_t cmd_bo, uint32_t *cmds,
gem_write(fd, cmd_bo, 0, cmds, size);
- reloc[0].offset = patch_offset;
- reloc[0].delta = 0;
- reloc[0].target_handle = target_bo;
- reloc[0].read_domains = I915_GEM_DOMAIN_RENDER;
- reloc[0].write_domain = I915_GEM_DOMAIN_RENDER;
- reloc[0].presumed_offset = 0;
+ memset(objs, 0, sizeof(objs));
objs[0].handle = target_bo;
- objs[0].relocation_count = 0;
- objs[0].relocs_ptr = 0;
- objs[0].alignment = 0;
- objs[0].offset = 0;
- objs[0].flags = 0;
- objs[0].rsvd1 = 0;
- objs[0].rsvd2 = 0;
-
objs[1].handle = cmd_bo;
- objs[1].relocation_count = 1;
+
+ memset(reloc, 0, sizeof(reloc));
+ reloc[0].offset = patch_offset;
+ reloc[0].target_handle = objs[0].handle;
+ reloc[0].delta = 0;
+ reloc[0].read_domains = I915_GEM_DOMAIN_COMMAND;
+ reloc[0].write_domain = I915_GEM_DOMAIN_COMMAND;
objs[1].relocs_ptr = (uintptr_t)reloc;
- objs[1].alignment = 0;
- objs[1].offset = 0;
- objs[1].flags = 0;
- objs[1].rsvd1 = 0;
- objs[1].rsvd2 = 0;
+ objs[1].relocation_count = 1;
+ memset(&execbuf, 0, sizeof(execbuf));
execbuf.buffers_ptr = (uintptr_t)objs;
execbuf.buffer_count = 2;
- execbuf.batch_start_offset = 0;
execbuf.batch_len = size;
- execbuf.cliprects_ptr = 0;
- execbuf.num_cliprects = 0;
- execbuf.DR1 = 0;
- execbuf.DR4 = 0;
execbuf.flags = I915_EXEC_RENDER;
- i915_execbuffer2_set_context_id(execbuf, 0);
- execbuf.rsvd2 = 0;
gem_execbuf(fd, &execbuf);
gem_sync(fd, cmd_bo);
gem_read(fd,target_bo, 0, &actual_value, sizeof(actual_value));
- igt_assert_eq(expected_value, actual_value);
gem_close(fd, target_bo);
+
+ return actual_value;
+}
+
+static void exec_batch_patched(int fd, uint32_t cmd_bo, uint32_t *cmds,
+ int size, int patch_offset,
+ uint64_t expected_value)
+{
+ igt_assert_eq(__exec_batch_patched(fd, cmd_bo, cmds,
+ size, patch_offset),
+ expected_value);
}
static int __exec_batch(int fd, uint32_t cmd_bo, uint32_t *cmds,
@@ -320,14 +237,14 @@ static void exec_batch_chained(int fd, uint32_t cmd_bo, uint32_t *cmds,
reloc.offset = patch_offset;
reloc.delta = 0;
reloc.target_handle = target_bo;
- reloc.read_domains = I915_GEM_DOMAIN_RENDER;
- reloc.write_domain = I915_GEM_DOMAIN_RENDER;
+ reloc.read_domains = I915_GEM_DOMAIN_COMMAND;
+ reloc.write_domain = I915_GEM_DOMAIN_COMMAND;
reloc.presumed_offset = 0;
first_level_reloc.offset = 4;
first_level_reloc.delta = 0;
first_level_reloc.target_handle = cmd_bo;
- first_level_reloc.read_domains = I915_GEM_DOMAIN_INSTRUCTION;
+ first_level_reloc.read_domains = I915_GEM_DOMAIN_COMMAND;
first_level_reloc.write_domain = 0;
first_level_reloc.presumed_offset = 0;
@@ -380,15 +297,91 @@ static void exec_batch_chained(int fd, uint32_t cmd_bo, uint32_t *cmds,
gem_close(fd, target_bo);
}
-uint32_t handle;
-int fd;
+static void hsw_load_register_reg(void)
+{
+ uint32_t init_gpr0[16] = {
+ MI_LOAD_REGISTER_IMM | (3 - 2),
+ HSW_CS_GPR0,
+ 0xabcdabc0, /* leave [1:0] zero */
+ MI_BATCH_BUFFER_END,
+ };
+ uint32_t store_gpr0[16] = {
+ MI_STORE_REGISTER_MEM | (3 - 2),
+ HSW_CS_GPR0,
+ 0, /* reloc*/
+ MI_BATCH_BUFFER_END,
+ };
+ uint32_t do_lrr[16] = {
+ MI_LOAD_REGISTER_REG | (3 - 2),
+ 0, /* [1] = src */
+ HSW_CS_GPR0, /* dst */
+ MI_BATCH_BUFFER_END,
+ };
+ uint32_t allowed_regs[] = {
+ HSW_CS_GPR1,
+ SO_WRITE_OFFSET_0,
+ };
+ uint32_t disallowed_regs[] = {
+ 0,
+ DERRMR, /* master only */
+ 0x2038, /* RING_START: invalid */
+ };
+ int fd;
+ uint32_t handle;
-#define MI_ARB_ON_OFF (0x8 << 23)
-#define MI_DISPLAY_FLIP ((0x14 << 23) | 1)
+ /* Open again to get a non-master file descriptor */
+ fd = drm_open_driver(DRIVER_INTEL);
+
+ igt_require(IS_HASWELL(intel_get_drm_devid(fd)));
+ igt_require(command_parser_version(fd) >= 7);
+
+ handle = gem_create(fd, 4096);
+
+ for (int i = 0 ; i < ARRAY_LEN(allowed_regs); i++) {
+ uint32_t var;
+
+ exec_batch(fd, handle, init_gpr0, sizeof(init_gpr0),
+ I915_EXEC_RENDER,
+ 0);
+ exec_batch_patched(fd, handle,
+ store_gpr0, sizeof(store_gpr0),
+ 2 * sizeof(uint32_t), /* reloc */
+ 0xabcdabc0);
+ do_lrr[1] = allowed_regs[i];
+ exec_batch(fd, handle, do_lrr, sizeof(do_lrr),
+ I915_EXEC_RENDER,
+ 0);
+ var = __exec_batch_patched(fd, handle,
+ store_gpr0, sizeof(store_gpr0),
+ 2 * sizeof(uint32_t)); /* reloc */
+ igt_assert_neq(var, 0xabcdabc0);
+
+ }
+
+ for (int i = 0 ; i < ARRAY_LEN(disallowed_regs); i++) {
+ exec_batch(fd, handle, init_gpr0, sizeof(init_gpr0),
+ I915_EXEC_RENDER,
+ 0);
+ exec_batch_patched(fd, handle,
+ store_gpr0, sizeof(store_gpr0),
+ 2 * sizeof(uint32_t), /* reloc */
+ 0xabcdabc0);
+ do_lrr[1] = disallowed_regs[i];
+ exec_batch(fd, handle, do_lrr, sizeof(do_lrr),
+ I915_EXEC_RENDER,
+ 0);
+ exec_batch_patched(fd, handle,
+ store_gpr0, sizeof(store_gpr0),
+ 2 * sizeof(uint32_t), /* reloc */
+ 0xabcdabc0);
+ }
+
+ close(fd);
+}
-#define GFX_OP_PIPE_CONTROL ((0x3<<29)|(0x3<<27)|(0x2<<24)|2)
-#define PIPE_CONTROL_QW_WRITE (1<<14)
-#define PIPE_CONTROL_LRI_POST_OP (1<<23)
+
+uint32_t handle;
+int fd;
igt_main
{
@@ -428,57 +421,93 @@ igt_main
}
igt_subtest("basic-rejected") {
- uint32_t arb_on_off[] = {
- MI_ARB_ON_OFF,
+ uint32_t invalid_cmd[] = {
+ (0x7<<29), /* Reserved command type,
+ across all engines */
MI_BATCH_BUFFER_END,
};
- uint32_t display_flip[] = {
- MI_DISPLAY_FLIP,
- 0, 0, 0,
+ uint32_t invalid_set_context[] = {
+ MI_SET_CONTEXT | 32, /* invalid length */
MI_BATCH_BUFFER_END,
- 0
};
exec_batch(fd, handle,
- arb_on_off, sizeof(arb_on_off),
+ invalid_cmd, sizeof(invalid_cmd),
I915_EXEC_RENDER,
-EINVAL);
exec_batch(fd, handle,
- arb_on_off, sizeof(arb_on_off),
+ invalid_cmd, sizeof(invalid_cmd),
I915_EXEC_BSD,
-EINVAL);
+ exec_batch(fd, handle,
+ invalid_cmd, sizeof(invalid_cmd),
+ I915_EXEC_BLT,
+ -EINVAL);
if (gem_has_vebox(fd)) {
- exec_batch(fd, handle,
- arb_on_off, sizeof(arb_on_off),
- I915_EXEC_VEBOX,
- -EINVAL);
+ exec_batch(fd, handle,
+ invalid_cmd, sizeof(invalid_cmd),
+ I915_EXEC_BLT,
+ -EINVAL);
}
+
exec_batch(fd, handle,
- display_flip, sizeof(display_flip),
- I915_EXEC_BLT,
+ invalid_set_context, sizeof(invalid_set_context),
+ I915_EXEC_RENDER,
-EINVAL);
}
igt_subtest("registers") {
- uint32_t lri_bad[] = {
- MI_LOAD_REGISTER_IMM,
- 0, /* disallowed register address */
- 0x12000000,
- MI_BATCH_BUFFER_END,
- };
+ uint32_t lri_bad[] = {
+ MI_LOAD_REGISTER_IMM,
+ OACONTROL, /* disallowed register address */
+ 0x31337000,
+ MI_BATCH_BUFFER_END,
+ };
uint32_t lri_ok[] = {
- MI_LOAD_REGISTER_IMM,
- 0x5280, /* allowed register address (SO_WRITE_OFFSET[0]) */
- 0x1,
+ MI_LOAD_REGISTER_IMM | (3 - 2),
+ SO_WRITE_OFFSET_0, /* allowed register address */
+ 0xabcdabc0, /* [1:0] MBZ */
MI_BATCH_BUFFER_END,
};
- exec_batch(fd, handle,
- lri_bad, sizeof(lri_bad),
- I915_EXEC_RENDER,
- -EINVAL);
- exec_batch(fd, handle,
- lri_ok, sizeof(lri_ok),
- I915_EXEC_RENDER,
- 0);
+ uint32_t store_reg[] = {
+ MI_STORE_REGISTER_MEM | (3 - 2),
+ 0, /* reg */
+ 0, /* reloc */
+ MI_BATCH_BUFFER_END,
+ };
+ uint64_t val;
+
+ /* Note: we intentionally pick OACONTROL as the disallowed
+ * register to test here since it used to be white listed by
+ * the command parser.
+ *
+ * It's especially important to check that an LRI to OACONTROL
+ * doesn't result in an EINVAL error because Mesa attempts
+ * writing to OACONTROL to determine what extensions to expose
+ * and will abort() for execbuffer() errors.
+ *
+ * Mesa can gracefully recognise and handle the LRI becoming
+ * a NOOP.
+ */
+ exec_batch(fd, handle,
+ lri_bad, sizeof(lri_bad),
+ I915_EXEC_RENDER,
+ 0);
+ store_reg[1] = OACONTROL;
+ val = __exec_batch_patched(fd, handle,
+ store_reg, sizeof(store_reg),
+ 8); /* reloc offset */
+ igt_assert_neq(val, 0x31337000);
+
+ exec_batch(fd, handle,
+ lri_ok, sizeof(lri_ok),
+ I915_EXEC_RENDER,
+ 0);
+ store_reg[1] = SO_WRITE_OFFSET_0;
+ exec_batch_patched(fd, handle,
+ store_reg,
+ sizeof(store_reg),
+ 2 * sizeof(uint32_t), /* reloc offset */
+ 0xabcdabc0);
}
igt_subtest("bitmasks") {
@@ -491,10 +520,13 @@ igt_main
0,
MI_BATCH_BUFFER_END,
};
- exec_batch(fd, handle,
- pc, sizeof(pc),
- I915_EXEC_RENDER,
- -EINVAL);
+ /* Expect to read back zero since the command should be
+ * squashed to a NOOP
+ */
+ exec_batch_patched(fd, handle,
+ pc, sizeof(pc),
+ 8, /* patch offset, */
+ 0x0);
}
igt_subtest("batch-without-end") {
--
2.10.0
More information about the Intel-gfx
mailing list