| Message ID | 1440548265-4755-7-git-send-email-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Aug 26, 2015 at 02:17:42 +0200, Paolo Bonzini wrote: > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > cpu-exec.c | 6 +++++- > qom/cpu.c | 2 ++ > 2 files changed, 7 insertions(+), 1 deletion(-) I like this patch. Are we making sure that other writes to tcg_exit_req are preceded by a write barrier? For instance: qom/cpu.c-116- cpu->exit_request = 1; qom/cpu.c:117: cpu->tcg_exit_req = 1; translate-all.c-1478- } else { translate-all.c:1479: cpu->tcg_exit_req = 1; translate-all.c-1643- cpu->interrupt_request |= mask; translate-all.c:1644: cpu->tcg_exit_req = 1; Current master certainly doesn't have them, but I wonder if you have barriers at those places in the tree you're working on. If not I'd expand this patch to add them. And while at it, can we pleeease get rid of the hideous 'volatile sigatomic_t' type for tcg_exit_req? Thanks, Emilio
On 28/08/2015 04:19, Emilio G. Cota wrote: > translate-all.c-1478- } else { > translate-all.c:1479: cpu->tcg_exit_req = 1; This one is only run in the CPU thread. > translate-all.c-1643- cpu->interrupt_request |= mask; > translate-all.c:1644: cpu->tcg_exit_req = 1; This one is only run in user-mode emulation, which also means it is run in the CPU thread. Paolo > Current master certainly doesn't have them, but I wonder if > you have barriers at those places in the tree you're working on. If not > I'd expand this patch to add them.
diff --git a/cpu-exec.c b/cpu-exec.c index 7fcc46f..2128bf1 100644 --- a/cpu-exec.c +++ b/cpu-exec.c @@ -542,8 +542,12 @@ int cpu_exec(CPUState *cpu) * loop. Whatever requested the exit will also * have set something else (eg exit_request or * interrupt_request) which we will handle - * next time around the loop. + * next time around the loop. But we need to + * ensure the tcg_exit_req read in generated code + * comes before the next read of cpu->exit_request + * or cpu->interrupt_request. */ + smp_rmb(); next_tb = 0; break; case TB_EXIT_ICOUNT_EXPIRED: diff --git a/qom/cpu.c b/qom/cpu.c index 3e93223..3841f0d 100644 --- a/qom/cpu.c +++ b/qom/cpu.c @@ -114,6 +114,8 @@ void cpu_reset_interrupt(CPUState *cpu, int mask) void cpu_exit(CPUState *cpu) { cpu->exit_request = 1; + /* Ensure cpu_exec will see the exit request after TCG has exited. */ + smp_wmb(); cpu->tcg_exit_req = 1; }
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- cpu-exec.c | 6 +++++- qom/cpu.c | 2 ++ 2 files changed, 7 insertions(+), 1 deletion(-)