Re: [Qemu-ppc] [PATCH 1.7 v2 1/2] PPC: Make BookE FIT/WDT timers more lazy

[Tom Musta]

On 11/25/2013 3:46 PM, Alexander Graf wrote:
> Today we fire FIT and WDT timer events every time the respective bit
> position in TB flips from 0 -> 1.
> 
> However, there is no need to do this if the end result would be that
> we're changing a TSR bit that is set to 1 to 1 again. No guest visible
> change would have occured.
> 
> So whenever we see that the TSR bit to our timer is already set, don't
> even bother to update the timer that would potentially fire it off.
> 
> However, we do need to make sure that we update our timer that notifies
> us of the TB flip when the respective TSR bit gets unset. In that case
> we do care about the flip and need to notify the guest again. So add
> a callback into our timer handlers when TSR bits get unset.
> 
> This improves performance for me when the guest is busy processing things.
> 
> Signed-off-by: Alexander Graf <address@hidden>
> 
> ---
> 
> v1 -> v2:
> 
>   - add parenthesis
>   - use QEMUTimer *
> ---
>  hw/ppc/ppc_booke.c | 43 ++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 38 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/ppc/ppc_booke.c b/hw/ppc/ppc_booke.c
> index 8bbfc72..56c4196 100644
> --- a/hw/ppc/ppc_booke.c
> +++ b/hw/ppc/ppc_booke.c
> @@ -128,7 +128,8 @@ static uint8_t booke_get_wdt_target(CPUPPCState *env, 
> ppc_tb_t *tb_env)
>  static void booke_update_fixed_timer(CPUPPCState         *env,
>                                       uint8_t           target_bit,
>                                       uint64_t          *next,
> -                                     struct QEMUTimer *timer)
> +                                     QEMUTimer         *timer,
> +                                     int               tsr_bit)
>  {
>      ppc_tb_t *tb_env = env->tb_env;
>      uint64_t delta_tick, ticks = 0;
> @@ -136,6 +137,14 @@ static void booke_update_fixed_timer(CPUPPCState         
> *env,
>      uint64_t period;
>      uint64_t now;
>  
> +    if (!(env->spr[SPR_BOOKE_TSR] & tsr_bit)) {
> +        /*
> +         * Don't arm the timer again when the guest has the current
> +         * interrupt still pending. Wait for it to ack it.
> +         */
> +        return;
> +    }
> +
>      now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
>      tb  = cpu_ppc_get_tb(tb_env, now, tb_env->tb_offset);
>      period = 1ULL << target_bit;
> @@ -167,6 +176,7 @@ static void booke_update_fixed_timer(CPUPPCState         
> *env,
>          (*next)++;
>      }
>  
> +    /* Fire the next timer */
>      timer_mod(timer, *next);
>  }
>  
> @@ -200,7 +210,8 @@ static void booke_fit_cb(void *opaque)
>      booke_update_fixed_timer(env,
>                               booke_get_fit_target(env, tb_env),
>                               &booke_timer->fit_next,
> -                             booke_timer->fit_timer);
> +                             booke_timer->fit_timer,
> +                             TSR_FIS);
>  }
>  
>  static void booke_wdt_cb(void *opaque)
> @@ -220,15 +231,35 @@ static void booke_wdt_cb(void *opaque)
>      booke_update_fixed_timer(env,
>                               booke_get_wdt_target(env, tb_env),
>                               &booke_timer->wdt_next,
> -                             booke_timer->wdt_timer);
> +                             booke_timer->wdt_timer,
> +                             TSR_WIS);
>  }
>  
>  void store_booke_tsr(CPUPPCState *env, target_ulong val)
>  {
>      PowerPCCPU *cpu = ppc_env_get_cpu(env);
> +    ppc_tb_t *tb_env = env->tb_env;
> +    booke_timer_t *booke_timer = tb_env->opaque;
>  
>      env->spr[SPR_BOOKE_TSR] &= ~val;
>      kvmppc_clear_tsr_bits(cpu, val);
> +
> +    if (val & TSR_FIS) {
> +        booke_update_fixed_timer(env,
> +                                 booke_get_fit_target(env, tb_env),
> +                                 &booke_timer->fit_next,
> +                                 booke_timer->fit_timer,
> +                                 TSR_FIS);
> +    }
> +
> +    if (val & TSR_WIS) {
> +        booke_update_fixed_timer(env,
> +                                 booke_get_wdt_target(env, tb_env),
> +                                 &booke_timer->wdt_next,
> +                                 booke_timer->wdt_timer,
> +                                 TSR_WIS);
> +    }
> +
>      booke_update_irq(cpu);
>  }
>  
> @@ -247,12 +278,14 @@ void store_booke_tcr(CPUPPCState *env, target_ulong val)
>      booke_update_fixed_timer(env,
>                               booke_get_fit_target(env, tb_env),
>                               &booke_timer->fit_next,
> -                             booke_timer->fit_timer);
> +                             booke_timer->fit_timer,
> +                             TSR_FIS);
>  
>      booke_update_fixed_timer(env,
>                               booke_get_wdt_target(env, tb_env),
>                               &booke_timer->wdt_next,
> -                             booke_timer->wdt_timer);
> +                             booke_timer->wdt_timer,
> +                             TSR_WIS);
>  }
>  
>  static void ppc_booke_timer_reset_handle(void *opaque)
> 

@Reviewed-by: Tom Musta <address@hidden>