Re: [Qemu-devel] [PATCH 1.7 v2 2/2] PPC: BookE: Make FIT/WDT timers at best millisecond grained

[Stefan Weil]

Am 25.11.2013 22:46, schrieb Alexander Graf:
> The default granularity for the FIT timer on 440 is on every 0x1000th
> transition of TB from 0 to 1. Translated that means 48828 times a second.
>
> Since interrupts are quite expensive for 440 and we don't really care
> about the accuracy of the FIT to that significance, let's force FIT and
> WDT to at best millisecond granularity.
>
> This basically restores behavior as it was in QEMU 1.6, where timers
> could only deal with millisecond granularities at all.
>
> This patch greatly improves performance with the 440 target and restores
> roughly the same performance level that QEMU 1.6 had for me.
>
> Signed-off-by: Alexander Graf <address@hidden>
>
> ---
>
> v1 -> v2:
>
>   - s/microseconds/milliseconds/g
> ---
>  hw/ppc/ppc_booke.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/hw/ppc/ppc_booke.c b/hw/ppc/ppc_booke.c
> index 56c4196..b421620 100644
> --- a/hw/ppc/ppc_booke.c
> +++ b/hw/ppc/ppc_booke.c
> @@ -174,6 +174,12 @@ static void booke_update_fixed_timer(CPUPPCState         
> *env,
>  
>      if (*next == now) {
>          (*next)++;
> +    } else {
> +        /*
> +         * There's no point to fake any granularity that's more fine grained
> +         * than milliseconds. Anything beyond that just overloads the system.
> +         */
> +        *next = MAX(*next, now + SCALE_MS);
>      }
>  
>      /* Fire the next timer */

Reviewed-by: Stefan Weil <address@hidden>

Looking closer at the code above, I think that the 'if' part (*next)++
could be removed because the 'else' part does a better job with *next =
*next + SCALE_MS when *next == now. This might further improve the
performance. Maybe I am wrong and the guest expects the next timer
interrupt immediately in this case, then delaying it one millisecond
would be bad.

If you decide to make a v3 patch with this modification, you may use my
reviewed-by, too.

I cannot say much to patch 1 because I don't know the BookE hardware.
The code looks good.

Regards,
Stefan