[RFC PATCH-for-6.1 v2 0/6] target/mips/cp0_timer: Use new clock_ns_to_ticks()

[RFC PATCH-for-6.1 v2 0/6] target/mips/cp0_timer: Use new clock_ns_to_ticks()

[Philippe Mathieu-Daudé]

Since v1:
- Fix clock_has_source()
- Split patch to be more digestible
- Raise loongson3_virt/TCG clock

This series uses the recently introduced clock_ns_to_ticks()
function which "tells me how many times this clock would tick
in this length of time" to simplify the MIPS CP0 R4K timer
logic and remove the restriction to use frequencies >= 1GHz.

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

Also, env_archcpu() could eventually be dropped (by passing
MIPSCPU* instead of CPUMIPSState*).

Still, enough for a first step cleanup.

Regards,

Phil.

Philippe Mathieu-Daudé (6):
  target/mips/cpu: Use clock_has_source() instead of clock_get()
  target/mips/cpu: Update CP0 clock when CPU clock is propagated
  target/mips/cp0_timer: Add ns_to_count() helper
  target/mips/cp0_timer: Add ns_substract_to_count() helper
  target/mips/cp0_timer: Use new clock_ns_to_ticks()
  hw/mips/loongson3_virt: Raise CPU clock to 2 GHz

 target/mips/cpu.h        |  1 -
 hw/mips/loongson3_virt.c |  2 +-
 target/mips/cp0_timer.c  | 45 ++++++++++++++++++++++++++++++++--------
 target/mips/cpu.c        | 15 +++-----------
 4 files changed, 40 insertions(+), 23 deletions(-)

-- 
2.26.3

[RFC PATCH-for-6.1 v2 1/6] target/mips/cpu: Use clock_has_source() instead of clock_get()

[Philippe Mathieu-Daudé]

clock_get() returns the clock period. With an unconnected clock
we get 0. This is not what we want here. Use the API properly
by using the clock_has_source() function instead.

Fixes: a0713e85bfa ("target/mips/cpu: Allow the CPU to use dynamic frequencies")
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/mips/cpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/mips/cpu.c b/target/mips/cpu.c
index dce1e166bde..aa0b00256e6 100644
--- a/target/mips/cpu.c
+++ b/target/mips/cpu.c
@@ -612,7 +612,7 @@ static void mips_cpu_realizefn(DeviceState *dev, Error **errp)
     MIPSCPUClass *mcc = MIPS_CPU_GET_CLASS(dev);
     Error *local_err = NULL;
 
-    if (!clock_get(cpu->clock)) {
+    if (!clock_has_source(cpu->clock)) {
 #ifndef CONFIG_USER_ONLY
         if (!qtest_enabled()) {
             g_autofree char *cpu_freq_str = freq_to_str(CPU_FREQ_HZ_DEFAULT);
-- 
2.26.3

Re: [RFC PATCH-for-6.1 v2 1/6] target/mips/cpu: Use clock_has_source() instead of clock_get()

[Richard Henderson]

On 4/9/21 2:36 AM, Philippe Mathieu-Daudé wrote:
> clock_get() returns the clock period. With an unconnected clock
> we get 0. This is not what we want here. Use the API properly
> by using the clock_has_source() function instead.
> 
> Fixes: a0713e85bfa ("target/mips/cpu: Allow the CPU to use dynamic frequencies")
> Signed-off-by: Philippe Mathieu-Daudé<f4bug@amsat.org>
> ---
>   target/mips/cpu.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[RFC PATCH-for-6.1 v2 2/6] target/mips/cpu: Update CP0 clock when CPU clock is propagated

[Philippe Mathieu-Daudé]

Setting the CP0 clock simply on CPU init is incorrect. First
because the clock can not be yet propagated. Second because
we aimed to support dynamic frequencies in commit a0713e85bfa,
so the CPU frequency can be changed *after* init time.

The correct way is to register a ClockCallback, which will
update the CP0 period when the clock changes.

Fixes: a0713e85bfa ("target/mips/cpu: Allow the CPU to use dynamic frequencies")
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/mips/cpu.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/target/mips/cpu.c b/target/mips/cpu.c
index aa0b00256e6..db93d19c49a 100644
--- a/target/mips/cpu.c
+++ b/target/mips/cpu.c
@@ -604,6 +604,13 @@ static void mips_cp0_period_set(MIPSCPU *cpu)
     assert(env->cp0_count_ns);
 }
 
+static void mips_cpu_clk_update(void *opaque, ClockEvent event)
+{
+    MIPSCPU *cpu = opaque;
+
+    mips_cp0_period_set(cpu);
+}
+
 static void mips_cpu_realizefn(DeviceState *dev, Error **errp)
 {
     CPUState *cs = CPU(dev);
@@ -624,7 +631,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) {
@@ -653,7 +659,8 @@ static void mips_cpu_initfn(Object *obj)
     MIPSCPUClass *mcc = MIPS_CPU_GET_CLASS(obj);
 
     cpu_set_cpustate_pointers(cpu);
-    cpu->clock = qdev_init_clock_in(DEVICE(obj), "clk-in", NULL, cpu, 0);
+    cpu->clock = qdev_init_clock_in(DEVICE(obj), "clk-in",
+                                    mips_cpu_clk_update, cpu, ClockUpdate);
     env->cpu_model = mcc->cpu_def;
 }
 
-- 
2.26.3

Re: [RFC PATCH-for-6.1 v2 2/6] target/mips/cpu: Update CP0 clock when CPU clock is propagated

[Richard Henderson]

On 4/9/21 2:36 AM, Philippe Mathieu-Daudé wrote:
> Setting the CP0 clock simply on CPU init is incorrect. First
> because the clock can not be yet propagated. Second because
> we aimed to support dynamic frequencies in commit a0713e85bfa,
> so the CPU frequency can be changed*after*  init time.
> 
> The correct way is to register a ClockCallback, which will
> update the CP0 period when the clock changes.
> 
> Fixes: a0713e85bfa ("target/mips/cpu: Allow the CPU to use dynamic frequencies")
> Signed-off-by: Philippe Mathieu-Daudé<f4bug@amsat.org>
> ---
>   target/mips/cpu.c | 11 +++++++++--
>   1 file changed, 9 insertions(+), 2 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[RFC PATCH-for-6.1 v2 3/6] target/mips/cp0_timer: Add ns_to_count() helper

[Philippe Mathieu-Daudé]

Factor ns_to_count() out to simplify a bit.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/mips/cp0_timer.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/target/mips/cp0_timer.c b/target/mips/cp0_timer.c
index 70de95d338f..73355b44b15 100644
--- a/target/mips/cp0_timer.c
+++ b/target/mips/cp0_timer.c
@@ -27,6 +27,11 @@
 #include "sysemu/kvm.h"
 #include "internal.h"
 
+static uint32_t ns_to_count(CPUMIPSState *env, uint64_t ns)
+{
+    return ns / env->cp0_count_ns;
+}
+
 /* MIPS R4K timer */
 static void cpu_mips_timer_update(CPUMIPSState *env)
 {
@@ -34,8 +39,7 @@ static void cpu_mips_timer_update(CPUMIPSState *env)
     uint32_t wait;
 
     now_ns = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
-    wait = env->CP0_Compare - env->CP0_Count -
-           (uint32_t)(now_ns / env->cp0_count_ns);
+    wait = env->CP0_Compare - env->CP0_Count - ns_to_count(env, now_ns);
     next_ns = now_ns + (uint64_t)wait * env->cp0_count_ns;
     timer_mod(env->timer, next_ns);
 }
@@ -64,7 +68,7 @@ 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 + ns_to_count(env, now_ns);
     }
 }
 
@@ -79,9 +83,8 @@ void cpu_mips_store_count(CPUMIPSState *env, uint32_t count)
         env->CP0_Count = count;
     } else {
         /* 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 - ns_to_count(env,
+                                             qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
         /* Update timer timer */
         cpu_mips_timer_update(env);
     }
@@ -107,8 +110,7 @@ void cpu_mips_start_count(CPUMIPSState *env)
 void cpu_mips_stop_count(CPUMIPSState *env)
 {
     /* Store the current value */
-    env->CP0_Count += (uint32_t)(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) /
-                                 env->cp0_count_ns);
+    env->CP0_Count += ns_to_count(env, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
 }
 
 static void mips_timer_cb(void *opaque)
-- 
2.26.3

Re: [RFC PATCH-for-6.1 v2 3/6] target/mips/cp0_timer: Add ns_to_count() helper

[Richard Henderson]

On 4/9/21 2:36 AM, Philippe Mathieu-Daudé wrote:
> Factor ns_to_count() out to simplify a bit.
> 
> Signed-off-by: Philippe Mathieu-Daudé<f4bug@amsat.org>
> ---
>   target/mips/cp0_timer.c | 18 ++++++++++--------
>   1 file changed, 10 insertions(+), 8 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[RFC PATCH-for-6.1 v2 4/6] target/mips/cp0_timer: Add ns_substract_to_count() helper

[Philippe Mathieu-Daudé]

Factor ns_substract_to_count() out to simplify a bit.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/mips/cp0_timer.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/target/mips/cp0_timer.c b/target/mips/cp0_timer.c
index 73355b44b15..85f2f85838d 100644
--- a/target/mips/cp0_timer.c
+++ b/target/mips/cp0_timer.c
@@ -32,6 +32,12 @@ static uint32_t ns_to_count(CPUMIPSState *env, uint64_t ns)
     return ns / env->cp0_count_ns;
 }
 
+static uint32_t ns_substract_to_count(CPUMIPSState *env,
+                                      uint32_t count, uint64_t ns)
+{
+    return count - ns_to_count(env, ns);
+}
+
 /* MIPS R4K timer */
 static void cpu_mips_timer_update(CPUMIPSState *env)
 {
@@ -39,7 +45,7 @@ static void cpu_mips_timer_update(CPUMIPSState *env)
     uint32_t wait;
 
     now_ns = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
-    wait = env->CP0_Compare - env->CP0_Count - ns_to_count(env, now_ns);
+    wait = ns_substract_to_count(env, env->CP0_Compare - env->CP0_Count, now_ns);
     next_ns = now_ns + (uint64_t)wait * env->cp0_count_ns;
     timer_mod(env->timer, next_ns);
 }
@@ -83,8 +89,8 @@ void cpu_mips_store_count(CPUMIPSState *env, uint32_t count)
         env->CP0_Count = count;
     } else {
         /* Store new count register */
-        env->CP0_Count = count - ns_to_count(env,
-                                             qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
+        env->CP0_Count = ns_substract_to_count(env, count,
+                                               qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
         /* Update timer timer */
         cpu_mips_timer_update(env);
     }
-- 
2.26.3

Re: [RFC PATCH-for-6.1 v2 4/6] target/mips/cp0_timer: Add ns_substract_to_count() helper

[Richard Henderson]

On 4/9/21 2:36 AM, Philippe Mathieu-Daudé wrote:
> Factor ns_substract_to_count() out to simplify a bit.
> 
> Signed-off-by: Philippe Mathieu-Daudé<f4bug@amsat.org>
> ---
>   target/mips/cp0_timer.c | 12 +++++++++---
>   1 file changed, 9 insertions(+), 3 deletions(-)

s/substract/subtract/g
Also, one usually subtracts "from".

That said, I'm not sure I see this as clearer...


r~

[RFC PATCH-for-6.1 v2 5/6] target/mips/cp0_timer: Use new clock_ns_to_ticks()

[Philippe Mathieu-Daudé]

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() and mips_cpu_clk_update().

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 target/mips/cpu.h       |  1 -
 target/mips/cp0_timer.c | 41 ++++++++++++++++++++++++++++++-----------
 target/mips/cpu.c       | 18 +-----------------
 3 files changed, 31 insertions(+), 29 deletions(-)

diff --git a/target/mips/cpu.h b/target/mips/cpu.h
index 075c24abdad..10f10018b95 100644
--- a/target/mips/cpu.h
+++ b/target/mips/cpu.h
@@ -1160,7 +1160,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 85f2f85838d..3b76cc97f6a 100644
--- a/target/mips/cp0_timer.c
+++ b/target/mips/cp0_timer.c
@@ -27,26 +27,31 @@
 #include "sysemu/kvm.h"
 #include "internal.h"
 
-static uint32_t ns_to_count(CPUMIPSState *env, uint64_t ns)
+static uint32_t tick_to_count(MIPSCPU *cpu, uint64_t ticks)
 {
-    return ns / env->cp0_count_ns;
+    return ticks / cpu->cp0_count_rate;
 }
 
-static uint32_t ns_substract_to_count(CPUMIPSState *env,
-                                      uint32_t count, uint64_t ns)
+static uint32_t tick_substract_to_count(MIPSCPU *cpu,
+                                        uint32_t count, uint64_t ticks)
 {
-    return count - ns_to_count(env, ns);
+    return count - tick_to_count(cpu, ticks);
 }
 
 /* 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 = ns_substract_to_count(env, env->CP0_Compare - env->CP0_Count, now_ns);
-    next_ns = now_ns + (uint64_t)wait * env->cp0_count_ns;
+    now_ticks = clock_ns_to_ticks(cpu->clock, now_ns);
+    wait = tick_substract_to_count(cpu, env->CP0_Compare - env->CP0_Count,
+                                   now_ticks);
+    next_ns = now_ns + (uint64_t)wait * clock_ticks_to_ns(cpu->clock,
+                                                          cpu->cp0_count_rate);
     timer_mod(env->timer, next_ns);
 }
 
@@ -65,6 +70,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);
@@ -74,12 +80,16 @@ uint32_t cpu_mips_get_count(CPUMIPSState *env)
             cpu_mips_timer_expire(env);
         }
 
-        return env->CP0_Count + ns_to_count(env, now_ns);
+        return env->CP0_Count + tick_to_count(cpu,
+                                              clock_ns_to_ticks(cpu->clock,
+                                                                now_ns));
     }
 }
 
 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
@@ -88,9 +98,13 @@ 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 = ns_substract_to_count(env, count,
-                                               qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
+        env->CP0_Count = tick_substract_to_count(cpu, count, cp0_count_ticks);
+
         /* Update timer timer */
         cpu_mips_timer_update(env);
     }
@@ -115,8 +129,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 += ns_to_count(env, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
+    env->CP0_Count += tick_to_count(cpu, cp0_count_ticks);
 }
 
 static void mips_timer_cb(void *opaque)
diff --git a/target/mips/cpu.c b/target/mips/cpu.c
index db93d19c49a..a66caa44bee 100644
--- a/target/mips/cpu.c
+++ b/target/mips/cpu.c
@@ -595,22 +595,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_clk_update(void *opaque, ClockEvent event)
-{
-    MIPSCPU *cpu = opaque;
-
-    mips_cp0_period_set(cpu);
-}
-
 static void mips_cpu_realizefn(DeviceState *dev, Error **errp)
 {
     CPUState *cs = CPU(dev);
@@ -660,7 +644,7 @@ static void mips_cpu_initfn(Object *obj)
 
     cpu_set_cpustate_pointers(cpu);
     cpu->clock = qdev_init_clock_in(DEVICE(obj), "clk-in",
-                                    mips_cpu_clk_update, cpu, ClockUpdate);
+                                    NULL, NULL, ClockUpdate);
     env->cpu_model = mcc->cpu_def;
 }
 
-- 
2.26.3

Re: [RFC PATCH-for-6.1 v2 5/6] target/mips/cp0_timer: Use new clock_ns_to_ticks()

[Richard Henderson]

On 4/9/21 2:36 AM, Philippe Mathieu-Daudé wrote:
> -static uint32_t ns_to_count(CPUMIPSState *env, uint64_t ns)
> +static uint32_t tick_to_count(MIPSCPU *cpu, uint64_t ticks)
>   {
> -    return ns / env->cp0_count_ns;
> +    return ticks / cpu->cp0_count_rate;
>   }

I'm not clear on the difference between ticks and counts, and this change looks 
suspicious in terms of units.  Hasn't cp0_count_rate been used to initialize 
the clock in the first place?  And if so, why didn't clock_ns_to_ticks do the 
entire job?


r~

[RFC PATCH-for-6.1 v2 6/6] hw/mips/loongson3_virt: Raise CPU clock to 2 GHz

[Philippe Mathieu-Daudé]

Commit cd3a53b727d ("clock: Add clock_ns_to_ticks() function")
removed the limitation of using clock with a frequency of 1 GHz
or more.

The previous commit converted the MIPS CP0 timer to use this
new clock_ns_to_ticks() function. We can now use a clock of
2 GHz with the Loongson3 virt board.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/mips/loongson3_virt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/mips/loongson3_virt.c b/hw/mips/loongson3_virt.c
index b15071defc6..0b72ef8a684 100644
--- a/hw/mips/loongson3_virt.c
+++ b/hw/mips/loongson3_virt.c
@@ -152,7 +152,7 @@ static const MemoryRegionOps loongson3_pm_ops = {
     }
 };
 
-#define DEF_LOONGSON3_FREQ (800 * 1000 * 1000)
+#define DEF_LOONGSON3_FREQ (2000 * 1000 * 1000)
 
 static uint64_t get_cpu_freq_hz(void)
 {
-- 
2.26.3

Re: [RFC PATCH-for-6.1 v2 6/6] hw/mips/loongson3_virt: Raise CPU clock to 2 GHz

[Huacai Chen]

Hi, Philippe,

On Fri, Apr 9, 2021 at 5:36 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Commit cd3a53b727d ("clock: Add clock_ns_to_ticks() function")
> removed the limitation of using clock with a frequency of 1 GHz
> or more.
>
> The previous commit converted the MIPS CP0 timer to use this
> new clock_ns_to_ticks() function. We can now use a clock of
> 2 GHz with the Loongson3 virt board.
Yes, we can do this, but why should we do this? I don't think this can
bring any advantages.

Huacai
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/mips/loongson3_virt.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/mips/loongson3_virt.c b/hw/mips/loongson3_virt.c
> index b15071defc6..0b72ef8a684 100644
> --- a/hw/mips/loongson3_virt.c
> +++ b/hw/mips/loongson3_virt.c
> @@ -152,7 +152,7 @@ static const MemoryRegionOps loongson3_pm_ops = {
>      }
>  };
>
> -#define DEF_LOONGSON3_FREQ (800 * 1000 * 1000)
> +#define DEF_LOONGSON3_FREQ (2000 * 1000 * 1000)
>
>  static uint64_t get_cpu_freq_hz(void)
>  {
> --
> 2.26.3
>

Re: [RFC PATCH-for-6.1 v2 6/6] hw/mips/loongson3_virt: Raise CPU clock to 2 GHz

[Philippe Mathieu-Daudé]

Hi Huacai,

On 4/10/21 4:43 AM, Huacai Chen wrote:
> Hi, Philippe,
> 
> On Fri, Apr 9, 2021 at 5:36 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>>
>> Commit cd3a53b727d ("clock: Add clock_ns_to_ticks() function")
>> removed the limitation of using clock with a frequency of 1 GHz
>> or more.
>>
>> The previous commit converted the MIPS CP0 timer to use this
>> new clock_ns_to_ticks() function. We can now use a clock of
>> 2 GHz with the Loongson3 virt board.
> Yes, we can do this, but why should we do this? I don't think this can
> bring any advantages.

IIRC this was how you sent the earlier series, then we had to reduce
the frequency to <1GHz due to the DIV#0 bug. Now it is fixed I thought
you'd get this back.

I spent time with the R4K timer because it is often used by embedded
firmwares, and a mismatch with the CPU clock makes firmware not work
well. I suppose when using Linux guests it is not a real issue, because
1/ if there is another timer (different peripheral on a system-on-soc)
Linux will use it first, 2/ Linux does some early check to adapt with
the tick rate IIRC (it doesn't assume a precise rate).

I'm fine with the current Loongson3 virt board behavior with TCG,
so let's disregard this patch.

Thanks,

Phil.