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 <rob...@sixbynine.org>
---
 tests/gem_exec_parse.c | 95 +++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 74 insertions(+), 21 deletions(-)

diff --git a/tests/gem_exec_parse.c b/tests/gem_exec_parse.c
index 1c09cf2..c98f809 100644
--- a/tests/gem_exec_parse.c
+++ b/tests/gem_exec_parse.c
@@ -37,6 +37,7 @@
 #define ARRAY_LEN(A) (sizeof(A) / sizeof(A[0]))
 
 #define DERRMR 0x44050
+#define OACONTROL 0x2360
 #define SO_WRITE_OFFSET_0 0x5280
 
 #define HSW_CS_GPR(n) (0x2600 + 8*(n))
@@ -310,10 +311,21 @@ static void hsw_load_register_reg(void)
        }
 
        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,
-                          -EINVAL);
+                          0);
+               exec_batch_patched(fd, handle,
+                                  store_gpr0, sizeof(store_gpr0),
+                                  2 * sizeof(uint32_t), /* reloc */
+                                  0xabcdabc0);
        }
 
        close(fd);
@@ -360,57 +372,95 @@ 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);
+               if (gem_has_blt(fd)) {
+                       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),
+                                  invalid_cmd, sizeof(invalid_cmd),
                                   I915_EXEC_VEBOX,
                                   -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,
+                       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,
+                       SO_WRITE_OFFSET_0, /* allowed register address */
+                       0xabcdabc0, /* [1:0] MBZ */
                        MI_BATCH_BUFFER_END,
                };
+               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,
-                          -EINVAL);
+                          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") {
@@ -423,10 +473,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.1

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

Reply via email to