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

[Fabiano Rosas]

Nabih Estefan <nabihestefan@google.com> writes:

> 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>
> ---
>  scripts/meson-buildoptions.sh.tmp |  0
>  tests/qtest/sse-timer-test.c      | 19 ++++++++++++++-----
>  2 files changed, 14 insertions(+), 5 deletions(-)
>  create mode 100644 scripts/meson-buildoptions.sh.tmp
>
> diff --git a/scripts/meson-buildoptions.sh.tmp 
> b/scripts/meson-buildoptions.sh.tmp
> new file mode 100644
> index 0000000000..e69de29bb2

By the way, there's an extra file being created here that we don't want.

> 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);