qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH v10 2/7] hw/ptimer: Perform tick and counter wra


From: Peter Crosthwaite
Subject: Re: [Qemu-devel] [PATCH v10 2/7] hw/ptimer: Perform tick and counter wrap around if timer already expired
Date: Sat, 9 Jan 2016 16:44:51 -0800

On Sat, Jan 9, 2016 at 9:39 AM, Dmitry Osipenko <address@hidden> 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).
>
> In addition, there is no reason to keep expired timer tick deferred, so
> just perform the tick from ptimer_get_count().
>
> Signed-off-by: Dmitry Osipenko <address@hidden>
> ---
>  hw/core/ptimer.c | 26 +++++++++++++++++++-------
>  1 file changed, 19 insertions(+), 7 deletions(-)
>
> diff --git a/hw/core/ptimer.c b/hw/core/ptimer.c
> index c3d31cb..cf329eb 100644
> --- a/hw/core/ptimer.c
> +++ b/hw/core/ptimer.c
> @@ -83,14 +83,17 @@ static void ptimer_tick(void *opaque)
>
>  uint64_t ptimer_get_count(ptimer_state *s)
>  {
> -    int64_t now;
> +    int enabled = s->enabled;

You haven't added any additional usages of s->enabled to really
warrant the new local variable, I think it should just stay as
s->enabled to avoid churn.

>      uint64_t counter;
>
> -    if (s->enabled) {
> -        now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> +    if (enabled) {
> +        int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> +        int64_t next = s->next_event;
> +        int expired = (now - next >= 0);

bool.

> +        int oneshot = (enabled == 2);

bool.

Otherwise:

Reviewed-by: Peter Crosthwaite <address@hidden>

> +
>          /* Figure out the current counter value.  */
> -        if (now - s->next_event > 0
> -            || s->period == 0) {
> +        if (s->period == 0 || (expired && (use_icount || oneshot))) {
>              /* Prevent timer underflowing if it should already have
>                 triggered.  */
>              counter = 0;
> @@ -102,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 && !use_icount && (s->delta * period < 10000)) {
>                  period = 10000 / s->delta;
>                  period_frac = 0;
>              }
> @@ -117,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);
> @@ -137,6 +140,15 @@ 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;
> +            }
> +        }
> +
> +        if (expired) {
> +            ptimer_tick(s);
>          }
>      } else {
>          counter = s->delta;
> --
> 2.6.4
>



reply via email to

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