On Thu, 14 Dec 2023 at 08:20, Michael Tokarev <m...@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-sta...@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
