From: Matt Turner <[email protected]> The next patch dispatches a goto_jc by probing the TB jump cache from generated code. That probe cannot check everything helper_lookup_tb_ptr() checks, and the one that matters is breakpoints: check_for_breakpoints() raises EXCP_DEBUG on an exact pc match and selects CF_BP_PAGE cflags for the rest of the page, and inserting a breakpoint deliberately invalidates no TB.
The probe compares the destination's cflags against the cflags of the block doing the dispatching, and only takes the destination when they are equal. So a cflag is all that is needed. Add CF_NO_GOTO_JC, set it in CPUState::tcg_cflags while cpu->breakpoints is non-empty, and blocks translated from then on both decline to dispatch inline themselves -- the next patch makes them emit the plain helper call -- and are unreachable from blocks that do, because their cflags no longer match. The two ends of the flag are cpu_breakpoint_insert() and cpu_breakpoint_remove_by_ref(), which are the only places the list changes. Both already run either on the CPU's own thread or with it stopped, or reach another CPU exactly as cpu_single_step() does, which is where the previous patch put the same kind of update. That leaves blocks translated before the breakpoint was inserted, which are still live and still chain to each other. They do so on the old cflags, so inline dispatch among them keeps working until the vCPU reaches its main loop, which then looks up with the new cflags and translates afresh. In system mode gdb inserts breakpoints with the vCPUs stopped, so there is no window at all. In user mode the window is the one goto_tb chaining already has: a chained direct jump consults nothing either, and is not broken by inserting a breakpoint. Nothing reads CF_NO_GOTO_JC yet; the next patch does. Signed-off-by: Matt Turner <[email protected]> Signed-off-by: Richard Henderson <[email protected]> Message-ID: <[email protected]> --- include/exec/translation-block.h | 5 +++-- accel/tcg/cpu-exec-common.c | 16 ++++++++++++++-- cpu-common.c | 25 ++++++++++++++++++++++++- 3 files changed, 41 insertions(+), 5 deletions(-) diff --git a/include/exec/translation-block.h b/include/exec/translation-block.h index 40cc6990318..f3410dbcb93 100644 --- a/include/exec/translation-block.h +++ b/include/exec/translation-block.h @@ -76,14 +76,15 @@ struct TranslationBlock { #define CF_COUNT_MASK 0x000001ff #define CF_NO_GOTO_TB 0x00000200 /* Do not chain with goto_tb */ #define CF_NO_GOTO_PTR 0x00000400 /* Do not chain with goto_ptr */ -#define CF_SINGLE_STEP 0x00000800 /* gdbstub single-step in effect */ -#define CF_MEMI_ONLY 0x00001000 /* Only instrument memory ops */ +#define CF_NO_GOTO_JC 0x00000800 /* Do not dispatch via the inline probe */ +#define CF_SINGLE_STEP 0x00001000 /* gdbstub single-step in effect */ #define CF_USE_ICOUNT 0x00002000 #define CF_INVALID 0x00004000 /* TB is stale. Set with @jmp_lock held */ #define CF_PARALLEL 0x00008000 /* Generate code for a parallel context */ #define CF_NOIRQ 0x00010000 /* Generate an uninterruptible TB */ #define CF_PCREL 0x00020000 /* Opcodes in TB are PC-relative */ #define CF_BP_PAGE 0x00040000 /* Breakpoint present in code page */ +#define CF_MEMI_ONLY 0x00080000 /* Only instrument memory ops */ #define CF_CLUSTER_MASK 0xff000000 /* Top 8 bits are cluster ID */ #define CF_CLUSTER_SHIFT 24 diff --git a/accel/tcg/cpu-exec-common.c b/accel/tcg/cpu-exec-common.c index 9f3517f36b6..9f9555e34f6 100644 --- a/accel/tcg/cpu-exec-common.c +++ b/accel/tcg/cpu-exec-common.c @@ -38,10 +38,11 @@ void tcg_cflags_set(CPUState *cpu, uint32_t flags) /* * The bits of CPUState::tcg_cflags that tcg_cflags_set() never sets, because - * they are derived from gdb single-step, one-insn-per-tb and -d nochain. + * they are derived from gdb single-step, one-insn-per-tb and -d nochain, + * or from various cpu parameters. */ #define CF_DERIVED (CF_COUNT_MASK | CF_NO_GOTO_TB | CF_NO_GOTO_PTR | \ - CF_SINGLE_STEP) + CF_NO_GOTO_JC | CF_SINGLE_STEP) void tcg_update_cflags(CPUState *cpu) { @@ -62,6 +63,17 @@ void tcg_update_cflags(CPUState *cpu) cflags |= CF_NO_GOTO_TB; } + /* + * A block that dispatches through the jump cache inline does not consult + * cpu->breakpoints, and inserting a breakpoint deliberately invalidates + * nothing. Give blocks translated while one is set a distinct cflags, so + * that they neither dispatch inline themselves nor are reached by a block + * that does, and check_for_breakpoints() gets to run on every dispatch. + */ + if (unlikely(!QTAILQ_EMPTY(&cpu->breakpoints))) { + cflags |= CF_NO_GOTO_JC; + } + cpu->tcg_cflags = cflags; } diff --git a/cpu-common.c b/cpu-common.c index adb76b3a786..909f8d059b4 100644 --- a/cpu-common.c +++ b/cpu-common.c @@ -22,6 +22,7 @@ #include "exec/cpu-common.h" #include "hw/core/cpu.h" #include "qemu/lockable.h" +#include "system/tcg.h" #include "trace/trace-root.h" QemuMutex qemu_cpu_list_lock; @@ -429,6 +430,11 @@ int cpu_breakpoint_insert(CPUState *cpu, vaddr pc, int flags, *breakpoint = bp; } + /* The first breakpoint takes the CPU off the inline dispatch path. */ + if (tcg_enabled()) { + tcg_update_cflags(cpu); + } + trace_breakpoint_insert(cpu->cpu_index, pc, flags); return 0; } @@ -452,7 +458,7 @@ int cpu_breakpoint_remove(CPUState *cpu, vaddr pc, int flags) } /* Remove a specific breakpoint by reference. */ -void cpu_breakpoint_remove_by_ref(CPUState *cpu, CPUBreakpoint *bp) +static void cpu_breakpoint_remove_by_ref_int(CPUState *cpu, CPUBreakpoint *bp) { QTAILQ_REMOVE(&cpu->breakpoints, bp, entry); @@ -460,14 +466,31 @@ void cpu_breakpoint_remove_by_ref(CPUState *cpu, CPUBreakpoint *bp) g_free(bp); } +void cpu_breakpoint_remove_by_ref(CPUState *cpu, CPUBreakpoint *bp) +{ + cpu_breakpoint_remove_by_ref_int(cpu, bp); + + /* The last breakpoint puts the CPU back on the inline dispatch path. */ + if (tcg_enabled()) { + tcg_update_cflags(cpu); + } +} + /* Remove all matching breakpoints. */ void cpu_breakpoint_remove_all(CPUState *cpu, int mask) { CPUBreakpoint *bp, *next; + bool removed = false; QTAILQ_FOREACH_SAFE(bp, &cpu->breakpoints, entry, next) { if (bp->flags & mask) { cpu_breakpoint_remove_by_ref(cpu, bp); + removed = true; } } + + /* The last breakpoint puts the CPU back on the inline dispatch path. */ + if (tcg_enabled() && removed) { + tcg_update_cflags(cpu); + } } -- 2.53.0
