Writing to any of the load, counter or control registers can require a reload of the timer. Currently load and counter share a code path, but the control logic is separate. Consolidate them by reducing the switch to only sync the timer state. For load/counter this just means setting tb->count to the new value. For control, this means setting tb->count to the current value. Then outside the switch, any old timers are discarded, and if the timer is (still is, or has just become) enabled, setup a new one.
A fast escape path is added to control writes. If it is detected that the timer was, and still is running without change of prescaler, don't do the restart. This avoid an un-needed restart that could potentially cause timer rounding errors. For further consolidation, move the auto-load refresh logic to timerblock_reload. This change fixes two bugs and implements a missing feature. Previously a running timer could not be stopped, the commonifying of the timer_del call now means clearing the enable bit in the control register now correctly stops a running timer. There was a bug where if starting a timer in periodic mode after one-shot expiration, the timer would not restart. This was because of a bad conditional for the old do-nothing fast path. This implements pause and resumption of the timer. Clearing the enable bit, and the setting it again later will cause the timer to pick up where it left off. The paused value of the timer can be read by the guest. Another use of this stop and restart feature, is it also now models a change of prescaler for a running timer. Signed-off-by: Peter Crosthwaite <crosthwaite.pe...@gmail.com> --- hw/timer/arm_mptimer.c | 44 ++++++++++++++++++++++++-------------------- 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/hw/timer/arm_mptimer.c b/hw/timer/arm_mptimer.c index 04dfb63..69899cf 100644 --- a/hw/timer/arm_mptimer.c +++ b/hw/timer/arm_mptimer.c @@ -50,7 +50,11 @@ static inline uint32_t timerblock_scale(TimerBlock *tb) static void timerblock_reload(TimerBlock *tb, int restart) { if (tb->count == 0) { - return; + if (tb->control & 2) { + tb->count = tb->load; + } else { + return; + } } if (restart) { tb->tick = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); @@ -63,12 +67,8 @@ static void timerblock_tick(void *opaque) { TimerBlock *tb = (TimerBlock *)opaque; tb->status = 1; - if (tb->control & 2) { - tb->count = tb->load; - timerblock_reload(tb, 0); - } else { - tb->count = 0; - } + tb->count = 0; + timerblock_reload(tb, 0); timerblock_update_irq(tb); } @@ -113,33 +113,37 @@ static void timerblock_write(void *opaque, hwaddr addr, TimerBlock *tb = (TimerBlock *)opaque; int64_t old; switch (addr) { + /* Breaking from this switch implies that timer needs to be refreshed. + * Operations that do not affect the running timer must return directly + * to avoid a spurious reload of the timer. + */ case 0: /* Load */ tb->load = value; /* Fall through. */ case 4: /* Counter. */ - if ((tb->control & 1) && tb->count) { - /* Cancel the previous timer. */ - timer_del(tb->timer); - } tb->count = value; - if (tb->control & 1) { - timerblock_reload(tb, 1); - } break; case 8: /* Control. */ old = tb->control; tb->control = value; - if (((old & 1) == 0) && (value & 1)) { - if (tb->count == 0 && (tb->control & 2)) { - tb->count = tb->load; - } - timerblock_reload(tb, 1); + if ((value & 1) && (old & 1) && tb->count != 0 && + !extract64(value ^ old, 8, 8)) { + /* Timer was running, and still is, without prescale change */ + return; } + timerblock_sync(tb); break; case 12: /* Interrupt status. */ tb->status &= ~value; timerblock_update_irq(tb); - break; + return; + } + + /* Cancel any previous timer. */ + timer_del(tb->timer); + + if (tb->control & 1) { + timerblock_reload(tb, 1); } } -- 1.9.1