On Fri, Oct 18, 2019 at 03:31:49PM +0100, Peter Maydell wrote: > From: Guenter Roeck <li...@roeck-us.net> > > When booting a recent Linux kernel, the qemu message "Timer with delta > zero, disabling" is seen, apparently because a ptimer is started before > being initialized. Fix the problem by initializing the offending ptimer > before starting it. > > The bug is effectively harmless in the old QEMUBH setup > because the sequence of events is: > * the delta zero means the timer expires immediately > * ptimer_reload() arranges for exynos4210_gfrc_event() to be called > * ptimer_reload() notices the zero delta and disables the timer > * later, the QEMUBH runs, and exynos4210_gfrc_event() correctly > configures the timer and restarts it > > In the new transaction based API the bug is still harmless, > but differences of when the callback function runs mean the > message is not printed any more: > * ptimer_run() does nothing as it's inside a transaction block > * ptimer_transaction_commit() sees it has work to do and > calls ptimer_reload() > * the zero delta means the timer expires immediately > * ptimer_reload() calls exynos4210_gfrc_event() directly > * exynos4210_gfrc_event() configures the timer > * the delta is no longer zero so ptimer_reload() doesn't complain > (the zero-delta test is after the trigger-callback in > the ptimer_reload() function) > > Regardless, the behaviour here was not intentional, and we should > just program the ptimer correctly to start with. > > Signed-off-by: Guenter Roeck <li...@roeck-us.net> > [PMM: Expansion/clarification of the commit message: > the message is about a zero delta, not a zero period; > added detail to the commit message of the analysis of what > is happening and why the kernel boots even with the message; > added note that the message goes away with the new ptimer API] > Signed-off-by: Peter Maydell <peter.mayd...@linaro.org> > --- > Philippe pointed me at this bugfix from Guenter. At one point > in my working on the ptimer API changes I thought this bugfix > would be necessary as a prerequisite, but in fact the issue > was in my ptimer changes, and it just happened that fixing > the MCT bug was a workaround for my bug. Even though the > ptimer API changes actually coincidentally now suppress the > annoying message about a zero delta, the behaviour is definitely > not intentional, and since I spent the time working through the > analysis of what was actually going on here I don't want > to waste it :-) > ---
Thanks a lot for picking this up, and for the great analysis! Guenter > hw/timer/exynos4210_mct.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/timer/exynos4210_mct.c b/hw/timer/exynos4210_mct.c > index 72257584145..944120aea59 100644 > --- a/hw/timer/exynos4210_mct.c > +++ b/hw/timer/exynos4210_mct.c > @@ -1254,7 +1254,7 @@ static void exynos4210_mct_write(void *opaque, hwaddr > offset, > /* Start FRC if transition from disabled to enabled */ > if ((value & G_TCON_TIMER_ENABLE) > (old_val & > G_TCON_TIMER_ENABLE)) { > - exynos4210_gfrc_start(&s->g_timer); > + exynos4210_gfrc_restart(s); > } > if ((value & G_TCON_TIMER_ENABLE) < (old_val & > G_TCON_TIMER_ENABLE)) { > -- > 2.20.1 >