[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: |
Peter Maydell |
Subject: |
Re: [PATCH v2] tests/qtest/sse-timer-test: Add watchdog reset to sse-timer test |
Date: |
Tue, 17 Dec 2024 10:28:13 +0000 |
On Mon, 16 Dec 2024 at 19:27, Nabih Estefan <nabihestefan@google.com> wrote:
>
> 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.
Ah, thanks for digging into this further. We already know that
commit has problems (reported in
https://gitlab.com/qemu-project/qemu/-/issues/2687 ).
Alex was looking at this -- I've cc'd him.
thanks
-- PMM