Re: [RFC PATCH v2 11/29] target/ppc: add power-saving interrupt masking logic to p9_next_unmasked_interrupt

[Fabiano Rosas]

Matheus Ferst <matheus.ferst@eldorado.org.br> writes:

> Export p9_interrupt_powersave and use it in p9_next_unmasked_interrupt.
>
> Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
> ---
> Temporarily putting the prototype in internal.h for lack of a better place,
> we will un-export p9_interrupt_powersave in future patches.
> ---
>  target/ppc/cpu_init.c    |  2 +-
>  target/ppc/excp_helper.c | 46 ++++++++++++++++++++++++++++------------
>  target/ppc/internal.h    |  4 ++++
>  3 files changed, 38 insertions(+), 14 deletions(-)
>
> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> index 1f8f6c6ef2..7889158c52 100644
> --- a/target/ppc/cpu_init.c
> +++ b/target/ppc/cpu_init.c
> @@ -6351,7 +6351,7 @@ static bool ppc_pvr_match_power9(PowerPCCPUClass *pcc, 
> uint32_t pvr, bool best)
>      return false;
>  }
>  
> -static int p9_interrupt_powersave(CPUPPCState *env)
> +int p9_interrupt_powersave(CPUPPCState *env)
>  {
>      /* External Exception */
>      if ((env->pending_interrupts & PPC_INTERRUPT_EXT) &&
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 67e73f30ab..5a0d2c11a2 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -1686,28 +1686,39 @@ void ppc_cpu_do_interrupt(CPUState *cs)
>  
>  static int p9_next_unmasked_interrupt(CPUPPCState *env)
>  {
> -    bool async_deliver;
> +    PowerPCCPU *cpu = env_archcpu(env);
> +    CPUState *cs = CPU(cpu);
> +    /* Ignore MSR[EE] when coming out of some power management states */
> +    bool msr_ee = FIELD_EX64(env->msr, MSR, EE) || env->resume_as_sreset;
>  
>      assert((env->pending_interrupts & P9_UNUSED_INTERRUPTS) == 0);
>  
> +    if (cs->halted) {
> +        if (env->spr[SPR_PSSCR] & PSSCR_EC) {
> +            /*
> +             * When PSSCR[EC] is set, LPCR[PECE] controls which interrupts 
> can
> +             * wakeup the processor
> +             */
> +            return p9_interrupt_powersave(env);
> +        } else {
> +            /*
> +             * When it's clear, any system-caused exception exits 
> power-saving
> +             * mode, even the ones that gate on MSR[EE].
> +             */
> +            msr_ee = true;
> +        }
> +    }
> +
>      /* Machine check exception */
>      if (env->pending_interrupts & PPC_INTERRUPT_MCK) {
>          return PPC_INTERRUPT_MCK;
>      }
>  
> -    /*
> -     * For interrupts that gate on MSR:EE, we need to do something a
> -     * bit more subtle, as we need to let them through even when EE is
> -     * clear when coming out of some power management states (in order
> -     * for them to become a 0x100).
> -     */
> -    async_deliver = FIELD_EX64(env->msr, MSR, EE) || env->resume_as_sreset;
> -

You could simplify the code below if you bail early here when !msr_ee.

>      /* Hypervisor decrementer exception */
>      if (env->pending_interrupts & PPC_INTERRUPT_HDECR) {
>          /* LPCR will be clear when not supported so this will work */
>          bool hdice = !!(env->spr[SPR_LPCR] & LPCR_HDICE);
> -        if ((async_deliver || !FIELD_EX64_HV(env->msr)) && hdice) {
> +        if ((msr_ee || !FIELD_EX64_HV(env->msr)) && hdice) {
>              /* HDEC clears on delivery */
>              return PPC_INTERRUPT_HDECR;
>          }
> @@ -1717,7 +1728,7 @@ static int p9_next_unmasked_interrupt(CPUPPCState *env)
>      if (env->pending_interrupts & PPC_INTERRUPT_HVIRT) {
>          /* LPCR will be clear when not supported so this will work */
>          bool hvice = !!(env->spr[SPR_LPCR] & LPCR_HVICE);
> -        if ((async_deliver || !FIELD_EX64_HV(env->msr)) && hvice) {
> +        if ((msr_ee || !FIELD_EX64_HV(env->msr)) && hvice) {
>              return PPC_INTERRUPT_HVIRT;
>          }
>      }
> @@ -1727,13 +1738,13 @@ static int p9_next_unmasked_interrupt(CPUPPCState 
> *env)
>          bool lpes0 = !!(env->spr[SPR_LPCR] & LPCR_LPES0);
>          bool heic = !!(env->spr[SPR_LPCR] & LPCR_HEIC);
>          /* HEIC blocks delivery to the hypervisor */
> -        if ((async_deliver && !(heic && FIELD_EX64_HV(env->msr) &&
> +        if ((msr_ee && !(heic && FIELD_EX64_HV(env->msr) &&
>              !FIELD_EX64(env->msr, MSR, PR))) ||
>              (env->has_hv_mode && !FIELD_EX64_HV(env->msr) && !lpes0)) {
>              return PPC_INTERRUPT_EXT;
>          }
>      }
> -    if (async_deliver != 0) {
> +    if (msr_ee != 0) {
>          /* Decrementer exception */
>          if (env->pending_interrupts & PPC_INTERRUPT_DECR) {
>              return PPC_INTERRUPT_DECR;
> @@ -1895,6 +1906,15 @@ static void p9_deliver_interrupt(CPUPPCState *env, int 
> interrupt)
>      PowerPCCPU *cpu = env_archcpu(env);
>      CPUState *cs = env_cpu(env);
>  
> +    if (cs->halted && !(env->spr[SPR_PSSCR] & PSSCR_EC) &&
> +        !FIELD_EX64(env->msr, MSR, EE)) {
> +        /*
> +         * A pending interrupt took us out of power-saving, but MSR[EE] says
> +         * that we should return to NIP+4 instead of delivering it.
> +         */
> +        return;

How will the NIP be advanced in this case?

> +    }
> +
>      switch (interrupt) {
>      case PPC_INTERRUPT_MCK: /* Machine check exception */
>          env->pending_interrupts &= ~PPC_INTERRUPT_MCK;
> diff --git a/target/ppc/internal.h b/target/ppc/internal.h
> index 337a362205..41e79adfdb 100644
> --- a/target/ppc/internal.h
> +++ b/target/ppc/internal.h
> @@ -306,4 +306,8 @@ static inline int ger_pack_masks(int pmsk, int ymsk, int 
> xmsk)
>      return msk;
>  }
>  
> +#if defined(TARGET_PPC64)
> +int p9_interrupt_powersave(CPUPPCState *env);
> +#endif
> +
>  #endif /* PPC_INTERNAL_H */