Re: [PATCH 25/37] common-user: Split out watchpoint-stub.c

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 13/3/25 11:07, Philippe Mathieu-Daudé wrote:
> On 13/3/25 04:45, Richard Henderson wrote:
>> Uninline the user-only stubs from hw/core/cpu.h.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>   include/hw/core/cpu.h         | 23 -----------------------
>>   common-user/watchpoint-stub.c | 28 ++++++++++++++++++++++++++++
>>   common-user/meson.build       |  1 +
>>   3 files changed, 29 insertions(+), 23 deletions(-)
>>   create mode 100644 common-user/watchpoint-stub.c
>>
>> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
>> index 5d11d26556..2fdb115b19 100644
>> --- a/include/hw/core/cpu.h
>> +++ b/include/hw/core/cpu.h
>> @@ -1109,35 +1109,12 @@ 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:
>> diff --git a/common-user/watchpoint-stub.c b/common-user/watchpoint- 
>> stub.c
>> new file mode 100644
>> index 0000000000..2489fca4f3
>> --- /dev/null
>> +++ b/common-user/watchpoint-stub.c
>> @@ -0,0 +1,28 @@
>> +/*
>> + * CPU watchpoint stubs
>> + *
>> + * Copyright (c) 2003 Fabrice Bellard
>> + * SPDX-License-Identifier: LGPL-2.1-or-later
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "hw/core/cpu.h"
>> +
>> +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)
>> +{
> 
> Again, can this be elide? Otherwise better use g_assert_not_reached().

We can, including:

-- >8 --
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index dba1b3ffef..54d3879c56 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -7545,4 +7545,6 @@ static void x86_cpu_reset_hold(Object *obj, 
ResetType type)
      env->dr[7] = DR7_FIXED_1;
+#ifndef CONFIG_USER_ONLY
      cpu_breakpoint_remove_all(cs, BP_CPU);
      cpu_watchpoint_remove_all(cs, BP_CPU);
+#endif

diff --git a/target/arm/debug_helper.c b/target/arm/debug_helper.c
index a9a619ba6b..f798569de7 100644
--- a/target/arm/debug_helper.c
+++ b/target/arm/debug_helper.c
@@ -368,2 +368,3 @@ static bool check_watchpoints(ARMCPU *cpu)

+#ifndef CONFIG_USER_ONLY
      for (n = 0; n < ARRAY_SIZE(env->cpu_watchpoint); n++) {
@@ -373,2 +374,3 @@ static bool check_watchpoints(ARMCPU *cpu)
      }
+#endif
      return false;
@@ -555,2 +557,3 @@ void hw_watchpoint_update(ARMCPU *cpu, int n)

+#ifndef CONFIG_USER_ONLY
      if (env->cpu_watchpoint[n]) {
@@ -559,2 +562,3 @@ void hw_watchpoint_update(ARMCPU *cpu, int n)
      }
+#endif

@@ -631,4 +635,6 @@ void hw_watchpoint_update(ARMCPU *cpu, int n)

+#ifndef CONFIG_USER_ONLY
      cpu_watchpoint_insert(CPU(cpu), wvr, len, flags,
                            &env->cpu_watchpoint[n]);
+#endif
  }
@@ -637,3 +643,3 @@ void hw_watchpoint_update_all(ARMCPU *cpu)
  {
-    int i;
+#ifndef CONFIG_USER_ONLY
      CPUARMState *env = &cpu->env;
@@ -646,4 +652,5 @@ void hw_watchpoint_update_all(ARMCPU *cpu)
      memset(env->cpu_watchpoint, 0, sizeof(env->cpu_watchpoint));
+#endif

-    for (i = 0; i < ARRAY_SIZE(cpu->env.cpu_watchpoint); i++) {
+    for (unsigned i = 0; i < ARRAY_SIZE(cpu->env.cpu_watchpoint); i++) {
          hw_watchpoint_update(cpu, i);
---