1. Patchew
  2. QEMU
  3. [PATCH v3 00/10] misc ppc improvements/optimizations
  4. View patch

[PATCH v3 09/10] target/ppc: simplify var usage in ppc_next_unmasked_interrupt

Harsh Prateek Bora posted 10 patches 2 months, 1 week ago
There is a newer version of this series
[PATCH v3 09/10] target/ppc: simplify var usage in ppc_next_unmasked_interrupt
Posted by Harsh Prateek Bora 2 months, 1 week ago 
As previously done for arch specific handlers, simplify var usage in
ppc_next_unmasked_interrupt by caching the env->pending_interrupts and
env->spr[SPR_LPCR] in local vars and using it later at multiple places.

Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
---
 target/ppc/excp_helper.c | 54 ++++++++++++++++++++--------------------
 1 file changed, 27 insertions(+), 27 deletions(-)

diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index d0e0f609a0..4eeeedff5b 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -2022,31 +2022,31 @@ static int p9_next_unmasked_interrupt(CPUPPCState *env,
 
 static int ppc_next_unmasked_interrupt(CPUPPCState *env)
 {
+    uint32_t pending_interrupts = env->pending_interrupts;
+    target_ulong lpcr = env->spr[SPR_LPCR];
+    bool async_deliver;
+
 #ifdef TARGET_PPC64
     switch (env->excp_model) {
     case POWERPC_EXCP_POWER7:
-        return p7_next_unmasked_interrupt(env, env->pending_interrupts,
-                                          env->spr[SPR_LPCR]);
+        return p7_next_unmasked_interrupt(env, pending_interrupts, lpcr);
     case POWERPC_EXCP_POWER8:
-        return p8_next_unmasked_interrupt(env, env->pending_interrupts,
-                                          env->spr[SPR_LPCR]);
+        return p8_next_unmasked_interrupt(env, pending_interrupts, lpcr);
     case POWERPC_EXCP_POWER9:
     case POWERPC_EXCP_POWER10:
     case POWERPC_EXCP_POWER11:
-        return p9_next_unmasked_interrupt(env, env->pending_interrupts,
-			                  env->spr[SPR_LPCR]);
+        return p9_next_unmasked_interrupt(env, pending_interrupts, lpcr);
     default:
         break;
     }
 #endif
-    bool async_deliver;
 
     /* External reset */
-    if (env->pending_interrupts & PPC_INTERRUPT_RESET) {
+    if (pending_interrupts & PPC_INTERRUPT_RESET) {
         return PPC_INTERRUPT_RESET;
     }
     /* Machine check exception */
-    if (env->pending_interrupts & PPC_INTERRUPT_MCK) {
+    if (pending_interrupts & PPC_INTERRUPT_MCK) {
         return PPC_INTERRUPT_MCK;
     }
 #if 0 /* TODO */
@@ -2065,9 +2065,9 @@ static int ppc_next_unmasked_interrupt(CPUPPCState *env)
     async_deliver = FIELD_EX64(env->msr, MSR, EE) || env->resume_as_sreset;
 
     /* Hypervisor decrementer exception */
-    if (env->pending_interrupts & PPC_INTERRUPT_HDECR) {
+    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 ((async_deliver || !FIELD_EX64_HV(env->msr)) && hdice) {
             /* HDEC clears on delivery */
             return PPC_INTERRUPT_HDECR;
@@ -2075,18 +2075,18 @@ static int ppc_next_unmasked_interrupt(CPUPPCState *env)
     }
 
     /* Hypervisor virtualization interrupt */
-    if (env->pending_interrupts & PPC_INTERRUPT_HVIRT) {
+    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 ((async_deliver || !FIELD_EX64_HV(env->msr)) && hvice) {
             return PPC_INTERRUPT_HVIRT;
         }
     }
 
     /* External interrupt can ignore MSR:EE under some circumstances */
-    if (env->pending_interrupts & PPC_INTERRUPT_EXT) {
-        bool lpes0 = !!(env->spr[SPR_LPCR] & LPCR_LPES0);
-        bool heic = !!(env->spr[SPR_LPCR] & LPCR_HEIC);
+    if (pending_interrupts & PPC_INTERRUPT_EXT) {
+        bool lpes0 = !!(lpcr & LPCR_LPES0);
+        bool heic = !!(lpcr & LPCR_HEIC);
         /* HEIC blocks delivery to the hypervisor */
         if ((async_deliver && !(heic && FIELD_EX64_HV(env->msr) &&
             !FIELD_EX64(env->msr, MSR, PR))) ||
@@ -2096,45 +2096,45 @@ static int ppc_next_unmasked_interrupt(CPUPPCState *env)
     }
     if (FIELD_EX64(env->msr, MSR, CE)) {
         /* External critical interrupt */
-        if (env->pending_interrupts & PPC_INTERRUPT_CEXT) {
+        if (pending_interrupts & PPC_INTERRUPT_CEXT) {
             return PPC_INTERRUPT_CEXT;
         }
     }
     if (async_deliver != 0) {
         /* Watchdog timer on embedded PowerPC */
-        if (env->pending_interrupts & PPC_INTERRUPT_WDT) {
+        if (pending_interrupts & PPC_INTERRUPT_WDT) {
             return PPC_INTERRUPT_WDT;
         }
-        if (env->pending_interrupts & PPC_INTERRUPT_CDOORBELL) {
+        if (pending_interrupts & PPC_INTERRUPT_CDOORBELL) {
             return PPC_INTERRUPT_CDOORBELL;
         }
         /* Fixed interval timer on embedded PowerPC */
-        if (env->pending_interrupts & PPC_INTERRUPT_FIT) {
+        if (pending_interrupts & PPC_INTERRUPT_FIT) {
             return PPC_INTERRUPT_FIT;
         }
         /* Programmable interval timer on embedded PowerPC */
-        if (env->pending_interrupts & PPC_INTERRUPT_PIT) {
+        if (pending_interrupts & PPC_INTERRUPT_PIT) {
             return PPC_INTERRUPT_PIT;
         }
         /* Decrementer exception */
-        if (env->pending_interrupts & PPC_INTERRUPT_DECR) {
+        if (pending_interrupts & PPC_INTERRUPT_DECR) {
             return PPC_INTERRUPT_DECR;
         }
-        if (env->pending_interrupts & PPC_INTERRUPT_DOORBELL) {
+        if (pending_interrupts & PPC_INTERRUPT_DOORBELL) {
             return PPC_INTERRUPT_DOORBELL;
         }
-        if (env->pending_interrupts & PPC_INTERRUPT_HDOORBELL) {
+        if (pending_interrupts & PPC_INTERRUPT_HDOORBELL) {
             return PPC_INTERRUPT_HDOORBELL;
         }
-        if (env->pending_interrupts & PPC_INTERRUPT_PERFM) {
+        if (pending_interrupts & PPC_INTERRUPT_PERFM) {
             return PPC_INTERRUPT_PERFM;
         }
         /* Thermal interrupt */
-        if (env->pending_interrupts & PPC_INTERRUPT_THERM) {
+        if (pending_interrupts & PPC_INTERRUPT_THERM) {
             return PPC_INTERRUPT_THERM;
         }
         /* EBB exception */
-        if (env->pending_interrupts & PPC_INTERRUPT_EBB) {
+        if (pending_interrupts & PPC_INTERRUPT_EBB) {
             /*
              * EBB exception must be taken in problem state and
              * with BESCR_GE set.
-- 
2.45.2
Re: [PATCH v3 09/10] target/ppc: simplify var usage in ppc_next_unmasked_interrupt
Posted by Nicholas Piggin 1 month, 2 weeks ago 
On Fri Sep 13, 2024 at 2:13 PM AEST, Harsh Prateek Bora wrote:
> As previously done for arch specific handlers, simplify var usage in
> ppc_next_unmasked_interrupt by caching the env->pending_interrupts and
> env->spr[SPR_LPCR] in local vars and using it later at multiple places.
>
> Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com>

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

> ---
>  target/ppc/excp_helper.c | 54 ++++++++++++++++++++--------------------
>  1 file changed, 27 insertions(+), 27 deletions(-)
>
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index d0e0f609a0..4eeeedff5b 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -2022,31 +2022,31 @@ static int p9_next_unmasked_interrupt(CPUPPCState *env,
>  
>  static int ppc_next_unmasked_interrupt(CPUPPCState *env)
>  {
> +    uint32_t pending_interrupts = env->pending_interrupts;
> +    target_ulong lpcr = env->spr[SPR_LPCR];
> +    bool async_deliver;
> +
>  #ifdef TARGET_PPC64
>      switch (env->excp_model) {
>      case POWERPC_EXCP_POWER7:
> -        return p7_next_unmasked_interrupt(env, env->pending_interrupts,
> -                                          env->spr[SPR_LPCR]);
> +        return p7_next_unmasked_interrupt(env, pending_interrupts, lpcr);
>      case POWERPC_EXCP_POWER8:
> -        return p8_next_unmasked_interrupt(env, env->pending_interrupts,
> -                                          env->spr[SPR_LPCR]);
> +        return p8_next_unmasked_interrupt(env, pending_interrupts, lpcr);
>      case POWERPC_EXCP_POWER9:
>      case POWERPC_EXCP_POWER10:
>      case POWERPC_EXCP_POWER11:
> -        return p9_next_unmasked_interrupt(env, env->pending_interrupts,
> -			                  env->spr[SPR_LPCR]);
> +        return p9_next_unmasked_interrupt(env, pending_interrupts, lpcr);
>      default:
>          break;
>      }
>  #endif
> -    bool async_deliver;
>  
>      /* External reset */
> -    if (env->pending_interrupts & PPC_INTERRUPT_RESET) {
> +    if (pending_interrupts & PPC_INTERRUPT_RESET) {
>          return PPC_INTERRUPT_RESET;
>      }
>      /* Machine check exception */
> -    if (env->pending_interrupts & PPC_INTERRUPT_MCK) {
> +    if (pending_interrupts & PPC_INTERRUPT_MCK) {
>          return PPC_INTERRUPT_MCK;
>      }
>  #if 0 /* TODO */
> @@ -2065,9 +2065,9 @@ static int ppc_next_unmasked_interrupt(CPUPPCState *env)
>      async_deliver = FIELD_EX64(env->msr, MSR, EE) || env->resume_as_sreset;
>  
>      /* Hypervisor decrementer exception */
> -    if (env->pending_interrupts & PPC_INTERRUPT_HDECR) {
> +    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 ((async_deliver || !FIELD_EX64_HV(env->msr)) && hdice) {
>              /* HDEC clears on delivery */
>              return PPC_INTERRUPT_HDECR;
> @@ -2075,18 +2075,18 @@ static int ppc_next_unmasked_interrupt(CPUPPCState *env)
>      }
>  
>      /* Hypervisor virtualization interrupt */
> -    if (env->pending_interrupts & PPC_INTERRUPT_HVIRT) {
> +    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 ((async_deliver || !FIELD_EX64_HV(env->msr)) && hvice) {
>              return PPC_INTERRUPT_HVIRT;
>          }
>      }
>  
>      /* External interrupt can ignore MSR:EE under some circumstances */
> -    if (env->pending_interrupts & PPC_INTERRUPT_EXT) {
> -        bool lpes0 = !!(env->spr[SPR_LPCR] & LPCR_LPES0);
> -        bool heic = !!(env->spr[SPR_LPCR] & LPCR_HEIC);
> +    if (pending_interrupts & PPC_INTERRUPT_EXT) {
> +        bool lpes0 = !!(lpcr & LPCR_LPES0);
> +        bool heic = !!(lpcr & LPCR_HEIC);
>          /* HEIC blocks delivery to the hypervisor */
>          if ((async_deliver && !(heic && FIELD_EX64_HV(env->msr) &&
>              !FIELD_EX64(env->msr, MSR, PR))) ||
> @@ -2096,45 +2096,45 @@ static int ppc_next_unmasked_interrupt(CPUPPCState *env)
>      }
>      if (FIELD_EX64(env->msr, MSR, CE)) {
>          /* External critical interrupt */
> -        if (env->pending_interrupts & PPC_INTERRUPT_CEXT) {
> +        if (pending_interrupts & PPC_INTERRUPT_CEXT) {
>              return PPC_INTERRUPT_CEXT;
>          }
>      }
>      if (async_deliver != 0) {
>          /* Watchdog timer on embedded PowerPC */
> -        if (env->pending_interrupts & PPC_INTERRUPT_WDT) {
> +        if (pending_interrupts & PPC_INTERRUPT_WDT) {
>              return PPC_INTERRUPT_WDT;
>          }
> -        if (env->pending_interrupts & PPC_INTERRUPT_CDOORBELL) {
> +        if (pending_interrupts & PPC_INTERRUPT_CDOORBELL) {
>              return PPC_INTERRUPT_CDOORBELL;
>          }
>          /* Fixed interval timer on embedded PowerPC */
> -        if (env->pending_interrupts & PPC_INTERRUPT_FIT) {
> +        if (pending_interrupts & PPC_INTERRUPT_FIT) {
>              return PPC_INTERRUPT_FIT;
>          }
>          /* Programmable interval timer on embedded PowerPC */
> -        if (env->pending_interrupts & PPC_INTERRUPT_PIT) {
> +        if (pending_interrupts & PPC_INTERRUPT_PIT) {
>              return PPC_INTERRUPT_PIT;
>          }
>          /* Decrementer exception */
> -        if (env->pending_interrupts & PPC_INTERRUPT_DECR) {
> +        if (pending_interrupts & PPC_INTERRUPT_DECR) {
>              return PPC_INTERRUPT_DECR;
>          }
> -        if (env->pending_interrupts & PPC_INTERRUPT_DOORBELL) {
> +        if (pending_interrupts & PPC_INTERRUPT_DOORBELL) {
>              return PPC_INTERRUPT_DOORBELL;
>          }
> -        if (env->pending_interrupts & PPC_INTERRUPT_HDOORBELL) {
> +        if (pending_interrupts & PPC_INTERRUPT_HDOORBELL) {
>              return PPC_INTERRUPT_HDOORBELL;
>          }
> -        if (env->pending_interrupts & PPC_INTERRUPT_PERFM) {
> +        if (pending_interrupts & PPC_INTERRUPT_PERFM) {
>              return PPC_INTERRUPT_PERFM;
>          }
>          /* Thermal interrupt */
> -        if (env->pending_interrupts & PPC_INTERRUPT_THERM) {
> +        if (pending_interrupts & PPC_INTERRUPT_THERM) {
>              return PPC_INTERRUPT_THERM;
>          }
>          /* EBB exception */
> -        if (env->pending_interrupts & PPC_INTERRUPT_EBB) {
> +        if (pending_interrupts & PPC_INTERRUPT_EBB) {
>              /*
>               * EBB exception must be taken in problem state and
>               * with BESCR_GE set.
Re: [PATCH v3 09/10] target/ppc: simplify var usage in ppc_next_unmasked_interrupt
Posted by BALATON Zoltan 2 months, 1 week ago 
On Fri, 13 Sep 2024, Harsh Prateek Bora wrote:
> As previously done for arch specific handlers, simplify var usage in
> ppc_next_unmasked_interrupt by caching the env->pending_interrupts and
> env->spr[SPR_LPCR] in local vars and using it later at multiple places.
>
> Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
> ---
> target/ppc/excp_helper.c | 54 ++++++++++++++++++++--------------------
> 1 file changed, 27 insertions(+), 27 deletions(-)
>
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index d0e0f609a0..4eeeedff5b 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -2022,31 +2022,31 @@ static int p9_next_unmasked_interrupt(CPUPPCState *env,
>
> static int ppc_next_unmasked_interrupt(CPUPPCState *env)
> {
> +    uint32_t pending_interrupts = env->pending_interrupts;
> +    target_ulong lpcr = env->spr[SPR_LPCR];
> +    bool async_deliver;

Maybe easier to review if split into one patch for each variable added so 
it's easier to see what's replaced and that nothing is missed.

Regards,
BALATON Zoltan

> +
> #ifdef TARGET_PPC64
>     switch (env->excp_model) {
>     case POWERPC_EXCP_POWER7:
> -        return p7_next_unmasked_interrupt(env, env->pending_interrupts,
> -                                          env->spr[SPR_LPCR]);
> +        return p7_next_unmasked_interrupt(env, pending_interrupts, lpcr);
>     case POWERPC_EXCP_POWER8:
> -        return p8_next_unmasked_interrupt(env, env->pending_interrupts,
> -                                          env->spr[SPR_LPCR]);
> +        return p8_next_unmasked_interrupt(env, pending_interrupts, lpcr);
>     case POWERPC_EXCP_POWER9:
>     case POWERPC_EXCP_POWER10:
>     case POWERPC_EXCP_POWER11:
> -        return p9_next_unmasked_interrupt(env, env->pending_interrupts,
> -			                  env->spr[SPR_LPCR]);
> +        return p9_next_unmasked_interrupt(env, pending_interrupts, lpcr);
>     default:
>         break;
>     }
> #endif
> -    bool async_deliver;
>
>     /* External reset */
> -    if (env->pending_interrupts & PPC_INTERRUPT_RESET) {
> +    if (pending_interrupts & PPC_INTERRUPT_RESET) {
>         return PPC_INTERRUPT_RESET;
>     }
>     /* Machine check exception */
> -    if (env->pending_interrupts & PPC_INTERRUPT_MCK) {
> +    if (pending_interrupts & PPC_INTERRUPT_MCK) {
>         return PPC_INTERRUPT_MCK;
>     }
> #if 0 /* TODO */
> @@ -2065,9 +2065,9 @@ static int ppc_next_unmasked_interrupt(CPUPPCState *env)
>     async_deliver = FIELD_EX64(env->msr, MSR, EE) || env->resume_as_sreset;
>
>     /* Hypervisor decrementer exception */
> -    if (env->pending_interrupts & PPC_INTERRUPT_HDECR) {
> +    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 ((async_deliver || !FIELD_EX64_HV(env->msr)) && hdice) {
>             /* HDEC clears on delivery */
>             return PPC_INTERRUPT_HDECR;
> @@ -2075,18 +2075,18 @@ static int ppc_next_unmasked_interrupt(CPUPPCState *env)
>     }
>
>     /* Hypervisor virtualization interrupt */
> -    if (env->pending_interrupts & PPC_INTERRUPT_HVIRT) {
> +    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 ((async_deliver || !FIELD_EX64_HV(env->msr)) && hvice) {
>             return PPC_INTERRUPT_HVIRT;
>         }
>     }
>
>     /* External interrupt can ignore MSR:EE under some circumstances */
> -    if (env->pending_interrupts & PPC_INTERRUPT_EXT) {
> -        bool lpes0 = !!(env->spr[SPR_LPCR] & LPCR_LPES0);
> -        bool heic = !!(env->spr[SPR_LPCR] & LPCR_HEIC);
> +    if (pending_interrupts & PPC_INTERRUPT_EXT) {
> +        bool lpes0 = !!(lpcr & LPCR_LPES0);
> +        bool heic = !!(lpcr & LPCR_HEIC);
>         /* HEIC blocks delivery to the hypervisor */
>         if ((async_deliver && !(heic && FIELD_EX64_HV(env->msr) &&
>             !FIELD_EX64(env->msr, MSR, PR))) ||
> @@ -2096,45 +2096,45 @@ static int ppc_next_unmasked_interrupt(CPUPPCState *env)
>     }
>     if (FIELD_EX64(env->msr, MSR, CE)) {
>         /* External critical interrupt */
> -        if (env->pending_interrupts & PPC_INTERRUPT_CEXT) {
> +        if (pending_interrupts & PPC_INTERRUPT_CEXT) {
>             return PPC_INTERRUPT_CEXT;
>         }
>     }
>     if (async_deliver != 0) {
>         /* Watchdog timer on embedded PowerPC */
> -        if (env->pending_interrupts & PPC_INTERRUPT_WDT) {
> +        if (pending_interrupts & PPC_INTERRUPT_WDT) {
>             return PPC_INTERRUPT_WDT;
>         }
> -        if (env->pending_interrupts & PPC_INTERRUPT_CDOORBELL) {
> +        if (pending_interrupts & PPC_INTERRUPT_CDOORBELL) {
>             return PPC_INTERRUPT_CDOORBELL;
>         }
>         /* Fixed interval timer on embedded PowerPC */
> -        if (env->pending_interrupts & PPC_INTERRUPT_FIT) {
> +        if (pending_interrupts & PPC_INTERRUPT_FIT) {
>             return PPC_INTERRUPT_FIT;
>         }
>         /* Programmable interval timer on embedded PowerPC */
> -        if (env->pending_interrupts & PPC_INTERRUPT_PIT) {
> +        if (pending_interrupts & PPC_INTERRUPT_PIT) {
>             return PPC_INTERRUPT_PIT;
>         }
>         /* Decrementer exception */
> -        if (env->pending_interrupts & PPC_INTERRUPT_DECR) {
> +        if (pending_interrupts & PPC_INTERRUPT_DECR) {
>             return PPC_INTERRUPT_DECR;
>         }
> -        if (env->pending_interrupts & PPC_INTERRUPT_DOORBELL) {
> +        if (pending_interrupts & PPC_INTERRUPT_DOORBELL) {
>             return PPC_INTERRUPT_DOORBELL;
>         }
> -        if (env->pending_interrupts & PPC_INTERRUPT_HDOORBELL) {
> +        if (pending_interrupts & PPC_INTERRUPT_HDOORBELL) {
>             return PPC_INTERRUPT_HDOORBELL;
>         }
> -        if (env->pending_interrupts & PPC_INTERRUPT_PERFM) {
> +        if (pending_interrupts & PPC_INTERRUPT_PERFM) {
>             return PPC_INTERRUPT_PERFM;
>         }
>         /* Thermal interrupt */
> -        if (env->pending_interrupts & PPC_INTERRUPT_THERM) {
> +        if (pending_interrupts & PPC_INTERRUPT_THERM) {
>             return PPC_INTERRUPT_THERM;
>         }
>         /* EBB exception */
> -        if (env->pending_interrupts & PPC_INTERRUPT_EBB) {
> +        if (pending_interrupts & PPC_INTERRUPT_EBB) {
>             /*
>              * EBB exception must be taken in problem state and
>              * with BESCR_GE set.
>
Re: [PATCH v3 09/10] target/ppc: simplify var usage in ppc_next_unmasked_interrupt
Posted by Nicholas Piggin 1 month, 2 weeks ago 
On Fri Sep 13, 2024 at 10:50 PM AEST, BALATON Zoltan wrote:
> On Fri, 13 Sep 2024, Harsh Prateek Bora wrote:
> > As previously done for arch specific handlers, simplify var usage in
> > ppc_next_unmasked_interrupt by caching the env->pending_interrupts and
> > env->spr[SPR_LPCR] in local vars and using it later at multiple places.
> >
> > Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
> > ---
> > target/ppc/excp_helper.c | 54 ++++++++++++++++++++--------------------
> > 1 file changed, 27 insertions(+), 27 deletions(-)
> >
> > diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> > index d0e0f609a0..4eeeedff5b 100644
> > --- a/target/ppc/excp_helper.c
> > +++ b/target/ppc/excp_helper.c
> > @@ -2022,31 +2022,31 @@ static int p9_next_unmasked_interrupt(CPUPPCState *env,
> >
> > static int ppc_next_unmasked_interrupt(CPUPPCState *env)
> > {
> > +    uint32_t pending_interrupts = env->pending_interrupts;
> > +    target_ulong lpcr = env->spr[SPR_LPCR];
> > +    bool async_deliver;
>
> Maybe easier to review if split into one patch for each variable added so 
> it's easier to see what's replaced and that nothing is missed.

I'm happy to leave squashed since it's pretty simple search/replace
with no logic change, and touching the same lines.

Thanks,
Nick

>
> Regards,
> BALATON Zoltan
>
> > +
> > #ifdef TARGET_PPC64
> >     switch (env->excp_model) {
> >     case POWERPC_EXCP_POWER7:
> > -        return p7_next_unmasked_interrupt(env, env->pending_interrupts,
> > -                                          env->spr[SPR_LPCR]);
> > +        return p7_next_unmasked_interrupt(env, pending_interrupts, lpcr);
> >     case POWERPC_EXCP_POWER8:
> > -        return p8_next_unmasked_interrupt(env, env->pending_interrupts,
> > -                                          env->spr[SPR_LPCR]);
> > +        return p8_next_unmasked_interrupt(env, pending_interrupts, lpcr);
> >     case POWERPC_EXCP_POWER9:
> >     case POWERPC_EXCP_POWER10:
> >     case POWERPC_EXCP_POWER11:
> > -        return p9_next_unmasked_interrupt(env, env->pending_interrupts,
> > -			                  env->spr[SPR_LPCR]);
> > +        return p9_next_unmasked_interrupt(env, pending_interrupts, lpcr);
> >     default:
> >         break;
> >     }
> > #endif
> > -    bool async_deliver;
> >
> >     /* External reset */
> > -    if (env->pending_interrupts & PPC_INTERRUPT_RESET) {
> > +    if (pending_interrupts & PPC_INTERRUPT_RESET) {
> >         return PPC_INTERRUPT_RESET;
> >     }
> >     /* Machine check exception */
> > -    if (env->pending_interrupts & PPC_INTERRUPT_MCK) {
> > +    if (pending_interrupts & PPC_INTERRUPT_MCK) {
> >         return PPC_INTERRUPT_MCK;
> >     }
> > #if 0 /* TODO */
> > @@ -2065,9 +2065,9 @@ static int ppc_next_unmasked_interrupt(CPUPPCState *env)
> >     async_deliver = FIELD_EX64(env->msr, MSR, EE) || env->resume_as_sreset;
> >
> >     /* Hypervisor decrementer exception */
> > -    if (env->pending_interrupts & PPC_INTERRUPT_HDECR) {
> > +    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 ((async_deliver || !FIELD_EX64_HV(env->msr)) && hdice) {
> >             /* HDEC clears on delivery */
> >             return PPC_INTERRUPT_HDECR;
> > @@ -2075,18 +2075,18 @@ static int ppc_next_unmasked_interrupt(CPUPPCState *env)
> >     }
> >
> >     /* Hypervisor virtualization interrupt */
> > -    if (env->pending_interrupts & PPC_INTERRUPT_HVIRT) {
> > +    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 ((async_deliver || !FIELD_EX64_HV(env->msr)) && hvice) {
> >             return PPC_INTERRUPT_HVIRT;
> >         }
> >     }
> >
> >     /* External interrupt can ignore MSR:EE under some circumstances */
> > -    if (env->pending_interrupts & PPC_INTERRUPT_EXT) {
> > -        bool lpes0 = !!(env->spr[SPR_LPCR] & LPCR_LPES0);
> > -        bool heic = !!(env->spr[SPR_LPCR] & LPCR_HEIC);
> > +    if (pending_interrupts & PPC_INTERRUPT_EXT) {
> > +        bool lpes0 = !!(lpcr & LPCR_LPES0);
> > +        bool heic = !!(lpcr & LPCR_HEIC);
> >         /* HEIC blocks delivery to the hypervisor */
> >         if ((async_deliver && !(heic && FIELD_EX64_HV(env->msr) &&
> >             !FIELD_EX64(env->msr, MSR, PR))) ||
> > @@ -2096,45 +2096,45 @@ static int ppc_next_unmasked_interrupt(CPUPPCState *env)
> >     }
> >     if (FIELD_EX64(env->msr, MSR, CE)) {
> >         /* External critical interrupt */
> > -        if (env->pending_interrupts & PPC_INTERRUPT_CEXT) {
> > +        if (pending_interrupts & PPC_INTERRUPT_CEXT) {
> >             return PPC_INTERRUPT_CEXT;
> >         }
> >     }
> >     if (async_deliver != 0) {
> >         /* Watchdog timer on embedded PowerPC */
> > -        if (env->pending_interrupts & PPC_INTERRUPT_WDT) {
> > +        if (pending_interrupts & PPC_INTERRUPT_WDT) {
> >             return PPC_INTERRUPT_WDT;
> >         }
> > -        if (env->pending_interrupts & PPC_INTERRUPT_CDOORBELL) {
> > +        if (pending_interrupts & PPC_INTERRUPT_CDOORBELL) {
> >             return PPC_INTERRUPT_CDOORBELL;
> >         }
> >         /* Fixed interval timer on embedded PowerPC */
> > -        if (env->pending_interrupts & PPC_INTERRUPT_FIT) {
> > +        if (pending_interrupts & PPC_INTERRUPT_FIT) {
> >             return PPC_INTERRUPT_FIT;
> >         }
> >         /* Programmable interval timer on embedded PowerPC */
> > -        if (env->pending_interrupts & PPC_INTERRUPT_PIT) {
> > +        if (pending_interrupts & PPC_INTERRUPT_PIT) {
> >             return PPC_INTERRUPT_PIT;
> >         }
> >         /* Decrementer exception */
> > -        if (env->pending_interrupts & PPC_INTERRUPT_DECR) {
> > +        if (pending_interrupts & PPC_INTERRUPT_DECR) {
> >             return PPC_INTERRUPT_DECR;
> >         }
> > -        if (env->pending_interrupts & PPC_INTERRUPT_DOORBELL) {
> > +        if (pending_interrupts & PPC_INTERRUPT_DOORBELL) {
> >             return PPC_INTERRUPT_DOORBELL;
> >         }
> > -        if (env->pending_interrupts & PPC_INTERRUPT_HDOORBELL) {
> > +        if (pending_interrupts & PPC_INTERRUPT_HDOORBELL) {
> >             return PPC_INTERRUPT_HDOORBELL;
> >         }
> > -        if (env->pending_interrupts & PPC_INTERRUPT_PERFM) {
> > +        if (pending_interrupts & PPC_INTERRUPT_PERFM) {
> >             return PPC_INTERRUPT_PERFM;
> >         }
> >         /* Thermal interrupt */
> > -        if (env->pending_interrupts & PPC_INTERRUPT_THERM) {
> > +        if (pending_interrupts & PPC_INTERRUPT_THERM) {
> >             return PPC_INTERRUPT_THERM;
> >         }
> >         /* EBB exception */
> > -        if (env->pending_interrupts & PPC_INTERRUPT_EBB) {
> > +        if (pending_interrupts & PPC_INTERRUPT_EBB) {
> >             /*
> >              * EBB exception must be taken in problem state and
> >              * with BESCR_GE set.
> >
Re: [PATCH v3 09/10] target/ppc: simplify var usage in ppc_next_unmasked_interrupt
Posted by Harsh Prateek Bora 2 months, 1 week ago 

On 9/13/24 18:20, BALATON Zoltan wrote:
> On Fri, 13 Sep 2024, Harsh Prateek Bora wrote:
>> As previously done for arch specific handlers, simplify var usage in
>> ppc_next_unmasked_interrupt by caching the env->pending_interrupts and
>> env->spr[SPR_LPCR] in local vars and using it later at multiple places.
>>
>> Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
>> ---
>> target/ppc/excp_helper.c | 54 ++++++++++++++++++++--------------------
>> 1 file changed, 27 insertions(+), 27 deletions(-)
>>
>> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
>> index d0e0f609a0..4eeeedff5b 100644
>> --- a/target/ppc/excp_helper.c
>> +++ b/target/ppc/excp_helper.c
>> @@ -2022,31 +2022,31 @@ static int 
>> p9_next_unmasked_interrupt(CPUPPCState *env,
>>
>> static int ppc_next_unmasked_interrupt(CPUPPCState *env)
>> {
>> +    uint32_t pending_interrupts = env->pending_interrupts;
>> +    target_ulong lpcr = env->spr[SPR_LPCR];
>> +    bool async_deliver;
> 
> Maybe easier to review if split into one patch for each variable added 
> so it's easier to see what's replaced and that nothing is missed.
> 

Thanks for the reviews. I shall split as suggested in v4.

regards,
Harsh

> Regards,
> BALATON Zoltan
> 
>> +
>> #ifdef TARGET_PPC64
>>     switch (env->excp_model) {
>>     case POWERPC_EXCP_POWER7:
>> -        return p7_next_unmasked_interrupt(env, env->pending_interrupts,
>> -                                          env->spr[SPR_LPCR]);
>> +        return p7_next_unmasked_interrupt(env, pending_interrupts, 
>> lpcr);
>>     case POWERPC_EXCP_POWER8:
>> -        return p8_next_unmasked_interrupt(env, env->pending_interrupts,
>> -                                          env->spr[SPR_LPCR]);
>> +        return p8_next_unmasked_interrupt(env, pending_interrupts, 
>> lpcr);
>>     case POWERPC_EXCP_POWER9:
>>     case POWERPC_EXCP_POWER10:
>>     case POWERPC_EXCP_POWER11:
>> -        return p9_next_unmasked_interrupt(env, env->pending_interrupts,
>> -                              env->spr[SPR_LPCR]);
>> +        return p9_next_unmasked_interrupt(env, pending_interrupts, 
>> lpcr);
>>     default:
>>         break;
>>     }
>> #endif
>> -    bool async_deliver;
>>
>>     /* External reset */
>> -    if (env->pending_interrupts & PPC_INTERRUPT_RESET) {
>> +    if (pending_interrupts & PPC_INTERRUPT_RESET) {
>>         return PPC_INTERRUPT_RESET;
>>     }
>>     /* Machine check exception */
>> -    if (env->pending_interrupts & PPC_INTERRUPT_MCK) {
>> +    if (pending_interrupts & PPC_INTERRUPT_MCK) {
>>         return PPC_INTERRUPT_MCK;
>>     }
>> #if 0 /* TODO */
>> @@ -2065,9 +2065,9 @@ static int 
>> ppc_next_unmasked_interrupt(CPUPPCState *env)
>>     async_deliver = FIELD_EX64(env->msr, MSR, EE) || 
>> env->resume_as_sreset;
>>
>>     /* Hypervisor decrementer exception */
>> -    if (env->pending_interrupts & PPC_INTERRUPT_HDECR) {
>> +    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 ((async_deliver || !FIELD_EX64_HV(env->msr)) && hdice) {
>>             /* HDEC clears on delivery */
>>             return PPC_INTERRUPT_HDECR;
>> @@ -2075,18 +2075,18 @@ static int 
>> ppc_next_unmasked_interrupt(CPUPPCState *env)
>>     }
>>
>>     /* Hypervisor virtualization interrupt */
>> -    if (env->pending_interrupts & PPC_INTERRUPT_HVIRT) {
>> +    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 ((async_deliver || !FIELD_EX64_HV(env->msr)) && hvice) {
>>             return PPC_INTERRUPT_HVIRT;
>>         }
>>     }
>>
>>     /* External interrupt can ignore MSR:EE under some circumstances */
>> -    if (env->pending_interrupts & PPC_INTERRUPT_EXT) {
>> -        bool lpes0 = !!(env->spr[SPR_LPCR] & LPCR_LPES0);
>> -        bool heic = !!(env->spr[SPR_LPCR] & LPCR_HEIC);
>> +    if (pending_interrupts & PPC_INTERRUPT_EXT) {
>> +        bool lpes0 = !!(lpcr & LPCR_LPES0);
>> +        bool heic = !!(lpcr & LPCR_HEIC);
>>         /* HEIC blocks delivery to the hypervisor */
>>         if ((async_deliver && !(heic && FIELD_EX64_HV(env->msr) &&
>>             !FIELD_EX64(env->msr, MSR, PR))) ||
>> @@ -2096,45 +2096,45 @@ static int 
>> ppc_next_unmasked_interrupt(CPUPPCState *env)
>>     }
>>     if (FIELD_EX64(env->msr, MSR, CE)) {
>>         /* External critical interrupt */
>> -        if (env->pending_interrupts & PPC_INTERRUPT_CEXT) {
>> +        if (pending_interrupts & PPC_INTERRUPT_CEXT) {
>>             return PPC_INTERRUPT_CEXT;
>>         }
>>     }
>>     if (async_deliver != 0) {
>>         /* Watchdog timer on embedded PowerPC */
>> -        if (env->pending_interrupts & PPC_INTERRUPT_WDT) {
>> +        if (pending_interrupts & PPC_INTERRUPT_WDT) {
>>             return PPC_INTERRUPT_WDT;
>>         }
>> -        if (env->pending_interrupts & PPC_INTERRUPT_CDOORBELL) {
>> +        if (pending_interrupts & PPC_INTERRUPT_CDOORBELL) {
>>             return PPC_INTERRUPT_CDOORBELL;
>>         }
>>         /* Fixed interval timer on embedded PowerPC */
>> -        if (env->pending_interrupts & PPC_INTERRUPT_FIT) {
>> +        if (pending_interrupts & PPC_INTERRUPT_FIT) {
>>             return PPC_INTERRUPT_FIT;
>>         }
>>         /* Programmable interval timer on embedded PowerPC */
>> -        if (env->pending_interrupts & PPC_INTERRUPT_PIT) {
>> +        if (pending_interrupts & PPC_INTERRUPT_PIT) {
>>             return PPC_INTERRUPT_PIT;
>>         }
>>         /* Decrementer exception */
>> -        if (env->pending_interrupts & PPC_INTERRUPT_DECR) {
>> +        if (pending_interrupts & PPC_INTERRUPT_DECR) {
>>             return PPC_INTERRUPT_DECR;
>>         }
>> -        if (env->pending_interrupts & PPC_INTERRUPT_DOORBELL) {
>> +        if (pending_interrupts & PPC_INTERRUPT_DOORBELL) {
>>             return PPC_INTERRUPT_DOORBELL;
>>         }
>> -        if (env->pending_interrupts & PPC_INTERRUPT_HDOORBELL) {
>> +        if (pending_interrupts & PPC_INTERRUPT_HDOORBELL) {
>>             return PPC_INTERRUPT_HDOORBELL;
>>         }
>> -        if (env->pending_interrupts & PPC_INTERRUPT_PERFM) {
>> +        if (pending_interrupts & PPC_INTERRUPT_PERFM) {
>>             return PPC_INTERRUPT_PERFM;
>>         }
>>         /* Thermal interrupt */
>> -        if (env->pending_interrupts & PPC_INTERRUPT_THERM) {
>> +        if (pending_interrupts & PPC_INTERRUPT_THERM) {
>>             return PPC_INTERRUPT_THERM;
>>         }
>>         /* EBB exception */
>> -        if (env->pending_interrupts & PPC_INTERRUPT_EBB) {
>> +        if (pending_interrupts & PPC_INTERRUPT_EBB) {
>>             /*
>>              * EBB exception must be taken in problem state and
>>              * with BESCR_GE set.
>>

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.