qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PULL 17/28] hw/ptimer: Perform counter wrap around if


From: Mark Cave-Ayland
Subject: Re: [Qemu-devel] [PULL 17/28] hw/ptimer: Perform counter wrap around if timer already expired
Date: Fri, 24 Jun 2016 16:58:25 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Icedove/45.1.0

On 06/06/16 15:47, Peter Maydell wrote:

From: Dmitry Osipenko <address@hidden>

ptimer_get_count() might be called while QEMU timer already been expired.
In that case ptimer would return counter = 0, which might be undesirable
in case of polled timer. Do counter wrap around for periodic timer to keep
it distributed. In order to achieve more accurate emulation behaviour of
certain hardware, don't perform wrap around when in icount mode and return
counter = 0 in that case (that doesn't affect polled counter distribution).

Signed-off-by: Dmitry Osipenko <address@hidden>
Reviewed-by: Peter Crosthwaite <address@hidden>
Message-id: address@hidden
Signed-off-by: Peter Maydell <address@hidden>
---
 hw/core/ptimer.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/hw/core/ptimer.c b/hw/core/ptimer.c
index 16d7dd5..7e6fc2d 100644
--- a/hw/core/ptimer.c
+++ b/hw/core/ptimer.c
@@ -84,14 +84,16 @@ static void ptimer_tick(void *opaque)

 uint64_t ptimer_get_count(ptimer_state *s)
 {
-    int64_t now;
     uint64_t counter;

     if (s->enabled) {
-        now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+        int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+        int64_t next = s->next_event;
+        bool expired = (now - next >= 0);
+        bool oneshot = (s->enabled == 2);
+
         /* Figure out the current counter value.  */
-        if (now - s->next_event > 0
-            || s->period == 0) {
+        if (s->period == 0 || (expired && (oneshot || use_icount))) {
             /* Prevent timer underflowing if it should already have
                triggered.  */
             counter = 0;
@@ -103,7 +105,7 @@ uint64_t ptimer_get_count(ptimer_state *s)
             uint32_t period_frac = s->period_frac;
             uint64_t period = s->period;

-            if ((s->enabled == 1) && !use_icount && (s->delta * period < 
10000)) {
+            if (!oneshot && (s->delta * period < 10000) && !use_icount) {
                 period = 10000 / s->delta;
                 period_frac = 0;
             }
@@ -118,7 +120,7 @@ uint64_t ptimer_get_count(ptimer_state *s)
                backwards.
             */

-            rem = s->next_event - now;
+            rem = expired ? now - next : next - now;
             div = period;

             clz1 = clz64(rem);
@@ -138,6 +140,11 @@ uint64_t ptimer_get_count(ptimer_state *s)
                     div += 1;
             }
             counter = rem / div;
+
+            if (expired && counter != 0) {
+                /* Wrap around periodic counter.  */
+                counter = s->limit - (counter - 1) % s->limit;
+            }
         }
     } else {
         counter = s->delta;


Hi Peter,

Whilst testing Artyom's qemu-system-sparc patch today, I noticed another regression which I've bisected down to the above commit.

Booting my NetBSD/OpenBSD test images with current git master causes the following warning to appear on the console: "WARNING: negative runtime; monotonic clock has gone backwards".

Could this be a regression or does qemu-system-sparc make an incorrect assumption as to how the timer should work in this scenario?


ATB,

Mark.




reply via email to

[Prev in Thread] Current Thread [Next in Thread]