[PATCH 0/3] target/sh4: Re-allow unaligned access on system emulation

[PATCH 0/3] target/sh4: Re-allow unaligned access on system emulation

[Philippe Mathieu-Daudé]

Restore unaligned behavior on system mode,
hold alignment in DisasContext.

Philippe Mathieu-Daudé (3):
  target/sh4: Re-allow unaligned access on system emulation
  target/sh4: Hold alignment as DisasContext::mo_align
  target/sh4: Inline UNALIGN() macro

 target/sh4/translate.c | 38 ++++++++++++++++++++------------------
 1 file changed, 20 insertions(+), 18 deletions(-)

-- 
2.53.0

[PATCH 1/3] target/sh4: Re-allow unaligned access on system emulation

[Philippe Mathieu-Daudé]

The change 0 -> MO_ALIGN in commit eb978e50e42 ("target/sh4: Use
MO_ALIGN for system UNALIGN()") would have been correct if it was
merged before commit 1fceff9c3ca ("tcg: Remove TARGET_ALIGNED_ONLY")
which defined MO_UNALN -> 0. What we really want here now is
MO_UNALN (MO_ALIGN is dealt with in commit 03a0d87e8dd "target/sh4:
Use MO_ALIGN where required").

Use the correct definition, otherwise we get when booting Linux:

  ok 14 bitfields
      KTAP version 1
      # Subtest: blackholedev
      # module: blackhole_dev_kunit
      1..1
  delay-slot-insn faulting in handle_unaligned_delayslot: 0000 [#1]
  Modules linked in:
  CPU: 0 UID: 0 PID: 298 Comm: kunit_try_catch Tainted: G                 N
  6.17.0-12907-g8765f467912f #1 NONE
  Tainted: [N]=TEST
  PC is at test_blackholedev+0xc6/0x1c8
  PR is at test_blackholedev+0xca/0x1c8
  PC  : 8c228d16 SP  : 8d40bed0 SR  : 40008001 TEA : 8cce4026
  R0  : 00000000 R1  : 00000000 R2  : 000000d2 R3  : 8cc77b80
  R4  : 8cc77b40 R5  : 0000000e R6  : 8c228cea R7  : 8cce400e
  R8  : 8cc77b40 R9  : 8cc77bbc R10 : 8cc77bbe R11 : 8c4b46b0
  R12 : 8c78310c R13 : 8cc29cf4 R14 : 8c6e3a88
  MACH: 0000d80d MACL: e554cb0f GBR : 00000000 PR  : 8c228d1a
  Call trace:
   [<8c05b378>] pick_next_task_fair+0x94/0x164
   [<8c5c2bce>] __schedule+0x306/0x6ec
   [<8c21fa68>] kunit_try_run_case+0x58/0x174
  ...

Cc: qemu-stable@nongnu.org
Fixes: eb978e50e42 ("target/sh4: Use MO_ALIGN for system UNALIGN()")
Reported-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/sh4/translate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/sh4/translate.c b/target/sh4/translate.c
index 5adf650744c..3ddd4f612e4 100644
--- a/target/sh4/translate.c
+++ b/target/sh4/translate.c
@@ -54,7 +54,7 @@ typedef struct DisasContext {
 #define UNALIGN(C)   (ctx->tbflags & TB_FLAG_UNALIGN ? MO_UNALN : MO_ALIGN)
 #else
 #define IS_USER(ctx) (!(ctx->tbflags & (1u << SR_MD)))
-#define UNALIGN(C)   MO_ALIGN
+#define UNALIGN(C)   MO_UNALN
 #endif
 
 /* Target-specific values for ctx->base.is_jmp.  */
-- 
2.53.0

Re: [PATCH 1/3] target/sh4: Re-allow unaligned access on system emulation

[Richard Henderson]

On 5/12/26 23:54, Philippe Mathieu-Daudé wrote:
> The change 0 -> MO_ALIGN in commit eb978e50e42 ("target/sh4: Use
> MO_ALIGN for system UNALIGN()") would have been correct if it was
> merged before commit 1fceff9c3ca ("tcg: Remove TARGET_ALIGNED_ONLY")
> which defined MO_UNALN -> 0. What we really want here now is
> MO_UNALN (MO_ALIGN is dealt with in commit 03a0d87e8dd "target/sh4:
> Use MO_ALIGN where required").

Not correct.  All system mode accesses are aligned, or trap.

All of the UNALIGN are unaligned for linux-user only, in that those are the insns that the 
linux kernel emulates.  Since the linux kernel does not emulate all insns, all other insns 
must be aligned -- that's what 03a0d87e8dd is about.

> Use the correct definition, otherwise we get when booting Linux:
> 
>    ok 14 bitfields
>        KTAP version 1
>        # Subtest: blackholedev
>        # module: blackhole_dev_kunit
>        1..1
>    delay-slot-insn faulting in handle_unaligned_delayslot: 0000 [#1]
>    Modules linked in:
>    CPU: 0 UID: 0 PID: 298 Comm: kunit_try_catch Tainted: G                 N
>    6.17.0-12907-g8765f467912f #1 NONE
>    Tainted: [N]=TEST
>    PC is at test_blackholedev+0xc6/0x1c8
>    PR is at test_blackholedev+0xca/0x1c8
>    PC  : 8c228d16 SP  : 8d40bed0 SR  : 40008001 TEA : 8cce4026
>    R0  : 00000000 R1  : 00000000 R2  : 000000d2 R3  : 8cc77b80
>    R4  : 8cc77b40 R5  : 0000000e R6  : 8c228cea R7  : 8cce400e
>    R8  : 8cc77b40 R9  : 8cc77bbc R10 : 8cc77bbe R11 : 8c4b46b0
>    R12 : 8c78310c R13 : 8cc29cf4 R14 : 8c6e3a88
>    MACH: 0000d80d MACL: e554cb0f GBR : 00000000 PR  : 8c228d1a
>    Call trace:
>     [<8c05b378>] pick_next_task_fair+0x94/0x164
>     [<8c5c2bce>] __schedule+0x306/0x6ec
>     [<8c21fa68>] kunit_try_run_case+0x58/0x174
>    ...

That would be a kernel bug then.


r~

Re: [PATCH 1/3] target/sh4: Re-allow unaligned access on system emulation

[Guenter Roeck]

On Wed, May 13, 2026 at 11:24:27AM -0700, Richard Henderson wrote:
> On 5/12/26 23:54, Philippe Mathieu-Daudé wrote:
> > The change 0 -> MO_ALIGN in commit eb978e50e42 ("target/sh4: Use
> > MO_ALIGN for system UNALIGN()") would have been correct if it was
> > merged before commit 1fceff9c3ca ("tcg: Remove TARGET_ALIGNED_ONLY")
> > which defined MO_UNALN -> 0. What we really want here now is
> > MO_UNALN (MO_ALIGN is dealt with in commit 03a0d87e8dd "target/sh4:
> > Use MO_ALIGN where required").
> 
> Not correct.  All system mode accesses are aligned, or trap.
> 
> All of the UNALIGN are unaligned for linux-user only, in that those are the
> insns that the linux kernel emulates.  Since the linux kernel does not
> emulate all insns, all other insns must be aligned -- that's what
> 03a0d87e8dd is about.
> 
> > Use the correct definition, otherwise we get when booting Linux:
> > 
> >    ok 14 bitfields
> >        KTAP version 1
> >        # Subtest: blackholedev
> >        # module: blackhole_dev_kunit
> >        1..1
> >    delay-slot-insn faulting in handle_unaligned_delayslot: 0000 [#1]
> >    Modules linked in:
> >    CPU: 0 UID: 0 PID: 298 Comm: kunit_try_catch Tainted: G                 N
> >    6.17.0-12907-g8765f467912f #1 NONE
> >    Tainted: [N]=TEST
> >    PC is at test_blackholedev+0xc6/0x1c8
> >    PR is at test_blackholedev+0xca/0x1c8
> >    PC  : 8c228d16 SP  : 8d40bed0 SR  : 40008001 TEA : 8cce4026
> >    R0  : 00000000 R1  : 00000000 R2  : 000000d2 R3  : 8cc77b80
> >    R4  : 8cc77b40 R5  : 0000000e R6  : 8c228cea R7  : 8cce400e
> >    R8  : 8cc77b40 R9  : 8cc77bbc R10 : 8cc77bbe R11 : 8c4b46b0
> >    R12 : 8c78310c R13 : 8cc29cf4 R14 : 8c6e3a88
> >    MACH: 0000d80d MACL: e554cb0f GBR : 00000000 PR  : 8c228d1a
> >    Call trace:
> >     [<8c05b378>] pick_next_task_fair+0x94/0x164
> >     [<8c5c2bce>] __schedule+0x306/0x6ec
> >     [<8c21fa68>] kunit_try_run_case+0x58/0x174
> >    ...
> 
> That would be a kernel bug then.

It is interesting that no one reported this against real hardware,
but then who knows if anyone still runs that hardware and/or that set
of tests.

If I recall correctly, there were other problems when running the sh4
emulation with eb978e50e42f3 in the tree. I have reverted that patch in
my downstream version of qemu, and I'll be happy to carry the revert
along, so from my perspective it is ok to not address the problem in qemu.

Guenter

Re: [PATCH 1/3] target/sh4: Re-allow unaligned access on system emulation

[yoshinori.sato]

On Thu, 14 May 2026 03:34:30 +0900,
Guenter Roeck wrote:
> 
> On Wed, May 13, 2026 at 11:24:27AM -0700, Richard Henderson wrote:
> > On 5/12/26 23:54, Philippe Mathieu-Daudé wrote:
> > > The change 0 -> MO_ALIGN in commit eb978e50e42 ("target/sh4: Use
> > > MO_ALIGN for system UNALIGN()") would have been correct if it was
> > > merged before commit 1fceff9c3ca ("tcg: Remove TARGET_ALIGNED_ONLY")
> > > which defined MO_UNALN -> 0. What we really want here now is
> > > MO_UNALN (MO_ALIGN is dealt with in commit 03a0d87e8dd "target/sh4:
> > > Use MO_ALIGN where required").
> > 
> > Not correct.  All system mode accesses are aligned, or trap.
> > 
> > All of the UNALIGN are unaligned for linux-user only, in that those are the
> > insns that the linux kernel emulates.  Since the linux kernel does not
> > emulate all insns, all other insns must be aligned -- that's what
> > 03a0d87e8dd is about.
> > 
> > > Use the correct definition, otherwise we get when booting Linux:
> > > 
> > >    ok 14 bitfields
> > >        KTAP version 1
> > >        # Subtest: blackholedev
> > >        # module: blackhole_dev_kunit
> > >        1..1
> > >    delay-slot-insn faulting in handle_unaligned_delayslot: 0000 [#1]
> > >    Modules linked in:
> > >    CPU: 0 UID: 0 PID: 298 Comm: kunit_try_catch Tainted: G                 N
> > >    6.17.0-12907-g8765f467912f #1 NONE
> > >    Tainted: [N]=TEST
> > >    PC is at test_blackholedev+0xc6/0x1c8
> > >    PR is at test_blackholedev+0xca/0x1c8
> > >    PC  : 8c228d16 SP  : 8d40bed0 SR  : 40008001 TEA : 8cce4026
> > >    R0  : 00000000 R1  : 00000000 R2  : 000000d2 R3  : 8cc77b80
> > >    R4  : 8cc77b40 R5  : 0000000e R6  : 8c228cea R7  : 8cce400e
> > >    R8  : 8cc77b40 R9  : 8cc77bbc R10 : 8cc77bbe R11 : 8c4b46b0
> > >    R12 : 8c78310c R13 : 8cc29cf4 R14 : 8c6e3a88
> > >    MACH: 0000d80d MACL: e554cb0f GBR : 00000000 PR  : 8c228d1a
> > >    Call trace:
> > >     [<8c05b378>] pick_next_task_fair+0x94/0x164
> > >     [<8c5c2bce>] __schedule+0x306/0x6ec
> > >     [<8c21fa68>] kunit_try_run_case+0x58/0x174
> > >    ...
> > 
> > That would be a kernel bug then.
> 
> It is interesting that no one reported this against real hardware,
> but then who knows if anyone still runs that hardware and/or that set
> of tests.
> 
> If I recall correctly, there were other problems when running the sh4
> emulation with eb978e50e42f3 in the tree. I have reverted that patch in
> my downstream version of qemu, and I'll be happy to carry the revert
> along, so from my perspective it is ok to not address the problem in qemu.
> 
> Guenter

According to the specifications of the SH4 CPU, an exception should occur
in that situation.
From the trace, we can see that the non-aligned access occurred in 8cce4026.
This is not a 32-bit boundary, so I think an exception occurred when trying
to read or write using "mov.L".

The `test_blackholedev` function, where the problem occurred, manipulates
several structures.
Looking at the values ​​of each register, R7 has a value close to the address
where the exception occurred, so it is likely that this is the starting
address of the structure.

The problem is that this address is also not on a 32-bit boundary with 8cce400e.
I believe the reason this doesn't cause problems on actual hardware is because
this is correctly defined as a 32-bit boundary.

First, I think it would be best to investigate the cause of this difference in
structure addresses.

-- 
Yosinori Sato

[PATCH 2/3] target/sh4: Hold alignment as DisasContext::mo_align

[Philippe Mathieu-Daudé]

Hold the alignment in DisasContext::mo_align,
initializing it once in TranslatorOps::init_disas_context().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/sh4/translate.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/target/sh4/translate.c b/target/sh4/translate.c
index 3ddd4f612e4..5957f294c22 100644
--- a/target/sh4/translate.c
+++ b/target/sh4/translate.c
@@ -39,6 +39,7 @@ typedef struct DisasContext {
     uint32_t tbflags;  /* should stay unmodified during the TB translation */
     uint32_t envflags; /* should stay in sync with env->flags using TCG ops */
     int memidx;
+    MemOp mo_align;
     int gbank;
     int fbank;
     uint32_t delayed_pc;
@@ -51,10 +52,10 @@ typedef struct DisasContext {
 
 #if defined(CONFIG_USER_ONLY)
 #define IS_USER(ctx) 1
-#define UNALIGN(C)   (ctx->tbflags & TB_FLAG_UNALIGN ? MO_UNALN : MO_ALIGN)
+#define UNALIGN(C)   ctx->mo_align
 #else
 #define IS_USER(ctx) (!(ctx->tbflags & (1u << SR_MD)))
-#define UNALIGN(C)   MO_UNALN
+#define UNALIGN(C)   ctx->mo_align
 #endif
 
 /* Target-specific values for ctx->base.is_jmp.  */
@@ -2225,6 +2226,9 @@ static void sh4_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
             return;
         }
     }
+    ctx->mo_align = (ctx->tbflags & TB_FLAG_UNALIGN) ? MO_UNALN : MO_ALIGN;
+#else /* !CONFIG_USER_ONLY */
+    ctx->mo_align = MO_UNALN;
 #endif
 
     /* Since the ISA is fixed-width, we can bound by the number
-- 
2.53.0

[PATCH 3/3] target/sh4: Inline UNALIGN() macro

[Philippe Mathieu-Daudé]

Directly access DisasContext::mo_align in place.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/sh4/translate.c | 34 ++++++++++++++++------------------
 1 file changed, 16 insertions(+), 18 deletions(-)

diff --git a/target/sh4/translate.c b/target/sh4/translate.c
index 5957f294c22..dfe7330d95d 100644
--- a/target/sh4/translate.c
+++ b/target/sh4/translate.c
@@ -52,10 +52,8 @@ typedef struct DisasContext {
 
 #if defined(CONFIG_USER_ONLY)
 #define IS_USER(ctx) 1
-#define UNALIGN(C)   ctx->mo_align
 #else
 #define IS_USER(ctx) (!(ctx->tbflags & (1u << SR_MD)))
-#define UNALIGN(C)   ctx->mo_align
 #endif
 
 /* Target-specific values for ctx->base.is_jmp.  */
@@ -496,7 +494,7 @@ static void _decode_opc(DisasContext * ctx)
             TCGv addr = tcg_temp_new();
             tcg_gen_addi_i32(addr, REG(B11_8), B3_0 * 4);
             tcg_gen_qemu_st_i32(REG(B7_4), addr, ctx->memidx,
-                                MO_TEUL | UNALIGN(ctx));
+                                MO_TEUL | ctx->mo_align);
         }
         return;
     case 0x5000: /* mov.l @(disp,Rm),Rn */
@@ -504,7 +502,7 @@ static void _decode_opc(DisasContext * ctx)
             TCGv addr = tcg_temp_new();
             tcg_gen_addi_i32(addr, REG(B7_4), B3_0 * 4);
             tcg_gen_qemu_ld_i32(REG(B11_8), addr, ctx->memidx,
-                                MO_TESL | UNALIGN(ctx));
+                                MO_TESL | ctx->mo_align);
         }
         return;
     case 0xe000: /* mov #imm,Rn */
@@ -564,22 +562,22 @@ static void _decode_opc(DisasContext * ctx)
         return;
     case 0x2001: /* mov.w Rm,@Rn */
         tcg_gen_qemu_st_i32(REG(B7_4), REG(B11_8), ctx->memidx,
-                            MO_TEUW | UNALIGN(ctx));
+                            MO_TEUW | ctx->mo_align);
         return;
     case 0x2002: /* mov.l Rm,@Rn */
         tcg_gen_qemu_st_i32(REG(B7_4), REG(B11_8), ctx->memidx,
-                            MO_TEUL | UNALIGN(ctx));
+                            MO_TEUL | ctx->mo_align);
         return;
     case 0x6000: /* mov.b @Rm,Rn */
         tcg_gen_qemu_ld_i32(REG(B11_8), REG(B7_4), ctx->memidx, MO_SB);
         return;
     case 0x6001: /* mov.w @Rm,Rn */
         tcg_gen_qemu_ld_i32(REG(B11_8), REG(B7_4), ctx->memidx,
-                            MO_TESW | UNALIGN(ctx));
+                            MO_TESW | ctx->mo_align);
         return;
     case 0x6002: /* mov.l @Rm,Rn */
         tcg_gen_qemu_ld_i32(REG(B11_8), REG(B7_4), ctx->memidx,
-                            MO_TESL | UNALIGN(ctx));
+                            MO_TESL | ctx->mo_align);
         return;
     case 0x2004: /* mov.b Rm,@-Rn */
         {
@@ -595,7 +593,7 @@ static void _decode_opc(DisasContext * ctx)
             TCGv addr = tcg_temp_new();
             tcg_gen_subi_i32(addr, REG(B11_8), 2);
             tcg_gen_qemu_st_i32(REG(B7_4), addr, ctx->memidx,
-                                MO_TEUW | UNALIGN(ctx));
+                                MO_TEUW | ctx->mo_align);
             tcg_gen_mov_i32(REG(B11_8), addr);
         }
         return;
@@ -604,7 +602,7 @@ static void _decode_opc(DisasContext * ctx)
             TCGv addr = tcg_temp_new();
             tcg_gen_subi_i32(addr, REG(B11_8), 4);
             tcg_gen_qemu_st_i32(REG(B7_4), addr, ctx->memidx,
-                                MO_TEUL | UNALIGN(ctx));
+                                MO_TEUL | ctx->mo_align);
             tcg_gen_mov_i32(REG(B11_8), addr);
         }
         return;
@@ -615,13 +613,13 @@ static void _decode_opc(DisasContext * ctx)
         return;
     case 0x6005: /* mov.w @Rm+,Rn */
         tcg_gen_qemu_ld_i32(REG(B11_8), REG(B7_4), ctx->memidx,
-                            MO_TESW | UNALIGN(ctx));
+                            MO_TESW | ctx->mo_align);
         if ( B11_8 != B7_4 )
                 tcg_gen_addi_i32(REG(B7_4), REG(B7_4), 2);
         return;
     case 0x6006: /* mov.l @Rm+,Rn */
         tcg_gen_qemu_ld_i32(REG(B11_8), REG(B7_4), ctx->memidx,
-                            MO_TESL | UNALIGN(ctx));
+                            MO_TESL | ctx->mo_align);
         if ( B11_8 != B7_4 )
                 tcg_gen_addi_i32(REG(B7_4), REG(B7_4), 4);
         return;
@@ -637,7 +635,7 @@ static void _decode_opc(DisasContext * ctx)
             TCGv addr = tcg_temp_new();
             tcg_gen_add_i32(addr, REG(B11_8), REG(0));
             tcg_gen_qemu_st_i32(REG(B7_4), addr, ctx->memidx,
-                                MO_TEUW | UNALIGN(ctx));
+                                MO_TEUW | ctx->mo_align);
         }
         return;
     case 0x0006: /* mov.l Rm,@(R0,Rn) */
@@ -645,7 +643,7 @@ static void _decode_opc(DisasContext * ctx)
             TCGv addr = tcg_temp_new();
             tcg_gen_add_i32(addr, REG(B11_8), REG(0));
             tcg_gen_qemu_st_i32(REG(B7_4), addr, ctx->memidx,
-                                MO_TEUL | UNALIGN(ctx));
+                                MO_TEUL | ctx->mo_align);
         }
         return;
     case 0x000c: /* mov.b @(R0,Rm),Rn */
@@ -660,7 +658,7 @@ static void _decode_opc(DisasContext * ctx)
             TCGv addr = tcg_temp_new();
             tcg_gen_add_i32(addr, REG(B7_4), REG(0));
             tcg_gen_qemu_ld_i32(REG(B11_8), addr, ctx->memidx,
-                                MO_TESW | UNALIGN(ctx));
+                                MO_TESW | ctx->mo_align);
         }
         return;
     case 0x000e: /* mov.l @(R0,Rm),Rn */
@@ -668,7 +666,7 @@ static void _decode_opc(DisasContext * ctx)
             TCGv addr = tcg_temp_new();
             tcg_gen_add_i32(addr, REG(B7_4), REG(0));
             tcg_gen_qemu_ld_i32(REG(B11_8), addr, ctx->memidx,
-                                MO_TESL | UNALIGN(ctx));
+                                MO_TESL | ctx->mo_align);
         }
         return;
     case 0x6008: /* swap.b Rm,Rn */
@@ -1222,7 +1220,7 @@ static void _decode_opc(DisasContext * ctx)
             TCGv addr = tcg_temp_new();
             tcg_gen_addi_i32(addr, REG(B7_4), B3_0 * 2);
             tcg_gen_qemu_st_i32(REG(0), addr, ctx->memidx,
-                                MO_TEUW | UNALIGN(ctx));
+                                MO_TEUW | ctx->mo_align);
         }
         return;
     case 0x8400: /* mov.b @(disp,Rn),R0 */
@@ -1237,7 +1235,7 @@ static void _decode_opc(DisasContext * ctx)
             TCGv addr = tcg_temp_new();
             tcg_gen_addi_i32(addr, REG(B7_4), B3_0 * 2);
             tcg_gen_qemu_ld_i32(REG(0), addr, ctx->memidx,
-                                MO_TESW | UNALIGN(ctx));
+                                MO_TESW | ctx->mo_align);
         }
         return;
     case 0xc700: /* mova @(disp,PC),R0 */
-- 
2.53.0