aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPeter Maydell <peter.maydell@linaro.org>2016-12-06 18:07:09 +0000
committerStefan Hajnoczi <stefanha@redhat.com>2016-12-06 20:21:46 +0000
commita9353fe897ca2687e5b3385ed39e3db3927a90e0 (patch)
treecdcbdc899172840177ee1330e69d5a101c07eb6d
parentd750c3a966d6409356739e97fa4d4a6446754e1e (diff)
exec.c: Fix breakpoint invalidation race
A bug (1647683) was reported showing a crash when removing breakpoints. The reproducer was bisected to 3359baad when tb_flush was finally made thread safe. While in MTTCG the locking in breakpoint_invalidate would have prevented any problems, but currently tb_lock() is a NOP for system emulation. The race is between a tb_flush from the gdbstub and the tb_invalidate_phys_addr() in breakpoint_invalidate(). Ideally we'd have actual locking here; for the moment the simple fix is to do a full tb_flush() for a bp invalidate, since that is thread-safe even if no lock is taken. Reported-by: Julian Brown <julian@codesourcery.com> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 1481047629-7763-1-git-send-email-peter.maydell@linaro.org Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
-rw-r--r--exec.c25
1 files changed, 6 insertions, 19 deletions
diff --git a/exec.c b/exec.c
index 3d867f166c..08c558eecf 100644
--- a/exec.c
+++ b/exec.c
@@ -684,28 +684,15 @@ void cpu_exec_realizefn(CPUState *cpu, Error **errp)
#endif
}
-#if defined(CONFIG_USER_ONLY)
-static void breakpoint_invalidate(CPUState *cpu, target_ulong pc)
-{
- mmap_lock();
- tb_lock();
- tb_invalidate_phys_page_range(pc, pc + 1, 0);
- tb_unlock();
- mmap_unlock();
-}
-#else
static void breakpoint_invalidate(CPUState *cpu, target_ulong pc)
{
- MemTxAttrs attrs;
- hwaddr phys = cpu_get_phys_page_attrs_debug(cpu, pc, &attrs);
- int asidx = cpu_asidx_from_attrs(cpu, attrs);
- if (phys != -1) {
- /* Locks grabbed by tb_invalidate_phys_addr */
- tb_invalidate_phys_addr(cpu->cpu_ases[asidx].as,
- phys | (pc & ~TARGET_PAGE_MASK));
- }
+ /* Flush the whole TB as this will not have race conditions
+ * even if we don't have proper locking yet.
+ * Ideally we would just invalidate the TBs for the
+ * specified PC.
+ */
+ tb_flush(cpu);
}
-#endif
#if defined(CONFIG_USER_ONLY)
void cpu_watchpoint_remove_all(CPUState *cpu, int mask)