Re: [Qemu-devel] [PATCH v2] hw/ptimer: Don't wrap around counter for expired timer that uses tick handler

[Dmitry Osipenko <digetx@gmail.com>]

On 27.06.2016 16:27, Peter Maydell wrote:
> I guess this fixes a regression, but it looks really weird.
> Why should the timer behaviour change if there happens to be
> a bottom half present? That should be an internal implementation
> detail. It's also a bit odd that use_icount is in the check:
> that shouldn't generally affect device emulation behaviour...

In case of a polled timer that doesn't have ptimer trigger bottom half callback
setup, we are free to wrap around counter since timer behaviour isn't changed
from ptimer user perspective, as it won't be able to change it's state in the
handler.

I just decided to keep that wraparound feature for a case of a polled free
running timer, this should result in a better distribution of the polled value.
The potential users of that feature are "imx_epit" and "digic" timer device
models. I should have mentioned it in the commit message to avoid confusion, sorry.

It is still an internal implementation detail, not sure what you are meaning.
Could you elaborate, please?

"use_icount" is redundant now and should be omitted, good point.

This patch is supposed to fix IRQ set -> timer expire/counter wraparound
ordering. I'm wondering whether we have same ordering issue with the scheduled
ptimer trigger callback. I can imagine the following scenario:

1) ptimer_tick() -> periodic counter reloaded or set to "0" in oneshot mode and
ptimer_trigger() schedules trigger callback

2) device uses ptimer_get_count() but the scheduled callback still pending
execution.

That should happen in the same QEMU "cycle" to cause a potential issue. Is this
possible? I'm not familiar with how AIO scheduler works.

The potential fix could be: to directly call trigger callback from ptimer_tick()
after changing the delta value.

-- 
Dmitry