30.01.2016 19:43, Dmitry Osipenko пишет:
Due to rounding down performed by ptimer_get_count, it returns counter - 1 for
the active timer. That's incorrect because counter should decrement only after
period been expired, not before. I.e. if running timer has been loaded with
value X, then timer counter should stay with X until period expired and
decrement after. Fix this by adding 1 to the counter value for the active and
unexpired timer.

Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
---
  hw/core/ptimer.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/core/ptimer.c b/hw/core/ptimer.c
index 62f8cb1..b2044fb 100644
--- a/hw/core/ptimer.c
+++ b/hw/core/ptimer.c
@@ -136,7 +136,7 @@ uint64_t ptimer_get_count(ptimer_state *s)
                  if ((uint32_t)(period_frac << shift))
                      div += 1;
              }
-            counter = rem / div;
+            counter = rem / div + (expired ? 0 : 1);

              if (expired && counter != 0) {
                  /* Wrap around periodic counter.  */


Noticed one nit here:

There is possibility to return timer counter = limit + 1, if the following ptimer calls execute in less than 1ns.

        ptimer_run(t, 1);
        // counter = 91, if set() count executed in less than 1ns
        ptimer_set_count(t, 90);
        counter = ptimer_get_count(t);

Likely, it would be impossible to trigger that issue on a real current machine.
But the fix is trivial, I'll incorporate it in V13 if it looks fine:

---
@@ -76,20 +76,20 @@ static void ptimer_tick(void *opaque)
 {
     ptimer_state *s = (ptimer_state *)opaque;
     s->delta = 0;
     ptimer_reload(s);
 }

 uint64_t ptimer_get_count(ptimer_state *s)
 {
+    int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
     uint64_t counter;

-    if (s->enabled && s->delta != 0) {
-        int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+    if (s->enabled && s->delta != 0 && now != s->last_event) {
         int64_t next = s->next_event;
         bool expired = (now - next >= 0);
         bool oneshot = (s->enabled == 2);

         /* Figure out the current counter value.  */
         if (expired && (oneshot || use_icount)) {
             /* Prevent timer underflowing if it should already have
                triggered.  */
@@ -131,17 +131,17 @@ uint64_t ptimer_get_count(ptimer_state *s)
             } else {
                 if (shift != 0)
                     div |= (period_frac >> (32 - shift));
                 /* Look at remaining bits of period_frac and round div up if
                    necessary.  */
                 if ((uint32_t)(period_frac << shift))
                     div += 1;
             }
-            counter = rem / div;
+            counter = rem / div + (expired ? 0 : 1);

             if (expired && counter != 0) {
                 /* Wrap around periodic counter.  */
                 counter = s->limit - (counter - 1) % s->limit;
             }
         }
     } else {
         counter = s->delta;


--
Dmitry

Reply via email to