From d5bd0e411ede67f5c56f95ae4d905d8d94c9be2f Mon Sep 17 00:00:00 2001 From: Jordan Justen Date: Tue, 28 Mar 2017 12:03:37 -0700 Subject: [PATCH] intel/gen_decoder: return -1 for unknown command formats 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 Acked-by: Lionel Landwerlin --- 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.30.2