[Qemu-devel] [PATCH v7 1/2] hw/ptimer: Fix issues caused by artificially limited timer timeout

[Dmitry Osipenko]

Multiple issues here related to the timer with a corrected .limit value:

1) ptimer_get_count() returns incorrect value for the disabled timer after
loading the counter with a small value, because corrected limit value
is used instead of the original.

For instance:
    1) ptimer_stop(t)
    2) ptimer_set_period(t, 1)
    3) ptimer_set_limit(t, 0, 1)
    4) ptimer_get_count(t) <-- would return 10000 instead of 0

2) ptimer_get_count() might return incorrect value for the timer running
with a corrected limit value.

For instance:
    1) ptimer_stop(t)
    2) ptimer_set_period(t, 1)
    3) ptimer_set_limit(t, 10, 1)
    4) ptimer_run(t)
    5) ptimer_get_count(t) <-- might return value > 10

3) Neither ptimer_set_period() nor ptimer_set_freq() are correcting the
limit value, so it is still possible to make timer timeout value
arbitrary small.

For instance:
    1) ptimer_set_period(t, 10000)
    2) ptimer_set_limit(t, 1, 0)
    3) ptimer_set_period(t, 1) <-- bypass limit correction

Fix all of the above issues by moving timeout value correction to the
ptimer_reload(). Instead of changing limit value, set new ptimer struct
member "next_event_corrected" when next_event was corrected and make
ptimer_get_count() always return 1 for the timer running with corrected
next_event value. Bump VMSD version since ptimer VM state is changed, but
keep .minimum_version_id as it is backward compatible.

Signed-off-by: Dmitry Osipenko <address@hidden>
---
 hw/core/ptimer.c | 37 +++++++++++++++++++++++--------------
 1 file changed, 23 insertions(+), 14 deletions(-)

diff --git a/hw/core/ptimer.c b/hw/core/ptimer.c
index 8437bd6..2441943 100644
--- a/hw/core/ptimer.c
+++ b/hw/core/ptimer.c
@@ -19,6 +19,7 @@ struct ptimer_state
     int64_t period;
     int64_t last_event;
     int64_t next_event;
+    uint8_t next_event_corrected;
     QEMUBH *bh;
     QEMUTimer *timer;
 };
@@ -48,6 +49,23 @@ static void ptimer_reload(ptimer_state *s)
     if (s->period_frac) {
         s->next_event += ((int64_t)s->period_frac * s->delta) >> 32;
     }
+
+    /*
+     * Artificially limit timeout to something
+     * achievable under QEMU.  Otherwise, QEMU spends all
+     * its time generating timer interrupts, and there
+     * is no forward progress.
+     * About ten microseconds is the fastest that really works
+     * on the current generation of host machines.
+     */
+
+    s->next_event_corrected = 0;
+
+    if (!use_icount && (s->next_event - s->last_event < 10000)) {
+        s->next_event = s->last_event + 10000;
+        s->next_event_corrected = 1;
+    }
+
     timer_mod(s->timer, s->next_event);
 }
 
@@ -76,6 +94,9 @@ uint64_t ptimer_get_count(ptimer_state *s)
             /* Prevent timer underflowing if it should already have
                triggered.  */
             counter = 0;
+        } else if (s->next_event_corrected) {
+            /* Always return 1 when timer expire value was corrected.  */
+            counter = 1;
         } else {
             uint64_t rem;
             uint64_t div;
@@ -180,19 +201,6 @@ void ptimer_set_freq(ptimer_state *s, uint32_t freq)
    count = limit.  */
 void ptimer_set_limit(ptimer_state *s, uint64_t limit, int reload)
 {
-    /*
-     * Artificially limit timeout rate to something
-     * achievable under QEMU.  Otherwise, QEMU spends all
-     * its time generating timer interrupts, and there
-     * is no forward progress.
-     * About ten microseconds is the fastest that really works
-     * on the current generation of host machines.
-     */
-
-    if (!use_icount && limit * s->period < 10000 && s->period) {
-        limit = 10000 / s->period;
-    }
-
     s->limit = limit;
     if (reload)
         s->delta = limit;
@@ -204,7 +212,7 @@ void ptimer_set_limit(ptimer_state *s, uint64_t limit, int 
reload)
 
 const VMStateDescription vmstate_ptimer = {
     .name = "ptimer",
-    .version_id = 1,
+    .version_id = 2,
     .minimum_version_id = 1,
     .fields = (VMStateField[]) {
         VMSTATE_UINT8(enabled, ptimer_state),
@@ -214,6 +222,7 @@ const VMStateDescription vmstate_ptimer = {
         VMSTATE_INT64(period, ptimer_state),
         VMSTATE_INT64(last_event, ptimer_state),
         VMSTATE_INT64(next_event, ptimer_state),
+        VMSTATE_UINT8(next_event_corrected, ptimer_state),
         VMSTATE_TIMER_PTR(timer, ptimer_state),
         VMSTATE_END_OF_LIST()
     }
-- 
2.6.1