gdbstub: move breakpoint logic to accel ops

As HW virtualization requires specific support to handle breakpoints
lets push out special casing out of the core gdbstub code and into
AccelOpsClass. This will make it easier to add other accelerator
support and reduces some of the stub shenanigans.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Mads Ynddal <mads@ynddal.dk>
Message-Id: <20220929114231.583801-45-alex.bennee@linaro.org>

5 files changed, 137 insertions, 118 deletions

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/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/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'))
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);
+}