| Message ID | 20230802161914.395443-4-keithp@keithp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | target/m68k: Fix a few semihosting bugs | expand |
On Wed, 2 Aug 2023 at 17:20, Keith Packard via <qemu-devel@nongnu.org> wrote: > > According to the m68k semihosting spec: > > "The instruction used to trigger a semihosting request depends on the > m68k processor variant. On ColdFire, "halt" is used; on other processors > (which don't implement "halt"), "bkpt #0" may be used." > > Add support for non-CodeFire processors by matching BKPT #0 > instructions. When semihosting is disabled, convert those > back to illegal op exceptions. > > Signed-off-by: Keith Packard <keithp@keithp.com> > --- > target/m68k/cpu.h | 1 + > target/m68k/op_helper.c | 16 ++++++++++++++++ > target/m68k/translate.c | 4 ++++ > 3 files changed, 21 insertions(+) > > diff --git a/target/m68k/cpu.h b/target/m68k/cpu.h > index cf70282717..b741c50a8f 100644 > --- a/target/m68k/cpu.h > +++ b/target/m68k/cpu.h > @@ -67,6 +67,7 @@ > > #define EXCP_RTE 0x100 > #define EXCP_HALT_INSN 0x101 > +#define EXCP_BKPT_INSN 0x102 > > #define M68K_DTTR0 0 > #define M68K_DTTR1 1 > diff --git a/target/m68k/op_helper.c b/target/m68k/op_helper.c > index 1ce850bbc5..2d89db6dde 100644 > --- a/target/m68k/op_helper.c > +++ b/target/m68k/op_helper.c > @@ -295,6 +295,22 @@ static void m68k_interrupt_all(CPUM68KState *env, int is_hw) > /* Return from an exception. */ > m68k_rte(env); > return; > + case EXCP_BKPT_INSN: > + if (semihosting_enabled((env->sr & SR_S) == 0) > + && (env->pc & 3) == 0 > + && cpu_lduw_code(env, env->pc - 4) == 0x4e71 > + && cpu_ldl_code(env, env->pc) == 0x4e7bf000) { This long condition is identical to the one used for semihosting-via-halt. It could usefully be moved out to a function, which can then have a comment noting what it's doing (i.e. checking for the required insns preceding and following the bkpt or halt). > + env->pc += 4; > + do_m68k_semihosting(env, env->dregs[0]); > + return; > + } > + /* > + * When semihosting is not enabled, translate this back to > + * an illegal op exception. > + */ > + cs->exception_index = EXCP_ILLEGAL; > + env->pc += 2; > + break; > } > } > > diff --git a/target/m68k/translate.c b/target/m68k/translate.c > index e07161d76f..d037c57453 100644 > --- a/target/m68k/translate.c > +++ b/target/m68k/translate.c > @@ -2640,6 +2640,10 @@ DISAS_INSN(bkpt) > #if defined(CONFIG_USER_ONLY) > gen_exception(s, s->base.pc_next, EXCP_DEBUG); > #else > + if ((insn & 7) == 0) { > + gen_exception(s, s->pc, EXCP_BKPT_INSN); > + return; > + } > gen_exception(s, s->base.pc_next, EXCP_ILLEGAL); This means that semihosting-via-bpkt will only work in system emulation, because in user mode the EXCP_DEBUG will take precedence. To handle that you need to also check in linux-user/m68k/cpu_loop.c. But this effectively also implies reverting most of a638af09b6c6b (where we took out a lot of "m68k usermode semihosting" because it wasn't reachable). So it's probably not worthwhile. -- PMM
diff --git a/target/m68k/cpu.h b/target/m68k/cpu.h index cf70282717..b741c50a8f 100644 --- a/target/m68k/cpu.h +++ b/target/m68k/cpu.h @@ -67,6 +67,7 @@ #define EXCP_RTE 0x100 #define EXCP_HALT_INSN 0x101 +#define EXCP_BKPT_INSN 0x102 #define M68K_DTTR0 0 #define M68K_DTTR1 1 diff --git a/target/m68k/op_helper.c b/target/m68k/op_helper.c index 1ce850bbc5..2d89db6dde 100644 --- a/target/m68k/op_helper.c +++ b/target/m68k/op_helper.c @@ -295,6 +295,22 @@ static void m68k_interrupt_all(CPUM68KState *env, int is_hw) /* Return from an exception. */ m68k_rte(env); return; + case EXCP_BKPT_INSN: + if (semihosting_enabled((env->sr & SR_S) == 0) + && (env->pc & 3) == 0 + && cpu_lduw_code(env, env->pc - 4) == 0x4e71 + && cpu_ldl_code(env, env->pc) == 0x4e7bf000) { + env->pc += 4; + do_m68k_semihosting(env, env->dregs[0]); + return; + } + /* + * When semihosting is not enabled, translate this back to + * an illegal op exception. + */ + cs->exception_index = EXCP_ILLEGAL; + env->pc += 2; + break; } } diff --git a/target/m68k/translate.c b/target/m68k/translate.c index e07161d76f..d037c57453 100644 --- a/target/m68k/translate.c +++ b/target/m68k/translate.c @@ -2640,6 +2640,10 @@ DISAS_INSN(bkpt) #if defined(CONFIG_USER_ONLY) gen_exception(s, s->base.pc_next, EXCP_DEBUG); #else + if ((insn & 7) == 0) { + gen_exception(s, s->pc, EXCP_BKPT_INSN); + return; + } gen_exception(s, s->base.pc_next, EXCP_ILLEGAL); #endif }
According to the m68k semihosting spec: "The instruction used to trigger a semihosting request depends on the m68k processor variant. On ColdFire, "halt" is used; on other processors (which don't implement "halt"), "bkpt #0" may be used." Add support for non-CodeFire processors by matching BKPT #0 instructions. When semihosting is disabled, convert those back to illegal op exceptions. Signed-off-by: Keith Packard <keithp@keithp.com> --- target/m68k/cpu.h | 1 + target/m68k/op_helper.c | 16 ++++++++++++++++ target/m68k/translate.c | 4 ++++ 3 files changed, 21 insertions(+)