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

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

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
>