Re: [PULL 02/13] target/arm: Handle overflow in calculation of next timer tick

[Peter Maydell]

On Thu, 14 Dec 2023 at 08:20, Michael Tokarev <mjt@tls.msk.ru> wrote:
>
> 27.11.2023 20:08, Peter Maydell:
> > In commit edac4d8a168 back in 2015 when we added support for
> > the virtual timer offset CNTVOFF_EL2, we didn't correctly update
> > the timer-recalculation code that figures out when the timer
> > interrupt is next going to change state. We got it wrong in
> > two ways:
> >   * for the 0->1 transition, we didn't notice that gt->cval + offset
> >     can overflow a uint64_t
> >   * for the 1->0 transition, we didn't notice that the transition
> >     might now happen before the count rolls over, if offset > count
> >
> > In the former case, we end up trying to set the next interrupt
> > for a time in the past, which results in QEMU hanging as the
> > timer fires continuously.
> >
> > In the latter case, we would fail to update the interrupt
> > status when we are supposed to.
> >
> > Fix the calculations in both cases.
> >
> > The test case is Alex Bennée's from the bug report, and tests
> > the 0->1 transition overflow case.
> >
> > Fixes: edac4d8a168 ("target-arm: Add CNTVOFF_EL2")
> > Cc: qemu-stable@nongnu.org
>
> This change, when applied to 7.2, causes the newly added tests to fail,
> eg: https://gitlab.com/qemu-project/qemu/-/pipelines/1103065860
> (timeout running plugin-vtimer-with-libbb.so etc).
>
> Any hint what can be wrong there?

The test passes fine as a normal test, it's only failing when
plugins are enabled; in the job log
https://gitlab.com/qemu-project/qemu/-/jobs/5727705602
we can see the
TEST vtimer on aarch64
line and that one doesn't time out.  Alex, any ideas?

As a fallback, this isn't really important to backport as far
as the 7.2 branch I think -- although it's a bug it's one that's
been present (as the commit message notes) for many years without
it being a problem in practice.

thanks
-- PMM