On 6 October 2014 10:46, Peter Maydell <peter.mayd...@linaro.org> wrote:
> On 30 September 2014 22:49, Greg Bellows <greg.bell...@linaro.org> wrote: > > From: Fabian Aggeler <aggel...@ethz.ch> > > > > Implements SMC instruction in Aarch32 using the A32 syndrome. When > executing > > SMC instruction from monitor CPU mode SCR.NS bit is reset. > > > > Signed-off-by: Sergey Fedorov <s.fedo...@samsung.com> > > Signed-off-by: Fabian Aggeler <aggel...@ethz.ch> > > Signed-off-by: Greg Bellows <greg.bell...@linaro.org> > > > > ---------- > > v4 -> v5 > > - Merge pre_smc upstream changes and incorporated ss_advance > > --- > > target-arm/helper.c | 11 +++++++++++ > > target-arm/internals.h | 7 ++++++- > > target-arm/op_helper.c | 3 +-- > > target-arm/translate.c | 39 +++++++++++++++++++++++++++++---------- > > 4 files changed, 47 insertions(+), 13 deletions(-) > > > > diff --git a/target-arm/helper.c b/target-arm/helper.c > > index 2381e6f..7f3f049 100644 > > --- a/target-arm/helper.c > > +++ b/target-arm/helper.c > > @@ -4090,6 +4090,12 @@ void arm_cpu_do_interrupt(CPUState *cs) > > mask = CPSR_A | CPSR_I | CPSR_F; > > offset = 4; > > break; > > + case EXCP_SMC: > > + new_mode = ARM_CPU_MODE_MON; > > + addr = 0x08; > > + mask = CPSR_A | CPSR_I | CPSR_F; > > + offset = 0; > > + break; > > default: > > cpu_abort(cs, "Unhandled exception 0x%x\n", > cs->exception_index); > > return; /* Never happens. Keep compiler happy. */ > > @@ -4108,6 +4114,11 @@ void arm_cpu_do_interrupt(CPUState *cs) > > */ > > addr += env->cp15.vbar_el[1]; > > } > > + > > + if ((env->uncached_cpsr & CPSR_M) == ARM_CPU_MODE_MON) { > > + env->cp15.scr_el3 &= ~SCR_NS; > > + } > > + > > switch_mode (env, new_mode); > > /* For exceptions taken to AArch32 we must clear the SS bit in both > > * PSTATE and in the old-state value we save to SPSR_<mode>, so > zero it now. > > diff --git a/target-arm/internals.h b/target-arm/internals.h > > index fd69a83..43a2e7d 100644 > > --- a/target-arm/internals.h > > +++ b/target-arm/internals.h > > @@ -236,7 +236,12 @@ static inline uint32_t syn_aa32_svc(uint32_t imm16, > bool is_thumb) > > | (is_thumb ? 0 : ARM_EL_IL); > > } > > > > -static inline uint32_t syn_aa64_bkpt(uint32_t imm16) > > +static inline uint32_t syn_aa32_smc(void) > > +{ > > + return (EC_AA32_SMC << ARM_EL_EC_SHIFT) | ARM_EL_IL; > > +} > > + > > +static inline uint32_t syn_aa64_bkpt(uint16_t imm16) > > Bogus change (probably introduced by accident in a merge > conflict fixup). > Fixed in v6. > > > { > > return (EC_AA64_BKPT << ARM_EL_EC_SHIFT) | ARM_EL_IL | (imm16 & > 0xffff); > > } > > diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c > > index 0809d63..8ed8ee9 100644 > > --- a/target-arm/op_helper.c > > +++ b/target-arm/op_helper.c > > @@ -419,8 +419,7 @@ void HELPER(pre_hvc)(CPUARMState *env) > > void HELPER(pre_smc)(CPUARMState *env, uint32_t syndrome) > > { > > int cur_el = arm_current_el(env); > > - /* FIXME: Use real secure state. */ > > - bool secure = false; > > + bool secure = arm_is_secure(env);; > > Doubled semicolon. > Fixed in v6 > > > bool smd = env->cp15.scr_el3 & SCR_SMD; > > /* On ARMv8 AArch32, SMD only applies to NS state. > > * On ARMv7 SMD only applies to NS state and only if EL2 is > available. > > diff --git a/target-arm/translate.c b/target-arm/translate.c > > index f6404be..3f3ddfb 100644 > > --- a/target-arm/translate.c > > +++ b/target-arm/translate.c > > @@ -7872,15 +7872,27 @@ static void disas_arm_insn(CPUARMState * env, > DisasContext *s) > > case 7: > > { > > int imm16 = extract32(insn, 0, 4) | (extract32(insn, 8, 12) > << 4); > > - /* SMC instruction (op1 == 3) > > - and undefined instructions (op1 == 0 || op1 == 2) > > - will trap */ > > - if (op1 != 1) { > > + if (op1 == 1) { > > + /* bkpt */ > > + ARCH(5); > > + gen_exception_insn(s, 4, EXCP_BKPT, > > + syn_aa32_bkpt(imm16, false)); > > + } else if (op1 == 3) { > > + /* smi/smc */ > > + if (!arm_dc_feature(s, ARM_FEATURE_EL3) || > > + s->current_pl == 0) { > > + goto illegal_op; > > + } > > + gen_set_pc_im(s, s->pc); > > This should be s->pc - 4, because if the pre_smc helper throws > an UNDEF exception you want the PC to point to the SMC insn, > not the insn after. (If the SMC executes as an SMC then the > PC for the EXCP_SMC will be correct because of the 0 offset > passed to gen_exception_insn below.) > Fixed in v6 > > > + tmp = tcg_const_i32(syn_aa32_smc()); > > + gen_helper_pre_smc(cpu_env, tmp); > > + tcg_temp_free_i32(tmp); > > + gen_ss_advance(s); > > + gen_exception_insn(s, 0, EXCP_SMC, syn_aa32_smc()); > > + break; > > + } else { > > goto illegal_op; > > } > > That said, I have a feeling we might want to put the SMC > handling into the end-of-loop processing for A32/T32, > the same way we do SVC. Ard has a patch onlist which does > that, though it is on my todo list to try to fix because > it has its own issues... > I'll leave it as-is for now because it appears to work. In the meantime, I'll hunt for Ard's patch... unless you have a pointer and can save me the trouble. > > -- PMM >