1. Patchew
  2. QEMU
  3. View series

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

Richard Henderson posted 1 patch 1 year, 9 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20220730021844.124503-1-richard.henderson@linaro.org
Maintainers: "Philippe Mathieu-Daudé" <f4bug@amsat.org>, Aurelien Jarno <aurelien@aurel32.net>, Jiaxun Yang <jiaxun.yang@flygoat.com>, Aleksandar Rikalo <aleksandar.rikalo@syrmia.com>, Stefan Pejic <stefan.pejic@syrmia.com>
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(-)
[PATCH] target/mips: Advance pc after semihosting exception
Posted by Richard Henderson 1 year, 9 months ago 
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);
+    }
     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);
-- 
2.34.1
Re: [PATCH] target/mips: Advance pc after semihosting exception
Posted by Philippe Mathieu-Daudé via 1 year, 8 months ago 
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);
Re: [PATCH] target/mips: Advance pc after semihosting exception
Posted by Richard Henderson 1 year, 8 months ago 
On 8/1/22 23:48, Philippe Mathieu-Daudé wrote:
> 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);

These error codes are specific to the matching EXCP.
We don't need to introduce extra storage, I don't think.


r~

> 
> 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);
>
Re: [PATCH] target/mips: Advance pc after semihosting exception
Posted by Philippe Mathieu-Daudé via 1 year, 8 months ago 
On Tue, Aug 2, 2022 at 4:11 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
> On 8/1/22 23:48, Philippe Mathieu-Daudé wrote:
> > 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(-)

> >> @@ -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);
>
> These error codes are specific to the matching EXCP.
> We don't need to introduce extra storage, I don't think.

OK, fine then.

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

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.