qemu-devel
[Top][All Lists]
Advanced
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] target/mips: Advance pc after semihosting exception

From: Philippe Mathieu-Daudé
Subject: Re: [PATCH] target/mips: Advance pc after semihosting exception
Date: Tue, 2 Aug 2022 08:48:35 +0200
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0) Gecko/20100101 Thunderbird/91.11.0 
Hi Richard,

On 30/7/22 04:18, Richard Henderson wrote:

Delay generating the exception until after we know the
insn length, and record that length in env->error_code.

Fixes: 8ec7e3c53d4 ("target/mips: Use an exception for semihosting")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1126
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
  target/mips/tcg/translate.h               |  4 ++++
  target/mips/tcg/sysemu/tlb_helper.c       |  1 +
  target/mips/tcg/translate.c               | 10 +++++-----
  target/mips/tcg/micromips_translate.c.inc |  6 +++---
  target/mips/tcg/mips16e_translate.c.inc   |  2 +-
  target/mips/tcg/nanomips_translate.c.inc  |  4 ++--
  6 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/target/mips/tcg/translate.h b/target/mips/tcg/translate.h
index 55053226ae..69f85841d2 100644
--- a/target/mips/tcg/translate.h
+++ b/target/mips/tcg/translate.h
@@ -51,6 +51,10 @@ typedef struct DisasContext {
      int gi;
  } DisasContext;
+#define DISAS_STOP DISAS_TARGET_0 
+#define DISAS_EXIT       DISAS_TARGET_1
+#define DISAS_SEMIHOST   DISAS_TARGET_2
+
  /* MIPS major opcodes */
  #define MASK_OP_MAJOR(op)   (op & (0x3F << 26))
diff --git a/target/mips/tcg/sysemu/tlb_helper.c b/target/mips/tcg/sysemu/tlb_helper.c 
index 57ffad2902..9d16859c0a 100644
--- a/target/mips/tcg/sysemu/tlb_helper.c
+++ b/target/mips/tcg/sysemu/tlb_helper.c
@@ -1056,6 +1056,7 @@ void mips_cpu_do_interrupt(CPUState *cs)
      case EXCP_SEMIHOST:
          cs->exception_index = EXCP_NONE;
          mips_semihosting(env);
+        env->active_tc.PC += env->error_code;
          return;
      case EXCP_DSS:
          env->CP0_Debug |= 1 << CP0DB_DSS;
diff --git a/target/mips/tcg/translate.c b/target/mips/tcg/translate.c
index 1f6a779808..de1511baaf 100644
--- a/target/mips/tcg/translate.c
+++ b/target/mips/tcg/translate.c
@@ -1213,9 +1213,6 @@ TCGv_i64 fpu_f64[32];
#include "exec/gen-icount.h" -#define DISAS_STOP DISAS_TARGET_0 
-#define DISAS_EXIT       DISAS_TARGET_1
-
  static const char regnames_HI[][4] = {
      "HI0", "HI1", "HI2", "HI3",
  };
@@ -13902,7 +13899,7 @@ static void decode_opc_special_r6(CPUMIPSState *env, 
DisasContext *ctx)
          break;
      case R6_OPC_SDBBP:
          if (is_uhi(extract32(ctx->opcode, 6, 20))) {
-            generate_exception_end(ctx, EXCP_SEMIHOST);
+            ctx->base.is_jmp = DISAS_SEMIHOST;
          } else {
              if (ctx->hflags & MIPS_HFLAG_SBRI) {
                  gen_reserved_instruction(ctx);
@@ -14314,7 +14311,7 @@ static void decode_opc_special2_legacy(CPUMIPSState 
*env, DisasContext *ctx)
          break;
      case OPC_SDBBP:
          if (is_uhi(extract32(ctx->opcode, 6, 20))) {
-            generate_exception_end(ctx, EXCP_SEMIHOST);
+            ctx->base.is_jmp = DISAS_SEMIHOST;
          } else {
              /*
               * XXX: not clear which exception should be raised
@@ -16098,6 +16095,9 @@ static void mips_tr_translate_insn(DisasContextBase 
*dcbase, CPUState *cs)
      if (is_slot) {
          gen_branch(ctx, insn_bytes);
      }
+    if (ctx->base.is_jmp == DISAS_SEMIHOST) {
+        generate_exception_err(ctx, EXCP_SEMIHOST, insn_bytes);


Hmm this API takes an error_code argument:

  int error_code;
  #define EXCP_TLB_NOMATCH   0x1
#define EXCP_INST_NOTAVAIL 0x2 /* No valid instruction word for BadInstr */ 

Now we pass bytes. What about adding an extra helper?

  void generate_exception_err_displace(DisasContext *ctx,
                                       int excp, int err,
                                       target_long displacement);

Otherwise LGTM.

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>


+    }
      ctx->base.pc_next += insn_bytes;
if (ctx->base.is_jmp != DISAS_NEXT) { 
diff --git a/target/mips/tcg/micromips_translate.c.inc 
b/target/mips/tcg/micromips_translate.c.inc
index 274caf2c3c..b2c696f891 100644
--- a/target/mips/tcg/micromips_translate.c.inc
+++ b/target/mips/tcg/micromips_translate.c.inc
@@ -826,7 +826,7 @@ static void gen_pool16c_insn(DisasContext *ctx)
          break;
      case SDBBP16:
          if (is_uhi(extract32(ctx->opcode, 0, 4))) {
-            generate_exception_end(ctx, EXCP_SEMIHOST);
+            ctx->base.is_jmp = DISAS_SEMIHOST;
          } else {
              /*
               * XXX: not clear which exception should be raised
@@ -942,7 +942,7 @@ static void gen_pool16c_r6_insn(DisasContext *ctx)
          case R6_SDBBP16:
              /* SDBBP16 */
              if (is_uhi(extract32(ctx->opcode, 6, 4))) {
-                generate_exception_end(ctx, EXCP_SEMIHOST);
+                ctx->base.is_jmp = DISAS_SEMIHOST;
              } else {
                  if (ctx->hflags & MIPS_HFLAG_SBRI) {
                      generate_exception(ctx, EXCP_RI);
@@ -1311,7 +1311,7 @@ static void gen_pool32axf(CPUMIPSState *env, DisasContext 
*ctx, int rt, int rs)
              break;
          case SDBBP:
              if (is_uhi(extract32(ctx->opcode, 16, 10))) {
-                generate_exception_end(ctx, EXCP_SEMIHOST);
+                ctx->base.is_jmp = DISAS_SEMIHOST;
              } else {
                  check_insn(ctx, ISA_MIPS_R1);
                  if (ctx->hflags & MIPS_HFLAG_SBRI) {
diff --git a/target/mips/tcg/mips16e_translate.c.inc 
b/target/mips/tcg/mips16e_translate.c.inc
index 0a3ba252e4..7568933e23 100644
--- a/target/mips/tcg/mips16e_translate.c.inc
+++ b/target/mips/tcg/mips16e_translate.c.inc
@@ -952,7 +952,7 @@ static int decode_ase_mips16e(CPUMIPSState *env, 
DisasContext *ctx)
              break;
          case RR_SDBBP:
              if (is_uhi(extract32(ctx->opcode, 5, 6))) {
-                generate_exception_end(ctx, EXCP_SEMIHOST);
+                ctx->base.is_jmp = DISAS_SEMIHOST;
              } else {
                  /*
                   * XXX: not clear which exception should be raised
diff --git a/target/mips/tcg/nanomips_translate.c.inc 
b/target/mips/tcg/nanomips_translate.c.inc
index ecb0ebed57..b3aff22c18 100644
--- a/target/mips/tcg/nanomips_translate.c.inc
+++ b/target/mips/tcg/nanomips_translate.c.inc
@@ -3695,7 +3695,7 @@ static int decode_nanomips_32_48_opc(CPUMIPSState *env, 
DisasContext *ctx)
                  break;
              case NM_SDBBP:
                  if (is_uhi(extract32(ctx->opcode, 0, 19))) {
-                    generate_exception_end(ctx, EXCP_SEMIHOST);
+                    ctx->base.is_jmp = DISAS_SEMIHOST;
                  } else {
                      if (ctx->hflags & MIPS_HFLAG_SBRI) {
                          gen_reserved_instruction(ctx);
@@ -4634,7 +4634,7 @@ static int decode_isa_nanomips(CPUMIPSState *env, 
DisasContext *ctx)
                  break;
              case NM_SDBBP16:
                  if (is_uhi(extract32(ctx->opcode, 0, 3))) {
-                    generate_exception_end(ctx, EXCP_SEMIHOST);
+                    ctx->base.is_jmp = DISAS_SEMIHOST;
                  } else {
                      if (ctx->hflags & MIPS_HFLAG_SBRI) {
                          gen_reserved_instruction(ctx);
reply via email to
[Prev in Thread] Current Thread [Next in Thread]