[v2,5/5] target/tricore: Fix tricore_tr_translate_insn

Commit Message

Bastian Koppelmann Aug. 21, 2019, 12:23 p.m. UTC

we now fetch 2 bytes first, check whether we have a 32 bit insn, and only then
fetch another 2 bytes. We also make sure that a 16 bit insn that still fits
into the current page does not end up in the next page.

Signed-off-by: Bastian Koppelmann <[e-mail]>
---
 target/tricore/translate.c | 47 +++++++++++++++++++++++++++-----------
 1 file changed, 34 insertions(+), 13 deletions(-)

Comments

Richard Henderson Aug. 21, 2019, 10:54 p.m. UTC | #1

On 8/21/19 5:23 AM, Bastian Koppelmann wrote:
> we now fetch 2 bytes first, check whether we have a 32 bit insn, and only then
> fetch another 2 bytes. We also make sure that a 16 bit insn that still fits
> into the current page does not end up in the next page.
> 
> Signed-off-by: Bastian Koppelmann <[e-mail]>
> ---
>  target/tricore/translate.c | 47 +++++++++++++++++++++++++++-----------
>  1 file changed, 34 insertions(+), 13 deletions(-)

Reviewed-by: Richard Henderson <[e-mail]>


r~

Patch

diff --git a/target/tricore/translate.c b/target/tricore/translate.c
index 19a0e4554c..3ffcf0440e 100644
--- a/target/tricore/translate.c
+++ b/target/tricore/translate.c
@@ -8781,17 +8781,9 @@  static void decode_32Bit_opc(DisasContext *ctx)
     }
 }
 
-static void decode_opc(DisasContext *ctx)
+static bool tricore_insn_is_16bit(uint32_t insn)
 {
-    /* 16-Bit Instruction */
-    if ((ctx->opcode & 0x1) == 0) {
-        ctx->pc_succ_insn = ctx->base.pc_next + 2;
-        decode_16Bit_opc(ctx);
-    /* 32-Bit Instruction */
-    } else {
-        ctx->pc_succ_insn = ctx->base.pc_next + 4;
-        decode_32Bit_opc(ctx);
-    }
+    return (insn & 0x1) == 0;
 }
 
 static void tricore_tr_init_disas_context(DisasContextBase *dcbase,
@@ -8829,20 +8821,49 @@  static bool tricore_tr_breakpoint_check(DisasContextBase *dcbase, CPUState *cpu,
     return true;
 }
 
+static bool insn_crosses_page(CPUTriCoreState *env, DisasContext *ctx)
+{
+    /*
+     * Return true if the insn at ctx->base.pc_next might cross a page boundary.
+     * (False positives are OK, false negatives are not.)
+     * Our caller ensures we are only called if dc->base.pc_next is less than
+     * 4 bytes from the page boundary, so we cross the page if the first
+     * 16 bits indicate that this is a 32 bit insn.
+     */
+    uint16_t insn = cpu_lduw_code(env, ctx->base.pc_next);
+
+    return !tricore_insn_is_16bit(insn);
+}
+
+
 static void tricore_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu)
 {
     DisasContext *ctx = container_of(dcbase, DisasContext, base);
     CPUTriCoreState *env = cpu->env_ptr;
+    uint16_t insn_lo;
+    bool is_16bit;
 
-    ctx->opcode = cpu_ldl_code(env, ctx->base.pc_next);
-    decode_opc(ctx);
+    insn_lo = cpu_lduw_code(env, ctx->base.pc_next);
+    is_16bit = tricore_insn_is_16bit(insn_lo);
+    if (is_16bit) {
+        ctx->opcode = insn_lo;
+        ctx->pc_succ_insn = ctx->base.pc_next + 2;
+        decode_16Bit_opc(ctx);
+    } else {
+        uint32_t insn_hi = cpu_lduw_code(env, ctx->base.pc_next + 2);
+        ctx->opcode = insn_hi << 16 | insn_lo;
+        ctx->pc_succ_insn = ctx->base.pc_next + 4;
+        decode_32Bit_opc(ctx);
+    }
     ctx->base.pc_next = ctx->pc_succ_insn;
 
     if (ctx->base.is_jmp == DISAS_NEXT) {
         target_ulong page_start;
 
         page_start = ctx->base.pc_first & TARGET_PAGE_MASK;
-        if (ctx->base.pc_next - page_start >= TARGET_PAGE_SIZE) {
+        if (ctx->base.pc_next - page_start >= TARGET_PAGE_SIZE
+            || (ctx->base.pc_next - page_start >= TARGET_PAGE_SIZE - 3
+                && insn_crosses_page(env, ctx))) {
             ctx->base.is_jmp = DISAS_TOO_MANY;
         }
     }