Re: [PATCH 00/18] Stop all qemu-cpu threads on a breakpoint

[Ilya Leoshkevich <iii@linux.ibm.com>]

On Wed, 2025-01-08 at 15:56 +0000, Alex Bennée wrote:
> Ilya Leoshkevich <iii@linux.ibm.com> writes:
> 
> > Hi,
> > 
> > On reporting a breakpoint in a non-non-stop mode, GDB remotes must
> > stop
> > all threads. Currently qemu-user doesn't do that, breaking the
> > debugging session for at least two reasons: concurrent access to
> > the
> > GDB socket, and an assertion within GDB [1].
> > 
> > This series fixes this by importing pause_all_vcpus() from qemu-
> > system.
> > This in turn requires introducing BQL and a few stubs to qemu-user.
> 
> Is there a conclusion to this design choice? I'd like to avoid
> bringing
> in a bunch of system-mode infrastructure if the existing exclusive
> code
> would work. For that I'll defer to the linux-user maintainer or
> Richard
> who knows the code better than I do.

I wanted to re-implement parking CPUs using a reserved host signal.
I've submitted the foundations for this in [1], and I'm currently
waiting for the review.

[1]
https://lore.kernel.org/qemu-devel/20241216123412.77450-1-iii@linux.ibm.com/

> 
> I could certainly harvest the early clean-up patches to keep the
> delta
> low while the details are worked out. Is there going to be a v2?

It would be great if clean-up patches could be taken separately, since
IMHO they make sense on their own. I plan to send a v2 after [1] is
integrated.

> > Best regards,
> > Ilya
> > 
> > [1] https://gitlab.com/qemu-project/qemu/-/issues/2465
> > 
> > Ilya Leoshkevich (18):
> >   gdbstub: Make gdb_get_char() static
> >   gdbstub: Move phy_memory_mode to GDBSystemState
> >   gdbstub: Move gdb_syscall_mode to GDBSyscallState
> >   gdbstub: Factor out gdb_try_stop()
> >   accel/tcg: Factor out cpu_exec_user()
> >   qemu-thread: Introduce QEMU_MUTEX_INITIALIZER
> >   qemu-thread: Introduce QEMU_COND_INITIALIZER
> >   replay: Add replay_mutex_{lock,unlock}() stubs for qemu-user
> >   qemu-timer: Provide qemu_clock_enable() stub for qemu-user
> >   cpu: Use BQL in qemu-user
> >   accel/tcg: Unify user implementations of qemu_cpu_kick()
> >   cpu: Track CPUs executing syscalls
> >   cpu: Implement cpu_thread_is_idle() for qemu-user
> >   cpu: Introduce cpu_is_paused()
> >   cpu: Set current_cpu early in qemu-user
> >   cpu: Allow pausing and resuming CPUs in qemu-user
> >   gdbstub: Pause all CPUs before sending stop replies
> >   tests/tcg: Stress test thread breakpoints
> > 
> >  accel/tcg/user-exec-stub.c                    |   4 -
> >  accel/tcg/user-exec.c                         |  55 ++++++
> >  bsd-user/aarch64/target_arch_cpu.h            |   6 +-
> >  bsd-user/arm/target_arch_cpu.h                |   5 +-
> >  bsd-user/freebsd/os-syscall.c                 |  10 +
> >  bsd-user/i386/target_arch_cpu.h               |   5 +-
> >  bsd-user/main.c                               |   8 +-
> >  bsd-user/x86_64/target_arch_cpu.h             |   5 +-
> >  cpu-common.c                                  | 179
> > ++++++++++++++++++
> >  gdbstub/gdbstub.c                             |  17 +-
> >  gdbstub/internals.h                           |   4 +-
> >  gdbstub/syscalls.c                            |  20 +-
> >  gdbstub/system.c                              |  18 +-
> >  gdbstub/user.c                                |  28 ++-
> >  include/exec/cpu-common.h                     |  15 ++
> >  include/exec/replay-core.h                    |  13 ++
> >  include/hw/core/cpu.h                         |   1 +
> >  include/qemu/thread-posix.h                   |   8 +
> >  include/qemu/thread-win32.h                   |   8 +
> >  include/sysemu/cpus.h                         |   6 -
> >  include/sysemu/replay.h                       |  13 --
> >  linux-user/aarch64/cpu_loop.c                 |   5 +-
> >  linux-user/alpha/cpu_loop.c                   |   5 +-
> >  linux-user/arm/cpu_loop.c                     |   5 +-
> >  linux-user/hexagon/cpu_loop.c                 |   5 +-
> >  linux-user/hppa/cpu_loop.c                    |   5 +-
> >  linux-user/i386/cpu_loop.c                    |   5 +-
> >  linux-user/loongarch64/cpu_loop.c             |   5 +-
> >  linux-user/m68k/cpu_loop.c                    |   5 +-
> >  linux-user/main.c                             |   9 +-
> >  linux-user/microblaze/cpu_loop.c              |   5 +-
> >  linux-user/mips/cpu_loop.c                    |   5 +-
> >  linux-user/openrisc/cpu_loop.c                |   5 +-
> >  linux-user/ppc/cpu_loop.c                     |   5 +-
> >  linux-user/riscv/cpu_loop.c                   |   5 +-
> >  linux-user/s390x/cpu_loop.c                   |   5 +-
> >  linux-user/sh4/cpu_loop.c                     |   5 +-
> >  linux-user/sparc/cpu_loop.c                   |   5 +-
> >  linux-user/syscall.c                          |  12 ++
> >  linux-user/xtensa/cpu_loop.c                  |   5 +-
> >  replay/stubs-system.c                         |   8 +
> >  stubs/meson.build                             |   8 +
> >  stubs/qemu-timer.c                            |   6 +
> >  stubs/replay-mutex.c                          |  10 +
> >  stubs/replay-tools.c                          |   8 -
> >  system/cpus.c                                 | 172 +-------------
> > ---
> >  tests/tcg/multiarch/Makefile.target           |  13 +-
> >  .../gdbstub/test-thread-breakpoint-stress.py  |  28 +++
> >  .../tcg/multiarch/thread-breakpoint-stress.c  |  92 +++++++++
> >  49 files changed, 552 insertions(+), 327 deletions(-)
> >  create mode 100644 stubs/qemu-timer.c
> >  create mode 100644 stubs/replay-mutex.c
> >  create mode 100644 tests/tcg/multiarch/gdbstub/test-thread-
> > breakpoint-stress.py
> >  create mode 100644 tests/tcg/multiarch/thread-breakpoint-stress.c
>