[PATCH] cpu_exec_step_atomic: update the cpu running flag

[PATCH] cpu_exec_step_atomic: update the cpu running flag

[Douglas Crosher]

The cpu_exec_step_atomic() function is called with the cpu->running
clear and proceeds to run target code without setting this flag. If
this target code generates an exception then handle_cpu_signal() will
unnecessarily abort.

For example if atomic code generates a memory protection fault.

This patch at least sets and clears this running flag.

The related code paths look rather convoluted and it is not immediately 
clear that this patch comprehensively addresses the issue, but it might 
at least direct people to a problem, and it might be an incremental 
improvement, and it gets some code running here. The patch adds some 
assertions to help detect other cases.

Signed-off-by: Douglas Crosher <dtc-ubuntu@scieneer.com>
---
  accel/tcg/cpu-exec.c | 4 ++++
  1 file changed, 4 insertions(+)

diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 66d38f9d85..c1cf1a01cb 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -241,6 +241,9 @@ void cpu_exec_step_atomic(CPUState *cpu)

      if (sigsetjmp(cpu->jmp_env, 0) == 0) {
          start_exclusive();
+        g_assert(cpu == current_cpu);
+        g_assert(!cpu->running);
+        cpu->running = true;

          tb = tb_lookup__cpu_state(cpu, &pc, &cs_base, &flags, cf_mask);
          if (tb == NULL) {
@@ -279,6 +282,7 @@ void cpu_exec_step_atomic(CPUState *cpu)
       */
      g_assert(cpu_in_exclusive_context(cpu));
      parallel_cpus = true;
+    cpu->running = false;
      end_exclusive();
  }

-- 
2.25.4

Re: [PATCH] cpu_exec_step_atomic: update the cpu running flag

[Richard Henderson]

On 9/21/20 9:42 PM, Douglas Crosher wrote:
> 
> The cpu_exec_step_atomic() function is called with the cpu->running
> clear and proceeds to run target code without setting this flag. If
> this target code generates an exception then handle_cpu_signal() will
> unnecessarily abort.
> 
> For example if atomic code generates a memory protection fault.
> 
> This patch at least sets and clears this running flag.
> 
> The related code paths look rather convoluted and it is not immediately clear
> that this patch comprehensively addresses the issue, but it might at least
> direct people to a problem, and it might be an incremental improvement, and it
> gets some code running here. The patch adds some assertions to help detect
> other cases.
> 
> Signed-off-by: Douglas Crosher <dtc-ubuntu@scieneer.com>

Sorry this got overlooked, but better late than never.
Yes, this looks right, thanks.

Queued to tcg-next.


r~