On 20.09.2016 20:20, Peter Maydell wrote:
> On 7 September 2016 at 14:22, Dmitry Osipenko <[email protected]> wrote:
>> Currently, periodic counter wraps around immediately once counter reaches
>> "0", this is wrong behaviour for some of the timers, resulting in one period
>> being lost. Add new ptimer policy that provides correct behaviour for such
>> timers, so that counter stays with "0" for a one period before wrapping
>> around.
>
> This says it's just adding a new policy...
>
>> @@ -91,7 +96,7 @@ uint64_t ptimer_get_count(ptimer_state *s)
>> {
>> uint64_t counter;
>>
>> - if (s->enabled) {
>> + if (s->enabled && s->delta != 0) {
>> int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
>> int64_t next = s->next_event;
>> bool expired = (now - next >= 0);
>> @@ -145,6 +150,14 @@ uint64_t ptimer_get_count(ptimer_state *s)
>> div += 1;
>> }
>> counter = rem / div;
>> +
>> + if (!oneshot && s->delta == s->limit) {
>> + /* Before wrapping around, timer should stay with counter = >> 0
>> + for a one period. The delta has been adjusted by +1 for
>> + the wrapped around counter, so taking a modulo of limit
>> + 1
>> + gives that period. */
>> + counter %= s->limit + 1;
>> + }
>> }
>> } else {
>> counter = s->delta;
>
> ...but the changes in this function look like they affect
> behaviour even if that policy flag isn't set.
>
No, the delta value is adjusted only when PTIMER_POLICY_WRAP_AFTER_ONE_PERIOD is
set.
+ if (s->policy_mask & PTIMER_POLICY_WRAP_AFTER_ONE_PERIOD) {
+ delta += delta_adjust;
+ }
+
That adjustment is applied only on reload of the periodic timer.
@@ -83,7 +88,7 @@ static void ptimer_tick(void *opaque)
if (s->enabled == 2) {
s->enabled = 0;
} else {
- ptimer_reload(s);
+ ptimer_reload(s, 1);
}
}
All other ptimer_reload's are ptimer_reload(s, 0).
The periodic timer is reloaded with the limit value. When s->delta == s->limit,
we can assume that timer is free running. When delta is adjusted by +1 on
reload, the counter = limit + 1 (counter value is calculated based on elapsed
QEMU time) and that "counter %= s->limit + 1" modulo gives counter = 0 while in
fact the counter = adjusted delta (limit + 1).
When delta is *not* adjusted, the modulo returns the actual counter value, since
counter value is always less than s->limit + 1.
So, this patch doesn't affect old behaviour at all.
Looking more at it now, I think "counter %= s->limit + 1" should be changed to
"counter %= s->limit" in this patch and +1 added in the "no counter round down"
patch, since the counter value is always rounded down here. Instead of the
"staying with counter = 0 for a one period" it would wraparound to the limit
value if "wraparound after one period" policy is used. I'll change it in V17.
--
Dmitry
