[RFC PATCH] target/mips/cp0_timer: Use new clock_ns_to_ticks()

* [RFC PATCH] target/mips/cp0_timer: Use new clock_ns_to_ticks()
@ 2021-02-10 23:43 Philippe Mathieu-Daudé
  2021-02-11 11:25 ` no-reply
  2021-02-21 20:07 ` Philippe Mathieu-Daudé
  0 siblings, 2 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-02-10 23:43 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Aleksandar Rikalo, Luc Michel,
	Philippe Mathieu-Daudé,
	Hao Wu, Aurelien Jarno

Use the new clock_ns_to_ticks() function in cp0_timer where
appropriate. This allows us to remove CPUMIPSState::cp0_count_ns
and mips_cp0_period_set().

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
Based-on: <20210209132040.5091-1-peter.maydell@linaro.org>

RFC because this is just a starter patch to test Peter's series
providing a handy function which "tells me how many times this
clock would tick in this length of time".

Also because we could rethink the MIPS CP0 timer logic to avoid
too frequent divu128 calls (painful on 32-bit hosts).

The style should be updated, using more variables for easier
review. env_archcpu() could eventually be dropped (by passing
MIPSCPU* instead of CPUMIPSState*).

This passes my MIPS tier1 tests, kicking CI before going to bed.
Posting meanwhile for comments.
---
 target/mips/cpu.h       |  1 -
 target/mips/cp0_timer.c | 34 +++++++++++++++++++++++++---------
 target/mips/cpu.c       | 10 ----------
 3 files changed, 25 insertions(+), 20 deletions(-)

diff --git a/target/mips/cpu.h b/target/mips/cpu.h
index b9e227a30e9..946748d8cc7 100644
--- a/target/mips/cpu.h
+++ b/target/mips/cpu.h
@@ -1156,7 +1156,6 @@ struct CPUMIPSState {
     struct MIPSITUState *itu;
     MemoryRegion *itc_tag; /* ITC Configuration Tags */
     target_ulong exception_base; /* ExceptionBase input to the core */
-    uint64_t cp0_count_ns; /* CP0_Count clock period (in nanoseconds) */
 };
 
 /**
diff --git a/target/mips/cp0_timer.c b/target/mips/cp0_timer.c
index 70de95d338f..3a128302122 100644
--- a/target/mips/cp0_timer.c
+++ b/target/mips/cp0_timer.c
@@ -30,13 +30,17 @@
 /* MIPS R4K timer */
 static void cpu_mips_timer_update(CPUMIPSState *env)
 {
+    MIPSCPU *cpu = env_archcpu(env);
     uint64_t now_ns, next_ns;
     uint32_t wait;
+    uint64_t now_ticks;
 
     now_ns = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
-    wait = env->CP0_Compare - env->CP0_Count -
-           (uint32_t)(now_ns / env->cp0_count_ns);
-    next_ns = now_ns + (uint64_t)wait * env->cp0_count_ns;
+    now_ticks = clock_ns_to_ticks(cpu->clock, now_ns);
+    wait = env->CP0_Compare - env->CP0_Count
+           - (uint32_t)(now_ticks / cpu->cp0_count_rate);
+    next_ns = now_ns + (uint64_t)wait * clock_ticks_to_ns(cpu->clock,
+                                                          cpu->cp0_count_rate);
     timer_mod(env->timer, next_ns);
 }
 
@@ -55,6 +59,7 @@ uint32_t cpu_mips_get_count(CPUMIPSState *env)
     if (env->CP0_Cause & (1 << CP0Ca_DC)) {
         return env->CP0_Count;
     } else {
+        MIPSCPU *cpu = env_archcpu(env);
         uint64_t now_ns;
 
         now_ns = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
@@ -64,12 +69,15 @@ uint32_t cpu_mips_get_count(CPUMIPSState *env)
             cpu_mips_timer_expire(env);
         }
 
-        return env->CP0_Count + (uint32_t)(now_ns / env->cp0_count_ns);
+        return env->CP0_Count + (uint32_t)(clock_ns_to_ticks(cpu->clock, now_ns)
+                                           / cpu->cp0_count_rate);
     }
 }
 
 void cpu_mips_store_count(CPUMIPSState *env, uint32_t count)
 {
+    MIPSCPU *cpu = env_archcpu(env);
+
     /*
      * This gets called from cpu_state_reset(), potentially before timer init.
      * So env->timer may be NULL, which is also the case with KVM enabled so
@@ -78,10 +86,14 @@ void cpu_mips_store_count(CPUMIPSState *env, uint32_t count)
     if (env->CP0_Cause & (1 << CP0Ca_DC) || !env->timer) {
         env->CP0_Count = count;
     } else {
+        uint64_t cp0_count_ticks;
+
+        cp0_count_ticks = clock_ns_to_ticks(cpu->clock,
+                                            qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
         /* Store new count register */
-        env->CP0_Count = count -
-               (uint32_t)(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) /
-                          env->cp0_count_ns);
+        env->CP0_Count = count - (uint32_t)(cp0_count_ticks / cpu->cp0_count_rate);
+
+
         /* Update timer timer */
         cpu_mips_timer_update(env);
     }
@@ -106,9 +118,13 @@ void cpu_mips_start_count(CPUMIPSState *env)
 
 void cpu_mips_stop_count(CPUMIPSState *env)
 {
+    MIPSCPU *cpu = env_archcpu(env);
+    uint64_t cp0_count_ticks;
+
+    cp0_count_ticks = clock_ns_to_ticks(cpu->clock,
+                                        qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
     /* Store the current value */
-    env->CP0_Count += (uint32_t)(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) /
-                                 env->cp0_count_ns);
+    env->CP0_Count += (uint32_t)(cp0_count_ticks / cpu->cp0_count_rate);
 }
 
 static void mips_timer_cb(void *opaque)
diff --git a/target/mips/cpu.c b/target/mips/cpu.c
index 2f3d9d2ce2c..17caf236da9 100644
--- a/target/mips/cpu.c
+++ b/target/mips/cpu.c
@@ -578,15 +578,6 @@ static void mips_cpu_disas_set_info(CPUState *s, disassemble_info *info)
 #define CPU_FREQ_HZ_DEFAULT     200000000
 #define CP0_COUNT_RATE_DEFAULT  2
 
-static void mips_cp0_period_set(MIPSCPU *cpu)
-{
-    CPUMIPSState *env = &cpu->env;
-
-    env->cp0_count_ns = clock_ticks_to_ns(MIPS_CPU(cpu)->clock,
-                                          cpu->cp0_count_rate);
-    assert(env->cp0_count_ns);
-}
-
 static void mips_cpu_realizefn(DeviceState *dev, Error **errp)
 {
     CPUState *cs = CPU(dev);
@@ -607,7 +598,6 @@ static void mips_cpu_realizefn(DeviceState *dev, Error **errp)
         /* Initialize the frequency in case the clock remains unconnected. */
         clock_set_hz(cpu->clock, CPU_FREQ_HZ_DEFAULT);
     }
-    mips_cp0_period_set(cpu);
 
     cpu_exec_realizefn(cs, &local_err);
     if (local_err != NULL) {
-- 
2.26.2



^ permalink raw reply related	[flat|nested] 6+ messages in thread