[v2] target/ppc: misc ppc improvements/optimizations

[PATCH v2 5/7] target/ppc: optimize p9 exception handling routines for lpcr

Posted by Harsh Prateek Bora 1 year, 8 months ago

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.

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 704eddac63..d3db81e6ae 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -1843,13 +1843,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;
@@ -1857,11 +1858,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;
         }
@@ -1871,17 +1872,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) {
@@ -1891,7 +1892,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);
 
@@ -1906,7 +1908,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
@@ -1924,7 +1926,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;
@@ -1934,7 +1936,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;
         }
@@ -1942,8 +1944,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))) ||
@@ -1992,7 +1994,8 @@ static int ppc_next_unmasked_interrupt(CPUPPCState *env)
         return p8_next_unmasked_interrupt(env);
     case POWERPC_EXCP_POWER9:
     case POWERPC_EXCP_POWER10:
-        return p9_next_unmasked_interrupt(env, env->pending_interrupts);
+        return p9_next_unmasked_interrupt(env, env->pending_interrupts,
+                                          env->spr[SPR_LPCR]);
     default:
         break;
     }
-- 
2.39.3

Re: [PATCH v2 5/7] target/ppc: optimize p9 exception handling routines for lpcr

Posted by Nicholas Piggin 1 year, 7 months ago

On Thu May 23, 2024 at 3:14 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.

Ditto for this (does it help code, other CPU functions should
be converted similarly).

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 704eddac63..d3db81e6ae 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -1843,13 +1843,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;
> @@ -1857,11 +1858,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;
>          }
> @@ -1871,17 +1872,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) {
> @@ -1891,7 +1892,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);
>  
> @@ -1906,7 +1908,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
> @@ -1924,7 +1926,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;
> @@ -1934,7 +1936,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;
>          }
> @@ -1942,8 +1944,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))) ||
> @@ -1992,7 +1994,8 @@ static int ppc_next_unmasked_interrupt(CPUPPCState *env)
>          return p8_next_unmasked_interrupt(env);
>      case POWERPC_EXCP_POWER9:
>      case POWERPC_EXCP_POWER10:
> -        return p9_next_unmasked_interrupt(env, env->pending_interrupts);
> +        return p9_next_unmasked_interrupt(env, env->pending_interrupts,
> +                                          env->spr[SPR_LPCR]);
>      default:
>          break;
>      }

Re: [PATCH v2 5/7] target/ppc: optimize p9 exception handling routines for lpcr

Posted by Harsh Prateek Bora 1 year, 4 months ago

Hi Nick,

On 7/4/24 13:13, Nicholas Piggin wrote:
> On Thu May 23, 2024 at 3:14 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.
> 
> Ditto for this (does it help code, other CPU functions should
> be converted similarly).

Thanks for your review comments and apologies for a late response.
I have posted v3 with suggested updates and some additional patches.
Patch 7/7 of this series had been picked by Aditya and updated in his
patchset for P11 support so I have excluded that in v3 and just rebased
on top of his v6 series.

v3: 
https://lore.kernel.org/qemu-devel/20240913041337.912876-1-harshpb@linux.ibm.com/T/#t

regards,
Harsh
> 
> 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 704eddac63..d3db81e6ae 100644
>> --- a/target/ppc/excp_helper.c
>> +++ b/target/ppc/excp_helper.c
>> @@ -1843,13 +1843,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;
>> @@ -1857,11 +1858,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;
>>           }
>> @@ -1871,17 +1872,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) {
>> @@ -1891,7 +1892,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);
>>   
>> @@ -1906,7 +1908,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
>> @@ -1924,7 +1926,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;
>> @@ -1934,7 +1936,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;
>>           }
>> @@ -1942,8 +1944,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))) ||
>> @@ -1992,7 +1994,8 @@ static int ppc_next_unmasked_interrupt(CPUPPCState *env)
>>           return p8_next_unmasked_interrupt(env);
>>       case POWERPC_EXCP_POWER9:
>>       case POWERPC_EXCP_POWER10:
>> -        return p9_next_unmasked_interrupt(env, env->pending_interrupts);
>> +        return p9_next_unmasked_interrupt(env, env->pending_interrupts,
>> +                                          env->spr[SPR_LPCR]);
>>       default:
>>           break;
>>       }
>

[PATCH v2 1/7] target/ppc: use locally stored msr and avoid indirect access
[PATCH v2 2/7] target/ppc: optimize hreg_compute_pmu_hflags_value
[PATCH v2 3/7] target/ppc: optimize hreg_compute_pmu_hflags_value
[PATCH v2 4/7] target/ppc: optimize p9 exception handling routines
[PATCH v2 5/7] target/ppc: optimize p9 exception handling routines for lpcr
[PATCH v2 6/7] target/ppc: reduce duplicate code between init_proc_POWER{9, 10}
[PATCH v2 7/7] target/ppc: redue code duplication across Power9/10 init code