qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Richard Henderson <richard.henderson@linaro.org>
To: qemu-devel@nongnu.org
Cc: "Philippe Mathieu-Daudé" <f4bug@amsat.org>
Subject: [PULL 13/24] target/mips: Fix single stepping
Date: Sat, 16 Oct 2021 11:15:03 -0700	[thread overview]
Message-ID: <20211016181514.3165661-14-richard.henderson@linaro.org> (raw)
In-Reply-To: <20211016181514.3165661-1-richard.henderson@linaro.org>

As per an ancient comment in mips_tr_translate_insn about the
expectations of gdb, when restarting the insn in a delay slot
we also re-execute the branch.  Which means that we are
expected to execute two insns in this case.

This has been broken since 8b86d6d2580, where we forced max_insns
to 1 while single-stepping.  This resulted in an exit from the
translator loop after the branch but before the delay slot is
translated.

Increase the max_insns to 2 for this case.  In addition, bypass
the end-of-page check, for when the branch itself ends the page.

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/mips/tcg/translate.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/target/mips/tcg/translate.c b/target/mips/tcg/translate.c
index 148afec9dc..f239f9ffc0 100644
--- a/target/mips/tcg/translate.c
+++ b/target/mips/tcg/translate.c
@@ -16016,6 +16016,16 @@ static void mips_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
     ctx->default_tcg_memop_mask = (ctx->insn_flags & (ISA_MIPS_R6 |
                                   INSN_LOONGSON3A)) ? MO_UNALN : MO_ALIGN;
 
+    /*
+     * Execute a branch and its delay slot as a single instruction.
+     * This is what GDB expects and is consistent with what the
+     * hardware does (e.g. if a delay slot instruction faults, the
+     * reported PC is the PC of the branch).
+     */
+    if (ctx->base.singlestep_enabled && (ctx->hflags & MIPS_HFLAG_BMASK)) {
+        ctx->base.max_insns = 2;
+    }
+
     LOG_DISAS("\ntb %p idx %d hflags %04x\n", ctx->base.tb, ctx->mem_idx,
               ctx->hflags);
 }
@@ -16085,17 +16095,14 @@ static void mips_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
     if (ctx->base.is_jmp != DISAS_NEXT) {
         return;
     }
+
     /*
-     * Execute a branch and its delay slot as a single instruction.
-     * This is what GDB expects and is consistent with what the
-     * hardware does (e.g. if a delay slot instruction faults, the
-     * reported PC is the PC of the branch).
+     * End the TB on (most) page crossings.
+     * See mips_tr_init_disas_context about single-stepping a branch
+     * together with its delay slot.
      */
-    if (ctx->base.singlestep_enabled &&
-        (ctx->hflags & MIPS_HFLAG_BMASK) == 0) {
-        ctx->base.is_jmp = DISAS_TOO_MANY;
-    }
-    if (ctx->base.pc_next - ctx->page_start >= TARGET_PAGE_SIZE) {
+    if (ctx->base.pc_next - ctx->page_start >= TARGET_PAGE_SIZE
+        && !ctx->base.singlestep_enabled) {
         ctx->base.is_jmp = DISAS_TOO_MANY;
     }
 }
-- 
2.25.1



  parent reply	other threads:[~2021-10-16 18:24 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-16 18:14 [PULL 00/24] tcg patch queue Richard Henderson
2021-10-16 18:14 ` [PULL 01/24] accel/tcg: Handle gdb singlestep in cpu_tb_exec Richard Henderson
2021-10-16 18:14 ` [PULL 02/24] target/alpha: Drop checks for singlestep_enabled Richard Henderson
2021-10-16 18:14 ` [PULL 03/24] target/avr: " Richard Henderson
2021-10-16 18:14 ` [PULL 04/24] target/cris: " Richard Henderson
2021-10-16 18:14 ` [PULL 05/24] target/hexagon: " Richard Henderson
2021-10-16 18:14 ` [PULL 06/24] target/arm: " Richard Henderson
2021-10-16 18:14 ` [PULL 07/24] target/hppa: " Richard Henderson
2021-10-16 18:14 ` [PULL 08/24] target/i386: Check CF_NO_GOTO_TB for dc->jmp_opt Richard Henderson
2021-10-16 18:14 ` [PULL 09/24] target/i386: Drop check for singlestep_enabled Richard Henderson
2021-10-16 18:15 ` [PULL 10/24] target/m68k: Drop checks " Richard Henderson
2021-10-16 18:15 ` [PULL 11/24] target/microblaze: Check CF_NO_GOTO_TB for DISAS_JUMP Richard Henderson
2021-10-16 18:15 ` [PULL 12/24] target/microblaze: Drop checks for singlestep_enabled Richard Henderson
2021-10-16 18:15 ` Richard Henderson [this message]
2021-10-16 18:15 ` [PULL 14/24] target/mips: Drop exit " Richard Henderson
2021-10-16 18:15 ` [PULL 15/24] target/openrisc: Drop " Richard Henderson
2021-10-16 18:15 ` [PULL 16/24] target/ppc: Drop exit " Richard Henderson
2021-10-16 18:15 ` [PULL 17/24] target/riscv: Remove dead code after exception Richard Henderson
2021-10-16 18:15 ` [PULL 18/24] target/riscv: Remove exit_tb and lookup_and_goto_ptr Richard Henderson
2021-10-16 18:15 ` [PULL 19/24] target/rx: Drop checks for singlestep_enabled Richard Henderson
2021-10-16 18:15 ` [PULL 20/24] target/s390x: Drop check " Richard Henderson
2021-10-16 18:15 ` [PULL 21/24] target/sh4: " Richard Henderson
2021-10-16 18:15 ` [PULL 22/24] target/tricore: " Richard Henderson
2021-10-16 18:15 ` [PULL 23/24] target/xtensa: " Richard Henderson
2021-10-16 18:15 ` [PULL 24/24] Revert "cpu: Move cpu_common_props to hw/core/cpu.c" Richard Henderson
2021-10-16 23:49 ` [PULL 00/24] tcg patch queue Richard Henderson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211016181514.3165661-14-richard.henderson@linaro.org \
    --to=richard.henderson@linaro.org \
    --cc=f4bug@amsat.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).