[Mesa-dev] [PATCH v2 3/4] intel/gen_decoder: return -1 for unknown command formats

[Jordan Justen]

Decoding with aubinator encountered a command of 0xffffffff. With the
previous code, it caused aubinator to jump 255 + 2 dwords to start
decoding again.

Instead we can attempt to detect the known instruction formats. If the
format is not recognized, then we can advance just 1 dword.

v2:
 * Update aubinator_error_decode
 * Actually convert the length variable returned into a *signed* integer
   in aubinator.c, intel_batchbuffer.c and aubinator_error_decode.c.

Signed-off-by: Jordan Justen <jordan.l.justen at intel.com>
Acked-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
---
 src/intel/common/gen_decoder.c                | 22 +++++++++++++++-------
 src/intel/tools/aubinator.c                   |  7 ++++---
 src/intel/tools/aubinator_error_decode.c      |  7 ++++---
 src/mesa/drivers/dri/i965/intel_batchbuffer.c |  8 ++++----
 4 files changed, 27 insertions(+), 17 deletions(-)

diff --git a/src/intel/common/gen_decoder.c b/src/intel/common/gen_decoder.c
index 1244f4c4480..03c9c7f8d46 100644
--- a/src/intel/common/gen_decoder.c
+++ b/src/intel/common/gen_decoder.c
@@ -692,28 +692,36 @@ gen_group_get_length(struct gen_group *group, const uint32_t *p)
 
    case 3: /* Render */ {
       uint32_t subtype = field(h, 27, 28);
+      uint32_t opcode = field(h, 24, 26);
       switch (subtype) {
       case 0:
-         return field(h, 0, 7) + 2;
+         if (opcode < 2)
+            return field(h, 0, 7) + 2;
+         else
+            return -1;
       case 1:
-         return 1;
+         if (opcode < 2)
+            return 1;
+         else
+            return -1;
       case 2: {
-         uint32_t opcode = field(h, 24, 26);
-         assert(opcode < 3 /* 3 and above currently reserved */);
          if (opcode == 0)
             return field(h, 0, 7) + 2;
          else if (opcode < 3)
             return field(h, 0, 15) + 2;
          else
-            return 1; /* FIXME: if more opcodes are added */
+            return -1;
       }
       case 3:
-         return field(h, 0, 7) + 2;
+         if (opcode < 4)
+            return field(h, 0, 7) + 2;
+         else
+            return -1;
       }
    }
    }
 
-   unreachable("bad opcode");
+   return -1;
 }
 
 void
diff --git a/src/intel/tools/aubinator.c b/src/intel/tools/aubinator.c
index cae578babac..05d932ea6c7 100644
--- a/src/intel/tools/aubinator.c
+++ b/src/intel/tools/aubinator.c
@@ -682,17 +682,18 @@ static void
 parse_commands(struct gen_spec *spec, uint32_t *cmds, int size, int engine)
 {
    uint32_t *p, *end = cmds + size / 4;
-   unsigned int length, i;
+   int length, i;
    struct gen_group *inst;
 
    for (p = cmds; p < end; p += length) {
       inst = gen_spec_find_instruction(spec, p);
+      length = gen_group_get_length(inst, p);
+      assert(inst == NULL || length > 0);
+      length = MAX2(1, length);
       if (inst == NULL) {
          fprintf(outfile, "unknown instruction %08x\n", p[0]);
-         length = (p[0] & 0xff) + 2;
          continue;
       }
-      length = gen_group_get_length(inst, p);
 
       const char *color, *reset_color = NORMAL;
       uint64_t offset;
diff --git a/src/intel/tools/aubinator_error_decode.c b/src/intel/tools/aubinator_error_decode.c
index 1bdab00a663..2e623698ed1 100644
--- a/src/intel/tools/aubinator_error_decode.c
+++ b/src/intel/tools/aubinator_error_decode.c
@@ -217,7 +217,7 @@ static void decode(struct gen_spec *spec,
                    int *count)
 {
    uint32_t *p, *end = (data + *count);
-   unsigned int length;
+   int length;
    struct gen_group *inst;
 
    for (p = data; p < end; p += length) {
@@ -226,9 +226,11 @@ static void decode(struct gen_spec *spec,
       uint64_t offset = gtt_offset + 4 * (p - data);
 
       inst = gen_spec_find_instruction(spec, p);
+      length = gen_group_get_length(inst, p);
+      assert(inst == NULL || length > 0);
+      length = MAX2(1, length);
       if (inst == NULL) {
          printf("unknown instruction %08x\n", p[0]);
-         length = (p[0] & 0xff) + 2;
          continue;
       }
       if (option_color == COLOR_NEVER) {
@@ -241,7 +243,6 @@ static void decode(struct gen_spec *spec,
 
       gen_print_group(stdout, inst, offset, p,
                       option_color == COLOR_ALWAYS);
-      length = gen_group_get_length(inst, p);
    }
 }
 
diff --git a/src/mesa/drivers/dri/i965/intel_batchbuffer.c b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
index 54bab9efb02..54777d1c34f 100644
--- a/src/mesa/drivers/dri/i965/intel_batchbuffer.c
+++ b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
@@ -220,7 +220,7 @@ do_batch_dump(struct brw_context *brw)
    uint32_t *data = batch->bo->virtual ? batch->bo->virtual : batch->map;
    uint32_t *end = data + USED_BATCH(*batch);
    uint32_t gtt_offset = batch->bo->virtual ? batch->bo->offset64 : 0;
-   unsigned int length;
+   int length;
 
    bool color = INTEL_DEBUG & DEBUG_COLOR;
    const char *header_color = color ? BLUE_HEADER : "";
@@ -228,9 +228,11 @@ do_batch_dump(struct brw_context *brw)
 
    for (uint32_t *p = data; p < end; p += length) {
       struct gen_group *inst = gen_spec_find_instruction(spec, p);
+      length = gen_group_get_length(inst, p);
+      assert(inst == NULL || length > 0);
+      length = MAX2(1, length);
       if (inst == NULL) {
          fprintf(stderr, "unknown instruction %08x\n", p[0]);
-         length = (p[0] & 0xff) + 2;
          continue;
       }
 
@@ -316,8 +318,6 @@ do_batch_dump(struct brw_context *brw)
                        gtt_offset, p[1] & ~0x3fu, color);
          break;
       }
-
-      length = gen_group_get_length(inst, p);
    }
 
    if (ret == 0) {
-- 
2.11.0