Message ID | 1506092407-26985-19-git-send-email-peter.maydell@linaro.org |
---|---|
State | New |
Headers | show |
Series | ARM v8M: exception entry, exit and security | expand |
On 09/22/2017 12:00 PM, Peter Maydell wrote: > Implement the BLXNS instruction, which allows secure code to > call non-secure code. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Acked-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > --- > target/arm/helper.h | 1 + > target/arm/internals.h | 1 + > target/arm/helper.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++ > target/arm/translate.c | 17 +++++++++++++-- > 4 files changed, 76 insertions(+), 2 deletions(-) > > diff --git a/target/arm/helper.h b/target/arm/helper.h > index 64afbac..2cf6f74 100644 > --- a/target/arm/helper.h > +++ b/target/arm/helper.h > @@ -64,6 +64,7 @@ DEF_HELPER_3(v7m_msr, void, env, i32, i32) > DEF_HELPER_2(v7m_mrs, i32, env, i32) > > DEF_HELPER_2(v7m_bxns, void, env, i32) > +DEF_HELPER_2(v7m_blxns, void, env, i32) > > DEF_HELPER_4(access_check_cp_reg, void, env, ptr, i32, i32) > DEF_HELPER_3(set_cp_reg, void, env, ptr, i32) > diff --git a/target/arm/internals.h b/target/arm/internals.h > index fd9a7e8..1746737 100644 > --- a/target/arm/internals.h > +++ b/target/arm/internals.h > @@ -60,6 +60,7 @@ static inline bool excp_is_internal(int excp) > FIELD(V7M_CONTROL, NPRIV, 0, 1) > FIELD(V7M_CONTROL, SPSEL, 1, 1) > FIELD(V7M_CONTROL, FPCA, 2, 1) > +FIELD(V7M_CONTROL, SFPA, 3, 1) > > /* Bit definitions for v7M exception return payload */ > FIELD(V7M_EXCRET, ES, 0, 1) > diff --git a/target/arm/helper.c b/target/arm/helper.c > index 8df819d..30dc2a9 100644 > --- a/target/arm/helper.c > +++ b/target/arm/helper.c > @@ -5890,6 +5890,12 @@ void HELPER(v7m_bxns)(CPUARMState *env, uint32_t dest) > g_assert_not_reached(); > } > > +void HELPER(v7m_blxns)(CPUARMState *env, uint32_t dest) > +{ > + /* translate.c should never generate calls here in user-only mode */ > + g_assert_not_reached(); > +} > + > void switch_mode(CPUARMState *env, int mode) > { > ARMCPU *cpu = arm_env_get_cpu(env); > @@ -6182,6 +6188,59 @@ void HELPER(v7m_bxns)(CPUARMState *env, uint32_t dest) > env->regs[15] = dest & ~1; > } > > +void HELPER(v7m_blxns)(CPUARMState *env, uint32_t dest) > +{ > + /* Handle v7M BLXNS: > + * - bit 0 of the destination address is the target security state > + */ > + > + /* At this point regs[15] is the address just after the BLXNS */ > + uint32_t nextinst = env->regs[15] | 1; > + uint32_t sp = env->regs[13] - 8; > + uint32_t saved_psr; > + > + /* translate.c will have made BLXNS UNDEF unless we're secure */ > + assert(env->v7m.secure); > + > + if (dest & 1) { > + /* target is Secure, so this is just a normal BLX, > + * except that the low bit doesn't indicate Thumb/not. > + */ > + env->regs[14] = nextinst; > + env->thumb = 1; > + env->regs[15] = dest & ~1; > + return; > + } > + > + /* Target is non-secure: first push a stack frame */ > + if (!QEMU_IS_ALIGNED(sp, 8)) { > + qemu_log_mask(LOG_GUEST_ERROR, > + "BLXNS with misaligned SP is UNPREDICTABLE\n"); > + } > + > + saved_psr = env->v7m.exception; > + if (env->v7m.control[M_REG_S] & R_V7M_CONTROL_SFPA_MASK) { > + saved_psr |= XPSR_SFPA; > + } > + > + /* Note that these stores can throw exceptions on MPU faults */ > + cpu_stl_data(env, sp, nextinst); > + cpu_stl_data(env, sp + 4, saved_psr); > + > + env->regs[13] = sp; > + env->regs[14] = 0xfeffffff; > + if (arm_v7m_is_handler_mode(env)) { > + /* Write a dummy value to IPSR, to avoid leaking the current secure > + * exception number to non-secure code. This is guaranteed not > + * to cause write_v7m_exception() to actually change stacks. > + */ > + write_v7m_exception(env, 1); > + } > + switch_v7m_security_state(env, dest & 1); > + env->thumb = 1; > + env->regs[15] = dest & ~1; > +} > + > static uint32_t *get_v7m_sp_ptr(CPUARMState *env, bool secure, bool threadmode, > bool spsel) > { > diff --git a/target/arm/translate.c b/target/arm/translate.c > index ab1a12a..53694bb 100644 > --- a/target/arm/translate.c > +++ b/target/arm/translate.c > @@ -1013,6 +1013,20 @@ static inline void gen_bxns(DisasContext *s, int rm) > s->base.is_jmp = DISAS_EXIT; > } > > +static inline void gen_blxns(DisasContext *s, int rm) > +{ > + TCGv_i32 var = load_reg(s, rm); > + > + /* We don't need to sync condexec state, for the same reason as blxns. > + * We do however need to set the PC, because the blxns helper reads it. > + * The blxns helper may throw an exception. > + */ > + gen_set_pc_im(s, s->pc); > + gen_helper_v7m_blxns(cpu_env, var); > + tcg_temp_free_i32(var); > + s->base.is_jmp = DISAS_EXIT; > +} > + > /* Variant of store_reg which uses branch&exchange logic when storing > to r15 in ARM architecture v7 and above. The source must be a temporary > and will be marked as dead. */ > @@ -11221,8 +11235,7 @@ static void disas_thumb_insn(CPUARMState *env, DisasContext *s) > goto undef; > } > if (link) { > - /* BLXNS: not yet implemented */ > - goto undef; > + gen_blxns(s, rm); > } else { > gen_bxns(s, rm); > } >
On 09/22/2017 11:00 AM, Peter Maydell wrote: > +void HELPER(v7m_blxns)(CPUARMState *env, uint32_t dest) > +{ ... > + if (dest & 1) { > + /* target is Secure, so this is just a normal BLX, > + * except that the low bit doesn't indicate Thumb/not. > + */ > + env->regs[14] = nextinst; > + env->thumb = 1; > + env->regs[15] = dest & ~1; > + return; > + } ... > + switch_v7m_security_state(env, dest & 1); > + env->thumb = 1; > + env->regs[15] = dest & ~1; dest & 1 is known to be 0. > +static inline void gen_blxns(DisasContext *s, int rm) > +{ > + TCGv_i32 var = load_reg(s, rm); > + > + /* We don't need to sync condexec state, for the same reason as blxns. s/blxns/bxns/ ? Otherwise, Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
On 5 October 2017 at 19:56, Richard Henderson <richard.henderson@linaro.org> wrote: > On 09/22/2017 11:00 AM, Peter Maydell wrote: >> +void HELPER(v7m_blxns)(CPUARMState *env, uint32_t dest) >> +{ > ... >> + if (dest & 1) { >> + /* target is Secure, so this is just a normal BLX, >> + * except that the low bit doesn't indicate Thumb/not. >> + */ >> + env->regs[14] = nextinst; >> + env->thumb = 1; >> + env->regs[15] = dest & ~1; >> + return; >> + } > ... >> + switch_v7m_security_state(env, dest & 1); >> + env->thumb = 1; >> + env->regs[15] = dest & ~1; > > dest & 1 is known to be 0. Yes. I liked the symmetry with the tail end of the v7m_bxns helper, which is conceptually doing the same thing, and assumed the compiler would be smart enough not to generate unnecessary code. >> +static inline void gen_blxns(DisasContext *s, int rm) >> +{ >> + TCGv_i32 var = load_reg(s, rm); >> + >> + /* We don't need to sync condexec state, for the same reason as blxns. > > s/blxns/bxns/ ? Yes. thanks -- PMM
diff --git a/target/arm/helper.h b/target/arm/helper.h index 64afbac..2cf6f74 100644 --- a/target/arm/helper.h +++ b/target/arm/helper.h @@ -64,6 +64,7 @@ DEF_HELPER_3(v7m_msr, void, env, i32, i32) DEF_HELPER_2(v7m_mrs, i32, env, i32) DEF_HELPER_2(v7m_bxns, void, env, i32) +DEF_HELPER_2(v7m_blxns, void, env, i32) DEF_HELPER_4(access_check_cp_reg, void, env, ptr, i32, i32) DEF_HELPER_3(set_cp_reg, void, env, ptr, i32) diff --git a/target/arm/internals.h b/target/arm/internals.h index fd9a7e8..1746737 100644 --- a/target/arm/internals.h +++ b/target/arm/internals.h @@ -60,6 +60,7 @@ static inline bool excp_is_internal(int excp) FIELD(V7M_CONTROL, NPRIV, 0, 1) FIELD(V7M_CONTROL, SPSEL, 1, 1) FIELD(V7M_CONTROL, FPCA, 2, 1) +FIELD(V7M_CONTROL, SFPA, 3, 1) /* Bit definitions for v7M exception return payload */ FIELD(V7M_EXCRET, ES, 0, 1) diff --git a/target/arm/helper.c b/target/arm/helper.c index 8df819d..30dc2a9 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -5890,6 +5890,12 @@ void HELPER(v7m_bxns)(CPUARMState *env, uint32_t dest) g_assert_not_reached(); } +void HELPER(v7m_blxns)(CPUARMState *env, uint32_t dest) +{ + /* translate.c should never generate calls here in user-only mode */ + g_assert_not_reached(); +} + void switch_mode(CPUARMState *env, int mode) { ARMCPU *cpu = arm_env_get_cpu(env); @@ -6182,6 +6188,59 @@ void HELPER(v7m_bxns)(CPUARMState *env, uint32_t dest) env->regs[15] = dest & ~1; } +void HELPER(v7m_blxns)(CPUARMState *env, uint32_t dest) +{ + /* Handle v7M BLXNS: + * - bit 0 of the destination address is the target security state + */ + + /* At this point regs[15] is the address just after the BLXNS */ + uint32_t nextinst = env->regs[15] | 1; + uint32_t sp = env->regs[13] - 8; + uint32_t saved_psr; + + /* translate.c will have made BLXNS UNDEF unless we're secure */ + assert(env->v7m.secure); + + if (dest & 1) { + /* target is Secure, so this is just a normal BLX, + * except that the low bit doesn't indicate Thumb/not. + */ + env->regs[14] = nextinst; + env->thumb = 1; + env->regs[15] = dest & ~1; + return; + } + + /* Target is non-secure: first push a stack frame */ + if (!QEMU_IS_ALIGNED(sp, 8)) { + qemu_log_mask(LOG_GUEST_ERROR, + "BLXNS with misaligned SP is UNPREDICTABLE\n"); + } + + saved_psr = env->v7m.exception; + if (env->v7m.control[M_REG_S] & R_V7M_CONTROL_SFPA_MASK) { + saved_psr |= XPSR_SFPA; + } + + /* Note that these stores can throw exceptions on MPU faults */ + cpu_stl_data(env, sp, nextinst); + cpu_stl_data(env, sp + 4, saved_psr); + + env->regs[13] = sp; + env->regs[14] = 0xfeffffff; + if (arm_v7m_is_handler_mode(env)) { + /* Write a dummy value to IPSR, to avoid leaking the current secure + * exception number to non-secure code. This is guaranteed not + * to cause write_v7m_exception() to actually change stacks. + */ + write_v7m_exception(env, 1); + } + switch_v7m_security_state(env, dest & 1); + env->thumb = 1; + env->regs[15] = dest & ~1; +} + static uint32_t *get_v7m_sp_ptr(CPUARMState *env, bool secure, bool threadmode, bool spsel) { diff --git a/target/arm/translate.c b/target/arm/translate.c index ab1a12a..53694bb 100644 --- a/target/arm/translate.c +++ b/target/arm/translate.c @@ -1013,6 +1013,20 @@ static inline void gen_bxns(DisasContext *s, int rm) s->base.is_jmp = DISAS_EXIT; } +static inline void gen_blxns(DisasContext *s, int rm) +{ + TCGv_i32 var = load_reg(s, rm); + + /* We don't need to sync condexec state, for the same reason as blxns. + * We do however need to set the PC, because the blxns helper reads it. + * The blxns helper may throw an exception. + */ + gen_set_pc_im(s, s->pc); + gen_helper_v7m_blxns(cpu_env, var); + tcg_temp_free_i32(var); + s->base.is_jmp = DISAS_EXIT; +} + /* Variant of store_reg which uses branch&exchange logic when storing to r15 in ARM architecture v7 and above. The source must be a temporary and will be marked as dead. */ @@ -11221,8 +11235,7 @@ static void disas_thumb_insn(CPUARMState *env, DisasContext *s) goto undef; } if (link) { - /* BLXNS: not yet implemented */ - goto undef; + gen_blxns(s, rm); } else { gen_bxns(s, rm); }
Implement the BLXNS instruction, which allows secure code to call non-secure code. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/helper.h | 1 + target/arm/internals.h | 1 + target/arm/helper.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++ target/arm/translate.c | 17 +++++++++++++-- 4 files changed, 76 insertions(+), 2 deletions(-)