diff mbox

[v11,2/7] hw/ptimer: Perform counter wrap around if timer already expired

Message ID 9fb47bb0f1355221ef074766d5cc49f2de97e79b.1453402860.git.digetx@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Dmitry Osipenko Jan. 21, 2016, 7:03 p.m. UTC
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 <digetx@gmail.com>
---
 hw/core/ptimer.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

Comments

Peter Crosthwaite Jan. 24, 2016, 5:29 a.m. UTC | #1
On Thu, Jan 21, 2016 at 11:03 AM, Dmitry Osipenko <digetx@gmail.com> wrote:
> 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 <digetx@gmail.com>

Reviewed-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>

> ---
>  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 6dc1677..cb50d30 100644
> --- a/hw/core/ptimer.c
> +++ b/hw/core/ptimer.c
> @@ -83,14 +83,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;
> @@ -102,7 +104,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;
>              }
> @@ -117,7 +119,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);
> @@ -137,6 +139,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 % s->limit;
> +            }
>          }
>      } else {
>          counter = s->delta;
> --
> 2.7.0
>
Dmitry Osipenko Jan. 25, 2016, 7:43 p.m. UTC | #2
Hello Peter,

24.01.2016 08:29, Peter Crosthwaite ?????:

[snip]

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

Noticed another nit here. This should be:

counter = s->limit - (counter - 1) % s->limit;

So wraparound for counter = -1 is counter = limit (not limit - 1). Will fix it 
in V12.

I suppose your r-b is still valid, otherwise please let me know.
diff mbox

Patch

diff --git a/hw/core/ptimer.c b/hw/core/ptimer.c
index 6dc1677..cb50d30 100644
--- a/hw/core/ptimer.c
+++ b/hw/core/ptimer.c
@@ -83,14 +83,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;
@@ -102,7 +104,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;
             }
@@ -117,7 +119,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);
@@ -137,6 +139,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 % s->limit;
+            }
         }
     } else {
         counter = s->delta;