[Qemu-devel] [PULL v5 11/12] target-lm32: stop VM on illegal or unknown instruction

[Michael Walle <michael@walle.cc>]

Instead of translating the instruction to a no-op, pause the VM and display
a message to the user.

As a side effect, this also works for instructions where the operands are
only known at runtime.

Signed-off-by: Michael Walle <michael@walle.cc>
---
 target-lm32/helper.h    |    1 +
 target-lm32/op_helper.c |   17 ++++++++
 target-lm32/translate.c |  106 ++++++++++++++++++++++++++++++-----------------
 3 files changed, 86 insertions(+), 38 deletions(-)

diff --git a/target-lm32/helper.h b/target-lm32/helper.h
index ad44fdf..f4442e0 100644
--- a/target-lm32/helper.h
+++ b/target-lm32/helper.h
@@ -13,5 +13,6 @@ DEF_HELPER_1(rcsr_im, i32, env)
 DEF_HELPER_1(rcsr_ip, i32, env)
 DEF_HELPER_1(rcsr_jtx, i32, env)
 DEF_HELPER_1(rcsr_jrx, i32, env)
+DEF_HELPER_1(ill, void, env)
 
 #include "exec/def-helper.h"
diff --git a/target-lm32/op_helper.c b/target-lm32/op_helper.c
index 71f21d1..7189cb5 100644
--- a/target-lm32/op_helper.c
+++ b/target-lm32/op_helper.c
@@ -8,6 +8,10 @@
 
 #include "exec/softmmu_exec.h"
 
+#ifndef CONFIG_USER_ONLY
+#include "sysemu/sysemu.h"
+#endif
+
 #if !defined(CONFIG_USER_ONLY)
 #define MMUSUFFIX _mmu
 #define SHIFT 0
@@ -39,6 +43,19 @@ void HELPER(hlt)(CPULM32State *env)
     cpu_loop_exit(env);
 }
 
+void HELPER(ill)(CPULM32State *env)
+{
+#ifndef CONFIG_USER_ONLY
+    CPUState *cs = CPU(lm32_env_get_cpu(env));
+    fprintf(stderr, "VM paused due to illegal instruction. "
+            "Connect a debugger or switch to the monitor console "
+            "to find out more.\n");
+    qemu_system_vmstop_request(RUN_STATE_PAUSED);
+    cs->halted = 1;
+    raise_exception(env, EXCP_HALTED);
+#endif
+}
+
 void HELPER(wcsr_bp)(CPULM32State *env, uint32_t bp, uint32_t idx)
 {
     uint32_t addr = bp & ~1;
diff --git a/target-lm32/translate.c b/target-lm32/translate.c
index f20460a..80bffc7 100644
--- a/target-lm32/translate.c
+++ b/target-lm32/translate.c
@@ -122,6 +122,12 @@ static inline void t_gen_raise_exception(DisasContext *dc, uint32_t index)
     tcg_temp_free_i32(tmp);
 }
 
+static inline void t_gen_illegal_insn(DisasContext *dc)
+{
+    tcg_gen_movi_tl(cpu_pc, dc->pc);
+    gen_helper_ill(cpu_env);
+}
+
 static void gen_goto_tb(DisasContext *dc, int n, target_ulong dest)
 {
     TranslationBlock *tb;
@@ -425,6 +431,7 @@ static void dec_divu(DisasContext *dc)
 
     if (!(dc->features & LM32_FEATURE_DIVIDE)) {
         qemu_log_mask(LOG_GUEST_ERROR, "hardware divider is not available\n");
+        t_gen_illegal_insn(dc);
         return;
     }
 
@@ -504,6 +511,7 @@ static void dec_modu(DisasContext *dc)
 
     if (!(dc->features & LM32_FEATURE_DIVIDE)) {
         qemu_log_mask(LOG_GUEST_ERROR, "hardware divider is not available\n");
+        t_gen_illegal_insn(dc);
         return;
     }
 
@@ -527,6 +535,7 @@ static void dec_mul(DisasContext *dc)
     if (!(dc->features & LM32_FEATURE_MULTIPLY)) {
         qemu_log_mask(LOG_GUEST_ERROR,
                       "hardware multiplier is not available\n");
+        t_gen_illegal_insn(dc);
         return;
     }
 
@@ -591,21 +600,21 @@ static void dec_orhi(DisasContext *dc)
 
 static void dec_scall(DisasContext *dc)
 {
-    if (dc->imm5 == 7) {
-        LOG_DIS("scall\n");
-    } else if (dc->imm5 == 2) {
+    switch (dc->imm5) {
+    case 2:
         LOG_DIS("break\n");
-    } else {
-        qemu_log_mask(LOG_GUEST_ERROR, "invalid opcode @0x%x", dc->pc);
-        return;
-    }
-
-    if (dc->imm5 == 7) {
-        tcg_gen_movi_tl(cpu_pc, dc->pc);
-        t_gen_raise_exception(dc, EXCP_SYSTEMCALL);
-    } else {
         tcg_gen_movi_tl(cpu_pc, dc->pc);
         t_gen_raise_exception(dc, EXCP_BREAKPOINT);
+        break;
+    case 7:
+        LOG_DIS("scall\n");
+        tcg_gen_movi_tl(cpu_pc, dc->pc);
+        t_gen_raise_exception(dc, EXCP_SYSTEMCALL);
+        break;
+    default:
+        qemu_log_mask(LOG_GUEST_ERROR, "invalid opcode @0x%x", dc->pc);
+        t_gen_illegal_insn(dc);
+        break;
     }
 }
 
@@ -681,6 +690,7 @@ static void dec_sextb(DisasContext *dc)
     if (!(dc->features & LM32_FEATURE_SIGN_EXTEND)) {
         qemu_log_mask(LOG_GUEST_ERROR,
                       "hardware sign extender is not available\n");
+        t_gen_illegal_insn(dc);
         return;
     }
 
@@ -694,6 +704,7 @@ static void dec_sexth(DisasContext *dc)
     if (!(dc->features & LM32_FEATURE_SIGN_EXTEND)) {
         qemu_log_mask(LOG_GUEST_ERROR,
                       "hardware sign extender is not available\n");
+        t_gen_illegal_insn(dc);
         return;
     }
 
@@ -722,6 +733,7 @@ static void dec_sl(DisasContext *dc)
 
     if (!(dc->features & LM32_FEATURE_SHIFT)) {
         qemu_log_mask(LOG_GUEST_ERROR, "hardware shifter is not available\n");
+        t_gen_illegal_insn(dc);
         return;
     }
 
@@ -743,24 +755,32 @@ static void dec_sr(DisasContext *dc)
         LOG_DIS("sr r%d, r%d, r%d\n", dc->r2, dc->r0, dc->r1);
     }
 
-    if (!(dc->features & LM32_FEATURE_SHIFT)) {
-        if (dc->format == OP_FMT_RI) {
-            /* TODO: check r1 == 1 during runtime */
-        } else {
-            if (dc->imm5 != 1) {
-                qemu_log_mask(LOG_GUEST_ERROR,
-                              "hardware shifter is not available\n");
-                return;
-            }
-        }
-    }
-
+    /* The real CPU (w/o hardware shifter) only supports right shift by exactly
+     * one bit */
     if (dc->format == OP_FMT_RI) {
+        if (!(dc->features & LM32_FEATURE_SHIFT) && (dc->imm5 != 1)) {
+            qemu_log_mask(LOG_GUEST_ERROR,
+                    "hardware shifter is not available\n");
+            t_gen_illegal_insn(dc);
+            return;
+        }
         tcg_gen_sari_tl(cpu_R[dc->r1], cpu_R[dc->r0], dc->imm5);
     } else {
-        TCGv t0 = tcg_temp_new();
+        int l1 = gen_new_label();
+        int l2 = gen_new_label();
+        TCGv t0 = tcg_temp_local_new();
         tcg_gen_andi_tl(t0, cpu_R[dc->r1], 0x1f);
+
+        if (!(dc->features & LM32_FEATURE_SHIFT)) {
+            tcg_gen_brcondi_tl(TCG_COND_EQ, t0, 1, l1);
+            t_gen_illegal_insn(dc);
+            tcg_gen_br(l2);
+        }
+
+        gen_set_label(l1);
         tcg_gen_sar_tl(cpu_R[dc->r2], cpu_R[dc->r0], t0);
+        gen_set_label(l2);
+
         tcg_temp_free(t0);
     }
 }
@@ -773,24 +793,30 @@ static void dec_sru(DisasContext *dc)
         LOG_DIS("sru r%d, r%d, r%d\n", dc->r2, dc->r0, dc->r1);
     }
 
-    if (!(dc->features & LM32_FEATURE_SHIFT)) {
-        if (dc->format == OP_FMT_RI) {
-            /* TODO: check r1 == 1 during runtime */
-        } else {
-            if (dc->imm5 != 1) {
-                qemu_log_mask(LOG_GUEST_ERROR,
-                              "hardware shifter is not available\n");
-                return;
-            }
-        }
-    }
-
     if (dc->format == OP_FMT_RI) {
+        if (!(dc->features & LM32_FEATURE_SHIFT) && (dc->imm5 != 1)) {
+            qemu_log_mask(LOG_GUEST_ERROR,
+                    "hardware shifter is not available\n");
+            t_gen_illegal_insn(dc);
+            return;
+        }
         tcg_gen_shri_tl(cpu_R[dc->r1], cpu_R[dc->r0], dc->imm5);
     } else {
-        TCGv t0 = tcg_temp_new();
+        int l1 = gen_new_label();
+        int l2 = gen_new_label();
+        TCGv t0 = tcg_temp_local_new();
         tcg_gen_andi_tl(t0, cpu_R[dc->r1], 0x1f);
+
+        if (!(dc->features & LM32_FEATURE_SHIFT)) {
+            tcg_gen_brcondi_tl(TCG_COND_EQ, t0, 1, l1);
+            t_gen_illegal_insn(dc);
+            tcg_gen_br(l2);
+        }
+
+        gen_set_label(l1);
         tcg_gen_shr_tl(cpu_R[dc->r2], cpu_R[dc->r0], t0);
+        gen_set_label(l2);
+
         tcg_temp_free(t0);
     }
 }
@@ -819,6 +845,7 @@ static void dec_user(DisasContext *dc)
     LOG_DIS("user");
 
     qemu_log_mask(LOG_GUEST_ERROR, "user instruction undefined\n");
+    t_gen_illegal_insn(dc);
 }
 
 static void dec_wcsr(DisasContext *dc)
@@ -886,6 +913,7 @@ static void dec_wcsr(DisasContext *dc)
         if (dc->num_breakpoints <= no) {
             qemu_log_mask(LOG_GUEST_ERROR,
                           "breakpoint #%i is not available\n", no);
+            t_gen_illegal_insn(dc);
             break;
         }
         gen_helper_wcsr_bp(cpu_env, cpu_R[dc->r1], tcg_const_i32(no));
@@ -898,6 +926,7 @@ static void dec_wcsr(DisasContext *dc)
         if (dc->num_watchpoints <= no) {
             qemu_log_mask(LOG_GUEST_ERROR,
                           "watchpoint #%i is not available\n", no);
+            t_gen_illegal_insn(dc);
             break;
         }
         gen_helper_wcsr_wp(cpu_env, cpu_R[dc->r1], tcg_const_i32(no));
@@ -956,6 +985,7 @@ static void dec_xor(DisasContext *dc)
 static void dec_ill(DisasContext *dc)
 {
     qemu_log_mask(LOG_GUEST_ERROR, "invalid opcode 0x%02x\n", dc->opcode);
+    t_gen_illegal_insn(dc);
 }
 
 typedef void (*DecoderInfo)(DisasContext *dc);
-- 
1.7.10.4