[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH for-6.1 v6 17/17] accel/tcg: Record singlestep_enabled in tb-
From: |
Alex Bennée |
Subject: |
Re: [PATCH for-6.1 v6 17/17] accel/tcg: Record singlestep_enabled in tb->cflags |
Date: |
Wed, 21 Jul 2021 11:38:37 +0100 |
User-agent: |
mu4e 1.5.14; emacs 28.0.50 |
Richard Henderson <richard.henderson@linaro.org> writes:
> 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 | 7 ++++++-
> accel/tcg/translate-all.c | 4 ----
> accel/tcg/translator.c | 7 +------
> cpu.c | 4 ----
> 5 files changed, 8 insertions(+), 15 deletions(-)
>
> 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 5cc6363f4c..fc895cf51e 100644
> --- a/accel/tcg/cpu-exec.c
> +++ b/accel/tcg/cpu-exec.c
> @@ -150,10 +150,15 @@ uint32_t curr_cflags(CPUState *cpu)
> uint32_t cflags = cpu->tcg_cflags;
>
> /*
> + * Record gdb single-step. We should be exiting the TB by raising
> + * EXCP_DEBUG, but to simplify other tests, disable chaining too.
> + *
> * For singlestep and -d nochain, suppress goto_tb so that
> * we can log -d cpu,exec after every TB.
> */
> - if (singlestep) {
> + if (unlikely(cpu->singlestep_enabled)) {
> + cflags |= CF_NO_GOTO_TB | CF_NO_GOTO_PTR | CF_SINGLE_STEP |
> 1;
What does CF_SINGLE_STEP achieve that isn't already handled by having:
cflags |= CF_NO_GOTO_TB | CF_NO_GOTO_PTR | 1;
(btw did we mask CF_COUNT_MASK somewhere else?). Because surely the
CF_COUNT is part of cflags so limits the TB's that could be returned
anyway?
> + } else if (singlestep) {
> cflags |= CF_NO_GOTO_TB | 1;
> } else if (qemu_loglevel_mask(CPU_LOG_TB_NOCHAIN)) {
> cflags |= CF_NO_GOTO_TB;
> 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/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 d6ae5ae581..e1799a15bc 100644
> --- a/cpu.c
> +++ b/cpu.c
> @@ -326,10 +326,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);
> }
--
Alex Bennée
- Re: [PATCH for-6.1 v6 11/17] hw/core: Introduce CPUClass.gdb_adjust_breakpoint, (continued)
[PATCH for-6.1 v6 13/17] accel/tcg: Merge tb_find into its only caller, Richard Henderson, 2021/07/20
[PATCH for-6.1 v6 16/17] accel/tcg: Hoist tb_cflags to a local in translator_loop, Richard Henderson, 2021/07/20
[PATCH for-6.1 v6 17/17] accel/tcg: Record singlestep_enabled in tb->cflags, Richard Henderson, 2021/07/20
[PATCH for-6.1 v6 12/17] target/avr: Implement gdb_adjust_breakpoint, Richard Henderson, 2021/07/20
[PATCH for-6.1 v6 15/17] accel/tcg: Remove TranslatorOps.breakpoint_check, Richard Henderson, 2021/07/20
[PATCH for-6.1 v6 14/17] accel/tcg: Move breakpoint recognition outside translation, Richard Henderson, 2021/07/20
Re: [PATCH for-6.1 v6 00/17] tcg: breakpoint reorg, Mark Cave-Ayland, 2021/07/20