diff mbox series

[v2] tests/qtest/sse-timer-test: Add watchdog reset to sse-timer test

Message ID 20241213224020.2982578-1-nabihestefan@google.com (mailing list archive)
State New
Headers show
Series [v2] tests/qtest/sse-timer-test: Add watchdog reset to sse-timer test | expand

Commit Message

Nabih Estefan Dec. 13, 2024, 10:40 p.m. UTC 
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

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

Signed-off-by: Nabih Estefan <nabihestefan@google.com>
Tested-by: Thomas Huth <thuth@redhat.com>
---
 tests/qtest/sse-timer-test.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

Comments

Nabih Estefan Dec. 13, 2024, 10:51 p.m. UTC | #1 
Some of the info I was asked for in V1 of the patch:

I'm seeing the failure on 2 different machines, the compiler versions are:
- gcc version 11.4.0 (Ubuntu 11.4.0-1ubuntu1~22.04)
- gcc version 14.2.0 (Debian 14.2.0-3+build3)

Both of the outputs below are run on the Debian machine.
Output of the command at a5ba0a7e4 (without this patch):
$ QTEST_LOG=1 QTEST_TRACE="sse_*" QTEST_QEMU_BINARY=./qemu-system-arm
./tests/qtest/sse-timer-test
TAP version 14
# random seed: R02Sef41cebd370ee89bac17cc72a839670c
# starting QEMU: exec ./qemu-system-arm -trace sse_* -qtest
unix:/tmp/qtest-3006002.sock -qtest-log /dev/fd/2 -chardev
socket,path=/tmp/qtest-3006002.qmp,id=char0 -mon
chardev=char0,mode=control -display none -audio none -machine
mps3-an547 -accel qtest
[I 0.000000] OPENED
sse_timer_reset SSE system timer: reset
sse_timer_reset SSE system timer: reset
sse_timer_reset SSE system timer: reset
sse_timer_reset SSE system timer: reset
sse_counter_reset SSE system counter: reset
[R +0.024763] endianness
[S +0.024774] OK little
{"QMP": {"version": {"qemu": {"micro": 50, "minor": 2, "major": 9},
"package": "v9.2.0-28-ga5ba0a7e4e"}, "capabilities": ["oob"]}}
{"execute": "qmp_capabilities"}

{"return": {}}
1..3
# Start of arm tests
# Start of sse-timer tests
[R +0.026195] writel 0x58100000 0x0
sse_counter_control_write SSE system counter control framen write:
offset 0x0 data 0x0 size 4
[S +0.026216] OK
[R +0.026240] writel 0x4800002c 0x0
[S +0.026247] OK
[R +0.026263] writel 0x4800004c 0x0
[S +0.026268] OK
[R +0.026284] writel 0x58100008 0x0
sse_counter_control_write SSE system counter control framen write:
offset 0x8 data 0x0 size 4
[S +0.026294] OK
[R +0.026309] writel 0x5810000c 0x0
sse_counter_control_write SSE system counter control framen write:
offset 0xc data 0x0 size 4
[S +0.026316] OK
[R +0.026331] clock_step 3125
[S +0.026340] OK 0
[R +0.026356] readl 0x58100008
sse_counter_control_read SSE system counter control frame read: offset
0x8 data 0x0 size 4
[S +0.026366] OK 0x0000000000000000
[R +0.026381] readl 0x5810000c
sse_counter_control_read SSE system counter control frame read: offset
0xc data 0x0 size 4
[S +0.026388] OK 0x0000000000000000
[R +0.026404] writel 0x58100000 0x1
sse_counter_control_write SSE system counter control framen write:
offset 0x0 data 0x1 size 4
[S +0.026412] OK
[R +0.026428] clock_step 3125
[S +0.026437] OK 0
[R +0.026453] readl 0x58100008
sse_counter_control_read SSE system counter control frame read: offset
0x8 data 0x0 size 4
[S +0.026461] OK 0x0000000000000000
**
ERROR:../tests/qtest/sse-timer-test.c:91:test_counter: assertion
failed (readl(COUNTER_BASE + CNTCV_LO) == 100): (0 == 100)
not ok /arm/sse-timer/counter -
ERROR:../tests/qtest/sse-timer-test.c:91:test_counter: assertion
failed (readl(COUNTER_BASE + CNTCV_LO) == 100): (0 == 100)
Bail out!
[I +0.026730] CLOSED
Aborted


Output of the command at a5ba0a7e4+this patch:
$ QTEST_LOG=1 QTEST_TRACE="sse_*" QTEST_QEMU_BINARY=./qemu-system-arm
./tests/qtest/sse-timer-test
TAP version 14
# random seed: R02Sfe7ced06933a483e656b6313bfe47a7d
# starting QEMU: exec ./qemu-system-arm -trace sse_* -qtest
unix:/tmp/qtest-2909753.sock -qtest-log /dev/fd/2 -chardev
socket,path=/tmp/qtest-2909753.qmp,id=char0 -mon
chardev=char0,mode=control -display none -audio none -machine
mps3-an547 -accel qtest
[I 0.000000] OPENED
sse_timer_reset SSE system timer: reset
sse_timer_reset SSE system timer: reset
sse_timer_reset SSE system timer: reset
sse_timer_reset SSE system timer: reset
sse_counter_reset SSE system counter: reset
[R +0.024304] endianness
[S +0.024314] OK little
{"QMP": {"version": {"qemu": {"micro": 50, "minor": 2, "major": 9},
"package": "v9.2.0-29-g556bf9b0e6"}, "capabilities": ["oob"]}}
{"execute": "qmp_capabilities"}

{"return": {}}
1..3
# Start of arm tests
# Start of sse-timer tests
[R +0.025692] writel 0x4802e008 0x0
[S +0.025704] OK
[R +0.025722] writel 0x58100000 0x0
sse_counter_control_write SSE system counter control framen write:
offset 0x0 data 0x0 size 4
[S +0.025732] OK
[R +0.025748] writel 0x4800002c 0x0
[S +0.025755] OK
[R +0.025771] writel 0x4800004c 0x0
[S +0.025776] OK
[R +0.025791] writel 0x58100008 0x0
sse_counter_control_write SSE system counter control framen write:
offset 0x8 data 0x0 size 4
[S +0.025801] OK
[R +0.025817] writel 0x5810000c 0x0
sse_counter_control_write SSE system counter control framen write:
offset 0xc data 0x0 size 4
[S +0.025825] OK
[R +0.025841] writel 0x4802e008 0x1
[S +0.025846] OK
[R +0.025862] clock_step 3125
[S +0.025873] OK 3125
[R +0.025902] readl 0x58100008
sse_counter_control_read SSE system counter control frame read: offset
0x8 data 0x0 size 4
[S +0.025912] OK 0x0000000000000000
[R +0.025927] readl 0x5810000c
sse_counter_control_read SSE system counter control frame read: offset
0xc data 0x0 size 4
[S +0.025933] OK 0x0000000000000000
[R +0.025948] writel 0x58100000 0x1
sse_counter_control_write SSE system counter control framen write:
offset 0x0 data 0x1 size 4
[S +0.025955] OK
[R +0.025974] clock_step 3125
[S +0.025982] OK 6250
[R +0.025997] readl 0x58100008
sse_counter_control_read SSE system counter control frame read: offset
0x8 data 0x64 size 4
[S +0.026005] OK 0x0000000000000064
[R +0.026021] readl 0x5810000c
sse_counter_control_read SSE system counter control frame read: offset
0xc data 0x0 size 4
[S +0.026032] OK 0x0000000000000000
[R +0.026048] writel 0x58100000 0x0
sse_counter_control_write SSE system counter control framen write:
offset 0x0 data 0x0 size 4
[S +0.026055] OK
[R +0.026070] writel 0x58100010 0x100000
sse_counter_control_write SSE system counter control framen write:
offset 0x10 data 0x100000 size 4
[S +0.026077] OK
[R +0.026091] writel 0x58100000 0x5
sse_counter_control_write SSE system counter control framen write:
offset 0x0 data 0x5 size 4
[S +0.026098] OK
[R +0.026113] clock_step 5000
[S +0.026121] OK 11250
[R +0.026140] readl 0x58100008
sse_counter_control_read SSE system counter control frame read: offset
0x8 data 0x6e size 4
[S +0.026145] OK 0x000000000000006e
[R +0.026160] readl 0x5810000c
sse_counter_control_read SSE system counter control frame read: offset
0xc data 0x0 size 4
[S +0.026167] OK 0x0000000000000000
ok 1 /arm/sse-timer/counter
[R +0.026200] writel 0x4802e008 0x0
[S +0.026205] OK
[R +0.026220] writel 0x58100000 0x0
sse_counter_control_write SSE system counter control framen write:
offset 0x0 data 0x0 size 4
[S +0.026228] OK
[R +0.026244] writel 0x4800002c 0x0
[S +0.026248] OK
[R +0.026263] writel 0x4800004c 0x0
[S +0.026268] OK
[R +0.026283] writel 0x58100008 0x0
sse_counter_control_write SSE system counter control framen write:
offset 0x8 data 0x0 size 4
[S +0.026290] OK
[R +0.026305] writel 0x5810000c 0x0
sse_counter_control_write SSE system counter control framen write:
offset 0xc data 0x0 size 4
[S +0.026311] OK
[R +0.026327] writel 0x4802e008 0x1
[S +0.026331] OK
[R +0.026346] writel 0x50080070 0x1
[S +0.026352] OK
[R +0.026368] writel 0x58100000 0x1
sse_counter_control_write SSE system counter control framen write:
offset 0x0 data 0x1 size 4
[S +0.026377] OK
[R +0.026392] readl 0x4800002c
sse_timer_read SSE system timer read: offset 0x2c data 0x0 size 4
[S +0.026402] OK 0x0000000000000000
[R +0.026417] readl 0x48000000
sse_timer_read SSE system timer read: offset 0x0 data 0x0 size 4
[S +0.026424] OK 0x0000000000000000
[R +0.026441] readl 0x48000004
sse_timer_read SSE system timer read: offset 0x4 data 0x0 size 4
[S +0.026450] OK 0x0000000000000000
[R +0.026468] writel 0x4800002c 0x1
sse_timer_write SSE system timer write: offset 0x2c data 0x1 size 4
[S +0.026479] OK
[R +0.026495] clock_step 3125
[S +0.026503] OK 14375
[R +0.026518] readl 0x48000000
sse_timer_read SSE system timer read: offset 0x0 data 0x64 size 4
[S +0.026524] OK 0x0000000000000064
[R +0.026540] readl 0x48000004
sse_timer_read SSE system timer read: offset 0x4 data 0x0 size 4
[S +0.026549] OK 0x0000000000000000
[R +0.026565] writel 0x48000020 0xfa0
sse_timer_write SSE system timer write: offset 0x20 data 0xfa0 size 4
[S +0.026572] OK
[R +0.026587] writel 0x48000024 0x0
sse_timer_write SSE system timer write: offset 0x24 data 0x0 size 4
[S +0.026595] OK
[R +0.026610] readl 0x48000028
sse_timer_read SSE system timer read: offset 0x28 data 0xf3c size 4
[S +0.026617] OK 0x0000000000000f3c
[R +0.026633] clock_step 121875
[S +0.026640] OK 136250
[R +0.026655] readl 0x48000028
sse_timer_read SSE system timer read: offset 0x28 data 0x0 size 4
[S +0.026662] OK 0x0000000000000000
[R +0.026677] readl 0x4800002c
sse_timer_read SSE system timer read: offset 0x2c data 0x5 size 4
[S +0.026685] OK 0x0000000000000005
[R +0.026699] writel 0x48000048 0xc8
sse_timer_write SSE system timer write: offset 0x48 data 0xc8 size 4
[S +0.026706] OK
[R +0.026722] writel 0x4800004c 0x1
sse_timer_write SSE system timer write: offset 0x4c data 0x1 size 4
[S +0.026729] OK
[R +0.026744] readl 0x48000040
sse_timer_read SSE system timer read: offset 0x40 data 0x1068 size 4
[S +0.026751] OK 0x0000000000001068
[R +0.026767] readl 0x48000044
sse_timer_read SSE system timer read: offset 0x44 data 0x0 size 4
[S +0.026773] OK 0x0000000000000000
[R +0.026787] readl 0x4800002c
sse_timer_read SSE system timer read: offset 0x2c data 0x1 size 4
[S +0.026794] OK 0x0000000000000001
[R +0.026809] clock_step 3125
[S +0.026817] OK 139375
[R +0.026832] readl 0x4800002c
sse_timer_read SSE system timer read: offset 0x2c data 0x1 size 4
[S +0.026839] OK 0x0000000000000001
[R +0.026855] clock_step 3125
[S +0.026863] OK 142500
[R +0.026878] readl 0x4800002c
sse_timer_read SSE system timer read: offset 0x2c data 0x5 size 4
[S +0.026883] OK 0x0000000000000005
[R +0.026899] readl 0x48000040
sse_timer_read SSE system timer read: offset 0x40 data 0x1130 size 4
[S +0.026908] OK 0x0000000000001130
[R +0.026922] readl 0x48000044
sse_timer_read SSE system timer read: offset 0x44 data 0x0 size 4
[S +0.026931] OK 0x0000000000000000
[R +0.026947] clock_step 3125
[S +0.026954] OK 145625
[R +0.026970] readl 0x4800002c
sse_timer_read SSE system timer read: offset 0x2c data 0x5 size 4
[S +0.026975] OK 0x0000000000000005
[R +0.026989] writel 0x4800004c 0x1
sse_timer_write SSE system timer write: offset 0x4c data 0x1 size 4
[S +0.026996] OK
[R +0.027011] readl 0x4800002c
sse_timer_read SSE system timer read: offset 0x2c data 0x1 size 4
[S +0.027018] OK 0x0000000000000001
[R +0.027033] clock_step 3125
[S +0.027041] OK 148750
[R +0.027054] readl 0x4800002c
sse_timer_read SSE system timer read: offset 0x2c data 0x5 size 4
[S +0.027059] OK 0x0000000000000005
[R +0.027075] readl 0x48000040
sse_timer_read SSE system timer read: offset 0x40 data 0x11f8 size 4
[S +0.027082] OK 0x00000000000011f8
[R +0.027098] readl 0x48000044
sse_timer_read SSE system timer read: offset 0x44 data 0x0 size 4
[S +0.027103] OK 0x0000000000000000
[R +0.027119] writel 0x4800004c 0x0
sse_timer_write SSE system timer write: offset 0x4c data 0x0 size 4
[S +0.027136] OK
[R +0.027151] clock_step 8858370048000
[S +0.027160] OK 8858370196750
[R +0.027176] readl 0x48000000
sse_timer_read SSE system timer read: offset 0x0 data 0x1130 size 4
[S +0.027186] OK 0x0000000000001130
[R +0.027201] readl 0x48000004
sse_timer_read SSE system timer read: offset 0x4 data 0x42 size 4
[S +0.027206] OK 0x0000000000000042
[R +0.027222] writel 0x4800004c 0x1
sse_timer_write SSE system timer write: offset 0x4c data 0x1 size 4
[S +0.027230] OK
[R +0.027246] readl 0x48000040
sse_timer_read SSE system timer read: offset 0x40 data 0x11f8 size 4
[S +0.027255] OK 0x00000000000011f8
[R +0.027271] readl 0x48000044
sse_timer_read SSE system timer read: offset 0x44 data 0x42 size 4
[S +0.027278] OK 0x0000000000000042
ok 2 /arm/sse-timer/timer
[R +0.027312] writel 0x4802e008 0x0
[S +0.027316] OK
[R +0.027334] writel 0x58100000 0x0
sse_counter_control_write SSE system counter control framen write:
offset 0x0 data 0x0 size 4
[S +0.027342] OK
[R +0.027357] writel 0x4800002c 0x0
sse_timer_write SSE system timer write: offset 0x2c data 0x0 size 4
[S +0.027362] OK
[R +0.027378] writel 0x4800004c 0x0
sse_timer_write SSE system timer write: offset 0x4c data 0x0 size 4
[S +0.027387] OK
[R +0.027400] writel 0x58100008 0x0
sse_counter_control_write SSE system counter control framen write:
offset 0x8 data 0x0 size 4
[S +0.027406] OK
[R +0.027422] writel 0x5810000c 0x0
sse_counter_control_write SSE system counter control framen write:
offset 0xc data 0x0 size 4
[S +0.027428] OK
[R +0.027443] writel 0x4802e008 0x1
[S +0.027448] OK
[R +0.027464] writel 0x50080070 0x1
[S +0.027469] OK
[R +0.027484] writel 0x58100000 0x1
sse_counter_control_write SSE system counter control framen write:
offset 0x0 data 0x1 size 4
[S +0.027493] OK
[R +0.027511] writel 0x4800002c 0x1
sse_timer_write SSE system timer write: offset 0x2c data 0x1 size 4
[S +0.027520] OK
[R +0.027536] writel 0x48000020 0xfa0
sse_timer_write SSE system timer write: offset 0x20 data 0xfa0 size 4
[S +0.027546] OK
[R +0.027561] writel 0x48000024 0x0
sse_timer_write SSE system timer write: offset 0x24 data 0x0 size 4
[S +0.027569] OK
[R +0.027585] clock_step 62500
[S +0.027595] OK 8858370259250
[R +0.027610] readl 0x4800002c
sse_timer_read SSE system timer read: offset 0x2c data 0x1 size 4
[S +0.027617] OK 0x0000000000000001
[R +0.027633] writel 0x58100000 0x0
sse_counter_control_write SSE system counter control framen write:
offset 0x0 data 0x0 size 4
[S +0.027640] OK
[R +0.027655] writel 0x58100010 0x100000
sse_counter_control_write SSE system counter control framen write:
offset 0x10 data 0x100000 size 4
[S +0.027666] OK
[R +0.027681] writel 0x58100000 0x5
sse_counter_control_write SSE system counter control framen write:
offset 0x0 data 0x5 size 4
[S +0.027688] OK
[R +0.027703] clock_step 62500
[S +0.027711] OK 8858370321750
[R +0.027726] readl 0x4800002c
sse_timer_read SSE system timer read: offset 0x2c data 0x1 size 4
[S +0.027732] OK 0x0000000000000001
[R +0.027748] clock_step 937375
[S +0.027755] OK 8858371259125
[R +0.027768] readl 0x4800002c
sse_timer_read SSE system timer read: offset 0x2c data 0x1 size 4
[S +0.027775] OK 0x0000000000000001
[R +0.027791] clock_step 125
[S +0.027797] OK 8858371259250
[R +0.027812] readl 0x4800002c
sse_timer_read SSE system timer read: offset 0x2c data 0x5 size 4
[S +0.027819] OK 0x0000000000000005
ok 3 /arm/sse-timer/timer-scale-change
# End of sse-timer tests
# End of arm tests
[I +0.028089] CLOSED


On Fri, Dec 13, 2024 at 2:40 PM 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
>
> 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
>
> Signed-off-by: Nabih Estefan <nabihestefan@google.com>
> Tested-by: Thomas Huth <thuth@redhat.com>
> ---
>  tests/qtest/sse-timer-test.c | 19 ++++++++++++++-----
>  1 file changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/tests/qtest/sse-timer-test.c b/tests/qtest/sse-timer-test.c
> index fd5635d4c9..d7a53ac23a 100644
> --- a/tests/qtest/sse-timer-test.c
> +++ b/tests/qtest/sse-timer-test.c
> @@ -29,6 +29,13 @@
>  /* Base of the System Counter control frame */
>  #define COUNTER_BASE 0x58100000
>
> +/* Base of the MSSDK APB Watchdog Device */
> +#define WDOG_BASE 0x4802e000
> +
> +/* CMSDK Watchdog offsets */
> +#define WDOGLOAD 0
> +#define WDOGCONTROL 8
> +
>  /* SSE counter register offsets in the control frame */
>  #define CNTCR 0
>  #define CNTSR 0x4
> @@ -63,24 +70,26 @@ static void clock_step_ticks(uint64_t ticks)
>      clock_step(FOUR_TICKS * (ticks >> 2));
>  }
>
> -static void reset_counter_and_timer(void)
> +static void reset_watchdog_counter_and_timer(void)
>  {
>      /*
> -     * Reset the system counter and the timer between tests. This
> +     * Reset the system watchdog, counter and the timer between tests. This
>       * isn't a full reset, but it's sufficient for what the tests check.
>       */
> +    writel(WDOG_BASE + WDOGCONTROL, 0);
>      writel(COUNTER_BASE + CNTCR, 0);
>      writel(TIMER_BASE + CNTP_CTL, 0);
>      writel(TIMER_BASE + CNTP_AIVAL_CTL, 0);
>      writel(COUNTER_BASE + CNTCV_LO, 0);
>      writel(COUNTER_BASE + CNTCV_HI, 0);
> +    writel(WDOG_BASE + WDOGCONTROL, 1);
>  }
>
>  static void test_counter(void)
>  {
>      /* Basic counter functionality test */
>
> -    reset_counter_and_timer();
> +    reset_watchdog_counter_and_timer();
>      /* The counter should start disabled: check that it doesn't move */
>      clock_step_ticks(100);
>      g_assert_cmpuint(readl(COUNTER_BASE + CNTCV_LO), ==, 0);
> @@ -103,7 +112,7 @@ static void test_timer(void)
>  {
>      /* Basic timer functionality test */
>
> -    reset_counter_and_timer();
> +    reset_watchdog_counter_and_timer();
>      /*
>       * The timer is behind a Peripheral Protection Controller, and
>       * qtest accesses are always non-secure (no memory attributes),
> @@ -195,7 +204,7 @@ static void test_timer_scale_change(void)
>       * Test that the timer responds correctly to counter
>       * scaling changes while it has an active timer.
>       */
> -    reset_counter_and_timer();
> +    reset_watchdog_counter_and_timer();
>      /* Give ourselves access to the timer, and enable the counter and timer */
>      writel(PERIPHNSPPC0, 1);
>      writel(COUNTER_BASE + CNTCR, 1);
> --
> 2.47.1.613.gc27f4b7a9f-goog
>
Peter Maydell Dec. 16, 2024, 1:39 p.m. UTC | #2 
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
Nabih Estefan Dec. 16, 2024, 7:26 p.m. UTC | #3 
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
Peter Maydell Dec. 17, 2024, 10:28 a.m. UTC | #4 
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
diff mbox series

Patch

diff --git a/tests/qtest/sse-timer-test.c b/tests/qtest/sse-timer-test.c
index fd5635d4c9..d7a53ac23a 100644
--- a/tests/qtest/sse-timer-test.c
+++ b/tests/qtest/sse-timer-test.c
@@ -29,6 +29,13 @@ 
 /* Base of the System Counter control frame */
 #define COUNTER_BASE 0x58100000
 
+/* Base of the MSSDK APB Watchdog Device */
+#define WDOG_BASE 0x4802e000
+
+/* CMSDK Watchdog offsets */
+#define WDOGLOAD 0
+#define WDOGCONTROL 8
+
 /* SSE counter register offsets in the control frame */
 #define CNTCR 0
 #define CNTSR 0x4
@@ -63,24 +70,26 @@  static void clock_step_ticks(uint64_t ticks)
     clock_step(FOUR_TICKS * (ticks >> 2));
 }
 
-static void reset_counter_and_timer(void)
+static void reset_watchdog_counter_and_timer(void)
 {
     /*
-     * Reset the system counter and the timer between tests. This
+     * Reset the system watchdog, counter and the timer between tests. This
      * isn't a full reset, but it's sufficient for what the tests check.
      */
+    writel(WDOG_BASE + WDOGCONTROL, 0);
     writel(COUNTER_BASE + CNTCR, 0);
     writel(TIMER_BASE + CNTP_CTL, 0);
     writel(TIMER_BASE + CNTP_AIVAL_CTL, 0);
     writel(COUNTER_BASE + CNTCV_LO, 0);
     writel(COUNTER_BASE + CNTCV_HI, 0);
+    writel(WDOG_BASE + WDOGCONTROL, 1);
 }
 
 static void test_counter(void)
 {
     /* Basic counter functionality test */
 
-    reset_counter_and_timer();
+    reset_watchdog_counter_and_timer();
     /* The counter should start disabled: check that it doesn't move */
     clock_step_ticks(100);
     g_assert_cmpuint(readl(COUNTER_BASE + CNTCV_LO), ==, 0);
@@ -103,7 +112,7 @@  static void test_timer(void)
 {
     /* Basic timer functionality test */
 
-    reset_counter_and_timer();
+    reset_watchdog_counter_and_timer();
     /*
      * The timer is behind a Peripheral Protection Controller, and
      * qtest accesses are always non-secure (no memory attributes),
@@ -195,7 +204,7 @@  static void test_timer_scale_change(void)
      * Test that the timer responds correctly to counter
      * scaling changes while it has an active timer.
      */
-    reset_counter_and_timer();
+    reset_watchdog_counter_and_timer();
     /* Give ourselves access to the timer, and enable the counter and timer */
     writel(PERIPHNSPPC0, 1);
     writel(COUNTER_BASE + CNTCR, 1);