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

Mikulas Patocka posted 1 patch 2 days, 22 hours ago
There is a newer version of this series
linux-user/sh4/cpu_loop.c |    5 +++++
1 file changed, 5 insertions(+)
[PATCH v3] linux-user/sh4: fix race in atomic variables
Posted by Mikulas Patocka 2 days, 22 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~