[PATCH] linux-user/sh4: fix race in atomic variables

Mikulas Patocka posted 1 patch 6 days, 7 hours ago
There is a newer version of this series
accel/tcg/cpu-exec.c        |   15 +++++++++++++++
include/accel/tcg/cpu-ops.h |   20 ++++++++++++++++++++
target/sh4/cpu.c            |   25 ++++++++++++++++++++++++-
3 files changed, 59 insertions(+), 1 deletion(-)
[PATCH] linux-user/sh4: fix race in atomic variables
Posted by Mikulas Patocka 6 days, 7 hours ago
I'm experiencing random deadlocks when running heavily multithreaded
workload in qemu-sh4 userspace emulation. This patch fixes them.

The sh4 architecture doesn't support multiprocessing, the processor
doesn't have atomic instructions and it uses a technique known as gUSA to
provide atomicity guarantees w.r.t. signals or thread scheduling (see the
commit 3b894b699c9a for a brief description of gUSA).

The function decode_gusa attempts to recognize several well-known gUSA
regions and turn them into atomic instructions. When it fails to
recognize a known gUSA pattern, it generates a call to helper_exclusive.
Qemu will attempt to stop all the other threads and execute a gUSA region
exclusively, so that it can't race with anything.

The problem is in the function cpu_exec_step_atomic - this function stops
all other threads with start_exclusive(), then it executes one
instruction and then it releases all other threads with end_exclusive().
This works for all the architectures except sh4. On sh4, executing one
instruction atomically is not enough - we must execute the full gUSA
region while the other threads are stopped.

This patch fixes cpu_exec_step_atomic - it adds two new per-architecture
functions: is_uninterruptible and revert_uninterruptible.

is_uninterruptible returns true if we are in a gUSA region and we should
continue executing code while the other threads are stopped.

If we got TB_EXIT_REQUESTED, we must stop executing code - in this case,
we call the function revert_uninterruptible that rolls back PC to the
beginning of the gUSA region.

Cc: qemu-stable@nongnu.org
Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>

---
 accel/tcg/cpu-exec.c        |   15 +++++++++++++++
 include/accel/tcg/cpu-ops.h |   20 ++++++++++++++++++++
 target/sh4/cpu.c            |   25 ++++++++++++++++++++++++-
 3 files changed, 59 insertions(+), 1 deletion(-)

Index: qemu/accel/tcg/cpu-exec.c
===================================================================
--- qemu.orig/accel/tcg/cpu-exec.c	2026-09-20 13:16:20.000000000 +0200
+++ qemu/accel/tcg/cpu-exec.c	2026-09-20 13:16:20.000000000 +0200
@@ -560,6 +560,9 @@ void cpu_exec_step_atomic(CPUState *cpu)
         g_assert(!cpu->running);
         cpu->running = true;
 
+#ifdef CONFIG_USER_ONLY
+next_instr:
+#endif
         TCGTBCPUState s = cpu->cc->tcg_ops->get_tb_cpu_state(cpu);
         s.cflags = curr_cflags(cpu);
 
@@ -586,7 +589,19 @@ void cpu_exec_step_atomic(CPUState *cpu)
         trace_exec_tb(tb, s.pc);
         cpu_tb_exec(cpu, tb, &tb_exit);
         cpu_exec_exit(cpu);
+#ifdef CONFIG_USER_ONLY
+        if (cpu->cc->tcg_ops->is_uninterruptible && cpu->cc->tcg_ops->is_uninterruptible(cpu)) {
+            if ((tb_exit & TB_EXIT_MASK) != TB_EXIT_REQUESTED)
+                goto next_instr;
+            if (cpu->cc->tcg_ops->revert_uninterruptible)
+                cpu->cc->tcg_ops->revert_uninterruptible(cpu);
+        }
+#endif
     } else {
+#ifdef CONFIG_USER_ONLY
+        if (cpu->cc->tcg_ops->revert_uninterruptible)
+            cpu->cc->tcg_ops->revert_uninterruptible(cpu);
+#endif
         cpu_exec_longjmp_cleanup(cpu);
     }
 
Index: qemu/include/accel/tcg/cpu-ops.h
===================================================================
--- qemu.orig/include/accel/tcg/cpu-ops.h	2026-09-20 13:16:20.000000000 +0200
+++ qemu/include/accel/tcg/cpu-ops.h	2026-09-20 13:16:20.000000000 +0200
@@ -168,6 +168,26 @@ struct TCGCPUOps {
      * @addr: tagged guest address
      */
     vaddr (*untagged_addr)(CPUState *cs, vaddr addr);
+
+    /**
+     * is_uninterruptible:
+     * @cpu: cpu context
+     *
+     * Returns true if we are in the middle of the gUSA region and
+     * cpu_exec_step_atomic must keep on executing instructions without
+     * dropping the exclusive lock.
+     */
+    bool (*is_uninterruptible)(CPUState *cs);
+
+    /**
+     * revert_uninterruptible:
+     * @cpu: cpu context
+     *
+     * This function is called if cpu_exec_step_atomic needs to exit. It
+     * tests if we are in the gUSA region and rolls back PC to the
+     * beginning of it.
+     */
+    void (*revert_uninterruptible)(CPUState *cs);
 #else
     /** @do_interrupt: Callback for interrupt handling.  */
     void (*do_interrupt)(CPUState *cpu);
Index: qemu/target/sh4/cpu.c
===================================================================
--- qemu.orig/target/sh4/cpu.c	2026-09-20 13:16:20.000000000 +0200
+++ qemu/target/sh4/cpu.c	2026-09-20 13:16:20.000000000 +0200
@@ -92,6 +92,26 @@ static void superh_restore_state_to_opc(
      */
 }
 
+#ifdef CONFIG_USER_ONLY
+static bool superh_cpu_is_uninterruptible(CPUState *cs)
+{
+    SuperHCPU *cpu = SUPERH_CPU(cs);
+
+    return cpu->env.gregs[15] >= -128u;
+}
+
+static void superh_cpu_revert_uninterruptible(CPUState *cs)
+{
+    SuperHCPU *cpu = SUPERH_CPU(cs);
+
+    if (cpu->env.gregs[15] >= -128u && cpu->env.pc < cpu->env.gregs[0]) {
+        cpu->env.pc = cpu->env.gregs[0] + cpu->env.gregs[15] - 2;
+        cpu->env.gregs[15] = cpu->env.gregs[1];
+        cpu->env.flags &= ~(TB_FLAG_DELAY_SLOT_MASK | TB_FLAG_GUSA_MASK);
+    }
+}
+#endif /* CONFIG_USER_ONLY */
+
 #ifndef CONFIG_USER_ONLY
 static bool superh_io_recompile_replay_branch(CPUState *cs,
                                               const TranslationBlock *tb)
@@ -308,7 +328,10 @@ static const TCGCPUOps superh_tcg_ops =
     .restore_state_to_opc = superh_restore_state_to_opc,
     .mmu_index = sh4_cpu_mmu_index,
 
-#ifndef CONFIG_USER_ONLY
+#ifdef CONFIG_USER_ONLY
+    .is_uninterruptible = superh_cpu_is_uninterruptible,
+    .revert_uninterruptible = superh_cpu_revert_uninterruptible,
+#else
     .tlb_fill = superh_cpu_tlb_fill,
     .pointer_wrap = cpu_pointer_wrap_notreached,
     .cpu_exec_interrupt = superh_cpu_exec_interrupt,
Re: [PATCH] linux-user/sh4: fix race in atomic variables
Posted by Richard Henderson 5 days, 21 hours ago
On 9/20/26 01:46, Mikulas Patocka wrote:
> I'm experiencing random deadlocks when running heavily multithreaded
> workload in qemu-sh4 userspace emulation. This patch fixes them.
> 
> The sh4 architecture doesn't support multiprocessing, the processor
> doesn't have atomic instructions and it uses a technique known as gUSA to
> provide atomicity guarantees w.r.t. signals or thread scheduling (see the
> commit 3b894b699c9a for a brief description of gUSA).
> 
> The function decode_gusa attempts to recognize several well-known gUSA
> regions and turn them into atomic instructions. When it fails to
> recognize a known gUSA pattern, it generates a call to helper_exclusive.
> Qemu will attempt to stop all the other threads and execute a gUSA region
> exclusively, so that it can't race with anything.
> 
> The problem is in the function cpu_exec_step_atomic - this function stops
> all other threads with start_exclusive(), then it executes one
> instruction and then it releases all other threads with end_exclusive().
> This works for all the architectures except sh4. On sh4, executing one
> instruction atomically is not enough - we must execute the full gUSA
> region while the other threads are stopped.
> 
> This patch fixes cpu_exec_step_atomic - it adds two new per-architecture
> functions: is_uninterruptible and revert_uninterruptible.
> 
> is_uninterruptible returns true if we are in a gUSA region and we should
> continue executing code while the other threads are stopped.
> 
> If we got TB_EXIT_REQUESTED, we must stop executing code - in this case,
> we call the function revert_uninterruptible that rolls back PC to the
> beginning of the gUSA region.
> 
> Cc:qemu-stable@nongnu.org
> Signed-off-by: Mikulas Patocka<mpatocka@redhat.com>
> 
> ---
>   accel/tcg/cpu-exec.c        |   15 +++++++++++++++
>   include/accel/tcg/cpu-ops.h |   20 ++++++++++++++++++++
>   target/sh4/cpu.c            |   25 ++++++++++++++++++++++++-
>   3 files changed, 59 insertions(+), 1 deletion(-)

I think this should be integrated into the sh4 translator instead.  We 
should not be single-stepping through the atomic region, but generate 
one TB that implements the entire region.

This may require some coordination with the translator loop.


r~
Re: [PATCH] linux-user/sh4: fix race in atomic variables
Posted by yoshinori.sato@nifty.com 4 days, 6 hours ago
On Mon, 21 Sep 2026 06:31:12 +0900,
Richard Henderson wrote:
> 
> On 9/20/26 01:46, Mikulas Patocka wrote:
> > I'm experiencing random deadlocks when running heavily multithreaded
> > workload in qemu-sh4 userspace emulation. This patch fixes them.
> > 
> > The sh4 architecture doesn't support multiprocessing, the processor
> > doesn't have atomic instructions and it uses a technique known as gUSA to
> > provide atomicity guarantees w.r.t. signals or thread scheduling (see the
> > commit 3b894b699c9a for a brief description of gUSA).
> > 
> > The function decode_gusa attempts to recognize several well-known gUSA
> > regions and turn them into atomic instructions. When it fails to
> > recognize a known gUSA pattern, it generates a call to helper_exclusive.
> > Qemu will attempt to stop all the other threads and execute a gUSA region
> > exclusively, so that it can't race with anything.
> > 
> > The problem is in the function cpu_exec_step_atomic - this function stops
> > all other threads with start_exclusive(), then it executes one
> > instruction and then it releases all other threads with end_exclusive().
> > This works for all the architectures except sh4. On sh4, executing one
> > instruction atomically is not enough - we must execute the full gUSA
> > region while the other threads are stopped.
> > 
> > This patch fixes cpu_exec_step_atomic - it adds two new per-architecture
> > functions: is_uninterruptible and revert_uninterruptible.
> > 
> > is_uninterruptible returns true if we are in a gUSA region and we should
> > continue executing code while the other threads are stopped.
> > 
> > If we got TB_EXIT_REQUESTED, we must stop executing code - in this case,
> > we call the function revert_uninterruptible that rolls back PC to the
> > beginning of the gUSA region.
> > 
> > Cc:qemu-stable@nongnu.org
> > Signed-off-by: Mikulas Patocka<mpatocka@redhat.com>
> > 
> > ---
> >   accel/tcg/cpu-exec.c        |   15 +++++++++++++++
> >   include/accel/tcg/cpu-ops.h |   20 ++++++++++++++++++++
> >   target/sh4/cpu.c            |   25 ++++++++++++++++++++++++-
> >   3 files changed, 59 insertions(+), 1 deletion(-)
> 
> I think this should be integrated into the sh4 translator instead.  We
> should not be single-stepping through the atomic region, but generate
> one TB that implements the entire region.
> 
> This may require some coordination with the translator loop.
> 
> 
> r~

Considering the principles of gUSA, I think it would be difficult to resolve
the issue using only a translator loop.
It seems like avoiding TB_EXIT while in the gUSA state might work,
but I am not sure if that would completely solve the issue.

-- 
Yosinori Sato
[PATCH v2] linux-user/sh4: fix race in atomic variables
Posted by Mikulas Patocka 4 days, 2 hours ago


On Sun, 20 Sep 2026, Richard Henderson wrote:

> On 9/20/26 01:46, Mikulas Patocka wrote:
> > I'm experiencing random deadlocks when running heavily multithreaded
> > workload in qemu-sh4 userspace emulation. This patch fixes them.
> > 
> > The sh4 architecture doesn't support multiprocessing, the processor
> > doesn't have atomic instructions and it uses a technique known as gUSA to
> > provide atomicity guarantees w.r.t. signals or thread scheduling (see the
> > commit 3b894b699c9a for a brief description of gUSA).
> > 
> > The function decode_gusa attempts to recognize several well-known gUSA
> > regions and turn them into atomic instructions. When it fails to
> > recognize a known gUSA pattern, it generates a call to helper_exclusive.
> > Qemu will attempt to stop all the other threads and execute a gUSA region
> > exclusively, so that it can't race with anything.
> > 
> > The problem is in the function cpu_exec_step_atomic - this function stops
> > all other threads with start_exclusive(), then it executes one
> > instruction and then it releases all other threads with end_exclusive().
> > This works for all the architectures except sh4. On sh4, executing one
> > instruction atomically is not enough - we must execute the full gUSA
> > region while the other threads are stopped.
> > 
> > This patch fixes cpu_exec_step_atomic - it adds two new per-architecture
> > functions: is_uninterruptible and revert_uninterruptible.
> > 
> > is_uninterruptible returns true if we are in a gUSA region and we should
> > continue executing code while the other threads are stopped.
> > 
> > If we got TB_EXIT_REQUESTED, we must stop executing code - in this case,
> > we call the function revert_uninterruptible that rolls back PC to the
> > beginning of the gUSA region.
> > 
> > Cc:qemu-stable@nongnu.org
> > Signed-off-by: Mikulas Patocka<mpatocka@redhat.com>
> > 
> > ---
> >   accel/tcg/cpu-exec.c        |   15 +++++++++++++++
> >   include/accel/tcg/cpu-ops.h |   20 ++++++++++++++++++++
> >   target/sh4/cpu.c            |   25 ++++++++++++++++++++++++-
> >   3 files changed, 59 insertions(+), 1 deletion(-)
> 
> I think this should be integrated into the sh4 translator instead.  We should
> not be single-stepping through the atomic region, but generate one TB that
> implements the entire region.
> 
> This may require some coordination with the translator loop.
> 
> 
> r~

Hi

I looked at the sh4 translator and it seems that it already tries to make 
sure that the full gUSA region is translated into one TB - i.e. there is 
"ctx->base.max_insns = max_insns" in sh4_tr_init_disas_context - that will 
override the value "1" that is supplied by cpu_exec_step_atomic. The 
comments suggest that the author is aware of the fact that the gUSA region 
must be completed atomically.

So, the code that single-steps through the gUSA region is not needed.

It seems that the misbehavior is caused by the fact that if we exit from 
the gUSA TB early, we execute the rest of the region in non-exclusive 
context.

I simplified the patch, so that it adds just one method - 
revert_uninterruptible. It tests whether we are in the unfinished gUSA 
region, and if we are, it reverts PC and SP back to the beginning.

Here I'm sending the updated patch.

Mikulas



From: Mikulas Patocka <mpatocka@redhat.com>

I'm experiencing random deadlocks when running heavily multithreaded
workload in qemu-sh4 userspace emulation. This patch fixes them.

The sh4 architecture doesn't support multiprocessing, the processor
doesn't have atomic instructions and it uses a technique known as gUSA to
provide atomicity guarantees w.r.t. signals or thread scheduling (see the
commit 3b894b699c9a for a brief description of gUSA).

The function decode_gusa attempts to recognize several well-known gUSA
regions and turn them into atomic instructions. When it fails to
recognize a known gUSA pattern, it generates a call to helper_exclusive.
Qemu will attempt to stop all the other threads and execute a gUSA region
exclusively, so that it can't race with anything.

The problem is in the function cpu_exec_step_atomic - this function stops
all other threads with start_exclusive(), then it executes one
TB and then it releases all other threads with end_exclusive(). If the TB
executing the gUSA region exited early, the exclusive lock is dropped and
the execution continues without holding it - this is the root cause for
this bug.

This patch fixes cpu_exec_step_atomic - it adds a new per-architecture
function: revert_uninterruptible. Its implementation
superh_cpu_revert_uninterruptible tests if we are in an unfinished gUSA
region and rolls back the PC to the beginning of the region.

Cc: qemu-stable@nongnu.org
Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>

---
 accel/tcg/cpu-exec.c        |    5 +++++
 include/accel/tcg/cpu-ops.h |   10 ++++++++++
 target/sh4/cpu.c            |   25 ++++++++++++++++++++++++-
 3 files changed, 39 insertions(+), 1 deletion(-)

Index: qemu/accel/tcg/cpu-exec.c
===================================================================
--- qemu.orig/accel/tcg/cpu-exec.c	2026-09-22 18:31:48.000000000 +0200
+++ qemu/accel/tcg/cpu-exec.c	2026-09-22 18:31:48.000000000 +0200
@@ -590,6 +590,11 @@ void cpu_exec_step_atomic(CPUState *cpu)
         cpu_exec_longjmp_cleanup(cpu);
     }
 
+#ifdef CONFIG_USER_ONLY
+    if (cpu->cc->tcg_ops->revert_uninterruptible)
+        cpu->cc->tcg_ops->revert_uninterruptible(cpu);
+#endif
+
     /*
      * As we start the exclusive region before codegen we must still
      * be in the region if we longjump out of either the codegen or
Index: qemu/include/accel/tcg/cpu-ops.h
===================================================================
--- qemu.orig/include/accel/tcg/cpu-ops.h	2026-09-22 18:31:48.000000000 +0200
+++ qemu/include/accel/tcg/cpu-ops.h	2026-09-22 18:31:48.000000000 +0200
@@ -168,6 +168,16 @@ struct TCGCPUOps {
      * @addr: tagged guest address
      */
     vaddr (*untagged_addr)(CPUState *cs, vaddr addr);
+
+    /**
+     * revert_uninterruptible:
+     * @cpu: cpu context
+     *
+     * This function is called if cpu_exec_step_atomic needs to exit. It
+     * tests if we are in the gUSA region and rolls back PC to the
+     * beginning of it.
+     */
+    void (*revert_uninterruptible)(CPUState *cs);
 #else
     /** @do_interrupt: Callback for interrupt handling.  */
     void (*do_interrupt)(CPUState *cpu);
Index: qemu/target/sh4/cpu.c
===================================================================
--- qemu.orig/target/sh4/cpu.c	2026-09-22 18:31:48.000000000 +0200
+++ qemu/target/sh4/cpu.c	2026-09-22 18:31:48.000000000 +0200
@@ -92,6 +92,27 @@ static void superh_restore_state_to_opc(
      */
 }
 
+#ifdef CONFIG_USER_ONLY
+static void superh_cpu_revert_uninterruptible(CPUState *cs)
+{
+    SuperHCPU *cpu = SUPERH_CPU(cs);
+    /*
+     * If we are interrupted in the middle of the gUSA region, we must
+     * roll-back PC to the beginning of the region. Continuing halfway
+     * through the region would break atomicity guarantees.
+     *
+     * If we are interrupted after the final write instruction (i.e.
+     * cpu->env.pc == cpu->env.gregs[0]), we must not roll-back, because
+     * the atomic write is already committed in the memory.
+     */
+    if (cpu->env.gregs[15] >= -128u && cpu->env.pc < cpu->env.gregs[0]) {
+        cpu->env.pc = cpu->env.gregs[0] + cpu->env.gregs[15] - 2;
+        cpu->env.gregs[15] = cpu->env.gregs[1];
+        cpu->env.flags &= ~TB_FLAG_ENVFLAGS_MASK;
+    }
+}
+#endif /* CONFIG_USER_ONLY */
+
 #ifndef CONFIG_USER_ONLY
 static bool superh_io_recompile_replay_branch(CPUState *cs,
                                               const TranslationBlock *tb)
@@ -308,7 +329,9 @@ static const TCGCPUOps superh_tcg_ops =
     .restore_state_to_opc = superh_restore_state_to_opc,
     .mmu_index = sh4_cpu_mmu_index,
 
-#ifndef CONFIG_USER_ONLY
+#ifdef CONFIG_USER_ONLY
+    .revert_uninterruptible = superh_cpu_revert_uninterruptible,
+#else
     .tlb_fill = superh_cpu_tlb_fill,
     .pointer_wrap = cpu_pointer_wrap_notreached,
     .cpu_exec_interrupt = superh_cpu_exec_interrupt,
Re: [PATCH v2] linux-user/sh4: fix race in atomic variables
Posted by Richard Henderson 4 days, 2 hours ago
On 9/22/26 10:04, Mikulas Patocka wrote:
> I looked at the sh4 translator and it seems that it already tries to make
> sure that the full gUSA region is translated into one TB - i.e. there is
> "ctx->base.max_insns = max_insns" in sh4_tr_init_disas_context - that will
> override the value "1" that is supplied by cpu_exec_step_atomic. The
> comments suggest that the author is aware of the fact that the gUSA region
> must be completed atomically.
> 
> So, the code that single-steps through the gUSA region is not needed.
> 
> It seems that the misbehavior is caused by the fact that if we exit from
> the gUSA TB early, we execute the rest of the region in non-exclusive
> context.
> 
> I simplified the patch, so that it adds just one method -
> revert_uninterruptible. It tests whether we are in the unfinished gUSA
> region, and if we are, it reverts PC and SP back to the beginning.

What good does that do?  You'll only restart the same block as before.

Is this really the case where you're encountering

>         if (pc != pc_end + backup || max_insns < 2) {
>             /* This is a malformed gUSA region.  Don't do anything special,
>                since the interpreter is likely to get confused.  */
>             ctx->envflags &= ~TB_FLAG_GUSA_MASK;

If you add an abort here, does it trigger?

> +    /*
> +     * If we are interrupted in the middle of the gUSA region, we must
> +     * roll-back PC to the beginning of the region. Continuing halfway
> +     * through the region would break atomicity guarantees.
> +     *
> +     * If we are interrupted after the final write instruction (i.e.
> +     * cpu->env.pc == cpu->env.gregs[0]), we must not roll-back, because
> +     * the atomic write is already committed in the memory.
> +     */
Just know that you *can't* be interrupted in the middle of a gUSA 
region.  Signals will always be delayed until the end of the 
TranslationBlock.

So this text is misleading at best.


r~
Re: [PATCH v2] linux-user/sh4: fix race in atomic variables
Posted by Mikulas Patocka 4 days, 1 hour ago

On Tue, 22 Sep 2026, Richard Henderson wrote:

> On 9/22/26 10:04, Mikulas Patocka wrote:
> > I looked at the sh4 translator and it seems that it already tries to make
> > sure that the full gUSA region is translated into one TB - i.e. there is
> > "ctx->base.max_insns = max_insns" in sh4_tr_init_disas_context - that will
> > override the value "1" that is supplied by cpu_exec_step_atomic. The
> > comments suggest that the author is aware of the fact that the gUSA region
> > must be completed atomically.
> > 
> > So, the code that single-steps through the gUSA region is not needed.
> > 
> > It seems that the misbehavior is caused by the fact that if we exit from
> > the gUSA TB early, we execute the rest of the region in non-exclusive
> > context.
> > 
> > I simplified the patch, so that it adds just one method -
> > revert_uninterruptible. It tests whether we are in the unfinished gUSA
> > region, and if we are, it reverts PC and SP back to the beginning.
> 
> What good does that do?  You'll only restart the same block as before.
> 
> Is this really the case where you're encountering

Without that patch, it deadlocks in 10-30 minutes. With the patch (either 
version 1 or version 2 that I sent), it stays running overnight.

Maybe the bug is somewhere else and the patch just papers over it.

> >         if (pc != pc_end + backup || max_insns < 2) {
> >             /* This is a malformed gUSA region.  Don't do anything special,
> >                since the interpreter is likely to get confused.  */
> >             ctx->envflags &= ~TB_FLAG_GUSA_MASK;
> 
> If you add an abort here, does it trigger?

I added the abort(). It deadlocked in 25 minutes, but didn't trigger it.

> > +    /*
> > +     * If we are interrupted in the middle of the gUSA region, we must
> > +     * roll-back PC to the beginning of the region. Continuing halfway
> > +     * through the region would break atomicity guarantees.
> > +     *
> > +     * If we are interrupted after the final write instruction (i.e.
> > +     * cpu->env.pc == cpu->env.gregs[0]), we must not roll-back, because
> > +     * the atomic write is already committed in the memory.
> > +     */
> Just know that you *can't* be interrupted in the middle of a gUSA region.
> Signals will always be delayed until the end of the TranslationBlock.
> 
> So this text is misleading at best.

Yes, I am not qemu expert.

BTW. what happens with synchronous signals inside the TB? (i.e. SIGSEGV 
due to writing into a write-protected page)

Mikulas
Re: [PATCH v2] linux-user/sh4: fix race in atomic variables
Posted by Richard Henderson 4 days, 1 hour ago
On 9/22/26 10:58, Mikulas Patocka wrote:
> 
> 
> On Tue, 22 Sep 2026, Richard Henderson wrote:
> 
>> On 9/22/26 10:04, Mikulas Patocka wrote:
>>> I looked at the sh4 translator and it seems that it already tries to make
>>> sure that the full gUSA region is translated into one TB - i.e. there is
>>> "ctx->base.max_insns = max_insns" in sh4_tr_init_disas_context - that will
>>> override the value "1" that is supplied by cpu_exec_step_atomic. The
>>> comments suggest that the author is aware of the fact that the gUSA region
>>> must be completed atomically.
>>>
>>> So, the code that single-steps through the gUSA region is not needed.
>>>
>>> It seems that the misbehavior is caused by the fact that if we exit from
>>> the gUSA TB early, we execute the rest of the region in non-exclusive
>>> context.
>>>
>>> I simplified the patch, so that it adds just one method -
>>> revert_uninterruptible. It tests whether we are in the unfinished gUSA
>>> region, and if we are, it reverts PC and SP back to the beginning.
>>
>> What good does that do?  You'll only restart the same block as before.
>>
>> Is this really the case where you're encountering
> 
> Without that patch, it deadlocks in 10-30 minutes. With the patch (either
> version 1 or version 2 that I sent), it stays running overnight.
> 
> Maybe the bug is somewhere else and the patch just papers over it.

Could well be.  Do you have a reliable reproducer?  Or is this a "run 
the container and things eventually fail" sort of thing?

Logging guest state at the point your revert hook fires could be 
informative...

> BTW. what happens with synchronous signals inside the TB? (i.e. SIGSEGV
> due to writing into a write-protected page)
Synchronous signals like that exit the translation block right away, 
leading to the signal being delivered.


r~
Re: [PATCH v2] linux-user/sh4: fix race in atomic variables
Posted by Mikulas Patocka 3 days, 23 hours ago

On Tue, 22 Sep 2026, Richard Henderson wrote:

> On 9/22/26 10:58, Mikulas Patocka wrote:
> > 
> > 
> > On Tue, 22 Sep 2026, Richard Henderson wrote:
> > 
> >> On 9/22/26 10:04, Mikulas Patocka wrote:
> >>> I looked at the sh4 translator and it seems that it already tries to make
> >>> sure that the full gUSA region is translated into one TB - i.e. there is
> >>> "ctx->base.max_insns = max_insns" in sh4_tr_init_disas_context - that will
> >>> override the value "1" that is supplied by cpu_exec_step_atomic. The
> >>> comments suggest that the author is aware of the fact that the gUSA region
> >>> must be completed atomically.
> >>>
> >>> So, the code that single-steps through the gUSA region is not needed.
> >>>
> >>> It seems that the misbehavior is caused by the fact that if we exit from
> >>> the gUSA TB early, we execute the rest of the region in non-exclusive
> >>> context.
> >>>
> >>> I simplified the patch, so that it adds just one method -
> >>> revert_uninterruptible. It tests whether we are in the unfinished gUSA
> >>> region, and if we are, it reverts PC and SP back to the beginning.
> >>
> >> What good does that do?  You'll only restart the same block as before.
> >>
> >> Is this really the case where you're encountering
> > 
> > Without that patch, it deadlocks in 10-30 minutes. With the patch (either
> > version 1 or version 2 that I sent), it stays running overnight.
> > 
> > Maybe the bug is somewhere else and the patch just papers over it.
> 
> Could well be.  Do you have a reliable reproducer?  Or is this a "run the
> container and things eventually fail" sort of thing?

Download the Ajla programming language from
git clone git://repo.or.cz/ajla.git
(there is a mirror at https://www.github.com/mikulas-patocka/ajla.git)
(see the homepage at https://www.ajla-lang.cz/ )

Install the sh4 toolchain: gcc-sh4-linux-gnu (I use Debian Sid)
Install autoconf, automake, ed, make

Compile Ajla:
CC=sh4-linux-gnu-gcc CF="-DDEBUG_ENV" ./rebuild --disable-rwx-mappings

--disable-rwx-mappings is a workaround for another qemu bug - see
https://gitlab.com/qemu-project/qemu/-/work_items/2998

Download https://ajla-lang.cz/downloads/examples/advent-2023/18-2.ajla

Create an input file:
cat >18-small.txt <<EOF
R 6 (#70c710)
D 5 (#0dc571)
L 2 (#5713f0)
D 2 (#d2c081)
R 2 (#59c680)
D 2 (#411b91)
L 5 (#8ceee2)
U 2 (#caa173)
L 1 (#1b58a2)
U 2 (#caa171)
R 2 (#7807d2)
U 3 (#a77fa3)
L 2 (#015232)
U 2 (#7a21e3)
EOF

Run
while time /usr/src/git/qemu/build/qemu-sh4 ./ajla --nosave --tick=100 --thread-tick 18-2.ajla <18-small.txt; do date; done 2>&1 | tee lockup.log

Wait for several tens of minutes - it deadlocks on exit, after printing 
the output "952408144115"

> Logging guest state at the point your revert hook fires could be
> informative...

Yes, I will look into it.

> > BTW. what happens with synchronous signals inside the TB? (i.e. SIGSEGV
> > due to writing into a write-protected page)
>
> Synchronous signals like that exit the translation block right away, leading
> to the signal being delivered.

So, the TB can exit halfway through?

When I was analyzing the bug 2998, I found out that Qemu marks pages with 
executed code as read-only. When the program writes into them, it 
invalidates the translated code and marks the pages as read-write.

How does this interact with TBs exits? What happens if we write into a 
read-only page in the middle of the TB?

Note that Ajla has JIT that generates sh4 code on the fly. But the option 
--disable-rwx-mappings makes it store the generated code in separate pages 
and not on the heap. So, I don't know if it can cause these problems.

Mikulas
Re: [PATCH v2] linux-user/sh4: fix race in atomic variables
Posted by Richard Henderson 3 days, 21 hours ago
On 9/22/26 12:22, Mikulas Patocka wrote:
>>> BTW. what happens with synchronous signals inside the TB? (i.e. SIGSEGV
>>> due to writing into a write-protected page)
>>
>> Synchronous signals like that exit the translation block right away, leading
>> to the signal being delivered.
> 
> So, the TB can exit halfway through?
> 
> When I was analyzing the bug 2998, I found out that Qemu marks pages with
> executed code as read-only. When the program writes into them, it
> invalidates the translated code and marks the pages as read-write.
> 
> How does this interact with TBs exits? What happens if we write into a
> read-only page in the middle of the TB?

If it's just about tracking writes to a page with translations, we won't 
exit the TB.  We'll restart the faulting store by returning from the 
signal handler.  So the gUSA region isn't affected.

"Real" faults to inaccessible pages will reach the signal handler, and 
unwind_gusa() will revert the region before stacking the signal frame.


r~
Re: [PATCH v2] linux-user/sh4: fix race in atomic variables
Posted by Mikulas Patocka 3 days, 23 hours ago

On Tue, 22 Sep 2026, Mikulas Patocka wrote:

> > Logging guest state at the point your revert hook fires could be
> > informative...
> 
> Yes, I will look into it.

So, I tried, and got a hit right away:

superh_cpu_revert_uninterruptible
pc: 45be9e
r0: 45bea4
r1: 55d55e40
r15: fffffffa
flags: 1fa0

  45be94:       10 89           bt      45beb8 <afree+0x84>
  45be96:       03 c7           mova    45bea4 <afree+0x70>,r0
  45be98:       f3 61           mov     r15,r1
  45be9a:       09 00           nop
  45be9c:       fa ef           mov     #-6,r15
  45be9e:       21 57           mov.l   @(4,r2),r7
  45bea0:       3b 27           or      r3,r7
  45bea2:       71 12           mov.l   r7,@(4,r2)
  45bea4:       13 6f           mov     r1,r15
  45bea6:       0b 00           rts

Then, I tried to log only cases where pc points deeper into the gUSA
region and got nothing.

So, if superh_cpu_revert_uninterruptible reverts the PC from 45be9e back
to 45be9c, the deadlock doesn't happen.

Could it be, that on the exit path from cpu_exec_step_atomic, Qemu somehow 
forgets that it is in the middle of the gUSA region and continues 
execution in parallel?

Mikulas
[PATCH v3] linux-user/sh4: fix race in atomic variables
Posted by Mikulas Patocka 2 days, 23 hours ago


On Tue, 22 Sep 2026, Mikulas Patocka wrote:

> 
> 
> On Tue, 22 Sep 2026, Mikulas Patocka wrote:
> 
> > > Logging guest state at the point your revert hook fires could be
> > > informative...
> > 
> > Yes, I will look into it.
> 
> So, I tried, and got a hit right away:
> 
> superh_cpu_revert_uninterruptible
> pc: 45be9e
> r0: 45bea4
> r1: 55d55e40
> r15: fffffffa
> flags: 1fa0
> 
>   45be94:       10 89           bt      45beb8 <afree+0x84>
>   45be96:       03 c7           mova    45bea4 <afree+0x70>,r0
>   45be98:       f3 61           mov     r15,r1
>   45be9a:       09 00           nop
>   45be9c:       fa ef           mov     #-6,r15
>   45be9e:       21 57           mov.l   @(4,r2),r7
>   45bea0:       3b 27           or      r3,r7
>   45bea2:       71 12           mov.l   r7,@(4,r2)
>   45bea4:       13 6f           mov     r1,r15
>   45bea6:       0b 00           rts

So, after putting debugging prints into the code, I think I have found
out what happens there:

* decode_gusa calls gen_restart_exclusive
* gen_restart_exclusive generates code that sets TB_FLAG_GUSA_EXCLUSIVE
  and generates a call to helper_exclusive

* when the code is executed, TB_FLAG_GUSA_EXCLUSIVE is set
* helper_exclusive calls cpu_loop_exit_atomic, this makes cpu_exec exit
  with EXCP_ATOMIC
* we go to cpu_loop, we execute cpu_exec_step_atomic
* suppose that exit request is set, cpu_exec_step_atomic does nothing, it
  leaves the CPU in the same state as it was before
* we go back to cpu_loop
* suppose that no signal is delivered, so the gUSA is not rewound
* cpu_loop goes to cpu_exec
* there is one difference - now, TB_FLAG_GUSA_EXCLUSIVE is set and it was
  clear before - so cpu_exec will not use the TB that calls
  helper_exclusive, it will instead use the TB that performs the atomic
  operation (both of these TBs have the same PC, they only differ in
  flags)
* the TB that performs the atomic operation is executed inside cpu_exec
  => race condition

So, I fixed this by clearing TB_FLAG_GUSA_EXCLUSIVE after
cpu_exec_step_atomic - to make sure that cpu_exec will find the TB that
calls helper_exclusive and not the TB that performs the atomic operation.

I tested it and the deadlock is gone. Does this seem reasonable? Or, do 
you think that we should fix it somewhere else?

Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
Cc: qemu-stable@nongnu.org

---
 linux-user/sh4/cpu_loop.c |    5 +++++
 1 file changed, 5 insertions(+)

Index: qemu/linux-user/sh4/cpu_loop.c
===================================================================
--- qemu.orig/linux-user/sh4/cpu_loop.c	2026-09-23 21:20:08.000000000 +0200
+++ qemu/linux-user/sh4/cpu_loop.c	2026-09-23 21:20:08.000000000 +0200
@@ -62,6 +62,11 @@ void cpu_loop(CPUSH4State *env)
             break;
         case EXCP_ATOMIC:
             cpu_exec_step_atomic(cs);
+            /* If cpu_exec_step_atomic exits due to exit request, we must make
+               sure that cpu_exec will execute the TB that contains a call to
+               helper_atomic and not the TB that contains the atomic operation
+               itself - therefore, we clear TB_FLAG_GUSA_EXCLUSIVE.  */
+            env->flags &= ~TB_FLAG_GUSA_EXCLUSIVE;
             arch_interrupt = false;
             break;
         case 0x180:
Re: [PATCH v3] linux-user/sh4: fix race in atomic variables
Posted by Richard Henderson 2 days, 22 hours ago
On 9/23/26 12:58, Mikulas Patocka wrote:
> So, after putting debugging prints into the code, I think I have found
> out what happens there:
> 
> * decode_gusa calls gen_restart_exclusive
> * gen_restart_exclusive generates code that sets TB_FLAG_GUSA_EXCLUSIVE
>    and generates a call to helper_exclusive
> 
> * when the code is executed, TB_FLAG_GUSA_EXCLUSIVE is set
> * helper_exclusive calls cpu_loop_exit_atomic, this makes cpu_exec exit
>    with EXCP_ATOMIC
> * we go to cpu_loop, we execute cpu_exec_step_atomic
> * suppose that exit request is set, cpu_exec_step_atomic does nothing, it
>    leaves the CPU in the same state as it was before
> * we go back to cpu_loop
> * suppose that no signal is delivered, so the gUSA is not rewound
> * cpu_loop goes to cpu_exec
> * there is one difference - now, TB_FLAG_GUSA_EXCLUSIVE is set and it was
>    clear before - so cpu_exec will not use the TB that calls
>    helper_exclusive, it will instead use the TB that performs the atomic
>    operation (both of these TBs have the same PC, they only differ in
>    flags)
> * the TB that performs the atomic operation is executed inside cpu_exec
>    => race condition
> 
> So, I fixed this by clearing TB_FLAG_GUSA_EXCLUSIVE after
> cpu_exec_step_atomic - to make sure that cpu_exec will find the TB that
> calls helper_exclusive and not the TB that performs the atomic operation.
> 
> I tested it and the deadlock is gone. Does this seem reasonable? Or, do
> you think that we should fix it somewhere else?

Thanks for the excellent analysis.

While I believe your fix is correct, I think the problem might be more 
general.  I think that the condition "running under atomic step" should 
be promoted to a general cflag.  I also think that the exit check could 
be suppressed for the single-step.  That way we go round the loop fewer 
times.

I'll prepare a patch.


r~