From: Thomas Huth <th.huth+qemu@posteo.eu>
To: 54weasels <54weasels@gmail.com>
Cc: qemu-devel@nongnu.org, laurent@vivier.eu, thuth@redhat.com
Subject: Re: [PATCH 1/7] target/m68k: Implement Physical Bus Error exception handling
Date: Sun, 10 May 2026 05:40:42 +0000 [thread overview]
Message-ID: <20260510074039.003600dd@tpx1> (raw)
In-Reply-To: <20260503015756.99176-2-54weasels@gmail.com>
Hi!
Thanks for your patches ... some comments inline below...
Am Sat, 2 May 2026 18:57:50 -0700
schrieb 54weasels <54weasels@gmail.com>:
> 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 <54weasels@gmail.com>
> ---
> 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
next prev parent reply other threads:[~2026-05-10 5:41 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-03 1:57 [PATCH 0/7] m68k: Add Sun-3 Machine Emulation 54weasels
2026-05-03 1:57 ` [PATCH 1/7] target/m68k: Implement Physical Bus Error exception handling 54weasels
2026-05-10 5:40 ` Thomas Huth [this message]
2026-05-11 6:38 ` Purr Box
2026-05-03 1:57 ` [PATCH 2/7] hw/net/lance: Add Sun-3 Native DMA byte-swapping support 54weasels
2026-05-03 1:57 ` [PATCH 3/7] hw/char/escc: Expose diagnostic RS232 I/O routing 54weasels
2026-05-03 1:57 ` [PATCH 4/7] hw/timer: Introduce Intersil 7170 RTC implementation 54weasels
2026-05-03 1:57 ` [PATCH 5/7] hw/m68k: Overhaul Sun-3 MMU and Boot PROM mapping 54weasels
2026-05-03 1:57 ` [PATCH 6/7] tests/qtest: Add Sun-3 hardware interaction tests 54weasels
2026-05-03 1:57 ` [PATCH 7/7] tests/functional: Add Sun-3 firmware boot and diagnostic test 54weasels
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260510074039.003600dd@tpx1 \
--to=th.huth+qemu@posteo.eu \
--cc=54weasels@gmail.com \
--cc=laurent@vivier.eu \
--cc=qemu-devel@nongnu.org \
--cc=thuth@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.