[4/6] target/ppc: optimize p9 exception handling routines

Currently, p9 exception handling has multiple if-condition checks where
it does an indirect access to pending_interrupts via env. Cache the
value during entry and reuse later to avoid multiple indirect accesses.

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

On Mon, 20 May 2024, Harsh Prateek Bora wrote:
> Currently, p9 exception handling has multiple if-condition checks where
> it does an indirect access to pending_interrupts via env. Cache the
> value during entry and reuse later to avoid multiple indirect accesses.
>
> Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
> ---
> target/ppc/excp_helper.c | 39 +++++++++++++++++++++------------------
> 1 file changed, 21 insertions(+), 18 deletions(-)
>
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 0712098cf7..4f158196bb 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -1844,8 +1844,10 @@ static int p8_next_unmasked_interrupt(CPUPPCState *env)
>
> static int p9_interrupt_powersave(CPUPPCState *env)
> {
> +    uint32_t pending_interrupts = env->pending_interrupts;
> +

LPCR also seems to be used a lot and other similar *_powersave functions 
only use pending_interrupts and LPCR so maybe you could change these 
functions to take the pending_interrupts and lpcr as parameters and pass 
them from the caller that already have it so even this dereference could 
be saved?

Regards,
BALATON Zoltan

>     /* External Exception */
> -    if ((env->pending_interrupts & PPC_INTERRUPT_EXT) &&
> +    if ((pending_interrupts & PPC_INTERRUPT_EXT) &&
>         (env->spr[SPR_LPCR] & LPCR_EEE)) {
>         bool heic = !!(env->spr[SPR_LPCR] & LPCR_HEIC);
>         if (!heic || !FIELD_EX64_HV(env->msr) ||
> @@ -1854,35 +1856,35 @@ static int p9_interrupt_powersave(CPUPPCState *env)
>         }
>     }
>     /* Decrementer Exception */
> -    if ((env->pending_interrupts & PPC_INTERRUPT_DECR) &&
> +    if ((pending_interrupts & PPC_INTERRUPT_DECR) &&
>         (env->spr[SPR_LPCR] & LPCR_DEE)) {
>         return PPC_INTERRUPT_DECR;
>     }
>     /* Machine Check or Hypervisor Maintenance Exception */
>     if (env->spr[SPR_LPCR] & LPCR_OEE) {
> -        if (env->pending_interrupts & PPC_INTERRUPT_MCK) {
> +        if (pending_interrupts & PPC_INTERRUPT_MCK) {
>             return PPC_INTERRUPT_MCK;
>         }
> -        if (env->pending_interrupts & PPC_INTERRUPT_HMI) {
> +        if (pending_interrupts & PPC_INTERRUPT_HMI) {
>             return PPC_INTERRUPT_HMI;
>         }
>     }
>     /* Privileged Doorbell Exception */
> -    if ((env->pending_interrupts & PPC_INTERRUPT_DOORBELL) &&
> +    if ((pending_interrupts & PPC_INTERRUPT_DOORBELL) &&
>         (env->spr[SPR_LPCR] & LPCR_PDEE)) {
>         return PPC_INTERRUPT_DOORBELL;
>     }
>     /* Hypervisor Doorbell Exception */
> -    if ((env->pending_interrupts & PPC_INTERRUPT_HDOORBELL) &&
> +    if ((pending_interrupts & PPC_INTERRUPT_HDOORBELL) &&
>         (env->spr[SPR_LPCR] & LPCR_HDEE)) {
>         return PPC_INTERRUPT_HDOORBELL;
>     }
>     /* Hypervisor virtualization exception */
> -    if ((env->pending_interrupts & PPC_INTERRUPT_HVIRT) &&
> +    if ((pending_interrupts & PPC_INTERRUPT_HVIRT) &&
>         (env->spr[SPR_LPCR] & LPCR_HVEE)) {
>         return PPC_INTERRUPT_HVIRT;
>     }
> -    if (env->pending_interrupts & PPC_INTERRUPT_RESET) {
> +    if (pending_interrupts & PPC_INTERRUPT_RESET) {
>         return PPC_INTERRUPT_RESET;
>     }
>     return 0;
> @@ -1891,11 +1893,12 @@ static int p9_interrupt_powersave(CPUPPCState *env)
> static int p9_next_unmasked_interrupt(CPUPPCState *env)
> {
>     CPUState *cs = env_cpu(env);
> +    uint32_t pending_interrupts = env->pending_interrupts;
>
>     /* 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);
> +    assert((pending_interrupts & P9_UNUSED_INTERRUPTS) == 0);
>
>     if (cs->halted) {
>         if (env->spr[SPR_PSSCR] & PSSCR_EC) {
> @@ -1914,12 +1917,12 @@ static int p9_next_unmasked_interrupt(CPUPPCState *env)
>     }
>
>     /* Machine check exception */
> -    if (env->pending_interrupts & PPC_INTERRUPT_MCK) {
> +    if (pending_interrupts & PPC_INTERRUPT_MCK) {
>         return PPC_INTERRUPT_MCK;
>     }
>
>     /* 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);
>         if ((msr_ee || !FIELD_EX64_HV(env->msr)) && hdice) {
> @@ -1929,7 +1932,7 @@ static int p9_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);
>         if ((msr_ee || !FIELD_EX64_HV(env->msr)) && hvice) {
> @@ -1938,7 +1941,7 @@ static int p9_next_unmasked_interrupt(CPUPPCState *env)
>     }
>
>     /* External interrupt can ignore MSR:EE under some circumstances */
> -    if (env->pending_interrupts & PPC_INTERRUPT_EXT) {
> +    if (pending_interrupts & PPC_INTERRUPT_EXT) {
>         bool lpes0 = !!(env->spr[SPR_LPCR] & LPCR_LPES0);
>         bool heic = !!(env->spr[SPR_LPCR] & LPCR_HEIC);
>         /* HEIC blocks delivery to the hypervisor */
> @@ -1950,20 +1953,20 @@ static int p9_next_unmasked_interrupt(CPUPPCState *env)
>     }
>     if (msr_ee != 0) {
>         /* 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;
>         }
>         /* 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.
>

On Mon, 20 May 2024, BALATON Zoltan wrote:
> On Mon, 20 May 2024, Harsh Prateek Bora wrote:
>> Currently, p9 exception handling has multiple if-condition checks where
>> it does an indirect access to pending_interrupts via env. Cache the
>> value during entry and reuse later to avoid multiple indirect accesses.
>> 
>> Signed-off-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
>> ---
>> target/ppc/excp_helper.c | 39 +++++++++++++++++++++------------------
>> 1 file changed, 21 insertions(+), 18 deletions(-)
>> 
>> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
>> index 0712098cf7..4f158196bb 100644
>> --- a/target/ppc/excp_helper.c
>> +++ b/target/ppc/excp_helper.c
>> @@ -1844,8 +1844,10 @@ static int p8_next_unmasked_interrupt(CPUPPCState 
>> *env)
>> 
>> static int p9_interrupt_powersave(CPUPPCState *env)
>> {
>> +    uint32_t pending_interrupts = env->pending_interrupts;
>> +
>
> LPCR also seems to be used a lot and other similar *_powersave functions only 
> use pending_interrupts and LPCR so maybe you could change these functions to 
> take the pending_interrupts and lpcr as parameters and pass them from the 
> caller that already have it so even this dereference could be saved?

(In that case this patch may need to be split in two as well.)

Regards,
BALATON Zoltan

>>     /* External Exception */
>> -    if ((env->pending_interrupts & PPC_INTERRUPT_EXT) &&
>> +    if ((pending_interrupts & PPC_INTERRUPT_EXT) &&
>>         (env->spr[SPR_LPCR] & LPCR_EEE)) {
>>         bool heic = !!(env->spr[SPR_LPCR] & LPCR_HEIC);
>>         if (!heic || !FIELD_EX64_HV(env->msr) ||
>> @@ -1854,35 +1856,35 @@ static int p9_interrupt_powersave(CPUPPCState *env)
>>         }
>>     }
>>     /* Decrementer Exception */
>> -    if ((env->pending_interrupts & PPC_INTERRUPT_DECR) &&
>> +    if ((pending_interrupts & PPC_INTERRUPT_DECR) &&
>>         (env->spr[SPR_LPCR] & LPCR_DEE)) {
>>         return PPC_INTERRUPT_DECR;
>>     }
>>     /* Machine Check or Hypervisor Maintenance Exception */
>>     if (env->spr[SPR_LPCR] & LPCR_OEE) {
>> -        if (env->pending_interrupts & PPC_INTERRUPT_MCK) {
>> +        if (pending_interrupts & PPC_INTERRUPT_MCK) {
>>             return PPC_INTERRUPT_MCK;
>>         }
>> -        if (env->pending_interrupts & PPC_INTERRUPT_HMI) {
>> +        if (pending_interrupts & PPC_INTERRUPT_HMI) {
>>             return PPC_INTERRUPT_HMI;
>>         }
>>     }
>>     /* Privileged Doorbell Exception */
>> -    if ((env->pending_interrupts & PPC_INTERRUPT_DOORBELL) &&
>> +    if ((pending_interrupts & PPC_INTERRUPT_DOORBELL) &&
>>         (env->spr[SPR_LPCR] & LPCR_PDEE)) {
>>         return PPC_INTERRUPT_DOORBELL;
>>     }
>>     /* Hypervisor Doorbell Exception */
>> -    if ((env->pending_interrupts & PPC_INTERRUPT_HDOORBELL) &&
>> +    if ((pending_interrupts & PPC_INTERRUPT_HDOORBELL) &&
>>         (env->spr[SPR_LPCR] & LPCR_HDEE)) {
>>         return PPC_INTERRUPT_HDOORBELL;
>>     }
>>     /* Hypervisor virtualization exception */
>> -    if ((env->pending_interrupts & PPC_INTERRUPT_HVIRT) &&
>> +    if ((pending_interrupts & PPC_INTERRUPT_HVIRT) &&
>>         (env->spr[SPR_LPCR] & LPCR_HVEE)) {
>>         return PPC_INTERRUPT_HVIRT;
>>     }
>> -    if (env->pending_interrupts & PPC_INTERRUPT_RESET) {
>> +    if (pending_interrupts & PPC_INTERRUPT_RESET) {
>>         return PPC_INTERRUPT_RESET;
>>     }
>>     return 0;
>> @@ -1891,11 +1893,12 @@ static int p9_interrupt_powersave(CPUPPCState *env)
>> static int p9_next_unmasked_interrupt(CPUPPCState *env)
>> {
>>     CPUState *cs = env_cpu(env);
>> +    uint32_t pending_interrupts = env->pending_interrupts;
>>
>>     /* 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);
>> +    assert((pending_interrupts & P9_UNUSED_INTERRUPTS) == 0);
>>
>>     if (cs->halted) {
>>         if (env->spr[SPR_PSSCR] & PSSCR_EC) {
>> @@ -1914,12 +1917,12 @@ static int p9_next_unmasked_interrupt(CPUPPCState 
>> *env)
>>     }
>>
>>     /* Machine check exception */
>> -    if (env->pending_interrupts & PPC_INTERRUPT_MCK) {
>> +    if (pending_interrupts & PPC_INTERRUPT_MCK) {
>>         return PPC_INTERRUPT_MCK;
>>     }
>>
>>     /* 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);
>>         if ((msr_ee || !FIELD_EX64_HV(env->msr)) && hdice) {
>> @@ -1929,7 +1932,7 @@ static int p9_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);
>>         if ((msr_ee || !FIELD_EX64_HV(env->msr)) && hvice) {
>> @@ -1938,7 +1941,7 @@ static int p9_next_unmasked_interrupt(CPUPPCState 
>> *env)
>>     }
>>
>>     /* External interrupt can ignore MSR:EE under some circumstances */
>> -    if (env->pending_interrupts & PPC_INTERRUPT_EXT) {
>> +    if (pending_interrupts & PPC_INTERRUPT_EXT) {
>>         bool lpes0 = !!(env->spr[SPR_LPCR] & LPCR_LPES0);
>>         bool heic = !!(env->spr[SPR_LPCR] & LPCR_HEIC);
>>         /* HEIC blocks delivery to the hypervisor */
>> @@ -1950,20 +1953,20 @@ static int p9_next_unmasked_interrupt(CPUPPCState 
>> *env)
>>     }
>>     if (msr_ee != 0) {
>>         /* 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;
>>         }
>>         /* 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.
>> 
>
>