Re: [PATCH v2 8/8] target/ppc: 74xx: Set SRRs directly in exception code

[BALATON Zoltan]

On Thu, 27 Jan 2022, BALATON Zoltan wrote:
> On Thu, 27 Jan 2022, Fabiano Rosas wrote:
> > The 74xx does not have alternate/hypervisor Save and Restore
> > Registers, so we can set SRR0 and SRR1 directly.
> >
> > Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
> > ---
> > target/ppc/excp_helper.c | 13 ++-----------
> > 1 file changed, 2 insertions(+), 11 deletions(-)
> >
> > diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> > index b7921c0956..4e6bb87b70 100644
> > --- a/target/ppc/excp_helper.c
> > +++ b/target/ppc/excp_helper.c
> > @@ -556,7 +556,6 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int 
> > excp)
> >     CPUState *cs = CPU(cpu);
> >     CPUPPCState *env = &cpu->env;
> >     target_ulong msr, new_msr, vector;
> > -    int srr0, srr1;
> >
> >     if (excp <= POWERPC_EXCP_NONE || excp >= POWERPC_EXCP_NB) {
> >         cpu_abort(cs, "Invalid PowerPC exception %d. Aborting\n", excp);
> > @@ -575,10 +574,6 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int 
> > excp)
> >      */
> >     new_msr = env->msr & ((target_ulong)1 << MSR_ME);
> >
> > -    /* target registers */
> > -    srr0 = SPR_SRR0;
> > -    srr1 = SPR_SRR1;
> > -
> >     /*
> >      * Hypervisor emulation assistance interrupt only exists on server
> >      * arch 2.05 server or later.
> > @@ -731,10 +726,6 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int 
> > excp)
> >             cpu_abort(cs, "Trying to deliver HV exception (MSR) %d with "
> >                       "no HV support\n", excp);
> >         }
>
> If we have ho MSR_HVB why is this still here? Shouldn't it have been gone in 
> patch 2? Or is this still reachable?

Additionally if it's still needed then the two ifs could be collapsed into 
one with && now that the other branch below is removed.

Regards,
BALATON Zoltan
> > -        if (srr0 == SPR_HSRR0) {
> > -            cpu_abort(cs, "Trying to deliver HV exception (HSRR) %d with "
> > -                      "no HV support\n", excp);
> > -        }
> >     }
> >
> >     /*
> > @@ -746,10 +737,10 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int 
> > excp)
> >     }
> >
> >     /* Save PC */
> > -    env->spr[srr0] = env->nip;
> > +    env->spr[SPR_SRR0] = env->nip;
> >
> >     /* Save MSR */
> > -    env->spr[srr1] = msr;
> > +    env->spr[SPR_SRR1] = msr;
> >
> >     powerpc_set_excp_state(cpu, vector, new_msr);
> > }