[Qemu-devel] [PULL 01/19] armv7m_nvic: systick: Reload the RELOAD value and count down only if ENABLE bit is set

[Peter Maydell <peter.maydell@linaro.org>]

From: Adrian Huang <adrianhuang0701@gmail.com>

Consider the following pseudo code to configure SYSTICK (The
recommended programming sequence from "the definitive guide to the
arm cortex-m3"):
    SYSTICK Reload Value Register = 0xffff
    SYSTICK Current Value Register = 0
    SYSTICK Control and Status Register = 0x7

The pseudo code "SYSTICK Current Value Register = 0" leads to invoking
systick_reload(). As a consequence, the systick.tick member is updated
and the systick timer starts to count down when the ENABLE bit of
SYSTICK Control and Status Register is cleared.

The worst case is that: during the system initialization, the reset
value of the SYSTICK Control and Status Register is 0x00000000.
When the code "SYSTICK Current Value Register = 0" is executed, the
systick.tick member is accumulated with "(s->systick.reload + 1) *
systick_scale(s)". The systick_scale() gets the external_ref_clock
scale because the CLKSOURCE bit of the SYSTICK Control and Status
Register is cleared. This is the incorrect behavior because of the
code "SYSTICK Control and Status Register = 0x7". Actually, we want
the processor clock instead of the external reference clock.

This incorrect behavior defers the generation of the first interrupt.

The patch fixes the above-mentioned issue by setting the systick.tick
member and modifying the systick timer only if the ENABLE bit of
the SYSTICK Control and Status Register is set.

In addition, the Cortex-M3 Devices Generic User Guide mentioned that
"When ENABLE is set to 1, the counter loads the RELOAD value from the
SYST RVR register and then counts down". This patch adheres to the
statement of the user guide.

Signed-off-by: Adrian Huang <adrianhuang0701@gmail.com>
Reviewed-by: Jim Huang <jserv.tw@gmail.com>
[PMM: minor tweak to comment text]
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/intc/armv7m_nvic.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index 6ff6c7f..ca19157 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -77,6 +77,15 @@ static inline int64_t systick_scale(nvic_state *s)
 
 static void systick_reload(nvic_state *s, int reset)
 {
+    /* The Cortex-M3 Devices Generic User Guide says that "When the
+     * ENABLE bit is set to 1, the counter loads the RELOAD value from the
+     * SYST RVR register and then counts down". So, we need to check the
+     * ENABLE bit before reloading the value.
+     */
+    if ((s->systick.control & SYSTICK_ENABLE) == 0) {
+        return;
+    }
+
     if (reset)
         s->systick.tick = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
     s->systick.tick += (s->systick.reload + 1) * systick_scale(s);
-- 
1.9.1