On 3/18/25 14:31, Richard Henderson wrote:
> Relatively few objects in qemu care about watchpoints, so split
> out to a new header. Removes an instance of CONFIG_USER_ONLY
> from hw/core/cpu.h.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> include/exec/watchpoint.h | 41 +++++++++++++++++++++++++++++
> include/hw/core/cpu.h | 30 ---------------------
> accel/tcg/tcg-accel-ops.c | 1 +
> system/watchpoint.c | 1 +
> target/arm/debug_helper.c | 1 +
> target/i386/cpu.c | 1 +
> target/i386/machine.c | 2 +-
> target/i386/tcg/system/bpt_helper.c | 1 +
> target/ppc/cpu.c | 1 +
> target/ppc/cpu_init.c | 2 +-
> target/riscv/debug.c | 1 +
> target/s390x/helper.c | 1 +
> target/s390x/tcg/excp_helper.c | 1 +
> target/xtensa/dbg_helper.c | 1 +
> 14 files changed, 53 insertions(+), 32 deletions(-)
> create mode 100644 include/exec/watchpoint.h
>
> diff --git a/include/exec/watchpoint.h b/include/exec/watchpoint.h
> new file mode 100644
> index 0000000000..4b6668826c
> --- /dev/null
> +++ b/include/exec/watchpoint.h
> @@ -0,0 +1,41 @@
> +/*
> + * 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
> +
> +#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/include/hw/core/cpu.h b/include/hw/core/cpu.h
> index 5d11d26556..d1c1fefea3 100644
> --- a/include/hw/core/cpu.h
> +++ b/include/hw/core/cpu.h
> @@ -1109,36 +1109,6 @@ static inline bool cpu_breakpoint_test(CPUState *cpu, vaddr pc, int mask)
> return false;
> }
>
> -#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
> -
> /**
> * cpu_get_address_space:
> * @cpu: CPU to get address space from
> diff --git a/accel/tcg/tcg-accel-ops.c b/accel/tcg/tcg-accel-ops.c
> index d9b662efe3..5c88056157 100644
> --- a/accel/tcg/tcg-accel-ops.c
> +++ b/accel/tcg/tcg-accel-ops.c
> @@ -37,6 +37,7 @@
> #include "exec/hwaddr.h"
> #include "exec/tb-flush.h"
> #include "exec/translation-block.h"
> +#include "exec/watchpoint.h"
> #include "gdbstub/enums.h"
>
> #include "hw/core/cpu.h"
> diff --git a/system/watchpoint.c b/system/watchpoint.c
> index 08dbd8483d..21d0bb36ca 100644
> --- a/system/watchpoint.c
> +++ b/system/watchpoint.c
> @@ -21,6 +21,7 @@
> #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. */
> diff --git a/target/arm/debug_helper.c b/target/arm/debug_helper.c
> index a9a619ba6b..473ee2af38 100644
> --- a/target/arm/debug_helper.c
> +++ b/target/arm/debug_helper.c
> @@ -13,6 +13,7 @@
> #include "cpregs.h"
> #include "exec/exec-all.h"
> #include "exec/helper-proto.h"
> +#include "exec/watchpoint.h"
> #include "system/tcg.h"
>
> #ifdef CONFIG_TCG
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index dba1b3ffef..af46c7a392 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -35,6 +35,7 @@
> #include "standard-headers/asm-x86/kvm_para.h"
> #include "hw/qdev-properties.h"
> #include "hw/i386/topology.h"
> +#include "exec/watchpoint.h"
> #ifndef CONFIG_USER_ONLY
> #include "system/reset.h"
> #include "qapi/qapi-commands-machine-target.h"
> diff --git a/target/i386/machine.c b/target/i386/machine.c
> index 70f632a36f..6cb561c632 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 "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 be232c1ca9..08ccd3f5e6 100644
> --- a/target/i386/tcg/system/bpt_helper.c
> +++ b/target/i386/tcg/system/bpt_helper.c
> @@ -21,6 +21,7 @@
> #include "cpu.h"
> #include "exec/exec-all.h"
> #include "exec/helper-proto.h"
> +#include "exec/watchpoint.h"
> #include "tcg/helper-tcg.h"
>
>
> diff --git a/target/ppc/cpu.c b/target/ppc/cpu.c
> index bfcc695de7..4d8faaddee 100644
> --- a/target/ppc/cpu.c
> +++ b/target/ppc/cpu.c
> @@ -22,6 +22,7 @@
> #include "cpu-models.h"
> #include "cpu-qom.h"
> #include "exec/log.h"
> +#include "exec/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 8b590e7f17..7394ffc557 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 "helper_regs.h"
> #include "internal.h"
> #include "spr_common.h"
> diff --git a/target/riscv/debug.c b/target/riscv/debug.c
> index 9db4048523..fea989afe9 100644
> --- a/target/riscv/debug.c
> +++ b/target/riscv/debug.c
> @@ -30,6 +30,7 @@
> #include "trace.h"
> #include "exec/exec-all.h"
> #include "exec/helper-proto.h"
> +#include "exec/watchpoint.h"
> #include "system/cpu-timers.h"
>
> /*
> diff --git a/target/s390x/helper.c b/target/s390x/helper.c
> index c689e11b46..e660c69f60 100644
> --- a/target/s390x/helper.c
> +++ b/target/s390x/helper.c
> @@ -27,6 +27,7 @@
> #include "target/s390x/kvm/pv.h"
> #include "system/hw_accel.h"
> #include "system/runstate.h"
> +#include "exec/watchpoint.h"
>
> void s390x_tod_timer(void *opaque)
> {
> diff --git a/target/s390x/tcg/excp_helper.c b/target/s390x/tcg/excp_helper.c
> index ac733f407f..1d51043e88 100644
> --- a/target/s390x/tcg/excp_helper.c
> +++ b/target/s390x/tcg/excp_helper.c
> @@ -24,6 +24,7 @@
> #include "exec/helper-proto.h"
> #include "exec/cputlb.h"
> #include "exec/exec-all.h"
> +#include "exec/watchpoint.h"
> #include "s390x-internal.h"
> #include "tcg_s390x.h"
> #ifndef CONFIG_USER_ONLY
> diff --git a/target/xtensa/dbg_helper.c b/target/xtensa/dbg_helper.c
> index 163a1ffc7b..c4f4298a50 100644
> --- a/target/xtensa/dbg_helper.c
> +++ b/target/xtensa/dbg_helper.c
> @@ -31,6 +31,7 @@
> #include "exec/helper-proto.h"
> #include "qemu/host-utils.h"
> #include "exec/exec-all.h"
> +#include "exec/watchpoint.h"
> #include "system/address-spaces.h"
>
> void HELPER(wsr_ibreakenable)(CPUXtensaState *env, uint32_t v)
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
To push further, it could be worth to remove inline stubs, put
definitions in common-user/watchpoint.c, and add this file to user_ss.