Thanks for the feedback, I'll perform the clean-ups and start splitting some stuff up. If you have more suggestions for how it'll be easiest to cut, let me know. I'll resubmit a new patch and reference this one asap.
Thanks again, Dan. On Sat, May 9, 2026 at 10:40 PM Thomas Huth <[email protected]> wrote: > > Hi! > > Thanks for your patches ... some comments inline below... > > Am Sat, 2 May 2026 18:57:50 -0700 > schrieb 54weasels <[email protected]>: > > > The M68020 natively maps hardware Bus Error (BERR) timeouts into a Long > Bus Cycle Fault (Format 0xB). This commit adds the memory exception routing > to natively synthesize these EXCP_ACCESS cycle faults. It also implements > the double-fault / watchdog reset behavior required for Sun-3 hardware > diagnostics, properly handles FSAVE/FRESTORE for 68881 FPU stubs, and > properly constructs the 84-byte internal bus fault frame. > > > > Signed-off-by: 54weasels <[email protected]> > > --- > > target/m68k/cpu.c | 5 +- > > target/m68k/cpu.h | 18 +++- > > target/m68k/helper.c | 130 ++++++++++++++++++++++++++++- > > target/m68k/op_helper.c | 176 ++++++++++++++++++++++++++-------------- > > target/m68k/translate.c | 31 +++++-- > > 5 files changed, 283 insertions(+), 77 deletions(-) > > > > diff --git a/target/m68k/cpu.c b/target/m68k/cpu.c > > index d849a4a90f..af375f0bce 100644 > > --- a/target/m68k/cpu.c > > +++ b/target/m68k/cpu.c > > @@ -1,3 +1,4 @@ > > +/* SPDX-License-Identifier: GPL-2.0-or-later */ > > Looking at the comment at the top of this file, it clearly states that this > file is licensed under the Lesser GPL, so this change is wrong. > > > /* > > * QEMU Motorola 68k CPU > > * > > @@ -51,8 +52,8 @@ static TCGTBCPUState m68k_get_tb_cpu_state(CPUState > *cs) > > flags = (env->macsr >> 4) & TB_FLAGS_MACSR; > > if (env->sr & SR_S) { > > flags |= TB_FLAGS_MSR_S; > > - flags |= (env->sfc << (TB_FLAGS_SFC_S_BIT - 2)) & > TB_FLAGS_SFC_S; > > - flags |= (env->dfc << (TB_FLAGS_DFC_S_BIT - 2)) & > TB_FLAGS_DFC_S; > > + flags |= (env->sfc << TB_FLAGS_SFC_S_BIT) & TB_FLAGS_SFC_S; > > + flags |= (env->dfc << TB_FLAGS_DFC_S_BIT) & TB_FLAGS_DFC_S; > > Wrong indentation now? > > > } > > if (M68K_SR_TRACE(env->sr) == M68K_SR_TRACE_ANY_INS) { > > flags |= TB_FLAGS_TRACE; > ... > > diff --git a/target/m68k/op_helper.c b/target/m68k/op_helper.c > > index 8148a8852e..84d5270767 100644 > > --- a/target/m68k/op_helper.c > > +++ b/target/m68k/op_helper.c > > @@ -1,3 +1,4 @@ > > +/* SPDX-License-Identifier: GPL-2.0-or-later */ > > /* > > * M68K helper routines > > * > > @@ -25,6 +26,7 @@ > > #include "qemu/plugin.h" > > > > #if !defined(CONFIG_USER_ONLY) > > +#include "system/runstate.h" > > > > static void cf_rte(CPUM68KState *env) > > { > > @@ -73,6 +75,12 @@ throwaway: > > case 7: > > sp += 52; > > break; > > + case 0xa: /* Short Bus Cycle Fault (Format 0xA) */ > > + sp += 32 - 8; /* 32 bytes total - 8 bytes header = 24 bytes > */ > > + break; > > + case 0xb: /* Long Bus Cycle Fault (Format 0xB) */ > > + sp += 92 - 8; /* 92 bytes total - 8 bytes header = 84 bytes > */ > > + break; > > } > > } > > env->aregs[7] = sp; > > @@ -342,56 +350,80 @@ static void m68k_interrupt_all(CPUM68KState *env, > int is_hw) > > switch (cs->exception_index) { > > case EXCP_ACCESS: > > if (env->mmu.fault) { > > - cpu_abort(cs, "DOUBLE MMU FAULT\n"); > > + qemu_log_mask(LOG_GUEST_ERROR, > > + "M68K: Double MMU Fault. Halting CPU and > requesting reset.\n"); > > + qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET); > > + cs->halted = 1; > > + cs->exception_index = EXCP_HLT; > > + cpu_loop_exit(cs); > > } > > env->mmu.fault = true; > > - /* push data 3 */ > > - sp -= 4; > > - cpu_stl_be_mmuidx_ra(env, sp, 0, MMU_KERNEL_IDX, 0); > > - /* push data 2 */ > > - sp -= 4; > > - cpu_stl_be_mmuidx_ra(env, sp, 0, MMU_KERNEL_IDX, 0); > > - /* push data 1 */ > > - sp -= 4; > > - cpu_stl_be_mmuidx_ra(env, sp, 0, MMU_KERNEL_IDX, 0); > > - /* write back 1 / push data 0 */ > > - sp -= 4; > > - cpu_stl_be_mmuidx_ra(env, sp, 0, MMU_KERNEL_IDX, 0); > > - /* write back 1 address */ > > - sp -= 4; > > - cpu_stl_be_mmuidx_ra(env, sp, 0, MMU_KERNEL_IDX, 0); > > - /* write back 2 data */ > > - sp -= 4; > > - cpu_stl_be_mmuidx_ra(env, sp, 0, MMU_KERNEL_IDX, 0); > > - /* write back 2 address */ > > - sp -= 4; > > - cpu_stl_be_mmuidx_ra(env, sp, 0, MMU_KERNEL_IDX, 0); > > - /* write back 3 data */ > > - sp -= 4; > > - cpu_stl_be_mmuidx_ra(env, sp, 0, MMU_KERNEL_IDX, 0); > > - /* write back 3 address */ > > - sp -= 4; > > - cpu_stl_be_mmuidx_ra(env, sp, env->mmu.ar, MMU_KERNEL_IDX, 0); > > - /* fault address */ > > - sp -= 4; > > - cpu_stl_be_mmuidx_ra(env, sp, env->mmu.ar, MMU_KERNEL_IDX, 0); > > - /* write back 1 status */ > > - sp -= 2; > > - cpu_stw_be_mmuidx_ra(env, sp, 0, MMU_KERNEL_IDX, 0); > > - /* write back 2 status */ > > - sp -= 2; > > - cpu_stw_be_mmuidx_ra(env, sp, 0, MMU_KERNEL_IDX, 0); > > - /* write back 3 status */ > > - sp -= 2; > > - cpu_stw_be_mmuidx_ra(env, sp, 0, MMU_KERNEL_IDX, 0); > > - /* special status word */ > > - sp -= 2; > > - cpu_stw_be_mmuidx_ra(env, sp, env->mmu.ssw, MMU_KERNEL_IDX, 0); > > - /* effective address */ > > - sp -= 4; > > - cpu_stl_be_mmuidx_ra(env, sp, env->mmu.ar, MMU_KERNEL_IDX, 0); > > - > > - do_stack_frame(env, &sp, 7, oldsr, 0, env->pc); > > + > > + if (m68k_feature(env, M68K_FEATURE_M68040)) { > > + /* push data 3 */ > > + sp -= 4; > > + cpu_stl_be_mmuidx_ra(env, sp, 0, MMU_KERNEL_IDX, 0); > > + /* push data 2 */ > > + sp -= 4; > > + cpu_stl_be_mmuidx_ra(env, sp, 0, MMU_KERNEL_IDX, 0); > > + /* push data 1 */ > > + sp -= 4; > > + cpu_stl_be_mmuidx_ra(env, sp, 0, MMU_KERNEL_IDX, 0); > > + /* write back 1 / push data 0 */ > > + sp -= 4; > > + cpu_stl_be_mmuidx_ra(env, sp, 0, MMU_KERNEL_IDX, 0); > > + /* write back 1 address */ > > + sp -= 4; > > + cpu_stl_be_mmuidx_ra(env, sp, 0, MMU_KERNEL_IDX, 0); > > + /* write back 2 data */ > > + sp -= 4; > > + cpu_stl_be_mmuidx_ra(env, sp, 0, MMU_KERNEL_IDX, 0); > > + /* write back 2 address */ > > + sp -= 4; > > + cpu_stl_be_mmuidx_ra(env, sp, 0, MMU_KERNEL_IDX, 0); > > + /* write back 3 data */ > > + sp -= 4; > > + cpu_stl_be_mmuidx_ra(env, sp, 0, MMU_KERNEL_IDX, 0); > > + /* write back 3 address */ > > + sp -= 4; > > + cpu_stl_be_mmuidx_ra(env, sp, env->mmu.ar, MMU_KERNEL_IDX, > 0); > > + /* fault address */ > > + sp -= 4; > > + cpu_stl_be_mmuidx_ra(env, sp, env->mmu.ar, MMU_KERNEL_IDX, > 0); > > + /* write back 1 status */ > > + sp -= 2; > > + cpu_stw_be_mmuidx_ra(env, sp, 0, MMU_KERNEL_IDX, 0); > > + /* write back 2 status */ > > + sp -= 2; > > + cpu_stw_be_mmuidx_ra(env, sp, 0, MMU_KERNEL_IDX, 0); > > + /* write back 3 status */ > > + sp -= 2; > > + cpu_stw_be_mmuidx_ra(env, sp, 0, MMU_KERNEL_IDX, 0); > > + /* special status word */ > > + sp -= 2; > > + cpu_stw_be_mmuidx_ra(env, sp, env->mmu.ssw, MMU_KERNEL_IDX, > 0); > > + /* effective address */ > > + sp -= 4; > > + cpu_stl_be_mmuidx_ra(env, sp, env->mmu.ar, MMU_KERNEL_IDX, > 0); > > + > > + do_stack_frame(env, &sp, 7, oldsr, 0, env->pc); > > + } else { > > + /* M68020 Long Bus Cycle Fault (Format 0xB) */ > > + /* > > + * 84 bytes of internal state are pushed before the generic > > + * 8-byte header > > + */ > > + sp -= 84; > > + for (int i = 0; i < 84; i += 4) { > > + cpu_stl_be_mmuidx_ra(env, sp + i, 0, MMU_KERNEL_IDX, 0); > > + } > > + /* Offset 0x02 from internal frame: SSW */ > > + cpu_stw_be_mmuidx_ra(env, sp + 2, env->mmu.ssw, > MMU_KERNEL_IDX, 0); > > + /* Offset 0x08 from internal frame: Fault Address */ > > + cpu_stl_be_mmuidx_ra(env, sp + 8, env->mmu.ar, > MMU_KERNEL_IDX, 0); > > + > > + do_stack_frame(env, &sp, 0xb, oldsr, 0, env->pc); > > + } > > Your patch got quite big already and does multiple things at once ... could > you maybe split independent parts like the above fixup for the stack frame > layout into a separate patch, so that this can get reviewed more easily? > > > diff --git a/target/m68k/translate.c b/target/m68k/translate.c > > index abc1c79f3c..d6fcd6c4d9 100644 > > --- a/target/m68k/translate.c > > +++ b/target/m68k/translate.c > > @@ -1,3 +1,4 @@ > > +/* SPDX-License-Identifier: GPL-2.0-or-later */ > > This file is also LPGL, not GPL. > > Thomas >
