[Qemu-devel] [PATCH 0/6] Drop the irredeemably racy cpu_unlink_tb()

[Peter Maydell]

This patch series gets rid of cpu_unlink_tb(), which is irredeemably
racy, since it modifies the TB graph with no locking from other
threads, signal handlers, etc etc. (The signal handler case is
why you can't just fix this with more locks.) Instead we take the
much simpler approach of setting a flag for the CPU when we want
it to stop executing TBs, and generate code to check the flag at
the start of every TB. The raciness is easiest to provoke with
multithreaded linux-user guests but it is I think also a risk
in system emulation mode.

This fixes the crashes seen in LP:668799; however there are another
class of crashes described in LP:1098729 which stem from the fact
that in linux-user with a multithreaded guest all threads will
use and modify the same global TCG date structures (including the
generated code buffer) without any kind of locking. This means that
multithreaded guest binaries are still in the "unsupported" category.

Patch 1 has been on the list before, but I improved the comment
a bit [no code changes from the previously posted version] which
is why I haven't given it rth's reviewed-by tag.

The ENV_OFFSET macros patch is one from an old patch series
of Andreas'; we need some way in generic code to find the offset
of a CPUState field from the CPUArchState pointer. (The commit
message is mine, though.)

I've tested this in various ways, and also ran a popular embedded
benchmark. This is the worst case for this change, since it is
compute intensive code in a situation that shouldn't have too
many interrupts; the slowdown varied from benchmark to benchmark
but was generally something like 3-5%. I think that's an acceptably
small hit to gain actual correctness and non-crashiness :-)


Andreas Färber (1):
  cpu: Introduce ENV_OFFSET macros

Peter Maydell (5):
  tcg: Document tcg_qemu_tb_exec() and provide constants for low bit uses
  cpu-exec: wrap tcg_qemu_tb_exec() in a fn to restore the PC
  Handle CPU interrupts by inline checking of a flag
  translate-all.c: Remove cpu_unlink_tb()
  gen-icount.h: Rename gen_icount_start/end to gen_tb_start/end

 cpu-exec.c                    |   59 +++++++++++++++++++++++++--------
 exec.c                        |    2 +-
 include/exec/gen-icount.h     |   18 ++++++++--
 include/qom/cpu.h             |    3 ++
 target-alpha/cpu-qom.h        |    1 +
 target-alpha/translate.c      |    4 +--
 target-arm/cpu-qom.h          |    2 ++
 target-arm/translate.c        |    4 +--
 target-cris/cpu-qom.h         |    1 +
 target-cris/translate.c       |    4 +--
 target-i386/cpu-qom.h         |    1 +
 target-i386/translate.c       |    4 +--
 target-lm32/cpu-qom.h         |    1 +
 target-lm32/translate.c       |    4 +--
 target-m68k/cpu-qom.h         |    1 +
 target-m68k/translate.c       |    4 +--
 target-microblaze/cpu-qom.h   |    1 +
 target-microblaze/translate.c |    4 +--
 target-mips/cpu-qom.h         |    1 +
 target-mips/translate.c       |    4 +--
 target-openrisc/cpu.h         |    2 ++
 target-openrisc/translate.c   |    4 +--
 target-ppc/cpu-qom.h          |    3 +-
 target-ppc/translate.c        |    4 +--
 target-s390x/cpu-qom.h        |    1 +
 target-s390x/translate.c      |    4 +--
 target-sh4/cpu-qom.h          |    1 +
 target-sh4/translate.c        |    4 +--
 target-sparc/cpu-qom.h        |    1 +
 target-sparc/translate.c      |    4 +--
 target-unicore32/cpu-qom.h    |    1 +
 target-unicore32/translate.c  |    4 +--
 target-xtensa/cpu-qom.h       |    1 +
 target-xtensa/translate.c     |    4 +--
 tcg/tcg.h                     |   49 ++++++++++++++++++++++++++-
 translate-all.c               |   73 ++---------------------------------------
 36 files changed, 162 insertions(+), 121 deletions(-)

-- 
1.7.9.5