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 <dig...@gmail.com> --- hw/core/ptimer.c | 36 ++++++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/hw/core/ptimer.c b/hw/core/ptimer.c index 8437bd6..0732b20 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,22 @@ 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 = !!(s->next_event - s->last_event < 10000); + + if (s->next_event_corrected) { + s->next_event = s->last_event + 10000; + } + timer_mod(s->timer, s->next_event); } @@ -76,6 +93,9 @@ uint64_t ptimer_get_count(ptimer_state *s) /* Prevent timer underflowing if it should already have triggered. */ counter = 0; + } 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 +200,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 +211,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 +221,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