[PATCH v4 00/17] target/m68k: Conditional traps + trap cleanup

[PATCH v4 00/17] target/m68k: Conditional traps + trap cleanup

[Richard Henderson]

Changes for v4:
  - Rebase, which requires QEMU_NORETURN -> G_NORETURN.
  - Cast -4096 to abi_ulong for print_syscall_err.


r~


v1: https://lore.kernel.org/qemu-devel/20211130103752.72099-1-richard.henderson@linaro.org/
v2: https://lore.kernel.org/qemu-devel/20211202204900.50973-1-richard.henderson@linaro.org/
v3: https://lore.kernel.org/qemu-devel/20220316055840.727571-1-richard.henderson@linaro.org/


Richard Henderson (17):
  target/m68k: Raise the TRAPn exception with the correct pc
  target/m68k: Switch over exception type in m68k_interrupt_all
  target/m68k: Fix coding style in m68k_interrupt_all
  linux-user/m68k: Handle EXCP_TRAP1 through EXCP_TRAP15
  target/m68k: Remove retaddr in m68k_interrupt_all
  target/m68k: Fix address argument for EXCP_CHK
  target/m68k: Fix pc, c flag, and address argument for EXCP_DIV0
  target/m68k: Fix address argument for EXCP_TRACE
  target/m68k: Fix stack frame for EXCP_ILLEGAL
  target/m68k: Implement TRAPcc
  target/m68k: Implement TPF in terms of TRAPcc
  target/m68k: Implement TRAPV
  target/m68k: Implement FTRAPcc
  tests/tcg/m68k: Add trap.c
  linux-user/strace: Fix print_syscall_err
  linux-user/strace: Adjust get_thread_area for m68k
  target/m68k: Mark helper_raise_exception as noreturn

 target/m68k/cpu.h              |   8 ++
 target/m68k/helper.h           |  14 +--
 linux-user/m68k/cpu_loop.c     |  11 +-
 linux-user/strace.c            |   4 +-
 target/m68k/cpu.c              |   1 +
 target/m68k/op_helper.c        | 173 ++++++++++++++++--------------
 target/m68k/translate.c        | 190 ++++++++++++++++++++++++---------
 tests/tcg/m68k/trap.c          | 129 ++++++++++++++++++++++
 linux-user/strace.list         |   5 +
 tests/tcg/m68k/Makefile.target |   3 +
 10 files changed, 394 insertions(+), 144 deletions(-)
 create mode 100644 tests/tcg/m68k/trap.c

-- 
2.34.1

[PATCH v4 01/17] target/m68k: Raise the TRAPn exception with the correct pc

[Richard Henderson]

Rather than adjust the PC in all of the consumers, raise
the exception with the correct PC in the first place.

Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/m68k/cpu_loop.c | 1 -
 target/m68k/op_helper.c    | 9 ---------
 target/m68k/translate.c    | 2 +-
 3 files changed, 1 insertion(+), 11 deletions(-)

diff --git a/linux-user/m68k/cpu_loop.c b/linux-user/m68k/cpu_loop.c
index d1bf8548b7..56417f7401 100644
--- a/linux-user/m68k/cpu_loop.c
+++ b/linux-user/m68k/cpu_loop.c
@@ -56,7 +56,6 @@ void cpu_loop(CPUM68KState *env)
             {
                 abi_long ret;
                 n = env->dregs[0];
-                env->pc += 2;
                 ret = do_syscall(env,
                                  n,
                                  env->dregs[1],
diff --git a/target/m68k/op_helper.c b/target/m68k/op_helper.c
index 8decc61240..d30f988ae0 100644
--- a/target/m68k/op_helper.c
+++ b/target/m68k/op_helper.c
@@ -217,11 +217,6 @@ static void cf_interrupt_all(CPUM68KState *env, int is_hw)
             cpu_loop_exit(cs);
             return;
         }
-        if (cs->exception_index >= EXCP_TRAP0
-            && cs->exception_index <= EXCP_TRAP15) {
-            /* Move the PC after the trap instruction.  */
-            retaddr += 2;
-        }
     }
 
     vector = cs->exception_index << 2;
@@ -304,10 +299,6 @@ static void m68k_interrupt_all(CPUM68KState *env, int is_hw)
             /* Return from an exception.  */
             m68k_rte(env);
             return;
-        case EXCP_TRAP0 ...  EXCP_TRAP15:
-            /* Move the PC after the trap instruction.  */
-            retaddr += 2;
-            break;
         }
     }
 
diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index 4026572ed8..6d6d026e3c 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -4860,7 +4860,7 @@ DISAS_INSN(wdebug)
 
 DISAS_INSN(trap)
 {
-    gen_exception(s, s->base.pc_next, EXCP_TRAP0 + (insn & 0xf));
+    gen_exception(s, s->pc, EXCP_TRAP0 + (insn & 0xf));
 }
 
 static void gen_load_fcr(DisasContext *s, TCGv res, int reg)
-- 
2.34.1

[PATCH v4 02/17] target/m68k: Switch over exception type in m68k_interrupt_all

[Richard Henderson]

Replace an if ladder with a switch for clarity.

Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/m68k/op_helper.c | 49 +++++++++++++++++++++++++----------------
 1 file changed, 30 insertions(+), 19 deletions(-)

diff --git a/target/m68k/op_helper.c b/target/m68k/op_helper.c
index d30f988ae0..2b94a6ec84 100644
--- a/target/m68k/op_helper.c
+++ b/target/m68k/op_helper.c
@@ -333,7 +333,8 @@ static void m68k_interrupt_all(CPUM68KState *env, int is_hw)
         sp &= ~1;
     }
 
-    if (cs->exception_index == EXCP_ACCESS) {
+    switch (cs->exception_index) {
+    case EXCP_ACCESS:
         if (env->mmu.fault) {
             cpu_abort(cs, "DOUBLE MMU FAULT\n");
         }
@@ -391,29 +392,39 @@ static void m68k_interrupt_all(CPUM68KState *env, int is_hw)
                      "ssw:  %08x ea:   %08x sfc:  %d    dfc: %d\n",
                      env->mmu.ssw, env->mmu.ar, env->sfc, env->dfc);
         }
-    } else if (cs->exception_index == EXCP_ADDRESS) {
+        break;
+
+    case EXCP_ADDRESS:
         do_stack_frame(env, &sp, 2, oldsr, 0, retaddr);
-    } else if (cs->exception_index == EXCP_ILLEGAL ||
-               cs->exception_index == EXCP_DIV0 ||
-               cs->exception_index == EXCP_CHK ||
-               cs->exception_index == EXCP_TRAPCC ||
-               cs->exception_index == EXCP_TRACE) {
+        break;
+
+    case EXCP_ILLEGAL:
+    case EXCP_DIV0:
+    case EXCP_CHK:
+    case EXCP_TRAPCC:
+    case EXCP_TRACE:
         /* FIXME: addr is not only env->pc */
         do_stack_frame(env, &sp, 2, oldsr, env->pc, retaddr);
-    } else if (is_hw && oldsr & SR_M &&
-               cs->exception_index >= EXCP_SPURIOUS &&
-               cs->exception_index <= EXCP_INT_LEVEL_7) {
-        do_stack_frame(env, &sp, 0, oldsr, 0, retaddr);
-        oldsr = sr;
-        env->aregs[7] = sp;
-        cpu_m68k_set_sr(env, sr &= ~SR_M);
-        sp = env->aregs[7];
-        if (!m68k_feature(env, M68K_FEATURE_UNALIGNED_DATA)) {
-            sp &= ~1;
+        break;
+
+    case EXCP_SPURIOUS ... EXCP_INT_LEVEL_7:
+        if (is_hw && oldsr & SR_M) {
+            do_stack_frame(env, &sp, 0, oldsr, 0, retaddr);
+            oldsr = sr;
+            env->aregs[7] = sp;
+            cpu_m68k_set_sr(env, sr &= ~SR_M);
+            sp = env->aregs[7];
+            if (!m68k_feature(env, M68K_FEATURE_UNALIGNED_DATA)) {
+                sp &= ~1;
+            }
+            do_stack_frame(env, &sp, 1, oldsr, 0, retaddr);
+            break;
         }
-        do_stack_frame(env, &sp, 1, oldsr, 0, retaddr);
-    } else {
+        /* fall through */
+
+    default:
         do_stack_frame(env, &sp, 0, oldsr, 0, retaddr);
+        break;
     }
 
     env->aregs[7] = sp;
-- 
2.34.1

[PATCH v4 03/17] target/m68k: Fix coding style in m68k_interrupt_all

[Richard Henderson]

Add parenthesis around & vs &&.

Remove assignment to sr in function call argument -- note that
sr is unused after the call, so the assignment was never needed,
only the result of the & expression.

Suggested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/m68k/op_helper.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/m68k/op_helper.c b/target/m68k/op_helper.c
index 2b94a6ec84..0f41c2dce3 100644
--- a/target/m68k/op_helper.c
+++ b/target/m68k/op_helper.c
@@ -408,11 +408,11 @@ static void m68k_interrupt_all(CPUM68KState *env, int is_hw)
         break;
 
     case EXCP_SPURIOUS ... EXCP_INT_LEVEL_7:
-        if (is_hw && oldsr & SR_M) {
+        if (is_hw && (oldsr & SR_M)) {
             do_stack_frame(env, &sp, 0, oldsr, 0, retaddr);
             oldsr = sr;
             env->aregs[7] = sp;
-            cpu_m68k_set_sr(env, sr &= ~SR_M);
+            cpu_m68k_set_sr(env, sr & ~SR_M);
             sp = env->aregs[7];
             if (!m68k_feature(env, M68K_FEATURE_UNALIGNED_DATA)) {
                 sp &= ~1;
-- 
2.34.1

Re: [PATCH v4 03/17] target/m68k: Fix coding style in m68k_interrupt_all

[Philippe Mathieu-Daudé via]

On 30/4/22 19:53, Richard Henderson wrote:
> Add parenthesis around & vs &&.
> 
> Remove assignment to sr in function call argument -- note that
> sr is unused after the call, so the assignment was never needed,
> only the result of the & expression.
> 
> Suggested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/m68k/op_helper.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)

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

Re: [PATCH v4 03/17] target/m68k: Fix coding style in m68k_interrupt_all

[Laurent Vivier]

Le 30/04/2022 à 19:53, Richard Henderson a écrit :
> Add parenthesis around & vs &&.
> 
> Remove assignment to sr in function call argument -- note that
> sr is unused after the call, so the assignment was never needed,
> only the result of the & expression.
> 
> Suggested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/m68k/op_helper.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target/m68k/op_helper.c b/target/m68k/op_helper.c
> index 2b94a6ec84..0f41c2dce3 100644
> --- a/target/m68k/op_helper.c
> +++ b/target/m68k/op_helper.c
> @@ -408,11 +408,11 @@ static void m68k_interrupt_all(CPUM68KState *env, int is_hw)
>           break;
>   
>       case EXCP_SPURIOUS ... EXCP_INT_LEVEL_7:
> -        if (is_hw && oldsr & SR_M) {
> +        if (is_hw && (oldsr & SR_M)) {
>               do_stack_frame(env, &sp, 0, oldsr, 0, retaddr);
>               oldsr = sr;
>               env->aregs[7] = sp;
> -            cpu_m68k_set_sr(env, sr &= ~SR_M);
> +            cpu_m68k_set_sr(env, sr & ~SR_M);
>               sp = env->aregs[7];
>               if (!m68k_feature(env, M68K_FEATURE_UNALIGNED_DATA)) {
>                   sp &= ~1;

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

[PATCH v4 04/17] linux-user/m68k: Handle EXCP_TRAP1 through EXCP_TRAP15

[Richard Henderson]

These are raised by guest instructions, and should not
fall through into the default abort case.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/m68k/cpu_loop.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/linux-user/m68k/cpu_loop.c b/linux-user/m68k/cpu_loop.c
index 56417f7401..6ca3e1e63a 100644
--- a/linux-user/m68k/cpu_loop.c
+++ b/linux-user/m68k/cpu_loop.c
@@ -44,6 +44,7 @@ void cpu_loop(CPUM68KState *env)
         case EXCP_ILLEGAL:
         case EXCP_LINEA:
         case EXCP_LINEF:
+        case EXCP_TRAP0 + 1 ... EXCP_TRAP0 + 14:
             force_sig_fault(TARGET_SIGILL, TARGET_ILL_ILLOPN, env->pc);
             break;
         case EXCP_CHK:
@@ -76,6 +77,7 @@ void cpu_loop(CPUM68KState *env)
             /* just indicate that signals should be handled asap */
             break;
         case EXCP_DEBUG:
+        case EXCP_TRAP15:
             force_sig_fault(TARGET_SIGTRAP, TARGET_TRAP_BRKPT, env->pc);
             break;
         case EXCP_ATOMIC:
-- 
2.34.1

Re: [PATCH v4 04/17] linux-user/m68k: Handle EXCP_TRAP1 through EXCP_TRAP15

[Laurent Vivier]

Le 30/04/2022 à 19:53, Richard Henderson a écrit :
> These are raised by guest instructions, and should not
> fall through into the default abort case.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   linux-user/m68k/cpu_loop.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/linux-user/m68k/cpu_loop.c b/linux-user/m68k/cpu_loop.c
> index 56417f7401..6ca3e1e63a 100644
> --- a/linux-user/m68k/cpu_loop.c
> +++ b/linux-user/m68k/cpu_loop.c
> @@ -44,6 +44,7 @@ void cpu_loop(CPUM68KState *env)
>           case EXCP_ILLEGAL:
>           case EXCP_LINEA:
>           case EXCP_LINEF:
> +        case EXCP_TRAP0 + 1 ... EXCP_TRAP0 + 14:
>               force_sig_fault(TARGET_SIGILL, TARGET_ILL_ILLOPN, env->pc);

In kernel, VEC_TRAP1 to VEC_TRAP14 use ILL_ILLTRP for si_code.

Thanks,
Laurent

[PATCH v4 05/17] target/m68k: Remove retaddr in m68k_interrupt_all

[Richard Henderson]

The only value this variable holds is now env->pc.

Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/m68k/op_helper.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/target/m68k/op_helper.c b/target/m68k/op_helper.c
index 0f41c2dce3..777869790b 100644
--- a/target/m68k/op_helper.c
+++ b/target/m68k/op_helper.c
@@ -287,12 +287,9 @@ static void m68k_interrupt_all(CPUM68KState *env, int is_hw)
 {
     CPUState *cs = env_cpu(env);
     uint32_t sp;
-    uint32_t retaddr;
     uint32_t vector;
     uint16_t sr, oldsr;
 
-    retaddr = env->pc;
-
     if (!is_hw) {
         switch (cs->exception_index) {
         case EXCP_RTE:
@@ -385,7 +382,7 @@ static void m68k_interrupt_all(CPUM68KState *env, int is_hw)
         sp -= 4;
         cpu_stl_mmuidx_ra(env, sp, env->mmu.ar, MMU_KERNEL_IDX, 0);
 
-        do_stack_frame(env, &sp, 7, oldsr, 0, retaddr);
+        do_stack_frame(env, &sp, 7, oldsr, 0, env->pc);
         env->mmu.fault = false;
         if (qemu_loglevel_mask(CPU_LOG_INT)) {
             qemu_log("            "
@@ -395,7 +392,7 @@ static void m68k_interrupt_all(CPUM68KState *env, int is_hw)
         break;
 
     case EXCP_ADDRESS:
-        do_stack_frame(env, &sp, 2, oldsr, 0, retaddr);
+        do_stack_frame(env, &sp, 2, oldsr, 0, env->pc);
         break;
 
     case EXCP_ILLEGAL:
@@ -404,12 +401,12 @@ static void m68k_interrupt_all(CPUM68KState *env, int is_hw)
     case EXCP_TRAPCC:
     case EXCP_TRACE:
         /* FIXME: addr is not only env->pc */
-        do_stack_frame(env, &sp, 2, oldsr, env->pc, retaddr);
+        do_stack_frame(env, &sp, 2, oldsr, env->pc, env->pc);
         break;
 
     case EXCP_SPURIOUS ... EXCP_INT_LEVEL_7:
         if (is_hw && (oldsr & SR_M)) {
-            do_stack_frame(env, &sp, 0, oldsr, 0, retaddr);
+            do_stack_frame(env, &sp, 0, oldsr, 0, env->pc);
             oldsr = sr;
             env->aregs[7] = sp;
             cpu_m68k_set_sr(env, sr & ~SR_M);
@@ -417,13 +414,13 @@ static void m68k_interrupt_all(CPUM68KState *env, int is_hw)
             if (!m68k_feature(env, M68K_FEATURE_UNALIGNED_DATA)) {
                 sp &= ~1;
             }
-            do_stack_frame(env, &sp, 1, oldsr, 0, retaddr);
+            do_stack_frame(env, &sp, 1, oldsr, 0, env->pc);
             break;
         }
         /* fall through */
 
     default:
-        do_stack_frame(env, &sp, 0, oldsr, 0, retaddr);
+        do_stack_frame(env, &sp, 0, oldsr, 0, env->pc);
         break;
     }
 
-- 
2.34.1

[PATCH v4 06/17] target/m68k: Fix address argument for EXCP_CHK

[Richard Henderson]

According to the M68040 Users Manual, section 8.4.3,
Six word stack frame (format 2), CHK, CHK2 (and others)
are supposed to record the next insn in PC and the
address of the trapping instruction in ADDRESS.

Create a raise_exception_format2 function to centralize recording
of the trapping pc in mmu.ar, plus advancing to the next insn.

Update m68k_interrupt_all to pass mmu.ar to do_stack_frame.
Update cpu_loop to pass mmu.ar to siginfo.si_addr, as the
kernel does in trap_c().

Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/m68k/cpu.h          |  6 +++++
 linux-user/m68k/cpu_loop.c |  2 +-
 target/m68k/op_helper.c    | 54 ++++++++++++++++++++------------------
 3 files changed, 36 insertions(+), 26 deletions(-)

diff --git a/target/m68k/cpu.h b/target/m68k/cpu.h
index 9b3bf7a448..558c3c67d6 100644
--- a/target/m68k/cpu.h
+++ b/target/m68k/cpu.h
@@ -122,6 +122,12 @@ typedef struct CPUArchState {
 
     /* MMU status.  */
     struct {
+        /*
+         * Holds the "address" value in between raising an exception
+         * and creation of the exception stack frame.
+         * Used for both Format 7 exceptions (Access, i.e. mmu)
+         * and Format 2 exceptions (chk, div0, trapcc, etc).
+         */
         uint32_t ar;
         uint32_t ssw;
         /* 68040 */
diff --git a/linux-user/m68k/cpu_loop.c b/linux-user/m68k/cpu_loop.c
index 6ca3e1e63a..5d9c1f3753 100644
--- a/linux-user/m68k/cpu_loop.c
+++ b/linux-user/m68k/cpu_loop.c
@@ -48,7 +48,7 @@ void cpu_loop(CPUM68KState *env)
             force_sig_fault(TARGET_SIGILL, TARGET_ILL_ILLOPN, env->pc);
             break;
         case EXCP_CHK:
-            force_sig_fault(TARGET_SIGFPE, TARGET_FPE_INTOVF, env->pc);
+            force_sig_fault(TARGET_SIGFPE, TARGET_FPE_INTOVF, env->mmu.ar);
             break;
         case EXCP_DIV0:
             force_sig_fault(TARGET_SIGFPE, TARGET_FPE_INTDIV, env->pc);
diff --git a/target/m68k/op_helper.c b/target/m68k/op_helper.c
index 777869790b..750d65576f 100644
--- a/target/m68k/op_helper.c
+++ b/target/m68k/op_helper.c
@@ -397,13 +397,16 @@ static void m68k_interrupt_all(CPUM68KState *env, int is_hw)
 
     case EXCP_ILLEGAL:
     case EXCP_DIV0:
-    case EXCP_CHK:
     case EXCP_TRAPCC:
     case EXCP_TRACE:
         /* FIXME: addr is not only env->pc */
         do_stack_frame(env, &sp, 2, oldsr, env->pc, env->pc);
         break;
 
+    case EXCP_CHK:
+        do_stack_frame(env, &sp, 2, oldsr, env->mmu.ar, env->pc);
+        break;
+
     case EXCP_SPURIOUS ... EXCP_INT_LEVEL_7:
         if (is_hw && (oldsr & SR_M)) {
             do_stack_frame(env, &sp, 0, oldsr, 0, env->pc);
@@ -548,6 +551,29 @@ void HELPER(raise_exception)(CPUM68KState *env, uint32_t tt)
     raise_exception(env, tt);
 }
 
+G_NORETURN static void
+raise_exception_format2(CPUM68KState *env, int tt, int ilen, uintptr_t raddr)
+{
+    CPUState *cs = env_cpu(env);
+
+    cs->exception_index = tt;
+
+    /* Recover PC and CC_OP for the beginning of the insn.  */
+    cpu_restore_state(cs, raddr, true);
+
+    /* Flags are current in env->cc_*, or are undefined. */
+    env->cc_op = CC_OP_FLAGS;
+
+    /*
+     * Remember original pc in mmu.ar, for the Format 2 stack frame.
+     * Adjust PC to end of the insn.
+     */
+    env->mmu.ar = env->pc;
+    env->pc += ilen;
+
+    cpu_loop_exit(cs);
+}
+
 void HELPER(divuw)(CPUM68KState *env, int destr, uint32_t den)
 {
     uint32_t num = env->dregs[destr];
@@ -1065,18 +1091,7 @@ void HELPER(chk)(CPUM68KState *env, int32_t val, int32_t ub)
     env->cc_c = 0 <= ub ? val < 0 || val > ub : val > ub && val < 0;
 
     if (val < 0 || val > ub) {
-        CPUState *cs = env_cpu(env);
-
-        /* Recover PC and CC_OP for the beginning of the insn.  */
-        cpu_restore_state(cs, GETPC(), true);
-
-        /* flags have been modified by gen_flush_flags() */
-        env->cc_op = CC_OP_FLAGS;
-        /* Adjust PC to end of the insn.  */
-        env->pc += 2;
-
-        cs->exception_index = EXCP_CHK;
-        cpu_loop_exit(cs);
+        raise_exception_format2(env, EXCP_CHK, 2, GETPC());
     }
 }
 
@@ -1097,17 +1112,6 @@ void HELPER(chk2)(CPUM68KState *env, int32_t val, int32_t lb, int32_t ub)
     env->cc_c = lb <= ub ? val < lb || val > ub : val > ub && val < lb;
 
     if (env->cc_c) {
-        CPUState *cs = env_cpu(env);
-
-        /* Recover PC and CC_OP for the beginning of the insn.  */
-        cpu_restore_state(cs, GETPC(), true);
-
-        /* flags have been modified by gen_flush_flags() */
-        env->cc_op = CC_OP_FLAGS;
-        /* Adjust PC to end of the insn.  */
-        env->pc += 4;
-
-        cs->exception_index = EXCP_CHK;
-        cpu_loop_exit(cs);
+        raise_exception_format2(env, EXCP_CHK, 4, GETPC());
     }
 }
-- 
2.34.1

[PATCH v4 07/17] target/m68k: Fix pc, c flag, and address argument for EXCP_DIV0

[Richard Henderson]

According to the M68040 Users Manual, section 8.4.3,
Six word stack frame (format 2), Zero Div (and others)
is supposed to record the next insn in PC and the
address of the trapping instruction in ADDRESS.

While the N, Z and V flags are documented to be undefine on DIV0,
the C flag is documented as always cleared.

Update helper_div* to take the instruction length as an argument
and use raise_exception_format2.  Hoist the reset of the C flag
above the division by zero check.

Update m68k_interrupt_all to pass mmu.ar to do_stack_frame.
Update cpu_loop to pass mmu.ar to siginfo.si_addr, as the
kernel does in trap_c().

Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/m68k/helper.h       | 12 +++++-----
 linux-user/m68k/cpu_loop.c |  2 +-
 target/m68k/op_helper.c    | 48 +++++++++++++++++++++++---------------
 target/m68k/translate.c    | 33 +++++++++++++-------------
 4 files changed, 52 insertions(+), 43 deletions(-)

diff --git a/target/m68k/helper.h b/target/m68k/helper.h
index 0a6b4146f6..f016c4c1c2 100644
--- a/target/m68k/helper.h
+++ b/target/m68k/helper.h
@@ -1,12 +1,12 @@
 DEF_HELPER_1(bitrev, i32, i32)
 DEF_HELPER_1(ff1, i32, i32)
 DEF_HELPER_FLAGS_2(sats, TCG_CALL_NO_RWG_SE, i32, i32, i32)
-DEF_HELPER_3(divuw, void, env, int, i32)
-DEF_HELPER_3(divsw, void, env, int, s32)
-DEF_HELPER_4(divul, void, env, int, int, i32)
-DEF_HELPER_4(divsl, void, env, int, int, s32)
-DEF_HELPER_4(divull, void, env, int, int, i32)
-DEF_HELPER_4(divsll, void, env, int, int, s32)
+DEF_HELPER_4(divuw, void, env, int, i32, int)
+DEF_HELPER_4(divsw, void, env, int, s32, int)
+DEF_HELPER_5(divul, void, env, int, int, i32, int)
+DEF_HELPER_5(divsl, void, env, int, int, s32, int)
+DEF_HELPER_5(divull, void, env, int, int, i32, int)
+DEF_HELPER_5(divsll, void, env, int, int, s32, int)
 DEF_HELPER_2(set_sr, void, env, i32)
 DEF_HELPER_3(cf_movec_to, void, env, i32, i32)
 DEF_HELPER_3(m68k_movec_to, void, env, i32, i32)
diff --git a/linux-user/m68k/cpu_loop.c b/linux-user/m68k/cpu_loop.c
index 5d9c1f3753..45419d4471 100644
--- a/linux-user/m68k/cpu_loop.c
+++ b/linux-user/m68k/cpu_loop.c
@@ -51,7 +51,7 @@ void cpu_loop(CPUM68KState *env)
             force_sig_fault(TARGET_SIGFPE, TARGET_FPE_INTOVF, env->mmu.ar);
             break;
         case EXCP_DIV0:
-            force_sig_fault(TARGET_SIGFPE, TARGET_FPE_INTDIV, env->pc);
+            force_sig_fault(TARGET_SIGFPE, TARGET_FPE_INTDIV, env->mmu.ar);
             break;
         case EXCP_TRAP0:
             {
diff --git a/target/m68k/op_helper.c b/target/m68k/op_helper.c
index 750d65576f..729ee0e934 100644
--- a/target/m68k/op_helper.c
+++ b/target/m68k/op_helper.c
@@ -396,7 +396,6 @@ static void m68k_interrupt_all(CPUM68KState *env, int is_hw)
         break;
 
     case EXCP_ILLEGAL:
-    case EXCP_DIV0:
     case EXCP_TRAPCC:
     case EXCP_TRACE:
         /* FIXME: addr is not only env->pc */
@@ -404,6 +403,7 @@ static void m68k_interrupt_all(CPUM68KState *env, int is_hw)
         break;
 
     case EXCP_CHK:
+    case EXCP_DIV0:
         do_stack_frame(env, &sp, 2, oldsr, env->mmu.ar, env->pc);
         break;
 
@@ -574,18 +574,19 @@ raise_exception_format2(CPUM68KState *env, int tt, int ilen, uintptr_t raddr)
     cpu_loop_exit(cs);
 }
 
-void HELPER(divuw)(CPUM68KState *env, int destr, uint32_t den)
+void HELPER(divuw)(CPUM68KState *env, int destr, uint32_t den, int ilen)
 {
     uint32_t num = env->dregs[destr];
     uint32_t quot, rem;
 
+    env->cc_c = 0; /* always cleared, even if div0 */
+
     if (den == 0) {
-        raise_exception_ra(env, EXCP_DIV0, GETPC());
+        raise_exception_format2(env, EXCP_DIV0, ilen, GETPC());
     }
     quot = num / den;
     rem = num % den;
 
-    env->cc_c = 0; /* always cleared, even if overflow */
     if (quot > 0xffff) {
         env->cc_v = -1;
         /*
@@ -601,18 +602,19 @@ void HELPER(divuw)(CPUM68KState *env, int destr, uint32_t den)
     env->cc_v = 0;
 }
 
-void HELPER(divsw)(CPUM68KState *env, int destr, int32_t den)
+void HELPER(divsw)(CPUM68KState *env, int destr, int32_t den, int ilen)
 {
     int32_t num = env->dregs[destr];
     uint32_t quot, rem;
 
+    env->cc_c = 0; /* always cleared, even if overflow/div0 */
+
     if (den == 0) {
-        raise_exception_ra(env, EXCP_DIV0, GETPC());
+        raise_exception_format2(env, EXCP_DIV0, ilen, GETPC());
     }
     quot = num / den;
     rem = num % den;
 
-    env->cc_c = 0; /* always cleared, even if overflow */
     if (quot != (int16_t)quot) {
         env->cc_v = -1;
         /* nothing else is modified */
@@ -629,18 +631,20 @@ void HELPER(divsw)(CPUM68KState *env, int destr, int32_t den)
     env->cc_v = 0;
 }
 
-void HELPER(divul)(CPUM68KState *env, int numr, int regr, uint32_t den)
+void HELPER(divul)(CPUM68KState *env, int numr, int regr,
+                   uint32_t den, int ilen)
 {
     uint32_t num = env->dregs[numr];
     uint32_t quot, rem;
 
+    env->cc_c = 0; /* always cleared, even if div0 */
+
     if (den == 0) {
-        raise_exception_ra(env, EXCP_DIV0, GETPC());
+        raise_exception_format2(env, EXCP_DIV0, ilen, GETPC());
     }
     quot = num / den;
     rem = num % den;
 
-    env->cc_c = 0;
     env->cc_z = quot;
     env->cc_n = quot;
     env->cc_v = 0;
@@ -657,18 +661,20 @@ void HELPER(divul)(CPUM68KState *env, int numr, int regr, uint32_t den)
     }
 }
 
-void HELPER(divsl)(CPUM68KState *env, int numr, int regr, int32_t den)
+void HELPER(divsl)(CPUM68KState *env, int numr, int regr,
+                   int32_t den, int ilen)
 {
     int32_t num = env->dregs[numr];
     int32_t quot, rem;
 
+    env->cc_c = 0; /* always cleared, even if overflow/div0 */
+
     if (den == 0) {
-        raise_exception_ra(env, EXCP_DIV0, GETPC());
+        raise_exception_format2(env, EXCP_DIV0, ilen, GETPC());
     }
     quot = num / den;
     rem = num % den;
 
-    env->cc_c = 0;
     env->cc_z = quot;
     env->cc_n = quot;
     env->cc_v = 0;
@@ -685,19 +691,21 @@ void HELPER(divsl)(CPUM68KState *env, int numr, int regr, int32_t den)
     }
 }
 
-void HELPER(divull)(CPUM68KState *env, int numr, int regr, uint32_t den)
+void HELPER(divull)(CPUM68KState *env, int numr, int regr,
+                    uint32_t den, int ilen)
 {
     uint64_t num = deposit64(env->dregs[numr], 32, 32, env->dregs[regr]);
     uint64_t quot;
     uint32_t rem;
 
+    env->cc_c = 0; /* always cleared, even if overflow/div0 */
+
     if (den == 0) {
-        raise_exception_ra(env, EXCP_DIV0, GETPC());
+        raise_exception_format2(env, EXCP_DIV0, ilen, GETPC());
     }
     quot = num / den;
     rem = num % den;
 
-    env->cc_c = 0; /* always cleared, even if overflow */
     if (quot > 0xffffffffULL) {
         env->cc_v = -1;
         /*
@@ -720,19 +728,21 @@ void HELPER(divull)(CPUM68KState *env, int numr, int regr, uint32_t den)
     env->dregs[numr] = quot;
 }
 
-void HELPER(divsll)(CPUM68KState *env, int numr, int regr, int32_t den)
+void HELPER(divsll)(CPUM68KState *env, int numr, int regr,
+                    int32_t den, int ilen)
 {
     int64_t num = deposit64(env->dregs[numr], 32, 32, env->dregs[regr]);
     int64_t quot;
     int32_t rem;
 
+    env->cc_c = 0; /* always cleared, even if overflow/div0 */
+
     if (den == 0) {
-        raise_exception_ra(env, EXCP_DIV0, GETPC());
+        raise_exception_format2(env, EXCP_DIV0, ilen, GETPC());
     }
     quot = num / den;
     rem = num % den;
 
-    env->cc_c = 0; /* always cleared, even if overflow */
     if (quot != (int32_t)quot) {
         env->cc_v = -1;
         /*
diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index 6d6d026e3c..d775345bfa 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -1601,6 +1601,7 @@ DISAS_INSN(divw)
     int sign;
     TCGv src;
     TCGv destr;
+    TCGv ilen;
 
     /* divX.w <EA>,Dn    32/16 -> 16r:16q */
 
@@ -1609,20 +1610,20 @@ DISAS_INSN(divw)
     /* dest.l / src.w */
 
     SRC_EA(env, src, OS_WORD, sign, NULL);
-    destr = tcg_const_i32(REG(insn, 9));
+    destr = tcg_constant_i32(REG(insn, 9));
+    ilen = tcg_constant_i32(s->pc - s->base.pc_next);
     if (sign) {
-        gen_helper_divsw(cpu_env, destr, src);
+        gen_helper_divsw(cpu_env, destr, src, ilen);
     } else {
-        gen_helper_divuw(cpu_env, destr, src);
+        gen_helper_divuw(cpu_env, destr, src, ilen);
     }
-    tcg_temp_free(destr);
 
     set_cc_op(s, CC_OP_FLAGS);
 }
 
 DISAS_INSN(divl)
 {
-    TCGv num, reg, den;
+    TCGv num, reg, den, ilen;
     int sign;
     uint16_t ext;
 
@@ -1639,15 +1640,14 @@ DISAS_INSN(divl)
         /* divX.l <EA>, Dr:Dq    64/32 -> 32r:32q */
 
         SRC_EA(env, den, OS_LONG, 0, NULL);
-        num = tcg_const_i32(REG(ext, 12));
-        reg = tcg_const_i32(REG(ext, 0));
+        num = tcg_constant_i32(REG(ext, 12));
+        reg = tcg_constant_i32(REG(ext, 0));
+        ilen = tcg_constant_i32(s->pc - s->base.pc_next);
         if (sign) {
-            gen_helper_divsll(cpu_env, num, reg, den);
+            gen_helper_divsll(cpu_env, num, reg, den, ilen);
         } else {
-            gen_helper_divull(cpu_env, num, reg, den);
+            gen_helper_divull(cpu_env, num, reg, den, ilen);
         }
-        tcg_temp_free(reg);
-        tcg_temp_free(num);
         set_cc_op(s, CC_OP_FLAGS);
         return;
     }
@@ -1656,15 +1656,14 @@ DISAS_INSN(divl)
     /* divXl.l <EA>, Dr:Dq    32/32 -> 32r:32q */
 
     SRC_EA(env, den, OS_LONG, 0, NULL);
-    num = tcg_const_i32(REG(ext, 12));
-    reg = tcg_const_i32(REG(ext, 0));
+    num = tcg_constant_i32(REG(ext, 12));
+    reg = tcg_constant_i32(REG(ext, 0));
+    ilen = tcg_constant_i32(s->pc - s->base.pc_next);
     if (sign) {
-        gen_helper_divsl(cpu_env, num, reg, den);
+        gen_helper_divsl(cpu_env, num, reg, den, ilen);
     } else {
-        gen_helper_divul(cpu_env, num, reg, den);
+        gen_helper_divul(cpu_env, num, reg, den, ilen);
     }
-    tcg_temp_free(reg);
-    tcg_temp_free(num);
 
     set_cc_op(s, CC_OP_FLAGS);
 }
-- 
2.34.1

[PATCH v4 08/17] target/m68k: Fix address argument for EXCP_TRACE

[Richard Henderson]

According to the M68040 Users Manual, section 8.4.3,
Six word stack frame (format 2), Trace (and others) is
supposed to record the next insn in PC and the address
of the trapping instruction in ADDRESS.

Create gen_raise_exception_format2 to record the trapping
pc in env->mmu.ar.  Update m68k_interrupt_all to pass the
value to do_stack_frame.  Update cpu_loop to handle EXCP_TRACE.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/m68k/cpu_loop.c |  3 +++
 target/m68k/op_helper.c    |  2 +-
 target/m68k/translate.c    | 49 +++++++++++++++++++++++++-------------
 3 files changed, 36 insertions(+), 18 deletions(-)

diff --git a/linux-user/m68k/cpu_loop.c b/linux-user/m68k/cpu_loop.c
index 45419d4471..000bb44cc3 100644
--- a/linux-user/m68k/cpu_loop.c
+++ b/linux-user/m68k/cpu_loop.c
@@ -53,6 +53,9 @@ void cpu_loop(CPUM68KState *env)
         case EXCP_DIV0:
             force_sig_fault(TARGET_SIGFPE, TARGET_FPE_INTDIV, env->mmu.ar);
             break;
+        case EXCP_TRACE:
+            force_sig_fault(TARGET_SIGTRAP, TARGET_TRAP_TRACE, env->mmu.ar);
+            break;
         case EXCP_TRAP0:
             {
                 abi_long ret;
diff --git a/target/m68k/op_helper.c b/target/m68k/op_helper.c
index 729ee0e934..3cb71c9140 100644
--- a/target/m68k/op_helper.c
+++ b/target/m68k/op_helper.c
@@ -397,13 +397,13 @@ static void m68k_interrupt_all(CPUM68KState *env, int is_hw)
 
     case EXCP_ILLEGAL:
     case EXCP_TRAPCC:
-    case EXCP_TRACE:
         /* 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:
         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 d775345bfa..399d9232e4 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -114,6 +114,7 @@ typedef struct DisasContext {
     DisasContextBase base;
     CPUM68KState *env;
     target_ulong pc;
+    target_ulong pc_prev;
     CCOp cc_op; /* Current CC operation */
     int cc_op_synced;
     TCGv_i64 mactmp;
@@ -298,6 +299,21 @@ static void gen_raise_exception(int nr)
     tcg_temp_free_i32(tmp);
 }
 
+static void gen_raise_exception_format2(DisasContext *s, int nr,
+                                        target_ulong this_pc)
+{
+    /*
+     * Pass the address of the insn to the exception handler,
+     * for recording in the Format $2 (6-word) stack frame.
+     * Re-use mmu.ar for the purpose, since that's only valid
+     * after tlb_fill.
+     */
+    tcg_gen_st_i32(tcg_constant_i32(this_pc), cpu_env,
+                   offsetof(CPUM68KState, mmu.ar));
+    gen_raise_exception(nr);
+    s->base.is_jmp = DISAS_NORETURN;
+}
+
 static void gen_exception(DisasContext *s, uint32_t dest, int nr)
 {
     update_cc_op(s);
@@ -1494,12 +1510,13 @@ static void gen_exit_tb(DisasContext *s)
     } while (0)
 
 /* Generate a jump to an immediate address.  */
-static void gen_jmp_tb(DisasContext *s, int n, uint32_t dest)
+static void gen_jmp_tb(DisasContext *s, int n, target_ulong dest,
+                       target_ulong src)
 {
     if (unlikely(s->ss_active)) {
         update_cc_op(s);
         tcg_gen_movi_i32(QREG_PC, dest);
-        gen_raise_exception(EXCP_TRACE);
+        gen_raise_exception_format2(s, EXCP_TRACE, src);
     } else if (translator_use_goto_tb(&s->base, dest)) {
         tcg_gen_goto_tb(n);
         tcg_gen_movi_i32(QREG_PC, dest);
@@ -1548,9 +1565,9 @@ DISAS_INSN(dbcc)
     tcg_gen_addi_i32(tmp, tmp, -1);
     gen_partset_reg(OS_WORD, reg, tmp);
     tcg_gen_brcondi_i32(TCG_COND_EQ, tmp, -1, l1);
-    gen_jmp_tb(s, 1, base + offset);
+    gen_jmp_tb(s, 1, base + offset, s->base.pc_next);
     gen_set_label(l1);
-    gen_jmp_tb(s, 0, s->pc);
+    gen_jmp_tb(s, 0, s->pc, s->base.pc_next);
 }
 
 DISAS_INSN(undef_mac)
@@ -3096,13 +3113,13 @@ DISAS_INSN(branch)
         /* Bcc */
         TCGLabel *l1 = gen_new_label();
         gen_jmpcc(s, ((insn >> 8) & 0xf) ^ 1, l1);
-        gen_jmp_tb(s, 1, base + offset);
+        gen_jmp_tb(s, 1, base + offset, s->base.pc_next);
         gen_set_label(l1);
-        gen_jmp_tb(s, 0, s->pc);
+        gen_jmp_tb(s, 0, s->pc, s->base.pc_next);
     } else {
         /* Unconditional branch.  */
         update_cc_op(s);
-        gen_jmp_tb(s, 0, base + offset);
+        gen_jmp_tb(s, 0, base + offset, s->base.pc_next);
     }
 }
 
@@ -5485,9 +5502,9 @@ DISAS_INSN(fbcc)
     l1 = gen_new_label();
     update_cc_op(s);
     gen_fjmpcc(s, insn & 0x3f, l1);
-    gen_jmp_tb(s, 0, s->pc);
+    gen_jmp_tb(s, 0, s->pc, s->base.pc_next);
     gen_set_label(l1);
-    gen_jmp_tb(s, 1, base + offset);
+    gen_jmp_tb(s, 1, base + offset, s->base.pc_next);
 }
 
 DISAS_INSN(fscc)
@@ -6158,6 +6175,8 @@ static void m68k_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cpu)
 
     dc->env = env;
     dc->pc = dc->base.pc_first;
+    /* This value will always be filled in properly before m68k_tr_tb_stop. */
+    dc->pc_prev = 0xdeadbeef;
     dc->cc_op = CC_OP_DYNAMIC;
     dc->cc_op_synced = 1;
     dc->done_mac = 0;
@@ -6191,6 +6210,7 @@ static void m68k_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu)
     do_writebacks(dc);
     do_release(dc);
 
+    dc->pc_prev = dc->base.pc_next;
     dc->base.pc_next = dc->pc;
 
     if (dc->base.is_jmp == DISAS_NEXT) {
@@ -6225,17 +6245,12 @@ static void m68k_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
         break;
     case DISAS_TOO_MANY:
         update_cc_op(dc);
-        if (dc->ss_active) {
-            tcg_gen_movi_i32(QREG_PC, dc->pc);
-            gen_raise_exception(EXCP_TRACE);
-        } else {
-            gen_jmp_tb(dc, 0, dc->pc);
-        }
+        gen_jmp_tb(dc, 0, dc->pc, dc->pc_prev);
         break;
     case DISAS_JUMP:
         /* We updated CC_OP and PC in gen_jmp/gen_jmp_im.  */
         if (dc->ss_active) {
-            gen_raise_exception(EXCP_TRACE);
+            gen_raise_exception_format2(dc, EXCP_TRACE, dc->pc_prev);
         } else {
             tcg_gen_lookup_and_goto_ptr();
         }
@@ -6246,7 +6261,7 @@ static void m68k_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
          * other state that may require returning to the main loop.
          */
         if (dc->ss_active) {
-            gen_raise_exception(EXCP_TRACE);
+            gen_raise_exception_format2(dc, EXCP_TRACE, dc->pc_prev);
         } else {
             tcg_gen_exit_tb(NULL, 0);
         }
-- 
2.34.1

Re: [PATCH v4 08/17] target/m68k: Fix address argument for EXCP_TRACE

[Laurent Vivier]

Le 30/04/2022 à 19:53, Richard Henderson a écrit :
> According to the M68040 Users Manual, section 8.4.3,
> Six word stack frame (format 2), Trace (and others) is
> supposed to record the next insn in PC and the address
> of the trapping instruction in ADDRESS.
> 
> Create gen_raise_exception_format2 to record the trapping
> pc in env->mmu.ar.  Update m68k_interrupt_all to pass the
> value to do_stack_frame.  Update cpu_loop to handle EXCP_TRACE.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   linux-user/m68k/cpu_loop.c |  3 +++
>   target/m68k/op_helper.c    |  2 +-
>   target/m68k/translate.c    | 49 +++++++++++++++++++++++++-------------
>   3 files changed, 36 insertions(+), 18 deletions(-)
> 
> diff --git a/linux-user/m68k/cpu_loop.c b/linux-user/m68k/cpu_loop.c
> index 45419d4471..000bb44cc3 100644
> --- a/linux-user/m68k/cpu_loop.c
> +++ b/linux-user/m68k/cpu_loop.c
> @@ -53,6 +53,9 @@ void cpu_loop(CPUM68KState *env)
>           case EXCP_DIV0:
>               force_sig_fault(TARGET_SIGFPE, TARGET_FPE_INTDIV, env->mmu.ar);
>               break;
> +        case EXCP_TRACE:
> +            force_sig_fault(TARGET_SIGTRAP, TARGET_TRAP_TRACE, env->mmu.ar);
> +            break;
>           case EXCP_TRAP0:
>               {
>                   abi_long ret;
> diff --git a/target/m68k/op_helper.c b/target/m68k/op_helper.c
> index 729ee0e934..3cb71c9140 100644
> --- a/target/m68k/op_helper.c
> +++ b/target/m68k/op_helper.c
> @@ -397,13 +397,13 @@ static void m68k_interrupt_all(CPUM68KState *env, int is_hw)
>   
>       case EXCP_ILLEGAL:
>       case EXCP_TRAPCC:
> -    case EXCP_TRACE:
>           /* 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:
>           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 d775345bfa..399d9232e4 100644
> --- a/target/m68k/translate.c
> +++ b/target/m68k/translate.c
> @@ -114,6 +114,7 @@ typedef struct DisasContext {
>       DisasContextBase base;
>       CPUM68KState *env;
>       target_ulong pc;
> +    target_ulong pc_prev;
>       CCOp cc_op; /* Current CC operation */
>       int cc_op_synced;
>       TCGv_i64 mactmp;
> @@ -298,6 +299,21 @@ static void gen_raise_exception(int nr)
>       tcg_temp_free_i32(tmp);
>   }
>   
> +static void gen_raise_exception_format2(DisasContext *s, int nr,
> +                                        target_ulong this_pc)
> +{
> +    /*
> +     * Pass the address of the insn to the exception handler,
> +     * for recording in the Format $2 (6-word) stack frame.
> +     * Re-use mmu.ar for the purpose, since that's only valid
> +     * after tlb_fill.
> +     */
> +    tcg_gen_st_i32(tcg_constant_i32(this_pc), cpu_env,
> +                   offsetof(CPUM68KState, mmu.ar));
> +    gen_raise_exception(nr);
> +    s->base.is_jmp = DISAS_NORETURN;
> +}
> +
>   static void gen_exception(DisasContext *s, uint32_t dest, int nr)
>   {
>       update_cc_op(s);
> @@ -1494,12 +1510,13 @@ static void gen_exit_tb(DisasContext *s)
>       } while (0)
>   
>   /* Generate a jump to an immediate address.  */
> -static void gen_jmp_tb(DisasContext *s, int n, uint32_t dest)
> +static void gen_jmp_tb(DisasContext *s, int n, target_ulong dest,
> +                       target_ulong src)
>   {
>       if (unlikely(s->ss_active)) {
>           update_cc_op(s);
>           tcg_gen_movi_i32(QREG_PC, dest);
> -        gen_raise_exception(EXCP_TRACE);
> +        gen_raise_exception_format2(s, EXCP_TRACE, src);
>       } else if (translator_use_goto_tb(&s->base, dest)) {
>           tcg_gen_goto_tb(n);
>           tcg_gen_movi_i32(QREG_PC, dest);
> @@ -1548,9 +1565,9 @@ DISAS_INSN(dbcc)
>       tcg_gen_addi_i32(tmp, tmp, -1);
>       gen_partset_reg(OS_WORD, reg, tmp);
>       tcg_gen_brcondi_i32(TCG_COND_EQ, tmp, -1, l1);
> -    gen_jmp_tb(s, 1, base + offset);
> +    gen_jmp_tb(s, 1, base + offset, s->base.pc_next);
>       gen_set_label(l1);
> -    gen_jmp_tb(s, 0, s->pc);
> +    gen_jmp_tb(s, 0, s->pc, s->base.pc_next);
>   }
>   
>   DISAS_INSN(undef_mac)
> @@ -3096,13 +3113,13 @@ DISAS_INSN(branch)
>           /* Bcc */
>           TCGLabel *l1 = gen_new_label();
>           gen_jmpcc(s, ((insn >> 8) & 0xf) ^ 1, l1);
> -        gen_jmp_tb(s, 1, base + offset);
> +        gen_jmp_tb(s, 1, base + offset, s->base.pc_next);
>           gen_set_label(l1);
> -        gen_jmp_tb(s, 0, s->pc);
> +        gen_jmp_tb(s, 0, s->pc, s->base.pc_next);
>       } else {
>           /* Unconditional branch.  */
>           update_cc_op(s);
> -        gen_jmp_tb(s, 0, base + offset);
> +        gen_jmp_tb(s, 0, base + offset, s->base.pc_next);
>       }
>   }
>   
> @@ -5485,9 +5502,9 @@ DISAS_INSN(fbcc)
>       l1 = gen_new_label();
>       update_cc_op(s);
>       gen_fjmpcc(s, insn & 0x3f, l1);
> -    gen_jmp_tb(s, 0, s->pc);
> +    gen_jmp_tb(s, 0, s->pc, s->base.pc_next);
>       gen_set_label(l1);
> -    gen_jmp_tb(s, 1, base + offset);
> +    gen_jmp_tb(s, 1, base + offset, s->base.pc_next);
>   }
>   
>   DISAS_INSN(fscc)
> @@ -6158,6 +6175,8 @@ static void m68k_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cpu)
>   
>       dc->env = env;
>       dc->pc = dc->base.pc_first;
> +    /* This value will always be filled in properly before m68k_tr_tb_stop. */
> +    dc->pc_prev = 0xdeadbeef;
>       dc->cc_op = CC_OP_DYNAMIC;
>       dc->cc_op_synced = 1;
>       dc->done_mac = 0;
> @@ -6191,6 +6210,7 @@ static void m68k_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu)
>       do_writebacks(dc);
>       do_release(dc);
>   
> +    dc->pc_prev = dc->base.pc_next;
>       dc->base.pc_next = dc->pc;
>   
>       if (dc->base.is_jmp == DISAS_NEXT) {
> @@ -6225,17 +6245,12 @@ static void m68k_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
>           break;
>       case DISAS_TOO_MANY:
>           update_cc_op(dc);
> -        if (dc->ss_active) {
> -            tcg_gen_movi_i32(QREG_PC, dc->pc);
> -            gen_raise_exception(EXCP_TRACE);
> -        } else {
> -            gen_jmp_tb(dc, 0, dc->pc);
> -        }
> +        gen_jmp_tb(dc, 0, dc->pc, dc->pc_prev);
>           break;
>       case DISAS_JUMP:
>           /* We updated CC_OP and PC in gen_jmp/gen_jmp_im.  */
>           if (dc->ss_active) {
> -            gen_raise_exception(EXCP_TRACE);
> +            gen_raise_exception_format2(dc, EXCP_TRACE, dc->pc_prev);
>           } else {
>               tcg_gen_lookup_and_goto_ptr();
>           }
> @@ -6246,7 +6261,7 @@ static void m68k_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu)
>            * other state that may require returning to the main loop.
>            */
>           if (dc->ss_active) {
> -            gen_raise_exception(EXCP_TRACE);
> +            gen_raise_exception_format2(dc, EXCP_TRACE, dc->pc_prev);
>           } else {
>               tcg_gen_exit_tb(NULL, 0);
>           }

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

[PATCH v4 09/17] target/m68k: Fix stack frame for EXCP_ILLEGAL

[Richard Henderson]

According to the M68040 Users Manual, section 8.4.3, Four word
stack frame (format 0), includes Illegal Instruction.  Use the
correct frame format, which does not use the ADDR argument.

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

diff --git a/target/m68k/op_helper.c b/target/m68k/op_helper.c
index 3cb71c9140..aa62158eb9 100644
--- a/target/m68k/op_helper.c
+++ b/target/m68k/op_helper.c
@@ -391,11 +391,14 @@ static void m68k_interrupt_all(CPUM68KState *env, int is_hw)
         }
         break;
 
+    case EXCP_ILLEGAL:
+        do_stack_frame(env, &sp, 0, oldsr, 0, env->pc);
+        break;
+
     case EXCP_ADDRESS:
         do_stack_frame(env, &sp, 2, oldsr, 0, env->pc);
         break;
 
-    case EXCP_ILLEGAL:
     case EXCP_TRAPCC:
         /* FIXME: addr is not only env->pc */
         do_stack_frame(env, &sp, 2, oldsr, env->pc, env->pc);
-- 
2.34.1

Re: [PATCH v4 09/17] target/m68k: Fix stack frame for EXCP_ILLEGAL

[Laurent Vivier]

Le 30/04/2022 à 19:53, Richard Henderson a écrit :
> According to the M68040 Users Manual, section 8.4.3, Four word

This is in section 8.4.1

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

> stack frame (format 0), includes Illegal Instruction.  Use the
> correct frame format, which does not use the ADDR argument.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/m68k/op_helper.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/target/m68k/op_helper.c b/target/m68k/op_helper.c
> index 3cb71c9140..aa62158eb9 100644
> --- a/target/m68k/op_helper.c
> +++ b/target/m68k/op_helper.c
> @@ -391,11 +391,14 @@ static void m68k_interrupt_all(CPUM68KState *env, int is_hw)
>           }
>           break;
>   
> +    case EXCP_ILLEGAL:
> +        do_stack_frame(env, &sp, 0, oldsr, 0, env->pc);
> +        break;
> +
>       case EXCP_ADDRESS:
>           do_stack_frame(env, &sp, 2, oldsr, 0, env->pc);
>           break;
>   
> -    case EXCP_ILLEGAL:
>       case EXCP_TRAPCC:
>           /* FIXME: addr is not only env->pc */
>           do_stack_frame(env, &sp, 2, oldsr, env->pc, env->pc);

[PATCH v4 10/17] 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 ++
 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;
+    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.  */
-- 
2.34.1

Re: [PATCH v4 10/17] target/m68k: Implement TRAPcc

[Laurent Vivier]

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>

Re: [PATCH v4 10/17] target/m68k: Implement TRAPcc

[Richard Henderson]

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~

Re: [PATCH v4 10/17] target/m68k: Implement TRAPcc

[Laurent Vivier]

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

Re: [PATCH v4 10/17] target/m68k: Implement TRAPcc

[Richard Henderson]

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~

[PATCH v4 11/17] target/m68k: Implement TPF in terms of TRAPcc

[Richard Henderson]

TPF stands for "trap false", and is a long-form nop for ColdFire.
Re-use the immediate consumption code from trapcc; the insn will
already expand to a nop because of the TCG_COND_NEVER test
within do_trapcc.

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

diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index c4fe8abc03..bb5ed1b7b1 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -3075,22 +3075,6 @@ DISAS_INSN(addsubq)
     tcg_temp_free(dest);
 }
 
-DISAS_INSN(tpf)
-{
-    switch (insn & 7) {
-    case 2: /* One extension word.  */
-        s->pc += 2;
-        break;
-    case 3: /* Two extension words.  */
-        s->pc += 4;
-        break;
-    case 4: /* No extension words.  */
-        break;
-    default:
-        disas_undef(env, s, insn);
-    }
-}
-
 DISAS_INSN(branch)
 {
     int32_t offset;
@@ -6099,7 +6083,7 @@ void register_m68k_insns (CPUM68KState *env)
     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);
+    INSN(trapcc,    51f8, fff8, CF_ISA_A); /* TPF (trapf) */
 
     /* Branch instructions.  */
     BASE(branch,    6000, f000);
-- 
2.34.1

Re: [PATCH v4 11/17] target/m68k: Implement TPF in terms of TRAPcc

[Philippe Mathieu-Daudé via]

On 30/4/22 19:53, Richard Henderson wrote:
> TPF stands for "trap false", and is a long-form nop for ColdFire.
> Re-use the immediate consumption code from trapcc; the insn will
> already expand to a nop because of the TCG_COND_NEVER test
> within do_trapcc.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/m68k/translate.c | 18 +-----------------
>   1 file changed, 1 insertion(+), 17 deletions(-)

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

Re: [PATCH v4 11/17] target/m68k: Implement TPF in terms of TRAPcc

[Laurent Vivier]

Le 30/04/2022 à 19:53, Richard Henderson a écrit :
> TPF stands for "trap false", and is a long-form nop for ColdFire.
> Re-use the immediate consumption code from trapcc; the insn will
> already expand to a nop because of the TCG_COND_NEVER test
> within do_trapcc.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/m68k/translate.c | 18 +-----------------
>   1 file changed, 1 insertion(+), 17 deletions(-)
> 
> diff --git a/target/m68k/translate.c b/target/m68k/translate.c
> index c4fe8abc03..bb5ed1b7b1 100644
> --- a/target/m68k/translate.c
> +++ b/target/m68k/translate.c
> @@ -3075,22 +3075,6 @@ DISAS_INSN(addsubq)
>       tcg_temp_free(dest);
>   }
>   
> -DISAS_INSN(tpf)
> -{
> -    switch (insn & 7) {
> -    case 2: /* One extension word.  */
> -        s->pc += 2;
> -        break;
> -    case 3: /* Two extension words.  */
> -        s->pc += 4;
> -        break;
> -    case 4: /* No extension words.  */
> -        break;
> -    default:
> -        disas_undef(env, s, insn);
> -    }
> -}
> -
>   DISAS_INSN(branch)
>   {
>       int32_t offset;
> @@ -6099,7 +6083,7 @@ void register_m68k_insns (CPUM68KState *env)
>       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);
> +    INSN(trapcc,    51f8, fff8, CF_ISA_A); /* TPF (trapf) */
>   
>       /* Branch instructions.  */
>       BASE(branch,    6000, f000);

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

[PATCH v4 12/17] target/m68k: Implement TRAPV

[Richard Henderson]

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

diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index bb5ed1b7b1..0cd7ef89e3 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -4911,6 +4911,14 @@ DISAS_INSN(trapcc)
     do_trapcc(s, &c);
 }
 
+DISAS_INSN(trapv)
+{
+    DisasCompare c;
+
+    gen_cc_cond(&c, s, 9); /* V set */
+    do_trapcc(s, &c);
+}
+
 static void gen_load_fcr(DisasContext *s, TCGv res, int reg)
 {
     switch (reg) {
@@ -6074,6 +6082,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.34.1

Re: [PATCH v4 12/17] target/m68k: Implement TRAPV

[Laurent Vivier]

Le 30/04/2022 à 19:53, Richard Henderson a écrit :
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/m68k/translate.c | 9 +++++++++
>   1 file changed, 9 insertions(+)
> 
> diff --git a/target/m68k/translate.c b/target/m68k/translate.c
> index bb5ed1b7b1..0cd7ef89e3 100644
> --- a/target/m68k/translate.c
> +++ b/target/m68k/translate.c
> @@ -4911,6 +4911,14 @@ DISAS_INSN(trapcc)
>       do_trapcc(s, &c);
>   }
>   
> +DISAS_INSN(trapv)
> +{
> +    DisasCompare c;
> +
> +    gen_cc_cond(&c, s, 9); /* V set */
> +    do_trapcc(s, &c);
> +}
> +
>   static void gen_load_fcr(DisasContext *s, TCGv res, int reg)
>   {
>       switch (reg) {
> @@ -6074,6 +6082,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);

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

[PATCH v4 13/17] target/m68k: Implement FTRAPcc

[Richard Henderson]

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

diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index 0cd7ef89e3..a3141d7f77 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -5567,6 +5567,35 @@ DISAS_INSN(fscc)
     tcg_temp_free(tmp);
 }
 
+DISAS_INSN(ftrapcc)
+{
+    DisasCompare c;
+    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;
+    }
+
+    gen_fcc_cond(&c, s, cond);
+    do_trapcc(s, &c);
+}
+
 #if defined(CONFIG_SOFTMMU)
 DISAS_INSN(frestore)
 {
@@ -6190,6 +6219,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, fff8, FPU);
     INSN(fbcc,      f280, ff80, FPU);
 #if defined(CONFIG_SOFTMMU)
     INSN(frestore,  f340, ffc0, CF_FPU);
-- 
2.34.1

Re: [PATCH v4 13/17] target/m68k: Implement FTRAPcc

[Laurent Vivier]

Le 30/04/2022 à 19:53, Richard Henderson a écrit :
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/m68k/translate.c | 30 ++++++++++++++++++++++++++++++
>   1 file changed, 30 insertions(+)
> 
> diff --git a/target/m68k/translate.c b/target/m68k/translate.c
> index 0cd7ef89e3..a3141d7f77 100644
> --- a/target/m68k/translate.c
> +++ b/target/m68k/translate.c
> @@ -5567,6 +5567,35 @@ DISAS_INSN(fscc)
>       tcg_temp_free(tmp);
>   }
>   
> +DISAS_INSN(ftrapcc)
> +{
> +    DisasCompare c;
> +    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;
> +    }
> +
> +    gen_fcc_cond(&c, s, cond);
> +    do_trapcc(s, &c);
> +}
> +
>   #if defined(CONFIG_SOFTMMU)
>   DISAS_INSN(frestore)
>   {
> @@ -6190,6 +6219,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, fff8, FPU);
>       INSN(fbcc,      f280, ff80, FPU);
>   #if defined(CONFIG_SOFTMMU)
>       INSN(frestore,  f340, ffc0, CF_FPU);

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

[PATCH v4 14/17] tests/tcg/m68k: Add trap.c

[Richard Henderson]

Test various trap instructions: chk, div, trap, trapv, trapcc, ftrapcc,
and the signals and addresses that we expect from them.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 tests/tcg/m68k/trap.c          | 129 +++++++++++++++++++++++++++++++++
 tests/tcg/m68k/Makefile.target |   3 +
 2 files changed, 132 insertions(+)
 create mode 100644 tests/tcg/m68k/trap.c

diff --git a/tests/tcg/m68k/trap.c b/tests/tcg/m68k/trap.c
new file mode 100644
index 0000000000..08ab56b2ca
--- /dev/null
+++ b/tests/tcg/m68k/trap.c
@@ -0,0 +1,129 @@
+/*
+ * Test m68k trap addresses.
+ */
+
+#define _GNU_SOURCE 1
+#include <signal.h>
+#include <assert.h>
+#include <limits.h>
+
+static int expect_sig;
+static int expect_si_code;
+static void *expect_si_addr;
+static greg_t expect_mc_pc;
+static volatile int got_signal;
+
+static void sig_handler(int sig, siginfo_t *si, void *puc)
+{
+    ucontext_t *uc = puc;
+    mcontext_t *mc = &uc->uc_mcontext;
+
+    assert(sig == expect_sig);
+    assert(si->si_code == expect_si_code);
+    assert(si->si_addr == expect_si_addr);
+    assert(mc->gregs[R_PC] == expect_mc_pc);
+
+    got_signal = 1;
+}
+
+#define FMT_INS     [ad] "a"(&expect_si_addr), [pc] "a"(&expect_mc_pc)
+#define FMT0_STR(S) \
+    "move.l #1f, (%[ad])\n\tmove.l #1f, (%[pc])\n" S "\n1:\n"
+#define FMT2_STR(S) \
+    "move.l #0f, (%[ad])\n\tmove.l #1f, (%[pc])\n" S "\n1:\n"
+
+#define CHECK_SIG   do { assert(got_signal); got_signal = 0; } while (0)
+
+int main(int argc, char **argv)
+{
+    struct sigaction act = {
+        .sa_sigaction = sig_handler,
+        .sa_flags = SA_SIGINFO
+    };
+    int t0, t1;
+
+    sigaction(SIGILL, &act, NULL);
+    sigaction(SIGTRAP, &act, NULL);
+    sigaction(SIGFPE, &act, NULL);
+
+    expect_sig = SIGFPE;
+    expect_si_code = FPE_INTOVF;
+    asm volatile(FMT2_STR("0:\tchk %0, %1") : : "d"(0), "d"(-1), FMT_INS);
+    CHECK_SIG;
+
+#if 0
+    /* FIXME: chk2 not correctly translated. */
+    int bounds[2] = { 0, 1 };
+    asm volatile(FMT2_STR("0:\tchk2.l %0, %1")
+                 : : "m"(bounds), "d"(2), FMT_INS);
+    CHECK_SIG;
+#endif
+
+    asm volatile(FMT2_STR("cmp.l %0, %1\n0:\ttrapv")
+                 : : "d"(INT_MIN), "d"(1), FMT_INS);
+    CHECK_SIG;
+
+    asm volatile(FMT2_STR("cmp.l %0, %0\n0:\ttrapeq")
+                 : : "d"(0), FMT_INS);
+    CHECK_SIG;
+
+    asm volatile(FMT2_STR("cmp.l %0, %0\n0:\ttrapeq.w #0x1234")
+                 : : "d"(0), FMT_INS);
+    CHECK_SIG;
+
+    asm volatile(FMT2_STR("cmp.l %0, %0\n0:\ttrapeq.l #0x12345678")
+                 : : "d"(0), FMT_INS);
+    CHECK_SIG;
+
+    asm volatile(FMT2_STR("fcmp.x %0, %0\n0:\tftrapeq")
+                 : : "f"(0.0L), FMT_INS);
+    CHECK_SIG;
+
+    expect_si_code = FPE_INTDIV;
+
+    asm volatile(FMT2_STR("0:\tdivs.w %1, %0")
+                 : "=d"(t0) : "d"(0), "0"(1), FMT_INS);
+    CHECK_SIG;
+
+    asm volatile(FMT2_STR("0:\tdivsl.l %2, %1:%0")
+                 : "=d"(t0), "=d"(t1) : "d"(0), "0"(1), FMT_INS);
+    CHECK_SIG;
+
+    expect_sig = SIGILL;
+    expect_si_code = ILL_ILLOPN;
+    asm volatile(FMT0_STR("trap #1") : : FMT_INS);
+    CHECK_SIG;
+    asm volatile(FMT0_STR("trap #2") : : FMT_INS);
+    CHECK_SIG;
+    asm volatile(FMT0_STR("trap #3") : : FMT_INS);
+    CHECK_SIG;
+    asm volatile(FMT0_STR("trap #4") : : FMT_INS);
+    CHECK_SIG;
+    asm volatile(FMT0_STR("trap #5") : : FMT_INS);
+    CHECK_SIG;
+    asm volatile(FMT0_STR("trap #6") : : FMT_INS);
+    CHECK_SIG;
+    asm volatile(FMT0_STR("trap #7") : : FMT_INS);
+    CHECK_SIG;
+    asm volatile(FMT0_STR("trap #8") : : FMT_INS);
+    CHECK_SIG;
+    asm volatile(FMT0_STR("trap #9") : : FMT_INS);
+    CHECK_SIG;
+    asm volatile(FMT0_STR("trap #10") : : FMT_INS);
+    CHECK_SIG;
+    asm volatile(FMT0_STR("trap #11") : : FMT_INS);
+    CHECK_SIG;
+    asm volatile(FMT0_STR("trap #12") : : FMT_INS);
+    CHECK_SIG;
+    asm volatile(FMT0_STR("trap #13") : : FMT_INS);
+    CHECK_SIG;
+    asm volatile(FMT0_STR("trap #14") : : FMT_INS);
+    CHECK_SIG;
+
+    expect_sig = SIGTRAP;
+    expect_si_code = TRAP_BRKPT;
+    asm volatile(FMT0_STR("trap #15") : : FMT_INS);
+    CHECK_SIG;
+
+    return 0;
+}
diff --git a/tests/tcg/m68k/Makefile.target b/tests/tcg/m68k/Makefile.target
index 62f109eef4..1163c7ef03 100644
--- a/tests/tcg/m68k/Makefile.target
+++ b/tests/tcg/m68k/Makefile.target
@@ -3,5 +3,8 @@
 # m68k specific tweaks - specifically masking out broken tests
 #
 
+VPATH += $(SRC_PATH)/tests/tcg/m68k
+TESTS += trap
+
 # On m68k Linux supports 4k and 8k pages (but 8k is currently broken)
 EXTRA_RUNS+=run-test-mmap-4096 # run-test-mmap-8192
-- 
2.34.1

Re: [PATCH v4 14/17] tests/tcg/m68k: Add trap.c

[Laurent Vivier]

Le 30/04/2022 à 19:53, Richard Henderson a écrit :
> Test various trap instructions: chk, div, trap, trapv, trapcc, ftrapcc,
> and the signals and addresses that we expect from them.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   tests/tcg/m68k/trap.c          | 129 +++++++++++++++++++++++++++++++++
>   tests/tcg/m68k/Makefile.target |   3 +
>   2 files changed, 132 insertions(+)
>   create mode 100644 tests/tcg/m68k/trap.c
> 
> diff --git a/tests/tcg/m68k/trap.c b/tests/tcg/m68k/trap.c
> new file mode 100644
> index 0000000000..08ab56b2ca
> --- /dev/null
> +++ b/tests/tcg/m68k/trap.c
> @@ -0,0 +1,129 @@
> +/*
> + * Test m68k trap addresses.
> + */
> +
> +#define _GNU_SOURCE 1
> +#include <signal.h>
> +#include <assert.h>
> +#include <limits.h>
> +
> +static int expect_sig;
> +static int expect_si_code;
> +static void *expect_si_addr;
> +static greg_t expect_mc_pc;
> +static volatile int got_signal;
> +
> +static void sig_handler(int sig, siginfo_t *si, void *puc)
> +{
> +    ucontext_t *uc = puc;
> +    mcontext_t *mc = &uc->uc_mcontext;
> +
> +    assert(sig == expect_sig);
> +    assert(si->si_code == expect_si_code);
> +    assert(si->si_addr == expect_si_addr);
> +    assert(mc->gregs[R_PC] == expect_mc_pc);
> +
> +    got_signal = 1;
> +}
> +
> +#define FMT_INS     [ad] "a"(&expect_si_addr), [pc] "a"(&expect_mc_pc)
> +#define FMT0_STR(S) \
> +    "move.l #1f, (%[ad])\n\tmove.l #1f, (%[pc])\n" S "\n1:\n"
> +#define FMT2_STR(S) \
> +    "move.l #0f, (%[ad])\n\tmove.l #1f, (%[pc])\n" S "\n1:\n"
> +
> +#define CHECK_SIG   do { assert(got_signal); got_signal = 0; } while (0)
> +
> +int main(int argc, char **argv)
> +{
> +    struct sigaction act = {
> +        .sa_sigaction = sig_handler,
> +        .sa_flags = SA_SIGINFO
> +    };
> +    int t0, t1;
> +
> +    sigaction(SIGILL, &act, NULL);
> +    sigaction(SIGTRAP, &act, NULL);
> +    sigaction(SIGFPE, &act, NULL);
> +
> +    expect_sig = SIGFPE;
> +    expect_si_code = FPE_INTOVF;
> +    asm volatile(FMT2_STR("0:\tchk %0, %1") : : "d"(0), "d"(-1), FMT_INS);
> +    CHECK_SIG;
> +
> +#if 0
> +    /* FIXME: chk2 not correctly translated. */
> +    int bounds[2] = { 0, 1 };
> +    asm volatile(FMT2_STR("0:\tchk2.l %0, %1")
> +                 : : "m"(bounds), "d"(2), FMT_INS);
> +    CHECK_SIG;
> +#endif
> +
> +    asm volatile(FMT2_STR("cmp.l %0, %1\n0:\ttrapv")
> +                 : : "d"(INT_MIN), "d"(1), FMT_INS);
> +    CHECK_SIG;
> +
> +    asm volatile(FMT2_STR("cmp.l %0, %0\n0:\ttrapeq")
> +                 : : "d"(0), FMT_INS);
> +    CHECK_SIG;
> +
> +    asm volatile(FMT2_STR("cmp.l %0, %0\n0:\ttrapeq.w #0x1234")
> +                 : : "d"(0), FMT_INS);
> +    CHECK_SIG;
> +
> +    asm volatile(FMT2_STR("cmp.l %0, %0\n0:\ttrapeq.l #0x12345678")
> +                 : : "d"(0), FMT_INS);
> +    CHECK_SIG;
> +
> +    asm volatile(FMT2_STR("fcmp.x %0, %0\n0:\tftrapeq")
> +                 : : "f"(0.0L), FMT_INS);
> +    CHECK_SIG;
> +
> +    expect_si_code = FPE_INTDIV;
> +
> +    asm volatile(FMT2_STR("0:\tdivs.w %1, %0")
> +                 : "=d"(t0) : "d"(0), "0"(1), FMT_INS);
> +    CHECK_SIG;
> +
> +    asm volatile(FMT2_STR("0:\tdivsl.l %2, %1:%0")
> +                 : "=d"(t0), "=d"(t1) : "d"(0), "0"(1), FMT_INS);
> +    CHECK_SIG;
> +
> +    expect_sig = SIGILL;
> +    expect_si_code = ILL_ILLOPN;
> +    asm volatile(FMT0_STR("trap #1") : : FMT_INS);
> +    CHECK_SIG;
> +    asm volatile(FMT0_STR("trap #2") : : FMT_INS);
> +    CHECK_SIG;
> +    asm volatile(FMT0_STR("trap #3") : : FMT_INS);
> +    CHECK_SIG;
> +    asm volatile(FMT0_STR("trap #4") : : FMT_INS);
> +    CHECK_SIG;
> +    asm volatile(FMT0_STR("trap #5") : : FMT_INS);
> +    CHECK_SIG;
> +    asm volatile(FMT0_STR("trap #6") : : FMT_INS);
> +    CHECK_SIG;
> +    asm volatile(FMT0_STR("trap #7") : : FMT_INS);
> +    CHECK_SIG;
> +    asm volatile(FMT0_STR("trap #8") : : FMT_INS);
> +    CHECK_SIG;
> +    asm volatile(FMT0_STR("trap #9") : : FMT_INS);
> +    CHECK_SIG;
> +    asm volatile(FMT0_STR("trap #10") : : FMT_INS);
> +    CHECK_SIG;
> +    asm volatile(FMT0_STR("trap #11") : : FMT_INS);
> +    CHECK_SIG;
> +    asm volatile(FMT0_STR("trap #12") : : FMT_INS);
> +    CHECK_SIG;
> +    asm volatile(FMT0_STR("trap #13") : : FMT_INS);
> +    CHECK_SIG;
> +    asm volatile(FMT0_STR("trap #14") : : FMT_INS);
> +    CHECK_SIG;
> +
> +    expect_sig = SIGTRAP;
> +    expect_si_code = TRAP_BRKPT;
> +    asm volatile(FMT0_STR("trap #15") : : FMT_INS);
> +    CHECK_SIG;
> +
> +    return 0;
> +}
> diff --git a/tests/tcg/m68k/Makefile.target b/tests/tcg/m68k/Makefile.target
> index 62f109eef4..1163c7ef03 100644
> --- a/tests/tcg/m68k/Makefile.target
> +++ b/tests/tcg/m68k/Makefile.target
> @@ -3,5 +3,8 @@
>   # m68k specific tweaks - specifically masking out broken tests
>   #
>   
> +VPATH += $(SRC_PATH)/tests/tcg/m68k
> +TESTS += trap
> +
>   # On m68k Linux supports 4k and 8k pages (but 8k is currently broken)
>   EXTRA_RUNS+=run-test-mmap-4096 # run-test-mmap-8192

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

[PATCH v4 15/17] linux-user/strace: Fix print_syscall_err

[Richard Henderson]

Errors are not all negative numbers, but only the top 4k.

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/strace.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/linux-user/strace.c b/linux-user/strace.c
index 2cdbf030ba..dc4f810bd3 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -684,12 +684,12 @@ print_ipc(void *cpu_env, const struct syscallname *name,
  */
 
 static bool
-print_syscall_err(abi_long ret)
+print_syscall_err(abi_ulong ret)
 {
     const char *errstr;
 
     qemu_log(" = ");
-    if (ret < 0) {
+    if (ret > (abi_ulong)-4096) {
         errstr = target_strerror(-ret);
         if (errstr) {
             qemu_log("-1 errno=%d (%s)", (int)-ret, errstr);
-- 
2.34.1

Re: [PATCH v4 15/17] linux-user/strace: Fix print_syscall_err

[Laurent Vivier]

Le 30/04/2022 à 19:53, Richard Henderson a écrit :
> Errors are not all negative numbers, but only the top 4k.
> 
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   linux-user/strace.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/linux-user/strace.c b/linux-user/strace.c
> index 2cdbf030ba..dc4f810bd3 100644
> --- a/linux-user/strace.c
> +++ b/linux-user/strace.c
> @@ -684,12 +684,12 @@ print_ipc(void *cpu_env, const struct syscallname *name,
>    */
>   
>   static bool
> -print_syscall_err(abi_long ret)
> +print_syscall_err(abi_ulong ret)
>   {
>       const char *errstr;
>   
>       qemu_log(" = ");
> -    if (ret < 0) {
> +    if (ret > (abi_ulong)-4096) {
>           errstr = target_strerror(-ret);
>           if (errstr) {
>               qemu_log("-1 errno=%d (%s)", (int)-ret, errstr);

Perhaps we can use is_error() here?

Thanks,
Laurent

[PATCH v4 16/17] linux-user/strace: Adjust get_thread_area for m68k

[Richard Henderson]

Unlike i386, m68k get_thread_area has no arguments.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/strace.list | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/linux-user/strace.list b/linux-user/strace.list
index 278596acd1..72e17b1acf 100644
--- a/linux-user/strace.list
+++ b/linux-user/strace.list
@@ -384,8 +384,13 @@
 { TARGET_NR_getsockopt, "getsockopt" , NULL, NULL, NULL },
 #endif
 #ifdef TARGET_NR_get_thread_area
+#if defined(TARGET_I386) && defined(TARGET_ABI32)
 { TARGET_NR_get_thread_area, "get_thread_area", "%s(0x"TARGET_ABI_FMT_lx")",
   NULL, NULL },
+#elif defined(TARGET_M68K)
+{ TARGET_NR_get_thread_area, "get_thread_area" , "%s()",
+  NULL, print_syscall_ret_addr },
+#endif
 #endif
 #ifdef TARGET_NR_gettid
 { TARGET_NR_gettid, "gettid" , "%s()", NULL, NULL },
-- 
2.34.1

Re: [PATCH v4 16/17] linux-user/strace: Adjust get_thread_area for m68k

[Laurent Vivier]

Le 30/04/2022 à 19:53, Richard Henderson a écrit :
> Unlike i386, m68k get_thread_area has no arguments.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   linux-user/strace.list | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/linux-user/strace.list b/linux-user/strace.list
> index 278596acd1..72e17b1acf 100644
> --- a/linux-user/strace.list
> +++ b/linux-user/strace.list
> @@ -384,8 +384,13 @@
>   { TARGET_NR_getsockopt, "getsockopt" , NULL, NULL, NULL },
>   #endif
>   #ifdef TARGET_NR_get_thread_area
> +#if defined(TARGET_I386) && defined(TARGET_ABI32)
>   { TARGET_NR_get_thread_area, "get_thread_area", "%s(0x"TARGET_ABI_FMT_lx")",
>     NULL, NULL },
> +#elif defined(TARGET_M68K)
> +{ TARGET_NR_get_thread_area, "get_thread_area" , "%s()",
> +  NULL, print_syscall_ret_addr },
> +#endif
>   #endif
>   #ifdef TARGET_NR_gettid
>   { TARGET_NR_gettid, "gettid" , "%s()", NULL, NULL },

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

[PATCH v4 17/17] target/m68k: Mark helper_raise_exception as noreturn

[Richard Henderson]

Also mark raise_exception_ra and raise_exception, lest we
generate a warning about helper_raise_exception returning.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/m68k/helper.h    | 2 +-
 target/m68k/op_helper.c | 5 +++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/target/m68k/helper.h b/target/m68k/helper.h
index f016c4c1c2..c9bed2b884 100644
--- a/target/m68k/helper.h
+++ b/target/m68k/helper.h
@@ -109,7 +109,7 @@ DEF_HELPER_3(set_mac_extu, void, env, i32, i32)
 DEF_HELPER_2(flush_flags, void, env, i32)
 DEF_HELPER_2(set_ccr, void, env, i32)
 DEF_HELPER_FLAGS_1(get_ccr, TCG_CALL_NO_WG_SE, i32, env)
-DEF_HELPER_2(raise_exception, void, env, i32)
+DEF_HELPER_2(raise_exception, noreturn, env, i32)
 
 DEF_HELPER_FLAGS_3(bfffo_reg, TCG_CALL_NO_RWG_SE, i32, i32, i32, i32)
 
diff --git a/target/m68k/op_helper.c b/target/m68k/op_helper.c
index 61948d92bb..d9937ca8dc 100644
--- a/target/m68k/op_helper.c
+++ b/target/m68k/op_helper.c
@@ -532,7 +532,8 @@ bool m68k_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
 
 #endif /* !CONFIG_USER_ONLY */
 
-static void raise_exception_ra(CPUM68KState *env, int tt, uintptr_t raddr)
+G_NORETURN static void
+raise_exception_ra(CPUM68KState *env, int tt, uintptr_t raddr)
 {
     CPUState *cs = env_cpu(env);
 
@@ -540,7 +541,7 @@ static void raise_exception_ra(CPUM68KState *env, int tt, uintptr_t raddr)
     cpu_loop_exit_restore(cs, raddr);
 }
 
-static void raise_exception(CPUM68KState *env, int tt)
+G_NORETURN static void raise_exception(CPUM68KState *env, int tt)
 {
     raise_exception_ra(env, tt, 0);
 }
-- 
2.34.1

Re: [PATCH v4 17/17] target/m68k: Mark helper_raise_exception as noreturn

[Philippe Mathieu-Daudé via]

On 30/4/22 19:53, Richard Henderson wrote:
> Also mark raise_exception_ra and raise_exception, lest we
> generate a warning about helper_raise_exception returning.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/m68k/helper.h    | 2 +-
>   target/m68k/op_helper.c | 5 +++--
>   2 files changed, 4 insertions(+), 3 deletions(-)

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

Re: [PATCH v4 17/17] target/m68k: Mark helper_raise_exception as noreturn

[Laurent Vivier]

Le 30/04/2022 à 19:53, Richard Henderson a écrit :
> Also mark raise_exception_ra and raise_exception, lest we
> generate a warning about helper_raise_exception returning.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/m68k/helper.h    | 2 +-
>   target/m68k/op_helper.c | 5 +++--
>   2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/target/m68k/helper.h b/target/m68k/helper.h
> index f016c4c1c2..c9bed2b884 100644
> --- a/target/m68k/helper.h
> +++ b/target/m68k/helper.h
> @@ -109,7 +109,7 @@ DEF_HELPER_3(set_mac_extu, void, env, i32, i32)
>   DEF_HELPER_2(flush_flags, void, env, i32)
>   DEF_HELPER_2(set_ccr, void, env, i32)
>   DEF_HELPER_FLAGS_1(get_ccr, TCG_CALL_NO_WG_SE, i32, env)
> -DEF_HELPER_2(raise_exception, void, env, i32)
> +DEF_HELPER_2(raise_exception, noreturn, env, i32)
>   
>   DEF_HELPER_FLAGS_3(bfffo_reg, TCG_CALL_NO_RWG_SE, i32, i32, i32, i32)
>   
> diff --git a/target/m68k/op_helper.c b/target/m68k/op_helper.c
> index 61948d92bb..d9937ca8dc 100644
> --- a/target/m68k/op_helper.c
> +++ b/target/m68k/op_helper.c
> @@ -532,7 +532,8 @@ bool m68k_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
>   
>   #endif /* !CONFIG_USER_ONLY */
>   
> -static void raise_exception_ra(CPUM68KState *env, int tt, uintptr_t raddr)
> +G_NORETURN static void
> +raise_exception_ra(CPUM68KState *env, int tt, uintptr_t raddr)
>   {
>       CPUState *cs = env_cpu(env);
>   
> @@ -540,7 +541,7 @@ static void raise_exception_ra(CPUM68KState *env, int tt, uintptr_t raddr)
>       cpu_loop_exit_restore(cs, raddr);
>   }
>   
> -static void raise_exception(CPUM68KState *env, int tt)
> +G_NORETURN static void raise_exception(CPUM68KState *env, int tt)
>   {
>       raise_exception_ra(env, tt, 0);
>   }

And why not

   G_NORETURN void HELPER(raise_exception)(CPUM68KState *env, uint32_t tt)

?

Thanks,
Laurent

Re: [PATCH v4 17/17] target/m68k: Mark helper_raise_exception as noreturn

[Richard Henderson]

On 5/25/22 12:45, Laurent Vivier wrote:
>> +DEF_HELPER_2(raise_exception, noreturn, env, i32)
...
>> -static void raise_exception_ra(CPUM68KState *env, int tt, uintptr_t raddr)
>> +G_NORETURN static void
>> +raise_exception_ra(CPUM68KState *env, int tt, uintptr_t raddr)
>>   {
>>       CPUState *cs = env_cpu(env);
>> @@ -540,7 +541,7 @@ static void raise_exception_ra(CPUM68KState *env, int tt, uintptr_t 
>> raddr)
>>       cpu_loop_exit_restore(cs, raddr);
>>   }
>> -static void raise_exception(CPUM68KState *env, int tt)
>> +G_NORETURN static void raise_exception(CPUM68KState *env, int tt)
>>   {
>>       raise_exception_ra(env, tt, 0);
>>   }
> 
> And why not
> 
>    G_NORETURN void HELPER(raise_exception)(CPUM68KState *env, uint32_t tt)
> 
> ?

Because the declaration in the header file takes care of that.
No need to replicate it in the definition.


r~

Re: [PATCH v4 17/17] target/m68k: Mark helper_raise_exception as noreturn

[Laurent Vivier]

Le 30/04/2022 à 19:53, Richard Henderson a écrit :
> Also mark raise_exception_ra and raise_exception, lest we
> generate a warning about helper_raise_exception returning.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/m68k/helper.h    | 2 +-
>   target/m68k/op_helper.c | 5 +++--
>   2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/target/m68k/helper.h b/target/m68k/helper.h
> index f016c4c1c2..c9bed2b884 100644
> --- a/target/m68k/helper.h
> +++ b/target/m68k/helper.h
> @@ -109,7 +109,7 @@ DEF_HELPER_3(set_mac_extu, void, env, i32, i32)
>   DEF_HELPER_2(flush_flags, void, env, i32)
>   DEF_HELPER_2(set_ccr, void, env, i32)
>   DEF_HELPER_FLAGS_1(get_ccr, TCG_CALL_NO_WG_SE, i32, env)
> -DEF_HELPER_2(raise_exception, void, env, i32)
> +DEF_HELPER_2(raise_exception, noreturn, env, i32)
>   
>   DEF_HELPER_FLAGS_3(bfffo_reg, TCG_CALL_NO_RWG_SE, i32, i32, i32, i32)
>   
> diff --git a/target/m68k/op_helper.c b/target/m68k/op_helper.c
> index 61948d92bb..d9937ca8dc 100644
> --- a/target/m68k/op_helper.c
> +++ b/target/m68k/op_helper.c
> @@ -532,7 +532,8 @@ bool m68k_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
>   
>   #endif /* !CONFIG_USER_ONLY */
>   
> -static void raise_exception_ra(CPUM68KState *env, int tt, uintptr_t raddr)
> +G_NORETURN static void
> +raise_exception_ra(CPUM68KState *env, int tt, uintptr_t raddr)
>   {
>       CPUState *cs = env_cpu(env);
>   
> @@ -540,7 +541,7 @@ static void raise_exception_ra(CPUM68KState *env, int tt, uintptr_t raddr)
>       cpu_loop_exit_restore(cs, raddr);
>   }
>   
> -static void raise_exception(CPUM68KState *env, int tt)
> +G_NORETURN static void raise_exception(CPUM68KState *env, int tt)
>   {
>       raise_exception_ra(env, tt, 0);
>   }

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

Re: [PATCH v4 00/17] target/m68k: Conditional traps + trap cleanup

[Richard Henderson]

Ping.

r~

On 4/30/22 10:53, Richard Henderson wrote:
> Changes for v4:
>    - Rebase, which requires QEMU_NORETURN -> G_NORETURN.
>    - Cast -4096 to abi_ulong for print_syscall_err.
> 
> 
> r~
> 
> 
> v1: https://lore.kernel.org/qemu-devel/20211130103752.72099-1-richard.henderson@linaro.org/
> v2: https://lore.kernel.org/qemu-devel/20211202204900.50973-1-richard.henderson@linaro.org/
> v3: https://lore.kernel.org/qemu-devel/20220316055840.727571-1-richard.henderson@linaro.org/
> 
> 
> Richard Henderson (17):
>    target/m68k: Raise the TRAPn exception with the correct pc
>    target/m68k: Switch over exception type in m68k_interrupt_all
>    target/m68k: Fix coding style in m68k_interrupt_all
>    linux-user/m68k: Handle EXCP_TRAP1 through EXCP_TRAP15
>    target/m68k: Remove retaddr in m68k_interrupt_all
>    target/m68k: Fix address argument for EXCP_CHK
>    target/m68k: Fix pc, c flag, and address argument for EXCP_DIV0
>    target/m68k: Fix address argument for EXCP_TRACE
>    target/m68k: Fix stack frame for EXCP_ILLEGAL
>    target/m68k: Implement TRAPcc
>    target/m68k: Implement TPF in terms of TRAPcc
>    target/m68k: Implement TRAPV
>    target/m68k: Implement FTRAPcc
>    tests/tcg/m68k: Add trap.c
>    linux-user/strace: Fix print_syscall_err
>    linux-user/strace: Adjust get_thread_area for m68k
>    target/m68k: Mark helper_raise_exception as noreturn
> 
>   target/m68k/cpu.h              |   8 ++
>   target/m68k/helper.h           |  14 +--
>   linux-user/m68k/cpu_loop.c     |  11 +-
>   linux-user/strace.c            |   4 +-
>   target/m68k/cpu.c              |   1 +
>   target/m68k/op_helper.c        | 173 ++++++++++++++++--------------
>   target/m68k/translate.c        | 190 ++++++++++++++++++++++++---------
>   tests/tcg/m68k/trap.c          | 129 ++++++++++++++++++++++
>   linux-user/strace.list         |   5 +
>   tests/tcg/m68k/Makefile.target |   3 +
>   10 files changed, 394 insertions(+), 144 deletions(-)
>   create mode 100644 tests/tcg/m68k/trap.c
>