All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Philippe Mathieu-Daudé" <f4bug@amsat.org>
To: qemu-devel@nongnu.org
Cc: "Peter Maydell" <peter.maydell@linaro.org>,
	"Aleksandar Rikalo" <aleksandar.rikalo@syrmia.com>,
	"Luc Michel" <luc@lmichel.fr>,
	"Philippe Mathieu-Daudé" <f4bug@amsat.org>,
	"Hao Wu" <wuhaotsh@google.com>,
	"Aurelien Jarno" <aurelien@aurel32.net>
Subject: [RFC PATCH] target/mips/cp0_timer: Use new clock_ns_to_ticks()
Date: Thu, 11 Feb 2021 00:43:34 +0100	[thread overview]
Message-ID: <20210210234334.3750022-1-f4bug@amsat.org> (raw)

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



             reply	other threads:[~2021-02-10 23:45 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-10 23:43 Philippe Mathieu-Daudé [this message]
2021-02-11 11:25 ` [RFC PATCH] target/mips/cp0_timer: Use new clock_ns_to_ticks() no-reply
2021-02-11 11:34   ` Philippe Mathieu-Daudé
2021-02-21 20:07 ` Philippe Mathieu-Daudé
2021-02-21 20:12   ` Peter Maydell
2021-02-21 22:51     ` Philippe Mathieu-Daudé

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210210234334.3750022-1-f4bug@amsat.org \
    --to=f4bug@amsat.org \
    --cc=aleksandar.rikalo@syrmia.com \
    --cc=aurelien@aurel32.net \
    --cc=luc@lmichel.fr \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=wuhaotsh@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.