[PATCH v1] tests/qtest: Avoid timers from default devices

Fabiano Rosas posted 1 patch 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260428160120.12130-1-farosas@suse.de
Maintainers: Tyrone Ting <kfting@nuvoton.com>, Hao Wu <wuhaotsh@google.com>, Fabiano Rosas <farosas@suse.de>, Laurent Vivier <lvivier@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>
tests/qtest/npcm7xx_timer-test.c | 2 +-
tests/qtest/pflash-cfi02-test.c  | 4 ++--
tests/qtest/rtc-test.c           | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
[PATCH v1] tests/qtest: Avoid timers from default devices
Posted by Fabiano Rosas 1 month ago
Having default devices enabled while testing timers leads to
non-deterministic behavior on the usage of the QTest
qtest_clock_step_next() command as the implicit devices can have
timers of their own that may have a deadline more recent than the
tested device.

In the particular case of npcm7xx-timer-test, having SLIRP enabled in
the QEMU binary may cause the net_slirp_timer to shadow the clock
advance that was expected to trigger the NPCM7xx timer:

npcm7xx_timer-test.c:475:test_periodic_interrupt:
  assertion failed (tim_read(td, TISR) == tim_timer_bit(td)): (0x00000000 == 0x00000010)
npcm7xx_timer-test.c:476:test_periodic_interrupt:
'qtest_get_irq(global_qtest, tim_timer_irq(td))' should be TRUE

Add the -nodefaults command line option to all tests that invoke
qtest_clock_step_next.

CC: Alex Bennée <alex.bennee@linaro.org>
Reported-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 tests/qtest/npcm7xx_timer-test.c | 2 +-
 tests/qtest/pflash-cfi02-test.c  | 4 ++--
 tests/qtest/rtc-test.c           | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/qtest/npcm7xx_timer-test.c b/tests/qtest/npcm7xx_timer-test.c
index 43711049ca..0e2284669d 100644
--- a/tests/qtest/npcm7xx_timer-test.c
+++ b/tests/qtest/npcm7xx_timer-test.c
@@ -551,7 +551,7 @@ int main(int argc, char **argv)
         }
     }
 
-    qtest_start("-machine npcm750-evb");
+    qtest_start("-machine npcm750-evb -nodefaults");
     qtest_irq_intercept_in(global_qtest, "/machine/soc/a9mpcore/gic");
     ret = g_test_run();
     qtest_end();
diff --git a/tests/qtest/pflash-cfi02-test.c b/tests/qtest/pflash-cfi02-test.c
index 8c073efcb4..65b35811fa 100644
--- a/tests/qtest/pflash-cfi02-test.c
+++ b/tests/qtest/pflash-cfi02-test.c
@@ -260,7 +260,7 @@ static void test_geometry(const void *opaque)
 {
     const FlashConfig *config = opaque;
     QTestState *qtest;
-    qtest = qtest_initf("-M musicpal"
+    qtest = qtest_initf("-M musicpal -nodefaults"
                         " -drive if=pflash,file=%s,format=raw,copy-on-read=on"
                         /* Device geometry properties. */
                         " -global driver=cfi.pflash02,"
@@ -580,7 +580,7 @@ static void test_cfi_in_autoselect(const void *opaque)
 {
     const FlashConfig *config = opaque;
     QTestState *qtest;
-    qtest = qtest_initf("-M musicpal"
+    qtest = qtest_initf("-M musicpal -nodefaults"
                         " -drive if=pflash,file=%s,format=raw,copy-on-read=on",
                         image_path);
     FlashConfig explicit_config = expand_config_defaults(config);
diff --git a/tests/qtest/rtc-test.c b/tests/qtest/rtc-test.c
index 02ed4e1238..09b6de9166 100644
--- a/tests/qtest/rtc-test.c
+++ b/tests/qtest/rtc-test.c
@@ -691,7 +691,7 @@ int main(int argc, char **argv)
 
     g_test_init(&argc, &argv, NULL);
 
-    s = qtest_start("-rtc clock=vm");
+    s = qtest_start("-rtc clock=vm -nodefaults");
     qtest_irq_intercept_in(s, "ioapic");
 
     qtest_add_func("/rtc/check-time/bcd", bcd_check_time);
-- 
2.51.0


Re: [PATCH v1] tests/qtest: Avoid timers from default devices
Posted by Stefan Hajnoczi 1 month ago
On Tue, Apr 28, 2026 at 12:04 PM Fabiano Rosas <farosas@suse.de> wrote:
>
> Having default devices enabled while testing timers leads to
> non-deterministic behavior on the usage of the QTest
> qtest_clock_step_next() command as the implicit devices can have
> timers of their own that may have a deadline more recent than the
> tested device.
>
> In the particular case of npcm7xx-timer-test, having SLIRP enabled in
> the QEMU binary may cause the net_slirp_timer to shadow the clock
> advance that was expected to trigger the NPCM7xx timer:
>
> npcm7xx_timer-test.c:475:test_periodic_interrupt:
>   assertion failed (tim_read(td, TISR) == tim_timer_bit(td)): (0x00000000 == 0x00000010)
> npcm7xx_timer-test.c:476:test_periodic_interrupt:
> 'qtest_get_irq(global_qtest, tim_timer_irq(td))' should be TRUE
>
> Add the -nodefaults command line option to all tests that invoke
> qtest_clock_step_next.
>
> CC: Alex Bennée <alex.bennee@linaro.org>
> Reported-by: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
>  tests/qtest/npcm7xx_timer-test.c | 2 +-
>  tests/qtest/pflash-cfi02-test.c  | 4 ++--
>  tests/qtest/rtc-test.c           | 2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)

Hi Fabiano,
Thank you for the patch! I'm not sure this is a long-term fix since
any code in QEMU could use timers (monitor, io channels, etc)?

Whether it is a full fix or not, it's definitely an improvement:
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Re: [PATCH v1] tests/qtest: Avoid timers from default devices
Posted by Fabiano Rosas 1 month ago
Stefan Hajnoczi <stefanha@gmail.com> writes:

> On Tue, Apr 28, 2026 at 12:04 PM Fabiano Rosas <farosas@suse.de> wrote:
>>
>> Having default devices enabled while testing timers leads to
>> non-deterministic behavior on the usage of the QTest
>> qtest_clock_step_next() command as the implicit devices can have
>> timers of their own that may have a deadline more recent than the
>> tested device.
>>
>> In the particular case of npcm7xx-timer-test, having SLIRP enabled in
>> the QEMU binary may cause the net_slirp_timer to shadow the clock
>> advance that was expected to trigger the NPCM7xx timer:
>>
>> npcm7xx_timer-test.c:475:test_periodic_interrupt:
>>   assertion failed (tim_read(td, TISR) == tim_timer_bit(td)): (0x00000000 == 0x00000010)
>> npcm7xx_timer-test.c:476:test_periodic_interrupt:
>> 'qtest_get_irq(global_qtest, tim_timer_irq(td))' should be TRUE
>>
>> Add the -nodefaults command line option to all tests that invoke
>> qtest_clock_step_next.
>>
>> CC: Alex Bennée <alex.bennee@linaro.org>
>> Reported-by: Stefan Hajnoczi <stefanha@redhat.com>
>> Signed-off-by: Fabiano Rosas <farosas@suse.de>
>> ---
>>  tests/qtest/npcm7xx_timer-test.c | 2 +-
>>  tests/qtest/pflash-cfi02-test.c  | 4 ++--
>>  tests/qtest/rtc-test.c           | 2 +-
>>  3 files changed, 4 insertions(+), 4 deletions(-)
>
> Hi Fabiano,
> Thank you for the patch! I'm not sure this is a long-term fix since
> any code in QEMU could use timers (monitor, io channels, etc)?
>

Good point, if we go this way I'll add more words to the commit message
to make that pitfall clear.

@Alex, I was even under the impression that the issues with slirp timers
getting in the way of clock_step were entirely fixed by your changes
from:
https://lore.kernel.org/r/20250210161451.3273284-1-alex.bennee@linaro.org

Was that the intention at the time? Should we consider a timer test that
expects it to fire right after advancing the clock a mistake in the
test?

> Whether it is a full fix or not, it's definitely an improvement:
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>