kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH  v3 04/15] target/arm: ensure KVM traps set appropriate MemTxAttrs
       [not found] <20220927141504.3886314-1-alex.bennee@linaro.org>
@ 2022-09-27 14:14 ` Alex Bennée
  2022-09-28 16:49   ` Richard Henderson
  2022-10-19 10:44   ` Philippe Mathieu-Daudé
  2022-09-27 14:15 ` [PATCH v3 12/15] gdbstub: move sstep flags probing into AccelClass Alex Bennée
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 11+ messages in thread
From: Alex Bennée @ 2022-09-27 14:14 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-arm, Alex Bennée, Peter Maydell, Paolo Bonzini,
	open list:Overall KVM CPUs

Although most KVM users will use the in-kernel GIC emulation it is
perfectly possible not to. In this case we need to ensure the
MemTxAttrs are correctly populated so the GIC can divine the source
CPU of the operation.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

---
v3
  - new for v3
---
 target/arm/kvm.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index e5c1bd50d2..05056562f4 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -801,13 +801,14 @@ MemTxAttrs kvm_arch_post_run(CPUState *cs, struct kvm_run *run)
 {
     ARMCPU *cpu;
     uint32_t switched_level;
+    MemTxAttrs attrs = MEMTXATTRS_CPU(cs);
 
     if (kvm_irqchip_in_kernel()) {
         /*
          * We only need to sync timer states with user-space interrupt
          * controllers, so return early and save cycles if we don't.
          */
-        return MEMTXATTRS_UNSPECIFIED;
+        return attrs;
     }
 
     cpu = ARM_CPU(cs);
@@ -848,7 +849,7 @@ MemTxAttrs kvm_arch_post_run(CPUState *cs, struct kvm_run *run)
         qemu_mutex_unlock_iothread();
     }
 
-    return MEMTXATTRS_UNSPECIFIED;
+    return attrs;
 }
 
 void kvm_arm_vm_state_change(void *opaque, bool running, RunState state)
@@ -1003,6 +1004,10 @@ int kvm_arch_fixup_msi_route(struct kvm_irq_routing_entry *route,
     hwaddr xlat, len, doorbell_gpa;
     MemoryRegionSection mrs;
     MemoryRegion *mr;
+    MemTxAttrs attrs = {
+        .requester_type = MTRT_PCI,
+        .requester_id = pci_requester_id(dev)
+    };
 
     if (as == &address_space_memory) {
         return 0;
@@ -1012,8 +1017,7 @@ int kvm_arch_fixup_msi_route(struct kvm_irq_routing_entry *route,
 
     RCU_READ_LOCK_GUARD();
 
-    mr = address_space_translate(as, address, &xlat, &len, true,
-                                 MEMTXATTRS_UNSPECIFIED);
+    mr = address_space_translate(as, address, &xlat, &len, true, attrs);
 
     if (!mr) {
         return 1;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH  v3 12/15] gdbstub: move sstep flags probing into AccelClass
       [not found] <20220927141504.3886314-1-alex.bennee@linaro.org>
  2022-09-27 14:14 ` [PATCH v3 04/15] target/arm: ensure KVM traps set appropriate MemTxAttrs Alex Bennée
@ 2022-09-27 14:15 ` Alex Bennée
  2022-10-04 10:25   ` Mads Ynddal
  2022-09-27 14:15 ` [PATCH v3 13/15] gdbstub: move breakpoint logic to accel ops Alex Bennée
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Alex Bennée @ 2022-09-27 14:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-arm, Alex Bennée, Richard Henderson,
	Philippe Mathieu-Daudé,
	Mads Ynddal, Paolo Bonzini, open list:Overall KVM CPUs

The support of single-stepping is very much dependent on support from
the accelerator we are using. To avoid special casing in gdbstub move
the probing out to an AccelClass function so future accelerators can
put their code there.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Cc: Mads Ynddal <mads@ynddal.dk>
---
 include/qemu/accel.h | 12 ++++++++++++
 include/sysemu/kvm.h |  8 --------
 accel/accel-common.c | 10 ++++++++++
 accel/kvm/kvm-all.c  | 14 +++++++++++++-
 accel/tcg/tcg-all.c  | 17 +++++++++++++++++
 gdbstub/gdbstub.c    | 22 ++++------------------
 6 files changed, 56 insertions(+), 27 deletions(-)

diff --git a/include/qemu/accel.h b/include/qemu/accel.h
index be56da1b99..ce4747634a 100644
--- a/include/qemu/accel.h
+++ b/include/qemu/accel.h
@@ -43,6 +43,10 @@ typedef struct AccelClass {
     bool (*has_memory)(MachineState *ms, AddressSpace *as,
                        hwaddr start_addr, hwaddr size);
 #endif
+
+    /* gdbstub related hooks */
+    int (*gdbstub_supported_sstep_flags)(void);
+
     bool *allowed;
     /*
      * Array of global properties that would be applied when specific
@@ -92,4 +96,12 @@ void accel_cpu_instance_init(CPUState *cpu);
  */
 bool accel_cpu_realizefn(CPUState *cpu, Error **errp);
 
+/**
+ * accel_supported_gdbstub_sstep_flags:
+ *
+ * Returns the supported single step modes for the configured
+ * accelerator.
+ */
+int accel_supported_gdbstub_sstep_flags(void);
+
 #endif /* QEMU_ACCEL_H */
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index efd6dee818..a20ad51aad 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -47,7 +47,6 @@ extern bool kvm_direct_msi_allowed;
 extern bool kvm_ioeventfd_any_length_allowed;
 extern bool kvm_msi_use_devid;
 extern bool kvm_has_guest_debug;
-extern int kvm_sstep_flags;
 
 #define kvm_enabled()           (kvm_allowed)
 /**
@@ -174,12 +173,6 @@ extern int kvm_sstep_flags;
  */
 #define kvm_supports_guest_debug() (kvm_has_guest_debug)
 
-/*
- * kvm_supported_sstep_flags
- * Returns: SSTEP_* flags that KVM supports for guest debug
- */
-#define kvm_get_supported_sstep_flags() (kvm_sstep_flags)
-
 #else
 
 #define kvm_enabled()           (0)
@@ -198,7 +191,6 @@ extern int kvm_sstep_flags;
 #define kvm_ioeventfd_any_length_enabled() (false)
 #define kvm_msi_devid_required() (false)
 #define kvm_supports_guest_debug() (false)
-#define kvm_get_supported_sstep_flags() (0)
 
 #endif  /* CONFIG_KVM_IS_POSSIBLE */
 
diff --git a/accel/accel-common.c b/accel/accel-common.c
index 50035bda55..df72cc989a 100644
--- a/accel/accel-common.c
+++ b/accel/accel-common.c
@@ -129,6 +129,16 @@ bool accel_cpu_realizefn(CPUState *cpu, Error **errp)
     return true;
 }
 
+int accel_supported_gdbstub_sstep_flags(void)
+{
+    AccelState *accel = current_accel();
+    AccelClass *acc = ACCEL_GET_CLASS(accel);
+    if (acc->gdbstub_supported_sstep_flags) {
+        return acc->gdbstub_supported_sstep_flags();
+    }
+    return 0;
+}
+
 static const TypeInfo accel_cpu_type = {
     .name = TYPE_ACCEL_CPU,
     .parent = TYPE_OBJECT,
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 5acab1767f..c55938453a 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -175,7 +175,7 @@ bool kvm_direct_msi_allowed;
 bool kvm_ioeventfd_any_length_allowed;
 bool kvm_msi_use_devid;
 bool kvm_has_guest_debug;
-int kvm_sstep_flags;
+static int kvm_sstep_flags;
 static bool kvm_immediate_exit;
 static hwaddr kvm_max_slot_size = ~0;
 
@@ -3712,6 +3712,17 @@ static void kvm_accel_instance_init(Object *obj)
     s->kvm_dirty_ring_size = 0;
 }
 
+/**
+ * kvm_gdbstub_sstep_flags():
+ *
+ * Returns: SSTEP_* flags that KVM supports for guest debug. The
+ * support is probed during kvm_init()
+ */
+static int kvm_gdbstub_sstep_flags(void)
+{
+    return kvm_sstep_flags;
+}
+
 static void kvm_accel_class_init(ObjectClass *oc, void *data)
 {
     AccelClass *ac = ACCEL_CLASS(oc);
@@ -3719,6 +3730,7 @@ static void kvm_accel_class_init(ObjectClass *oc, void *data)
     ac->init_machine = kvm_init;
     ac->has_memory = kvm_accel_has_memory;
     ac->allowed = &kvm_allowed;
+    ac->gdbstub_supported_sstep_flags = kvm_gdbstub_sstep_flags;
 
     object_class_property_add(oc, "kernel-irqchip", "on|off|split",
         NULL, kvm_set_kernel_irqchip,
diff --git a/accel/tcg/tcg-all.c b/accel/tcg/tcg-all.c
index 47952eecd7..30b503fb22 100644
--- a/accel/tcg/tcg-all.c
+++ b/accel/tcg/tcg-all.c
@@ -25,6 +25,7 @@
 
 #include "qemu/osdep.h"
 #include "sysemu/tcg.h"
+#include "sysemu/replay.h"
 #include "sysemu/cpu-timers.h"
 #include "tcg/tcg.h"
 #include "qapi/error.h"
@@ -207,12 +208,28 @@ static void tcg_set_splitwx(Object *obj, bool value, Error **errp)
     s->splitwx_enabled = value;
 }
 
+static int tcg_gdbstub_supported_sstep_flags(void)
+{
+    /*
+     * In replay mode all events will come from the log and can't be
+     * suppressed otherwise we would break determinism. However as those
+     * events are tied to the number of executed instructions we won't see
+     * them occurring every time we single step.
+     */
+    if (replay_mode != REPLAY_MODE_NONE) {
+        return SSTEP_ENABLE;
+    } else {
+        return SSTEP_ENABLE | SSTEP_NOIRQ | SSTEP_NOTIMER;
+    }
+}
+
 static void tcg_accel_class_init(ObjectClass *oc, void *data)
 {
     AccelClass *ac = ACCEL_CLASS(oc);
     ac->name = "tcg";
     ac->init_machine = tcg_init_machine;
     ac->allowed = &tcg_allowed;
+    ac->gdbstub_supported_sstep_flags = tcg_gdbstub_supported_sstep_flags;
 
     object_class_property_add_str(oc, "thread",
                                   tcg_get_thread,
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index 7d8fe475b3..a0755e6505 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -383,27 +383,13 @@ static void init_gdbserver_state(void)
     gdbserver_state.last_packet = g_byte_array_sized_new(MAX_PACKET_LENGTH + 4);
 
     /*
-     * In replay mode all events will come from the log and can't be
-     * suppressed otherwise we would break determinism. However as those
-     * events are tied to the number of executed instructions we won't see
-     * them occurring every time we single step.
-     */
-    if (replay_mode != REPLAY_MODE_NONE) {
-        gdbserver_state.supported_sstep_flags = SSTEP_ENABLE;
-    } else if (kvm_enabled()) {
-        gdbserver_state.supported_sstep_flags = kvm_get_supported_sstep_flags();
-    } else {
-        gdbserver_state.supported_sstep_flags =
-            SSTEP_ENABLE | SSTEP_NOIRQ | SSTEP_NOTIMER;
-    }
-
-    /*
-     * By default use no IRQs and no timers while single stepping so as to
-     * make single stepping like an ICE HW step.
+     * What single-step modes are supported is accelerator dependent.
+     * By default try to use no IRQs and no timers while single
+     * stepping so as to make single stepping like a typical ICE HW step.
      */
+    gdbserver_state.supported_sstep_flags = accel_supported_gdbstub_sstep_flags();
     gdbserver_state.sstep_flags = SSTEP_ENABLE | SSTEP_NOIRQ | SSTEP_NOTIMER;
     gdbserver_state.sstep_flags &= gdbserver_state.supported_sstep_flags;
-
 }
 
 #ifndef CONFIG_USER_ONLY
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH  v3 13/15] gdbstub: move breakpoint logic to accel ops
       [not found] <20220927141504.3886314-1-alex.bennee@linaro.org>
  2022-09-27 14:14 ` [PATCH v3 04/15] target/arm: ensure KVM traps set appropriate MemTxAttrs Alex Bennée
  2022-09-27 14:15 ` [PATCH v3 12/15] gdbstub: move sstep flags probing into AccelClass Alex Bennée
@ 2022-09-27 14:15 ` Alex Bennée
  2022-10-04 10:25   ` Mads Ynddal
  2022-09-27 14:15 ` [PATCH v3 14/15] gdbstub: move guest debug support check to ops Alex Bennée
  2022-09-27 14:15 ` [PATCH v3 15/15] accel/kvm: move kvm_update_guest_debug to inline stub Alex Bennée
  4 siblings, 1 reply; 11+ messages in thread
From: Alex Bennée @ 2022-09-27 14:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-arm, Alex Bennée, Richard Henderson, Mads Ynddal,
	Paolo Bonzini, Philippe Mathieu-Daudé,
	open list:Overall KVM CPUs

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>
Cc: Mads Ynddal <mads@ynddal.dk>
---
 accel/kvm/kvm-cpus.h       |   3 +
 gdbstub/internals.h        |  16 +++++
 include/sysemu/accel-ops.h |   6 ++
 include/sysemu/cpus.h      |   3 +
 include/sysemu/kvm.h       |   5 --
 accel/kvm/kvm-accel-ops.c  |   8 +++
 accel/kvm/kvm-all.c        |  24 +------
 accel/stubs/kvm-stub.c     |  16 -----
 accel/tcg/tcg-accel-ops.c  |  92 +++++++++++++++++++++++++++
 gdbstub/gdbstub.c          | 127 +++----------------------------------
 gdbstub/softmmu.c          |  42 ++++++++++++
 gdbstub/user.c             |  62 ++++++++++++++++++
 softmmu/cpus.c             |   7 ++
 gdbstub/meson.build        |   8 +++
 14 files changed, 259 insertions(+), 160 deletions(-)
 create mode 100644 gdbstub/internals.h
 create mode 100644 gdbstub/softmmu.c
 create mode 100644 gdbstub/user.c

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'))
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH  v3 14/15] gdbstub: move guest debug support check to ops
       [not found] <20220927141504.3886314-1-alex.bennee@linaro.org>
                   ` (2 preceding siblings ...)
  2022-09-27 14:15 ` [PATCH v3 13/15] gdbstub: move breakpoint logic to accel ops Alex Bennée
@ 2022-09-27 14:15 ` Alex Bennée
  2022-10-04 10:25   ` Mads Ynddal
  2022-09-27 14:15 ` [PATCH v3 15/15] accel/kvm: move kvm_update_guest_debug to inline stub Alex Bennée
  4 siblings, 1 reply; 11+ messages in thread
From: Alex Bennée @ 2022-09-27 14:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-arm, Alex Bennée, Philippe Mathieu-Daudé,
	Mads Ynddal, Paolo Bonzini, Richard Henderson,
	open list:Overall KVM CPUs

This removes the final hard coding of kvm_enabled() in gdbstub and
moves the check to an AccelOps.

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Cc: Mads Ynddal <mads@ynddal.dk>
---
 accel/kvm/kvm-cpus.h       | 1 +
 gdbstub/internals.h        | 1 +
 include/sysemu/accel-ops.h | 1 +
 include/sysemu/kvm.h       | 7 -------
 accel/kvm/kvm-accel-ops.c  | 1 +
 accel/kvm/kvm-all.c        | 6 ++++++
 accel/tcg/tcg-accel-ops.c  | 6 ++++++
 gdbstub/gdbstub.c          | 5 ++---
 gdbstub/softmmu.c          | 9 +++++++++
 gdbstub/user.c             | 6 ++++++
 10 files changed, 33 insertions(+), 10 deletions(-)

diff --git a/accel/kvm/kvm-cpus.h b/accel/kvm/kvm-cpus.h
index 33e435d62b..fd63fe6a59 100644
--- a/accel/kvm/kvm-cpus.h
+++ b/accel/kvm/kvm-cpus.h
@@ -18,6 +18,7 @@ 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);
+bool kvm_supports_guest_debug(void);
 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);
diff --git a/gdbstub/internals.h b/gdbstub/internals.h
index 41e2e72dbf..eabb0341d1 100644
--- a/gdbstub/internals.h
+++ b/gdbstub/internals.h
@@ -9,6 +9,7 @@
 #ifndef _INTERNALS_H_
 #define _INTERNALS_H_
 
+bool gdb_supports_guest_debug(void);
 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);
diff --git a/include/sysemu/accel-ops.h b/include/sysemu/accel-ops.h
index 86794ac273..8cc7996def 100644
--- a/include/sysemu/accel-ops.h
+++ b/include/sysemu/accel-ops.h
@@ -47,6 +47,7 @@ struct AccelOpsClass {
     int64_t (*get_elapsed_ticks)(void);
 
     /* gdbstub hooks */
+    bool (*supports_guest_debug)(void);
     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);
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index 21d3f1d01e..6e1bd01725 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -46,7 +46,6 @@ extern bool kvm_readonly_mem_allowed;
 extern bool kvm_direct_msi_allowed;
 extern bool kvm_ioeventfd_any_length_allowed;
 extern bool kvm_msi_use_devid;
-extern bool kvm_has_guest_debug;
 
 #define kvm_enabled()           (kvm_allowed)
 /**
@@ -168,11 +167,6 @@ extern bool kvm_has_guest_debug;
  */
 #define kvm_msi_devid_required() (kvm_msi_use_devid)
 
-/*
- * Does KVM support guest debugging
- */
-#define kvm_supports_guest_debug() (kvm_has_guest_debug)
-
 #else
 
 #define kvm_enabled()           (0)
@@ -190,7 +184,6 @@ extern bool kvm_has_guest_debug;
 #define kvm_direct_msi_enabled() (false)
 #define kvm_ioeventfd_any_length_enabled() (false)
 #define kvm_msi_devid_required() (false)
-#define kvm_supports_guest_debug() (false)
 
 #endif  /* CONFIG_KVM_IS_POSSIBLE */
 
diff --git a/accel/kvm/kvm-accel-ops.c b/accel/kvm/kvm-accel-ops.c
index 5c0e37514c..fbf4fe3497 100644
--- a/accel/kvm/kvm-accel-ops.c
+++ b/accel/kvm/kvm-accel-ops.c
@@ -99,6 +99,7 @@ static void kvm_accel_ops_class_init(ObjectClass *oc, void *data)
     ops->synchronize_pre_loadvm = kvm_cpu_synchronize_pre_loadvm;
 
 #ifdef KVM_CAP_SET_GUEST_DEBUG
+    ops->supports_guest_debug = kvm_supports_guest_debug;
     ops->insert_breakpoint = kvm_insert_breakpoint;
     ops->remove_breakpoint = kvm_remove_breakpoint;
     ops->remove_all_breakpoints = kvm_remove_all_breakpoints;
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index b8c734fe3a..6ebff6e5a6 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -3287,6 +3287,12 @@ int kvm_update_guest_debug(CPUState *cpu, unsigned long reinject_trap)
     return data.err;
 }
 
+bool kvm_supports_guest_debug(void)
+{
+    /* probed during kvm_init() */
+    return kvm_has_guest_debug;
+}
+
 int kvm_insert_breakpoint(CPUState *cpu, int type, hwaddr addr, hwaddr len)
 {
     struct kvm_sw_breakpoint *bp;
diff --git a/accel/tcg/tcg-accel-ops.c b/accel/tcg/tcg-accel-ops.c
index 965c2ad581..19cbf1db3a 100644
--- a/accel/tcg/tcg-accel-ops.c
+++ b/accel/tcg/tcg-accel-ops.c
@@ -93,6 +93,11 @@ void tcg_handle_interrupt(CPUState *cpu, int mask)
     }
 }
 
+static bool tcg_supports_guest_debug(void)
+{
+    return true;
+}
+
 /* Translate GDB watchpoint type to a flags value for cpu_watchpoint_* */
 static inline int xlat_gdb_type(CPUState *cpu, int gdbtype)
 {
@@ -198,6 +203,7 @@ static void tcg_accel_ops_init(AccelOpsClass *ops)
         }
     }
 
+    ops->supports_guest_debug = tcg_supports_guest_debug;
     ops->insert_breakpoint = tcg_insert_breakpoint;
     ops->remove_breakpoint = tcg_remove_breakpoint;
     ops->remove_all_breakpoints = tcg_remove_all_breakpoints;
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index ff9f3f9586..be88ca0d71 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -45,7 +45,6 @@
 
 #include "qemu/sockets.h"
 #include "sysemu/hw_accel.h"
-#include "sysemu/kvm.h"
 #include "sysemu/runstate.h"
 #include "semihosting/semihost.h"
 #include "exec/exec-all.h"
@@ -3447,8 +3446,8 @@ int gdbserver_start(const char *device)
         return -1;
     }
 
-    if (kvm_enabled() && !kvm_supports_guest_debug()) {
-        error_report("gdbstub: KVM doesn't support guest debugging");
+    if (!gdb_supports_guest_debug()) {
+        error_report("gdbstub: current accelerator doesn't support guest debugging");
         return -1;
     }
 
diff --git a/gdbstub/softmmu.c b/gdbstub/softmmu.c
index 4e73890379..f208c6cf15 100644
--- a/gdbstub/softmmu.c
+++ b/gdbstub/softmmu.c
@@ -15,6 +15,15 @@
 #include "sysemu/cpus.h"
 #include "internals.h"
 
+bool gdb_supports_guest_debug(void)
+{
+    const AccelOpsClass *ops = cpus_get_accel();
+    if (ops->supports_guest_debug) {
+        return ops->supports_guest_debug();
+    }
+    return false;
+}
+
 int gdb_breakpoint_insert(CPUState *cs, int type, hwaddr addr, hwaddr len)
 {
     const AccelOpsClass *ops = cpus_get_accel();
diff --git a/gdbstub/user.c b/gdbstub/user.c
index 42652b28a7..033e5fdd71 100644
--- a/gdbstub/user.c
+++ b/gdbstub/user.c
@@ -14,6 +14,12 @@
 #include "hw/core/cpu.h"
 #include "internals.h"
 
+bool gdb_supports_guest_debug(void)
+{
+    /* user-mode == TCG == supported */
+    return true;
+}
+
 int gdb_breakpoint_insert(CPUState *cs, int type, hwaddr addr, hwaddr len)
 {
     CPUState *cpu;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH  v3 15/15] accel/kvm: move kvm_update_guest_debug to inline stub
       [not found] <20220927141504.3886314-1-alex.bennee@linaro.org>
                   ` (3 preceding siblings ...)
  2022-09-27 14:15 ` [PATCH v3 14/15] gdbstub: move guest debug support check to ops Alex Bennée
@ 2022-09-27 14:15 ` Alex Bennée
  2022-10-19 10:53   ` Philippe Mathieu-Daudé
  4 siblings, 1 reply; 11+ messages in thread
From: Alex Bennée @ 2022-09-27 14:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-arm, Alex Bennée, Paolo Bonzini, open list:Overall KVM CPUs

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 include/sysemu/kvm.h   | 16 ++++++++++++++++
 accel/kvm/kvm-all.c    |  6 ------
 accel/stubs/kvm-stub.c |  5 -----
 3 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index 6e1bd01725..790d35ef78 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -247,7 +247,23 @@ int kvm_on_sigbus(int code, void *addr);
 
 void kvm_flush_coalesced_mmio_buffer(void);
 
+/**
+ * kvm_update_guest_debug(): ensure KVM debug structures updated
+ * @cs: the CPUState for this cpu
+ * @reinject_trap: KVM trap injection control
+ *
+ * There are usually per-arch specifics which will be handled by
+ * calling down to kvm_arch_update_guest_debug after the generic
+ * fields have been set.
+ */
+#ifdef KVM_CAP_SET_GUEST_DEBUG
 int kvm_update_guest_debug(CPUState *cpu, unsigned long reinject_trap);
+#else
+static inline int kvm_update_guest_debug(CPUState *cpu, unsigned long reinject_trap)
+{
+    return -EINVAL;
+}
+#endif
 
 /* internal API */
 
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 6ebff6e5a6..423fb1936f 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -3395,12 +3395,6 @@ void kvm_remove_all_breakpoints(CPUState *cpu)
     }
 }
 
-#else /* !KVM_CAP_SET_GUEST_DEBUG */
-
-static int kvm_update_guest_debug(CPUState *cpu, unsigned long reinject_trap)
-{
-    return -EINVAL;
-}
 #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 2d79333143..5d2dd8f351 100644
--- a/accel/stubs/kvm-stub.c
+++ b/accel/stubs/kvm-stub.c
@@ -46,11 +46,6 @@ int kvm_has_many_ioeventfds(void)
     return 0;
 }
 
-int kvm_update_guest_debug(CPUState *cpu, unsigned long reinject_trap)
-{
-    return -ENOSYS;
-}
-
 int kvm_on_sigbus_vcpu(CPUState *cpu, int code, void *addr)
 {
     return 1;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 04/15] target/arm: ensure KVM traps set appropriate MemTxAttrs
  2022-09-27 14:14 ` [PATCH v3 04/15] target/arm: ensure KVM traps set appropriate MemTxAttrs Alex Bennée
@ 2022-09-28 16:49   ` Richard Henderson
  2022-10-19 10:44   ` Philippe Mathieu-Daudé
  1 sibling, 0 replies; 11+ messages in thread
From: Richard Henderson @ 2022-09-28 16:49 UTC (permalink / raw)
  To: Alex Bennée, qemu-devel
  Cc: qemu-arm, Peter Maydell, Paolo Bonzini, open list:Overall KVM CPUs

On 9/27/22 07:14, Alex Bennée wrote:
> Although most KVM users will use the in-kernel GIC emulation it is
> perfectly possible not to. In this case we need to ensure the
> MemTxAttrs are correctly populated so the GIC can divine the source
> CPU of the operation.
> 
> Signed-off-by: Alex Bennée<alex.bennee@linaro.org>
> 
> ---
> v3
>    - new for v3
> ---
>   target/arm/kvm.c | 12 ++++++++----
>   1 file changed, 8 insertions(+), 4 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH  v3 12/15] gdbstub: move sstep flags probing into AccelClass
  2022-09-27 14:15 ` [PATCH v3 12/15] gdbstub: move sstep flags probing into AccelClass Alex Bennée
@ 2022-10-04 10:25   ` Mads Ynddal
  0 siblings, 0 replies; 11+ messages in thread
From: Mads Ynddal @ 2022-10-04 10:25 UTC (permalink / raw)
  To: Alex Bennée
  Cc: qemu-devel, open list:ARM cores, Richard Henderson,
	Philippe Mathieu-Daudé,
	Paolo Bonzini, open list:Overall KVM CPUs


> The support of single-stepping is very much dependent on support from
> the accelerator we are using. To avoid special casing in gdbstub move
> the probing out to an AccelClass function so future accelerators can
> put their code there.
> 
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Cc: Mads Ynddal <mads@ynddal.dk>
> ---
> include/qemu/accel.h | 12 ++++++++++++
> include/sysemu/kvm.h |  8 --------
> accel/accel-common.c | 10 ++++++++++
> accel/kvm/kvm-all.c  | 14 +++++++++++++-
> accel/tcg/tcg-all.c  | 17 +++++++++++++++++
> gdbstub/gdbstub.c    | 22 ++++------------------
> 6 files changed, 56 insertions(+), 27 deletions(-)

Reviewed-by: Mads Ynddal <mads@ynddal.dk>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH  v3 13/15] gdbstub: move breakpoint logic to accel ops
  2022-09-27 14:15 ` [PATCH v3 13/15] gdbstub: move breakpoint logic to accel ops Alex Bennée
@ 2022-10-04 10:25   ` Mads Ynddal
  0 siblings, 0 replies; 11+ messages in thread
From: Mads Ynddal @ 2022-10-04 10:25 UTC (permalink / raw)
  To: Alex Bennée
  Cc: qemu-devel, qemu-arm, Richard Henderson, Paolo Bonzini,
	Philippe Mathieu-Daudé,
	open list:Overall KVM CPUs


> 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>
> Cc: Mads Ynddal <mads@ynddal.dk>
> ---
> accel/kvm/kvm-cpus.h       |   3 +
> gdbstub/internals.h        |  16 +++++
> include/sysemu/accel-ops.h |   6 ++
> include/sysemu/cpus.h      |   3 +
> include/sysemu/kvm.h       |   5 --
> accel/kvm/kvm-accel-ops.c  |   8 +++
> accel/kvm/kvm-all.c        |  24 +------
> accel/stubs/kvm-stub.c     |  16 -----
> accel/tcg/tcg-accel-ops.c  |  92 +++++++++++++++++++++++++++
> gdbstub/gdbstub.c          | 127 +++----------------------------------
> gdbstub/softmmu.c          |  42 ++++++++++++
> gdbstub/user.c             |  62 ++++++++++++++++++
> softmmu/cpus.c             |   7 ++
> gdbstub/meson.build        |   8 +++
> 14 files changed, 259 insertions(+), 160 deletions(-)

Reviewed-by: Mads Ynddal <mads@ynddal.dk>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH  v3 14/15] gdbstub: move guest debug support check to ops
  2022-09-27 14:15 ` [PATCH v3 14/15] gdbstub: move guest debug support check to ops Alex Bennée
@ 2022-10-04 10:25   ` Mads Ynddal
  0 siblings, 0 replies; 11+ messages in thread
From: Mads Ynddal @ 2022-10-04 10:25 UTC (permalink / raw)
  To: Alex Bennée
  Cc: qemu-devel, open list:ARM cores, Philippe Mathieu-Daudé,
	Paolo Bonzini, Richard Henderson, open list:Overall KVM CPUs


> This removes the final hard coding of kvm_enabled() in gdbstub and
> moves the check to an AccelOps.
> 
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Cc: Mads Ynddal <mads@ynddal.dk>
> ---
> accel/kvm/kvm-cpus.h       | 1 +
> gdbstub/internals.h        | 1 +
> include/sysemu/accel-ops.h | 1 +
> include/sysemu/kvm.h       | 7 -------
> accel/kvm/kvm-accel-ops.c  | 1 +
> accel/kvm/kvm-all.c        | 6 ++++++
> accel/tcg/tcg-accel-ops.c  | 6 ++++++
> gdbstub/gdbstub.c          | 5 ++---
> gdbstub/softmmu.c          | 9 +++++++++
> gdbstub/user.c             | 6 ++++++
> 10 files changed, 33 insertions(+), 10 deletions(-)

Reviewed-by: Mads Ynddal <mads@ynddal.dk>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 04/15] target/arm: ensure KVM traps set appropriate MemTxAttrs
  2022-09-27 14:14 ` [PATCH v3 04/15] target/arm: ensure KVM traps set appropriate MemTxAttrs Alex Bennée
  2022-09-28 16:49   ` Richard Henderson
@ 2022-10-19 10:44   ` Philippe Mathieu-Daudé
  1 sibling, 0 replies; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-10-19 10:44 UTC (permalink / raw)
  To: Alex Bennée, qemu-devel
  Cc: qemu-arm, Peter Maydell, Paolo Bonzini, open list:Overall KVM CPUs

On 27/9/22 16:14, Alex Bennée wrote:
> Although most KVM users will use the in-kernel GIC emulation it is
> perfectly possible not to. In this case we need to ensure the
> MemTxAttrs are correctly populated so the GIC can divine the source
> CPU of the operation.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> 
> ---
> v3
>    - new for v3
> ---
>   target/arm/kvm.c | 12 ++++++++----
>   1 file changed, 8 insertions(+), 4 deletions(-)

>   void kvm_arm_vm_state_change(void *opaque, bool running, RunState state)
> @@ -1003,6 +1004,10 @@ int kvm_arch_fixup_msi_route(struct kvm_irq_routing_entry *route,
>       hwaddr xlat, len, doorbell_gpa;
>       MemoryRegionSection mrs;
>       MemoryRegion *mr;
> +    MemTxAttrs attrs = {
> +        .requester_type = MTRT_PCI,
> +        .requester_id = pci_requester_id(dev)
> +    };

Can we add a MEMTXATTRS_PCI() macro similar to MEMTXATTRS_CPU()?

   #define MEMTXATTRS_PCI(pci_dev) ((MemTxAttrs) \
                             {.requester_type = MTRT_PCI, \
                              .requester_id = pci_requester_id(pci_dev)})

So here we can use:

   MemTxAttrs attrs = MEMTXATTRS_PCI(dev);

> @@ -1012,8 +1017,7 @@ int kvm_arch_fixup_msi_route(struct kvm_irq_routing_entry *route,
>   
>       RCU_READ_LOCK_GUARD();
>   
> -    mr = address_space_translate(as, address, &xlat, &len, true,
> -                                 MEMTXATTRS_UNSPECIFIED);
> +    mr = address_space_translate(as, address, &xlat, &len, true, attrs);
>   
>       if (!mr) {
>           return 1;

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 15/15] accel/kvm: move kvm_update_guest_debug to inline stub
  2022-09-27 14:15 ` [PATCH v3 15/15] accel/kvm: move kvm_update_guest_debug to inline stub Alex Bennée
@ 2022-10-19 10:53   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-10-19 10:53 UTC (permalink / raw)
  To: Alex Bennée, qemu-devel
  Cc: qemu-arm, Paolo Bonzini, open list:Overall KVM CPUs

On 27/9/22 16:15, Alex Bennée wrote:
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   include/sysemu/kvm.h   | 16 ++++++++++++++++
>   accel/kvm/kvm-all.c    |  6 ------
>   accel/stubs/kvm-stub.c |  5 -----
>   3 files changed, 16 insertions(+), 11 deletions(-)
> 
> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
> index 6e1bd01725..790d35ef78 100644
> --- a/include/sysemu/kvm.h
> +++ b/include/sysemu/kvm.h
> @@ -247,7 +247,23 @@ int kvm_on_sigbus(int code, void *addr);
>   
>   void kvm_flush_coalesced_mmio_buffer(void);
>   
> +/**
> + * kvm_update_guest_debug(): ensure KVM debug structures updated
> + * @cs: the CPUState for this cpu
> + * @reinject_trap: KVM trap injection control
> + *
> + * There are usually per-arch specifics which will be handled by
> + * calling down to kvm_arch_update_guest_debug after the generic
> + * fields have been set.
> + */
> +#ifdef KVM_CAP_SET_GUEST_DEBUG
>   int kvm_update_guest_debug(CPUState *cpu, unsigned long reinject_trap);
> +#else
> +static inline int kvm_update_guest_debug(CPUState *cpu, unsigned long reinject_trap)
> +{
> +    return -EINVAL;

Wouldn't -ENOSYS make more sense in this case?

> +}
> +#endif
>   
>   /* internal API */
>   
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index 6ebff6e5a6..423fb1936f 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -3395,12 +3395,6 @@ void kvm_remove_all_breakpoints(CPUState *cpu)
>       }
>   }
>   
> -#else /* !KVM_CAP_SET_GUEST_DEBUG */
> -
> -static int kvm_update_guest_debug(CPUState *cpu, unsigned long reinject_trap)
> -{
> -    return -EINVAL;
> -}
>   #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 2d79333143..5d2dd8f351 100644
> --- a/accel/stubs/kvm-stub.c
> +++ b/accel/stubs/kvm-stub.c
> @@ -46,11 +46,6 @@ int kvm_has_many_ioeventfds(void)
>       return 0;
>   }
>   
> -int kvm_update_guest_debug(CPUState *cpu, unsigned long reinject_trap)
> -{
> -    return -ENOSYS;
> -}
> -
>   int kvm_on_sigbus_vcpu(CPUState *cpu, int code, void *addr)
>   {
>       return 1;


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2022-10-19 11:45 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20220927141504.3886314-1-alex.bennee@linaro.org>
2022-09-27 14:14 ` [PATCH v3 04/15] target/arm: ensure KVM traps set appropriate MemTxAttrs Alex Bennée
2022-09-28 16:49   ` Richard Henderson
2022-10-19 10:44   ` Philippe Mathieu-Daudé
2022-09-27 14:15 ` [PATCH v3 12/15] gdbstub: move sstep flags probing into AccelClass Alex Bennée
2022-10-04 10:25   ` Mads Ynddal
2022-09-27 14:15 ` [PATCH v3 13/15] gdbstub: move breakpoint logic to accel ops Alex Bennée
2022-10-04 10:25   ` Mads Ynddal
2022-09-27 14:15 ` [PATCH v3 14/15] gdbstub: move guest debug support check to ops Alex Bennée
2022-10-04 10:25   ` Mads Ynddal
2022-09-27 14:15 ` [PATCH v3 15/15] accel/kvm: move kvm_update_guest_debug to inline stub Alex Bennée
2022-10-19 10:53   ` Philippe Mathieu-Daudé

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).