[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH] exec.c: simplify the breakpoint invalidation logic
From: |
Alex Bennée |
Subject: |
[Qemu-devel] [PATCH] exec.c: simplify the breakpoint invalidation logic |
Date: |
Tue, 6 Dec 2016 14:51:04 +0000 |
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 should have prevented any problems currently
tb_lock() is a NOP for system emulation.
On top of it all the invalidation code was special-cased between user
and system emulation which was ugly. As we now have a safe tb_flush()
we might as well use that when breakpoints and added and removed. This
is the same thing we do for single-stepping and as this is during
debugging it isn't a major performance concern.
Reported-by: Julian Brown
Signed-off-by: Alex Bennée <address@hidden>
---
exec.c | 31 ++++---------------------------
1 file changed, 4 insertions(+), 27 deletions(-)
diff --git a/exec.c b/exec.c
index 3d867f1..e991221 100644
--- a/exec.c
+++ b/exec.c
@@ -685,29 +685,6 @@ void cpu_exec_realizefn(CPUState *cpu, Error **errp)
}
#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));
- }
-}
-#endif
-
-#if defined(CONFIG_USER_ONLY)
void cpu_watchpoint_remove_all(CPUState *cpu, int mask)
{
@@ -839,7 +816,7 @@ int cpu_breakpoint_insert(CPUState *cpu, vaddr pc, int
flags,
QTAILQ_INSERT_TAIL(&cpu->breakpoints, bp, entry);
}
- breakpoint_invalidate(cpu, pc);
+ tb_flush(cpu);
if (breakpoint) {
*breakpoint = bp;
@@ -855,6 +832,7 @@ int cpu_breakpoint_remove(CPUState *cpu, vaddr pc, int
flags)
QTAILQ_FOREACH(bp, &cpu->breakpoints, entry) {
if (bp->pc == pc && bp->flags == flags) {
cpu_breakpoint_remove_by_ref(cpu, bp);
+ tb_flush(cpu);
return 0;
}
}
@@ -865,9 +843,6 @@ int cpu_breakpoint_remove(CPUState *cpu, vaddr pc, int
flags)
void cpu_breakpoint_remove_by_ref(CPUState *cpu, CPUBreakpoint *breakpoint)
{
QTAILQ_REMOVE(&cpu->breakpoints, breakpoint, entry);
-
- breakpoint_invalidate(cpu, breakpoint->pc);
-
g_free(breakpoint);
}
@@ -881,6 +856,8 @@ void cpu_breakpoint_remove_all(CPUState *cpu, int mask)
cpu_breakpoint_remove_by_ref(cpu, bp);
}
}
+
+ tb_flush(cpu);
}
/* enable or disable single step mode. EXCP_DEBUG is returned by the
--
2.10.2