[PATCH for-7.0 0/3] target/m68k: Implement conditional traps

[PATCH for-7.0 0/3] target/m68k: Implement conditional traps

[Richard Henderson]

While looking at #754 for trapcc, I noticed that the other
conditional traps, trapv and ftrapcc, are also missing.


r~


Richard Henderson (3):
  target/m68k: Implement TRAPV
  target/m68k: Implement TRAPcc
  target/m68k: Implement FTRAPcc

 target/m68k/cpu.h       |  2 ++
 target/m68k/cpu.c       |  1 +
 target/m68k/translate.c | 76 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 79 insertions(+)

-- 
2.25.1

[PATCH 1/3] target/m68k: Implement TRAPV

[Richard Henderson]

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/m68k/translate.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index af43c8eab8..858ba761fc 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -4863,6 +4863,22 @@ DISAS_INSN(trap)
     gen_exception(s, s->base.pc_next, EXCP_TRAP0 + (insn & 0xf));
 }
 
+static void do_trapcc(DisasContext *s, int cond)
+{
+    TCGLabel *over = gen_new_label();
+
+    /* Jump over if !cond. */
+    gen_jmpcc(s, cond ^ 1, over);
+
+    gen_exception(s, s->base.pc_next, EXCP_TRAPCC);
+    gen_set_label(over);
+}
+
+DISAS_INSN(trapv)
+{
+    do_trapcc(s, 9); /* VS */
+}
+
 static void gen_load_fcr(DisasContext *s, TCGv res, int reg)
 {
     switch (reg) {
@@ -6026,6 +6042,7 @@ void register_m68k_insns (CPUM68KState *env)
     BASE(nop,       4e71, ffff);
     INSN(rtd,       4e74, ffff, RTD);
     BASE(rts,       4e75, ffff);
+    INSN(trapv,     4e76, ffff, M68000);
     INSN(rtr,       4e77, ffff, M68000);
     BASE(jump,      4e80, ffc0);
     BASE(jump,      4ec0, ffc0);
-- 
2.25.1

[PATCH 2/3] target/m68k: Implement TRAPcc

[Richard Henderson]

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/754
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/m68k/cpu.h       |  2 ++
 target/m68k/cpu.c       |  1 +
 target/m68k/translate.c | 21 +++++++++++++++++++++
 3 files changed, 24 insertions(+)

diff --git a/target/m68k/cpu.h b/target/m68k/cpu.h
index a3423729ef..03f600f7e7 100644
--- a/target/m68k/cpu.h
+++ b/target/m68k/cpu.h
@@ -527,6 +527,8 @@ enum m68k_features {
     M68K_FEATURE_MOVEC,
     /* Unaligned data accesses (680[2346]0) */
     M68K_FEATURE_UNALIGNED_DATA,
+    /* TRAPCC insn. (680[2346]0, and CPU32) */
+    M68K_FEATURE_TRAPCC,
 };
 
 static inline int m68k_feature(CPUM68KState *env, int feature)
diff --git a/target/m68k/cpu.c b/target/m68k/cpu.c
index c7aeb7da9c..5f778773d1 100644
--- a/target/m68k/cpu.c
+++ b/target/m68k/cpu.c
@@ -162,6 +162,7 @@ static void m68020_cpu_initfn(Object *obj)
     m68k_set_feature(env, M68K_FEATURE_CHK2);
     m68k_set_feature(env, M68K_FEATURE_MSP);
     m68k_set_feature(env, M68K_FEATURE_UNALIGNED_DATA);
+    m68k_set_feature(env, M68K_FEATURE_TRAPCC);
 }
 
 /*
diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index 858ba761fc..cf29f35d91 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -4879,6 +4879,26 @@ DISAS_INSN(trapv)
     do_trapcc(s, 9); /* VS */
 }
 
+DISAS_INSN(trapcc)
+{
+    /* Consume and discard the immediate operand. */
+    switch (extract32(insn, 0, 3)) {
+    case 2: /* trapcc.w */
+        (void)read_im16(env, s);
+        break;
+    case 3: /* trapcc.l */
+        (void)read_im32(env, s);
+        break;
+    case 4: /* trapcc (no operand) */
+        break;
+    default:
+        /* Illegal insn */
+        disas_undef(env, s, insn);
+        return;
+    }
+    do_trapcc(s, extract32(insn, 8, 4));
+}
+
 static void gen_load_fcr(DisasContext *s, TCGv res, int reg)
 {
     switch (reg) {
@@ -6051,6 +6071,7 @@ void register_m68k_insns (CPUM68KState *env)
     INSN(scc,       50c0, f0f8, CF_ISA_A); /* Scc.B Dx   */
     INSN(scc,       50c0, f0c0, M68000);   /* Scc.B <EA> */
     INSN(dbcc,      50c8, f0f8, M68000);
+    INSN(trapcc,    50f8, f0f8, TRAPCC);
     INSN(tpf,       51f8, fff8, CF_ISA_A);
 
     /* Branch instructions.  */
-- 
2.25.1

[PATCH 3/3] target/m68k: Implement FTRAPcc

[Richard Henderson]

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/m68k/translate.c | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index cf29f35d91..3c04f9d1a9 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -5547,6 +5547,43 @@ DISAS_INSN(fscc)
     tcg_temp_free(tmp);
 }
 
+DISAS_INSN(ftrapcc)
+{
+    DisasCompare c;
+    TCGLabel *over;
+    uint16_t ext;
+    int cond;
+
+    ext = read_im16(env, s);
+    cond = ext & 0x3f;
+
+    /* Consume and discard the immediate operand. */
+    switch (extract32(insn, 0, 3)) {
+    case 2: /* ftrapcc.w */
+        (void)read_im16(env, s);
+        break;
+    case 3: /* ftrapcc.l */
+        (void)read_im32(env, s);
+        break;
+    case 4: /* ftrapcc (no operand) */
+        break;
+    default:
+        /* Illegal insn */
+        disas_undef(env, s, insn);
+        return;
+    }
+
+    /* Jump over if !cond. */
+    gen_fcc_cond(&c, s, cond);
+    update_cc_op(s);
+    over = gen_new_label();
+    tcg_gen_brcond_i32(tcg_invert_cond(c.tcond), c.v1, c.v2, over);
+    free_cond(&c);
+
+    gen_exception(s, s->base.pc_next, EXCP_TRAPCC);
+    gen_set_label(over);
+}
+
 #if defined(CONFIG_SOFTMMU)
 DISAS_INSN(frestore)
 {
@@ -6170,6 +6207,7 @@ void register_m68k_insns (CPUM68KState *env)
     INSN(fbcc,      f280, ffc0, CF_FPU);
     INSN(fpu,       f200, ffc0, FPU);
     INSN(fscc,      f240, ffc0, FPU);
+    INSN(ftrapcc,   f278, ff80, FPU);
     INSN(fbcc,      f280, ff80, FPU);
 #if defined(CONFIG_SOFTMMU)
     INSN(frestore,  f340, ffc0, CF_FPU);
-- 
2.25.1

Re: [PATCH 3/3] target/m68k: Implement FTRAPcc

[Richard Henderson]

On 11/30/21 11:37 AM, Richard Henderson wrote:
> +    INSN(ftrapcc,   f278, ff80, FPU);

Whoops, mask should be fff8.


r~

Re: [PATCH 2/3] target/m68k: Implement TRAPcc

[Laurent Vivier]

Le 30/11/2021 à 11:37, Richard Henderson a écrit :
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/754
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/m68k/cpu.h       |  2 ++
>   target/m68k/cpu.c       |  1 +
>   target/m68k/translate.c | 21 +++++++++++++++++++++
>   3 files changed, 24 insertions(+)
> 
> diff --git a/target/m68k/cpu.h b/target/m68k/cpu.h
> index a3423729ef..03f600f7e7 100644
> --- a/target/m68k/cpu.h
> +++ b/target/m68k/cpu.h
> @@ -527,6 +527,8 @@ enum m68k_features {
>       M68K_FEATURE_MOVEC,
>       /* Unaligned data accesses (680[2346]0) */
>       M68K_FEATURE_UNALIGNED_DATA,
> +    /* TRAPCC insn. (680[2346]0, and CPU32) */
> +    M68K_FEATURE_TRAPCC,
>   };
>   
>   static inline int m68k_feature(CPUM68KState *env, int feature)
> diff --git a/target/m68k/cpu.c b/target/m68k/cpu.c
> index c7aeb7da9c..5f778773d1 100644
> --- a/target/m68k/cpu.c
> +++ b/target/m68k/cpu.c
> @@ -162,6 +162,7 @@ static void m68020_cpu_initfn(Object *obj)
>       m68k_set_feature(env, M68K_FEATURE_CHK2);
>       m68k_set_feature(env, M68K_FEATURE_MSP);
>       m68k_set_feature(env, M68K_FEATURE_UNALIGNED_DATA);
> +    m68k_set_feature(env, M68K_FEATURE_TRAPCC);
>   }
>   
>   /*
> diff --git a/target/m68k/translate.c b/target/m68k/translate.c
> index 858ba761fc..cf29f35d91 100644
> --- a/target/m68k/translate.c
> +++ b/target/m68k/translate.c
> @@ -4879,6 +4879,26 @@ DISAS_INSN(trapv)
>       do_trapcc(s, 9); /* VS */
>   }
>   
> +DISAS_INSN(trapcc)
> +{
> +    /* Consume and discard the immediate operand. */
> +    switch (extract32(insn, 0, 3)) {
> +    case 2: /* trapcc.w */
> +        (void)read_im16(env, s);
> +        break;
> +    case 3: /* trapcc.l */
> +        (void)read_im32(env, s);
> +        break;

Do we need to actually read the memory to trigger a fault if needed or can we only increase the PC?

Normally these values are for the trap handler.

> +    case 4: /* trapcc (no operand) */
> +        break;
> +    default:
> +        /* Illegal insn */
> +        disas_undef(env, s, insn);
> +        return;
> +    }
> +    do_trapcc(s, extract32(insn, 8, 4));
> +}

Do we need to change something in m68k_interrupt_all()?

     if (!is_hw) {
         switch (cs->exception_index) {
         case EXCP_RTE:
             /* Return from an exception.  */
             m68k_rte(env);
             return;
         case EXCP_TRAP0 ...  EXCP_TRAP15:
             /* Move the PC after the trap instruction.  */
             retaddr += 2;
             break;
         }
     }

Thanks,
Laurent

> +
>   static void gen_load_fcr(DisasContext *s, TCGv res, int reg)
>   {
>       switch (reg) {
> @@ -6051,6 +6071,7 @@ void register_m68k_insns (CPUM68KState *env)
>       INSN(scc,       50c0, f0f8, CF_ISA_A); /* Scc.B Dx   */
>       INSN(scc,       50c0, f0c0, M68000);   /* Scc.B <EA> */
>       INSN(dbcc,      50c8, f0f8, M68000);
> +    INSN(trapcc,    50f8, f0f8, TRAPCC);
>       INSN(tpf,       51f8, fff8, CF_ISA_A);
>   
>       /* Branch instructions.  */
> 

Re: [PATCH 1/3] target/m68k: Implement TRAPV

[Laurent Vivier]

Le 30/11/2021 à 11:37, Richard Henderson a écrit :
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/m68k/translate.c | 17 +++++++++++++++++
>   1 file changed, 17 insertions(+)
> 
> diff --git a/target/m68k/translate.c b/target/m68k/translate.c
> index af43c8eab8..858ba761fc 100644
> --- a/target/m68k/translate.c
> +++ b/target/m68k/translate.c
> @@ -4863,6 +4863,22 @@ DISAS_INSN(trap)
>       gen_exception(s, s->base.pc_next, EXCP_TRAP0 + (insn & 0xf));
>   }
>   
> +static void do_trapcc(DisasContext *s, int cond)
> +{
> +    TCGLabel *over = gen_new_label();
> +
> +    /* Jump over if !cond. */
> +    gen_jmpcc(s, cond ^ 1, over);
> +
> +    gen_exception(s, s->base.pc_next, EXCP_TRAPCC);
> +    gen_set_label(over);
> +}
> +
> +DISAS_INSN(trapv)
> +{
> +    do_trapcc(s, 9); /* VS */
> +}
> +
>   static void gen_load_fcr(DisasContext *s, TCGv res, int reg)
>   {
>       switch (reg) {
> @@ -6026,6 +6042,7 @@ void register_m68k_insns (CPUM68KState *env)
>       BASE(nop,       4e71, ffff);
>       INSN(rtd,       4e74, ffff, RTD);
>       BASE(rts,       4e75, ffff);
> +    INSN(trapv,     4e76, ffff, M68000);
>       INSN(rtr,       4e77, ffff, M68000);
>       BASE(jump,      4e80, ffc0);
>       BASE(jump,      4ec0, ffc0);
> 

Same question as for PATCH 2 regarding m68k_interrupt_all()

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

Re: [PATCH 2/3] target/m68k: Implement TRAPcc

[Richard Henderson]

On 11/30/21 12:57 PM, Laurent Vivier wrote:
>> +DISAS_INSN(trapcc)
>> +{
>> +    /* Consume and discard the immediate operand. */
>> +    switch (extract32(insn, 0, 3)) {
>> +    case 2: /* trapcc.w */
>> +        (void)read_im16(env, s);
>> +        break;
>> +    case 3: /* trapcc.l */
>> +        (void)read_im32(env, s);
>> +        break;
> 
> Do we need to actually read the memory to trigger a fault if needed or can we only 
> increase the PC?

Yes, and to pass the entire instruction to plugins.

>> +    case 4: /* trapcc (no operand) */
>> +        break;
>> +    default:
>> +        /* Illegal insn */
>> +        disas_undef(env, s, insn);
>> +        return;
>> +    }
>> +    do_trapcc(s, extract32(insn, 8, 4));
>> +}
> 
> Do we need to change something in m68k_interrupt_all()?

Yes, and cpu_loop.  Thanks,


r~