| Message ID | 20210719212239.428740-16-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | tcg: breakpoint reorg | expand |
On 7/19/21 11:22 AM, Richard Henderson wrote: > Set CF_SINGLE_STEP when single-stepping is enabled. > This avoids the need to flush all tb's when turning > single-stepping on or off. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > include/exec/exec-all.h | 1 + > accel/tcg/cpu-exec.c | 12 ++++++++++++ > accel/tcg/translator.c | 7 +------ > cpu.c | 4 ---- > 4 files changed, 14 insertions(+), 10 deletions(-) Bah, this should also contain diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c index bf82c15aab..bbfcfb698c 100644 --- a/accel/tcg/translate-all.c +++ b/accel/tcg/translate-all.c @@ -1432,10 +1432,6 @@ TranslationBlock *tb_gen_code(CPUState *cpu, } QEMU_BUILD_BUG_ON(CF_COUNT_MASK + 1 != TCG_MAX_INSNS); - if (cpu->singlestep_enabled) { - max_insns = 1; - } - buffer_overflow: tb = tcg_tb_alloc(tcg_ctx); if (unlikely(!tb)) { > > diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h > index 6873cce8df..5d1b6d80fb 100644 > --- a/include/exec/exec-all.h > +++ b/include/exec/exec-all.h > @@ -497,6 +497,7 @@ 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_LAST_IO 0x00008000 /* Last insn may be an IO access. */ > #define CF_MEMI_ONLY 0x00010000 /* Only instrument memory ops */ > #define CF_USE_ICOUNT 0x00020000 > diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c > index 6710e15d8b..30a3be9ea7 100644 > --- a/accel/tcg/cpu-exec.c > +++ b/accel/tcg/cpu-exec.c > @@ -272,6 +272,14 @@ static uint32_t cflags_for_breakpoints(CPUState *cpu, target_ulong pc, > } > } > > + if (unlikely(cpu->singlestep_enabled)) { > + /* > + * Record gdb single-step. We should be exiting the TB by raising > + * EXCP_DEBUG, but to simplify other tests, disable chaining too. > + */ > + bflags = CF_NO_GOTO_TB | CF_NO_GOTO_PTR | CF_SINGLE_STEP | 1; > + } > + > if (unlikely(bflags)) { > cflags = (cflags & ~CF_COUNT_MASK) | bflags; > } > @@ -409,7 +417,11 @@ void cpu_exec_step_atomic(CPUState *cpu) > * We only arrive in cpu_exec_step_atomic after beginning execution > * of an insn that includes an atomic operation we can't handle. > * Any breakpoint for this insn will have been recognized earlier. > + * But do record single-stepping. > */ > + if (unlikely(cpu->singlestep_enabled)) { > + cflags |= CF_SINGLE_STEP; > + } > > tb = tb_lookup(cpu, pc, cs_base, flags, cflags); > if (tb == NULL) { > diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c > index b45337f3ba..c53a7f8e44 100644 > --- a/accel/tcg/translator.c > +++ b/accel/tcg/translator.c > @@ -38,11 +38,6 @@ bool translator_use_goto_tb(DisasContextBase *db, target_ulong dest) > return false; > } > > - /* Suppress goto_tb in the case of single-steping. */ > - if (db->singlestep_enabled) { > - return false; > - } > - > /* Check for the dest on the same page as the start of the TB. */ > return ((db->pc_first ^ dest) & TARGET_PAGE_MASK) == 0; > } > @@ -60,7 +55,7 @@ void translator_loop(const TranslatorOps *ops, DisasContextBase *db, > db->is_jmp = DISAS_NEXT; > db->num_insns = 0; > db->max_insns = max_insns; > - db->singlestep_enabled = cpu->singlestep_enabled; > + db->singlestep_enabled = cflags & CF_SINGLE_STEP; > > ops->init_disas_context(db, cpu); > tcg_debug_assert(db->is_jmp == DISAS_NEXT); /* no early exit */ > diff --git a/cpu.c b/cpu.c > index 660b56f431..addcb5db9c 100644 > --- a/cpu.c > +++ b/cpu.c > @@ -316,10 +316,6 @@ void cpu_single_step(CPUState *cpu, int enabled) > cpu->singlestep_enabled = enabled; > if (kvm_enabled()) { > kvm_update_guest_debug(cpu, 0); > - } else { > - /* must flush all the translated code to avoid inconsistencies */ > - /* XXX: only flush what is necessary */ > - tb_flush(cpu); > } > trace_breakpoint_singlestep(cpu->cpu_index, enabled); > } >
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h index 6873cce8df..5d1b6d80fb 100644 --- a/include/exec/exec-all.h +++ b/include/exec/exec-all.h @@ -497,6 +497,7 @@ 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_LAST_IO 0x00008000 /* Last insn may be an IO access. */ #define CF_MEMI_ONLY 0x00010000 /* Only instrument memory ops */ #define CF_USE_ICOUNT 0x00020000 diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c index 6710e15d8b..30a3be9ea7 100644 --- a/accel/tcg/cpu-exec.c +++ b/accel/tcg/cpu-exec.c @@ -272,6 +272,14 @@ static uint32_t cflags_for_breakpoints(CPUState *cpu, target_ulong pc, } } + if (unlikely(cpu->singlestep_enabled)) { + /* + * Record gdb single-step. We should be exiting the TB by raising + * EXCP_DEBUG, but to simplify other tests, disable chaining too. + */ + bflags = CF_NO_GOTO_TB | CF_NO_GOTO_PTR | CF_SINGLE_STEP | 1; + } + if (unlikely(bflags)) { cflags = (cflags & ~CF_COUNT_MASK) | bflags; } @@ -409,7 +417,11 @@ void cpu_exec_step_atomic(CPUState *cpu) * We only arrive in cpu_exec_step_atomic after beginning execution * of an insn that includes an atomic operation we can't handle. * Any breakpoint for this insn will have been recognized earlier. + * But do record single-stepping. */ + if (unlikely(cpu->singlestep_enabled)) { + cflags |= CF_SINGLE_STEP; + } tb = tb_lookup(cpu, pc, cs_base, flags, cflags); if (tb == NULL) { diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c index b45337f3ba..c53a7f8e44 100644 --- a/accel/tcg/translator.c +++ b/accel/tcg/translator.c @@ -38,11 +38,6 @@ bool translator_use_goto_tb(DisasContextBase *db, target_ulong dest) return false; } - /* Suppress goto_tb in the case of single-steping. */ - if (db->singlestep_enabled) { - return false; - } - /* Check for the dest on the same page as the start of the TB. */ return ((db->pc_first ^ dest) & TARGET_PAGE_MASK) == 0; } @@ -60,7 +55,7 @@ void translator_loop(const TranslatorOps *ops, DisasContextBase *db, db->is_jmp = DISAS_NEXT; db->num_insns = 0; db->max_insns = max_insns; - db->singlestep_enabled = cpu->singlestep_enabled; + db->singlestep_enabled = cflags & CF_SINGLE_STEP; ops->init_disas_context(db, cpu); tcg_debug_assert(db->is_jmp == DISAS_NEXT); /* no early exit */ diff --git a/cpu.c b/cpu.c index 660b56f431..addcb5db9c 100644 --- a/cpu.c +++ b/cpu.c @@ -316,10 +316,6 @@ void cpu_single_step(CPUState *cpu, int enabled) cpu->singlestep_enabled = enabled; if (kvm_enabled()) { kvm_update_guest_debug(cpu, 0); - } else { - /* must flush all the translated code to avoid inconsistencies */ - /* XXX: only flush what is necessary */ - tb_flush(cpu); } trace_breakpoint_singlestep(cpu->cpu_index, enabled); }
Set CF_SINGLE_STEP when single-stepping is enabled. This avoids the need to flush all tb's when turning single-stepping on or off. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- include/exec/exec-all.h | 1 + accel/tcg/cpu-exec.c | 12 ++++++++++++ accel/tcg/translator.c | 7 +------ cpu.c | 4 ---- 4 files changed, 14 insertions(+), 10 deletions(-)