[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2] tests/qtest/sse-timer-test: Add watchdog reset to sse-tim
From: |
Nabih Estefan |
Subject: |
Re: [PATCH v2] tests/qtest/sse-timer-test: Add watchdog reset to sse-timer test |
Date: |
Mon, 16 Dec 2024 11:26:59 -0800 |
Actually after some more debugging with the help of Roque (cc'd) we
realized that this patch doesn't actually fix the issue, it only hides
it behind the watchdog.
The root issue comes from
https://lists.gnu.org/archive/html/qemu-s390x/2024-09/msg00264.html.
The function `qemu_clock_advance_virtual_time` is broken with that
patch and the conditions of the sse-timer test. In the test (and other
tests) we run `clock_step_ticks()`. This function calls
`qemu_clock_advance_virtual_time` which now has a check with
`qemu_clock_deadline_ns_all`. This returns -1 if there is no timer
enabled, making it so the virtual time in this test is never updated,
thus leading to the failure. This was surfaced by the INTEN fix in the
watchdog because now we don't have that timer running free out of
reset. Once we enable the watchdog timer, we make it so
`qemu_clock_deadline_ns_all` will return anything but -1, letting us
continue through the test. My theory is that in other people's local
builds (as in one of our local cases) there is another timer being
activated (which in our case was the slirp timer) allowing the test to
get through this failure. This patch only covers the bug, not actually
fixing it. We shouldn't actually merge this, we should instead fix
https://lists.gnu.org/archive/html/qemu-s390x/2024-09/msg00264.html.
- Nabih
On Mon, Dec 16, 2024 at 5:40 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Fri, 13 Dec 2024 at 22:40, Nabih Estefan <nabihestefan@google.com> wrote:
> >
> > V2: Removed scripts/meson-buildoptions.sh.tmp Extra file that slipped
> > through the cracks and shouldn't be in this patch
> >
> > Recent CDMSK Watchdog changes (eff9dc5660fad3a610171c56a5ec3fada245e519)
> > updated the CDMSK APB Watchdog to not free run out of reset. That led to
> > this test failing since it never triggers the watchdog to start running.
> > No watchdog running means that the timer and counter in the test cannot
> > start, leading to failures in the assert statements throughout the test.
> > Adding a reset and enable of the watchdog to the reset function solves
> > this problem by enabling the watchdog and thus letting the timer and
> > counter run as expected
>
> I don't understand this. The watchdog on this board is not
> an input to the timer/counter, so whether the watchdog is
> running or not should not affect whether the timer and
> counter can run. Something else must be going wrong.
>
> > Also renaming the reset_counter_and_timer function since it now also
> > affects the watchdog.
> >
> > To reproduce the failure at HEAD:
> > ./configure --target-list=arm-softmmu
> > make -j check-report-qtest-arm.junit.xml
>
> This does not fail for me, and nor does running just the
> single test case in a loop. (Tested on ubuntu 22.04 with
> gcc (Ubuntu 11.4.0-1ubuntu1~22.04) 11.4.0.)
>
> thanks
> -- PMM