diff mbox

[pic32,v2,1/5] Speed of MIPS CPU timer made configurable per platform.

Message ID ea933a5c69f2b15ddbb55a5526af30022029c5c7.1435723168.git.serge.vakulenko@gmail.com
State New
Headers show

Commit Message

Serge Vakulenko July 1, 2015, 4:12 a.m. UTC
Signed-off-by: Serge Vakulenko <serge.vakulenko@gmail.com>
---
 hw/mips/cputimer.c        | 13 +++++++------
 hw/mips/mips_fulong2e.c   |  2 +-
 hw/mips/mips_jazz.c       |  2 +-
 hw/mips/mips_malta.c      |  4 ++--
 hw/mips/mips_mipssim.c    |  2 +-
 hw/mips/mips_r4k.c        |  2 +-
 include/hw/mips/cpudevs.h |  2 +-
 target-mips/cpu.h         |  1 +
 8 files changed, 15 insertions(+), 13 deletions(-)

Comments

Aurelien Jarno July 1, 2015, 10:02 a.m. UTC | #1
On 2015-06-30 21:12, Serge Vakulenko wrote:
> Signed-off-by: Serge Vakulenko <serge.vakulenko@gmail.com>
> ---
>  hw/mips/cputimer.c        | 13 +++++++------
>  hw/mips/mips_fulong2e.c   |  2 +-
>  hw/mips/mips_jazz.c       |  2 +-
>  hw/mips/mips_malta.c      |  4 ++--
>  hw/mips/mips_mipssim.c    |  2 +-
>  hw/mips/mips_r4k.c        |  2 +-
>  include/hw/mips/cpudevs.h |  2 +-
>  target-mips/cpu.h         |  1 +
>  8 files changed, 15 insertions(+), 13 deletions(-)
> 
> diff --git a/hw/mips/cputimer.c b/hw/mips/cputimer.c
> index 577c9ae..4f02a9f 100644
> --- a/hw/mips/cputimer.c
> +++ b/hw/mips/cputimer.c
> @@ -50,8 +50,8 @@ static void cpu_mips_timer_update(CPUMIPSState *env)
>  
>      now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
>      wait = env->CP0_Compare - env->CP0_Count -
> -	    (uint32_t)muldiv64(now, TIMER_FREQ, get_ticks_per_sec());
> -    next = now + muldiv64(wait, get_ticks_per_sec(), TIMER_FREQ);
> +        (uint32_t)muldiv64(now, env->count_freq, get_ticks_per_sec());
> +    next = now + muldiv64(wait, get_ticks_per_sec(), env->count_freq);
>      timer_mod(env->timer, next);
>  }
>  
> @@ -80,7 +80,7 @@ uint32_t cpu_mips_get_count (CPUMIPSState *env)
>          }
>  
>          return env->CP0_Count +
> -            (uint32_t)muldiv64(now, TIMER_FREQ, get_ticks_per_sec());
> +            (uint32_t)muldiv64(now, env->count_freq, get_ticks_per_sec());
>      }
>  }
>  
> @@ -97,7 +97,7 @@ void cpu_mips_store_count (CPUMIPSState *env, uint32_t count)
>          /* Store new count register */
>          env->CP0_Count =
>              count - (uint32_t)muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
> -                                       TIMER_FREQ, get_ticks_per_sec());
> +                                       env->count_freq, get_ticks_per_sec());
>          /* Update timer timer */
>          cpu_mips_timer_update(env);
>      }
> @@ -122,7 +122,7 @@ void cpu_mips_stop_count(CPUMIPSState *env)
>  {
>      /* Store the current value */
>      env->CP0_Count += (uint32_t)muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
> -                                         TIMER_FREQ, get_ticks_per_sec());
> +                                         env->count_freq, get_ticks_per_sec());
>  }
>  
>  static void mips_timer_cb (void *opaque)
> @@ -145,7 +145,7 @@ static void mips_timer_cb (void *opaque)
>      env->CP0_Count--;
>  }
>  
> -void cpu_mips_clock_init (CPUMIPSState *env)
> +void cpu_mips_clock_init(CPUMIPSState *env, unsigned count_freq)
>  {
>      /*
>       * If we're in KVM mode, don't create the periodic timer, that is handled in
> @@ -153,5 +153,6 @@ void cpu_mips_clock_init (CPUMIPSState *env)
>       */
>      if (!kvm_enabled()) {
>          env->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, &mips_timer_cb, env);
> +        env->count_freq = count_freq;
>      }
>  }

So it means the value passed as an argument to this function is ignored
in the KVM case. I guess we want to be able to tell the kernel about the
request frequency.

Otherwise it looks fine.

> diff --git a/hw/mips/mips_fulong2e.c b/hw/mips/mips_fulong2e.c
> index dea941a..5decc30 100644
> --- a/hw/mips/mips_fulong2e.c
> +++ b/hw/mips/mips_fulong2e.c
> @@ -343,7 +343,7 @@ static void mips_fulong2e_init(MachineState *machine)
>  
>      /* Init internal devices */
>      cpu_mips_irq_init_cpu(env);
> -    cpu_mips_clock_init(env);
> +    cpu_mips_clock_init(env, 100*1000*1000);
>  
>      /* North bridge, Bonito --> IP2 */
>      pci_bus = bonito_init((qemu_irq *)&(env->irq[2]));
> diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c
> index 9d60633..02629fa 100644
> --- a/hw/mips/mips_jazz.c
> +++ b/hw/mips/mips_jazz.c
> @@ -210,7 +210,7 @@ static void mips_jazz_init(MachineState *machine,
>  
>      /* Init CPU internal devices */
>      cpu_mips_irq_init_cpu(env);
> -    cpu_mips_clock_init(env);
> +    cpu_mips_clock_init(env, 100*1000*1000);
>  
>      /* Chipset */
>      rc4030 = rc4030_init(&dmas, &rc4030_dma_mr);
> diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
> index 3082e75..0393017 100644
> --- a/hw/mips/mips_malta.c
> +++ b/hw/mips/mips_malta.c
> @@ -988,7 +988,7 @@ void mips_malta_init(MachineState *machine)
>  
>          /* Init internal devices */
>          cpu_mips_irq_init_cpu(env);
> -        cpu_mips_clock_init(env);
> +        cpu_mips_clock_init(env, 100*1000*1000);
>          qemu_register_reset(main_cpu_reset, cpu);
>      }
>      cpu = MIPS_CPU(first_cpu);
> @@ -1144,7 +1144,7 @@ void mips_malta_init(MachineState *machine)
>  
>      /* Init internal devices */
>      cpu_mips_irq_init_cpu(env);
> -    cpu_mips_clock_init(env);
> +    cpu_mips_clock_init(env, 100*1000*1000);
>  
>      /*
>       * We have a circular dependency problem: pci_bus depends on isa_irq,
> diff --git a/hw/mips/mips_mipssim.c b/hw/mips/mips_mipssim.c
> index 61f74a6..940ca58 100644
> --- a/hw/mips/mips_mipssim.c
> +++ b/hw/mips/mips_mipssim.c
> @@ -213,7 +213,7 @@ mips_mipssim_init(MachineState *machine)
>  
>      /* Init CPU internal devices. */
>      cpu_mips_irq_init_cpu(env);
> -    cpu_mips_clock_init(env);
> +    cpu_mips_clock_init(env, 100*1000*1000);
>  
>      /* Register 64 KB of ISA IO space at 0x1fd00000. */
>      memory_region_init_alias(isa, NULL, "isa_mmio",
> diff --git a/hw/mips/mips_r4k.c b/hw/mips/mips_r4k.c
> index f4dcacd..5f0b766 100644
> --- a/hw/mips/mips_r4k.c
> +++ b/hw/mips/mips_r4k.c
> @@ -266,7 +266,7 @@ void mips_r4k_init(MachineState *machine)
>  
>      /* Init CPU internal devices */
>      cpu_mips_irq_init_cpu(env);
> -    cpu_mips_clock_init(env);
> +    cpu_mips_clock_init(env, 100*1000*1000);
>  
>      /* ISA bus: IO space at 0x14000000, mem space at 0x10000000 */
>      memory_region_init_alias(isa_io, NULL, "isa-io",
> diff --git a/include/hw/mips/cpudevs.h b/include/hw/mips/cpudevs.h
> index b2626f2..4d4e62a 100644
> --- a/include/hw/mips/cpudevs.h
> +++ b/include/hw/mips/cpudevs.h
> @@ -12,6 +12,6 @@ uint64_t cpu_mips_kvm_um_phys_to_kseg0(void *opaque, uint64_t addr);
>  void cpu_mips_irq_init_cpu(CPUMIPSState *env);
>  
>  /* mips_timer.c */
> -void cpu_mips_clock_init(CPUMIPSState *);
> +void cpu_mips_clock_init(CPUMIPSState *, unsigned count_freq);
>  
>  #endif
> diff --git a/target-mips/cpu.h b/target-mips/cpu.h
> index 474a0e3..c476166 100644
> --- a/target-mips/cpu.h
> +++ b/target-mips/cpu.h
> @@ -595,6 +595,7 @@ struct CPUMIPSState {
>      const mips_def_t *cpu_model;
>      void *irq[8];
>      QEMUTimer *timer; /* Internal timer */
> +    unsigned count_freq; /* rate of Count register */
>  };
>  
>  #include "cpu-qom.h"
> -- 
> 1.9.1
> 
> 
>
Serge Vakulenko July 5, 2015, 11:25 p.m. UTC | #2
On Wed, Jul 1, 2015 at 3:02 AM, Aurelien Jarno <aurelien@aurel32.net> wrote:
> On 2015-06-30 21:12, Serge Vakulenko wrote:
>> @@ -153,5 +153,6 @@ void cpu_mips_clock_init (CPUMIPSState *env)
>>       */
>>      if (!kvm_enabled()) {
>>          env->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, &mips_timer_cb, env);
>> +        env->count_freq = count_freq;
>>      }
>>  }
>
> So it means the value passed as an argument to this function is ignored
> in the KVM case. I guess we want to be able to tell the kernel about the
> request frequency.

Sound like a new feature request for MIPS KVM developers. I cannot
find any such possibility in the current KVM API.

My patch changes nothing for existing platforms like Malta, Fulong or
MIPSsim. Everything continues to work as it is. Only for pic32mx7 cpu
the clock rate is decreased to 40MHz. I'm not sure anybody could ever
run KVM on this processor. :)

Regards,
--Serge

> Otherwise it looks fine.
>
> --
> Aurelien Jarno                          GPG: 4096R/1DDD8C9B
> aurelien@aurel32.net                 http://www.aurel32.net
Aurelien Jarno July 6, 2015, 8:31 a.m. UTC | #3
On 2015-07-05 16:25, Serge Vakulenko wrote:
> On Wed, Jul 1, 2015 at 3:02 AM, Aurelien Jarno <aurelien@aurel32.net> wrote:
> > On 2015-06-30 21:12, Serge Vakulenko wrote:
> >> @@ -153,5 +153,6 @@ void cpu_mips_clock_init (CPUMIPSState *env)
> >>       */
> >>      if (!kvm_enabled()) {
> >>          env->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, &mips_timer_cb, env);
> >> +        env->count_freq = count_freq;
> >>      }
> >>  }
> >
> > So it means the value passed as an argument to this function is ignored
> > in the KVM case. I guess we want to be able to tell the kernel about the
> > request frequency.
> 
> Sound like a new feature request for MIPS KVM developers. I cannot
> find any such possibility in the current KVM API.

Ok.

> My patch changes nothing for existing platforms like Malta, Fulong or
> MIPSsim. Everything continues to work as it is. Only for pic32mx7 cpu
> the clock rate is decreased to 40MHz. I'm not sure anybody could ever
> run KVM on this processor. :)

Yes, but you give the possibility to tweak the speed, so later someone
might wrongly pass a value different than 100MHz for a CPU usable with
KVM.

The way to go is to do add a comment with an assert:

      if (kvm_enabled()) {
          /* FIXME: KVM only supports a 100MHz clock. */
          assert(count_freq == 100*1000*1000);
      } else {
          env->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, &mips_timer_cb, env);
          env->count_freq = count_freq;
      }
diff mbox

Patch

diff --git a/hw/mips/cputimer.c b/hw/mips/cputimer.c
index 577c9ae..4f02a9f 100644
--- a/hw/mips/cputimer.c
+++ b/hw/mips/cputimer.c
@@ -50,8 +50,8 @@  static void cpu_mips_timer_update(CPUMIPSState *env)
 
     now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
     wait = env->CP0_Compare - env->CP0_Count -
-	    (uint32_t)muldiv64(now, TIMER_FREQ, get_ticks_per_sec());
-    next = now + muldiv64(wait, get_ticks_per_sec(), TIMER_FREQ);
+        (uint32_t)muldiv64(now, env->count_freq, get_ticks_per_sec());
+    next = now + muldiv64(wait, get_ticks_per_sec(), env->count_freq);
     timer_mod(env->timer, next);
 }
 
@@ -80,7 +80,7 @@  uint32_t cpu_mips_get_count (CPUMIPSState *env)
         }
 
         return env->CP0_Count +
-            (uint32_t)muldiv64(now, TIMER_FREQ, get_ticks_per_sec());
+            (uint32_t)muldiv64(now, env->count_freq, get_ticks_per_sec());
     }
 }
 
@@ -97,7 +97,7 @@  void cpu_mips_store_count (CPUMIPSState *env, uint32_t count)
         /* Store new count register */
         env->CP0_Count =
             count - (uint32_t)muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
-                                       TIMER_FREQ, get_ticks_per_sec());
+                                       env->count_freq, get_ticks_per_sec());
         /* Update timer timer */
         cpu_mips_timer_update(env);
     }
@@ -122,7 +122,7 @@  void cpu_mips_stop_count(CPUMIPSState *env)
 {
     /* Store the current value */
     env->CP0_Count += (uint32_t)muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
-                                         TIMER_FREQ, get_ticks_per_sec());
+                                         env->count_freq, get_ticks_per_sec());
 }
 
 static void mips_timer_cb (void *opaque)
@@ -145,7 +145,7 @@  static void mips_timer_cb (void *opaque)
     env->CP0_Count--;
 }
 
-void cpu_mips_clock_init (CPUMIPSState *env)
+void cpu_mips_clock_init(CPUMIPSState *env, unsigned count_freq)
 {
     /*
      * If we're in KVM mode, don't create the periodic timer, that is handled in
@@ -153,5 +153,6 @@  void cpu_mips_clock_init (CPUMIPSState *env)
      */
     if (!kvm_enabled()) {
         env->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, &mips_timer_cb, env);
+        env->count_freq = count_freq;
     }
 }
diff --git a/hw/mips/mips_fulong2e.c b/hw/mips/mips_fulong2e.c
index dea941a..5decc30 100644
--- a/hw/mips/mips_fulong2e.c
+++ b/hw/mips/mips_fulong2e.c
@@ -343,7 +343,7 @@  static void mips_fulong2e_init(MachineState *machine)
 
     /* Init internal devices */
     cpu_mips_irq_init_cpu(env);
-    cpu_mips_clock_init(env);
+    cpu_mips_clock_init(env, 100*1000*1000);
 
     /* North bridge, Bonito --> IP2 */
     pci_bus = bonito_init((qemu_irq *)&(env->irq[2]));
diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c
index 9d60633..02629fa 100644
--- a/hw/mips/mips_jazz.c
+++ b/hw/mips/mips_jazz.c
@@ -210,7 +210,7 @@  static void mips_jazz_init(MachineState *machine,
 
     /* Init CPU internal devices */
     cpu_mips_irq_init_cpu(env);
-    cpu_mips_clock_init(env);
+    cpu_mips_clock_init(env, 100*1000*1000);
 
     /* Chipset */
     rc4030 = rc4030_init(&dmas, &rc4030_dma_mr);
diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index 3082e75..0393017 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -988,7 +988,7 @@  void mips_malta_init(MachineState *machine)
 
         /* Init internal devices */
         cpu_mips_irq_init_cpu(env);
-        cpu_mips_clock_init(env);
+        cpu_mips_clock_init(env, 100*1000*1000);
         qemu_register_reset(main_cpu_reset, cpu);
     }
     cpu = MIPS_CPU(first_cpu);
@@ -1144,7 +1144,7 @@  void mips_malta_init(MachineState *machine)
 
     /* Init internal devices */
     cpu_mips_irq_init_cpu(env);
-    cpu_mips_clock_init(env);
+    cpu_mips_clock_init(env, 100*1000*1000);
 
     /*
      * We have a circular dependency problem: pci_bus depends on isa_irq,
diff --git a/hw/mips/mips_mipssim.c b/hw/mips/mips_mipssim.c
index 61f74a6..940ca58 100644
--- a/hw/mips/mips_mipssim.c
+++ b/hw/mips/mips_mipssim.c
@@ -213,7 +213,7 @@  mips_mipssim_init(MachineState *machine)
 
     /* Init CPU internal devices. */
     cpu_mips_irq_init_cpu(env);
-    cpu_mips_clock_init(env);
+    cpu_mips_clock_init(env, 100*1000*1000);
 
     /* Register 64 KB of ISA IO space at 0x1fd00000. */
     memory_region_init_alias(isa, NULL, "isa_mmio",
diff --git a/hw/mips/mips_r4k.c b/hw/mips/mips_r4k.c
index f4dcacd..5f0b766 100644
--- a/hw/mips/mips_r4k.c
+++ b/hw/mips/mips_r4k.c
@@ -266,7 +266,7 @@  void mips_r4k_init(MachineState *machine)
 
     /* Init CPU internal devices */
     cpu_mips_irq_init_cpu(env);
-    cpu_mips_clock_init(env);
+    cpu_mips_clock_init(env, 100*1000*1000);
 
     /* ISA bus: IO space at 0x14000000, mem space at 0x10000000 */
     memory_region_init_alias(isa_io, NULL, "isa-io",
diff --git a/include/hw/mips/cpudevs.h b/include/hw/mips/cpudevs.h
index b2626f2..4d4e62a 100644
--- a/include/hw/mips/cpudevs.h
+++ b/include/hw/mips/cpudevs.h
@@ -12,6 +12,6 @@  uint64_t cpu_mips_kvm_um_phys_to_kseg0(void *opaque, uint64_t addr);
 void cpu_mips_irq_init_cpu(CPUMIPSState *env);
 
 /* mips_timer.c */
-void cpu_mips_clock_init(CPUMIPSState *);
+void cpu_mips_clock_init(CPUMIPSState *, unsigned count_freq);
 
 #endif
diff --git a/target-mips/cpu.h b/target-mips/cpu.h
index 474a0e3..c476166 100644
--- a/target-mips/cpu.h
+++ b/target-mips/cpu.h
@@ -595,6 +595,7 @@  struct CPUMIPSState {
     const mips_def_t *cpu_model;
     void *irq[8];
     QEMUTimer *timer; /* Internal timer */
+    unsigned count_freq; /* rate of Count register */
 };
 
 #include "cpu-qom.h"