On Mon, 16 Dec 2024 at 19:27, Nabih Estefan <nabiheste...@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
