| Message ID | 20220103220746.3916246-6-farosas@linux.ibm.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | target/ppc: powerpc_excp improvements (2/n) | expand |
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. Thanks, C. > + if (ppc_interrupts_little_endian(cpu, !!(new_msr & MSR_HVB))) { > new_msr |= (target_ulong)1 << MSR_LE; > } > } else if (msr_ile) { >
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. 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".
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.
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) { + if (ppc_interrupts_little_endian(cpu, !!(new_msr & MSR_HVB))) { new_msr |= (target_ulong)1 << MSR_LE; } } else if (msr_ile) {
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(-)