| Message ID | 20220118184448.852996-14-farosas@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | target/ppc: powerpc_excp improvements [40x] (3/n) | expand |
On Tue, Jan 18, 2022 at 03:44:47PM -0300, Fabiano Rosas wrote: > The 405 Program Interrupt does not set SRR1 with any diagnostic bits, > just a clean copy of the MSR. > > We're using the BookE Exception Syndrome Register which is different > from the 405. Hrm. We really do want to set the 40x ESR bits here, though. > > Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com> > --- > target/ppc/excp_helper.c | 16 ---------------- > 1 file changed, 16 deletions(-) > > diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c > index 13674a102f..2efec6d13b 100644 > --- a/target/ppc/excp_helper.c > +++ b/target/ppc/excp_helper.c > @@ -484,30 +484,14 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp) > env->error_code = 0; > return; > } > - > - /* > - * FP exceptions always have NIP pointing to the faulting > - * instruction, so always use store_next and claim we are > - * precise in the MSR. > - */ > - msr |= 0x00100000; > - env->spr[SPR_BOOKE_ESR] = ESR_FP; > break; > case POWERPC_EXCP_INVAL: > trace_ppc_excp_inval(env->nip); > - msr |= 0x00080000; > - env->spr[SPR_BOOKE_ESR] = ESR_PIL; > break; > case POWERPC_EXCP_PRIV: > - msr |= 0x00040000; > - env->spr[SPR_BOOKE_ESR] = ESR_PPR; > - break; > case POWERPC_EXCP_TRAP: > - msr |= 0x00020000; > - env->spr[SPR_BOOKE_ESR] = ESR_PTR; > break; > default: > - /* Should never occur */ > cpu_abort(cs, "Invalid program exception %d. Aborting\n", > env->error_code); > break;
David Gibson <david@gibson.dropbear.id.au> writes: > On Tue, Jan 18, 2022 at 03:44:47PM -0300, Fabiano Rosas wrote: >> The 405 Program Interrupt does not set SRR1 with any diagnostic bits, >> just a clean copy of the MSR. >> >> We're using the BookE Exception Syndrome Register which is different >> from the 405. > > Hrm. We really do want to set the 40x ESR bits here, though. Well I wrote the code and nothing changed so I dropped it. Not sure if we are even raising these properly in the translation code. I'll take another look.
On 1/19/22 13:54, Fabiano Rosas wrote: > David Gibson <david@gibson.dropbear.id.au> writes: > >> On Tue, Jan 18, 2022 at 03:44:47PM -0300, Fabiano Rosas wrote: >>> The 405 Program Interrupt does not set SRR1 with any diagnostic bits, >>> just a clean copy of the MSR. >>> >>> We're using the BookE Exception Syndrome Register which is different >>> from the 405. >> >> Hrm. We really do want to set the 40x ESR bits here, though. > > Well I wrote the code and nothing changed so I dropped it. Not sure if > we are even raising these properly in the translation code. I'll take > another look. > For instance, this ESR bit allows Linux to handle traps correctly in some cases, like when CONFIG_DEBUG_VM=y : @@ -488,7 +488,9 @@ static void powerpc_excp_40x(PowerPCCPU trace_ppc_excp_inval(env->nip); break; case POWERPC_EXCP_PRIV: + break; case POWERPC_EXCP_TRAP: + env->spr[SPR_40x_ESR] = ESR_PTR; break; default: cpu_abort(cs, "Invalid program exception %d. Aborting\n", These could be reported to Linux : /* On 4xx, the reason for the machine check or program exception is in the ESR. */ #define get_reason(regs) ((regs)->esr) #define REASON_FP ESR_FP #define REASON_ILLEGAL (ESR_PIL | ESR_PUO) #define REASON_PRIVILEGED ESR_PPR #define REASON_TRAP ESR_PTR #define REASON_PREFIXED 0 #define REASON_BOUNDARY 0 Thanks, C.
On 1/18/22 19:44, Fabiano Rosas wrote: > The 405 Program Interrupt does not set SRR1 with any diagnostic bits, > just a clean copy of the MSR. > > We're using the BookE Exception Syndrome Register which is different > from the 405. I restored the setting of SPR_40x_ESR. > Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com> Reviewed-by: Cédric Le Goater <clg@kaod.org> Thanks, C. > --- > target/ppc/excp_helper.c | 16 ---------------- > 1 file changed, 16 deletions(-) > > diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c > index 13674a102f..2efec6d13b 100644 > --- a/target/ppc/excp_helper.c > +++ b/target/ppc/excp_helper.c > @@ -484,30 +484,14 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp) > env->error_code = 0; > return; > } > - > - /* > - * FP exceptions always have NIP pointing to the faulting > - * instruction, so always use store_next and claim we are > - * precise in the MSR. > - */ > - msr |= 0x00100000; > - env->spr[SPR_BOOKE_ESR] = ESR_FP; > break; > case POWERPC_EXCP_INVAL: > trace_ppc_excp_inval(env->nip); > - msr |= 0x00080000; > - env->spr[SPR_BOOKE_ESR] = ESR_PIL; > break; > case POWERPC_EXCP_PRIV: > - msr |= 0x00040000; > - env->spr[SPR_BOOKE_ESR] = ESR_PPR; > - break; > case POWERPC_EXCP_TRAP: > - msr |= 0x00020000; > - env->spr[SPR_BOOKE_ESR] = ESR_PTR; > break; > default: > - /* Should never occur */ > cpu_abort(cs, "Invalid program exception %d. Aborting\n", > env->error_code); > break; >
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c index 13674a102f..2efec6d13b 100644 --- a/target/ppc/excp_helper.c +++ b/target/ppc/excp_helper.c @@ -484,30 +484,14 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp) env->error_code = 0; return; } - - /* - * FP exceptions always have NIP pointing to the faulting - * instruction, so always use store_next and claim we are - * precise in the MSR. - */ - msr |= 0x00100000; - env->spr[SPR_BOOKE_ESR] = ESR_FP; break; case POWERPC_EXCP_INVAL: trace_ppc_excp_inval(env->nip); - msr |= 0x00080000; - env->spr[SPR_BOOKE_ESR] = ESR_PIL; break; case POWERPC_EXCP_PRIV: - msr |= 0x00040000; - env->spr[SPR_BOOKE_ESR] = ESR_PPR; - break; case POWERPC_EXCP_TRAP: - msr |= 0x00020000; - env->spr[SPR_BOOKE_ESR] = ESR_PTR; break; default: - /* Should never occur */ cpu_abort(cs, "Invalid program exception %d. Aborting\n", env->error_code); break;
The 405 Program Interrupt does not set SRR1 with any diagnostic bits, just a clean copy of the MSR. We're using the BookE Exception Syndrome Register which is different from the 405. Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com> --- target/ppc/excp_helper.c | 16 ---------------- 1 file changed, 16 deletions(-)