Re: [PATCH v3 05/10] target/ppc: optimize p9 exception handling routines for lpcr

[Nicholas Piggin]

On Fri Sep 13, 2024 at 2:13 PM AEST, Harsh Prateek Bora wrote:
> Like pending_interrupts, env->spr[SPR_LPCR] is being used at multiple
> places across p9 exception handlers. Pass the value during entry and
> avoid multiple indirect accesses.

Could this be merged with patch 4 to do pending_interrupts and lpcr
at once, to match p7 and p8? Otherwise,

Reviewed-by: Nicholas Piggin <npiggin@gmail.com>

Thanks,
Nick

>
> Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
> ---
>  target/ppc/excp_helper.c | 33 ++++++++++++++++++---------------
>  1 file changed, 18 insertions(+), 15 deletions(-)
>
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 31c1653e2b..c7641898ca 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -1873,13 +1873,14 @@ static int p8_next_unmasked_interrupt(CPUPPCState 
> *env)
>       PPC_INTERRUPT_PIT | PPC_INTERRUPT_THERM)
>  
>  static int p9_interrupt_powersave(CPUPPCState *env,
> -                                  uint32_t pending_interrupts)
> +                                  uint32_t pending_interrupts,
> +                                  target_ulong lpcr)
>  {
>  
>      /* External Exception */
>      if ((pending_interrupts & PPC_INTERRUPT_EXT) &&
> -        (env->spr[SPR_LPCR] & LPCR_EEE)) {
> -        bool heic = !!(env->spr[SPR_LPCR] & LPCR_HEIC);
> +        (lpcr & LPCR_EEE)) {
> +        bool heic = !!(lpcr & LPCR_HEIC);
>          if (!heic || !FIELD_EX64_HV(env->msr) ||
>              FIELD_EX64(env->msr, MSR, PR)) {
>              return PPC_INTERRUPT_EXT;
> @@ -1887,11 +1888,11 @@ static int p9_interrupt_powersave(CPUPPCState *env,
>      }
>      /* Decrementer Exception */
>      if ((pending_interrupts & PPC_INTERRUPT_DECR) &&
> -        (env->spr[SPR_LPCR] & LPCR_DEE)) {
> +        (lpcr & LPCR_DEE)) {
>          return PPC_INTERRUPT_DECR;
>      }
>      /* Machine Check or Hypervisor Maintenance Exception */
> -    if (env->spr[SPR_LPCR] & LPCR_OEE) {
> +    if (lpcr & LPCR_OEE) {
>          if (pending_interrupts & PPC_INTERRUPT_MCK) {
>              return PPC_INTERRUPT_MCK;
>          }
> @@ -1901,17 +1902,17 @@ static int p9_interrupt_powersave(CPUPPCState *env,
>      }
>      /* Privileged Doorbell Exception */
>      if ((pending_interrupts & PPC_INTERRUPT_DOORBELL) &&
> -        (env->spr[SPR_LPCR] & LPCR_PDEE)) {
> +        (lpcr & LPCR_PDEE)) {
>          return PPC_INTERRUPT_DOORBELL;
>      }
>      /* Hypervisor Doorbell Exception */
>      if ((pending_interrupts & PPC_INTERRUPT_HDOORBELL) &&
> -        (env->spr[SPR_LPCR] & LPCR_HDEE)) {
> +        (lpcr & LPCR_HDEE)) {
>          return PPC_INTERRUPT_HDOORBELL;
>      }
>      /* Hypervisor virtualization exception */
>      if ((pending_interrupts & PPC_INTERRUPT_HVIRT) &&
> -        (env->spr[SPR_LPCR] & LPCR_HVEE)) {
> +        (lpcr & LPCR_HVEE)) {
>          return PPC_INTERRUPT_HVIRT;
>      }
>      if (pending_interrupts & PPC_INTERRUPT_RESET) {
> @@ -1921,7 +1922,8 @@ static int p9_interrupt_powersave(CPUPPCState *env,
>  }
>  
>  static int p9_next_unmasked_interrupt(CPUPPCState *env,
> -                                      uint32_t pending_interrupts)
> +                                      uint32_t pending_interrupts,
> +                                      target_ulong lpcr)
>  {
>      CPUState *cs = env_cpu(env);
>  
> @@ -1936,7 +1938,7 @@ static int p9_next_unmasked_interrupt(CPUPPCState *env,
>               * When PSSCR[EC] is set, LPCR[PECE] controls which interrupts 
> can
>               * wakeup the processor
>               */
> -            return p9_interrupt_powersave(env, pending_interrupts);
> +            return p9_interrupt_powersave(env, pending_interrupts, lpcr);
>          } else {
>              /*
>               * When it's clear, any system-caused exception exits 
> power-saving
> @@ -1954,7 +1956,7 @@ static int p9_next_unmasked_interrupt(CPUPPCState *env,
>      /* Hypervisor decrementer exception */
>      if (pending_interrupts & PPC_INTERRUPT_HDECR) {
>          /* LPCR will be clear when not supported so this will work */
> -        bool hdice = !!(env->spr[SPR_LPCR] & LPCR_HDICE);
> +        bool hdice = !!(lpcr & LPCR_HDICE);
>          if ((msr_ee || !FIELD_EX64_HV(env->msr)) && hdice) {
>              /* HDEC clears on delivery */
>              return PPC_INTERRUPT_HDECR;
> @@ -1964,7 +1966,7 @@ static int p9_next_unmasked_interrupt(CPUPPCState *env,
>      /* Hypervisor virtualization interrupt */
>      if (pending_interrupts & PPC_INTERRUPT_HVIRT) {
>          /* LPCR will be clear when not supported so this will work */
> -        bool hvice = !!(env->spr[SPR_LPCR] & LPCR_HVICE);
> +        bool hvice = !!(lpcr & LPCR_HVICE);
>          if ((msr_ee || !FIELD_EX64_HV(env->msr)) && hvice) {
>              return PPC_INTERRUPT_HVIRT;
>          }
> @@ -1972,8 +1974,8 @@ static int p9_next_unmasked_interrupt(CPUPPCState *env,
>  
>      /* External interrupt can ignore MSR:EE under some circumstances */
>      if (pending_interrupts & PPC_INTERRUPT_EXT) {
> -        bool lpes0 = !!(env->spr[SPR_LPCR] & LPCR_LPES0);
> -        bool heic = !!(env->spr[SPR_LPCR] & LPCR_HEIC);
> +        bool lpes0 = !!(lpcr & LPCR_LPES0);
> +        bool heic = !!(lpcr & LPCR_HEIC);
>          /* HEIC blocks delivery to the hypervisor */
>          if ((msr_ee && !(heic && FIELD_EX64_HV(env->msr) &&
>              !FIELD_EX64(env->msr, MSR, PR))) ||
> @@ -2023,7 +2025,8 @@ static int ppc_next_unmasked_interrupt(CPUPPCState *env)
>      case POWERPC_EXCP_POWER9:
>      case POWERPC_EXCP_POWER10:
>      case POWERPC_EXCP_POWER11:
> -        return p9_next_unmasked_interrupt(env, env->pending_interrupts);
> +        return p9_next_unmasked_interrupt(env, env->pending_interrupts,
> +                                       env->spr[SPR_LPCR]);
>      default:
>          break;
>      }