[Qemu-devel] [PATCH_v2 5/9] openrisc-timer: Reduce overhead, Separate clock update functions

[Sebastian Macke <sebastian@macke.de>]

The clock value is only evaluated when really necessary reducing
the overhead of the timer handling.

This also solves a problem in the way the Linux kernel
handles the timer and the expected accuracy.
The old version could lead to inaccurate timings.

Signed-off-by: Sebastian Macke <sebastian@macke.de>
---
 hw/openrisc/cputimer.c       | 29 +++++++++++++++++++----------
 target-openrisc/cpu.h        |  1 +
 target-openrisc/sys_helper.c | 38 ++++++++++++++++++--------------------
 3 files changed, 38 insertions(+), 30 deletions(-)

diff --git a/hw/openrisc/cputimer.c b/hw/openrisc/cputimer.c
index 988ca20..9c54945 100644
--- a/hw/openrisc/cputimer.c
+++ b/hw/openrisc/cputimer.c
@@ -30,19 +30,28 @@ static int is_counting;
 
 void cpu_openrisc_count_update(OpenRISCCPU *cpu)
 {
-    uint64_t now, next;
-    uint32_t wait;
+    uint64_t now;
 
-    now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
     if (!is_counting) {
-        timer_del(cpu->env.timer);
-        last_clk = now;
         return;
     }
-
+    now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
     cpu->env.ttcr += (uint32_t)muldiv64(now - last_clk, TIMER_FREQ,
                                         get_ticks_per_sec());
     last_clk = now;
+}
+
+void cpu_openrisc_timer_update(OpenRISCCPU *cpu)
+{
+    uint32_t wait;
+    uint64_t now, next;
+
+    if (!is_counting) {
+        return;
+    }
+
+    cpu_openrisc_count_update(cpu);
+    now = last_clk;
 
     if ((cpu->env.ttmr & TTMR_TP) <= (cpu->env.ttcr & TTMR_TP)) {
         wait = TTMR_TP - (cpu->env.ttcr & TTMR_TP) + 1;
@@ -50,7 +59,6 @@ void cpu_openrisc_count_update(OpenRISCCPU *cpu)
     } else {
         wait = (cpu->env.ttmr & TTMR_TP) - (cpu->env.ttcr & TTMR_TP);
     }
-
     next = now + muldiv64(wait, get_ticks_per_sec(), TIMER_FREQ);
     timer_mod(cpu->env.timer, next);
 }
@@ -63,8 +71,9 @@ void cpu_openrisc_count_start(OpenRISCCPU *cpu)
 
 void cpu_openrisc_count_stop(OpenRISCCPU *cpu)
 {
-    is_counting = 0;
+    timer_del(cpu->env.timer);
     cpu_openrisc_count_update(cpu);
+    is_counting = 0;
 }
 
 static void openrisc_timer_cb(void *opaque)
@@ -84,15 +93,15 @@ static void openrisc_timer_cb(void *opaque)
         break;
     case TIMER_INTR:
         cpu->env.ttcr = 0;
-        cpu_openrisc_count_start(cpu);
         break;
     case TIMER_SHOT:
         cpu_openrisc_count_stop(cpu);
         break;
     case TIMER_CONT:
-        cpu_openrisc_count_start(cpu);
         break;
     }
+
+    cpu_openrisc_timer_update(cpu);
 }
 
 void cpu_openrisc_clock_init(OpenRISCCPU *cpu)
diff --git a/target-openrisc/cpu.h b/target-openrisc/cpu.h
index 8fd0bc0..0f9efdf 100644
--- a/target-openrisc/cpu.h
+++ b/target-openrisc/cpu.h
@@ -373,6 +373,7 @@ void cpu_openrisc_pic_init(OpenRISCCPU *cpu);
 /* hw/openrisc_timer.c */
 void cpu_openrisc_clock_init(OpenRISCCPU *cpu);
 void cpu_openrisc_count_update(OpenRISCCPU *cpu);
+void cpu_openrisc_timer_update(OpenRISCCPU *cpu);
 void cpu_openrisc_count_start(OpenRISCCPU *cpu);
 void cpu_openrisc_count_stop(OpenRISCCPU *cpu);
 
diff --git a/target-openrisc/sys_helper.c b/target-openrisc/sys_helper.c
index cccbc0e..f116588 100644
--- a/target-openrisc/sys_helper.c
+++ b/target-openrisc/sys_helper.c
@@ -127,33 +127,31 @@ void HELPER(mtspr)(CPUOpenRISCState *env,
         break;
     case TO_SPR(10, 0): /* TTMR */
         {
+            if ((env->ttmr & TTMR_M) ^ (rb & TTMR_M)) {
+                switch (rb & TTMR_M) {
+                case TIMER_NONE:
+                    cpu_openrisc_count_stop(cpu);
+                    break;
+                case TIMER_INTR:
+                case TIMER_SHOT:
+                case TIMER_CONT:
+                    cpu_openrisc_count_start(cpu);
+                    break;
+                default:
+                    break;
+                }
+            }
+
             int ip = env->ttmr & TTMR_IP;
 
             if (rb & TTMR_IP) {    /* Keep IP bit.  */
-                env->ttmr = (rb & ~TTMR_IP) + ip;
+                env->ttmr = (rb & ~TTMR_IP) | ip;
             } else {    /* Clear IP bit.  */
                 env->ttmr = rb & ~TTMR_IP;
                 cs->interrupt_request &= ~CPU_INTERRUPT_TIMER;
             }
 
-            cpu_openrisc_count_update(cpu);
-
-            switch (env->ttmr & TTMR_M) {
-            case TIMER_NONE:
-                cpu_openrisc_count_stop(cpu);
-                break;
-            case TIMER_INTR:
-                cpu_openrisc_count_start(cpu);
-                break;
-            case TIMER_SHOT:
-                cpu_openrisc_count_start(cpu);
-                break;
-            case TIMER_CONT:
-                cpu_openrisc_count_start(cpu);
-                break;
-            default:
-                break;
-            }
+            cpu_openrisc_timer_update(cpu);
         }
         break;
 
@@ -162,7 +160,7 @@ void HELPER(mtspr)(CPUOpenRISCState *env,
         if (env->ttmr & TIMER_NONE) {
             return;
         }
-        cpu_openrisc_count_start(cpu);
+        cpu_openrisc_timer_update(cpu);
         break;
     default:
 
-- 
1.8.4.1