| Message ID | 20210818191920.390759-10-richard.henderson@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Unaligned access for user-only | expand |
On Wed, 18 Aug 2021 at 20:24, Richard Henderson <richard.henderson@linaro.org> wrote: > > By doing this while sending the exception, we will have already > done the unwinding, which makes the ppc_cpu_do_unaligned_access > code a bit cleaner. > > Update the comment about the expected instruction format. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > target/ppc/excp_helper.c | 21 +++++++++------------ > 1 file changed, 9 insertions(+), 12 deletions(-) > > diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c > index a79a0ed465..0df358f93f 100644 > --- a/target/ppc/excp_helper.c > +++ b/target/ppc/excp_helper.c > @@ -478,13 +478,15 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) > break; > } > case POWERPC_EXCP_ALIGN: /* Alignment exception */ > - /* Get rS/rD and rA from faulting opcode */ > /* > - * Note: the opcode fields will not be set properly for a > - * direct store load/store, but nobody cares as nobody > - * actually uses direct store segments. > + * Get rS/rD and rA from faulting opcode. > + * Note: We will only invoke ALIGN for atomic operations, > + * so all instructions are X-form. > */ > - env->spr[SPR_DSISR] |= (env->error_code & 0x03FF0000) >> 16; > + { > + uint32_t insn = cpu_ldl_code(env, env->nip); > + env->spr[SPR_DSISR] |= (insn & 0x03FF0000) >> 16; > + } > break; > case POWERPC_EXCP_PROGRAM: /* Program exception */ > switch (env->error_code & ~0xF) { > @@ -1501,14 +1503,9 @@ void ppc_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr, > int mmu_idx, uintptr_t retaddr) > { > CPUPPCState *env = cs->env_ptr; > - uint32_t insn; > - > - /* Restore state and reload the insn we executed, for filling in DSISR. */ > - cpu_restore_state(cs, retaddr, true); > - insn = cpu_ldl_code(env, env->nip); > > cs->exception_index = POWERPC_EXCP_ALIGN; > - env->error_code = insn & 0x03FF0000; > - cpu_loop_exit(cs); > + env->error_code = 0; > + cpu_loop_exit_restore(cs, retaddr); cpu_ldl_code() in the unaligned-access handler is strictly speaking bogus, because the page might have been unmapped after translation but before we got round to actually running it. Better would be to stash the relevant bits of info from the insn in the insn_start param, the way Arm does with the syndrome info. But it's the way we currently implement this, so Reviewed-by: Peter Maydell <peter.maydell@linaro.org> thanks -- PMM
On 8/19/21 5:39 AM, Peter Maydell wrote: > cpu_ldl_code() in the unaligned-access handler is strictly speaking > bogus, because the page might have been unmapped after translation > but before we got round to actually running it. Better would be to > stash the relevant bits of info from the insn in the insn_start param, > the way Arm does with the syndrome info. Yep. That was more than I was prepared to do here. r~
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c index a79a0ed465..0df358f93f 100644 --- a/target/ppc/excp_helper.c +++ b/target/ppc/excp_helper.c @@ -478,13 +478,15 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp) break; } case POWERPC_EXCP_ALIGN: /* Alignment exception */ - /* Get rS/rD and rA from faulting opcode */ /* - * Note: the opcode fields will not be set properly for a - * direct store load/store, but nobody cares as nobody - * actually uses direct store segments. + * Get rS/rD and rA from faulting opcode. + * Note: We will only invoke ALIGN for atomic operations, + * so all instructions are X-form. */ - env->spr[SPR_DSISR] |= (env->error_code & 0x03FF0000) >> 16; + { + uint32_t insn = cpu_ldl_code(env, env->nip); + env->spr[SPR_DSISR] |= (insn & 0x03FF0000) >> 16; + } break; case POWERPC_EXCP_PROGRAM: /* Program exception */ switch (env->error_code & ~0xF) { @@ -1501,14 +1503,9 @@ void ppc_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr, int mmu_idx, uintptr_t retaddr) { CPUPPCState *env = cs->env_ptr; - uint32_t insn; - - /* Restore state and reload the insn we executed, for filling in DSISR. */ - cpu_restore_state(cs, retaddr, true); - insn = cpu_ldl_code(env, env->nip); cs->exception_index = POWERPC_EXCP_ALIGN; - env->error_code = insn & 0x03FF0000; - cpu_loop_exit(cs); + env->error_code = 0; + cpu_loop_exit_restore(cs, retaddr); } #endif
By doing this while sending the exception, we will have already done the unwinding, which makes the ppc_cpu_do_unaligned_access code a bit cleaner. Update the comment about the expected instruction format. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- target/ppc/excp_helper.c | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) -- 2.25.1