Re: [PULL 48/48] hw/timer/armv7m_systick: Rewrite to use ptimers

From: Andrew Jones <drjones@redhat.com>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: qemu-devel@nongnu.org
Subject: Re: [PULL 48/48] hw/timer/armv7m_systick: Rewrite to use ptimers
Date: Wed, 4 Nov 2020 11:03:57 +0100	[thread overview]
Message-ID: <20201104100357.7t4nnwqot6ahprrh@kamzik.brq.redhat.com> (raw)
In-Reply-To: <20201027114438.17662-49-peter.maydell@linaro.org>

On Tue, Oct 27, 2020 at 11:44:38AM +0000, Peter Maydell 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).
> 
> 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>
> Reviewed-by: Philippe Mathieu-DaudÃ© <f4bug@amsat.org>
> Message-id: 20201015151829.14656-3-peter.maydell@linaro.org
> ---
>  include/hw/timer/armv7m_systick.h |   3 +-
>  hw/timer/armv7m_systick.c         | 124 +++++++++++++-----------------
>  2 files changed, 54 insertions(+), 73 deletions(-)
>

Do we also need something like the diff below now?

diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index fdf4464b9484..7d5d89e1acf9 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -291,6 +291,7 @@ config ZYNQ
 
 config ARM_V7M
     bool
+    select PTIMER
 
 config ALLWINNER_A10
     bool


Thanks,
drew


