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 RFC 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.
I'm sending this out as an RFC to get opinions on the new API
before I proceed to converting all the users of ptimer. (My
plan is that the final patchset will start with these patches,
have another couple of dozen patches changing the other timer
devices using ptimer, and then have a "delete the BH API/code"
patch. Or we could split the "change to new API" parts into
smaller per-architecture ones.)
Patch 1 is some new trace events I added to the arm_timer code
while I was investigating the original bug report.
Patch 2 is just a function rename so we can keep the nicer
"ptimer_init()" name for the new API.
Patch 3 adds the new API and its implementation.
Patch 4 is a sample conversion of a ptimer-using device,
which fixes the reported bug:
https://bugs.launchpad.net/qemu/+bug/1777777
thanks
-- PMM
Peter Maydell (4):
hw/timer/arm_timer: Add trace events
ptimer: Rename ptimer_init() to ptimer_init_with_bh()
ptimer: Provide new transaction-based API
hw/timer/arm_timer.c: Switch to transaction-based ptimer API
include/hw/ptimer.h | 77 ++++++++++++++++++--
hw/arm/musicpal.c | 2 +-
hw/core/ptimer.c | 116 +++++++++++++++++++++++++++----
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 | 2 +-
hw/timer/allwinner-a10-pit.c | 2 +-
hw/timer/altera_timer.c | 2 +-
hw/timer/arm_mptimer.c | 6 +-
hw/timer/arm_timer.c | 43 +++++++++---
hw/timer/cmsdk-apb-dualtimer.c | 2 +-
hw/timer/cmsdk-apb-timer.c | 2 +-
hw/timer/digic-timer.c | 2 +-
hw/timer/etraxfs_timer.c | 6 +-
hw/timer/exynos4210_mct.c | 7 +-
hw/timer/exynos4210_pwm.c | 2 +-
hw/timer/exynos4210_rtc.c | 4 +-
hw/timer/grlib_gptimer.c | 2 +-
hw/timer/imx_epit.c | 4 +-
hw/timer/imx_gpt.c | 2 +-
hw/timer/lm32_timer.c | 2 +-
hw/timer/milkymist-sysctl.c | 4 +-
hw/timer/mss-timer.c | 2 +-
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 | 2 +-
tests/ptimer-test.c | 22 +++---
hw/timer/trace-events | 7 ++
32 files changed, 263 insertions(+), 75 deletions(-)
--
2.20.1
