| Message ID | 20211202204900.50973-7-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | target/m68k: Conditional traps + trap cleanup | expand |
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>
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>
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
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()); } }
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(-)