qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH for-6.1 v6 00/17] tcg: breakpoint reorg


From: Mark Cave-Ayland
Subject: Re: [PATCH for-6.1 v6 00/17] tcg: breakpoint reorg
Date: Tue, 20 Jul 2021 22:47:46 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.12.0

On 20/07/2021 20:54, Richard Henderson wrote:

This is fixing #404 ("windows xp boot takes much longer...")
and several other similar reports.

Changes for v6:
   * Reinstate accidental loss of singlestep overriding breakpoint check.
     Shows up in the record-replay avocado tests failing to make progress.
   * Add CPUState.gdb_adjust_breakpoint hook for AVR weirdness.

Changes for v5:
   * Include missing hunk in tb_gen_code, as noted in reply to v4.
   * Remove helper_check_breakpoints from target/arm/.
   * Reorg cflags_for_breakpoints into check_for_breakpoints;
     reorg cpu_exec to use a break instead of a longjmp.
   * Move singlestep_enabled check from cflags_for_breakpoints
     to curr_cflags, which makes cpu_exec_step_atomic cleaner.

Changes for v4:
   * Issue breakpoints directly from cflags_for_breakpoints.
     Do not generate code for a TB beginning with a BP at all.
   * Drop the problematic TranslatorOps.breakpoint_check hook entirely.

Changes for v3:
   * Map CF_COUNT_MASK == 0 -> TCG_MAX_INSNS.
   * Split out *_breakpoint_check fixes for avr, mips, riscv.

Changes for v2:
   * All prerequisites and 7 of the patches from v1 with are merged.

Patches lacking review:
   11-hw-core-Introduce-CPUClass.gdb_adjust_breakpoint.patch
   12-target-avr-Implement-gdb_adjust_breakpoint.patch
   15-accel-tcg-Remove-TranslatorOps.breakpoint_check.patch
   17-accel-tcg-Record-singlestep_enabled-in-tb-cflags.patch


r~


Richard Henderson (17):
   accel/tcg: Reduce CF_COUNT_MASK to match TCG_MAX_INSNS
   accel/tcg: Move curr_cflags into cpu-exec.c
   target/alpha: Drop goto_tb path in gen_call_pal
   accel/tcg: Add CF_NO_GOTO_TB and CF_NO_GOTO_PTR
   accel/tcg: Drop CF_NO_GOTO_PTR from -d nochain
   accel/tcg: Handle -singlestep in curr_cflags
   accel/tcg: Use CF_NO_GOTO_{TB, PTR} in cpu_exec_step_atomic
   hw/core: Introduce TCGCPUOps.debug_check_breakpoint
   target/arm: Implement debug_check_breakpoint
   target/i386: Implement debug_check_breakpoint
   hw/core: Introduce CPUClass.gdb_adjust_breakpoint
   target/avr: Implement gdb_adjust_breakpoint
   accel/tcg: Merge tb_find into its only caller
   accel/tcg: Move breakpoint recognition outside translation
   accel/tcg: Remove TranslatorOps.breakpoint_check
   accel/tcg: Hoist tb_cflags to a local in translator_loop
   accel/tcg: Record singlestep_enabled in tb->cflags

  include/exec/exec-all.h       |  24 ++--
  include/exec/translator.h     |  11 --
  include/hw/core/cpu.h         |   4 +
  include/hw/core/tcg-cpu-ops.h |   6 +
  target/arm/helper.h           |   2 -
  target/arm/internals.h        |   3 +
  target/avr/cpu.h              |   1 +
  accel/tcg/cpu-exec.c          | 205 ++++++++++++++++++++++++++--------
  accel/tcg/translate-all.c     |   7 +-
  accel/tcg/translator.c        |  39 ++-----
  cpu.c                         |  34 ++----
  target/alpha/translate.c      |  31 +----
  target/arm/cpu.c              |   1 +
  target/arm/cpu_tcg.c          |   1 +
  target/arm/debug_helper.c     |  12 +-
  target/arm/translate-a64.c    |  25 -----
  target/arm/translate.c        |  29 -----
  target/avr/cpu.c              |   1 +
  target/avr/gdbstub.c          |  13 +++
  target/avr/translate.c        |  32 ------
  target/cris/translate.c       |  20 ----
  target/hexagon/translate.c    |  17 ---
  target/hppa/translate.c       |  11 --
  target/i386/tcg/tcg-cpu.c     |  12 ++
  target/i386/tcg/translate.c   |  28 -----
  target/m68k/translate.c       |  18 ---
  target/microblaze/translate.c |  18 ---
  target/mips/tcg/translate.c   |  19 ----
  target/nios2/translate.c      |  27 -----
  target/openrisc/translate.c   |  17 ---
  target/ppc/translate.c        |  18 ---
  target/riscv/translate.c      |  17 ---
  target/rx/translate.c         |  14 ---
  target/s390x/tcg/translate.c  |  24 ----
  target/sh4/translate.c        |  18 ---
  target/sparc/translate.c      |  17 ---
  target/tricore/translate.c    |  16 ---
  target/xtensa/translate.c     |  17 ---
  tcg/tcg-op.c                  |  28 ++---
  39 files changed, 248 insertions(+), 589 deletions(-)

I spent a bit of time this evening testing this patchset with some typical OpenBIOS debugging patterns across SPARC32, SPARC64 and PPC and didn't spot any regressions, and the WinXP image still boots in 25s so:

Tested-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

I did find the slow-down noticeable when testing a few OpenBIOS breakpoints e.g. a breakpoint on OpenBIOS SPARC64's init_memory() upped the boot time to the Forth prompt from 9s to 30s even though it hits only once early in boot (presumably due to its proximity to a hot routine). Having said that, the changes look like a good improvement and patch 14 suggests that there could be further optimisations to be made in future, so in general this feels like a net win.


ATB,

Mark.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]