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

Philippe Mathieu-Daudé posted 5 patches 1 month ago
Maintainers: Richard Henderson <richard.henderson@linaro.org>, Paolo Bonzini <pbonzini@redhat.com>, Riku Voipio <riku.voipio@iki.fi>, Eduardo Habkost <eduardo@habkost.net>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Yanan Wang <wangyanan55@huawei.com>, Zhao Liu <zhao1.liu@intel.com>, Peter Maydell <peter.maydell@linaro.org>, Nicholas Piggin <npiggin@gmail.com>, Chinmay Rath <rathc@linux.ibm.com>, Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Weiwei Li <liwei1518@gmail.com>, Daniel Henrique Barboza <dbarboza@ventanamicro.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>, Ilya Leoshkevich <iii@linux.ibm.com>, David Hildenbrand <david@kernel.org>, Thomas Huth <thuth@redhat.com>, Max Filippov <jcmvbkbc@gmail.com>
[PATCH 2/5] target/ppc: Restrict WatchPoint API to TCG
Posted by Philippe Mathieu-Daudé 1 month ago
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


Re: [PATCH 2/5] target/ppc: Restrict WatchPoint API to TCG
Posted by Chinmay Rath 1 month ago
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 2/5] target/ppc: Restrict WatchPoint API to TCG
Posted by Pierrick Bouvier 1 month ago
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>