[PATCH v2 00/21] transaction-based ptimer API

From: Peter Maydell <peter.maydell@linaro.org>
To: qemu-arm@nongnu.org, qemu-devel@nongnu.org
Cc: "Philippe Mathieu-Daudé" <philmd@redhat.com>,
	"Richard Henderson" <richard.henderson@linaro.org>,
	"Paolo Bonzini" <pbonzini@redhat.com>
Subject: [PATCH v2 00/21] transaction-based ptimer API
Date: Tue,  8 Oct 2019 18:17:19 +0100	[thread overview]
Message-ID: <20191008171740.9679-1-peter.maydell@linaro.org> (raw)

Currently the ptimer design uses a QEMU bottom-half as its mechanism
for calling back into the device model using the ptimer when the
timer has expired.  Unfortunately this design is fatally flawed,
because it means that there is a lag between the ptimer updating its
own state and the device callback function updating device state, and
guest accesses to device registers between the two can return
inconsistent device state. This was reported as a bug in a specific
timer device but it's a problem with the generic ptimer code:
https://bugs.launchpad.net/qemu/+bug/1777777

This patchset introduces a change to the ptimer API so that instead
of using a bottom-half for the trigger a device can choose to use a
new 'transaction' based approach.  In this design (suggested by RTH)
all calls which modify ptimer state:
     - ptimer_set_period()
     - ptimer_set_freq()
     - ptimer_set_limit()
     - ptimer_set_count()
     - ptimer_run()
     - ptimer_stop()
must be between matched calls to ptimer_transaction_begin() and
ptimer_transaction_commit().  When ptimer_transaction_commit() is
called it will evaluate the state of the timer after all the changes
in the transaction, and call the callback if necessary.
The callback itself is called from within a transaction block,
so any changes it makes to ptimer state that re-trigger the
timer will mean the callback is called again once it has returned.

Changes since the v1 RFC patchset:
 - In the ptimer implementation patch itself:
     * ptimer_transaction_begin() now sets need_reload to false
     * fixed assert condition in ptimer_transaction_begin()
     * ptimer_transaction_commit() now has a loop to call
       ptimer_reload() again if the callback function updated
       the ptimer state such that it needs to trigger again
     * fixed callback_opaque arg name mismatch in doc comment
     * don't cache delta, period, etc across ptimer_trigger() call,
       because the device's trigger function might update ptimer
       state
 - New patches which update all the devices used in various
   Arm boards to the new transaction-based API

(Most of the bugfixes listed are the result of the extra testing
in the wider variety of ptimer use cases. Thanks in particular
to Philippe for putting together a test image for the exynos4210,
which has several ptimer-using devices some of which are pretty
complicated.)

There are ten non-arm devices using ptimer:

microblaze and ppc:
  hw/timer/xilinx_timer.c
ppc:
  hw/net/fsl_etsec/etsec.c
nios2:
  hw/timer/altera_timer.c
cris:
  hw/timer/etraxfs_timer.c
lm32:
  hw/timer/lm32_timer.c
  hw/timer/milkymist-sysctl.c
sparc:
  hw/timer/grlib_gptimer.c
  hw/timer/slavio_timer.c
sh4:
  hw/timer/sh_timer.c
unicore32:
  hw/timer/puv3_ost.c

I do plan to convert those as well but this series seems big
enough to be going on with, and it means I can avoid the
awkwardness of getting acks from multiple submaintainers
on top of wrangling a big patchset.

thanks
-- PMM

Peter Maydell (21):
  ptimer: Rename ptimer_init() to ptimer_init_with_bh()
  ptimer: Provide new transaction-based API
  tests/ptimer-test: Switch to transaction-based ptimer API
  hw/timer/arm_timer.c: Switch to transaction-based ptimer API
  hw/arm/musicpal.c: Switch to transaction-based ptimer API
  hw/timer/allwinner-a10-pit.c: Switch to transaction-based ptimer API
  hw/timer/arm_mptimer.c: Switch to transaction-based ptimer API
  hw/timer/cmsdk-apb-dualtimer.c: Switch to transaction-based ptimer API
  hw/timer/cmsdk-apb-timer.c: Switch to transaction-based ptimer API
  hw/timer/digic-timer.c: Switch to transaction-based ptimer API
  hw/timer/exynos4210_mct.c: Switch GFRC to transaction-based ptimer API
  hw/timer/exynos4210_mct.c: Switch LFRC to transaction-based ptimer API
  hw/timer/exynos4210_mct.c: Switch ltick to transaction-based ptimer
    API
  hw/timer/exynos4210_pwm.c: Switch to transaction-based ptimer API
  hw/timer/exynos4210_rtc.c: Switch 1Hz ptimer to transaction-based API
  hw/timer/exynos4210_rtc.c: Switch main ptimer to transaction-based API
  hw/timer/imx_epit.c: Switch to transaction-based ptimer API
  hw/timer/imx_gpt.c: Switch to transaction-based ptimer API
  hw/timer/mss-timerc: Switch to transaction-based ptimer API
  hw/watchdog/cmsdk-apb-watchdog.c: Switch to transaction-based ptimer
    API
  hw/net/lan9118.c: Switch to transaction-based ptimer API

 include/hw/ptimer.h              |  83 ++++++++++++++++-
 include/hw/timer/mss-timer.h     |   1 -
 hw/arm/musicpal.c                |  16 ++--
 hw/core/ptimer.c                 | 154 +++++++++++++++++++++++++++----
 hw/dma/xilinx_axidma.c           |   2 +-
 hw/m68k/mcf5206.c                |   2 +-
 hw/m68k/mcf5208.c                |   2 +-
 hw/net/fsl_etsec/etsec.c         |   2 +-
 hw/net/lan9118.c                 |  11 ++-
 hw/timer/allwinner-a10-pit.c     |  12 ++-
 hw/timer/altera_timer.c          |   2 +-
 hw/timer/arm_mptimer.c           |  18 +++-
 hw/timer/arm_timer.c             |  16 +++-
 hw/timer/cmsdk-apb-dualtimer.c   |  14 ++-
 hw/timer/cmsdk-apb-timer.c       |  15 ++-
 hw/timer/digic-timer.c           |  16 +++-
 hw/timer/etraxfs_timer.c         |   6 +-
 hw/timer/exynos4210_mct.c        | 107 ++++++++++++++++++---
 hw/timer/exynos4210_pwm.c        |  17 +++-
 hw/timer/exynos4210_rtc.c        |  22 +++--
 hw/timer/grlib_gptimer.c         |   2 +-
 hw/timer/imx_epit.c              |  32 ++++++-
 hw/timer/imx_gpt.c               |  21 ++++-
 hw/timer/lm32_timer.c            |   2 +-
 hw/timer/milkymist-sysctl.c      |   4 +-
 hw/timer/mss-timer.c             |  11 ++-
 hw/timer/puv3_ost.c              |   2 +-
 hw/timer/sh_timer.c              |   2 +-
 hw/timer/slavio_timer.c          |   2 +-
 hw/timer/xilinx_timer.c          |   2 +-
 hw/watchdog/cmsdk-apb-watchdog.c |  13 ++-
 tests/ptimer-test.c              | 106 ++++++++++++++++-----
 32 files changed, 584 insertions(+), 133 deletions(-)

-- 
2.20.1