| Message ID | 1469571686-7284-16-git-send-email-benh@kernel.crashing.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jul 27, 2016 at 08:21:10AM +1000, Benjamin Herrenschmidt wrote: > We make env->nip almost always point to the faulting instruction, > thus avoiding a mess of "store_current" vs "store_next" in the > exception handling. The syscall exception knows to move the PC by > 4 and that's really about it. > > This actually fixes a number of cases where the translator was > setting env->nip to ctx->nip - 4 (ie. the *current* instruction) > but the program check exception handler would branch to > "store_current" which applies another -4 offset. > > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> I'm having a fair bit of trouble wrapping my head around this. Still comments on a couple of small matters. > --- > linux-user/main.c | 12 ++-- > target-ppc/excp_helper.c | 148 ++++++++++++++++++----------------------------- > target-ppc/translate.c | 59 +++++++++++-------- > 3 files changed, 96 insertions(+), 123 deletions(-) > > diff --git a/linux-user/main.c b/linux-user/main.c > index 462e820..1d149dc 100644 > --- a/linux-user/main.c > +++ b/linux-user/main.c > @@ -1814,7 +1814,7 @@ void cpu_loop(CPUPPCState *env) > env->error_code); > break; > } > - info._sifields._sigfault._addr = env->nip - 4; > + info._sifields._sigfault._addr = env->nip; > queue_signal(env, info.si_signo, &info); > break; > case POWERPC_EXCP_FPU: /* Floating-point unavailable exception */ > @@ -1822,7 +1822,7 @@ void cpu_loop(CPUPPCState *env) > info.si_signo = TARGET_SIGILL; > info.si_errno = 0; > info.si_code = TARGET_ILL_COPROC; > - info._sifields._sigfault._addr = env->nip - 4; > + info._sifields._sigfault._addr = env->nip; > queue_signal(env, info.si_signo, &info); > break; > case POWERPC_EXCP_SYSCALL: /* System call exception */ > @@ -1834,7 +1834,7 @@ void cpu_loop(CPUPPCState *env) > info.si_signo = TARGET_SIGILL; > info.si_errno = 0; > info.si_code = TARGET_ILL_COPROC; > - info._sifields._sigfault._addr = env->nip - 4; > + info._sifields._sigfault._addr = env->nip; > queue_signal(env, info.si_signo, &info); > break; > case POWERPC_EXCP_DECR: /* Decrementer exception */ > @@ -1862,7 +1862,7 @@ void cpu_loop(CPUPPCState *env) > info.si_signo = TARGET_SIGILL; > info.si_errno = 0; > info.si_code = TARGET_ILL_COPROC; > - info._sifields._sigfault._addr = env->nip - 4; > + info._sifields._sigfault._addr = env->nip; > queue_signal(env, info.si_signo, &info); > break; > case POWERPC_EXCP_EFPDI: /* Embedded floating-point data IRQ */ > @@ -1926,7 +1926,7 @@ void cpu_loop(CPUPPCState *env) > info.si_signo = TARGET_SIGILL; > info.si_errno = 0; > info.si_code = TARGET_ILL_COPROC; > - info._sifields._sigfault._addr = env->nip - 4; > + info._sifields._sigfault._addr = env->nip; > queue_signal(env, info.si_signo, &info); > break; > case POWERPC_EXCP_PIT: /* Programmable interval timer IRQ */ > @@ -2001,9 +2001,9 @@ void cpu_loop(CPUPPCState *env) > env->gpr[5], env->gpr[6], env->gpr[7], > env->gpr[8], 0, 0); > if (ret == -TARGET_ERESTARTSYS) { > - env->nip -= 4; > break; > } > + env->nip += 4; > if (ret == (target_ulong)(-TARGET_QEMU_ESIGRETURN)) { > /* Returning from a successful sigreturn syscall. > Avoid corrupting register state. */ > diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c > index 563c7bc..570d188 100644 > --- a/target-ppc/excp_helper.c > +++ b/target-ppc/excp_helper.c > @@ -198,7 +198,7 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) > default: > goto excp_invalid; > } > - goto store_next; > + break; > case POWERPC_EXCP_MCHECK: /* Machine check exception */ > if (msr_me == 0) { > /* Machine check exception is not enabled. > @@ -209,7 +209,7 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) > if (qemu_log_separate()) { > qemu_log("Machine check while not allowed. " > "Entering checkstop state\n"); > - } > + } Looks like an accidental whitespace change. > cs->halted = 1; > cs->interrupt_request |= CPU_INTERRUPT_EXITTB; > } > @@ -235,16 +235,16 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) > default: > break; > } > - goto store_next; > + break; > case POWERPC_EXCP_DSI: /* Data storage exception */ > LOG_EXCP("DSI exception: DSISR=" TARGET_FMT_lx" DAR=" TARGET_FMT_lx > "\n", env->spr[SPR_DSISR], env->spr[SPR_DAR]); > - goto store_next; > + break; > case POWERPC_EXCP_ISI: /* Instruction storage exception */ > LOG_EXCP("ISI exception: msr=" TARGET_FMT_lx ", nip=" TARGET_FMT_lx > "\n", msr, env->nip); > msr |= env->error_code; > - goto store_next; > + break; > case POWERPC_EXCP_EXTERNAL: /* External input */ > cs = CPU(cpu); > > @@ -258,13 +258,14 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) > /* IACK the IRQ on delivery */ > env->spr[SPR_BOOKE_EPR] = ldl_phys(cs->as, env->mpic_iack); > } > - goto store_next; > + break; > case POWERPC_EXCP_ALIGN: /* Alignment exception */ > /* XXX: this is false */ > /* Get rS/rD and rA from faulting opcode */ > - env->spr[SPR_DSISR] |= (cpu_ldl_code(env, (env->nip - 4)) > + /* Broken for LE mode */ > + env->spr[SPR_DSISR] |= (cpu_ldl_code(env, env->nip) > & 0x03FF0000) >> 16; > - goto store_next; > + break; > case POWERPC_EXCP_PROGRAM: /* Program exception */ > switch (env->error_code & ~0xF) { > case POWERPC_EXCP_FP: > @@ -280,15 +281,11 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) > * precise in the MSR. > */ > msr |= 0x00100000; > - goto store_next; > + break; > case POWERPC_EXCP_INVAL: > LOG_EXCP("Invalid instruction at " TARGET_FMT_lx "\n", env->nip); > msr |= 0x00080000; > env->spr[SPR_BOOKE_ESR] = ESR_PIL; > - /* Some invalids will have the PC in the right place already */ > - if (env->error_code & POWERPC_EXCP_INVAL_LSWX) { > - goto store_next; > - } > break; > case POWERPC_EXCP_PRIV: > msr |= 0x00040000; > @@ -304,23 +301,16 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) > env->error_code); > break; > } > - goto store_current; > - case POWERPC_EXCP_HV_EMU: > - srr0 = SPR_HSRR0; > - srr1 = SPR_HSRR1; > - new_msr |= (target_ulong)MSR_HVB; > - new_msr |= env->msr & ((target_ulong)1 << MSR_RI); > - /* Some invalids will have the PC in the right place already */ > - if (env->error_code == (POWERPC_EXCP_INVAL|POWERPC_EXCP_INVAL_LSWX)) { > - goto store_next; > - } > - goto store_current; > - case POWERPC_EXCP_FPU: /* Floating-point unavailable exception */ > - goto store_current; > + break; > case POWERPC_EXCP_SYSCALL: /* System call exception */ > dump_syscall(env); > lev = env->error_code; > > + /* We need to correct the NIP which in this case is supposed > + * to point to the next instruction > + */ > + env->nip += 4; > + > /* "PAPR mode" built-in hypercall emulation */ > if ((lev == 1) && cpu_ppc_hypercall) { > cpu_ppc_hypercall(cpu); > @@ -329,15 +319,15 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) > if (lev == 1) { > new_msr |= (target_ulong)MSR_HVB; > } > - goto store_next; > + break; > + case POWERPC_EXCP_FPU: /* Floating-point unavailable exception */ > case POWERPC_EXCP_APU: /* Auxiliary processor unavailable */ > - goto store_current; > case POWERPC_EXCP_DECR: /* Decrementer exception */ > - goto store_next; > + break; > case POWERPC_EXCP_FIT: /* Fixed-interval timer interrupt */ > /* FIT on 4xx */ > LOG_EXCP("FIT exception\n"); > - goto store_next; > + break; > case POWERPC_EXCP_WDT: /* Watchdog timer interrupt */ > LOG_EXCP("WDT exception\n"); > switch (excp_model) { > @@ -348,11 +338,10 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) > default: > break; > } > - goto store_next; > + break; > case POWERPC_EXCP_DTLB: /* Data TLB error */ > - goto store_next; > case POWERPC_EXCP_ITLB: /* Instruction TLB error */ > - goto store_next; > + break; > case POWERPC_EXCP_DEBUG: /* Debug interrupt */ > switch (excp_model) { > case POWERPC_EXCP_BOOKE: > @@ -367,33 +356,33 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) > } > /* XXX: TODO */ > cpu_abort(cs, "Debug exception is not implemented yet !\n"); > - goto store_next; > + break; > case POWERPC_EXCP_SPEU: /* SPE/embedded floating-point unavailable */ > env->spr[SPR_BOOKE_ESR] = ESR_SPV; > - goto store_current; > + break; > case POWERPC_EXCP_EFPDI: /* Embedded floating-point data interrupt */ > /* XXX: TODO */ > cpu_abort(cs, "Embedded floating point data exception " > "is not implemented yet !\n"); > env->spr[SPR_BOOKE_ESR] = ESR_SPV; > - goto store_next; > + break; > case POWERPC_EXCP_EFPRI: /* Embedded floating-point round interrupt */ > /* XXX: TODO */ > cpu_abort(cs, "Embedded floating point round exception " > "is not implemented yet !\n"); > env->spr[SPR_BOOKE_ESR] = ESR_SPV; > - goto store_next; > + break; > case POWERPC_EXCP_EPERFM: /* Embedded performance monitor interrupt */ > /* XXX: TODO */ > cpu_abort(cs, > "Performance counter exception is not implemented yet !\n"); > - goto store_next; > + break; > case POWERPC_EXCP_DOORI: /* Embedded doorbell interrupt */ > - goto store_next; > + break; > case POWERPC_EXCP_DOORCI: /* Embedded doorbell critical interrupt */ > srr0 = SPR_BOOKE_CSRR0; > srr1 = SPR_BOOKE_CSRR1; > - goto store_next; > + break; > case POWERPC_EXCP_RESET: /* System reset exception */ > if (msr_pow) { > /* indicate that we resumed from power save mode */ > @@ -404,65 +393,42 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) > > new_msr |= (target_ulong)MSR_HVB; > ail = 0; > - goto store_next; > + break; > case POWERPC_EXCP_DSEG: /* Data segment exception */ > - goto store_next; > case POWERPC_EXCP_ISEG: /* Instruction segment exception */ > - goto store_next; > - case POWERPC_EXCP_HDECR: /* Hypervisor decrementer exception */ > - srr0 = SPR_HSRR0; > - srr1 = SPR_HSRR1; > - new_msr |= (target_ulong)MSR_HVB; > - new_msr |= env->msr & ((target_ulong)1 << MSR_RI); > - goto store_next; > case POWERPC_EXCP_TRACE: /* Trace exception */ > - goto store_next; > + break; > + case POWERPC_EXCP_HDECR: /* Hypervisor decrementer exception */ > case POWERPC_EXCP_HDSI: /* Hypervisor data storage exception */ > - srr0 = SPR_HSRR0; > - srr1 = SPR_HSRR1; > - new_msr |= (target_ulong)MSR_HVB; > - new_msr |= env->msr & ((target_ulong)1 << MSR_RI); > - goto store_next; > case POWERPC_EXCP_HISI: /* Hypervisor instruction storage exception */ > - srr0 = SPR_HSRR0; > - srr1 = SPR_HSRR1; > - new_msr |= (target_ulong)MSR_HVB; > - new_msr |= env->msr & ((target_ulong)1 << MSR_RI); > - goto store_next; > case POWERPC_EXCP_HDSEG: /* Hypervisor data segment exception */ > - srr0 = SPR_HSRR0; > - srr1 = SPR_HSRR1; > - new_msr |= (target_ulong)MSR_HVB; > - new_msr |= env->msr & ((target_ulong)1 << MSR_RI); > - goto store_next; > case POWERPC_EXCP_HISEG: /* Hypervisor instruction segment exception */ > + case POWERPC_EXCP_HV_EMU: > srr0 = SPR_HSRR0; > srr1 = SPR_HSRR1; > new_msr |= (target_ulong)MSR_HVB; > new_msr |= env->msr & ((target_ulong)1 << MSR_RI); > - goto store_next; > + break; > case POWERPC_EXCP_VPU: /* Vector unavailable exception */ > - goto store_current; > case POWERPC_EXCP_VSXU: /* VSX unavailable exception */ > - goto store_current; > case POWERPC_EXCP_FU: /* Facility unavailable exception */ > - goto store_current; > + break; > case POWERPC_EXCP_PIT: /* Programmable interval timer interrupt */ > LOG_EXCP("PIT exception\n"); > - goto store_next; > + break; > case POWERPC_EXCP_IO: /* IO error exception */ > /* XXX: TODO */ > cpu_abort(cs, "601 IO error exception is not implemented yet !\n"); > - goto store_next; > + break; > case POWERPC_EXCP_RUNM: /* Run mode exception */ > /* XXX: TODO */ > cpu_abort(cs, "601 run mode exception is not implemented yet !\n"); > - goto store_next; > + break; > case POWERPC_EXCP_EMUL: /* Emulation trap exception */ > /* XXX: TODO */ > cpu_abort(cs, "602 emulation trap exception " > "is not implemented yet !\n"); > - goto store_next; > + break; > case POWERPC_EXCP_IFTLB: /* Instruction fetch TLB error */ > switch (excp_model) { > case POWERPC_EXCP_602: > @@ -577,71 +543,67 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) > cpu_abort(cs, "Invalid data store TLB miss exception\n"); > break; > } > - goto store_next; > + break; > case POWERPC_EXCP_FPA: /* Floating-point assist exception */ > /* XXX: TODO */ > cpu_abort(cs, "Floating point assist exception " > "is not implemented yet !\n"); > - goto store_next; > + break; > case POWERPC_EXCP_DABR: /* Data address breakpoint */ > /* XXX: TODO */ > cpu_abort(cs, "DABR exception is not implemented yet !\n"); > - goto store_next; > + break; > case POWERPC_EXCP_IABR: /* Instruction address breakpoint */ > /* XXX: TODO */ > cpu_abort(cs, "IABR exception is not implemented yet !\n"); > - goto store_next; > + break; > case POWERPC_EXCP_SMI: /* System management interrupt */ > /* XXX: TODO */ > cpu_abort(cs, "SMI exception is not implemented yet !\n"); > - goto store_next; > + break; > case POWERPC_EXCP_THERM: /* Thermal interrupt */ > /* XXX: TODO */ > cpu_abort(cs, "Thermal management exception " > "is not implemented yet !\n"); > - goto store_next; > + break; > case POWERPC_EXCP_PERFM: /* Embedded performance monitor interrupt */ > /* XXX: TODO */ > cpu_abort(cs, > "Performance counter exception is not implemented yet !\n"); > - goto store_next; > + break; > case POWERPC_EXCP_VPUA: /* Vector assist exception */ > /* XXX: TODO */ > cpu_abort(cs, "VPU assist exception is not implemented yet !\n"); > - goto store_next; > + break; > case POWERPC_EXCP_SOFTP: /* Soft patch exception */ > /* XXX: TODO */ > cpu_abort(cs, > "970 soft-patch exception is not implemented yet !\n"); > - goto store_next; > + break; > case POWERPC_EXCP_MAINT: /* Maintenance exception */ > /* XXX: TODO */ > cpu_abort(cs, > "970 maintenance exception is not implemented yet !\n"); > - goto store_next; > + break; > case POWERPC_EXCP_MEXTBR: /* Maskable external breakpoint */ > /* XXX: TODO */ > cpu_abort(cs, "Maskable external exception " > "is not implemented yet !\n"); > - goto store_next; > + break; > case POWERPC_EXCP_NMEXTBR: /* Non maskable external breakpoint */ > /* XXX: TODO */ > cpu_abort(cs, "Non maskable external exception " > "is not implemented yet !\n"); > - goto store_next; > + break; > default: > excp_invalid: > cpu_abort(cs, "Invalid PowerPC exception %d. Aborting\n", excp); > break; > - store_current: > - /* save current instruction location */ > - env->spr[srr0] = env->nip - 4; > - break; > - store_next: > - /* save next instruction location */ > - env->spr[srr0] = env->nip; > - break; > } > + > + /* Save PC */ > + env->spr[srr0] = env->nip; > + > /* Save MSR */ > env->spr[srr1] = msr; > > diff --git a/target-ppc/translate.c b/target-ppc/translate.c > index c4f8916..84bcb09 100644 > --- a/target-ppc/translate.c > +++ b/target-ppc/translate.c > @@ -276,8 +276,12 @@ void gen_update_current_nip(void *opaque) > static void gen_exception_err(DisasContext *ctx, uint32_t excp, uint32_t error) > { > TCGv_i32 t0, t1; > + > + /* These are all synchronous exceptions, we set the PC back to > + * the faulting instruction > + */ > if (ctx->exception == POWERPC_EXCP_NONE) { > - gen_update_nip(ctx, ctx->nip); > + gen_update_nip(ctx, ctx->nip - 4); > } > t0 = tcg_const_i32(excp); > t1 = tcg_const_i32(error); > @@ -290,8 +294,12 @@ static void gen_exception_err(DisasContext *ctx, uint32_t excp, uint32_t error) > static void gen_exception(DisasContext *ctx, uint32_t excp) > { > TCGv_i32 t0; > + > + /* These are all synchronous exceptions, we set the PC back to > + * the faulting instruction > + */ > if (ctx->exception == POWERPC_EXCP_NONE) { > - gen_update_nip(ctx, ctx->nip); > + gen_update_nip(ctx, ctx->nip - 4); > } > t0 = tcg_const_i32(excp); > gen_helper_raise_exception(cpu_env, t0); > @@ -299,13 +307,28 @@ static void gen_exception(DisasContext *ctx, uint32_t excp) > ctx->exception = (excp); > } > > +static void gen_exception_nip(DisasContext *ctx, uint32_t excp, > + target_ulong nip) > +{ > + TCGv_i32 t0; > + > + gen_update_nip(ctx, nip); > + t0 = tcg_const_i32(excp); > + gen_helper_raise_exception(cpu_env, t0); > + tcg_temp_free_i32(t0); > + ctx->exception = (excp); > +} > + > static void gen_debug_exception(DisasContext *ctx) > { > TCGv_i32 t0; > > + /* These are all synchronous exceptions, we set the PC back to > + * the faulting instruction > + */ > if ((ctx->exception != POWERPC_EXCP_BRANCH) && > (ctx->exception != POWERPC_EXCP_SYNC)) { > - gen_update_nip(ctx, ctx->nip); > + gen_update_nip(ctx, ctx->nip - 4); > } > t0 = tcg_const_i32(EXCP_DEBUG); > gen_helper_raise_exception(cpu_env, t0); > @@ -1437,7 +1460,7 @@ static void gen_pause(DisasContext *ctx) > tcg_temp_free_i32(t0); > > /* Stop translation, this gives other CPUs a chance to run */ > - gen_exception_err(ctx, EXCP_HLT, 1); > + gen_exception_nip(ctx, EXCP_HLT, ctx->nip); > } > #endif /* defined(TARGET_PPC64) */ > > @@ -2697,12 +2720,6 @@ static void gen_lswi(DisasContext *ctx) > nb = 32; > nr = (nb + 3) / 4; > if (unlikely(lsw_reg_in_range(start, nr, ra))) { > - /* The handler expects the PC to point to *this* instruction, > - * so setting ctx->exception here prevents it from being > - * improperly updated again by gen_inval_exception > - */ > - gen_update_nip(ctx, ctx->nip - 4); > - ctx->exception = POWERPC_EXCP_HV_EMU; > gen_inval_exception(ctx, POWERPC_EXCP_INVAL_LSWX); > return; > } > @@ -2845,10 +2862,7 @@ static void gen_conditional_store(DisasContext *ctx, TCGv EA, > tcg_gen_movi_tl(t0, (size << 5) | reg); > tcg_gen_st_tl(t0, cpu_env, offsetof(CPUPPCState, reserve_info)); > tcg_temp_free(t0); > - gen_update_nip(ctx, ctx->nip-4); > - ctx->exception = POWERPC_EXCP_BRANCH; > - gen_exception(ctx, POWERPC_EXCP_STCX); > - ctx->exception = save_exception; > + gen_exception_err(ctx, POWERPC_EXCP_STCX, 0); > } > #else > static void gen_conditional_store(DisasContext *ctx, TCGv EA, > @@ -2987,7 +3001,7 @@ static void gen_wait(DisasContext *ctx) > -offsetof(PowerPCCPU, env) + offsetof(CPUState, halted)); > tcg_temp_free_i32(t0); > /* Stop translation, as the CPU is supposed to sleep from now */ > - gen_exception_err(ctx, EXCP_HLT, 1); > + gen_exception_nip(ctx, EXCP_HLT, ctx->nip); > } > > #if defined(TARGET_PPC64) > @@ -3090,10 +3104,7 @@ static inline void gen_goto_tb(DisasContext *ctx, int n, target_ulong dest) > (CPU_BRANCH_STEP | CPU_SINGLE_STEP)) && > (ctx->exception == POWERPC_EXCP_BRANCH || > ctx->exception == POWERPC_EXCP_TRACE)) { > - target_ulong tmp = ctx->nip; > - ctx->nip = dest; > - gen_exception(ctx, POWERPC_EXCP_TRACE); > - ctx->nip = tmp; > + gen_exception_nip(ctx, POWERPC_EXCP_TRACE, dest); > } > if (ctx->singlestep_enabled & GDBSTUB_SINGLE_STEP) { > gen_debug_exception(ctx); > @@ -3362,7 +3373,7 @@ static void gen_tw(DisasContext *ctx) > { > TCGv_i32 t0 = tcg_const_i32(TO(ctx->opcode)); > /* Update the nip since this might generate a trap exception */ > - gen_update_nip(ctx, ctx->nip); > + gen_update_nip(ctx, ctx->nip - 4); twi etc will generally resume from the next instruction if they trap, yes? In which case I'm a bit confused by the nip - 4. But possibly I just haven't correctly followed all the nip update logic changed by this patch. > gen_helper_tw(cpu_env, cpu_gpr[rA(ctx->opcode)], cpu_gpr[rB(ctx->opcode)], > t0); > tcg_temp_free_i32(t0); > @@ -3374,7 +3385,7 @@ static void gen_twi(DisasContext *ctx) > TCGv t0 = tcg_const_tl(SIMM(ctx->opcode)); > TCGv_i32 t1 = tcg_const_i32(TO(ctx->opcode)); > /* Update the nip since this might generate a trap exception */ > - gen_update_nip(ctx, ctx->nip); > + gen_update_nip(ctx, ctx->nip - 4); > gen_helper_tw(cpu_env, cpu_gpr[rA(ctx->opcode)], t0, t1); > tcg_temp_free(t0); > tcg_temp_free_i32(t1); > @@ -3386,7 +3397,7 @@ static void gen_td(DisasContext *ctx) > { > TCGv_i32 t0 = tcg_const_i32(TO(ctx->opcode)); > /* Update the nip since this might generate a trap exception */ > - gen_update_nip(ctx, ctx->nip); > + gen_update_nip(ctx, ctx->nip - 4); > gen_helper_td(cpu_env, cpu_gpr[rA(ctx->opcode)], cpu_gpr[rB(ctx->opcode)], > t0); > tcg_temp_free_i32(t0); > @@ -3398,7 +3409,7 @@ static void gen_tdi(DisasContext *ctx) > TCGv t0 = tcg_const_tl(SIMM(ctx->opcode)); > TCGv_i32 t1 = tcg_const_i32(TO(ctx->opcode)); > /* Update the nip since this might generate a trap exception */ > - gen_update_nip(ctx, ctx->nip); > + gen_update_nip(ctx, ctx->nip - 4); > gen_helper_td(cpu_env, cpu_gpr[rA(ctx->opcode)], t0, t1); > tcg_temp_free(t0); > tcg_temp_free_i32(t1); > @@ -6757,7 +6768,7 @@ void gen_intermediate_code(CPUPPCState *env, struct TranslationBlock *tb) > ctx.exception != POWERPC_SYSCALL && > ctx.exception != POWERPC_EXCP_TRAP && > ctx.exception != POWERPC_EXCP_BRANCH)) { > - gen_exception(ctxp, POWERPC_EXCP_TRACE); > + gen_exception_nip(ctxp, POWERPC_EXCP_TRACE, ctx.nip); > } else if (unlikely(((ctx.nip & (TARGET_PAGE_SIZE - 1)) == 0) || > (cs->singlestep_enabled) || > singlestep ||
On Wed, 2016-07-27 at 12:19 +1000, David Gibson wrote: > On Wed, Jul 27, 2016 at 08:21:10AM +1000, Benjamin Herrenschmidt > wrote: > > > > We make env->nip almost always point to the faulting instruction, > > thus avoiding a mess of "store_current" vs "store_next" in the > > exception handling. The syscall exception knows to move the PC by > > 4 and that's really about it. > > > > This actually fixes a number of cases where the translator was > > setting env->nip to ctx->nip - 4 (ie. the *current* instruction) > > but the program check exception handler would branch to > > "store_current" which applies another -4 offset. > > > > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashinng.org > > I'm having a fair bit of trouble wrapping my head around this. Still > comments on a couple of small matters. Because the original code was a bloody mess ? :-) Basically, we make sure that we either don't set env->nip (because the helper will use the TB scan mechanism based on the return address) or we set it to the address of the instruction that caused the fault *always*. Then, in powerpc_excp, we just put that in SRR1 with one exception, the syscall, where by spec, it has to be on the next instruction. To fully understand this, you need to know that when the generator of an instruction is called, ctx->nip has already been moved to the next instruction (+4). Cheers, Ben. > > > > --- > > linux-user/main.c | 12 ++-- > > target-ppc/excp_helper.c | 148 ++++++++++++++++++----------------- > > ------------ > > target-ppc/translate.c | 59 +++++++++++-------- > > 3 files changed, 96 insertions(+), 123 deletions(-) > > > > diff --git a/linux-user/main.c b/linux-user/main.c > > index 462e820..1d149dc 100644 > > --- a/linux-user/main.c > > +++ b/linux-user/main.c > > @@ -1814,7 +1814,7 @@ void cpu_loop(CPUPPCState *env) > > env->error_code); > > break; > > } > > - info._sifields._sigfault._addr = env->nip - 4; > > + info._sifields._sigfault._addr = env->nip; > > queue_signal(env, info.si_signo, &info); > > break; > > case POWERPC_EXCP_FPU: /* Floating-point unavailable > > exception */ > > @@ -1822,7 +1822,7 @@ void cpu_loop(CPUPPCState *env) > > info.si_signo = TARGET_SIGILL; > > info.si_errno = 0; > > info.si_code = TARGET_ILL_COPROC; > > - info._sifields._sigfault._addr = env->nip - 4; > > + info._sifields._sigfault._addr = env->nip; > > queue_signal(env, info.si_signo, &info); > > break; > > case POWERPC_EXCP_SYSCALL: /* System call > > exception */ > > @@ -1834,7 +1834,7 @@ void cpu_loop(CPUPPCState *env) > > info.si_signo = TARGET_SIGILL; > > info.si_errno = 0; > > info.si_code = TARGET_ILL_COPROC; > > - info._sifields._sigfault._addr = env->nip - 4; > > + info._sifields._sigfault._addr = env->nip; > > queue_signal(env, info.si_signo, &info); > > break; > > case POWERPC_EXCP_DECR: /* Decrementer > > exception */ > > @@ -1862,7 +1862,7 @@ void cpu_loop(CPUPPCState *env) > > info.si_signo = TARGET_SIGILL; > > info.si_errno = 0; > > info.si_code = TARGET_ILL_COPROC; > > - info._sifields._sigfault._addr = env->nip - 4; > > + info._sifields._sigfault._addr = env->nip; > > queue_signal(env, info.si_signo, &info); > > break; > > case POWERPC_EXCP_EFPDI: /* Embedded floating-point > > data IRQ */ > > @@ -1926,7 +1926,7 @@ void cpu_loop(CPUPPCState *env) > > info.si_signo = TARGET_SIGILL; > > info.si_errno = 0; > > info.si_code = TARGET_ILL_COPROC; > > - info._sifields._sigfault._addr = env->nip - 4; > > + info._sifields._sigfault._addr = env->nip; > > queue_signal(env, info.si_signo, &info); > > break; > > case POWERPC_EXCP_PIT: /* Programmable interval timer > > IRQ */ > > @@ -2001,9 +2001,9 @@ void cpu_loop(CPUPPCState *env) > > env->gpr[5], env->gpr[6], env- > > >gpr[7], > > env->gpr[8], 0, 0); > > if (ret == -TARGET_ERESTARTSYS) { > > - env->nip -= 4; > > break; > > } > > + env->nip += 4; > > if (ret == (target_ulong)(-TARGET_QEMU_ESIGRETURN)) { > > /* Returning from a successful sigreturn syscall. > > Avoid corrupting register state. */ > > diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c > > index 563c7bc..570d188 100644 > > --- a/target-ppc/excp_helper.c > > +++ b/target-ppc/excp_helper.c > > @@ -198,7 +198,7 @@ static inline void powerpc_excp(PowerPCCPU > > *cpu, int excp_model, int excp) > > default: > > goto excp_invalid; > > } > > - goto store_next; > > + break; > > case POWERPC_EXCP_MCHECK: /* Machine check > > exception */ > > if (msr_me == 0) { > > /* Machine check exception is not enabled. > > @@ -209,7 +209,7 @@ static inline void powerpc_excp(PowerPCCPU > > *cpu, int excp_model, int excp) > > if (qemu_log_separate()) { > > qemu_log("Machine check while not allowed. " > > "Entering checkstop state\n"); > > - } > > + } > > Looks like an accidental whitespace change. > > > > > cs->halted = 1; > > cs->interrupt_request |= CPU_INTERRUPT_EXITTB; > > } > > @@ -235,16 +235,16 @@ static inline void powerpc_excp(PowerPCCPU > > *cpu, int excp_model, int excp) > > default: > > break; > > } > > - goto store_next; > > + break; > > case POWERPC_EXCP_DSI: /* Data storage > > exception */ > > LOG_EXCP("DSI exception: DSISR=" TARGET_FMT_lx" DAR=" > > TARGET_FMT_lx > > "\n", env->spr[SPR_DSISR], env->spr[SPR_DAR]); > > - goto store_next; > > + break; > > case POWERPC_EXCP_ISI: /* Instruction storage > > exception */ > > LOG_EXCP("ISI exception: msr=" TARGET_FMT_lx ", nip=" > > TARGET_FMT_lx > > "\n", msr, env->nip); > > msr |= env->error_code; > > - goto store_next; > > + break; > > case POWERPC_EXCP_EXTERNAL: /* External > > input */ > > cs = CPU(cpu); > > > > @@ -258,13 +258,14 @@ static inline void powerpc_excp(PowerPCCPU > > *cpu, int excp_model, int excp) > > /* IACK the IRQ on delivery */ > > env->spr[SPR_BOOKE_EPR] = ldl_phys(cs->as, env- > > >mpic_iack); > > } > > - goto store_next; > > + break; > > case POWERPC_EXCP_ALIGN: /* Alignment > > exception */ > > /* XXX: this is false */ > > /* Get rS/rD and rA from faulting opcode */ > > - env->spr[SPR_DSISR] |= (cpu_ldl_code(env, (env->nip - 4)) > > + /* Broken for LE mode */ > > + env->spr[SPR_DSISR] |= (cpu_ldl_code(env, env->nip) > > & 0x03FF0000) >> 16; > > - goto store_next; > > + break; > > case POWERPC_EXCP_PROGRAM: /* Program > > exception */ > > switch (env->error_code & ~0xF) { > > case POWERPC_EXCP_FP: > > @@ -280,15 +281,11 @@ static inline void powerpc_excp(PowerPCCPU > > *cpu, int excp_model, int excp) > > * precise in the MSR. > > */ > > msr |= 0x00100000; > > - goto store_next; > > + break; > > case POWERPC_EXCP_INVAL: > > LOG_EXCP("Invalid instruction at " TARGET_FMT_lx "\n", > > env->nip); > > msr |= 0x00080000; > > env->spr[SPR_BOOKE_ESR] = ESR_PIL; > > - /* Some invalids will have the PC in the right place > > already */ > > - if (env->error_code & POWERPC_EXCP_INVAL_LSWX) { > > - goto store_next; > > - } > > break; > > case POWERPC_EXCP_PRIV: > > msr |= 0x00040000; > > @@ -304,23 +301,16 @@ static inline void powerpc_excp(PowerPCCPU > > *cpu, int excp_model, int excp) > > env->error_code); > > break; > > } > > - goto store_current; > > - case POWERPC_EXCP_HV_EMU: > > - srr0 = SPR_HSRR0; > > - srr1 = SPR_HSRR1; > > - new_msr |= (target_ulong)MSR_HVB; > > - new_msr |= env->msr & ((target_ulong)1 << MSR_RI); > > - /* Some invalids will have the PC in the right place > > already */ > > - if (env->error_code == > > (POWERPC_EXCP_INVAL|POWERPC_EXCP_INVAL_LSWX)) { > > - goto store_next; > > - } > > - goto store_current; > > - case POWERPC_EXCP_FPU: /* Floating-point unavailable > > exception */ > > - goto store_current; > > + break; > > case POWERPC_EXCP_SYSCALL: /* System call > > exception */ > > dump_syscall(env); > > lev = env->error_code; > > > > + /* We need to correct the NIP which in this case is > > supposed > > + * to point to the next instruction > > + */ > > + env->nip += 4; > > + > > /* "PAPR mode" built-in hypercall emulation */ > > if ((lev == 1) && cpu_ppc_hypercall) { > > cpu_ppc_hypercall(cpu); > > @@ -329,15 +319,15 @@ static inline void powerpc_excp(PowerPCCPU > > *cpu, int excp_model, int excp) > > if (lev == 1) { > > new_msr |= (target_ulong)MSR_HVB; > > } > > - goto store_next; > > + break; > > + case POWERPC_EXCP_FPU: /* Floating-point unavailable > > exception */ > > case POWERPC_EXCP_APU: /* Auxiliary processor > > unavailable */ > > - goto store_current; > > case POWERPC_EXCP_DECR: /* Decrementer > > exception */ > > - goto store_next; > > + break; > > case POWERPC_EXCP_FIT: /* Fixed-interval timer > > interrupt */ > > /* FIT on 4xx */ > > LOG_EXCP("FIT exception\n"); > > - goto store_next; > > + break; > > case POWERPC_EXCP_WDT: /* Watchdog timer > > interrupt */ > > LOG_EXCP("WDT exception\n"); > > switch (excp_model) { > > @@ -348,11 +338,10 @@ static inline void powerpc_excp(PowerPCCPU > > *cpu, int excp_model, int excp) > > default: > > break; > > } > > - goto store_next; > > + break; > > case POWERPC_EXCP_DTLB: /* Data TLB > > error */ > > - goto store_next; > > case POWERPC_EXCP_ITLB: /* Instruction TLB > > error */ > > - goto store_next; > > + break; > > case POWERPC_EXCP_DEBUG: /* Debug > > interrupt */ > > switch (excp_model) { > > case POWERPC_EXCP_BOOKE: > > @@ -367,33 +356,33 @@ static inline void powerpc_excp(PowerPCCPU > > *cpu, int excp_model, int excp) > > } > > /* XXX: TODO */ > > cpu_abort(cs, "Debug exception is not implemented yet > > !\n"); > > - goto store_next; > > + break; > > case POWERPC_EXCP_SPEU: /* SPE/embedded floating-point > > unavailable */ > > env->spr[SPR_BOOKE_ESR] = ESR_SPV; > > - goto store_current; > > + break; > > case POWERPC_EXCP_EFPDI: /* Embedded floating-point data > > interrupt */ > > /* XXX: TODO */ > > cpu_abort(cs, "Embedded floating point data exception " > > "is not implemented yet !\n"); > > env->spr[SPR_BOOKE_ESR] = ESR_SPV; > > - goto store_next; > > + break; > > case POWERPC_EXCP_EFPRI: /* Embedded floating-point round > > interrupt */ > > /* XXX: TODO */ > > cpu_abort(cs, "Embedded floating point round exception " > > "is not implemented yet !\n"); > > env->spr[SPR_BOOKE_ESR] = ESR_SPV; > > - goto store_next; > > + break; > > case POWERPC_EXCP_EPERFM: /* Embedded performance monitor > > interrupt */ > > /* XXX: TODO */ > > cpu_abort(cs, > > "Performance counter exception is not > > implemented yet !\n"); > > - goto store_next; > > + break; > > case POWERPC_EXCP_DOORI: /* Embedded doorbell > > interrupt */ > > - goto store_next; > > + break; > > case POWERPC_EXCP_DOORCI: /* Embedded doorbell critical > > interrupt */ > > srr0 = SPR_BOOKE_CSRR0; > > srr1 = SPR_BOOKE_CSRR1; > > - goto store_next; > > + break; > > case POWERPC_EXCP_RESET: /* System reset > > exception */ > > if (msr_pow) { > > /* indicate that we resumed from power save mode */ > > @@ -404,65 +393,42 @@ static inline void powerpc_excp(PowerPCCPU > > *cpu, int excp_model, int excp) > > > > new_msr |= (target_ulong)MSR_HVB; > > ail = 0; > > - goto store_next; > > + break; > > case POWERPC_EXCP_DSEG: /* Data segment > > exception */ > > - goto store_next; > > case POWERPC_EXCP_ISEG: /* Instruction segment > > exception */ > > - goto store_next; > > - case POWERPC_EXCP_HDECR: /* Hypervisor decrementer > > exception */ > > - srr0 = SPR_HSRR0; > > - srr1 = SPR_HSRR1; > > - new_msr |= (target_ulong)MSR_HVB; > > - new_msr |= env->msr & ((target_ulong)1 << MSR_RI); > > - goto store_next; > > case POWERPC_EXCP_TRACE: /* Trace > > exception */ > > - goto store_next; > > + break; > > + case POWERPC_EXCP_HDECR: /* Hypervisor decrementer > > exception */ > > case POWERPC_EXCP_HDSI: /* Hypervisor data storage > > exception */ > > - srr0 = SPR_HSRR0; > > - srr1 = SPR_HSRR1; > > - new_msr |= (target_ulong)MSR_HVB; > > - new_msr |= env->msr & ((target_ulong)1 << MSR_RI); > > - goto store_next; > > case POWERPC_EXCP_HISI: /* Hypervisor instruction storage > > exception */ > > - srr0 = SPR_HSRR0; > > - srr1 = SPR_HSRR1; > > - new_msr |= (target_ulong)MSR_HVB; > > - new_msr |= env->msr & ((target_ulong)1 << MSR_RI); > > - goto store_next; > > case POWERPC_EXCP_HDSEG: /* Hypervisor data segment > > exception */ > > - srr0 = SPR_HSRR0; > > - srr1 = SPR_HSRR1; > > - new_msr |= (target_ulong)MSR_HVB; > > - new_msr |= env->msr & ((target_ulong)1 << MSR_RI); > > - goto store_next; > > case POWERPC_EXCP_HISEG: /* Hypervisor instruction segment > > exception */ > > + case POWERPC_EXCP_HV_EMU: > > srr0 = SPR_HSRR0; > > srr1 = SPR_HSRR1; > > new_msr |= (target_ulong)MSR_HVB; > > new_msr |= env->msr & ((target_ulong)1 << MSR_RI); > > - goto store_next; > > + break; > > case POWERPC_EXCP_VPU: /* Vector unavailable > > exception */ > > - goto store_current; > > case POWERPC_EXCP_VSXU: /* VSX unavailable > > exception */ > > - goto store_current; > > case POWERPC_EXCP_FU: /* Facility unavailable > > exception */ > > - goto store_current; > > + break; > > case POWERPC_EXCP_PIT: /* Programmable interval timer > > interrupt */ > > LOG_EXCP("PIT exception\n"); > > - goto store_next; > > + break; > > case POWERPC_EXCP_IO: /* IO error > > exception */ > > /* XXX: TODO */ > > cpu_abort(cs, "601 IO error exception is not implemented > > yet !\n"); > > - goto store_next; > > + break; > > case POWERPC_EXCP_RUNM: /* Run mode > > exception */ > > /* XXX: TODO */ > > cpu_abort(cs, "601 run mode exception is not implemented > > yet !\n"); > > - goto store_next; > > + break; > > case POWERPC_EXCP_EMUL: /* Emulation trap > > exception */ > > /* XXX: TODO */ > > cpu_abort(cs, "602 emulation trap exception " > > "is not implemented yet !\n"); > > - goto store_next; > > + break; > > case POWERPC_EXCP_IFTLB: /* Instruction fetch TLB > > error */ > > switch (excp_model) { > > case POWERPC_EXCP_602: > > @@ -577,71 +543,67 @@ static inline void powerpc_excp(PowerPCCPU > > *cpu, int excp_model, int excp) > > cpu_abort(cs, "Invalid data store TLB miss > > exception\n"); > > break; > > } > > - goto store_next; > > + break; > > case POWERPC_EXCP_FPA: /* Floating-point assist > > exception */ > > /* XXX: TODO */ > > cpu_abort(cs, "Floating point assist exception " > > "is not implemented yet !\n"); > > - goto store_next; > > + break; > > case POWERPC_EXCP_DABR: /* Data address > > breakpoint */ > > /* XXX: TODO */ > > cpu_abort(cs, "DABR exception is not implemented yet > > !\n"); > > - goto store_next; > > + break; > > case POWERPC_EXCP_IABR: /* Instruction address > > breakpoint */ > > /* XXX: TODO */ > > cpu_abort(cs, "IABR exception is not implemented yet > > !\n"); > > - goto store_next; > > + break; > > case POWERPC_EXCP_SMI: /* System management > > interrupt */ > > /* XXX: TODO */ > > cpu_abort(cs, "SMI exception is not implemented yet !\n"); > > - goto store_next; > > + break; > > case POWERPC_EXCP_THERM: /* Thermal > > interrupt */ > > /* XXX: TODO */ > > cpu_abort(cs, "Thermal management exception " > > "is not implemented yet !\n"); > > - goto store_next; > > + break; > > case POWERPC_EXCP_PERFM: /* Embedded performance monitor > > interrupt */ > > /* XXX: TODO */ > > cpu_abort(cs, > > "Performance counter exception is not > > implemented yet !\n"); > > - goto store_next; > > + break; > > case POWERPC_EXCP_VPUA: /* Vector assist > > exception */ > > /* XXX: TODO */ > > cpu_abort(cs, "VPU assist exception is not implemented yet > > !\n"); > > - goto store_next; > > + break; > > case POWERPC_EXCP_SOFTP: /* Soft patch > > exception */ > > /* XXX: TODO */ > > cpu_abort(cs, > > "970 soft-patch exception is not implemented yet > > !\n"); > > - goto store_next; > > + break; > > case POWERPC_EXCP_MAINT: /* Maintenance > > exception */ > > /* XXX: TODO */ > > cpu_abort(cs, > > "970 maintenance exception is not implemented > > yet !\n"); > > - goto store_next; > > + break; > > case POWERPC_EXCP_MEXTBR: /* Maskable external > > breakpoint */ > > /* XXX: TODO */ > > cpu_abort(cs, "Maskable external exception " > > "is not implemented yet !\n"); > > - goto store_next; > > + break; > > case POWERPC_EXCP_NMEXTBR: /* Non maskable external > > breakpoint */ > > /* XXX: TODO */ > > cpu_abort(cs, "Non maskable external exception " > > "is not implemented yet !\n"); > > - goto store_next; > > + break; > > default: > > excp_invalid: > > cpu_abort(cs, "Invalid PowerPC exception %d. Aborting\n", > > excp); > > break; > > - store_current: > > - /* save current instruction location */ > > - env->spr[srr0] = env->nip - 4; > > - break; > > - store_next: > > - /* save next instruction location */ > > - env->spr[srr0] = env->nip; > > - break; > > } > > + > > + /* Save PC */ > > + env->spr[srr0] = env->nip; > > + > > /* Save MSR */ > > env->spr[srr1] = msr; > > > > diff --git a/target-ppc/translate.c b/target-ppc/translate.c > > index c4f8916..84bcb09 100644 > > --- a/target-ppc/translate.c > > +++ b/target-ppc/translate.c > > @@ -276,8 +276,12 @@ void gen_update_current_nip(void *opaque) > > static void gen_exception_err(DisasContext *ctx, uint32_t excp, > > uint32_t error) > > { > > TCGv_i32 t0, t1; > > + > > + /* These are all synchronous exceptions, we set the PC back to > > + * the faulting instruction > > + */ > > if (ctx->exception == POWERPC_EXCP_NONE) { > > - gen_update_nip(ctx, ctx->nip); > > + gen_update_nip(ctx, ctx->nip - 4); > > } > > t0 = tcg_const_i32(excp); > > t1 = tcg_const_i32(error); > > @@ -290,8 +294,12 @@ static void gen_exception_err(DisasContext > > *ctx, uint32_t excp, uint32_t error) > > static void gen_exception(DisasContext *ctx, uint32_t excp) > > { > > TCGv_i32 t0; > > + > > + /* These are all synchronous exceptions, we set the PC back to > > + * the faulting instruction > > + */ > > if (ctx->exception == POWERPC_EXCP_NONE) { > > - gen_update_nip(ctx, ctx->nip); > > + gen_update_nip(ctx, ctx->nip - 4); > > } > > t0 = tcg_const_i32(excp); > > gen_helper_raise_exception(cpu_env, t0); > > @@ -299,13 +307,28 @@ static void gen_exception(DisasContext *ctx, > > uint32_t excp) > > ctx->exception = (excp); > > } > > > > +static void gen_exception_nip(DisasContext *ctx, uint32_t excp, > > + target_ulong nip) > > +{ > > + TCGv_i32 t0; > > + > > + gen_update_nip(ctx, nip); > > + t0 = tcg_const_i32(excp); > > + gen_helper_raise_exception(cpu_env, t0); > > + tcg_temp_free_i32(t0); > > + ctx->exception = (excp); > > +} > > + > > static void gen_debug_exception(DisasContext *ctx) > > { > > TCGv_i32 t0; > > > > + /* These are all synchronous exceptions, we set the PC back to > > + * the faulting instruction > > + */ > > if ((ctx->exception != POWERPC_EXCP_BRANCH) && > > (ctx->exception != POWERPC_EXCP_SYNC)) { > > - gen_update_nip(ctx, ctx->nip); > > + gen_update_nip(ctx, ctx->nip - 4); > > } > > t0 = tcg_const_i32(EXCP_DEBUG); > > gen_helper_raise_exception(cpu_env, t0); > > @@ -1437,7 +1460,7 @@ static void gen_pause(DisasContext *ctx) > > tcg_temp_free_i32(t0); > > > > /* Stop translation, this gives other CPUs a chance to run */ > > - gen_exception_err(ctx, EXCP_HLT, 1); > > + gen_exception_nip(ctx, EXCP_HLT, ctx->nip); > > } > > #endif /* defined(TARGET_PPC64) */ > > > > @@ -2697,12 +2720,6 @@ static void gen_lswi(DisasContext *ctx) > > nb = 32; > > nr = (nb + 3) / 4; > > if (unlikely(lsw_reg_in_range(start, nr, ra))) { > > - /* The handler expects the PC to point to *this* > > instruction, > > - * so setting ctx->exception here prevents it from being > > - * improperly updated again by gen_inval_exception > > - */ > > - gen_update_nip(ctx, ctx->nip - 4); > > - ctx->exception = POWERPC_EXCP_HV_EMU; > > gen_inval_exception(ctx, POWERPC_EXCP_INVAL_LSWX); > > return; > > } > > @@ -2845,10 +2862,7 @@ static void > > gen_conditional_store(DisasContext *ctx, TCGv EA, > > tcg_gen_movi_tl(t0, (size << 5) | reg); > > tcg_gen_st_tl(t0, cpu_env, offsetof(CPUPPCState, > > reserve_info)); > > tcg_temp_free(t0); > > - gen_update_nip(ctx, ctx->nip-4); > > - ctx->exception = POWERPC_EXCP_BRANCH; > > - gen_exception(ctx, POWERPC_EXCP_STCX); > > - ctx->exception = save_exception; > > + gen_exception_err(ctx, POWERPC_EXCP_STCX, 0); > > } > > #else > > static void gen_conditional_store(DisasContext *ctx, TCGv EA, > > @@ -2987,7 +3001,7 @@ static void gen_wait(DisasContext *ctx) > > -offsetof(PowerPCCPU, env) + offsetof(CPUState, > > halted)); > > tcg_temp_free_i32(t0); > > /* Stop translation, as the CPU is supposed to sleep from now > > */ > > - gen_exception_err(ctx, EXCP_HLT, 1); > > + gen_exception_nip(ctx, EXCP_HLT, ctx->nip); > > } > > > > #if defined(TARGET_PPC64) > > @@ -3090,10 +3104,7 @@ static inline void gen_goto_tb(DisasContext > > *ctx, int n, target_ulong dest) > > (CPU_BRANCH_STEP | CPU_SINGLE_STEP)) && > > (ctx->exception == POWERPC_EXCP_BRANCH || > > ctx->exception == POWERPC_EXCP_TRACE)) { > > - target_ulong tmp = ctx->nip; > > - ctx->nip = dest; > > - gen_exception(ctx, POWERPC_EXCP_TRACE); > > - ctx->nip = tmp; > > + gen_exception_nip(ctx, POWERPC_EXCP_TRACE, dest); > > } > > if (ctx->singlestep_enabled & GDBSTUB_SINGLE_STEP) { > > gen_debug_exception(ctx); > > @@ -3362,7 +3373,7 @@ static void gen_tw(DisasContext *ctx) > > { > > TCGv_i32 t0 = tcg_const_i32(TO(ctx->opcode)); > > /* Update the nip since this might generate a trap exception > > */ > > - gen_update_nip(ctx, ctx->nip); > > + gen_update_nip(ctx, ctx->nip - 4); > > twi etc will generally resume from the next instruction if they trap, > yes? In which case I'm a bit confused by the nip - 4. But possibly > I > just haven't correctly followed all the nip update logic changed by > this patch. > > > > > gen_helper_tw(cpu_env, cpu_gpr[rA(ctx->opcode)], > > cpu_gpr[rB(ctx->opcode)], > > t0); > > tcg_temp_free_i32(t0); > > @@ -3374,7 +3385,7 @@ static void gen_twi(DisasContext *ctx) > > TCGv t0 = tcg_const_tl(SIMM(ctx->opcode)); > > TCGv_i32 t1 = tcg_const_i32(TO(ctx->opcode)); > > /* Update the nip since this might generate a trap exception > > */ > > - gen_update_nip(ctx, ctx->nip); > > + gen_update_nip(ctx, ctx->nip - 4); > > gen_helper_tw(cpu_env, cpu_gpr[rA(ctx->opcode)], t0, t1); > > tcg_temp_free(t0); > > tcg_temp_free_i32(t1); > > @@ -3386,7 +3397,7 @@ static void gen_td(DisasContext *ctx) > > { > > TCGv_i32 t0 = tcg_const_i32(TO(ctx->opcode)); > > /* Update the nip since this might generate a trap exception > > */ > > - gen_update_nip(ctx, ctx->nip); > > + gen_update_nip(ctx, ctx->nip - 4); > > gen_helper_td(cpu_env, cpu_gpr[rA(ctx->opcode)], > > cpu_gpr[rB(ctx->opcode)], > > t0); > > tcg_temp_free_i32(t0); > > @@ -3398,7 +3409,7 @@ static void gen_tdi(DisasContext *ctx) > > TCGv t0 = tcg_const_tl(SIMM(ctx->opcode)); > > TCGv_i32 t1 = tcg_const_i32(TO(ctx->opcode)); > > /* Update the nip since this might generate a trap exception > > */ > > - gen_update_nip(ctx, ctx->nip); > > + gen_update_nip(ctx, ctx->nip - 4); > > gen_helper_td(cpu_env, cpu_gpr[rA(ctx->opcode)], t0, t1); > > tcg_temp_free(t0); > > tcg_temp_free_i32(t1); > > @@ -6757,7 +6768,7 @@ void gen_intermediate_code(CPUPPCState *env, > > struct TranslationBlock *tb) > > ctx.exception != POWERPC_SYSCALL && > > ctx.exception != POWERPC_EXCP_TRAP && > > ctx.exception != POWERPC_EXCP_BRANCH)) { > > - gen_exception(ctxp, POWERPC_EXCP_TRACE); > > + gen_exception_nip(ctxp, POWERPC_EXCP_TRACE, ctx.nip); > > } else if (unlikely(((ctx.nip & (TARGET_PAGE_SIZE - 1)) == > > 0) || > > (cs->singlestep_enabled) || > > singlestep || >
On Wed, 2016-07-27 at 12:19 +1000, David Gibson wrote: > twi etc will generally resume from the next instruction if they trap, > yes? In which case I'm a bit confused by the nip - 4. But possibly I > just haven't correctly followed all the nip update logic changed by > this patch. From the ISA (Program Check interrupt) Trap A Trap type Program interrupt is generated when any of the conditions specified in a Trap instruction is met. The following registers are set: SRR0 For all Program interrupts except a Float- ing-Point Enabled Exception type Program interrupt, set to the effective address of the instruction that caused the corresponding exception. Cheers, Ben.
diff --git a/linux-user/main.c b/linux-user/main.c index 462e820..1d149dc 100644 --- a/linux-user/main.c +++ b/linux-user/main.c @@ -1814,7 +1814,7 @@ void cpu_loop(CPUPPCState *env) env->error_code); break; } - info._sifields._sigfault._addr = env->nip - 4; + info._sifields._sigfault._addr = env->nip; queue_signal(env, info.si_signo, &info); break; case POWERPC_EXCP_FPU: /* Floating-point unavailable exception */ @@ -1822,7 +1822,7 @@ void cpu_loop(CPUPPCState *env) info.si_signo = TARGET_SIGILL; info.si_errno = 0; info.si_code = TARGET_ILL_COPROC; - info._sifields._sigfault._addr = env->nip - 4; + info._sifields._sigfault._addr = env->nip; queue_signal(env, info.si_signo, &info); break; case POWERPC_EXCP_SYSCALL: /* System call exception */ @@ -1834,7 +1834,7 @@ void cpu_loop(CPUPPCState *env) info.si_signo = TARGET_SIGILL; info.si_errno = 0; info.si_code = TARGET_ILL_COPROC; - info._sifields._sigfault._addr = env->nip - 4; + info._sifields._sigfault._addr = env->nip; queue_signal(env, info.si_signo, &info); break; case POWERPC_EXCP_DECR: /* Decrementer exception */ @@ -1862,7 +1862,7 @@ void cpu_loop(CPUPPCState *env) info.si_signo = TARGET_SIGILL; info.si_errno = 0; info.si_code = TARGET_ILL_COPROC; - info._sifields._sigfault._addr = env->nip - 4; + info._sifields._sigfault._addr = env->nip; queue_signal(env, info.si_signo, &info); break; case POWERPC_EXCP_EFPDI: /* Embedded floating-point data IRQ */ @@ -1926,7 +1926,7 @@ void cpu_loop(CPUPPCState *env) info.si_signo = TARGET_SIGILL; info.si_errno = 0; info.si_code = TARGET_ILL_COPROC; - info._sifields._sigfault._addr = env->nip - 4; + info._sifields._sigfault._addr = env->nip; queue_signal(env, info.si_signo, &info); break; case POWERPC_EXCP_PIT: /* Programmable interval timer IRQ */ @@ -2001,9 +2001,9 @@ void cpu_loop(CPUPPCState *env) env->gpr[5], env->gpr[6], env->gpr[7], env->gpr[8], 0, 0); if (ret == -TARGET_ERESTARTSYS) { - env->nip -= 4; break; } + env->nip += 4; if (ret == (target_ulong)(-TARGET_QEMU_ESIGRETURN)) { /* Returning from a successful sigreturn syscall. Avoid corrupting register state. */ diff --git a/target-ppc/excp_helper.c b/target-ppc/excp_helper.c index 563c7bc..570d188 100644 --- a/target-ppc/excp_helper.c +++ b/target-ppc/excp_helper.c @@ -198,7 +198,7 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) default: goto excp_invalid; } - goto store_next; + break; case POWERPC_EXCP_MCHECK: /* Machine check exception */ if (msr_me == 0) { /* Machine check exception is not enabled. @@ -209,7 +209,7 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) if (qemu_log_separate()) { qemu_log("Machine check while not allowed. " "Entering checkstop state\n"); - } + } cs->halted = 1; cs->interrupt_request |= CPU_INTERRUPT_EXITTB; } @@ -235,16 +235,16 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) default: break; } - goto store_next; + break; case POWERPC_EXCP_DSI: /* Data storage exception */ LOG_EXCP("DSI exception: DSISR=" TARGET_FMT_lx" DAR=" TARGET_FMT_lx "\n", env->spr[SPR_DSISR], env->spr[SPR_DAR]); - goto store_next; + break; case POWERPC_EXCP_ISI: /* Instruction storage exception */ LOG_EXCP("ISI exception: msr=" TARGET_FMT_lx ", nip=" TARGET_FMT_lx "\n", msr, env->nip); msr |= env->error_code; - goto store_next; + break; case POWERPC_EXCP_EXTERNAL: /* External input */ cs = CPU(cpu); @@ -258,13 +258,14 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) /* IACK the IRQ on delivery */ env->spr[SPR_BOOKE_EPR] = ldl_phys(cs->as, env->mpic_iack); } - goto store_next; + break; case POWERPC_EXCP_ALIGN: /* Alignment exception */ /* XXX: this is false */ /* Get rS/rD and rA from faulting opcode */ - env->spr[SPR_DSISR] |= (cpu_ldl_code(env, (env->nip - 4)) + /* Broken for LE mode */ + env->spr[SPR_DSISR] |= (cpu_ldl_code(env, env->nip) & 0x03FF0000) >> 16; - goto store_next; + break; case POWERPC_EXCP_PROGRAM: /* Program exception */ switch (env->error_code & ~0xF) { case POWERPC_EXCP_FP: @@ -280,15 +281,11 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) * precise in the MSR. */ msr |= 0x00100000; - goto store_next; + break; case POWERPC_EXCP_INVAL: LOG_EXCP("Invalid instruction at " TARGET_FMT_lx "\n", env->nip); msr |= 0x00080000; env->spr[SPR_BOOKE_ESR] = ESR_PIL; - /* Some invalids will have the PC in the right place already */ - if (env->error_code & POWERPC_EXCP_INVAL_LSWX) { - goto store_next; - } break; case POWERPC_EXCP_PRIV: msr |= 0x00040000; @@ -304,23 +301,16 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) env->error_code); break; } - goto store_current; - case POWERPC_EXCP_HV_EMU: - srr0 = SPR_HSRR0; - srr1 = SPR_HSRR1; - new_msr |= (target_ulong)MSR_HVB; - new_msr |= env->msr & ((target_ulong)1 << MSR_RI); - /* Some invalids will have the PC in the right place already */ - if (env->error_code == (POWERPC_EXCP_INVAL|POWERPC_EXCP_INVAL_LSWX)) { - goto store_next; - } - goto store_current; - case POWERPC_EXCP_FPU: /* Floating-point unavailable exception */ - goto store_current; + break; case POWERPC_EXCP_SYSCALL: /* System call exception */ dump_syscall(env); lev = env->error_code; + /* We need to correct the NIP which in this case is supposed + * to point to the next instruction + */ + env->nip += 4; + /* "PAPR mode" built-in hypercall emulation */ if ((lev == 1) && cpu_ppc_hypercall) { cpu_ppc_hypercall(cpu); @@ -329,15 +319,15 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) if (lev == 1) { new_msr |= (target_ulong)MSR_HVB; } - goto store_next; + break; + case POWERPC_EXCP_FPU: /* Floating-point unavailable exception */ case POWERPC_EXCP_APU: /* Auxiliary processor unavailable */ - goto store_current; case POWERPC_EXCP_DECR: /* Decrementer exception */ - goto store_next; + break; case POWERPC_EXCP_FIT: /* Fixed-interval timer interrupt */ /* FIT on 4xx */ LOG_EXCP("FIT exception\n"); - goto store_next; + break; case POWERPC_EXCP_WDT: /* Watchdog timer interrupt */ LOG_EXCP("WDT exception\n"); switch (excp_model) { @@ -348,11 +338,10 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) default: break; } - goto store_next; + break; case POWERPC_EXCP_DTLB: /* Data TLB error */ - goto store_next; case POWERPC_EXCP_ITLB: /* Instruction TLB error */ - goto store_next; + break; case POWERPC_EXCP_DEBUG: /* Debug interrupt */ switch (excp_model) { case POWERPC_EXCP_BOOKE: @@ -367,33 +356,33 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) } /* XXX: TODO */ cpu_abort(cs, "Debug exception is not implemented yet !\n"); - goto store_next; + break; case POWERPC_EXCP_SPEU: /* SPE/embedded floating-point unavailable */ env->spr[SPR_BOOKE_ESR] = ESR_SPV; - goto store_current; + break; case POWERPC_EXCP_EFPDI: /* Embedded floating-point data interrupt */ /* XXX: TODO */ cpu_abort(cs, "Embedded floating point data exception " "is not implemented yet !\n"); env->spr[SPR_BOOKE_ESR] = ESR_SPV; - goto store_next; + break; case POWERPC_EXCP_EFPRI: /* Embedded floating-point round interrupt */ /* XXX: TODO */ cpu_abort(cs, "Embedded floating point round exception " "is not implemented yet !\n"); env->spr[SPR_BOOKE_ESR] = ESR_SPV; - goto store_next; + break; case POWERPC_EXCP_EPERFM: /* Embedded performance monitor interrupt */ /* XXX: TODO */ cpu_abort(cs, "Performance counter exception is not implemented yet !\n"); - goto store_next; + break; case POWERPC_EXCP_DOORI: /* Embedded doorbell interrupt */ - goto store_next; + break; case POWERPC_EXCP_DOORCI: /* Embedded doorbell critical interrupt */ srr0 = SPR_BOOKE_CSRR0; srr1 = SPR_BOOKE_CSRR1; - goto store_next; + break; case POWERPC_EXCP_RESET: /* System reset exception */ if (msr_pow) { /* indicate that we resumed from power save mode */ @@ -404,65 +393,42 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) new_msr |= (target_ulong)MSR_HVB; ail = 0; - goto store_next; + break; case POWERPC_EXCP_DSEG: /* Data segment exception */ - goto store_next; case POWERPC_EXCP_ISEG: /* Instruction segment exception */ - goto store_next; - case POWERPC_EXCP_HDECR: /* Hypervisor decrementer exception */ - srr0 = SPR_HSRR0; - srr1 = SPR_HSRR1; - new_msr |= (target_ulong)MSR_HVB; - new_msr |= env->msr & ((target_ulong)1 << MSR_RI); - goto store_next; case POWERPC_EXCP_TRACE: /* Trace exception */ - goto store_next; + break; + case POWERPC_EXCP_HDECR: /* Hypervisor decrementer exception */ case POWERPC_EXCP_HDSI: /* Hypervisor data storage exception */ - srr0 = SPR_HSRR0; - srr1 = SPR_HSRR1; - new_msr |= (target_ulong)MSR_HVB; - new_msr |= env->msr & ((target_ulong)1 << MSR_RI); - goto store_next; case POWERPC_EXCP_HISI: /* Hypervisor instruction storage exception */ - srr0 = SPR_HSRR0; - srr1 = SPR_HSRR1; - new_msr |= (target_ulong)MSR_HVB; - new_msr |= env->msr & ((target_ulong)1 << MSR_RI); - goto store_next; case POWERPC_EXCP_HDSEG: /* Hypervisor data segment exception */ - srr0 = SPR_HSRR0; - srr1 = SPR_HSRR1; - new_msr |= (target_ulong)MSR_HVB; - new_msr |= env->msr & ((target_ulong)1 << MSR_RI); - goto store_next; case POWERPC_EXCP_HISEG: /* Hypervisor instruction segment exception */ + case POWERPC_EXCP_HV_EMU: srr0 = SPR_HSRR0; srr1 = SPR_HSRR1; new_msr |= (target_ulong)MSR_HVB; new_msr |= env->msr & ((target_ulong)1 << MSR_RI); - goto store_next; + break; case POWERPC_EXCP_VPU: /* Vector unavailable exception */ - goto store_current; case POWERPC_EXCP_VSXU: /* VSX unavailable exception */ - goto store_current; case POWERPC_EXCP_FU: /* Facility unavailable exception */ - goto store_current; + break; case POWERPC_EXCP_PIT: /* Programmable interval timer interrupt */ LOG_EXCP("PIT exception\n"); - goto store_next; + break; case POWERPC_EXCP_IO: /* IO error exception */ /* XXX: TODO */ cpu_abort(cs, "601 IO error exception is not implemented yet !\n"); - goto store_next; + break; case POWERPC_EXCP_RUNM: /* Run mode exception */ /* XXX: TODO */ cpu_abort(cs, "601 run mode exception is not implemented yet !\n"); - goto store_next; + break; case POWERPC_EXCP_EMUL: /* Emulation trap exception */ /* XXX: TODO */ cpu_abort(cs, "602 emulation trap exception " "is not implemented yet !\n"); - goto store_next; + break; case POWERPC_EXCP_IFTLB: /* Instruction fetch TLB error */ switch (excp_model) { case POWERPC_EXCP_602: @@ -577,71 +543,67 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) cpu_abort(cs, "Invalid data store TLB miss exception\n"); break; } - goto store_next; + break; case POWERPC_EXCP_FPA: /* Floating-point assist exception */ /* XXX: TODO */ cpu_abort(cs, "Floating point assist exception " "is not implemented yet !\n"); - goto store_next; + break; case POWERPC_EXCP_DABR: /* Data address breakpoint */ /* XXX: TODO */ cpu_abort(cs, "DABR exception is not implemented yet !\n"); - goto store_next; + break; case POWERPC_EXCP_IABR: /* Instruction address breakpoint */ /* XXX: TODO */ cpu_abort(cs, "IABR exception is not implemented yet !\n"); - goto store_next; + break; case POWERPC_EXCP_SMI: /* System management interrupt */ /* XXX: TODO */ cpu_abort(cs, "SMI exception is not implemented yet !\n"); - goto store_next; + break; case POWERPC_EXCP_THERM: /* Thermal interrupt */ /* XXX: TODO */ cpu_abort(cs, "Thermal management exception " "is not implemented yet !\n"); - goto store_next; + break; case POWERPC_EXCP_PERFM: /* Embedded performance monitor interrupt */ /* XXX: TODO */ cpu_abort(cs, "Performance counter exception is not implemented yet !\n"); - goto store_next; + break; case POWERPC_EXCP_VPUA: /* Vector assist exception */ /* XXX: TODO */ cpu_abort(cs, "VPU assist exception is not implemented yet !\n"); - goto store_next; + break; case POWERPC_EXCP_SOFTP: /* Soft patch exception */ /* XXX: TODO */ cpu_abort(cs, "970 soft-patch exception is not implemented yet !\n"); - goto store_next; + break; case POWERPC_EXCP_MAINT: /* Maintenance exception */ /* XXX: TODO */ cpu_abort(cs, "970 maintenance exception is not implemented yet !\n"); - goto store_next; + break; case POWERPC_EXCP_MEXTBR: /* Maskable external breakpoint */ /* XXX: TODO */ cpu_abort(cs, "Maskable external exception " "is not implemented yet !\n"); - goto store_next; + break; case POWERPC_EXCP_NMEXTBR: /* Non maskable external breakpoint */ /* XXX: TODO */ cpu_abort(cs, "Non maskable external exception " "is not implemented yet !\n"); - goto store_next; + break; default: excp_invalid: cpu_abort(cs, "Invalid PowerPC exception %d. Aborting\n", excp); break; - store_current: - /* save current instruction location */ - env->spr[srr0] = env->nip - 4; - break; - store_next: - /* save next instruction location */ - env->spr[srr0] = env->nip; - break; } + + /* Save PC */ + env->spr[srr0] = env->nip; + /* Save MSR */ env->spr[srr1] = msr; diff --git a/target-ppc/translate.c b/target-ppc/translate.c index c4f8916..84bcb09 100644 --- a/target-ppc/translate.c +++ b/target-ppc/translate.c @@ -276,8 +276,12 @@ void gen_update_current_nip(void *opaque) static void gen_exception_err(DisasContext *ctx, uint32_t excp, uint32_t error) { TCGv_i32 t0, t1; + + /* These are all synchronous exceptions, we set the PC back to + * the faulting instruction + */ if (ctx->exception == POWERPC_EXCP_NONE) { - gen_update_nip(ctx, ctx->nip); + gen_update_nip(ctx, ctx->nip - 4); } t0 = tcg_const_i32(excp); t1 = tcg_const_i32(error); @@ -290,8 +294,12 @@ static void gen_exception_err(DisasContext *ctx, uint32_t excp, uint32_t error) static void gen_exception(DisasContext *ctx, uint32_t excp) { TCGv_i32 t0; + + /* These are all synchronous exceptions, we set the PC back to + * the faulting instruction + */ if (ctx->exception == POWERPC_EXCP_NONE) { - gen_update_nip(ctx, ctx->nip); + gen_update_nip(ctx, ctx->nip - 4); } t0 = tcg_const_i32(excp); gen_helper_raise_exception(cpu_env, t0); @@ -299,13 +307,28 @@ static void gen_exception(DisasContext *ctx, uint32_t excp) ctx->exception = (excp); } +static void gen_exception_nip(DisasContext *ctx, uint32_t excp, + target_ulong nip) +{ + TCGv_i32 t0; + + gen_update_nip(ctx, nip); + t0 = tcg_const_i32(excp); + gen_helper_raise_exception(cpu_env, t0); + tcg_temp_free_i32(t0); + ctx->exception = (excp); +} + static void gen_debug_exception(DisasContext *ctx) { TCGv_i32 t0; + /* These are all synchronous exceptions, we set the PC back to + * the faulting instruction + */ if ((ctx->exception != POWERPC_EXCP_BRANCH) && (ctx->exception != POWERPC_EXCP_SYNC)) { - gen_update_nip(ctx, ctx->nip); + gen_update_nip(ctx, ctx->nip - 4); } t0 = tcg_const_i32(EXCP_DEBUG); gen_helper_raise_exception(cpu_env, t0); @@ -1437,7 +1460,7 @@ static void gen_pause(DisasContext *ctx) tcg_temp_free_i32(t0); /* Stop translation, this gives other CPUs a chance to run */ - gen_exception_err(ctx, EXCP_HLT, 1); + gen_exception_nip(ctx, EXCP_HLT, ctx->nip); } #endif /* defined(TARGET_PPC64) */ @@ -2697,12 +2720,6 @@ static void gen_lswi(DisasContext *ctx) nb = 32; nr = (nb + 3) / 4; if (unlikely(lsw_reg_in_range(start, nr, ra))) { - /* The handler expects the PC to point to *this* instruction, - * so setting ctx->exception here prevents it from being - * improperly updated again by gen_inval_exception - */ - gen_update_nip(ctx, ctx->nip - 4); - ctx->exception = POWERPC_EXCP_HV_EMU; gen_inval_exception(ctx, POWERPC_EXCP_INVAL_LSWX); return; } @@ -2845,10 +2862,7 @@ static void gen_conditional_store(DisasContext *ctx, TCGv EA, tcg_gen_movi_tl(t0, (size << 5) | reg); tcg_gen_st_tl(t0, cpu_env, offsetof(CPUPPCState, reserve_info)); tcg_temp_free(t0); - gen_update_nip(ctx, ctx->nip-4); - ctx->exception = POWERPC_EXCP_BRANCH; - gen_exception(ctx, POWERPC_EXCP_STCX); - ctx->exception = save_exception; + gen_exception_err(ctx, POWERPC_EXCP_STCX, 0); } #else static void gen_conditional_store(DisasContext *ctx, TCGv EA, @@ -2987,7 +3001,7 @@ static void gen_wait(DisasContext *ctx) -offsetof(PowerPCCPU, env) + offsetof(CPUState, halted)); tcg_temp_free_i32(t0); /* Stop translation, as the CPU is supposed to sleep from now */ - gen_exception_err(ctx, EXCP_HLT, 1); + gen_exception_nip(ctx, EXCP_HLT, ctx->nip); } #if defined(TARGET_PPC64) @@ -3090,10 +3104,7 @@ static inline void gen_goto_tb(DisasContext *ctx, int n, target_ulong dest) (CPU_BRANCH_STEP | CPU_SINGLE_STEP)) && (ctx->exception == POWERPC_EXCP_BRANCH || ctx->exception == POWERPC_EXCP_TRACE)) { - target_ulong tmp = ctx->nip; - ctx->nip = dest; - gen_exception(ctx, POWERPC_EXCP_TRACE); - ctx->nip = tmp; + gen_exception_nip(ctx, POWERPC_EXCP_TRACE, dest); } if (ctx->singlestep_enabled & GDBSTUB_SINGLE_STEP) { gen_debug_exception(ctx); @@ -3362,7 +3373,7 @@ static void gen_tw(DisasContext *ctx) { TCGv_i32 t0 = tcg_const_i32(TO(ctx->opcode)); /* Update the nip since this might generate a trap exception */ - gen_update_nip(ctx, ctx->nip); + gen_update_nip(ctx, ctx->nip - 4); gen_helper_tw(cpu_env, cpu_gpr[rA(ctx->opcode)], cpu_gpr[rB(ctx->opcode)], t0); tcg_temp_free_i32(t0); @@ -3374,7 +3385,7 @@ static void gen_twi(DisasContext *ctx) TCGv t0 = tcg_const_tl(SIMM(ctx->opcode)); TCGv_i32 t1 = tcg_const_i32(TO(ctx->opcode)); /* Update the nip since this might generate a trap exception */ - gen_update_nip(ctx, ctx->nip); + gen_update_nip(ctx, ctx->nip - 4); gen_helper_tw(cpu_env, cpu_gpr[rA(ctx->opcode)], t0, t1); tcg_temp_free(t0); tcg_temp_free_i32(t1); @@ -3386,7 +3397,7 @@ static void gen_td(DisasContext *ctx) { TCGv_i32 t0 = tcg_const_i32(TO(ctx->opcode)); /* Update the nip since this might generate a trap exception */ - gen_update_nip(ctx, ctx->nip); + gen_update_nip(ctx, ctx->nip - 4); gen_helper_td(cpu_env, cpu_gpr[rA(ctx->opcode)], cpu_gpr[rB(ctx->opcode)], t0); tcg_temp_free_i32(t0); @@ -3398,7 +3409,7 @@ static void gen_tdi(DisasContext *ctx) TCGv t0 = tcg_const_tl(SIMM(ctx->opcode)); TCGv_i32 t1 = tcg_const_i32(TO(ctx->opcode)); /* Update the nip since this might generate a trap exception */ - gen_update_nip(ctx, ctx->nip); + gen_update_nip(ctx, ctx->nip - 4); gen_helper_td(cpu_env, cpu_gpr[rA(ctx->opcode)], t0, t1); tcg_temp_free(t0); tcg_temp_free_i32(t1); @@ -6757,7 +6768,7 @@ void gen_intermediate_code(CPUPPCState *env, struct TranslationBlock *tb) ctx.exception != POWERPC_SYSCALL && ctx.exception != POWERPC_EXCP_TRAP && ctx.exception != POWERPC_EXCP_BRANCH)) { - gen_exception(ctxp, POWERPC_EXCP_TRACE); + gen_exception_nip(ctxp, POWERPC_EXCP_TRACE, ctx.nip); } else if (unlikely(((ctx.nip & (TARGET_PAGE_SIZE - 1)) == 0) || (cs->singlestep_enabled) || singlestep ||
We make env->nip almost always point to the faulting instruction, thus avoiding a mess of "store_current" vs "store_next" in the exception handling. The syscall exception knows to move the PC by 4 and that's really about it. This actually fixes a number of cases where the translator was setting env->nip to ctx->nip - 4 (ie. the *current* instruction) but the program check exception handler would branch to "store_current" which applies another -4 offset. Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> --- linux-user/main.c | 12 ++-- target-ppc/excp_helper.c | 148 ++++++++++++++++++----------------------------- target-ppc/translate.c | 59 +++++++++++-------- 3 files changed, 96 insertions(+), 123 deletions(-)