Re: [PATCH 5/9] target/ppc: Use ppc_interrupts_little_endian in powerpc_excp

[Cédric Le Goater]

On 1/4/22 15:11, Fabiano Rosas wrote:
> Cédric Le Goater <clg@kaod.org> writes:
>
> > On 1/3/22 23:07, Fabiano Rosas wrote:
> > > The ppc_interrupts_little_endian function is suitable for determining
> > > the endianness of interrupts for all Book3S CPUs.
> > >
> > > (I'm keeping the MSR check for the rest of the CPUs, but it will go
> > > away in the next patch.)
> > >
> > > Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
> > > ---
> > >    target/ppc/excp_helper.c | 21 ++-------------------
> > >    1 file changed, 2 insertions(+), 19 deletions(-)
> > >
> > > diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> > > index 0dbadc5d07..5d31940426 100644
> > > --- a/target/ppc/excp_helper.c
> > > +++ b/target/ppc/excp_helper.c
> > > @@ -760,25 +760,8 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp)
> > >         * CPU, the HV mode, etc...
> > >         */
> > >    #ifdef TARGET_PPC64
> > > -    if (excp_model == POWERPC_EXCP_POWER7) {
> > > -        if (!(new_msr & MSR_HVB) && (env->spr[SPR_LPCR] & LPCR_ILE)) {
> > > -            new_msr |= (target_ulong)1 << MSR_LE;
> > > -        }
> > > -    } else if (excp_model == POWERPC_EXCP_POWER8) {
> > > -        if (new_msr & MSR_HVB) {
> > > -            if (env->spr[SPR_HID0] & HID0_HILE) {
> > > -                new_msr |= (target_ulong)1 << MSR_LE;
> > > -            }
> > > -        } else if (env->spr[SPR_LPCR] & LPCR_ILE) {
> > > -            new_msr |= (target_ulong)1 << MSR_LE;
> > > -        }
> > > -    } else if (excp_model == POWERPC_EXCP_POWER9 ||
> > > -               excp_model == POWERPC_EXCP_POWER10) {
> > > -        if (new_msr & MSR_HVB) {
> > > -            if (env->spr[SPR_HID0] & HID0_POWER9_HILE) {
> > > -                new_msr |= (target_ulong)1 << MSR_LE;
> > > -            }
> > > -        } else if (env->spr[SPR_LPCR] & LPCR_ILE) {
> > > +    if (excp_model >= POWERPC_EXCP_970) {
> >
> > why include POWERPC_EXCP_970 ? These CPUs do not support Little Endian.
>
> Because the 970 exception model covers POWER5P as well which has ILE.
we need to untangle this first.

POWERPC_EXCP_970 is checked in dbcz and the HID5 bits are specific to 970.
May be add a POWERPC_EXCP_POWER5P ?

> And looking at cpu_init.c, POWER5 uses a bunch of 970 functions. And
> POWER7 uses the POWER5 ones. So we kind of have a dependency chain
> there. That is why I'm always handling ">= 970" as "books".

This is a mess. We also have is_book3s_arch2x() but it will not apply
here.

C.