[PATCH 0/2] armv7m_systick: Rewrite to use ptimers

[PATCH 0/2] armv7m_systick: Rewrite to use ptimers

[Peter Maydell]

This patch series rewrites our implementation of the armv7m systick
timer to use ptimers.

The armv7m systick timer is a 24-bit decrementing, wrap-on-zero,
clear-on-write counter.  Our current implementation has various bugs
and dubious workarounds in it (for instance see
https://bugs.launchpad.net/qemu/+bug/1872237).
    
We have an implementation of a simple decrementing counter and we put
a lot of effort into making sure it handles the interesting corner
cases (like "spend a cycle at 0 before reloading"), so rather than
trying to fix these all over again in systick's hand-rolled countdown
code it's much simpler to just rewrite it to use a ptimer.
    
Unfortunately this is a migration compatibility break, which will
affect all M-profile boards.
   
Among other bugs, this fixes
https://bugs.launchpad.net/qemu/+bug/1872237 : now writes to SYST_CVR
when the timer is enabled correctly do nothing; when the timer is
enabled via SYST_CSR.ENABLE, the ptimer code will (because of
POLICY_NO_IMMEDIATE_RELOAD) arrange that after one timer tick the
counter is reloaded from SYST_RVR and then counts down from there, as
the architecture requires.

Side note: the trace from the test program in LP1872237 won't look
quite like it does on the hardware: under QEMU the "waiting for 1000
ms" debug printing generally reports a SYST_CVR value of 0.  This is
because QEMU's emulated CPU is comparatively fast and our systick has a
hard-wired value of 1MHz for the frequency of the 'external reference
clock', which means that execution of the guest code reaches "read
SYST_CVR" before the first tick of the timer clock after enabling of
the timer (which is where the reload of SYST_CVR from SYST_RVR is
required).  The exception is the first iteration, where the time QEMU
takes to translate the guest code is enough that the timer tick
happens before the register read.  You can also get the timer tick to
win the race by fiddling around with the -icount option (which
effectively is slowing down the emulated CPU speed).

Some day we should model both the 'system_clock_scale' (ie the CPU
clock frequency) and the 'external reference clock' as QEMU clock
source/sinks so that board code can specify the correct reference
clock frequency.

Patch 1 is a minor tweak to the ptimer code to suppress a spurious
warning message for the "timer callback function disabled the ptimer"
case, which the systick timer uses.  Patch 2 is the actual
conversion.

thanks
-- PMM


Peter Maydell (2):
  hw/core/ptimer: Support ptimer being disabled by timer callback
  hw/timer/armv7m_systick: Rewrite to use ptimers

 include/hw/timer/armv7m_systick.h |   3 +-
 hw/core/ptimer.c                  |   4 +
 hw/timer/armv7m_systick.c         | 124 +++++++++++++-----------------
 3 files changed, 58 insertions(+), 73 deletions(-)

-- 
2.20.1

[PATCH 1/2] hw/core/ptimer: Support ptimer being disabled by timer callback

[Peter Maydell]

In ptimer_reload(), we call the callback function provided by the
timer device that is using the ptimer.  This callback might disable
the ptimer.  The code mostly handles this correctly, except that
we'll still print the warning about "Timer with delta zero,
disabling" if the now-disabled timer happened to be set such that it
would fire again immediately if it were enabled (eg because the
limit/reload value is zero).

Suppress the spurious warning message and the unnecessary
repeat-deletion of the underlying timer in this case.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/core/ptimer.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/core/ptimer.c b/hw/core/ptimer.c
index c6d2beb1dac..2aa97cb665c 100644
--- a/hw/core/ptimer.c
+++ b/hw/core/ptimer.c
@@ -117,6 +117,10 @@ static void ptimer_reload(ptimer_state *s, int delta_adjust)
     }
 
     if (delta == 0) {
+        if (s->enabled == 0) {
+            /* trigger callback disabled the timer already */
+            return;
+        }
         if (!qtest_enabled()) {
             fprintf(stderr, "Timer with delta zero, disabling\n");
         }
-- 
2.20.1

[PATCH 2/2] hw/timer/armv7m_systick: Rewrite to use ptimers

[Peter Maydell]

The armv7m systick timer is a 24-bit decrementing, wrap-on-zero,
clear-on-write counter. Our current implementation has various
bugs and dubious workarounds in it (for instance see
https://bugs.launchpad.net/qemu/+bug/1872237).

We have an implementation of a simple decrementing counter
and we put a lot of effort into making sure it handles the
interesting corner cases (like "spend a cycle at 0 before
reloading") -- ptimer.

Rewrite the systick timer to use a ptimer rather than
a raw QEMU timer.

Unfortunately this is a migration compatibility break,
which will affect all M-profile boards.

Among other bugs, this fixes
https://bugs.launchpad.net/qemu/+bug/1872237 :
now writes to SYST_CVR when the timer is enabled correctly
do nothing; when the timer is enabled via SYST_CSR.ENABLE,
the ptimer code will (because of POLICY_NO_IMMEDIATE_RELOAD)
arrange that after one timer tick the counter is reloaded
from SYST_RVR and then counts down from there, as the
architecture requires.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/hw/timer/armv7m_systick.h |   3 +-
 hw/timer/armv7m_systick.c         | 124 +++++++++++++-----------------
 2 files changed, 54 insertions(+), 73 deletions(-)

diff --git a/include/hw/timer/armv7m_systick.h b/include/hw/timer/armv7m_systick.h
index 97cb345ddb4..84496faaf96 100644
--- a/include/hw/timer/armv7m_systick.h
+++ b/include/hw/timer/armv7m_systick.h
@@ -14,6 +14,7 @@
 
 #include "hw/sysbus.h"
 #include "qom/object.h"
+#include "hw/ptimer.h"
 
 #define TYPE_SYSTICK "armv7m_systick"
 
@@ -27,7 +28,7 @@ struct SysTickState {
     uint32_t control;
     uint32_t reload;
     int64_t tick;
-    QEMUTimer *timer;
+    ptimer_state *ptimer;
     MemoryRegion iomem;
     qemu_irq irq;
 };
diff --git a/hw/timer/armv7m_systick.c b/hw/timer/armv7m_systick.c
index a8cec7eb56b..2f192011eb0 100644
--- a/hw/timer/armv7m_systick.c
+++ b/hw/timer/armv7m_systick.c
@@ -39,26 +39,6 @@ static inline int64_t systick_scale(SysTickState *s)
     }
 }
 
-static void systick_reload(SysTickState *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.
-     */
-    trace_systick_reload();
-
-    if ((s->control & SYSTICK_ENABLE) == 0) {
-        return;
-    }
-
-    if (reset) {
-        s->tick = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
-    }
-    s->tick += (s->reload + 1) * systick_scale(s);
-    timer_mod(s->timer, s->tick);
-}
-
 static void systick_timer_tick(void *opaque)
 {
     SysTickState *s = (SysTickState *)opaque;
@@ -70,10 +50,12 @@ static void systick_timer_tick(void *opaque)
         /* Tell the NVIC to pend the SysTick exception */
         qemu_irq_pulse(s->irq);
     }
-    if (s->reload == 0) {
-        s->control &= ~SYSTICK_ENABLE;
-    } else {
-        systick_reload(s, 0);
+    if (ptimer_get_limit(s->ptimer) == 0) {
+        /*
+         * Timer expiry with SYST_RVR zero disables the timer
+         * (but doesn't clear SYST_CSR.ENABLE)
+         */
+        ptimer_stop(s->ptimer);
     }
 }
 
@@ -94,30 +76,11 @@ static MemTxResult systick_read(void *opaque, hwaddr addr, uint64_t *data,
         s->control &= ~SYSTICK_COUNTFLAG;
         break;
     case 0x4: /* SysTick Reload Value.  */
-        val = s->reload;
+        val = ptimer_get_limit(s->ptimer);
         break;
     case 0x8: /* SysTick Current Value.  */
-    {
-        int64_t t;
-
-        if ((s->control & SYSTICK_ENABLE) == 0) {
-            val = 0;
-            break;
-        }
-        t = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
-        if (t >= s->tick) {
-            val = 0;
-            break;
-        }
-        val = ((s->tick - (t + 1)) / systick_scale(s)) + 1;
-        /* The interrupt in triggered when the timer reaches zero.
-           However the counter is not reloaded until the next clock
-           tick.  This is a hack to return zero during the first tick.  */
-        if (val > s->reload) {
-            val = 0;
-        }
+        val = ptimer_get_count(s->ptimer);
         break;
-    }
     case 0xc: /* SysTick Calibration Value.  */
         val = 10000;
         break;
@@ -149,39 +112,50 @@ static MemTxResult systick_write(void *opaque, hwaddr addr,
     switch (addr) {
     case 0x0: /* SysTick Control and Status.  */
     {
-        uint32_t oldval = s->control;
+        uint32_t oldval;
 
+        ptimer_transaction_begin(s->ptimer);
+        oldval = s->control;
         s->control &= 0xfffffff8;
         s->control |= value & 7;
+
         if ((oldval ^ value) & SYSTICK_ENABLE) {
-            int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
             if (value & SYSTICK_ENABLE) {
-                if (s->tick) {
-                    s->tick += now;
-                    timer_mod(s->timer, s->tick);
-                } else {
-                    systick_reload(s, 1);
-                }
+                /*
+                 * Always reload the period in case board code has
+                 * changed system_clock_scale. If we ever replace that
+                 * global with a more sensible API then we might be able
+                 * to set the period only when it actually changes.
+                 */
+                ptimer_set_period(s->ptimer, systick_scale(s));
+                ptimer_run(s->ptimer, 0);
             } else {
-                timer_del(s->timer);
-                s->tick -= now;
-                if (s->tick < 0) {
-                    s->tick = 0;
-                }
+                ptimer_stop(s->ptimer);
             }
         } else if ((oldval ^ value) & SYSTICK_CLKSOURCE) {
-            /* This is a hack. Force the timer to be reloaded
-               when the reference clock is changed.  */
-            systick_reload(s, 1);
+            ptimer_set_period(s->ptimer, systick_scale(s));
         }
+        ptimer_transaction_commit(s->ptimer);
         break;
     }
     case 0x4: /* SysTick Reload Value.  */
-        s->reload = value;
+        ptimer_transaction_begin(s->ptimer);
+        ptimer_set_limit(s->ptimer, value & 0xffffff, 0);
+        ptimer_transaction_commit(s->ptimer);
         break;
-    case 0x8: /* SysTick Current Value.  Writes reload the timer.  */
-        systick_reload(s, 1);
+    case 0x8: /* SysTick Current Value. */
+        /*
+         * Writing any value clears SYST_CVR to zero and clears
+         * SYST_CSR.COUNTFLAG. The counter will then reload from SYST_RVR
+         * on the next clock edge unless SYST_RVR is zero.
+         */
+        ptimer_transaction_begin(s->ptimer);
+        if (ptimer_get_limit(s->ptimer) == 0) {
+            ptimer_stop(s->ptimer);
+        }
+        ptimer_set_count(s->ptimer, 0);
         s->control &= ~SYSTICK_COUNTFLAG;
+        ptimer_transaction_commit(s->ptimer);
         break;
     default:
         qemu_log_mask(LOG_GUEST_ERROR,
@@ -210,10 +184,13 @@ static void systick_reset(DeviceState *dev)
      */
     assert(system_clock_scale != 0);
 
+    ptimer_transaction_begin(s->ptimer);
     s->control = 0;
-    s->reload = 0;
-    s->tick = 0;
-    timer_del(s->timer);
+    ptimer_stop(s->ptimer);
+    ptimer_set_count(s->ptimer, 0);
+    ptimer_set_limit(s->ptimer, 0, 0);
+    ptimer_set_period(s->ptimer, systick_scale(s));
+    ptimer_transaction_commit(s->ptimer);
 }
 
 static void systick_instance_init(Object *obj)
@@ -229,18 +206,21 @@ static void systick_instance_init(Object *obj)
 static void systick_realize(DeviceState *dev, Error **errp)
 {
     SysTickState *s = SYSTICK(dev);
-    s->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, systick_timer_tick, s);
+    s->ptimer = ptimer_init(systick_timer_tick, s,
+                            PTIMER_POLICY_WRAP_AFTER_ONE_PERIOD |
+                            PTIMER_POLICY_NO_COUNTER_ROUND_DOWN |
+                            PTIMER_POLICY_NO_IMMEDIATE_RELOAD |
+                            PTIMER_POLICY_TRIGGER_ONLY_ON_DECREMENT);
 }
 
 static const VMStateDescription vmstate_systick = {
     .name = "armv7m_systick",
-    .version_id = 1,
-    .minimum_version_id = 1,
+    .version_id = 2,
+    .minimum_version_id = 2,
     .fields = (VMStateField[]) {
         VMSTATE_UINT32(control, SysTickState),
-        VMSTATE_UINT32(reload, SysTickState),
         VMSTATE_INT64(tick, SysTickState),
-        VMSTATE_TIMER_PTR(timer, SysTickState),
+        VMSTATE_PTIMER(ptimer, SysTickState),
         VMSTATE_END_OF_LIST()
     }
 };
-- 
2.20.1

Re: [PATCH 2/2] hw/timer/armv7m_systick: Rewrite to use ptimers

[Peter Maydell]

On Thu, 15 Oct 2020 at 16:18, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> The armv7m systick timer is a 24-bit decrementing, wrap-on-zero,
> clear-on-write counter. Our current implementation has various
> bugs and dubious workarounds in it (for instance see
> https://bugs.launchpad.net/qemu/+bug/1872237).

...and 10 minutes after sending this I realized I'd forgotten
to finish removing the no-longer-needed state struct fields.
This should be squashed in:

diff --git a/include/hw/timer/armv7m_systick.h
b/include/hw/timer/armv7m_systick.h
index 84496faaf96..d57e299fd89 100644
--- a/include/hw/timer/armv7m_systick.h
+++ b/include/hw/timer/armv7m_systick.h
@@ -26,8 +26,6 @@ struct SysTickState {
     /*< public >*/

     uint32_t control;
-    uint32_t reload;
-    int64_t tick;
     ptimer_state *ptimer;
     MemoryRegion iomem;
     qemu_irq irq;
diff --git a/hw/timer/armv7m_systick.c b/hw/timer/armv7m_systick.c
index 2f192011eb0..5ac3a8a7e81 100644
--- a/hw/timer/armv7m_systick.c
+++ b/hw/timer/armv7m_systick.c
@@ -219,7 +219,6 @@ static const VMStateDescription vmstate_systick = {
     .minimum_version_id = 2,
     .fields = (VMStateField[]) {
         VMSTATE_UINT32(control, SysTickState),
-        VMSTATE_INT64(tick, SysTickState),
         VMSTATE_PTIMER(ptimer, SysTickState),
         VMSTATE_END_OF_LIST()
     }

thanks
-- PMM

Re: [PATCH 0/2] armv7m_systick: Rewrite to use ptimers

[Peter Maydell]

Ping for review ?

thanks
-- PMM

On Thu, 15 Oct 2020 at 16:18, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> This patch series rewrites our implementation of the armv7m systick
> timer to use ptimers.
>
> The armv7m systick timer is a 24-bit decrementing, wrap-on-zero,
> clear-on-write counter.  Our current implementation has various bugs
> and dubious workarounds in it (for instance see
> https://bugs.launchpad.net/qemu/+bug/1872237).
>
> We have an implementation of a simple decrementing counter and we put
> a lot of effort into making sure it handles the interesting corner
> cases (like "spend a cycle at 0 before reloading"), so rather than
> trying to fix these all over again in systick's hand-rolled countdown
> code it's much simpler to just rewrite it to use a ptimer.
>
> Unfortunately this is a migration compatibility break, which will
> affect all M-profile boards.
>
> Among other bugs, this fixes
> https://bugs.launchpad.net/qemu/+bug/1872237 : now writes to SYST_CVR
> when the timer is enabled correctly do nothing; when the timer is
> enabled via SYST_CSR.ENABLE, the ptimer code will (because of
> POLICY_NO_IMMEDIATE_RELOAD) arrange that after one timer tick the
> counter is reloaded from SYST_RVR and then counts down from there, as
> the architecture requires.
>
> Side note: the trace from the test program in LP1872237 won't look
> quite like it does on the hardware: under QEMU the "waiting for 1000
> ms" debug printing generally reports a SYST_CVR value of 0.  This is
> because QEMU's emulated CPU is comparatively fast and our systick has a
> hard-wired value of 1MHz for the frequency of the 'external reference
> clock', which means that execution of the guest code reaches "read
> SYST_CVR" before the first tick of the timer clock after enabling of
> the timer (which is where the reload of SYST_CVR from SYST_RVR is
> required).  The exception is the first iteration, where the time QEMU
> takes to translate the guest code is enough that the timer tick
> happens before the register read.  You can also get the timer tick to
> win the race by fiddling around with the -icount option (which
> effectively is slowing down the emulated CPU speed).
>
> Some day we should model both the 'system_clock_scale' (ie the CPU
> clock frequency) and the 'external reference clock' as QEMU clock
> source/sinks so that board code can specify the correct reference
> clock frequency.
>
> Patch 1 is a minor tweak to the ptimer code to suppress a spurious
> warning message for the "timer callback function disabled the ptimer"
> case, which the systick timer uses.  Patch 2 is the actual
> conversion.
>
> thanks
> -- PMM
>
>
> Peter Maydell (2):
>   hw/core/ptimer: Support ptimer being disabled by timer callback
>   hw/timer/armv7m_systick: Rewrite to use ptimers
>
>  include/hw/timer/armv7m_systick.h |   3 +-
>  hw/core/ptimer.c                  |   4 +
>  hw/timer/armv7m_systick.c         | 124 +++++++++++++-----------------
>  3 files changed, 58 insertions(+), 73 deletions(-)
>
> --
> 2.20.1
>

Re: [PATCH 0/2] armv7m_systick: Rewrite to use ptimers

[罗勇刚(Yonggang Luo)]

[-- Attachment #1: Type: text/plain, Size: 3586 bytes --]

On Tue, Oct 27, 2020 at 4:44 AM Peter Maydell <peter.maydell@linaro.org>
wrote:
>
> Ping for review ?
>
Maybe nobody can review this, anyway, is that possible add a test case for
this?
I found https://github.com/oxidecomputer/qemu-systick-bug are simple enough.
> thanks
> -- PMM
>
> On Thu, 15 Oct 2020 at 16:18, Peter Maydell <peter.maydell@linaro.org>
wrote:
> >
> > This patch series rewrites our implementation of the armv7m systick
> > timer to use ptimers.
> >
> > The armv7m systick timer is a 24-bit decrementing, wrap-on-zero,
> > clear-on-write counter.  Our current implementation has various bugs
> > and dubious workarounds in it (for instance see
> > https://bugs.launchpad.net/qemu/+bug/1872237).
> >
> > We have an implementation of a simple decrementing counter and we put
> > a lot of effort into making sure it handles the interesting corner
> > cases (like "spend a cycle at 0 before reloading"), so rather than
> > trying to fix these all over again in systick's hand-rolled countdown
> > code it's much simpler to just rewrite it to use a ptimer.
> >
> > Unfortunately this is a migration compatibility break, which will
> > affect all M-profile boards.
> >
> > Among other bugs, this fixes
> > https://bugs.launchpad.net/qemu/+bug/1872237 : now writes to SYST_CVR
> > when the timer is enabled correctly do nothing; when the timer is
> > enabled via SYST_CSR.ENABLE, the ptimer code will (because of
> > POLICY_NO_IMMEDIATE_RELOAD) arrange that after one timer tick the
> > counter is reloaded from SYST_RVR and then counts down from there, as
> > the architecture requires.
> >
> > Side note: the trace from the test program in LP1872237 won't look
> > quite like it does on the hardware: under QEMU the "waiting for 1000
> > ms" debug printing generally reports a SYST_CVR value of 0.  This is
> > because QEMU's emulated CPU is comparatively fast and our systick has a
> > hard-wired value of 1MHz for the frequency of the 'external reference
> > clock', which means that execution of the guest code reaches "read
> > SYST_CVR" before the first tick of the timer clock after enabling of
> > the timer (which is where the reload of SYST_CVR from SYST_RVR is
> > required).  The exception is the first iteration, where the time QEMU
> > takes to translate the guest code is enough that the timer tick
> > happens before the register read.  You can also get the timer tick to
> > win the race by fiddling around with the -icount option (which
> > effectively is slowing down the emulated CPU speed).
> >
> > Some day we should model both the 'system_clock_scale' (ie the CPU
> > clock frequency) and the 'external reference clock' as QEMU clock
> > source/sinks so that board code can specify the correct reference
> > clock frequency.
> >
> > Patch 1 is a minor tweak to the ptimer code to suppress a spurious
> > warning message for the "timer callback function disabled the ptimer"
> > case, which the systick timer uses.  Patch 2 is the actual
> > conversion.
> >
> > thanks
> > -- PMM
> >
> >
> > Peter Maydell (2):
> >   hw/core/ptimer: Support ptimer being disabled by timer callback
> >   hw/timer/armv7m_systick: Rewrite to use ptimers
> >
> >  include/hw/timer/armv7m_systick.h |   3 +-
> >  hw/core/ptimer.c                  |   4 +
> >  hw/timer/armv7m_systick.c         | 124 +++++++++++++-----------------
> >  3 files changed, 58 insertions(+), 73 deletions(-)
> >
> > --
> > 2.20.1
> >
>


--
         此致
礼
罗勇刚
Yours
    sincerely,
Yonggang Luo

[-- Attachment #2: Type: text/html, Size: 4616 bytes --]

Re: [PATCH 1/2] hw/core/ptimer: Support ptimer being disabled by timer callback

[Philippe Mathieu-Daudé]

On 10/15/20 5:18 PM, Peter Maydell wrote:
> In ptimer_reload(), we call the callback function provided by the
> timer device that is using the ptimer.  This callback might disable
> the ptimer.  The code mostly handles this correctly, except that
> we'll still print the warning about "Timer with delta zero,
> disabling" if the now-disabled timer happened to be set such that it
> would fire again immediately if it were enabled (eg because the
> limit/reload value is zero).
> 
> Suppress the spurious warning message and the unnecessary
> repeat-deletion of the underlying timer in this case.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/core/ptimer.c | 4 ++++
>   1 file changed, 4 insertions(+)

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Re: [PATCH 2/2] hw/timer/armv7m_systick: Rewrite to use ptimers

[Philippe Mathieu-Daudé]

On 10/15/20 5:31 PM, Peter Maydell wrote:
> On Thu, 15 Oct 2020 at 16:18, Peter Maydell <peter.maydell@linaro.org> wrote:
>>
>> The armv7m systick timer is a 24-bit decrementing, wrap-on-zero,
>> clear-on-write counter. Our current implementation has various
>> bugs and dubious workarounds in it (for instance see
>> https://bugs.launchpad.net/qemu/+bug/1872237).

Nice cleanup :)

Nitpicking we should replace the 24-bit 0xffffff constant by
a definition.

> 
> ...and 10 minutes after sending this I realized I'd forgotten
> to finish removing the no-longer-needed state struct fields.
> This should be squashed in:
> 
> diff --git a/include/hw/timer/armv7m_systick.h
> b/include/hw/timer/armv7m_systick.h
> index 84496faaf96..d57e299fd89 100644
> --- a/include/hw/timer/armv7m_systick.h
> +++ b/include/hw/timer/armv7m_systick.h
> @@ -26,8 +26,6 @@ struct SysTickState {
>       /*< public >*/
> 
>       uint32_t control;
> -    uint32_t reload;
> -    int64_t tick;
>       ptimer_state *ptimer;
>       MemoryRegion iomem;
>       qemu_irq irq;
> diff --git a/hw/timer/armv7m_systick.c b/hw/timer/armv7m_systick.c
> index 2f192011eb0..5ac3a8a7e81 100644
> --- a/hw/timer/armv7m_systick.c
> +++ b/hw/timer/armv7m_systick.c
> @@ -219,7 +219,6 @@ static const VMStateDescription vmstate_systick = {
>       .minimum_version_id = 2,
>       .fields = (VMStateField[]) {
>           VMSTATE_UINT32(control, SysTickState),
> -        VMSTATE_INT64(tick, SysTickState),
>           VMSTATE_PTIMER(ptimer, SysTickState),
>           VMSTATE_END_OF_LIST()
>       }

Patch + hunk:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>