[v2,10/11] gdbstub: move breakpoint logic to accel ops

Message ID	20220926133904.3297263-11-alex.bennee@linaro.org
State	Superseded
Headers	show Delivered-To: patch@linaro.org Received-SPF: pass (google.com: domain of qemu-devel-bounces+patch=linaro.org@nongnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.17; From: =?utf-8?q?Alex_Benn=C3=A9e?= <alex.bennee@linaro.org> To: qemu-devel@nongnu.org Cc: f4bug@amsat.org, mads@ynddal.dk, qemu-arm@nongnu.org, =?utf-8?q?Alex_Ben?= =?utf-8?q?n=C3=A9e?= <alex.bennee@linaro.org>, Richard Henderson <richard.henderson@linaro.org>, Paolo Bonzini <pbonzini@redhat.com>, kvm@vger.kernel.org (open list:Overall KVM CPUs) Subject: [PATCH v2 10/11] gdbstub: move breakpoint logic to accel ops Date: Mon, 26 Sep 2022 14:39:03 +0100 Message-Id: <20220926133904.3297263-11-alex.bennee@linaro.org> In-Reply-To: <20220926133904.3297263-1-alex.bennee@linaro.org> References: <20220926133904.3297263-1-alex.bennee@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Received-SPF: pass client-ip=2a00:1450:4864:20::329; envelope-from=alex.bennee@linaro.org; helo=mail-wm1-x329.google.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action Precedence: list Errors-To: qemu-devel-bounces+patch=linaro.org@nongnu.org Sender: "Qemu-devel" <qemu-devel-bounces+patch=linaro.org@nongnu.org>
Series	gdbstub/next (MemTxAttrs and re-factoring) \| expand [v2,00/11] gdbstub/next (MemTxAttrs and re-factoring) [v2,01/11] hw: encode accessing CPU index in MemTxAttrs [v2,02/11] target/arm: enable tracking of CPU index in MemTxAttrs [v2,03/11] target/arm: ensure HVF traps set appropriate MemTxAttrs [v2,04/11] qtest: make read/write operation appear to be from CPU [v2,05/11] hw/intc/gic: use MxTxAttrs to divine accessing CPU [v2,06/11] hw/timer: convert mptimer access to attrs to derive cpu index [v2,07/11] configure: move detected gdb to TCG's config-host.mak [v2,08/11] gdbstub: move into its own sub directory [v2,09/11] gdbstub: move sstep flags probing into AccelClass [v2,10/11] gdbstub: move breakpoint logic to accel ops [v2,11/11] gdbstub: move guest debug support check to ops

diff --git a/accel/kvm/kvm-cpus.h b/accel/kvm/kvm-cpus.h index bf0bd1bee4..33e435d62b 100644 --- a/accel/kvm/kvm-cpus.h +++ b/accel/kvm/kvm-cpus.h @@ -18,5 +18,8 @@ void kvm_destroy_vcpu(CPUState *cpu); void kvm_cpu_synchronize_post_reset(CPUState *cpu); void kvm_cpu_synchronize_post_init(CPUState *cpu); void kvm_cpu_synchronize_pre_loadvm(CPUState *cpu); +int kvm_insert_breakpoint(CPUState *cpu, int type, hwaddr addr, hwaddr len); +int kvm_remove_breakpoint(CPUState *cpu, int type, hwaddr addr, hwaddr len); +void kvm_remove_all_breakpoints(CPUState *cpu); #endif /* KVM_CPUS_H */ diff --git a/gdbstub/internals.h b/gdbstub/internals.h new file mode 100644 index 0000000000..41e2e72dbf --- /dev/null +++ b/gdbstub/internals.h @@ -0,0 +1,16 @@ +/* + * gdbstub internals + * + * Copyright (c) 2022 Linaro Ltd + * + * SPDX-License-Identifier: GPL-2.0-or-later + */ + +#ifndef _INTERNALS_H_ +#define _INTERNALS_H_ + +int gdb_breakpoint_insert(CPUState *cs, int type, hwaddr addr, hwaddr len); +int gdb_breakpoint_remove(CPUState *cs, int type, hwaddr addr, hwaddr len); +void gdb_breakpoint_remove_all(CPUState *cs); + +#endif /* _INTERNALS_H_ */ diff --git a/include/sysemu/accel-ops.h b/include/sysemu/accel-ops.h index a0572ea87a..86794ac273 100644 --- a/include/sysemu/accel-ops.h +++ b/include/sysemu/accel-ops.h @@ -10,6 +10,7 @@ #ifndef ACCEL_OPS_H #define ACCEL_OPS_H +#include "exec/hwaddr.h" #include "qom/object.h" #define ACCEL_OPS_SUFFIX "-ops" @@ -44,6 +45,11 @@ struct AccelOpsClass { int64_t (*get_virtual_clock)(void); int64_t (*get_elapsed_ticks)(void); + + /* gdbstub hooks */ + int (*insert_breakpoint)(CPUState *cpu, int type, hwaddr addr, hwaddr len); + int (*remove_breakpoint)(CPUState *cpu, int type, hwaddr addr, hwaddr len); + void (*remove_all_breakpoints)(CPUState *cpu); }; #endif /* ACCEL_OPS_H */ diff --git a/include/sysemu/cpus.h b/include/sysemu/cpus.h index b5c87d48b3..1bace3379b 100644 --- a/include/sysemu/cpus.h +++ b/include/sysemu/cpus.h @@ -7,6 +7,9 @@ /* register accel-specific operations */ void cpus_register_accel(const AccelOpsClass *i); +/* return registers ops */ +const AccelOpsClass *cpus_get_accel(void); + /* accel/dummy-cpus.c */ /* Create a dummy vcpu for AccelOpsClass->create_vcpu_thread */ diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h index a20ad51aad..21d3f1d01e 100644 --- a/include/sysemu/kvm.h +++ b/include/sysemu/kvm.h @@ -254,11 +254,6 @@ int kvm_on_sigbus(int code, void *addr); void kvm_flush_coalesced_mmio_buffer(void); -int kvm_insert_breakpoint(CPUState *cpu, target_ulong addr, - target_ulong len, int type); -int kvm_remove_breakpoint(CPUState *cpu, target_ulong addr, - target_ulong len, int type); -void kvm_remove_all_breakpoints(CPUState *cpu); int kvm_update_guest_debug(CPUState *cpu, unsigned long reinject_trap); /* internal API */ diff --git a/accel/kvm/kvm-accel-ops.c b/accel/kvm/kvm-accel-ops.c index c4244a23c6..5c0e37514c 100644 --- a/accel/kvm/kvm-accel-ops.c +++ b/accel/kvm/kvm-accel-ops.c @@ -16,12 +16,14 @@ #include "qemu/osdep.h" #include "qemu/error-report.h" #include "qemu/main-loop.h" +#include "sysemu/kvm.h" #include "sysemu/kvm_int.h" #include "sysemu/runstate.h" #include "sysemu/cpus.h" #include "qemu/guest-random.h" #include "qapi/error.h" +#include <linux/kvm.h> #include "kvm-cpus.h" static void *kvm_vcpu_thread_fn(void *arg) @@ -95,6 +97,12 @@ static void kvm_accel_ops_class_init(ObjectClass *oc, void *data) ops->synchronize_post_init = kvm_cpu_synchronize_post_init; ops->synchronize_state = kvm_cpu_synchronize_state; ops->synchronize_pre_loadvm = kvm_cpu_synchronize_pre_loadvm; + +#ifdef KVM_CAP_SET_GUEST_DEBUG + ops->insert_breakpoint = kvm_insert_breakpoint; + ops->remove_breakpoint = kvm_remove_breakpoint; + ops->remove_all_breakpoints = kvm_remove_all_breakpoints; +#endif } static const TypeInfo kvm_accel_ops_type = { diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c index c55938453a..b8c734fe3a 100644 --- a/accel/kvm/kvm-all.c +++ b/accel/kvm/kvm-all.c @@ -3287,8 +3287,7 @@ int kvm_update_guest_debug(CPUState *cpu, unsigned long reinject_trap) return data.err; } -int kvm_insert_breakpoint(CPUState *cpu, target_ulong addr, - target_ulong len, int type) +int kvm_insert_breakpoint(CPUState *cpu, int type, hwaddr addr, hwaddr len) { struct kvm_sw_breakpoint *bp; int err; @@ -3326,8 +3325,7 @@ int kvm_insert_breakpoint(CPUState *cpu, target_ulong addr, return 0; } -int kvm_remove_breakpoint(CPUState *cpu, target_ulong addr, - target_ulong len, int type) +int kvm_remove_breakpoint(CPUState *cpu, int type, hwaddr addr, hwaddr len) { struct kvm_sw_breakpoint *bp; int err; @@ -3393,26 +3391,10 @@ void kvm_remove_all_breakpoints(CPUState *cpu) #else /* !KVM_CAP_SET_GUEST_DEBUG */ -int kvm_update_guest_debug(CPUState *cpu, unsigned long reinject_trap) +static int kvm_update_guest_debug(CPUState *cpu, unsigned long reinject_trap) { return -EINVAL; } - -int kvm_insert_breakpoint(CPUState *cpu, target_ulong addr, - target_ulong len, int type) -{ - return -EINVAL; -} - -int kvm_remove_breakpoint(CPUState *cpu, target_ulong addr, - target_ulong len, int type) -{ - return -EINVAL; -} - -void kvm_remove_all_breakpoints(CPUState *cpu) -{ -} #endif /* !KVM_CAP_SET_GUEST_DEBUG */ static int kvm_set_signal_mask(CPUState *cpu, const sigset_t *sigset) diff --git a/accel/stubs/kvm-stub.c b/accel/stubs/kvm-stub.c index 2ac5f9c036..2d79333143 100644 --- a/accel/stubs/kvm-stub.c +++ b/accel/stubs/kvm-stub.c @@ -51,22 +51,6 @@ int kvm_update_guest_debug(CPUState *cpu, unsigned long reinject_trap) return -ENOSYS; } -int kvm_insert_breakpoint(CPUState *cpu, target_ulong addr, - target_ulong len, int type) -{ - return -EINVAL; -} - -int kvm_remove_breakpoint(CPUState *cpu, target_ulong addr, - target_ulong len, int type) -{ - return -EINVAL; -} - -void kvm_remove_all_breakpoints(CPUState *cpu) -{ -} - int kvm_on_sigbus_vcpu(CPUState *cpu, int code, void *addr) { return 1; diff --git a/accel/tcg/tcg-accel-ops.c b/accel/tcg/tcg-accel-ops.c index 786d90c08f..965c2ad581 100644 --- a/accel/tcg/tcg-accel-ops.c +++ b/accel/tcg/tcg-accel-ops.c @@ -32,6 +32,8 @@ #include "qemu/main-loop.h" #include "qemu/guest-random.h" #include "exec/exec-all.h" +#include "exec/hwaddr.h" +#include "exec/gdbstub.h" #include "tcg-accel-ops.h" #include "tcg-accel-ops-mttcg.h" @@ -91,6 +93,92 @@ void tcg_handle_interrupt(CPUState *cpu, int mask) } } +/* Translate GDB watchpoint type to a flags value for cpu_watchpoint_* */ +static inline int xlat_gdb_type(CPUState *cpu, int gdbtype) +{ + static const int xlat[] = { + [GDB_WATCHPOINT_WRITE] = BP_GDB | BP_MEM_WRITE, + [GDB_WATCHPOINT_READ] = BP_GDB | BP_MEM_READ, + [GDB_WATCHPOINT_ACCESS] = BP_GDB | BP_MEM_ACCESS, + }; + + CPUClass *cc = CPU_GET_CLASS(cpu); + int cputype = xlat[gdbtype]; + + if (cc->gdb_stop_before_watchpoint) { + cputype |= BP_STOP_BEFORE_ACCESS; + } + return cputype; +} + +static int tcg_insert_breakpoint(CPUState *cs, int type, hwaddr addr, hwaddr len) +{ + CPUState *cpu; + int err = 0; + + switch (type) { + case GDB_BREAKPOINT_SW: + case GDB_BREAKPOINT_HW: + CPU_FOREACH(cpu) { + err = cpu_breakpoint_insert(cpu, addr, BP_GDB, NULL); + if (err) { + break; + } + } + return err; + case GDB_WATCHPOINT_WRITE: + case GDB_WATCHPOINT_READ: + case GDB_WATCHPOINT_ACCESS: + CPU_FOREACH(cpu) { + err = cpu_watchpoint_insert(cpu, addr, len, + xlat_gdb_type(cpu, type), NULL); + if (err) { + break; + } + } + return err; + default: + return -ENOSYS; + } +} + +static int tcg_remove_breakpoint(CPUState *cs, int type, hwaddr addr, hwaddr len) +{ + CPUState *cpu; + int err = 0; + + switch (type) { + case GDB_BREAKPOINT_SW: + case GDB_BREAKPOINT_HW: + CPU_FOREACH(cpu) { + err = cpu_breakpoint_remove(cpu, addr, BP_GDB); + if (err) { + break; + } + } + return err; + case GDB_WATCHPOINT_WRITE: + case GDB_WATCHPOINT_READ: + case GDB_WATCHPOINT_ACCESS: + CPU_FOREACH(cpu) { + err = cpu_watchpoint_remove(cpu, addr, len, + xlat_gdb_type(cpu, type)); + if (err) { + break; + } + } + return err; + default: + return -ENOSYS; + } +} + +static inline void tcg_remove_all_breakpoints(CPUState *cpu) +{ + cpu_breakpoint_remove_all(cpu, BP_GDB); + cpu_watchpoint_remove_all(cpu, BP_GDB); +} + static void tcg_accel_ops_init(AccelOpsClass *ops) { if (qemu_tcg_mttcg_enabled()) { @@ -109,6 +197,10 @@ static void tcg_accel_ops_init(AccelOpsClass *ops) ops->handle_interrupt = tcg_handle_interrupt; } } + + ops->insert_breakpoint = tcg_insert_breakpoint; + ops->remove_breakpoint = tcg_remove_breakpoint; + ops->remove_all_breakpoints = tcg_remove_all_breakpoints; } static void tcg_accel_ops_class_init(ObjectClass *oc, void *data) diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c index a0755e6505..ff9f3f9586 100644 --- a/gdbstub/gdbstub.c +++ b/gdbstub/gdbstub.c @@ -49,8 +49,11 @@ #include "sysemu/runstate.h" #include "semihosting/semihost.h" #include "exec/exec-all.h" +#include "exec/hwaddr.h" #include "sysemu/replay.h" +#include "internals.h" + #ifdef CONFIG_USER_ONLY #define GDB_ATTACHED "0" #else @@ -1012,130 +1015,16 @@ void gdb_register_coprocessor(CPUState *cpu, } } -#ifndef CONFIG_USER_ONLY -/* Translate GDB watchpoint type to a flags value for cpu_watchpoint_* */ -static inline int xlat_gdb_type(CPUState *cpu, int gdbtype) -{ - static const int xlat[] = { - [GDB_WATCHPOINT_WRITE] = BP_GDB | BP_MEM_WRITE, - [GDB_WATCHPOINT_READ] = BP_GDB | BP_MEM_READ, - [GDB_WATCHPOINT_ACCESS] = BP_GDB | BP_MEM_ACCESS, - }; - - CPUClass *cc = CPU_GET_CLASS(cpu); - int cputype = xlat[gdbtype]; - - if (cc->gdb_stop_before_watchpoint) { - cputype |= BP_STOP_BEFORE_ACCESS; - } - return cputype; -} -#endif - -static int gdb_breakpoint_insert(int type, target_ulong addr, target_ulong len) -{ - CPUState *cpu; - int err = 0; - - if (kvm_enabled()) { - return kvm_insert_breakpoint(gdbserver_state.c_cpu, addr, len, type); - } - - switch (type) { - case GDB_BREAKPOINT_SW: - case GDB_BREAKPOINT_HW: - CPU_FOREACH(cpu) { - err = cpu_breakpoint_insert(cpu, addr, BP_GDB, NULL); - if (err) { - break; - } - } - return err; -#ifndef CONFIG_USER_ONLY - case GDB_WATCHPOINT_WRITE: - case GDB_WATCHPOINT_READ: - case GDB_WATCHPOINT_ACCESS: - CPU_FOREACH(cpu) { - err = cpu_watchpoint_insert(cpu, addr, len, - xlat_gdb_type(cpu, type), NULL); - if (err) { - break; - } - } - return err; -#endif - default: - return -ENOSYS; - } -} - -static int gdb_breakpoint_remove(int type, target_ulong addr, target_ulong len) -{ - CPUState *cpu; - int err = 0; - - if (kvm_enabled()) { - return kvm_remove_breakpoint(gdbserver_state.c_cpu, addr, len, type); - } - - switch (type) { - case GDB_BREAKPOINT_SW: - case GDB_BREAKPOINT_HW: - CPU_FOREACH(cpu) { - err = cpu_breakpoint_remove(cpu, addr, BP_GDB); - if (err) { - break; - } - } - return err; -#ifndef CONFIG_USER_ONLY - case GDB_WATCHPOINT_WRITE: - case GDB_WATCHPOINT_READ: - case GDB_WATCHPOINT_ACCESS: - CPU_FOREACH(cpu) { - err = cpu_watchpoint_remove(cpu, addr, len, - xlat_gdb_type(cpu, type)); - if (err) - break; - } - return err; -#endif - default: - return -ENOSYS; - } -} - -static inline void gdb_cpu_breakpoint_remove_all(CPUState *cpu) -{ - cpu_breakpoint_remove_all(cpu, BP_GDB); -#ifndef CONFIG_USER_ONLY - cpu_watchpoint_remove_all(cpu, BP_GDB); -#endif -} - static void gdb_process_breakpoint_remove_all(GDBProcess *p) { CPUState *cpu = get_first_cpu_in_process(p); while (cpu) { - gdb_cpu_breakpoint_remove_all(cpu); + gdb_breakpoint_remove_all(cpu); cpu = gdb_next_cpu_in_process(cpu); } } -static void gdb_breakpoint_remove_all(void) -{ - CPUState *cpu; - - if (kvm_enabled()) { - kvm_remove_all_breakpoints(gdbserver_state.c_cpu); - return; - } - - CPU_FOREACH(cpu) { - gdb_cpu_breakpoint_remove_all(cpu); - } -} static void gdb_set_cpu_pc(target_ulong pc) { @@ -1667,7 +1556,8 @@ static void handle_insert_bp(GArray *params, void *user_ctx) return; } - res = gdb_breakpoint_insert(get_param(params, 0)->val_ul, + res = gdb_breakpoint_insert(gdbserver_state.c_cpu, + get_param(params, 0)->val_ul, get_param(params, 1)->val_ull, get_param(params, 2)->val_ull); if (res >= 0) { @@ -1690,7 +1580,8 @@ static void handle_remove_bp(GArray *params, void *user_ctx) return; } - res = gdb_breakpoint_remove(get_param(params, 0)->val_ul, + res = gdb_breakpoint_remove(gdbserver_state.c_cpu, + get_param(params, 0)->val_ul, get_param(params, 1)->val_ull, get_param(params, 2)->val_ull); if (res >= 0) { @@ -2541,7 +2432,7 @@ static void handle_target_halt(GArray *params, void *user_ctx) * because gdb is doing an initial connect and the state * should be cleaned up. */ - gdb_breakpoint_remove_all(); + gdb_breakpoint_remove_all(gdbserver_state.c_cpu); } static int gdb_handle_packet(const char *line_buf) diff --git a/gdbstub/softmmu.c b/gdbstub/softmmu.c new file mode 100644 index 0000000000..4e73890379 --- /dev/null +++ b/gdbstub/softmmu.c @@ -0,0 +1,42 @@ +/* + * gdb server stub - softmmu specific bits + * + * Debug integration depends on support from the individual + * accelerators so most of this involves calling the ops helpers. + * + * Copyright (c) 2022 Linaro Ltd + * + * SPDX-License-Identifier: GPL-2.0-or-later + */ + +#include "qemu/osdep.h" +#include "exec/gdbstub.h" +#include "exec/hwaddr.h" +#include "sysemu/cpus.h" +#include "internals.h" + +int gdb_breakpoint_insert(CPUState *cs, int type, hwaddr addr, hwaddr len) +{ + const AccelOpsClass *ops = cpus_get_accel(); + if (ops->insert_breakpoint) { + return ops->insert_breakpoint(cs, type, addr, len); + } + return -ENOSYS; +} + +int gdb_breakpoint_remove(CPUState *cs, int type, hwaddr addr, hwaddr len) +{ + const AccelOpsClass *ops = cpus_get_accel(); + if (ops->remove_breakpoint) { + return ops->remove_breakpoint(cs, type, addr, len); + } + return -ENOSYS; +} + +void gdb_breakpoint_remove_all(CPUState *cs) +{ + const AccelOpsClass *ops = cpus_get_accel(); + if (ops->remove_all_breakpoints) { + ops->remove_all_breakpoints(cs); + } +} diff --git a/gdbstub/user.c b/gdbstub/user.c new file mode 100644 index 0000000000..42652b28a7 --- /dev/null +++ b/gdbstub/user.c @@ -0,0 +1,62 @@ +/* + * gdbstub user-mode helper routines. + * + * We know for user-mode we are using TCG so we can call stuff directly. + * + * Copyright (c) 2022 Linaro Ltd + * + * SPDX-License-Identifier: GPL-2.0-or-later + */ + +#include "qemu/osdep.h" +#include "exec/hwaddr.h" +#include "exec/gdbstub.h" +#include "hw/core/cpu.h" +#include "internals.h" + +int gdb_breakpoint_insert(CPUState *cs, int type, hwaddr addr, hwaddr len) +{ + CPUState *cpu; + int err = 0; + + switch (type) { + case GDB_BREAKPOINT_SW: + case GDB_BREAKPOINT_HW: + CPU_FOREACH(cpu) { + err = cpu_breakpoint_insert(cpu, addr, BP_GDB, NULL); + if (err) { + break; + } + } + return err; + default: + /* user-mode doesn't support watchpoints */ + return -ENOSYS; + } +} + +int gdb_breakpoint_remove(CPUState *cs, int type, hwaddr addr, hwaddr len) +{ + CPUState *cpu; + int err = 0; + + switch (type) { + case GDB_BREAKPOINT_SW: + case GDB_BREAKPOINT_HW: + CPU_FOREACH(cpu) { + err = cpu_breakpoint_remove(cpu, addr, BP_GDB); + if (err) { + break; + } + } + return err; + default: + /* user-mode doesn't support watchpoints */ + return -ENOSYS; + } +} + +void gdb_breakpoint_remove_all(CPUState *cs) +{ + cpu_breakpoint_remove_all(cs, BP_GDB); +} diff --git a/softmmu/cpus.c b/softmmu/cpus.c index 23b30484b2..61b27ff59d 100644 --- a/softmmu/cpus.c +++ b/softmmu/cpus.c @@ -617,6 +617,13 @@ void cpus_register_accel(const AccelOpsClass *ops) cpus_accel = ops; } +const AccelOpsClass *cpus_get_accel(void) +{ + /* broken if we call this early */ + assert(cpus_accel); + return cpus_accel; +} + void qemu_init_vcpu(CPUState *cpu) { MachineState *ms = MACHINE(qdev_get_machine()); diff --git a/gdbstub/meson.build b/gdbstub/meson.build index 6d4ae2d03c..fc895a2c39 100644 --- a/gdbstub/meson.build +++ b/gdbstub/meson.build @@ -1 +1,9 @@ +# +# The main gdbstub still relies on per-build definitions of various +# types. The bits pushed to softmmu/user.c try to use guest agnostic +# types such as hwaddr. +# + specific_ss.add(files('gdbstub.c')) +softmmu_ss.add(files('softmmu.c')) +user_ss.add(files('user.c'))

[v2,10/11] gdbstub: move breakpoint logic to accel ops

Commit Message

Patch