[PATCH 0/5] accel/tcg: Restrict WatchPoint API to TCG

[PATCH 0/5] accel/tcg: Restrict WatchPoint API to TCG

[Philippe Mathieu-Daudé]

Hi,

This series is related to single binary, where we want
to build compilation units once. Headers using the
'CONFIG_USER_ONLY' definition are "poisoned", except if
we duplicate the meson source set, in that case we define
COMPILING_SYSTEM_VS_USER and CONFIG_USER_ONLY is no more
poisoned.

Looking at the watchpoint API, CONFIG_USER_ONLY is only
used to avoid stubs, so it can easily be reworked to avoid
the need of duplicated source set.

Since here, we also restrict the API to TCG, and unify it
in a single header: "accel/tcg/watchpoint.h", since it is
distinct to the BreakPoint API which can be used by hardware
accelerators.

Philippe Mathieu-Daudé (5):
  target/i386: Restrict WatchPoint API to TCG
  target/ppc: Restrict WatchPoint API to TCG
  target/s390x: Restrict WatchPoint API to TCG
  accel/tcg: Un-inline WatchPoint API user-emulation stubs
  accel/tcg: Unify watchpoint API

 MAINTAINERS                         |   1 -
 include/accel/tcg/cpu-ops.h         |  45 +-----------
 include/accel/tcg/watchpoint.h      |  57 ++++++++++++++++
 include/exec/breakpoint.h           |  10 ---
 include/exec/watchpoint.h           |  41 -----------
 include/hw/core/cpu.h               |   3 +-
 target/arm/internals.h              |   2 +-
 target/ppc/internal.h               |   2 +-
 target/riscv/debug.h                |   2 +-
 target/s390x/s390x-internal.h       |   1 -
 target/s390x/tcg/tcg_s390x.h        |   4 ++
 accel/tcg/cputlb.c                  |   1 +
 accel/tcg/tcg-accel-ops.c           |   2 +-
 accel/tcg/user-exec-stub.c          |  32 +++++++++
 accel/tcg/watchpoint.c              |  83 +++++++++++++++++++++-
 system/watchpoint.c                 | 102 ----------------------------
 target/arm/debug_helper.c           |   2 +-
 target/arm/tcg/mte_helper.c         |   2 +-
 target/arm/tcg/sve_helper.c         |   2 +-
 target/i386/cpu.c                   |   6 +-
 target/i386/machine.c               |   2 +-
 target/i386/tcg/system/bpt_helper.c |   2 +-
 target/ppc/cpu.c                    |  83 +---------------------
 target/ppc/cpu_init.c               |   2 +-
 target/ppc/watchpoint.c             |  93 +++++++++++++++++++++++++
 target/riscv/cpu_helper.c           |   2 +-
 target/riscv/debug.c                |   2 +-
 target/s390x/cpu.c                  |  30 ++++----
 target/s390x/helper.c               |  40 +----------
 target/s390x/tcg/debug.c            |  53 +++++++++++++++
 target/s390x/tcg/excp_helper.c      |   2 +-
 target/s390x/tcg/mem_helper.c       |   1 +
 target/xtensa/dbg_helper.c          |   2 +-
 system/meson.build                  |   1 -
 target/ppc/meson.build              |   1 +
 target/s390x/tcg/meson.build        |   3 +
 36 files changed, 366 insertions(+), 353 deletions(-)
 create mode 100644 include/accel/tcg/watchpoint.h
 delete mode 100644 include/exec/watchpoint.h
 delete mode 100644 system/watchpoint.c
 create mode 100644 target/ppc/watchpoint.c
 create mode 100644 target/s390x/tcg/debug.c

-- 
2.52.0

[PATCH 1/5] target/i386: Restrict WatchPoint API to TCG

[Philippe Mathieu-Daudé]

Watchpoints are specific to the TCG accelerator. Restrict
the cpu_watchpoint_remove_all() call.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/i386/cpu.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 37803cd7249..9831e2bc210 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -9184,7 +9184,9 @@ static void x86_cpu_reset_hold(Object *obj, ResetType type)
     env->dr[6] = DR6_FIXED_1;
     env->dr[7] = DR7_FIXED_1;
     cpu_breakpoint_remove_all(cs, BP_CPU);
-    cpu_watchpoint_remove_all(cs, BP_CPU);
+    if (tcg_enabled()) {
+        cpu_watchpoint_remove_all(cs, BP_CPU);
+    }
 
     cr4 = 0;
     xcr0 = XSTATE_FP_MASK;
-- 
2.52.0

[PATCH 2/5] target/ppc: Restrict WatchPoint API to TCG

[Philippe Mathieu-Daudé]

Watchpoints are specific to the TCG accelerator. Since the
Data Address Watchpoint helpers are only called from
translated code, move them to a new 'watchpoint.c' file,
specific to TCG. Thus restricting the WatchPoint API to TCG.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/ppc/cpu.c        | 81 +----------------------------------
 target/ppc/watchpoint.c | 93 +++++++++++++++++++++++++++++++++++++++++
 target/ppc/meson.build  |  1 +
 3 files changed, 96 insertions(+), 79 deletions(-)
 create mode 100644 target/ppc/watchpoint.c

diff --git a/target/ppc/cpu.c b/target/ppc/cpu.c
index 4d8faaddee2..9cb3f00aa88 100644
--- a/target/ppc/cpu.c
+++ b/target/ppc/cpu.c
@@ -131,85 +131,8 @@ void ppc_store_ciabr(CPUPPCState *env, target_ulong val)
     ppc_update_ciabr(env);
 }
 
-void ppc_update_daw(CPUPPCState *env, int rid)
-{
-    CPUState *cs = env_cpu(env);
-    int spr_dawr = rid ? SPR_DAWR1 : SPR_DAWR0;
-    int spr_dawrx = rid ? SPR_DAWRX1 : SPR_DAWRX0;
-    target_ulong deaw = env->spr[spr_dawr] & PPC_BITMASK(0, 60);
-    uint32_t dawrx = env->spr[spr_dawrx];
-    int mrd = extract32(dawrx, PPC_BIT_NR(48), 54 - 48);
-    bool dw = extract32(dawrx, PPC_BIT_NR(57), 1);
-    bool dr = extract32(dawrx, PPC_BIT_NR(58), 1);
-    bool hv = extract32(dawrx, PPC_BIT_NR(61), 1);
-    bool sv = extract32(dawrx, PPC_BIT_NR(62), 1);
-    bool pr = extract32(dawrx, PPC_BIT_NR(62), 1);
-    vaddr len;
-    int flags;
-
-    if (env->dawr_watchpoint[rid]) {
-        cpu_watchpoint_remove_by_ref(cs, env->dawr_watchpoint[rid]);
-        env->dawr_watchpoint[rid] = NULL;
-    }
-
-    if (!dr && !dw) {
-        return;
-    }
-
-    if (!hv && !sv && !pr) {
-        return;
-    }
-
-    len = (mrd + 1) * 8;
-    flags = BP_CPU | BP_STOP_BEFORE_ACCESS;
-    if (dr) {
-        flags |= BP_MEM_READ;
-    }
-    if (dw) {
-        flags |= BP_MEM_WRITE;
-    }
-
-    cpu_watchpoint_insert(cs, deaw, len, flags, &env->dawr_watchpoint[rid]);
-}
-
-void ppc_store_dawr0(CPUPPCState *env, target_ulong val)
-{
-    env->spr[SPR_DAWR0] = val;
-    ppc_update_daw(env, 0);
-}
-
-static void ppc_store_dawrx(CPUPPCState *env, uint32_t val, int rid)
-{
-    int hrammc = extract32(val, PPC_BIT_NR(56), 1);
-
-    if (hrammc) {
-        /* This might be done with a second watchpoint at the xor of DEAW[0] */
-        qemu_log_mask(LOG_UNIMP, "%s: DAWRX%d[HRAMMC] is unimplemented\n",
-                      __func__, rid);
-    }
-
-    env->spr[rid ? SPR_DAWRX1 : SPR_DAWRX0] = val;
-    ppc_update_daw(env, rid);
-}
-
-void ppc_store_dawrx0(CPUPPCState *env, uint32_t val)
-{
-    ppc_store_dawrx(env, val, 0);
-}
-
-void ppc_store_dawr1(CPUPPCState *env, target_ulong val)
-{
-    env->spr[SPR_DAWR1] = val;
-    ppc_update_daw(env, 1);
-}
-
-void ppc_store_dawrx1(CPUPPCState *env, uint32_t val)
-{
-    ppc_store_dawrx(env, val, 1);
-}
-
-#endif
-#endif
+#endif /* TARGET_PPC64 */
+#endif /* !CONFIG_USER_ONLY */
 
 static inline void fpscr_set_rounding_mode(CPUPPCState *env)
 {
diff --git a/target/ppc/watchpoint.c b/target/ppc/watchpoint.c
new file mode 100644
index 00000000000..c71dd4550b7
--- /dev/null
+++ b/target/ppc/watchpoint.c
@@ -0,0 +1,93 @@
+/*
+ * PowerPC watchpoint routines for QEMU
+ *
+ * Copyright (c) 2017 Nikunj A Dadhania, IBM Corporation.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "exec/log.h"
+#include "accel/tcg/watchpoint.h"
+#include "target/ppc/cpu.h"
+
+#if defined(TARGET_PPC64)
+
+void ppc_update_daw(CPUPPCState *env, int rid)
+{
+    CPUState *cs = env_cpu(env);
+    int spr_dawr = rid ? SPR_DAWR1 : SPR_DAWR0;
+    int spr_dawrx = rid ? SPR_DAWRX1 : SPR_DAWRX0;
+    target_ulong deaw = env->spr[spr_dawr] & PPC_BITMASK(0, 60);
+    uint32_t dawrx = env->spr[spr_dawrx];
+    int mrd = extract32(dawrx, PPC_BIT_NR(48), 54 - 48);
+    bool dw = extract32(dawrx, PPC_BIT_NR(57), 1);
+    bool dr = extract32(dawrx, PPC_BIT_NR(58), 1);
+    bool hv = extract32(dawrx, PPC_BIT_NR(61), 1);
+    bool sv = extract32(dawrx, PPC_BIT_NR(62), 1);
+    bool pr = extract32(dawrx, PPC_BIT_NR(62), 1);
+    vaddr len;
+    int flags;
+
+    if (env->dawr_watchpoint[rid]) {
+        cpu_watchpoint_remove_by_ref(cs, env->dawr_watchpoint[rid]);
+        env->dawr_watchpoint[rid] = NULL;
+    }
+
+    if (!dr && !dw) {
+        return;
+    }
+
+    if (!hv && !sv && !pr) {
+        return;
+    }
+
+    len = (mrd + 1) * 8;
+    flags = BP_CPU | BP_STOP_BEFORE_ACCESS;
+    if (dr) {
+        flags |= BP_MEM_READ;
+    }
+    if (dw) {
+        flags |= BP_MEM_WRITE;
+    }
+
+    cpu_watchpoint_insert(cs, deaw, len, flags, &env->dawr_watchpoint[rid]);
+}
+
+void ppc_store_dawr0(CPUPPCState *env, target_ulong val)
+{
+    env->spr[SPR_DAWR0] = val;
+    ppc_update_daw(env, 0);
+}
+
+static void ppc_store_dawrx(CPUPPCState *env, uint32_t val, int rid)
+{
+    int hrammc = extract32(val, PPC_BIT_NR(56), 1);
+
+    if (hrammc) {
+        /* This might be done with a second watchpoint at the xor of DEAW[0] */
+        qemu_log_mask(LOG_UNIMP, "%s: DAWRX%d[HRAMMC] is unimplemented\n",
+                      __func__, rid);
+    }
+
+    env->spr[rid ? SPR_DAWRX1 : SPR_DAWRX0] = val;
+    ppc_update_daw(env, rid);
+}
+
+void ppc_store_dawrx0(CPUPPCState *env, uint32_t val)
+{
+    ppc_store_dawrx(env, val, 0);
+}
+
+void ppc_store_dawr1(CPUPPCState *env, target_ulong val)
+{
+    env->spr[SPR_DAWR1] = val;
+    ppc_update_daw(env, 1);
+}
+
+void ppc_store_dawrx1(CPUPPCState *env, uint32_t val)
+{
+    ppc_store_dawrx(env, val, 1);
+}
+
+#endif
diff --git a/target/ppc/meson.build b/target/ppc/meson.build
index 8eed1fa40ca..d354c3240a2 100644
--- a/target/ppc/meson.build
+++ b/target/ppc/meson.build
@@ -43,6 +43,7 @@ ppc_system_ss.add(files(
   'ppc-qmp-cmds.c',
 ))
 ppc_system_ss.add(when: 'CONFIG_TCG', if_true: files(
+  'watchpoint.c',
   'mmu_helper.c',
 ), if_false: files(
   'tcg-stub.c',
-- 
2.52.0

[PATCH 3/5] target/s390x: Restrict WatchPoint API to TCG

[Philippe Mathieu-Daudé]

By inverting the 'tcg_enabled()' check in s390_cpu_set_psw()
we can let the compiler elide the s390_cpu_recompute_watchpoints()
call when TCG is not available. Move it to a TCG specific
file to avoid compiling dead code on KVM. This restricts the
WatchPoint API calls to TCG.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/s390x/s390x-internal.h |  1 -
 target/s390x/tcg/tcg_s390x.h  |  4 +++
 target/s390x/cpu.c            | 30 ++++++++++----------
 target/s390x/helper.c         | 38 -------------------------
 target/s390x/tcg/debug.c      | 53 +++++++++++++++++++++++++++++++++++
 target/s390x/tcg/meson.build  |  3 ++
 6 files changed, 75 insertions(+), 54 deletions(-)
 create mode 100644 target/s390x/tcg/debug.c

diff --git a/target/s390x/s390x-internal.h b/target/s390x/s390x-internal.h
index 9691366ec91..c4cd17d4d7d 100644
--- a/target/s390x/s390x-internal.h
+++ b/target/s390x/s390x-internal.h
@@ -317,7 +317,6 @@ void s390_cpu_gdb_init(CPUState *cs);
 void s390_cpu_dump_state(CPUState *cpu, FILE *f, int flags);
 void do_restart_interrupt(CPUS390XState *env);
 #ifndef CONFIG_USER_ONLY
-void s390_cpu_recompute_watchpoints(CPUState *cs);
 void s390x_tod_timer(void *opaque);
 void s390x_cpu_timer(void *opaque);
 void s390_handle_wait(S390CPU *cpu);
diff --git a/target/s390x/tcg/tcg_s390x.h b/target/s390x/tcg/tcg_s390x.h
index 78558912f99..33f26f26c0f 100644
--- a/target/s390x/tcg/tcg_s390x.h
+++ b/target/s390x/tcg/tcg_s390x.h
@@ -21,4 +21,8 @@ G_NORETURN void tcg_s390_data_exception(CPUS390XState *env, uint32_t dxc,
 G_NORETURN void tcg_s390_vector_exception(CPUS390XState *env, uint32_t vxc,
                                           uintptr_t ra);
 
+#ifndef CONFIG_USER_ONLY
+void s390_cpu_recompute_watchpoints(CPUState *cs);
+#endif
+
 #endif /* TCG_S390X_H */
diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index 6c4198eb1b1..f68b288e364 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -40,6 +40,7 @@
 #include "system/reset.h"
 #endif
 #include "hw/s390x/cpu-topology.h"
+#include "tcg/tcg_s390x.h"
 
 #define CR0_RESET       0xE0UL
 #define CR14_RESET      0xC2000000UL;
@@ -74,26 +75,25 @@ void s390_cpu_set_psw(CPUS390XState *env, uint64_t mask, uint64_t addr)
     env->psw.mask = mask;
 
     /* KVM will handle all WAITs and trigger a WAIT exit on disabled_wait */
-    if (!tcg_enabled()) {
-        return;
-    }
-    env->cc_op = (mask >> 44) & 3;
+    if (tcg_enabled()) {
+        env->cc_op = (mask >> 44) & 3;
 
 #ifndef CONFIG_USER_ONLY
-    if (is_early_exception_psw(mask, addr)) {
-        env->int_pgm_ilen = 0;
-        trigger_pgm_exception(env, PGM_SPECIFICATION);
-        return;
-    }
+        if (is_early_exception_psw(mask, addr)) {
+            env->int_pgm_ilen = 0;
+            trigger_pgm_exception(env, PGM_SPECIFICATION);
+            return;
+        }
 
-    if ((old_mask ^ mask) & PSW_MASK_PER) {
-        s390_cpu_recompute_watchpoints(env_cpu(env));
-    }
+        if ((old_mask ^ mask) & PSW_MASK_PER) {
+            s390_cpu_recompute_watchpoints(env_cpu(env));
+        }
 
-    if (mask & PSW_MASK_WAIT) {
-        s390_handle_wait(env_archcpu(env));
-    }
+        if (mask & PSW_MASK_WAIT) {
+            s390_handle_wait(env_archcpu(env));
+        }
 #endif
+    }
 }
 
 uint64_t s390_cpu_get_psw_mask(CPUS390XState *env)
diff --git a/target/s390x/helper.c b/target/s390x/helper.c
index 184428c6d9d..8d1e03f6768 100644
--- a/target/s390x/helper.c
+++ b/target/s390x/helper.c
@@ -143,41 +143,3 @@ void do_restart_interrupt(CPUS390XState *env)
 
     s390_cpu_set_psw(env, mask, addr);
 }
-
-void s390_cpu_recompute_watchpoints(CPUState *cs)
-{
-    const int wp_flags = BP_CPU | BP_MEM_WRITE | BP_STOP_BEFORE_ACCESS;
-    CPUS390XState *env = cpu_env(cs);
-
-    /* We are called when the watchpoints have changed. First
-       remove them all.  */
-    cpu_watchpoint_remove_all(cs, BP_CPU);
-
-    /* Return if PER is not enabled */
-    if (!(env->psw.mask & PSW_MASK_PER)) {
-        return;
-    }
-
-    /* Return if storage-alteration event is not enabled.  */
-    if (!(env->cregs[9] & PER_CR9_EVENT_STORE)) {
-        return;
-    }
-
-    if (env->cregs[10] == 0 && env->cregs[11] == -1LL) {
-        /* We can't create a watchoint spanning the whole memory range, so
-           split it in two parts.   */
-        cpu_watchpoint_insert(cs, 0, 1ULL << 63, wp_flags, NULL);
-        cpu_watchpoint_insert(cs, 1ULL << 63, 1ULL << 63, wp_flags, NULL);
-    } else if (env->cregs[10] > env->cregs[11]) {
-        /* The address range loops, create two watchpoints.  */
-        cpu_watchpoint_insert(cs, env->cregs[10], -env->cregs[10],
-                              wp_flags, NULL);
-        cpu_watchpoint_insert(cs, 0, env->cregs[11] + 1, wp_flags, NULL);
-
-    } else {
-        /* Default case, create a single watchpoint.  */
-        cpu_watchpoint_insert(cs, env->cregs[10],
-                              env->cregs[11] - env->cregs[10] + 1,
-                              wp_flags, NULL);
-    }
-}
diff --git a/target/s390x/tcg/debug.c b/target/s390x/tcg/debug.c
new file mode 100644
index 00000000000..12ae95d4fe8
--- /dev/null
+++ b/target/s390x/tcg/debug.c
@@ -0,0 +1,53 @@
+/*
+ * QEMU S/390 debug routines
+ *
+ * Copyright (c) 2009 Ulrich Hecht
+ * Copyright (c) 2011 Alexander Graf
+ * Copyright (c) 2012 SUSE LINUX Products GmbH
+ * Copyright (c) 2012 IBM Corp.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "exec/watchpoint.h"
+#include "target/s390x/cpu.h"
+#include "tcg_s390x.h"
+
+void s390_cpu_recompute_watchpoints(CPUState *cs)
+{
+    const int wp_flags = BP_CPU | BP_MEM_WRITE | BP_STOP_BEFORE_ACCESS;
+    CPUS390XState *env = cpu_env(cs);
+
+    /* We are called when the watchpoints have changed. First
+       remove them all.  */
+    cpu_watchpoint_remove_all(cs, BP_CPU);
+
+    /* Return if PER is not enabled */
+    if (!(env->psw.mask & PSW_MASK_PER)) {
+        return;
+    }
+
+    /* Return if storage-alteration event is not enabled.  */
+    if (!(env->cregs[9] & PER_CR9_EVENT_STORE)) {
+        return;
+    }
+
+    if (env->cregs[10] == 0 && env->cregs[11] == -1LL) {
+        /* We can't create a watchoint spanning the whole memory range, so
+           split it in two parts.   */
+        cpu_watchpoint_insert(cs, 0, 1ULL << 63, wp_flags, NULL);
+        cpu_watchpoint_insert(cs, 1ULL << 63, 1ULL << 63, wp_flags, NULL);
+    } else if (env->cregs[10] > env->cregs[11]) {
+        /* The address range loops, create two watchpoints.  */
+        cpu_watchpoint_insert(cs, env->cregs[10], -env->cregs[10],
+                              wp_flags, NULL);
+        cpu_watchpoint_insert(cs, 0, env->cregs[11] + 1, wp_flags, NULL);
+
+    } else {
+        /* Default case, create a single watchpoint.  */
+        cpu_watchpoint_insert(cs, env->cregs[10],
+                              env->cregs[11] - env->cregs[10] + 1,
+                              wp_flags, NULL);
+    }
+}
diff --git a/target/s390x/tcg/meson.build b/target/s390x/tcg/meson.build
index ee4e8fec77c..515cb8b473d 100644
--- a/target/s390x/tcg/meson.build
+++ b/target/s390x/tcg/meson.build
@@ -12,3 +12,6 @@ s390x_ss.add(when: 'CONFIG_TCG', if_true: files(
   'vec_int_helper.c',
   'vec_string_helper.c',
 ))
+s390x_system_ss.add(when: 'CONFIG_TCG', if_true: files(
+  'debug.c',
+))
-- 
2.52.0

[PATCH 4/5] accel/tcg: Un-inline WatchPoint API user-emulation stubs

[Philippe Mathieu-Daudé]

Currently we can not build files including "exec/watchpoint.h"
as meson common objects because the CONFIG_USER_ONLY definition
is poisoned. We can easily fix that by un-inlining the
user-emulation stubs.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/accel/tcg/cpu-ops.h | 17 -----------------
 include/exec/watchpoint.h   | 23 -----------------------
 accel/tcg/user-exec-stub.c  | 33 +++++++++++++++++++++++++++++++++
 3 files changed, 33 insertions(+), 40 deletions(-)

diff --git a/include/accel/tcg/cpu-ops.h b/include/accel/tcg/cpu-ops.h
index dd8ea300168..5950cdcaab1 100644
--- a/include/accel/tcg/cpu-ops.h
+++ b/include/accel/tcg/cpu-ops.h
@@ -281,21 +281,6 @@ struct TCGCPUOps {
 #endif /* !CONFIG_USER_ONLY */
 };
 
-#if defined(CONFIG_USER_ONLY)
-
-static inline void cpu_check_watchpoint(CPUState *cpu, vaddr addr, vaddr len,
-                                        MemTxAttrs atr, int fl, uintptr_t ra)
-{
-}
-
-static inline int cpu_watchpoint_address_matches(CPUState *cpu,
-                                                 vaddr addr, vaddr len)
-{
-    return 0;
-}
-
-#else
-
 /**
  * cpu_check_watchpoint:
  * @cpu: cpu context
@@ -328,6 +313,4 @@ int cpu_watchpoint_address_matches(CPUState *cpu, vaddr addr, vaddr len);
 vaddr cpu_pointer_wrap_notreached(CPUState *, int, vaddr, vaddr);
 vaddr cpu_pointer_wrap_uint32(CPUState *, int, vaddr, vaddr);
 
-#endif
-
 #endif /* TCG_CPU_OPS_H */
diff --git a/include/exec/watchpoint.h b/include/exec/watchpoint.h
index 4b6668826c7..c4d069425ba 100644
--- a/include/exec/watchpoint.h
+++ b/include/exec/watchpoint.h
@@ -8,34 +8,11 @@
 #ifndef EXEC_WATCHPOINT_H
 #define EXEC_WATCHPOINT_H
 
-#if defined(CONFIG_USER_ONLY)
-static inline int cpu_watchpoint_insert(CPUState *cpu, vaddr addr, vaddr len,
-                                        int flags, CPUWatchpoint **watchpoint)
-{
-    return -ENOSYS;
-}
-
-static inline int cpu_watchpoint_remove(CPUState *cpu, vaddr addr,
-                                        vaddr len, int flags)
-{
-    return -ENOSYS;
-}
-
-static inline void cpu_watchpoint_remove_by_ref(CPUState *cpu,
-                                                CPUWatchpoint *wp)
-{
-}
-
-static inline void cpu_watchpoint_remove_all(CPUState *cpu, int mask)
-{
-}
-#else
 int cpu_watchpoint_insert(CPUState *cpu, vaddr addr, vaddr len,
                           int flags, CPUWatchpoint **watchpoint);
 int cpu_watchpoint_remove(CPUState *cpu, vaddr addr,
                           vaddr len, int flags);
 void cpu_watchpoint_remove_by_ref(CPUState *cpu, CPUWatchpoint *watchpoint);
 void cpu_watchpoint_remove_all(CPUState *cpu, int mask);
-#endif
 
 #endif /* EXEC_WATCHPOINT_H */
diff --git a/accel/tcg/user-exec-stub.c b/accel/tcg/user-exec-stub.c
index 1d52f48226a..28286e11a60 100644
--- a/accel/tcg/user-exec-stub.c
+++ b/accel/tcg/user-exec-stub.c
@@ -1,6 +1,8 @@
 #include "qemu/osdep.h"
 #include "hw/core/cpu.h"
+#include "accel/tcg/cpu-ops.h"
 #include "exec/replay-core.h"
+#include "exec/watchpoint.h"
 #include "internal-common.h"
 
 void cpu_resume(CPUState *cpu)
@@ -19,6 +21,37 @@ void cpu_exec_reset_hold(CPUState *cpu)
 {
 }
 
+int cpu_watchpoint_insert(CPUState *cpu, vaddr addr, vaddr len,
+                          int flags, CPUWatchpoint **watchpoint)
+{
+    return -ENOSYS;
+}
+
+int cpu_watchpoint_remove(CPUState *cpu, vaddr addr,
+                          vaddr len, int flags)
+{
+    return -ENOSYS;
+}
+
+void cpu_watchpoint_remove_by_ref(CPUState *cpu, CPUWatchpoint *wp)
+{
+}
+
+void cpu_watchpoint_remove_all(CPUState *cpu, int mask)
+{
+}
+
+int cpu_watchpoint_address_matches(CPUState *cpu, vaddr addr, vaddr len)
+{
+    return 0;
+}
+
+void cpu_check_watchpoint(CPUState *cpu, vaddr addr, vaddr len,
+                          MemTxAttrs atr, int fl, uintptr_t ra)
+{
+}
+
+
 /* User mode emulation does not support softmmu yet.  */
 
 void tlb_init(CPUState *cpu)
-- 
2.52.0

[PATCH 5/5] accel/tcg: Unify watchpoint API

[Philippe Mathieu-Daudé]

Currently "exec/breakpoint.h" contains both BreakPoint *and*
WatchPoint APIs, however very few files requires the former,
and more the latter:

  $ git grep -l CPUBreakpoint | wc -l
        12
  $ git grep -l CPUWatchpoint | wc -l
        25

So extracting the WatchPoint API to its own header will reduce
compilation pressure.

But more importantly, the API is scattered in two distinct headers.
Unify them ("accel/tcg/cpu-ops.h" and "exec/watchpoint.h") to the
new "accel/tcg/watchpoint.h" header, making the emphasis the API is
specific to TCG.

Have accel/tcg/watchpoint.c absorb system/watchpoint.c code.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 MAINTAINERS                         |   1 -
 include/accel/tcg/cpu-ops.h         |  28 +-------
 include/accel/tcg/watchpoint.h      |  57 ++++++++++++++++
 include/exec/breakpoint.h           |  10 ---
 include/exec/watchpoint.h           |  18 -----
 include/hw/core/cpu.h               |   3 +-
 target/arm/internals.h              |   2 +-
 target/ppc/internal.h               |   2 +-
 target/riscv/debug.h                |   2 +-
 accel/tcg/cputlb.c                  |   1 +
 accel/tcg/tcg-accel-ops.c           |   2 +-
 accel/tcg/user-exec-stub.c          |   3 +-
 accel/tcg/watchpoint.c              |  83 +++++++++++++++++++++-
 system/watchpoint.c                 | 102 ----------------------------
 target/arm/debug_helper.c           |   2 +-
 target/arm/tcg/mte_helper.c         |   2 +-
 target/arm/tcg/sve_helper.c         |   2 +-
 target/i386/cpu.c                   |   2 +-
 target/i386/machine.c               |   2 +-
 target/i386/tcg/system/bpt_helper.c |   2 +-
 target/ppc/cpu.c                    |   2 +-
 target/ppc/cpu_init.c               |   2 +-
 target/riscv/cpu_helper.c           |   2 +-
 target/riscv/debug.c                |   2 +-
 target/s390x/helper.c               |   2 +-
 target/s390x/tcg/debug.c            |   2 +-
 target/s390x/tcg/excp_helper.c      |   2 +-
 target/s390x/tcg/mem_helper.c       |   1 +
 target/xtensa/dbg_helper.c          |   2 +-
 system/meson.build                  |   1 -
 30 files changed, 162 insertions(+), 182 deletions(-)
 create mode 100644 include/accel/tcg/watchpoint.h
 delete mode 100644 include/exec/watchpoint.h
 delete mode 100644 system/watchpoint.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 0afee64a625..7c1c0d526a0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -155,7 +155,6 @@ Overall TCG CPUs
 M: Richard Henderson <richard.henderson@linaro.org>
 R: Paolo Bonzini <pbonzini@redhat.com>
 S: Maintained
-F: system/watchpoint.c
 F: page-vary-target.c
 F: page-vary-common.c
 F: accel/tcg/
diff --git a/include/accel/tcg/cpu-ops.h b/include/accel/tcg/cpu-ops.h
index 5950cdcaab1..e20a79325e5 100644
--- a/include/accel/tcg/cpu-ops.h
+++ b/include/accel/tcg/cpu-ops.h
@@ -10,7 +10,7 @@
 #ifndef TCG_CPU_OPS_H
 #define TCG_CPU_OPS_H
 
-#include "exec/breakpoint.h"
+#include "accel/tcg/watchpoint.h"
 #include "exec/hwaddr.h"
 #include "exec/memattrs.h"
 #include "exec/memop.h"
@@ -281,32 +281,6 @@ struct TCGCPUOps {
 #endif /* !CONFIG_USER_ONLY */
 };
 
-/**
- * cpu_check_watchpoint:
- * @cpu: cpu context
- * @addr: guest virtual address
- * @len: access length
- * @attrs: memory access attributes
- * @flags: watchpoint access type
- * @ra: unwind return address
- *
- * Check for a watchpoint hit in [addr, addr+len) of the type
- * specified by @flags.  Exit via exception with a hit.
- */
-void cpu_check_watchpoint(CPUState *cpu, vaddr addr, vaddr len,
-                          MemTxAttrs attrs, int flags, uintptr_t ra);
-
-/**
- * cpu_watchpoint_address_matches:
- * @cpu: cpu context
- * @addr: guest virtual address
- * @len: access length
- *
- * Return the watchpoint flags that apply to [addr, addr+len).
- * If no watchpoint is registered for the range, the result is 0.
- */
-int cpu_watchpoint_address_matches(CPUState *cpu, vaddr addr, vaddr len);
-
 /*
  * Common pointer_wrap implementations.
  */
diff --git a/include/accel/tcg/watchpoint.h b/include/accel/tcg/watchpoint.h
new file mode 100644
index 00000000000..f28be5fc67f
--- /dev/null
+++ b/include/accel/tcg/watchpoint.h
@@ -0,0 +1,57 @@
+/*
+ * CPU watchpoints
+ *
+ * Copyright (c) 2012 SUSE LINUX Products GmbH
+ * SPDX-License-Identifier: LGPL-2.1-or-later
+ */
+
+#ifndef ACCEL_TCG_WATCHPOINT_H
+#define ACCEL_TCG_WATCHPOINT_H
+
+#include "qemu/queue.h"
+#include "exec/vaddr.h"
+#include "exec/memattrs.h"
+
+typedef struct CPUWatchpoint {
+    vaddr vaddr;
+    vaddr len;
+    vaddr hitaddr;
+    MemTxAttrs hitattrs;
+    int flags; /* BP_* */
+    QTAILQ_ENTRY(CPUWatchpoint) entry;
+} CPUWatchpoint;
+
+int cpu_watchpoint_insert(CPUState *cpu, vaddr addr, vaddr len,
+                          int flags, CPUWatchpoint **watchpoint);
+int cpu_watchpoint_remove(CPUState *cpu, vaddr addr,
+                          vaddr len, int flags);
+void cpu_watchpoint_remove_by_ref(CPUState *cpu, CPUWatchpoint *watchpoint);
+void cpu_watchpoint_remove_all(CPUState *cpu, int mask);
+
+/**
+ * cpu_check_watchpoint:
+ * @cpu: cpu context
+ * @addr: guest virtual address
+ * @len: access length
+ * @attrs: memory access attributes
+ * @flags: watchpoint access type
+ * @ra: unwind return address
+ *
+ * Check for a watchpoint hit in [addr, addr+len) of the type
+ * specified by @flags.  Exit via exception with a hit.
+ */
+void cpu_check_watchpoint(CPUState *cpu, vaddr addr, vaddr len,
+                          MemTxAttrs attrs, int flags, uintptr_t ra);
+
+/**
+ * cpu_watchpoint_address_matches:
+ * @cpu: cpu context
+ * @addr: guest virtual address
+ * @len: access length
+ *
+ * Return the watchpoint flags that apply to [addr, addr+len).
+ * If no watchpoint is registered for the range, the result is 0.
+ */
+int cpu_watchpoint_address_matches(CPUState *cpu, vaddr addr, vaddr len);
+
+#endif /* ACCEL_TCG_WATCHPOINT_H */
diff --git a/include/exec/breakpoint.h b/include/exec/breakpoint.h
index 95f0482e6d0..ccc302fea0f 100644
--- a/include/exec/breakpoint.h
+++ b/include/exec/breakpoint.h
@@ -10,7 +10,6 @@
 
 #include "qemu/queue.h"
 #include "exec/vaddr.h"
-#include "exec/memattrs.h"
 
 typedef struct CPUBreakpoint {
     vaddr pc;
@@ -18,13 +17,4 @@ typedef struct CPUBreakpoint {
     QTAILQ_ENTRY(CPUBreakpoint) entry;
 } CPUBreakpoint;
 
-typedef struct CPUWatchpoint {
-    vaddr vaddr;
-    vaddr len;
-    vaddr hitaddr;
-    MemTxAttrs hitattrs;
-    int flags; /* BP_* */
-    QTAILQ_ENTRY(CPUWatchpoint) entry;
-} CPUWatchpoint;
-
 #endif
diff --git a/include/exec/watchpoint.h b/include/exec/watchpoint.h
deleted file mode 100644
index c4d069425ba..00000000000
--- a/include/exec/watchpoint.h
+++ /dev/null
@@ -1,18 +0,0 @@
-/*
- * CPU watchpoints
- *
- * Copyright (c) 2012 SUSE LINUX Products GmbH
- * SPDX-License-Identifier: LGPL-2.1-or-later
- */
-
-#ifndef EXEC_WATCHPOINT_H
-#define EXEC_WATCHPOINT_H
-
-int cpu_watchpoint_insert(CPUState *cpu, vaddr addr, vaddr len,
-                          int flags, CPUWatchpoint **watchpoint);
-int cpu_watchpoint_remove(CPUState *cpu, vaddr addr,
-                          vaddr len, int flags);
-void cpu_watchpoint_remove_by_ref(CPUState *cpu, CPUWatchpoint *watchpoint);
-void cpu_watchpoint_remove_all(CPUState *cpu, int mask);
-
-#endif /* EXEC_WATCHPOINT_H */
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index f6f17df9e64..9648ba8f6f6 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -28,6 +28,7 @@
 #include "exec/memattrs.h"
 #include "exec/mmu-access-type.h"
 #include "exec/tlb-common.h"
+#include "accel/tcg/watchpoint.h"
 #include "qapi/qapi-types-machine.h"
 #include "qapi/qapi-types-run-state.h"
 #include "qemu/bitmap.h"
@@ -84,8 +85,6 @@ DECLARE_CLASS_CHECKERS(CPUClass, CPU,
     typedef struct ArchCPU CpuInstanceType; \
     OBJECT_DECLARE_TYPE(ArchCPU, CpuClassType, CPU_MODULE_OBJ_NAME);
 
-typedef struct CPUWatchpoint CPUWatchpoint;
-
 /* see physmem.c */
 struct CPUAddressSpace;
 
diff --git a/target/arm/internals.h b/target/arm/internals.h
index 9cd4bf74efb..e7d83836242 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -27,7 +27,7 @@
 
 #include "exec/hwaddr.h"
 #include "exec/vaddr.h"
-#include "exec/breakpoint.h"
+#include "accel/tcg/watchpoint.h"
 #include "accel/tcg/tb-cpu-state.h"
 #include "hw/core/registerfields.h"
 #include "tcg/tcg-gvec-desc.h"
diff --git a/target/ppc/internal.h b/target/ppc/internal.h
index 58f315ffcf5..f468bcbb7fa 100644
--- a/target/ppc/internal.h
+++ b/target/ppc/internal.h
@@ -18,7 +18,7 @@
 #ifndef PPC_INTERNAL_H
 #define PPC_INTERNAL_H
 
-#include "exec/breakpoint.h"
+#include "accel/tcg/watchpoint.h"
 #include "hw/core/registerfields.h"
 #include "exec/page-protection.h"
 #include "accel/tcg/tb-cpu-state.h"
diff --git a/target/riscv/debug.h b/target/riscv/debug.h
index f76b8f944a2..7aa9dfd89f9 100644
--- a/target/riscv/debug.h
+++ b/target/riscv/debug.h
@@ -22,7 +22,7 @@
 #ifndef RISCV_DEBUG_H
 #define RISCV_DEBUG_H
 
-#include "exec/breakpoint.h"
+#include "accel/tcg/watchpoint.h"
 
 #define RV_MAX_TRIGGERS         2
 
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index c30073326a3..922fc503874 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -28,6 +28,7 @@
 #include "system/physmem.h"
 #include "accel/tcg/cpu-ldst-common.h"
 #include "accel/tcg/cpu-mmu-index.h"
+#include "accel/tcg/watchpoint.h"
 #include "exec/cputlb.h"
 #include "exec/tb-flush.h"
 #include "system/ramblock.h"
diff --git a/accel/tcg/tcg-accel-ops.c b/accel/tcg/tcg-accel-ops.c
index 3bd98005042..2a6423aab8d 100644
--- a/accel/tcg/tcg-accel-ops.c
+++ b/accel/tcg/tcg-accel-ops.c
@@ -38,7 +38,7 @@
 #include "exec/hwaddr.h"
 #include "exec/tb-flush.h"
 #include "exec/translation-block.h"
-#include "exec/watchpoint.h"
+#include "accel/tcg/watchpoint.h"
 #include "gdbstub/enums.h"
 
 #include "hw/core/cpu.h"
diff --git a/accel/tcg/user-exec-stub.c b/accel/tcg/user-exec-stub.c
index 28286e11a60..a5e2534f2bf 100644
--- a/accel/tcg/user-exec-stub.c
+++ b/accel/tcg/user-exec-stub.c
@@ -1,8 +1,7 @@
 #include "qemu/osdep.h"
 #include "hw/core/cpu.h"
-#include "accel/tcg/cpu-ops.h"
 #include "exec/replay-core.h"
-#include "exec/watchpoint.h"
+#include "accel/tcg/watchpoint.h"
 #include "internal-common.h"
 
 void cpu_resume(CPUState *cpu)
diff --git a/accel/tcg/watchpoint.c b/accel/tcg/watchpoint.c
index cfb37a49e72..0d9de0fca2e 100644
--- a/accel/tcg/watchpoint.c
+++ b/accel/tcg/watchpoint.c
@@ -19,10 +19,14 @@
 
 #include "qemu/osdep.h"
 #include "qemu/main-loop.h"
-#include "exec/breakpoint.h"
+#include "qemu/error-report.h"
+#include "accel/tcg/watchpoint.h"
 #include "exec/cpu-interrupt.h"
+#include "exec/cputlb.h"
 #include "exec/page-protection.h"
+#include "exec/target_page.h"
 #include "exec/translation-block.h"
+#include "accel/tcg/watchpoint.h"
 #include "system/tcg.h"
 #include "system/replay.h"
 #include "accel/tcg/cpu-ops.h"
@@ -139,3 +143,80 @@ void cpu_check_watchpoint(CPUState *cpu, vaddr addr, vaddr len,
         }
     }
 }
+
+/* Add a watchpoint.  */
+int cpu_watchpoint_insert(CPUState *cpu, vaddr addr, vaddr len,
+                          int flags, CPUWatchpoint **watchpoint)
+{
+    CPUWatchpoint *wp;
+    vaddr in_page;
+
+    /* forbid ranges which are empty or run off the end of the address space */
+    if (len == 0 || (addr + len - 1) < addr) {
+        error_report("tried to set invalid watchpoint at %"
+                     VADDR_PRIx ", len=%" VADDR_PRIu, addr, len);
+        return -EINVAL;
+    }
+    wp = g_malloc(sizeof(*wp));
+
+    wp->vaddr = addr;
+    wp->len = len;
+    wp->flags = flags;
+
+    /* keep all GDB-injected watchpoints in front */
+    if (flags & BP_GDB) {
+        QTAILQ_INSERT_HEAD(&cpu->watchpoints, wp, entry);
+    } else {
+        QTAILQ_INSERT_TAIL(&cpu->watchpoints, wp, entry);
+    }
+
+    in_page = -(addr | TARGET_PAGE_MASK);
+    if (len <= in_page) {
+        tlb_flush_page(cpu, addr);
+    } else {
+        tlb_flush(cpu);
+    }
+
+    if (watchpoint) {
+        *watchpoint = wp;
+    }
+    return 0;
+}
+
+/* Remove a specific watchpoint.  */
+int cpu_watchpoint_remove(CPUState *cpu, vaddr addr, vaddr len,
+                          int flags)
+{
+    CPUWatchpoint *wp;
+
+    QTAILQ_FOREACH(wp, &cpu->watchpoints, entry) {
+        if (addr == wp->vaddr && len == wp->len
+                && flags == (wp->flags & ~BP_WATCHPOINT_HIT)) {
+            cpu_watchpoint_remove_by_ref(cpu, wp);
+            return 0;
+        }
+    }
+    return -ENOENT;
+}
+
+/* Remove a specific watchpoint by reference.  */
+void cpu_watchpoint_remove_by_ref(CPUState *cpu, CPUWatchpoint *watchpoint)
+{
+    QTAILQ_REMOVE(&cpu->watchpoints, watchpoint, entry);
+
+    tlb_flush_page(cpu, watchpoint->vaddr);
+
+    g_free(watchpoint);
+}
+
+/* Remove all matching watchpoints.  */
+void cpu_watchpoint_remove_all(CPUState *cpu, int mask)
+{
+    CPUWatchpoint *wp, *next;
+
+    QTAILQ_FOREACH_SAFE(wp, &cpu->watchpoints, entry, next) {
+        if (wp->flags & mask) {
+            cpu_watchpoint_remove_by_ref(cpu, wp);
+        }
+    }
+}
diff --git a/system/watchpoint.c b/system/watchpoint.c
deleted file mode 100644
index 21d0bb36cae..00000000000
--- a/system/watchpoint.c
+++ /dev/null
@@ -1,102 +0,0 @@
-/*
- * CPU watchpoints
- *
- *  Copyright (c) 2003 Fabrice Bellard
- *
- * This library is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation; either
- * version 2.1 of the License, or (at your option) any later version.
- *
- * This library is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with this library; if not, see <http://www.gnu.org/licenses/>.
- */
-
-#include "qemu/osdep.h"
-#include "qemu/error-report.h"
-#include "exec/cputlb.h"
-#include "exec/target_page.h"
-#include "exec/watchpoint.h"
-#include "hw/core/cpu.h"
-
-/* Add a watchpoint.  */
-int cpu_watchpoint_insert(CPUState *cpu, vaddr addr, vaddr len,
-                          int flags, CPUWatchpoint **watchpoint)
-{
-    CPUWatchpoint *wp;
-    vaddr in_page;
-
-    /* forbid ranges which are empty or run off the end of the address space */
-    if (len == 0 || (addr + len - 1) < addr) {
-        error_report("tried to set invalid watchpoint at %"
-                     VADDR_PRIx ", len=%" VADDR_PRIu, addr, len);
-        return -EINVAL;
-    }
-    wp = g_malloc(sizeof(*wp));
-
-    wp->vaddr = addr;
-    wp->len = len;
-    wp->flags = flags;
-
-    /* keep all GDB-injected watchpoints in front */
-    if (flags & BP_GDB) {
-        QTAILQ_INSERT_HEAD(&cpu->watchpoints, wp, entry);
-    } else {
-        QTAILQ_INSERT_TAIL(&cpu->watchpoints, wp, entry);
-    }
-
-    in_page = -(addr | TARGET_PAGE_MASK);
-    if (len <= in_page) {
-        tlb_flush_page(cpu, addr);
-    } else {
-        tlb_flush(cpu);
-    }
-
-    if (watchpoint) {
-        *watchpoint = wp;
-    }
-    return 0;
-}
-
-/* Remove a specific watchpoint.  */
-int cpu_watchpoint_remove(CPUState *cpu, vaddr addr, vaddr len,
-                          int flags)
-{
-    CPUWatchpoint *wp;
-
-    QTAILQ_FOREACH(wp, &cpu->watchpoints, entry) {
-        if (addr == wp->vaddr && len == wp->len
-                && flags == (wp->flags & ~BP_WATCHPOINT_HIT)) {
-            cpu_watchpoint_remove_by_ref(cpu, wp);
-            return 0;
-        }
-    }
-    return -ENOENT;
-}
-
-/* Remove a specific watchpoint by reference.  */
-void cpu_watchpoint_remove_by_ref(CPUState *cpu, CPUWatchpoint *watchpoint)
-{
-    QTAILQ_REMOVE(&cpu->watchpoints, watchpoint, entry);
-
-    tlb_flush_page(cpu, watchpoint->vaddr);
-
-    g_free(watchpoint);
-}
-
-/* Remove all matching watchpoints.  */
-void cpu_watchpoint_remove_all(CPUState *cpu, int mask)
-{
-    CPUWatchpoint *wp, *next;
-
-    QTAILQ_FOREACH_SAFE(wp, &cpu->watchpoints, entry, next) {
-        if (wp->flags & mask) {
-            cpu_watchpoint_remove_by_ref(cpu, wp);
-        }
-    }
-}
diff --git a/target/arm/debug_helper.c b/target/arm/debug_helper.c
index 579516e1541..9dc8d47f664 100644
--- a/target/arm/debug_helper.c
+++ b/target/arm/debug_helper.c
@@ -11,7 +11,7 @@
 #include "internals.h"
 #include "cpu-features.h"
 #include "cpregs.h"
-#include "exec/watchpoint.h"
+#include "accel/tcg/watchpoint.h"
 #include "system/tcg.h"
 
 #define HELPER_H "tcg/helper.h"
diff --git a/target/arm/tcg/mte_helper.c b/target/arm/tcg/mte_helper.c
index bb48fe359b8..f71ed4d9178 100644
--- a/target/arm/tcg/mte_helper.c
+++ b/target/arm/tcg/mte_helper.c
@@ -31,9 +31,9 @@
 #endif
 #include "accel/tcg/cpu-ldst.h"
 #include "accel/tcg/probe.h"
+#include "accel/tcg/watchpoint.h"
 #include "exec/helper-proto.h"
 #include "exec/tlb-flags.h"
-#include "accel/tcg/cpu-ops.h"
 #include "qapi/error.h"
 #include "qemu/guest-random.h"
 #include "mte_helper.h"
diff --git a/target/arm/tcg/sve_helper.c b/target/arm/tcg/sve_helper.c
index c442fcb540d..7aa88fdeea0 100644
--- a/target/arm/tcg/sve_helper.c
+++ b/target/arm/tcg/sve_helper.c
@@ -31,8 +31,8 @@
 #include "sve_ldst_internal.h"
 #include "accel/tcg/cpu-ldst.h"
 #include "accel/tcg/helper-retaddr.h"
-#include "accel/tcg/cpu-ops.h"
 #include "accel/tcg/probe.h"
+#include "accel/tcg/watchpoint.h"
 #ifdef CONFIG_USER_ONLY
 #include "user/page-protection.h"
 #endif
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 9831e2bc210..e7273d67369 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -36,7 +36,7 @@
 #include "standard-headers/asm-x86/kvm_para.h"
 #include "hw/core/qdev-properties.h"
 #include "hw/i386/topology.h"
-#include "exec/watchpoint.h"
+#include "accel/tcg/watchpoint.h"
 #ifndef CONFIG_USER_ONLY
 #include "confidential-guest.h"
 #include "system/reset.h"
diff --git a/target/i386/machine.c b/target/i386/machine.c
index c9139612813..863769b9c9b 100644
--- a/target/i386/machine.c
+++ b/target/i386/machine.c
@@ -7,7 +7,7 @@
 #include "hw/i386/x86.h"
 #include "kvm/kvm_i386.h"
 #include "hw/xen/xen.h"
-#include "exec/watchpoint.h"
+#include "accel/tcg/watchpoint.h"
 #include "system/kvm.h"
 #include "system/kvm_xen.h"
 #include "system/tcg.h"
diff --git a/target/i386/tcg/system/bpt_helper.c b/target/i386/tcg/system/bpt_helper.c
index aebb5caac37..f722774bb91 100644
--- a/target/i386/tcg/system/bpt_helper.c
+++ b/target/i386/tcg/system/bpt_helper.c
@@ -20,7 +20,7 @@
 #include "qemu/osdep.h"
 #include "cpu.h"
 #include "exec/helper-proto.h"
-#include "exec/watchpoint.h"
+#include "accel/tcg/watchpoint.h"
 #include "tcg/helper-tcg.h"
 
 
diff --git a/target/ppc/cpu.c b/target/ppc/cpu.c
index 9cb3f00aa88..25d950aeedc 100644
--- a/target/ppc/cpu.c
+++ b/target/ppc/cpu.c
@@ -22,7 +22,7 @@
 #include "cpu-models.h"
 #include "cpu-qom.h"
 #include "exec/log.h"
-#include "exec/watchpoint.h"
+#include "accel/tcg/watchpoint.h"
 #include "fpu/softfloat-helpers.h"
 #include "mmu-hash64.h"
 #include "helper_regs.h"
diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index 929254827d6..cca2096fb57 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -40,7 +40,7 @@
 #include "qemu/cutils.h"
 #include "disas/capstone.h"
 #include "fpu/softfloat.h"
-#include "exec/watchpoint.h"
+#include "accel/tcg/watchpoint.h"
 #include "helper_regs.h"
 #include "internal.h"
 #include "spr_common.h"
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index c4fb68b5de8..1db4a6fc36f 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -29,7 +29,7 @@
 #include "system/memory.h"
 #include "instmap.h"
 #include "tcg/tcg-op.h"
-#include "accel/tcg/cpu-ops.h"
+#include "accel/tcg/watchpoint.h"
 #include "trace.h"
 #include "semihosting/common-semi.h"
 #include "exec/icount.h"
diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index 56644667497..f3a72dd4b7a 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -29,7 +29,7 @@
 #include "cpu.h"
 #include "trace.h"
 #include "exec/helper-proto.h"
-#include "exec/watchpoint.h"
+#include "accel/tcg/watchpoint.h"
 #include "system/cpu-timers.h"
 #include "exec/icount.h"
 
diff --git a/target/s390x/helper.c b/target/s390x/helper.c
index 8d1e03f6768..7dede1e7ed3 100644
--- a/target/s390x/helper.c
+++ b/target/s390x/helper.c
@@ -28,7 +28,7 @@
 #include "system/memory.h"
 #include "system/runstate.h"
 #include "exec/target_page.h"
-#include "exec/watchpoint.h"
+#include "accel/tcg/watchpoint.h"
 
 void s390x_tod_timer(void *opaque)
 {
diff --git a/target/s390x/tcg/debug.c b/target/s390x/tcg/debug.c
index 12ae95d4fe8..74c1d5f3242 100644
--- a/target/s390x/tcg/debug.c
+++ b/target/s390x/tcg/debug.c
@@ -10,7 +10,7 @@
  */
 
 #include "qemu/osdep.h"
-#include "exec/watchpoint.h"
+#include "accel/tcg/watchpoint.h"
 #include "target/s390x/cpu.h"
 #include "tcg_s390x.h"
 
diff --git a/target/s390x/tcg/excp_helper.c b/target/s390x/tcg/excp_helper.c
index d4a096f5998..1485dbf83be 100644
--- a/target/s390x/tcg/excp_helper.c
+++ b/target/s390x/tcg/excp_helper.c
@@ -24,7 +24,7 @@
 #include "exec/helper-proto.h"
 #include "exec/cputlb.h"
 #include "exec/target_page.h"
-#include "exec/watchpoint.h"
+#include "accel/tcg/watchpoint.h"
 #include "s390x-internal.h"
 #include "tcg_s390x.h"
 #ifndef CONFIG_USER_ONLY
diff --git a/target/s390x/tcg/mem_helper.c b/target/s390x/tcg/mem_helper.c
index 2972b7ddb97..b94d12ee7f8 100644
--- a/target/s390x/tcg/mem_helper.c
+++ b/target/s390x/tcg/mem_helper.c
@@ -33,6 +33,7 @@
 #include "exec/tlb-flags.h"
 #include "accel/tcg/cpu-ops.h"
 #include "accel/tcg/helper-retaddr.h"
+#include "accel/tcg/watchpoint.h"
 #include "qemu/int128.h"
 #include "qemu/atomic128.h"
 
diff --git a/target/xtensa/dbg_helper.c b/target/xtensa/dbg_helper.c
index 3b91f7c38ac..25341dc0a71 100644
--- a/target/xtensa/dbg_helper.c
+++ b/target/xtensa/dbg_helper.c
@@ -30,7 +30,7 @@
 #include "cpu.h"
 #include "exec/helper-proto.h"
 #include "qemu/host-utils.h"
-#include "exec/watchpoint.h"
+#include "accel/tcg/watchpoint.h"
 #include "system/address-spaces.h"
 
 void HELPER(wsr_ibreakenable)(CPUXtensaState *env, uint32_t v)
diff --git a/system/meson.build b/system/meson.build
index 4b69ef0f5fb..d24efbb44ba 100644
--- a/system/meson.build
+++ b/system/meson.build
@@ -29,7 +29,6 @@ system_ss.add(files(
   'runstate-hmp-cmds.c',
   'runstate.c',
   'tpm-hmp-cmds.c',
-  'watchpoint.c',
 ))
 
 if have_tpm
-- 
2.52.0

Re: [PATCH 3/5] target/s390x: Restrict WatchPoint API to TCG

[Thomas Huth]

On 07/01/2026 00.19, Philippe Mathieu-Daudé wrote:
> By inverting the 'tcg_enabled()' check in s390_cpu_set_psw()
> we can let the compiler elide the s390_cpu_recompute_watchpoints()
> call when TCG is not available. Move it to a TCG specific
> file to avoid compiling dead code on KVM. This restricts the
> WatchPoint API calls to TCG.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/s390x/s390x-internal.h |  1 -
>   target/s390x/tcg/tcg_s390x.h  |  4 +++
>   target/s390x/cpu.c            | 30 ++++++++++----------
>   target/s390x/helper.c         | 38 -------------------------
>   target/s390x/tcg/debug.c      | 53 +++++++++++++++++++++++++++++++++++
>   target/s390x/tcg/meson.build  |  3 ++
>   6 files changed, 75 insertions(+), 54 deletions(-)
>   create mode 100644 target/s390x/tcg/debug.c
> 
> diff --git a/target/s390x/s390x-internal.h b/target/s390x/s390x-internal.h
> index 9691366ec91..c4cd17d4d7d 100644
> --- a/target/s390x/s390x-internal.h
> +++ b/target/s390x/s390x-internal.h
> @@ -317,7 +317,6 @@ void s390_cpu_gdb_init(CPUState *cs);
>   void s390_cpu_dump_state(CPUState *cpu, FILE *f, int flags);
>   void do_restart_interrupt(CPUS390XState *env);
>   #ifndef CONFIG_USER_ONLY
> -void s390_cpu_recompute_watchpoints(CPUState *cs);
>   void s390x_tod_timer(void *opaque);
>   void s390x_cpu_timer(void *opaque);
>   void s390_handle_wait(S390CPU *cpu);
> diff --git a/target/s390x/tcg/tcg_s390x.h b/target/s390x/tcg/tcg_s390x.h
> index 78558912f99..33f26f26c0f 100644
> --- a/target/s390x/tcg/tcg_s390x.h
> +++ b/target/s390x/tcg/tcg_s390x.h
> @@ -21,4 +21,8 @@ G_NORETURN void tcg_s390_data_exception(CPUS390XState *env, uint32_t dxc,
>   G_NORETURN void tcg_s390_vector_exception(CPUS390XState *env, uint32_t vxc,
>                                             uintptr_t ra);
>   
> +#ifndef CONFIG_USER_ONLY
> +void s390_cpu_recompute_watchpoints(CPUState *cs);
> +#endif
> +
>   #endif /* TCG_S390X_H */
> diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
> index 6c4198eb1b1..f68b288e364 100644
> --- a/target/s390x/cpu.c
> +++ b/target/s390x/cpu.c
> @@ -40,6 +40,7 @@
>   #include "system/reset.h"
>   #endif
>   #include "hw/s390x/cpu-topology.h"
> +#include "tcg/tcg_s390x.h"
>   
>   #define CR0_RESET       0xE0UL
>   #define CR14_RESET      0xC2000000UL;
> @@ -74,26 +75,25 @@ void s390_cpu_set_psw(CPUS390XState *env, uint64_t mask, uint64_t addr)
>       env->psw.mask = mask;
>   
>       /* KVM will handle all WAITs and trigger a WAIT exit on disabled_wait */
> -    if (!tcg_enabled()) {
> -        return;
> -    }
> -    env->cc_op = (mask >> 44) & 3;
> +    if (tcg_enabled()) {
> +        env->cc_op = (mask >> 44) & 3;
>   
>   #ifndef CONFIG_USER_ONLY
> -    if (is_early_exception_psw(mask, addr)) {
> -        env->int_pgm_ilen = 0;
> -        trigger_pgm_exception(env, PGM_SPECIFICATION);
> -        return;
> -    }
> +        if (is_early_exception_psw(mask, addr)) {
> +            env->int_pgm_ilen = 0;
> +            trigger_pgm_exception(env, PGM_SPECIFICATION);
> +            return;
> +        }
>   
> -    if ((old_mask ^ mask) & PSW_MASK_PER) {
> -        s390_cpu_recompute_watchpoints(env_cpu(env));
> -    }
> +        if ((old_mask ^ mask) & PSW_MASK_PER) {
> +            s390_cpu_recompute_watchpoints(env_cpu(env));
> +        }
>   
> -    if (mask & PSW_MASK_WAIT) {
> -        s390_handle_wait(env_archcpu(env));
> -    }
> +        if (mask & PSW_MASK_WAIT) {
> +            s390_handle_wait(env_archcpu(env));
> +        }
>   #endif
> +    }
>   }
>   
>   uint64_t s390_cpu_get_psw_mask(CPUS390XState *env)
> diff --git a/target/s390x/helper.c b/target/s390x/helper.c
> index 184428c6d9d..8d1e03f6768 100644
> --- a/target/s390x/helper.c
> +++ b/target/s390x/helper.c
> @@ -143,41 +143,3 @@ void do_restart_interrupt(CPUS390XState *env)
>   
>       s390_cpu_set_psw(env, mask, addr);
>   }
> -
> -void s390_cpu_recompute_watchpoints(CPUState *cs)
> -{
> -    const int wp_flags = BP_CPU | BP_MEM_WRITE | BP_STOP_BEFORE_ACCESS;
> -    CPUS390XState *env = cpu_env(cs);
> -
> -    /* We are called when the watchpoints have changed. First
> -       remove them all.  */
> -    cpu_watchpoint_remove_all(cs, BP_CPU);
> -
> -    /* Return if PER is not enabled */
> -    if (!(env->psw.mask & PSW_MASK_PER)) {
> -        return;
> -    }
> -
> -    /* Return if storage-alteration event is not enabled.  */
> -    if (!(env->cregs[9] & PER_CR9_EVENT_STORE)) {
> -        return;
> -    }
> -
> -    if (env->cregs[10] == 0 && env->cregs[11] == -1LL) {
> -        /* We can't create a watchoint spanning the whole memory range, so
> -           split it in two parts.   */
> -        cpu_watchpoint_insert(cs, 0, 1ULL << 63, wp_flags, NULL);
> -        cpu_watchpoint_insert(cs, 1ULL << 63, 1ULL << 63, wp_flags, NULL);
> -    } else if (env->cregs[10] > env->cregs[11]) {
> -        /* The address range loops, create two watchpoints.  */
> -        cpu_watchpoint_insert(cs, env->cregs[10], -env->cregs[10],
> -                              wp_flags, NULL);
> -        cpu_watchpoint_insert(cs, 0, env->cregs[11] + 1, wp_flags, NULL);
> -
> -    } else {
> -        /* Default case, create a single watchpoint.  */
> -        cpu_watchpoint_insert(cs, env->cregs[10],
> -                              env->cregs[11] - env->cregs[10] + 1,
> -                              wp_flags, NULL);
> -    }
> -}
> diff --git a/target/s390x/tcg/debug.c b/target/s390x/tcg/debug.c
> new file mode 100644
> index 00000000000..12ae95d4fe8
> --- /dev/null
> +++ b/target/s390x/tcg/debug.c
> @@ -0,0 +1,53 @@
> +/*
> + * QEMU S/390 debug routines
> + *
> + * Copyright (c) 2009 Ulrich Hecht
> + * Copyright (c) 2011 Alexander Graf
> + * Copyright (c) 2012 SUSE LINUX Products GmbH
> + * Copyright (c) 2012 IBM Corp.

Looking at "git blame," the code has been introduced in commit 
311918b979c5364c30392c1054ed77d047a83953, so none of the above 
people/companies has been involved.

That original commit also introduced another function called 
s390x_cpu_debug_excp_handler() which now resides in 
target/s390x/tcg/excp_helper.c ... so maybe s390_cpu_recompute_watchpoints() 
should simply be moved there, too, instead?

  Thomas

Re: [PATCH 1/5] target/i386: Restrict WatchPoint API to TCG

[Pierrick Bouvier]

On 1/6/26 3:19 PM, Philippe Mathieu-Daudé wrote:
> Watchpoints are specific to the TCG accelerator. Restrict
> the cpu_watchpoint_remove_all() call.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/i386/cpu.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Re: [PATCH 2/5] target/ppc: Restrict WatchPoint API to TCG

[Pierrick Bouvier]

On 1/6/26 3:19 PM, Philippe Mathieu-Daudé wrote:
> Watchpoints are specific to the TCG accelerator. Since the
> Data Address Watchpoint helpers are only called from
> translated code, move them to a new 'watchpoint.c' file,
> specific to TCG. Thus restricting the WatchPoint API to TCG.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/ppc/cpu.c        | 81 +----------------------------------
>   target/ppc/watchpoint.c | 93 +++++++++++++++++++++++++++++++++++++++++
>   target/ppc/meson.build  |  1 +
>   3 files changed, 96 insertions(+), 79 deletions(-)
>   create mode 100644 target/ppc/watchpoint.c
> 

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Re: [PATCH 3/5] target/s390x: Restrict WatchPoint API to TCG

[Pierrick Bouvier]

On 1/6/26 3:19 PM, Philippe Mathieu-Daudé wrote:
> By inverting the 'tcg_enabled()' check in s390_cpu_set_psw()
> we can let the compiler elide the s390_cpu_recompute_watchpoints()
> call when TCG is not available. Move it to a TCG specific
> file to avoid compiling dead code on KVM. This restricts the
> WatchPoint API calls to TCG.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/s390x/s390x-internal.h |  1 -
>   target/s390x/tcg/tcg_s390x.h  |  4 +++
>   target/s390x/cpu.c            | 30 ++++++++++----------
>   target/s390x/helper.c         | 38 -------------------------
>   target/s390x/tcg/debug.c      | 53 +++++++++++++++++++++++++++++++++++
>   target/s390x/tcg/meson.build  |  3 ++
>   6 files changed, 75 insertions(+), 54 deletions(-)
>   create mode 100644 target/s390x/tcg/debug.c
> 

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Re: [PATCH 4/5] accel/tcg: Un-inline WatchPoint API user-emulation stubs

[Pierrick Bouvier]

On 1/6/26 3:19 PM, Philippe Mathieu-Daudé wrote:
> Currently we can not build files including "exec/watchpoint.h"
> as meson common objects because the CONFIG_USER_ONLY definition
> is poisoned. We can easily fix that by un-inlining the
> user-emulation stubs.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/accel/tcg/cpu-ops.h | 17 -----------------
>   include/exec/watchpoint.h   | 23 -----------------------
>   accel/tcg/user-exec-stub.c  | 33 +++++++++++++++++++++++++++++++++
>   3 files changed, 33 insertions(+), 40 deletions(-)
> 

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Re: [PATCH 5/5] accel/tcg: Unify watchpoint API

[Pierrick Bouvier]

On 1/6/26 3:19 PM, Philippe Mathieu-Daudé wrote:
> Currently "exec/breakpoint.h" contains both BreakPoint *and*
> WatchPoint APIs, however very few files requires the former,
> and more the latter:
> 
>    $ git grep -l CPUBreakpoint | wc -l
>          12
>    $ git grep -l CPUWatchpoint | wc -l
>          25
> 
> So extracting the WatchPoint API to its own header will reduce
> compilation pressure.
> 
> But more importantly, the API is scattered in two distinct headers.
> Unify them ("accel/tcg/cpu-ops.h" and "exec/watchpoint.h") to the
> new "accel/tcg/watchpoint.h" header, making the emphasis the API is
> specific to TCG.
> 
> Have accel/tcg/watchpoint.c absorb system/watchpoint.c code.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   MAINTAINERS                         |   1 -
>   include/accel/tcg/cpu-ops.h         |  28 +-------
>   include/accel/tcg/watchpoint.h      |  57 ++++++++++++++++
>   include/exec/breakpoint.h           |  10 ---
>   include/exec/watchpoint.h           |  18 -----
>   include/hw/core/cpu.h               |   3 +-
>   target/arm/internals.h              |   2 +-
>   target/ppc/internal.h               |   2 +-
>   target/riscv/debug.h                |   2 +-
>   accel/tcg/cputlb.c                  |   1 +
>   accel/tcg/tcg-accel-ops.c           |   2 +-
>   accel/tcg/user-exec-stub.c          |   3 +-
>   accel/tcg/watchpoint.c              |  83 +++++++++++++++++++++-
>   system/watchpoint.c                 | 102 ----------------------------
>   target/arm/debug_helper.c           |   2 +-
>   target/arm/tcg/mte_helper.c         |   2 +-
>   target/arm/tcg/sve_helper.c         |   2 +-
>   target/i386/cpu.c                   |   2 +-
>   target/i386/machine.c               |   2 +-
>   target/i386/tcg/system/bpt_helper.c |   2 +-
>   target/ppc/cpu.c                    |   2 +-
>   target/ppc/cpu_init.c               |   2 +-
>   target/riscv/cpu_helper.c           |   2 +-
>   target/riscv/debug.c                |   2 +-
>   target/s390x/helper.c               |   2 +-
>   target/s390x/tcg/debug.c            |   2 +-
>   target/s390x/tcg/excp_helper.c      |   2 +-
>   target/s390x/tcg/mem_helper.c       |   1 +
>   target/xtensa/dbg_helper.c          |   2 +-
>   system/meson.build                  |   1 -
>   30 files changed, 162 insertions(+), 182 deletions(-)
>   create mode 100644 include/accel/tcg/watchpoint.h
>   delete mode 100644 include/exec/watchpoint.h
>   delete mode 100644 system/watchpoint.c
> 

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Re: [PATCH 1/5] target/i386: Restrict WatchPoint API to TCG

[Zhao Liu]

On Wed, Jan 07, 2026 at 12:19:03AM +0100, Philippe Mathieu-Daudé wrote:
> Date: Wed,  7 Jan 2026 00:19:03 +0100
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
> Subject: [PATCH 1/5] target/i386: Restrict WatchPoint API to TCG
> X-Mailer: git-send-email 2.52.0
> 
> Watchpoints are specific to the TCG accelerator. Restrict
> the cpu_watchpoint_remove_all() call.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  target/i386/cpu.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Re: [PATCH 5/5] accel/tcg: Unify watchpoint API

[Zhao Liu]

On Wed, Jan 07, 2026 at 12:19:07AM +0100, Philippe Mathieu-Daudé wrote:
> Date: Wed,  7 Jan 2026 00:19:07 +0100
> From: Philippe Mathieu-Daudé <philmd@linaro.org>
> Subject: [PATCH 5/5] accel/tcg: Unify watchpoint API
> X-Mailer: git-send-email 2.52.0
> 
> Currently "exec/breakpoint.h" contains both BreakPoint *and*
> WatchPoint APIs, however very few files requires the former,
> and more the latter:
> 
>   $ git grep -l CPUBreakpoint | wc -l
>         12
>   $ git grep -l CPUWatchpoint | wc -l
>         25
> 
> So extracting the WatchPoint API to its own header will reduce
> compilation pressure.
> 
> But more importantly, the API is scattered in two distinct headers.
> Unify them ("accel/tcg/cpu-ops.h" and "exec/watchpoint.h") to the
> new "accel/tcg/watchpoint.h" header, making the emphasis the API is
> specific to TCG.
> 
> Have accel/tcg/watchpoint.c absorb system/watchpoint.c code.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  MAINTAINERS                         |   1 -
>  include/accel/tcg/cpu-ops.h         |  28 +-------
>  include/accel/tcg/watchpoint.h      |  57 ++++++++++++++++
>  include/exec/breakpoint.h           |  10 ---
>  include/exec/watchpoint.h           |  18 -----
>  include/hw/core/cpu.h               |   3 +-
>  target/arm/internals.h              |   2 +-
>  target/ppc/internal.h               |   2 +-
>  target/riscv/debug.h                |   2 +-
>  accel/tcg/cputlb.c                  |   1 +
>  accel/tcg/tcg-accel-ops.c           |   2 +-
>  accel/tcg/user-exec-stub.c          |   3 +-
>  accel/tcg/watchpoint.c              |  83 +++++++++++++++++++++-
>  system/watchpoint.c                 | 102 ----------------------------
>  target/arm/debug_helper.c           |   2 +-
>  target/arm/tcg/mte_helper.c         |   2 +-
>  target/arm/tcg/sve_helper.c         |   2 +-
>  target/i386/cpu.c                   |   2 +-
>  target/i386/machine.c               |   2 +-
>  target/i386/tcg/system/bpt_helper.c |   2 +-
>  target/ppc/cpu.c                    |   2 +-
>  target/ppc/cpu_init.c               |   2 +-
>  target/riscv/cpu_helper.c           |   2 +-
>  target/riscv/debug.c                |   2 +-
>  target/s390x/helper.c               |   2 +-
>  target/s390x/tcg/debug.c            |   2 +-
>  target/s390x/tcg/excp_helper.c      |   2 +-
>  target/s390x/tcg/mem_helper.c       |   1 +
>  target/xtensa/dbg_helper.c          |   2 +-
>  system/meson.build                  |   1 -
>  30 files changed, 162 insertions(+), 182 deletions(-)
>  create mode 100644 include/accel/tcg/watchpoint.h
>  delete mode 100644 include/exec/watchpoint.h
>  delete mode 100644 system/watchpoint.c

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>

Re: [PATCH 2/5] target/ppc: Restrict WatchPoint API to TCG

[Chinmay Rath]

On 1/7/26 04:49, Philippe Mathieu-Daudé wrote:
> Watchpoints are specific to the TCG accelerator. Since the
> Data Address Watchpoint helpers are only called from
> translated code, move them to a new 'watchpoint.c' file,
> specific to TCG. Thus restricting the WatchPoint API to TCG.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Chinmay Rath <rathc@linux.ibm.com>
> ---
>   target/ppc/cpu.c        | 81 +----------------------------------
>   target/ppc/watchpoint.c | 93 +++++++++++++++++++++++++++++++++++++++++
>   target/ppc/meson.build  |  1 +
>   3 files changed, 96 insertions(+), 79 deletions(-)
>   create mode 100644 target/ppc/watchpoint.c
>
> diff --git a/target/ppc/cpu.c b/target/ppc/cpu.c
> index 4d8faaddee2..9cb3f00aa88 100644
> --- a/target/ppc/cpu.c
> +++ b/target/ppc/cpu.c
> @@ -131,85 +131,8 @@ void ppc_store_ciabr(CPUPPCState *env, target_ulong val)
>       ppc_update_ciabr(env);
>   }
>   
> -void ppc_update_daw(CPUPPCState *env, int rid)
> -{
> -    CPUState *cs = env_cpu(env);
> -    int spr_dawr = rid ? SPR_DAWR1 : SPR_DAWR0;
> -    int spr_dawrx = rid ? SPR_DAWRX1 : SPR_DAWRX0;
> -    target_ulong deaw = env->spr[spr_dawr] & PPC_BITMASK(0, 60);
> -    uint32_t dawrx = env->spr[spr_dawrx];
> -    int mrd = extract32(dawrx, PPC_BIT_NR(48), 54 - 48);
> -    bool dw = extract32(dawrx, PPC_BIT_NR(57), 1);
> -    bool dr = extract32(dawrx, PPC_BIT_NR(58), 1);
> -    bool hv = extract32(dawrx, PPC_BIT_NR(61), 1);
> -    bool sv = extract32(dawrx, PPC_BIT_NR(62), 1);
> -    bool pr = extract32(dawrx, PPC_BIT_NR(62), 1);
> -    vaddr len;
> -    int flags;
> -
> -    if (env->dawr_watchpoint[rid]) {
> -        cpu_watchpoint_remove_by_ref(cs, env->dawr_watchpoint[rid]);
> -        env->dawr_watchpoint[rid] = NULL;
> -    }
> -
> -    if (!dr && !dw) {
> -        return;
> -    }
> -
> -    if (!hv && !sv && !pr) {
> -        return;
> -    }
> -
> -    len = (mrd + 1) * 8;
> -    flags = BP_CPU | BP_STOP_BEFORE_ACCESS;
> -    if (dr) {
> -        flags |= BP_MEM_READ;
> -    }
> -    if (dw) {
> -        flags |= BP_MEM_WRITE;
> -    }
> -
> -    cpu_watchpoint_insert(cs, deaw, len, flags, &env->dawr_watchpoint[rid]);
> -}
> -
> -void ppc_store_dawr0(CPUPPCState *env, target_ulong val)
> -{
> -    env->spr[SPR_DAWR0] = val;
> -    ppc_update_daw(env, 0);
> -}
> -
> -static void ppc_store_dawrx(CPUPPCState *env, uint32_t val, int rid)
> -{
> -    int hrammc = extract32(val, PPC_BIT_NR(56), 1);
> -
> -    if (hrammc) {
> -        /* This might be done with a second watchpoint at the xor of DEAW[0] */
> -        qemu_log_mask(LOG_UNIMP, "%s: DAWRX%d[HRAMMC] is unimplemented\n",
> -                      __func__, rid);
> -    }
> -
> -    env->spr[rid ? SPR_DAWRX1 : SPR_DAWRX0] = val;
> -    ppc_update_daw(env, rid);
> -}
> -
> -void ppc_store_dawrx0(CPUPPCState *env, uint32_t val)
> -{
> -    ppc_store_dawrx(env, val, 0);
> -}
> -
> -void ppc_store_dawr1(CPUPPCState *env, target_ulong val)
> -{
> -    env->spr[SPR_DAWR1] = val;
> -    ppc_update_daw(env, 1);
> -}
> -
> -void ppc_store_dawrx1(CPUPPCState *env, uint32_t val)
> -{
> -    ppc_store_dawrx(env, val, 1);
> -}
> -
> -#endif
> -#endif
> +#endif /* TARGET_PPC64 */
> +#endif /* !CONFIG_USER_ONLY */
>   
>   static inline void fpscr_set_rounding_mode(CPUPPCState *env)
>   {
> diff --git a/target/ppc/watchpoint.c b/target/ppc/watchpoint.c
> new file mode 100644
> index 00000000000..c71dd4550b7
> --- /dev/null
> +++ b/target/ppc/watchpoint.c
> @@ -0,0 +1,93 @@
> +/*
> + * PowerPC watchpoint routines for QEMU
> + *
> + * Copyright (c) 2017 Nikunj A Dadhania, IBM Corporation.
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include "exec/log.h"
> +#include "accel/tcg/watchpoint.h"
> +#include "target/ppc/cpu.h"
> +
> +#if defined(TARGET_PPC64)
> +
> +void ppc_update_daw(CPUPPCState *env, int rid)
> +{
> +    CPUState *cs = env_cpu(env);
> +    int spr_dawr = rid ? SPR_DAWR1 : SPR_DAWR0;
> +    int spr_dawrx = rid ? SPR_DAWRX1 : SPR_DAWRX0;
> +    target_ulong deaw = env->spr[spr_dawr] & PPC_BITMASK(0, 60);
> +    uint32_t dawrx = env->spr[spr_dawrx];
> +    int mrd = extract32(dawrx, PPC_BIT_NR(48), 54 - 48);
> +    bool dw = extract32(dawrx, PPC_BIT_NR(57), 1);
> +    bool dr = extract32(dawrx, PPC_BIT_NR(58), 1);
> +    bool hv = extract32(dawrx, PPC_BIT_NR(61), 1);
> +    bool sv = extract32(dawrx, PPC_BIT_NR(62), 1);
> +    bool pr = extract32(dawrx, PPC_BIT_NR(62), 1);
> +    vaddr len;
> +    int flags;
> +
> +    if (env->dawr_watchpoint[rid]) {
> +        cpu_watchpoint_remove_by_ref(cs, env->dawr_watchpoint[rid]);
> +        env->dawr_watchpoint[rid] = NULL;
> +    }
> +
> +    if (!dr && !dw) {
> +        return;
> +    }
> +
> +    if (!hv && !sv && !pr) {
> +        return;
> +    }
> +
> +    len = (mrd + 1) * 8;
> +    flags = BP_CPU | BP_STOP_BEFORE_ACCESS;
> +    if (dr) {
> +        flags |= BP_MEM_READ;
> +    }
> +    if (dw) {
> +        flags |= BP_MEM_WRITE;
> +    }
> +
> +    cpu_watchpoint_insert(cs, deaw, len, flags, &env->dawr_watchpoint[rid]);
> +}
> +
> +void ppc_store_dawr0(CPUPPCState *env, target_ulong val)
> +{
> +    env->spr[SPR_DAWR0] = val;
> +    ppc_update_daw(env, 0);
> +}
> +
> +static void ppc_store_dawrx(CPUPPCState *env, uint32_t val, int rid)
> +{
> +    int hrammc = extract32(val, PPC_BIT_NR(56), 1);
> +
> +    if (hrammc) {
> +        /* This might be done with a second watchpoint at the xor of DEAW[0] */
> +        qemu_log_mask(LOG_UNIMP, "%s: DAWRX%d[HRAMMC] is unimplemented\n",
> +                      __func__, rid);
> +    }
> +
> +    env->spr[rid ? SPR_DAWRX1 : SPR_DAWRX0] = val;
> +    ppc_update_daw(env, rid);
> +}
> +
> +void ppc_store_dawrx0(CPUPPCState *env, uint32_t val)
> +{
> +    ppc_store_dawrx(env, val, 0);
> +}
> +
> +void ppc_store_dawr1(CPUPPCState *env, target_ulong val)
> +{
> +    env->spr[SPR_DAWR1] = val;
> +    ppc_update_daw(env, 1);
> +}
> +
> +void ppc_store_dawrx1(CPUPPCState *env, uint32_t val)
> +{
> +    ppc_store_dawrx(env, val, 1);
> +}
> +
> +#endif
> diff --git a/target/ppc/meson.build b/target/ppc/meson.build
> index 8eed1fa40ca..d354c3240a2 100644
> --- a/target/ppc/meson.build
> +++ b/target/ppc/meson.build
> @@ -43,6 +43,7 @@ ppc_system_ss.add(files(
>     'ppc-qmp-cmds.c',
>   ))
>   ppc_system_ss.add(when: 'CONFIG_TCG', if_true: files(
> +  'watchpoint.c',
>     'mmu_helper.c',
>   ), if_false: files(
>     'tcg-stub.c',

Re: [PATCH 0/5] accel/tcg: Restrict WatchPoint API to TCG

[Richard Henderson]

On 1/7/26 10:19, Philippe Mathieu-Daudé wrote:
> Hi,
> 
> This series is related to single binary, where we want
> to build compilation units once. Headers using the
> 'CONFIG_USER_ONLY' definition are "poisoned", except if
> we duplicate the meson source set, in that case we define
> COMPILING_SYSTEM_VS_USER and CONFIG_USER_ONLY is no more
> poisoned.
> 
> Looking at the watchpoint API, CONFIG_USER_ONLY is only
> used to avoid stubs, so it can easily be reworked to avoid
> the need of duplicated source set.
> 
> Since here, we also restrict the API to TCG, and unify it
> in a single header: "accel/tcg/watchpoint.h", since it is
> distinct to the BreakPoint API which can be used by hardware
> accelerators.

Watchpoints could be used on aarch64 kvm.

They probably could be used on other kvm, with modifications to kvm -- at present the api 
presented to the hypervisor is breakpoint specific, and I'm not sure why.

I think it would be shame to preclude other accelerators from handling watchpoints, even 
if none currently do so.


r~