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

["Alex Bennée" <alex.bennee@linaro.org>]

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 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?

>
> 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

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro