Re: [Qemu-devel] [PATCH V2] [PowerPC][RFC] booke timers

[Scott Wood]

On Fri, 1 Jul 2011 16:13:41 +0200
Fabien Chouteau <address@hidden> wrote:

> +uint64_t cpu_ppc_get_tb(ppc_tb_t *tb_env, uint64_t vmclk, int64_t tb_offset)
>  {
>      /* TB time in tb periods */
>      return muldiv64(vmclk, tb_env->tb_freq, get_ticks_per_sec()) + tb_offset;
> @@ -678,18 +661,23 @@ static void __cpu_ppc_store_decr (CPUState *env, 
> uint64_t *nextp,
>                  decr, value);
>      now = qemu_get_clock_ns(vm_clock);
>      next = now + muldiv64(value, get_ticks_per_sec(), tb_env->decr_freq);
> -    if (is_excp)
> +    if (is_excp) {
>          next += *nextp - now;
> -    if (next == now)
> +    }
> +    if (next == now) {
>          next++;
> +    }
>      *nextp = next;
>      /* Adjust timer */
>      qemu_mod_timer(timer, next);
>      /* If we set a negative value and the decrementer was positive,
> -     * raise an exception.
> +     * raise an exception (not for booke).
>       */
> -    if ((value & 0x80000000) && !(decr & 0x80000000))
> +    if (! (env->insns_flags & PPC_BOOKE)
> +        && (value & 0x80000000)
> +        && !(decr & 0x80000000)) {
>          (*raise_excp)(env);
> +    }
>  }
>  

Also, load_decr should go from this:

    if (diff >= 0)
        decr = muldiv64(diff, tb_env->decr_freq, get_ticks_per_sec());
    else
        decr = -muldiv64(-diff, tb_env->decr_freq, get_ticks_per_sec());

to something like this:

    if (diff >= 0) {
        decr = muldiv64(diff, tb_env->decr_freq, get_ticks_per_sec());
    } else if (env->insns_flags & PPC_BOOKE) {
        decr = 0;
    } else {
        decr = -muldiv64(-diff, tb_env->decr_freq, get_ticks_per_sec());
    }

> +/* Return the location of the bit of time base at which the FIT will raise an
> +   interrupt */
> +static uint8_t booke_get_fit_target(CPUState *env)
> +{
> +    uint8_t fp = (env->spr[SPR_BOOKE_TCR] & TCR_FP_MASK) >> TCR_FP_SHIFT;
> +
> +    /* Only for e500 */
> +    if (env->insns_flags2 & PPC2_E500) {
> +        uint32_t fpext = (env->spr[SPR_BOOKE_TCR] & TCR_E500_FPEXT_MASK)
> +            >> TCR_E500_FPEXT_SHIFT;
> +        fp |= fpext << 2;
> +    } else {
> +        fp = env->fit_period[fp];
> +    }
> +
> +    return fp;
> +}
> +
> +/* Return the location of the bit of time base at which the WDT will raise an
> +   interrupt */
> +static uint8_t booke_get_wdt_target(CPUState *env)
> +{
> +    uint8_t wp = (env->spr[SPR_BOOKE_TCR] & TCR_WP_MASK) >> TCR_WP_SHIFT;
> +
> +    /* Only for e500 */
> +    if (env->insns_flags2 & PPC2_E500) {
> +        uint32_t wpext = (env->spr[SPR_BOOKE_TCR] & TCR_E500_WPEXT_MASK)
> +            >> TCR_E500_WPEXT_SHIFT;
> +        wp |= wpext << 2;
> +    } else {
> +        wp = env->wdt_period[wp];
> +    }
> +
> +    return wp;
> +}

e500 fp/wp is expressed as bits from the MSB side of TB, so we need to
subtract from 63 to get something sane -- and document that fit/wdt_period
is from the LSB side.

> +static void booke_update_fixed_timer(CPUState         *env,
> +                                     uint8_t           target_bit,
> +                                     uint64_t          *next,
> +                                     struct QEMUTimer *timer)
> +{
> +    ppc_tb_t *tb_env = env->tb_env;
> +    uint64_t lapse;
> +    uint64_t tb;
> +    uint64_t period = 1 << (target_bit + 1);
> +    uint64_t now;
> +
> +    now = qemu_get_clock_ns(vm_clock);
> +    tb  = cpu_ppc_get_tb(tb_env, now, tb_env->tb_offset);
> +
> +    if (tb <= (1 << target_bit)) {
> +        lapse = (1 << target_bit) - tb;
> +    } else {
> +        lapse = period - ((tb - (1 << target_bit)) % period);

We know period is a power of two, so just do "& (period - 1)".

That should let you get rid of the special case for
"tb <= (1 << target_bit)" as well.

-Scott