The count_clock pointer is not something we can do a shallow copy of,
as linux-user cpu_copy() does, and although it is a system-mode piece
of state we unconditionally create it, so it is present also in
user-mode.

There isn't any need to keep this in the env struct rather than the
CPU struct, so move it to avoid possible memory leaks or
double-usage. This also puts it next to the other Clocks that this
CPU has.

I haven't seen any sanitizer reports about this field, so this is
averting a possible problem rather than correcting an observed one.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/mips/cpu.c              |  4 ++--
 target/mips/cpu.h              |  2 +-
 target/mips/system/cp0_timer.c | 12 ++++++++----
 3 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/target/mips/cpu.c b/target/mips/cpu.c
index 0663cda003..f803d47763 100644
--- a/target/mips/cpu.c
+++ b/target/mips/cpu.c
@@ -449,7 +449,7 @@ static void mips_cp0_period_set(MIPSCPU *cpu)
 
     clock_set_mul_div(cpu->count_div, env->cpu_model->CCRes, 1);
     clock_set_source(cpu->count_div, cpu->clock);
-    clock_set_source(env->count_clock, cpu->count_div);
+    clock_set_source(cpu->count_clock, cpu->count_div);
 }
 
 static void mips_cpu_realizefn(DeviceState *dev, Error **errp)
@@ -520,7 +520,7 @@ static void mips_cpu_initfn(Object *obj)
 
     cpu->clock = qdev_init_clock_in(DEVICE(obj), "clk-in", NULL, cpu, 0);
     cpu->count_div = clock_new(OBJECT(obj), "clk-div-count");
-    env->count_clock = clock_new(OBJECT(obj), "clk-count");
+    cpu->count_clock = clock_new(OBJECT(obj), "clk-count");
     env->cpu_model = mcc->cpu_def;
 }
 
diff --git a/target/mips/cpu.h b/target/mips/cpu.h
index ca36ca0d6f..cb72be9336 100644
--- a/target/mips/cpu.h
+++ b/target/mips/cpu.h
@@ -1188,7 +1188,6 @@ typedef struct CPUArchState {
 
     const mips_def_t *cpu_model;
     QEMUTimer *timer; /* Internal timer */
-    Clock *count_clock; /* CP0_Count clock */
     target_ulong exception_base; /* ExceptionBase input to the core */
 } CPUMIPSState;
 
@@ -1206,6 +1205,7 @@ struct ArchCPU {
     CPUMIPSState env;
 
     Clock *clock;
+    Clock *count_clock; /* CP0_Count clock */
     Clock *count_div; /* Divider for CP0_Count clock */
 
     CPUMIPSMVPContext *mvp;
diff --git a/target/mips/system/cp0_timer.c b/target/mips/system/cp0_timer.c
index afa163c319..634c2a66bb 100644
--- a/target/mips/system/cp0_timer.c
+++ b/target/mips/system/cp0_timer.c
@@ -29,14 +29,16 @@
 /* MIPS R4K timer */
 static uint32_t cpu_mips_get_count_val(CPUMIPSState *env)
 {
+    MIPSCPU *cpu = env_archcpu(env);
     int64_t now_ns;
     now_ns = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
     return env->CP0_Count +
-            (uint32_t)clock_ns_to_ticks(env->count_clock, now_ns);
+            (uint32_t)clock_ns_to_ticks(cpu->count_clock, now_ns);
 }
 
 static void cpu_mips_timer_update(CPUMIPSState *env)
 {
+    MIPSCPU *cpu = env_archcpu(env);
     uint64_t now_ns, next_ns;
     uint32_t wait;
 
@@ -46,7 +48,7 @@ static void cpu_mips_timer_update(CPUMIPSState *env)
     if (!wait) {
         wait = UINT32_MAX;
     }
-    next_ns = now_ns + clock_ticks_to_ns(env->count_clock, wait);
+    next_ns = now_ns + clock_ticks_to_ns(cpu->count_clock, wait);
     timer_mod(env->timer, next_ns);
 }
 
@@ -85,11 +87,12 @@ void cpu_mips_store_count(CPUMIPSState *env, uint32_t count)
      * So env->timer may be NULL, which is also the case with KVM enabled so
      * treat timer as disabled in that case.
      */
+    MIPSCPU *cpu = env_archcpu(env);
     if (env->CP0_Cause & (1 << CP0Ca_DC) || !env->timer) {
         env->CP0_Count = count;
     } else {
         /* Store new count register */
-        env->CP0_Count = count - (uint32_t)clock_ns_to_ticks(env->count_clock,
+        env->CP0_Count = count - (uint32_t)clock_ns_to_ticks(cpu->count_clock,
                         qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
         /* Update timer timer */
         cpu_mips_timer_update(env);
@@ -116,7 +119,8 @@ void cpu_mips_start_count(CPUMIPSState *env)
 void cpu_mips_stop_count(CPUMIPSState *env)
 {
     /* Store the current value */
-    env->CP0_Count += (uint32_t)clock_ns_to_ticks(env->count_clock,
+    MIPSCPU *cpu = env_archcpu(env);
+    env->CP0_Count += (uint32_t)clock_ns_to_ticks(cpu->count_clock,
                         qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
 }
 
-- 
2.43.0

On 17/3/26 18:50, Peter Maydell wrote:
> The count_clock pointer is not something we can do a shallow copy of,
> as linux-user cpu_copy() does, and although it is a system-mode piece
> of state we unconditionally create it, so it is present also in
> user-mode.
> 
> There isn't any need to keep this in the env struct rather than the
> CPU struct, so move it to avoid possible memory leaks or
> double-usage. This also puts it next to the other Clocks that this
> CPU has.
> 
> I haven't seen any sanitizer reports about this field, so this is
> averting a possible problem rather than correcting an observed one.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   target/mips/cpu.c              |  4 ++--
>   target/mips/cpu.h              |  2 +-
>   target/mips/system/cp0_timer.c | 12 ++++++++----
>   3 files changed, 11 insertions(+), 7 deletions(-)

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