All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 00/15] target/m68k: Conditional traps + trap cleanup
@ 2021-12-02 20:48 Richard Henderson
  2021-12-02 20:48 ` [PATCH v2 01/15] target/m68k: Raise the TRAPn exception with the correct pc Richard Henderson
                   ` (14 more replies)
  0 siblings, 15 replies; 27+ messages in thread
From: Richard Henderson @ 2021-12-02 20:48 UTC (permalink / raw)
  To: qemu-devel; +Cc: laurent

Supercedes: <20211130103752.72099-1-richard.henderson@linaro.org>

Reviewing v1, Laurent rightly noticed that there were changes
required in m68k_interrupt_all.  Matching sysemu, there were
changes needed in the linux-user cpu_loop.

In the process, I found a number of other trap related bugs,
and some strace problems.  I cherry-picked the linux-user/m68k
patch from my outstanding force_sig_fault patch set, to avoid
conflicts with that one.


r~


Richard Henderson (15):
  target/m68k: Raise the TRAPn exception with the correct pc
  target/m68k: Switch over exception type in m68k_interrupt_all
  linux-user/m68k: Use force_sig_fault
  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: Implement TRAPcc
  target/m68k: Implement TRAPV
  target/m68k: Implement FTRAPcc
  target/m68k: Fix stack frame for EXCP_ILLEGAL
  tests/tcg/m68k: Add trap.c
  linux-user/strace: Fix print_syscall_err
  linux-user/strace: Adjust get_thread_area for m68k

 target/m68k/cpu.h              |   2 +
 target/m68k/helper.h           |  12 +--
 linux-user/m68k/cpu_loop.c     |  31 ++-----
 linux-user/strace.c            |   4 +-
 target/m68k/cpu.c              |   1 +
 target/m68k/op_helper.c        | 164 ++++++++++++++++++---------------
 target/m68k/translate.c        | 142 ++++++++++++++++++++++------
 tests/tcg/m68k/trap.c          | 129 ++++++++++++++++++++++++++
 linux-user/strace.list         |   5 +
 tests/tcg/m68k/Makefile.target |   3 +
 10 files changed, 361 insertions(+), 132 deletions(-)
 create mode 100644 tests/tcg/m68k/trap.c

-- 
2.25.1



^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH v2 01/15] target/m68k: Raise the TRAPn exception with the correct pc
  2021-12-02 20:48 [PATCH v2 00/15] target/m68k: Conditional traps + trap cleanup Richard Henderson
@ 2021-12-02 20:48 ` Richard Henderson
  2021-12-03  9:04   ` Laurent Vivier
  2021-12-02 20:48 ` [PATCH v2 02/15] target/m68k: Switch over exception type in m68k_interrupt_all Richard Henderson
                   ` (13 subsequent siblings)
  14 siblings, 1 reply; 27+ messages in thread
From: Richard Henderson @ 2021-12-02 20:48 UTC (permalink / raw)
  To: qemu-devel; +Cc: laurent

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

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 790bd558c3..287f24cc60 100644
--- a/linux-user/m68k/cpu_loop.c
+++ b/linux-user/m68k/cpu_loop.c
@@ -70,7 +70,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 cfbc987ba6..36662de149 100644
--- a/target/m68k/op_helper.c
+++ b/target/m68k/op_helper.c
@@ -216,11 +216,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;
@@ -303,10 +298,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 af43c8eab8..af3febdd48 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.25.1



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 02/15] target/m68k: Switch over exception type in m68k_interrupt_all
  2021-12-02 20:48 [PATCH v2 00/15] target/m68k: Conditional traps + trap cleanup Richard Henderson
  2021-12-02 20:48 ` [PATCH v2 01/15] target/m68k: Raise the TRAPn exception with the correct pc Richard Henderson
@ 2021-12-02 20:48 ` Richard Henderson
  2021-12-03  9:02   ` Philippe Mathieu-Daudé
  2021-12-03  9:06   ` Laurent Vivier
  2021-12-02 20:48 ` [PATCH v2 03/15] linux-user/m68k: Use force_sig_fault Richard Henderson
                   ` (12 subsequent siblings)
  14 siblings, 2 replies; 27+ messages in thread
From: Richard Henderson @ 2021-12-02 20:48 UTC (permalink / raw)
  To: qemu-devel; +Cc: laurent

Replace an if ladder with a switch for clarity.

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

diff --git a/target/m68k/op_helper.c b/target/m68k/op_helper.c
index 36662de149..71176eb3d8 100644
--- a/target/m68k/op_helper.c
+++ b/target/m68k/op_helper.c
@@ -332,7 +332,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");
         }
@@ -390,26 +391,36 @@ 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] & ~1;
-        do_stack_frame(env, &sp, 1, oldsr, 0, retaddr);
-    } else {
+        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] & ~1;
+            do_stack_frame(env, &sp, 1, oldsr, 0, retaddr);
+            break;
+        }
+        /* fall through */
+
+    default:
         do_stack_frame(env, &sp, 0, oldsr, 0, retaddr);
+        break;
     }
 
     env->aregs[7] = sp;
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 03/15] linux-user/m68k: Use force_sig_fault
  2021-12-02 20:48 [PATCH v2 00/15] target/m68k: Conditional traps + trap cleanup Richard Henderson
  2021-12-02 20:48 ` [PATCH v2 01/15] target/m68k: Raise the TRAPn exception with the correct pc Richard Henderson
  2021-12-02 20:48 ` [PATCH v2 02/15] target/m68k: Switch over exception type in m68k_interrupt_all Richard Henderson
@ 2021-12-02 20:48 ` Richard Henderson
  2021-12-02 20:48 ` [PATCH v2 04/15] linux-user/m68k: Handle EXCP_TRAP1 through EXCP_TRAP15 Richard Henderson
                   ` (11 subsequent siblings)
  14 siblings, 0 replies; 27+ messages in thread
From: Richard Henderson @ 2021-12-02 20:48 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, laurent

Use the new function instead of setting up a target_siginfo_t
and calling queue_signal. Fill in the missing PC for SIGTRAP.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/m68k/cpu_loop.c | 24 ++++--------------------
 1 file changed, 4 insertions(+), 20 deletions(-)

diff --git a/linux-user/m68k/cpu_loop.c b/linux-user/m68k/cpu_loop.c
index 287f24cc60..396f98bd6c 100644
--- a/linux-user/m68k/cpu_loop.c
+++ b/linux-user/m68k/cpu_loop.c
@@ -29,7 +29,6 @@ void cpu_loop(CPUM68KState *env)
     CPUState *cs = env_cpu(env);
     int trapnr;
     unsigned int n;
-    target_siginfo_t info;
 
     for(;;) {
         cpu_exec_start(cs);
@@ -46,25 +45,13 @@ void cpu_loop(CPUM68KState *env)
         case EXCP_ILLEGAL:
         case EXCP_LINEA:
         case EXCP_LINEF:
-            info.si_signo = TARGET_SIGILL;
-            info.si_errno = 0;
-            info.si_code = TARGET_ILL_ILLOPN;
-            info._sifields._sigfault._addr = env->pc;
-            queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info);
+            force_sig_fault(TARGET_SIGILL, TARGET_ILL_ILLOPN, env->pc);
             break;
         case EXCP_CHK:
-            info.si_signo = TARGET_SIGFPE;
-            info.si_errno = 0;
-            info.si_code = TARGET_FPE_INTOVF;
-            info._sifields._sigfault._addr = env->pc;
-            queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info);
+            force_sig_fault(TARGET_SIGFPE, TARGET_FPE_INTOVF, env->pc);
             break;
         case EXCP_DIV0:
-            info.si_signo = TARGET_SIGFPE;
-            info.si_errno = 0;
-            info.si_code = TARGET_FPE_INTDIV;
-            info._sifields._sigfault._addr = env->pc;
-            queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info);
+            force_sig_fault(TARGET_SIGFPE, TARGET_FPE_INTDIV, env->pc);
             break;
         case EXCP_TRAP0:
             {
@@ -90,10 +77,7 @@ void cpu_loop(CPUM68KState *env)
             /* just indicate that signals should be handled asap */
             break;
         case EXCP_DEBUG:
-            info.si_signo = TARGET_SIGTRAP;
-            info.si_errno = 0;
-            info.si_code = TARGET_TRAP_BRKPT;
-            queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info);
+            force_sig_fault(TARGET_SIGTRAP, TARGET_TRAP_BRKPT, env->pc);
             break;
         case EXCP_ATOMIC:
             cpu_exec_step_atomic(cs);
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 04/15] linux-user/m68k: Handle EXCP_TRAP1 through EXCP_TRAP15
  2021-12-02 20:48 [PATCH v2 00/15] target/m68k: Conditional traps + trap cleanup Richard Henderson
                   ` (2 preceding siblings ...)
  2021-12-02 20:48 ` [PATCH v2 03/15] linux-user/m68k: Use force_sig_fault Richard Henderson
@ 2021-12-02 20:48 ` Richard Henderson
  2021-12-02 20:48 ` [PATCH v2 05/15] target/m68k: Remove retaddr in m68k_interrupt_all Richard Henderson
                   ` (10 subsequent siblings)
  14 siblings, 0 replies; 27+ messages in thread
From: Richard Henderson @ 2021-12-02 20:48 UTC (permalink / raw)
  To: qemu-devel; +Cc: laurent

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 396f98bd6c..0de11fb9bf 100644
--- a/linux-user/m68k/cpu_loop.c
+++ b/linux-user/m68k/cpu_loop.c
@@ -45,6 +45,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:
@@ -77,6 +78,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.25.1



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 05/15] target/m68k: Remove retaddr in m68k_interrupt_all
  2021-12-02 20:48 [PATCH v2 00/15] target/m68k: Conditional traps + trap cleanup Richard Henderson
                   ` (3 preceding siblings ...)
  2021-12-02 20:48 ` [PATCH v2 04/15] linux-user/m68k: Handle EXCP_TRAP1 through EXCP_TRAP15 Richard Henderson
@ 2021-12-02 20:48 ` Richard Henderson
  2021-12-03  9:03   ` Philippe Mathieu-Daudé
  2021-12-03  9:08   ` Laurent Vivier
  2021-12-02 20:48 ` [PATCH v2 06/15] target/m68k: Fix address argument for EXCP_CHK Richard Henderson
                   ` (9 subsequent siblings)
  14 siblings, 2 replies; 27+ messages in thread
From: Richard Henderson @ 2021-12-02 20:48 UTC (permalink / raw)
  To: qemu-devel; +Cc: laurent

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

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 71176eb3d8..afbbb8b4ca 100644
--- a/target/m68k/op_helper.c
+++ b/target/m68k/op_helper.c
@@ -286,12 +286,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:
@@ -384,7 +381,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("            "
@@ -394,7 +391,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:
@@ -403,23 +400,23 @@ 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);
             sp = env->aregs[7] & ~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.25.1



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 06/15] target/m68k: Fix address argument for EXCP_CHK
  2021-12-02 20:48 [PATCH v2 00/15] target/m68k: Conditional traps + trap cleanup Richard Henderson
                   ` (4 preceding siblings ...)
  2021-12-02 20:48 ` [PATCH v2 05/15] target/m68k: Remove retaddr in m68k_interrupt_all Richard Henderson
@ 2021-12-02 20:48 ` Richard Henderson
  2021-12-03 14:27   ` Laurent Vivier
  2021-12-02 20:48 ` [PATCH v2 07/15] target/m68k: Fix pc, c flag, and address argument for EXCP_DIV0 Richard Henderson
                   ` (8 subsequent siblings)
  14 siblings, 1 reply; 27+ messages in thread
From: Richard Henderson @ 2021-12-02 20:48 UTC (permalink / raw)
  To: qemu-devel; +Cc: laurent

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().

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

diff --git a/linux-user/m68k/cpu_loop.c b/linux-user/m68k/cpu_loop.c
index 0de11fb9bf..82b100aa87 100644
--- a/linux-user/m68k/cpu_loop.c
+++ b/linux-user/m68k/cpu_loop.c
@@ -49,7 +49,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 afbbb8b4ca..b549eb077c 100644
--- a/target/m68k/op_helper.c
+++ b/target/m68k/op_helper.c
@@ -396,13 +396,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);
@@ -544,6 +547,29 @@ void HELPER(raise_exception)(CPUM68KState *env, uint32_t tt)
     raise_exception(env, tt);
 }
 
+static void QEMU_NORETURN
+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];
@@ -1061,18 +1087,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());
     }
 }
 
@@ -1093,17 +1108,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.25.1



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 07/15] target/m68k: Fix pc, c flag, and address argument for EXCP_DIV0
  2021-12-02 20:48 [PATCH v2 00/15] target/m68k: Conditional traps + trap cleanup Richard Henderson
                   ` (5 preceding siblings ...)
  2021-12-02 20:48 ` [PATCH v2 06/15] target/m68k: Fix address argument for EXCP_CHK Richard Henderson
@ 2021-12-02 20:48 ` Richard Henderson
  2021-12-04 16:50   ` Laurent Vivier
  2021-12-02 20:48 ` [PATCH v2 08/15] target/m68k: Fix address argument for EXCP_TRACE Richard Henderson
                   ` (7 subsequent siblings)
  14 siblings, 1 reply; 27+ messages in thread
From: Richard Henderson @ 2021-12-02 20:48 UTC (permalink / raw)
  To: qemu-devel; +Cc: laurent

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().

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 9842eeaa95..813d180dd0 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 82b100aa87..267df05372 100644
--- a/linux-user/m68k/cpu_loop.c
+++ b/linux-user/m68k/cpu_loop.c
@@ -52,7 +52,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 b549eb077c..a6e4f5719f 100644
--- a/target/m68k/op_helper.c
+++ b/target/m68k/op_helper.c
@@ -395,7 +395,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 */
@@ -403,6 +402,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;
 
@@ -570,18 +570,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;
         /*
@@ -597,18 +598,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 */
@@ -625,18 +627,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;
@@ -653,18 +657,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;
@@ -681,19 +687,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;
         /*
@@ -716,19 +724,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 af3febdd48..ae9f5a5222 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.25.1



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 08/15] target/m68k: Fix address argument for EXCP_TRACE
  2021-12-02 20:48 [PATCH v2 00/15] target/m68k: Conditional traps + trap cleanup Richard Henderson
                   ` (6 preceding siblings ...)
  2021-12-02 20:48 ` [PATCH v2 07/15] target/m68k: Fix pc, c flag, and address argument for EXCP_DIV0 Richard Henderson
@ 2021-12-02 20:48 ` Richard Henderson
  2021-12-03 14:21   ` Richard Henderson
  2021-12-02 20:48 ` [PATCH v2 09/15] target/m68k: Implement TRAPcc Richard Henderson
                   ` (6 subsequent siblings)
  14 siblings, 1 reply; 27+ messages in thread
From: Richard Henderson @ 2021-12-02 20:48 UTC (permalink / raw)
  To: qemu-devel; +Cc: laurent

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    | 27 ++++++++++++++++++---------
 3 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/linux-user/m68k/cpu_loop.c b/linux-user/m68k/cpu_loop.c
index 267df05372..8e2b79550d 100644
--- a/linux-user/m68k/cpu_loop.c
+++ b/linux-user/m68k/cpu_loop.c
@@ -54,6 +54,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 a6e4f5719f..c9ea28bf68 100644
--- a/target/m68k/op_helper.c
+++ b/target/m68k/op_helper.c
@@ -396,13 +396,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 ae9f5a5222..fc2b6a3085 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -298,6 +298,20 @@ static void gen_raise_exception(int nr)
     tcg_temp_free_i32(tmp);
 }
 
+static void gen_raise_exception_format2(DisasContext *s, int nr)
+{
+    /*
+     * 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(s->base.pc_next), 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);
@@ -1499,7 +1513,7 @@ static void gen_jmp_tb(DisasContext *s, int n, uint32_t dest)
     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);
     } else if (translator_use_goto_tb(&s->base, dest)) {
         tcg_gen_goto_tb(n);
         tcg_gen_movi_i32(QREG_PC, dest);
@@ -6225,17 +6239,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);
         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);
         } else {
             tcg_gen_lookup_and_goto_ptr();
         }
@@ -6246,7 +6255,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);
         } else {
             tcg_gen_exit_tb(NULL, 0);
         }
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 09/15] target/m68k: Implement TRAPcc
  2021-12-02 20:48 [PATCH v2 00/15] target/m68k: Conditional traps + trap cleanup Richard Henderson
                   ` (7 preceding siblings ...)
  2021-12-02 20:48 ` [PATCH v2 08/15] target/m68k: Fix address argument for EXCP_TRACE Richard Henderson
@ 2021-12-02 20:48 ` Richard Henderson
  2021-12-02 20:48 ` [PATCH v2 10/15] target/m68k: Implement TRAPV Richard Henderson
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 27+ messages in thread
From: Richard Henderson @ 2021-12-02 20:48 UTC (permalink / raw)
  To: qemu-devel; +Cc: laurent

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    |  2 +-
 target/m68k/translate.c    | 41 ++++++++++++++++++++++++++++++++++++++
 5 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/target/m68k/cpu.h b/target/m68k/cpu.h
index a3423729ef..14e130f400 100644
--- a/target/m68k/cpu.h
+++ b/target/m68k/cpu.h
@@ -527,6 +527,8 @@ enum m68k_features {
     M68K_FEATURE_MOVEC,
     /* Unaligned data accesses (680[2346]0) */
     M68K_FEATURE_UNALIGNED_DATA,
+    /* TRAPcc insn. (680[2346]0, and CPU32) */
+    M68K_FEATURE_TRAPCC,
 };
 
 static inline int m68k_feature(CPUM68KState *env, int feature)
diff --git a/linux-user/m68k/cpu_loop.c b/linux-user/m68k/cpu_loop.c
index 8e2b79550d..ed366d1645 100644
--- a/linux-user/m68k/cpu_loop.c
+++ b/linux-user/m68k/cpu_loop.c
@@ -49,6 +49,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 c9ea28bf68..811b8bd117 100644
--- a/target/m68k/op_helper.c
+++ b/target/m68k/op_helper.c
@@ -395,7 +395,6 @@ static void m68k_interrupt_all(CPUM68KState *env, int is_hw)
         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);
         break;
@@ -403,6 +402,7 @@ static void m68k_interrupt_all(CPUM68KState *env, int is_hw)
     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 fc2b6a3085..e5338b50ad 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -4876,6 +4876,46 @@ DISAS_INSN(trap)
     gen_exception(s, s->pc, EXCP_TRAP0 + (insn & 0xf));
 }
 
+static void do_trapcc(DisasContext *s, DisasCompare *c)
+{
+    TCGLabel *over = gen_new_label();
+
+    /* Jump over if !c. */
+    update_cc_op(s);
+    tcg_gen_brcond_i32(tcg_invert_cond(c->tcond), c->v1, c->v2, over);
+    free_cond(c);
+
+    tcg_gen_movi_i32(QREG_PC, s->pc);
+    gen_raise_exception_format2(s, EXCP_TRAPCC);
+
+    gen_set_label(over);
+    s->base.is_jmp = DISAS_NEXT;
+}
+
+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) {
@@ -6047,6 +6087,7 @@ void register_m68k_insns (CPUM68KState *env)
     INSN(scc,       50c0, f0f8, CF_ISA_A); /* Scc.B Dx   */
     INSN(scc,       50c0, f0c0, M68000);   /* Scc.B <EA> */
     INSN(dbcc,      50c8, f0f8, M68000);
+    INSN(trapcc,    50f8, f0f8, TRAPCC);
     INSN(tpf,       51f8, fff8, CF_ISA_A);
 
     /* Branch instructions.  */
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 10/15] target/m68k: Implement TRAPV
  2021-12-02 20:48 [PATCH v2 00/15] target/m68k: Conditional traps + trap cleanup Richard Henderson
                   ` (8 preceding siblings ...)
  2021-12-02 20:48 ` [PATCH v2 09/15] target/m68k: Implement TRAPcc Richard Henderson
@ 2021-12-02 20:48 ` Richard Henderson
  2021-12-02 20:48 ` [PATCH v2 11/15] target/m68k: Implement FTRAPcc Richard Henderson
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 27+ messages in thread
From: Richard Henderson @ 2021-12-02 20:48 UTC (permalink / raw)
  To: qemu-devel; +Cc: laurent

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 e5338b50ad..cfe292c929 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -4916,6 +4916,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) {
@@ -6079,6 +6087,7 @@ void register_m68k_insns (CPUM68KState *env)
     BASE(nop,       4e71, ffff);
     INSN(rtd,       4e74, ffff, RTD);
     BASE(rts,       4e75, ffff);
+    INSN(trapv,     4e76, ffff, M68000);
     INSN(rtr,       4e77, ffff, M68000);
     BASE(jump,      4e80, ffc0);
     BASE(jump,      4ec0, ffc0);
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 11/15] target/m68k: Implement FTRAPcc
  2021-12-02 20:48 [PATCH v2 00/15] target/m68k: Conditional traps + trap cleanup Richard Henderson
                   ` (9 preceding siblings ...)
  2021-12-02 20:48 ` [PATCH v2 10/15] target/m68k: Implement TRAPV Richard Henderson
@ 2021-12-02 20:48 ` Richard Henderson
  2021-12-02 20:48 ` [PATCH v2 12/15] target/m68k: Fix stack frame for EXCP_ILLEGAL Richard Henderson
                   ` (3 subsequent siblings)
  14 siblings, 0 replies; 27+ messages in thread
From: Richard Henderson @ 2021-12-02 20:48 UTC (permalink / raw)
  To: qemu-devel; +Cc: laurent

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 cfe292c929..641f95ff8a 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -5572,6 +5572,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)
 {
@@ -6195,6 +6224,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.25.1



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 12/15] target/m68k: Fix stack frame for EXCP_ILLEGAL
  2021-12-02 20:48 [PATCH v2 00/15] target/m68k: Conditional traps + trap cleanup Richard Henderson
                   ` (10 preceding siblings ...)
  2021-12-02 20:48 ` [PATCH v2 11/15] target/m68k: Implement FTRAPcc Richard Henderson
@ 2021-12-02 20:48 ` Richard Henderson
  2021-12-02 20:48 ` [PATCH v2 13/15] tests/tcg/m68k: Add trap.c Richard Henderson
                   ` (2 subsequent siblings)
  14 siblings, 0 replies; 27+ messages in thread
From: Richard Henderson @ 2021-12-02 20:48 UTC (permalink / raw)
  To: qemu-devel; +Cc: laurent

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 | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/target/m68k/op_helper.c b/target/m68k/op_helper.c
index 811b8bd117..10d459f704 100644
--- a/target/m68k/op_helper.c
+++ b/target/m68k/op_helper.c
@@ -395,8 +395,7 @@ static void m68k_interrupt_all(CPUM68KState *env, int is_hw)
         break;
 
     case EXCP_ILLEGAL:
-        /* FIXME: addr is not only env->pc */
-        do_stack_frame(env, &sp, 2, oldsr, env->pc, env->pc);
+        do_stack_frame(env, &sp, 0, oldsr, 0, env->pc);
         break;
 
     case EXCP_CHK:
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 13/15] tests/tcg/m68k: Add trap.c
  2021-12-02 20:48 [PATCH v2 00/15] target/m68k: Conditional traps + trap cleanup Richard Henderson
                   ` (11 preceding siblings ...)
  2021-12-02 20:48 ` [PATCH v2 12/15] target/m68k: Fix stack frame for EXCP_ILLEGAL Richard Henderson
@ 2021-12-02 20:48 ` Richard Henderson
  2021-12-02 20:48 ` [PATCH v2 14/15] linux-user/strace: Fix print_syscall_err Richard Henderson
  2021-12-02 20:49 ` [PATCH v2 15/15] linux-user/strace: Adjust get_thread_area for m68k Richard Henderson
  14 siblings, 0 replies; 27+ messages in thread
From: Richard Henderson @ 2021-12-02 20:48 UTC (permalink / raw)
  To: qemu-devel; +Cc: laurent

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.25.1



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 14/15] linux-user/strace: Fix print_syscall_err
  2021-12-02 20:48 [PATCH v2 00/15] target/m68k: Conditional traps + trap cleanup Richard Henderson
                   ` (12 preceding siblings ...)
  2021-12-02 20:48 ` [PATCH v2 13/15] tests/tcg/m68k: Add trap.c Richard Henderson
@ 2021-12-02 20:48 ` Richard Henderson
  2021-12-03  8:57   ` Philippe Mathieu-Daudé
  2021-12-02 20:49 ` [PATCH v2 15/15] linux-user/strace: Adjust get_thread_area for m68k Richard Henderson
  14 siblings, 1 reply; 27+ messages in thread
From: Richard Henderson @ 2021-12-02 20:48 UTC (permalink / raw)
  To: qemu-devel; +Cc: laurent

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

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..f235118fb6 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 > -4096) {
         errstr = target_strerror(-ret);
         if (errstr) {
             qemu_log("-1 errno=%d (%s)", (int)-ret, errstr);
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 15/15] linux-user/strace: Adjust get_thread_area for m68k
  2021-12-02 20:48 [PATCH v2 00/15] target/m68k: Conditional traps + trap cleanup Richard Henderson
                   ` (13 preceding siblings ...)
  2021-12-02 20:48 ` [PATCH v2 14/15] linux-user/strace: Fix print_syscall_err Richard Henderson
@ 2021-12-02 20:49 ` Richard Henderson
  14 siblings, 0 replies; 27+ messages in thread
From: Richard Henderson @ 2021-12-02 20:49 UTC (permalink / raw)
  To: qemu-devel; +Cc: laurent

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.25.1



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 14/15] linux-user/strace: Fix print_syscall_err
  2021-12-02 20:48 ` [PATCH v2 14/15] linux-user/strace: Fix print_syscall_err Richard Henderson
@ 2021-12-03  8:57   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-12-03  8:57 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel; +Cc: laurent

On 12/2/21 21:48, Richard Henderson wrote:
> Errors are not all negative numbers, but only the top 4k.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  linux-user/strace.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

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


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 02/15] target/m68k: Switch over exception type in m68k_interrupt_all
  2021-12-02 20:48 ` [PATCH v2 02/15] target/m68k: Switch over exception type in m68k_interrupt_all Richard Henderson
@ 2021-12-03  9:02   ` Philippe Mathieu-Daudé
  2021-12-03  9:06   ` Laurent Vivier
  1 sibling, 0 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-12-03  9:02 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel; +Cc: laurent

On 12/2/21 21:48, Richard Henderson wrote:
> Replace an if ladder with a switch for clarity.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/m68k/op_helper.c | 45 +++++++++++++++++++++++++----------------
>  1 file changed, 28 insertions(+), 17 deletions(-)

> -    } 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] & ~1;
> -        do_stack_frame(env, &sp, 1, oldsr, 0, retaddr);
> -    } else {
> +        break;
> +
> +    case EXCP_SPURIOUS ... EXCP_INT_LEVEL_7:
> +        if (is_hw && oldsr & SR_M) {

This is code movement, but adding parenthesis would produce
clearer code.

> +            do_stack_frame(env, &sp, 0, oldsr, 0, retaddr);
> +            oldsr = sr;
> +            env->aregs[7] = sp;
> +            cpu_m68k_set_sr(env, sr &= ~SR_M);

Similarly code movement; this assignation deserves a followup
cleanup patch...

> +            sp = env->aregs[7] & ~1;
> +            do_stack_frame(env, &sp, 1, oldsr, 0, retaddr);
> +            break;
> +        }
> +        /* fall through */
> +
> +    default:
>          do_stack_frame(env, &sp, 0, oldsr, 0, retaddr);
> +        break;
>      }

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


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 05/15] target/m68k: Remove retaddr in m68k_interrupt_all
  2021-12-02 20:48 ` [PATCH v2 05/15] target/m68k: Remove retaddr in m68k_interrupt_all Richard Henderson
@ 2021-12-03  9:03   ` Philippe Mathieu-Daudé
  2021-12-03  9:08   ` Laurent Vivier
  1 sibling, 0 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-12-03  9:03 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel; +Cc: laurent

On 12/2/21 21:48, Richard Henderson wrote:
> The only value this variable holds is now env->pc.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/m68k/op_helper.c | 15 ++++++---------
>  1 file changed, 6 insertions(+), 9 deletions(-)

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


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 01/15] target/m68k: Raise the TRAPn exception with the correct pc
  2021-12-02 20:48 ` [PATCH v2 01/15] target/m68k: Raise the TRAPn exception with the correct pc Richard Henderson
@ 2021-12-03  9:04   ` Laurent Vivier
  0 siblings, 0 replies; 27+ messages in thread
From: Laurent Vivier @ 2021-12-03  9:04 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel

Le 02/12/2021 à 21:48, Richard Henderson a écrit :
> Rather than adjust the PC in all of the consumers, raise
> the exception with the correct PC in the first place.
> 
> 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 790bd558c3..287f24cc60 100644
> --- a/linux-user/m68k/cpu_loop.c
> +++ b/linux-user/m68k/cpu_loop.c
> @@ -70,7 +70,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 cfbc987ba6..36662de149 100644
> --- a/target/m68k/op_helper.c
> +++ b/target/m68k/op_helper.c
> @@ -216,11 +216,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;
> @@ -303,10 +298,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 af43c8eab8..af3febdd48 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)
> 

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


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 02/15] target/m68k: Switch over exception type in m68k_interrupt_all
  2021-12-02 20:48 ` [PATCH v2 02/15] target/m68k: Switch over exception type in m68k_interrupt_all Richard Henderson
  2021-12-03  9:02   ` Philippe Mathieu-Daudé
@ 2021-12-03  9:06   ` Laurent Vivier
  1 sibling, 0 replies; 27+ messages in thread
From: Laurent Vivier @ 2021-12-03  9:06 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel

Le 02/12/2021 à 21:48, Richard Henderson a écrit :
> Replace an if ladder with a switch for clarity.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/m68k/op_helper.c | 45 +++++++++++++++++++++++++----------------
>   1 file changed, 28 insertions(+), 17 deletions(-)
> 
> diff --git a/target/m68k/op_helper.c b/target/m68k/op_helper.c
> index 36662de149..71176eb3d8 100644
> --- a/target/m68k/op_helper.c
> +++ b/target/m68k/op_helper.c
> @@ -332,7 +332,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");
>           }
> @@ -390,26 +391,36 @@ 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] & ~1;
> -        do_stack_frame(env, &sp, 1, oldsr, 0, retaddr);
> -    } else {
> +        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] & ~1;
> +            do_stack_frame(env, &sp, 1, oldsr, 0, retaddr);
> +            break;
> +        }
> +        /* fall through */
> +
> +    default:
>           do_stack_frame(env, &sp, 0, oldsr, 0, retaddr);
> +        break;
>       }
>   
>       env->aregs[7] = sp;
> 

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


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 05/15] target/m68k: Remove retaddr in m68k_interrupt_all
  2021-12-02 20:48 ` [PATCH v2 05/15] target/m68k: Remove retaddr in m68k_interrupt_all Richard Henderson
  2021-12-03  9:03   ` Philippe Mathieu-Daudé
@ 2021-12-03  9:08   ` Laurent Vivier
  1 sibling, 0 replies; 27+ messages in thread
From: Laurent Vivier @ 2021-12-03  9:08 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel

Le 02/12/2021 à 21:48, Richard Henderson a écrit :
> The only value this variable holds is now env->pc.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/m68k/op_helper.c | 15 ++++++---------
>   1 file changed, 6 insertions(+), 9 deletions(-)

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



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 08/15] target/m68k: Fix address argument for EXCP_TRACE
  2021-12-02 20:48 ` [PATCH v2 08/15] target/m68k: Fix address argument for EXCP_TRACE Richard Henderson
@ 2021-12-03 14:21   ` Richard Henderson
  0 siblings, 0 replies; 27+ messages in thread
From: Richard Henderson @ 2021-12-03 14:21 UTC (permalink / raw)
  To: qemu-devel; +Cc: laurent

On 12/2/21 12:48 PM, Richard Henderson wrote:
> +static void gen_raise_exception_format2(DisasContext *s, int nr)
> +{
> +    /*
> +     * 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(s->base.pc_next), cpu_env,
> +                   offsetof(CPUM68KState, mmu.ar));
> +    gen_raise_exception(nr);
> +    s->base.is_jmp = DISAS_NORETURN;
> +}

Hmph, I think this only really works from within m68k_tr_translate_insn. But most of the 
uses are from within m68k_rt_tb_stop, where we have already advanced pc_next to the next 
instruction.

I'm not sure how to test this...


r~


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 06/15] target/m68k: Fix address argument for EXCP_CHK
  2021-12-02 20:48 ` [PATCH v2 06/15] target/m68k: Fix address argument for EXCP_CHK Richard Henderson
@ 2021-12-03 14:27   ` Laurent Vivier
  2021-12-03 14:29     ` Richard Henderson
  0 siblings, 1 reply; 27+ messages in thread
From: Laurent Vivier @ 2021-12-03 14:27 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel

Le 02/12/2021 à 21:48, Richard Henderson a écrit :
> 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.

It's weird to use mmu.ar as the field is used for MMU exceptions.

> 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().
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   linux-user/m68k/cpu_loop.c |  2 +-
>   target/m68k/op_helper.c    | 54 ++++++++++++++++++++------------------
>   2 files changed, 30 insertions(+), 26 deletions(-)

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


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 06/15] target/m68k: Fix address argument for EXCP_CHK
  2021-12-03 14:27   ` Laurent Vivier
@ 2021-12-03 14:29     ` Richard Henderson
  2021-12-03 14:58       ` Laurent Vivier
  0 siblings, 1 reply; 27+ messages in thread
From: Richard Henderson @ 2021-12-03 14:29 UTC (permalink / raw)
  To: Laurent Vivier, qemu-devel

On 12/3/21 6:27 AM, Laurent Vivier wrote:
> Le 02/12/2021 à 21:48, Richard Henderson a écrit :
>> 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.
> 
> It's weird to use mmu.ar as the field is used for MMU exceptions.

Should I rename the field to "excp_addr" or something?


r~

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



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 06/15] target/m68k: Fix address argument for EXCP_CHK
  2021-12-03 14:29     ` Richard Henderson
@ 2021-12-03 14:58       ` Laurent Vivier
  0 siblings, 0 replies; 27+ messages in thread
From: Laurent Vivier @ 2021-12-03 14:58 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel

Le 03/12/2021 à 15:29, Richard Henderson a écrit :
> On 12/3/21 6:27 AM, Laurent Vivier wrote:
>> Le 02/12/2021 à 21:48, Richard Henderson a écrit :
>>> 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.
>>
>> It's weird to use mmu.ar as the field is used for MMU exceptions.
> 
> Should I rename the field to "excp_addr" or something?
> 

No, I'm wondering if we shoud move it or duplicate it. It's not clear. I think we can keep it like 
this and later do a cleanup.

But I think you should add a comment in CPUM68KState next to ar to point out that it is also used to 
store address of CHK/CHK2/DIV/TRAP/....

Thanks,
Laurent


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 07/15] target/m68k: Fix pc, c flag, and address argument for EXCP_DIV0
  2021-12-02 20:48 ` [PATCH v2 07/15] target/m68k: Fix pc, c flag, and address argument for EXCP_DIV0 Richard Henderson
@ 2021-12-04 16:50   ` Laurent Vivier
  0 siblings, 0 replies; 27+ messages in thread
From: Laurent Vivier @ 2021-12-04 16:50 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel

Le 02/12/2021 à 21:48, Richard Henderson a écrit :
> 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().
> 
> 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(-)
> 

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



^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2021-12-04 16:51 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-02 20:48 [PATCH v2 00/15] target/m68k: Conditional traps + trap cleanup Richard Henderson
2021-12-02 20:48 ` [PATCH v2 01/15] target/m68k: Raise the TRAPn exception with the correct pc Richard Henderson
2021-12-03  9:04   ` Laurent Vivier
2021-12-02 20:48 ` [PATCH v2 02/15] target/m68k: Switch over exception type in m68k_interrupt_all Richard Henderson
2021-12-03  9:02   ` Philippe Mathieu-Daudé
2021-12-03  9:06   ` Laurent Vivier
2021-12-02 20:48 ` [PATCH v2 03/15] linux-user/m68k: Use force_sig_fault Richard Henderson
2021-12-02 20:48 ` [PATCH v2 04/15] linux-user/m68k: Handle EXCP_TRAP1 through EXCP_TRAP15 Richard Henderson
2021-12-02 20:48 ` [PATCH v2 05/15] target/m68k: Remove retaddr in m68k_interrupt_all Richard Henderson
2021-12-03  9:03   ` Philippe Mathieu-Daudé
2021-12-03  9:08   ` Laurent Vivier
2021-12-02 20:48 ` [PATCH v2 06/15] target/m68k: Fix address argument for EXCP_CHK Richard Henderson
2021-12-03 14:27   ` Laurent Vivier
2021-12-03 14:29     ` Richard Henderson
2021-12-03 14:58       ` Laurent Vivier
2021-12-02 20:48 ` [PATCH v2 07/15] target/m68k: Fix pc, c flag, and address argument for EXCP_DIV0 Richard Henderson
2021-12-04 16:50   ` Laurent Vivier
2021-12-02 20:48 ` [PATCH v2 08/15] target/m68k: Fix address argument for EXCP_TRACE Richard Henderson
2021-12-03 14:21   ` Richard Henderson
2021-12-02 20:48 ` [PATCH v2 09/15] target/m68k: Implement TRAPcc Richard Henderson
2021-12-02 20:48 ` [PATCH v2 10/15] target/m68k: Implement TRAPV Richard Henderson
2021-12-02 20:48 ` [PATCH v2 11/15] target/m68k: Implement FTRAPcc Richard Henderson
2021-12-02 20:48 ` [PATCH v2 12/15] target/m68k: Fix stack frame for EXCP_ILLEGAL Richard Henderson
2021-12-02 20:48 ` [PATCH v2 13/15] tests/tcg/m68k: Add trap.c Richard Henderson
2021-12-02 20:48 ` [PATCH v2 14/15] linux-user/strace: Fix print_syscall_err Richard Henderson
2021-12-03  8:57   ` Philippe Mathieu-Daudé
2021-12-02 20:49 ` [PATCH v2 15/15] linux-user/strace: Adjust get_thread_area for m68k Richard Henderson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.