From: KONRAD Frederic <fred.kon...@greensocs.com>
This protects TBContext with tb_lock to make tb_* thread safe.
We can still have issue with tb_flush in case of multithread TCG:
another CPU can be executing code during a flush.
This can be fixed later by making all other TCG thread exiting before calling
tb_flush().
Signed-off-by: KONRAD Frederic <fred.kon...@greensocs.com>
Message-Id: <1439220437-23957-8-git-send-email-fred.kon...@greensocs.com>
Signed-off-by: Emilio G. Cota <c...@braap.org>
Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
[AJB: moved into tree, clean-up history]
Signed-off-by: Alex Bennée <alex.ben...@linaro.org>
---
v3 (base-patches, ajb):
- more explicit comments on resetting tb_lock
- more explicit comments about thread safety of user-mode tb_flush
v2 (base-patches, ajb):
- re-base fixes
v7 (FK, MTTCG):
- Drop a tb_lock in already locked restore_state_to_opc.
v6 (FK, MTTCG):
- Drop a tb_lock arround tb_find_fast in cpu-exec.c.
---
cpu-exec.c | 6 ++++++
exec.c | 6 ++++++
hw/i386/kvmvapic.c | 4 ++++
translate-all.c | 35 +++++++++++++++++++++++++++++------
4 files changed, 45 insertions(+), 6 deletions(-)
diff --git a/cpu-exec.c b/cpu-exec.c
index b840e1d..ae81e8e 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -212,16 +212,22 @@ static void cpu_exec_nocache(CPUState *cpu, int
max_cycles,
old_tb_flushed = cpu->tb_flushed;
cpu->tb_flushed = false;
+ tb_lock();
tb = tb_gen_code(cpu, orig_tb->pc, orig_tb->cs_base, orig_tb->flags,
max_cycles | CF_NOCACHE
| (ignore_icount ? CF_IGNORE_ICOUNT : 0));
tb->orig_tb = cpu->tb_flushed ? NULL : orig_tb;
cpu->tb_flushed |= old_tb_flushed;
+
+ tb_unlock();
/* execute the generated code */
trace_exec_tb_nocache(tb, tb->pc);
cpu_tb_exec(cpu, tb);
+ tb_lock();
+
tb_phys_invalidate(tb, -1);
tb_free(tb);
+ tb_unlock();
}
#endif
diff --git a/exec.c b/exec.c
index b225282..e23039c 100644
--- a/exec.c
+++ b/exec.c
@@ -2148,6 +2148,12 @@ static void check_watchpoint(int offset, int len,
MemTxAttrs attrs, int flags)
continue;
}
cpu->watchpoint_hit = wp;
+
+ /* The tb_lock will be reset when cpu_loop_exit or
+ * cpu_resume_from_signal longjmp back into the cpu_exec
+ * main loop.
+ */
+ tb_lock();
tb_check_watchpoint(cpu);
if (wp->flags & BP_STOP_BEFORE_ACCESS) {
cpu->exception_index = EXCP_DEBUG;
diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c
index 5b71b1b..d98fe2a 100644
--- a/hw/i386/kvmvapic.c
+++ b/hw/i386/kvmvapic.c
@@ -17,6 +17,7 @@
#include "sysemu/kvm.h"
#include "hw/i386/apic_internal.h"
#include "hw/sysbus.h"
+#include "tcg/tcg.h"
#define VAPIC_IO_PORT 0x7e
@@ -449,6 +450,9 @@ static void patch_instruction(VAPICROMState *s, X86CPU
*cpu, target_ulong ip)
resume_all_vcpus();
if (!kvm_enabled()) {
+ /* tb_lock will be reset when cpu_resume_from_signal longjmps
+ * back into the cpu_exec loop. */
+ tb_lock();
tb_gen_code(cs, current_pc, current_cs_base, current_flags, 1);
cpu_resume_from_signal(cs, NULL);
}
diff --git a/translate-all.c b/translate-all.c
index aba6cb6..f54ab3e 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -891,8 +891,13 @@ static void page_flush_tb(void)
}
}
-/* flush all the translation blocks */
-/* XXX: tb_flush is currently not thread safe */
+/* Flush all the translation blocks:
+ *
+ * System emulation calls it only with tb_lock taken or from
+ * safe_work, so no need to take tb_lock here.
+ *
+ * User-mode tb_flush is currently not thread safe (FIXME).
+ */
void tb_flush(CPUState *cpu)
{
#if defined(DEBUG_TB_FLUSH)
@@ -1407,6 +1412,7 @@ void tb_invalidate_phys_page_range(tb_page_addr_t start,
tb_page_addr_t end,
/* we remove all the TBs in the range [start, end[ */
/* XXX: see if in some cases it could be faster to invalidate all
the code */
+ tb_lock();
tb = p->first_tb;
while (tb != NULL) {
n = (uintptr_t)tb & 3;
@@ -1466,6 +1472,7 @@ void tb_invalidate_phys_page_range(tb_page_addr_t start,
tb_page_addr_t end,
cpu_resume_from_signal(cpu, NULL);
}
#endif
+ tb_unlock();
}
#ifdef CONFIG_SOFTMMU
@@ -1532,6 +1539,8 @@ static void tb_invalidate_phys_page(tb_page_addr_t addr,
if (!p) {
return;
}
+
+ tb_lock();
tb = p->first_tb;
#ifdef TARGET_HAS_PRECISE_SMC
if (tb && pc != 0) {
@@ -1572,9 +1581,13 @@ static void tb_invalidate_phys_page(tb_page_addr_t addr,
if (locked) {
mmap_unlock();
}
+
+ /* tb_lock will be reset after cpu_resume_from_signal longjmps
+ * back into the cpu_exec loop. */
cpu_resume_from_signal(cpu, puc);
}
#endif
+ tb_unlock();
}
#endif
@@ -1668,6 +1681,7 @@ void cpu_io_recompile(CPUState *cpu, uintptr_t retaddr)
target_ulong pc, cs_base;
uint32_t flags;
+ tb_lock();
tb = tb_find_pc(retaddr);
if (!tb) {
cpu_abort(cpu, "cpu_io_recompile: could not find TB for pc=%p",
@@ -1719,11 +1733,16 @@ void cpu_io_recompile(CPUState *cpu, uintptr_t retaddr)
/* FIXME: In theory this could raise an exception. In practice
we have already translated the block once so it's probably ok. */
tb_gen_code(cpu, pc, cs_base, flags, cflags);
+
/* TODO: If env->pc != tb->pc (i.e. the faulting instruction was not
- the first in the TB) then we end up generating a whole new TB and
- repeating the fault, which is horribly inefficient.
- Better would be to execute just this insn uncached, or generate a
- second new TB. */
+ * the first in the TB) then we end up generating a whole new TB and
+ * repeating the fault, which is horribly inefficient.
+ * Better would be to execute just this insn uncached, or generate a
+ * second new TB.
+ *
+ * cpu_resume_from_signal will longjmp back to cpu_exec where the
+ * tb_lock gets reset.
+ */
cpu_resume_from_signal(cpu, NULL);
}
@@ -1752,6 +1771,8 @@ void dump_exec_info(FILE *f, fprintf_function cpu_fprintf)
size_t hgram_bins;
char *hgram;
+ tb_lock();
+
target_code_size = 0;
max_target_code_size = 0;
cross_page = 0;
@@ -1839,6 +1860,8 @@ void dump_exec_info(FILE *f, fprintf_function cpu_fprintf)
tcg_ctx.tb_ctx.tb_phys_invalidate_count);
cpu_fprintf(f, "TLB flush count %d\n", tlb_flush_count);
tcg_dump_info(f, cpu_fprintf);
+
+ tb_unlock();
}
void dump_opcount_info(FILE *f, fprintf_function cpu_fprintf)
--
2.7.4
