diff mbox series

[v4,10/17] target/m68k: Implement TRAPcc

Message ID 20220430175342.370628-11-richard.henderson@linaro.org
State Superseded
Headers show
Series target/m68k: Conditional traps + trap cleanup | expand

Commit Message

Richard Henderson April 30, 2022, 5:53 p.m. UTC
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/754
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/m68k/cpu.h          |  2 ++
 linux-user/m68k/cpu_loop.c |  1 +
 target/m68k/cpu.c          |  1 +
 target/m68k/op_helper.c    |  6 +----
 target/m68k/translate.c    | 49 ++++++++++++++++++++++++++++++++++++++
 5 files changed, 54 insertions(+), 5 deletions(-)

Comments

Laurent Vivier May 25, 2022, 9:40 p.m. UTC | #1
Le 30/04/2022 à 19:53, 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 ++
>   linux-user/m68k/cpu_loop.c |  1 +
>   target/m68k/cpu.c          |  1 +
>   target/m68k/op_helper.c    |  6 +----
>   target/m68k/translate.c    | 49 ++++++++++++++++++++++++++++++++++++++
>   5 files changed, 54 insertions(+), 5 deletions(-)
> 
> diff --git a/target/m68k/cpu.h b/target/m68k/cpu.h
> index 558c3c67d6..4d8f48e8c7 100644
> --- a/target/m68k/cpu.h
> +++ b/target/m68k/cpu.h
> @@ -534,6 +534,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/linux-user/m68k/cpu_loop.c b/linux-user/m68k/cpu_loop.c
> index 000bb44cc3..5007b24c03 100644
> --- a/linux-user/m68k/cpu_loop.c
> +++ b/linux-user/m68k/cpu_loop.c
> @@ -48,6 +48,7 @@ void cpu_loop(CPUM68KState *env)
>               force_sig_fault(TARGET_SIGILL, TARGET_ILL_ILLOPN, env->pc);
>               break;
>           case EXCP_CHK:
> +        case EXCP_TRAPCC:
>               force_sig_fault(TARGET_SIGFPE, TARGET_FPE_INTOVF, env->mmu.ar);
>               break;
>           case EXCP_DIV0:
> 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/op_helper.c b/target/m68k/op_helper.c
> index aa62158eb9..61948d92bb 100644
> --- a/target/m68k/op_helper.c
> +++ b/target/m68k/op_helper.c
> @@ -399,14 +399,10 @@ static void m68k_interrupt_all(CPUM68KState *env, int is_hw)
>           do_stack_frame(env, &sp, 2, oldsr, 0, env->pc);
>           break;
>   
> -    case EXCP_TRAPCC:
> -        /* FIXME: addr is not only env->pc */
> -        do_stack_frame(env, &sp, 2, oldsr, env->pc, env->pc);
> -        break;
> -
>       case EXCP_CHK:
>       case EXCP_DIV0:
>       case EXCP_TRACE:
> +    case EXCP_TRAPCC:
>           do_stack_frame(env, &sp, 2, oldsr, env->mmu.ar, env->pc);
>           break;
>   
> diff --git a/target/m68k/translate.c b/target/m68k/translate.c
> index 399d9232e4..c4fe8abc03 100644
> --- a/target/m68k/translate.c
> +++ b/target/m68k/translate.c
> @@ -4879,6 +4879,54 @@ DISAS_INSN(trap)
>       gen_exception(s, s->pc, EXCP_TRAP0 + (insn & 0xf));
>   }
>   
> +static void do_trapcc(DisasContext *s, DisasCompare *c)
> +{
> +    if (c->tcond != TCG_COND_NEVER) {
> +        TCGLabel *over = NULL;
> +
> +        update_cc_op(s);
> +
> +        if (c->tcond != TCG_COND_ALWAYS) {
> +            /* Jump over if !c. */
> +            over = gen_new_label();
> +            tcg_gen_brcond_i32(tcg_invert_cond(c->tcond), c->v1, c->v2, over);
> +        }
> +
> +        tcg_gen_movi_i32(QREG_PC, s->pc);
> +        gen_raise_exception_format2(s, EXCP_TRAPCC, s->base.pc_next);
> +
> +        if (over != NULL) {
> +            gen_set_label(over);
> +            s->base.is_jmp = DISAS_NEXT;
> +        }
> +    }
> +    free_cond(c);
> +}
> +
> +DISAS_INSN(trapcc)
> +{
> +    DisasCompare c;
> +
> +    /* 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 really need to read the data or do we only need to increment s->pc (as the data are only here 
to be available for the trap handler)?

> +    case 4: /* trapcc (no operand) */
> +        break;
> +    default:
> +        /* Illegal insn */
> +        disas_undef(env, s, insn);
> +        return;
> +    }
> +
> +    gen_cc_cond(&c, s, extract32(insn, 8, 4));
> +    do_trapcc(s, &c);
> +}
> +
>   static void gen_load_fcr(DisasContext *s, TCGv res, int reg)
>   {
>       switch (reg) {
> @@ -6050,6 +6098,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.  */

Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Richard Henderson May 25, 2022, 10:26 p.m. UTC | #2
On 5/25/22 14:40, Laurent Vivier wrote:
>> +DISAS_INSN(trapcc)
>> +{
>> +    DisasCompare c;
>> +
>> +    /* 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 really need to read the data or do we only need to increment s->pc (as the data are 
> only here to be available for the trap handler)?

We need to read the data to (1) trigger sigsegv when this insn crosses a page and (2) 
passing to tcg plugins.


r~
Laurent Vivier May 26, 2022, 12:15 a.m. UTC | #3
Le 26/05/2022 à 00:26, Richard Henderson a écrit :
> On 5/25/22 14:40, Laurent Vivier wrote:
>>> +DISAS_INSN(trapcc)
>>> +{
>>> +    DisasCompare c;
>>> +
>>> +    /* 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 really need to read the data or do we only need to increment s->pc (as the data are only 
>> here to be available for the trap handler)?
> 
> We need to read the data to (1) trigger sigsegv when this insn crosses a page and (2) passing to tcg 
> plugins.
> 

For (1) I was wondering if the real CPU is actually doing it.

Nothing is said about it in the instruction definition.

Thanks,
Laurent
Richard Henderson May 27, 2022, 3:47 p.m. UTC | #4
On 5/25/22 17:15, Laurent Vivier wrote:
> Le 26/05/2022 à 00:26, Richard Henderson a écrit :
>> On 5/25/22 14:40, Laurent Vivier wrote:
>>>> +DISAS_INSN(trapcc)
>>>> +{
>>>> +    DisasCompare c;
>>>> +
>>>> +    /* 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 really need to read the data or do we only need to increment s->pc (as the data 
>>> are only here to be available for the trap handler)?
>>
>> We need to read the data to (1) trigger sigsegv when this insn crosses a page and (2) 
>> passing to tcg plugins.
>>
> 
> For (1) I was wondering if the real CPU is actually doing it.
> 
> Nothing is said about it in the instruction definition.

Surely the cpu reads cachelines at a time, so of course it would.

r~
diff mbox series

Patch

diff --git a/target/m68k/cpu.h b/target/m68k/cpu.h
index 558c3c67d6..4d8f48e8c7 100644
--- a/target/m68k/cpu.h
+++ b/target/m68k/cpu.h
@@ -534,6 +534,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/linux-user/m68k/cpu_loop.c b/linux-user/m68k/cpu_loop.c
index 000bb44cc3..5007b24c03 100644
--- a/linux-user/m68k/cpu_loop.c
+++ b/linux-user/m68k/cpu_loop.c
@@ -48,6 +48,7 @@  void cpu_loop(CPUM68KState *env)
             force_sig_fault(TARGET_SIGILL, TARGET_ILL_ILLOPN, env->pc);
             break;
         case EXCP_CHK:
+        case EXCP_TRAPCC:
             force_sig_fault(TARGET_SIGFPE, TARGET_FPE_INTOVF, env->mmu.ar);
             break;
         case EXCP_DIV0:
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/op_helper.c b/target/m68k/op_helper.c
index aa62158eb9..61948d92bb 100644
--- a/target/m68k/op_helper.c
+++ b/target/m68k/op_helper.c
@@ -399,14 +399,10 @@  static void m68k_interrupt_all(CPUM68KState *env, int is_hw)
         do_stack_frame(env, &sp, 2, oldsr, 0, env->pc);
         break;
 
-    case EXCP_TRAPCC:
-        /* FIXME: addr is not only env->pc */
-        do_stack_frame(env, &sp, 2, oldsr, env->pc, env->pc);
-        break;
-
     case EXCP_CHK:
     case EXCP_DIV0:
     case EXCP_TRACE:
+    case EXCP_TRAPCC:
         do_stack_frame(env, &sp, 2, oldsr, env->mmu.ar, env->pc);
         break;
 
diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index 399d9232e4..c4fe8abc03 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -4879,6 +4879,54 @@  DISAS_INSN(trap)
     gen_exception(s, s->pc, EXCP_TRAP0 + (insn & 0xf));
 }
 
+static void do_trapcc(DisasContext *s, DisasCompare *c)
+{
+    if (c->tcond != TCG_COND_NEVER) {
+        TCGLabel *over = NULL;
+
+        update_cc_op(s);
+
+        if (c->tcond != TCG_COND_ALWAYS) {
+            /* Jump over if !c. */
+            over = gen_new_label();
+            tcg_gen_brcond_i32(tcg_invert_cond(c->tcond), c->v1, c->v2, over);
+        }
+
+        tcg_gen_movi_i32(QREG_PC, s->pc);
+        gen_raise_exception_format2(s, EXCP_TRAPCC, s->base.pc_next);
+
+        if (over != NULL) {
+            gen_set_label(over);
+            s->base.is_jmp = DISAS_NEXT;
+        }
+    }
+    free_cond(c);
+}
+
+DISAS_INSN(trapcc)
+{
+    DisasCompare c;
+
+    /* 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;
+    }
+
+    gen_cc_cond(&c, s, extract32(insn, 8, 4));
+    do_trapcc(s, &c);
+}
+
 static void gen_load_fcr(DisasContext *s, TCGv res, int reg)
 {
     switch (reg) {
@@ -6050,6 +6098,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.  */