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

[Harsh Prateek Bora]

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: 
20240913041337.912876-1-harshpb@linux.ibm.com/T/#t">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;
> >       }