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

[Andrew Jones]

On Wed, Nov 04, 2020 at 11:11:53AM +0100, Philippe Mathieu-Daudé wrote:
> On 11/4/20 11:03 AM, Andrew Jones wrote:
> > 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
> 
> Oops yes indeed.

OK, I'll post it.

Thanks,
drew

> 
> >  
> >  config ALLWINNER_A10
> >      bool
> > 
> > 
> > Thanks,
> > drew
> > 
> > 
> 
>