On Fri, Apr 11, 2014 at 2:15 AM, Peter Maydell <peter.mayd...@linaro.org> wrote: > For exceptions taken to AArch64, if a coprocessor/system register > access fails due to a trap or enable bit then the syndrome information > must include details of the failing instruction (crn/crm/opc1/opc2 > fields, etc). Make the decoder construct the syndrome information > at translate time so it can be passed at runtime to the access-check > helper function and used as required. > > Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
I think we decided that the SAME_EL common code issue was too hard in the end, so: Reviewed-by: Peter Crosthwaite <peter.crosthwa...@xilinx.com> > --- > target-arm/helper.h | 2 +- > target-arm/internals.h | 128 > +++++++++++++++++++++++++++++++++++++++++++++ > target-arm/op_helper.c | 8 +-- > target-arm/translate-a64.c | 8 ++- > target-arm/translate.c | 45 +++++++++++++++- > 5 files changed, 184 insertions(+), 7 deletions(-) > > diff --git a/target-arm/helper.h b/target-arm/helper.h > index 366c1b3..2cdeadd 100644 > --- a/target-arm/helper.h > +++ b/target-arm/helper.h > @@ -58,7 +58,7 @@ DEF_HELPER_1(cpsr_read, i32, env) > DEF_HELPER_3(v7m_msr, void, env, i32, i32) > DEF_HELPER_2(v7m_mrs, i32, env, i32) > > -DEF_HELPER_2(access_check_cp_reg, void, env, ptr) > +DEF_HELPER_3(access_check_cp_reg, void, env, ptr, i32) > DEF_HELPER_3(set_cp_reg, void, env, ptr, i32) > DEF_HELPER_2(get_cp_reg, i32, env, ptr) > DEF_HELPER_3(set_cp_reg64, void, env, ptr, i64) > diff --git a/target-arm/internals.h b/target-arm/internals.h > index a38a57f..cc3fbf9 100644 > --- a/target-arm/internals.h > +++ b/target-arm/internals.h > @@ -46,4 +46,132 @@ enum arm_fprounding { > > int arm_rmode_to_sf(int rmode); > > +/* Valid Syndrome Register EC field values */ > +enum arm_exception_class { > + EC_UNCATEGORIZED = 0x00, > + EC_WFX_TRAP = 0x01, > + EC_CP15RTTRAP = 0x03, > + EC_CP15RRTTRAP = 0x04, > + EC_CP14RTTRAP = 0x05, > + EC_CP14DTTRAP = 0x06, > + EC_ADVSIMDFPACCESSTRAP = 0x07, > + EC_FPIDTRAP = 0x08, > + EC_CP14RRTTRAP = 0x0c, > + EC_ILLEGALSTATE = 0x0e, > + EC_AA32_SVC = 0x11, > + EC_AA32_HVC = 0x12, > + EC_AA32_SMC = 0x13, > + EC_AA64_SVC = 0x15, > + EC_AA64_HVC = 0x16, > + EC_AA64_SMC = 0x17, > + EC_SYSTEMREGISTERTRAP = 0x18, > + EC_INSNABORT = 0x20, > + EC_INSNABORT_SAME_EL = 0x21, > + EC_PCALIGNMENT = 0x22, > + EC_DATAABORT = 0x24, > + EC_DATAABORT_SAME_EL = 0x25, > + EC_SPALIGNMENT = 0x26, > + EC_AA32_FPTRAP = 0x28, > + EC_AA64_FPTRAP = 0x2c, > + EC_SERROR = 0x2f, > + EC_BREAKPOINT = 0x30, > + EC_BREAKPOINT_SAME_EL = 0x31, > + EC_SOFTWARESTEP = 0x32, > + EC_SOFTWARESTEP_SAME_EL = 0x33, > + EC_WATCHPOINT = 0x34, > + EC_WATCHPOINT_SAME_EL = 0x35, > + EC_AA32_BKPT = 0x38, > + EC_VECTORCATCH = 0x3a, > + EC_AA64_BKPT = 0x3c, > +}; > + > +#define ARM_EL_EC_SHIFT 26 > +#define ARM_EL_IL_SHIFT 25 > +#define ARM_EL_IL (1 << ARM_EL_IL_SHIFT) > + > +/* Utility functions for constructing various kinds of syndrome value. > + * Note that in general we follow the AArch64 syndrome values; in a > + * few cases the value in HSR for exceptions taken to AArch32 Hyp > + * mode differs slightly, so if we ever implemented Hyp mode then the > + * syndrome value would need some massaging on exception entry. > + * (One example of this is that AArch64 defaults to IL bit set for > + * exceptions which don't specifically indicate information about the > + * trapping instruction, whereas AArch32 defaults to IL bit clear.) > + */ > +static inline uint32_t syn_uncategorized(void) > +{ > + return (EC_UNCATEGORIZED << ARM_EL_EC_SHIFT) | ARM_EL_IL; > +} > + > +static inline uint32_t syn_aa64_svc(uint32_t imm16) > +{ > + return (EC_AA64_SVC << ARM_EL_EC_SHIFT) | ARM_EL_IL | (imm16 & 0xffff); > +} > + > +static inline uint32_t syn_aa32_svc(uint32_t imm16, bool is_thumb) > +{ > + return (EC_AA32_SVC << ARM_EL_EC_SHIFT) | (imm16 & 0xffff) > + | (is_thumb ? 0 : ARM_EL_IL); > +} > + > +static inline uint32_t syn_aa64_bkpt(uint32_t imm16) > +{ > + return (EC_AA64_BKPT << ARM_EL_EC_SHIFT) | ARM_EL_IL | (imm16 & 0xffff); > +} > + > +static inline uint32_t syn_aa32_bkpt(uint32_t imm16, bool is_thumb) > +{ > + return (EC_AA32_BKPT << ARM_EL_EC_SHIFT) | (imm16 & 0xffff) > + | (is_thumb ? 0 : ARM_EL_IL); > +} > + > +static inline uint32_t syn_aa64_sysregtrap(int op0, int op1, int op2, > + int crn, int crm, int rt, > + int isread) > +{ > + return (EC_SYSTEMREGISTERTRAP << ARM_EL_EC_SHIFT) | ARM_EL_IL > + | (op0 << 20) | (op2 << 17) | (op1 << 14) | (crn << 10) | (rt << 5) > + | (crm << 1) | isread; > +} > + > +static inline uint32_t syn_cp14_rt_trap(int cv, int cond, int opc1, int opc2, > + int crn, int crm, int rt, int isread, > + bool is_thumb) > +{ > + return (EC_CP14RTTRAP << ARM_EL_EC_SHIFT) > + | (is_thumb ? 0 : ARM_EL_IL) > + | (cv << 24) | (cond << 20) | (opc2 << 17) | (opc1 << 14) > + | (crn << 10) | (rt << 5) | (crm << 1) | isread; > +} > + > +static inline uint32_t syn_cp15_rt_trap(int cv, int cond, int opc1, int opc2, > + int crn, int crm, int rt, int isread, > + bool is_thumb) > +{ > + return (EC_CP15RTTRAP << ARM_EL_EC_SHIFT) > + | (is_thumb ? 0 : ARM_EL_IL) > + | (cv << 24) | (cond << 20) | (opc2 << 17) | (opc1 << 14) > + | (crn << 10) | (rt << 5) | (crm << 1) | isread; > +} > + > +static inline uint32_t syn_cp14_rrt_trap(int cv, int cond, int opc1, int crm, > + int rt, int rt2, int isread, > + bool is_thumb) > +{ > + return (EC_CP14RRTTRAP << ARM_EL_EC_SHIFT) > + | (is_thumb ? 0 : ARM_EL_IL) > + | (cv << 24) | (cond << 20) | (opc1 << 16) > + | (rt2 << 10) | (rt << 5) | (crm << 1) | isread; > +} > + > +static inline uint32_t syn_cp15_rrt_trap(int cv, int cond, int opc1, int crm, > + int rt, int rt2, int isread, > + bool is_thumb) > +{ > + return (EC_CP15RRTTRAP << ARM_EL_EC_SHIFT) > + | (is_thumb ? 0 : ARM_EL_IL) > + | (cv << 24) | (cond << 20) | (opc1 << 16) > + | (rt2 << 10) | (rt << 5) | (crm << 1) | isread; > +} > + > #endif > diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c > index 4193eca..bacfbc0 100644 > --- a/target-arm/op_helper.c > +++ b/target-arm/op_helper.c > @@ -294,17 +294,17 @@ void HELPER(set_user_reg)(CPUARMState *env, uint32_t > regno, uint32_t val) > } > } > > -void HELPER(access_check_cp_reg)(CPUARMState *env, void *rip) > +void HELPER(access_check_cp_reg)(CPUARMState *env, void *rip, uint32_t > syndrome) > { > const ARMCPRegInfo *ri = rip; > switch (ri->accessfn(env, ri)) { > case CP_ACCESS_OK: > return; > case CP_ACCESS_TRAP: > + env->exception.syndrome = syndrome; > + break; > case CP_ACCESS_TRAP_UNCATEGORIZED: > - /* These cases will eventually need to generate different > - * syndrome information. > - */ > + env->exception.syndrome = syn_uncategorized(); > break; > default: > g_assert_not_reached(); > diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c > index 6689165..37399df 100644 > --- a/target-arm/translate-a64.c > +++ b/target-arm/translate-a64.c > @@ -1242,10 +1242,16 @@ static void handle_sys(DisasContext *s, uint32_t > insn, bool isread, > * runtime; this may result in an exception. > */ > TCGv_ptr tmpptr; > + TCGv_i32 tcg_syn; > + uint32_t syndrome; > + > gen_a64_set_pc_im(s->pc - 4); > tmpptr = tcg_const_ptr(ri); > - gen_helper_access_check_cp_reg(cpu_env, tmpptr); > + syndrome = syn_aa64_sysregtrap(op0, op1, op2, crn, crm, rt, isread); > + tcg_syn = tcg_const_i32(syndrome); > + gen_helper_access_check_cp_reg(cpu_env, tmpptr, tcg_syn); > tcg_temp_free_ptr(tmpptr); > + tcg_temp_free_i32(tcg_syn); > } > > /* Handle special cases first */ > diff --git a/target-arm/translate.c b/target-arm/translate.c > index f869bc6..d31f5c1 100644 > --- a/target-arm/translate.c > +++ b/target-arm/translate.c > @@ -6862,10 +6862,53 @@ static int disas_coproc_insn(CPUARMState * env, > DisasContext *s, uint32_t insn) > * runtime; this may result in an exception. > */ > TCGv_ptr tmpptr; > + TCGv_i32 tcg_syn; > + uint32_t syndrome; > + > + /* Note that since we are an implementation which takes an > + * exception on a trapped conditional instruction only if the > + * instruction passes its condition code check, we can take > + * advantage of the clause in the ARM ARM that allows us to set > + * the COND field in the instruction to 0xE in all cases. > + * We could fish the actual condition out of the insn (ARM) > + * or the condexec bits (Thumb) but it isn't necessary. > + */ > + switch (cpnum) { > + case 14: > + if (is64) { > + syndrome = syn_cp14_rrt_trap(1, 0xe, opc1, crm, rt, rt2, > + isread, s->thumb); > + } else { > + syndrome = syn_cp14_rt_trap(1, 0xe, opc1, opc2, crn, crm, > + rt, isread, s->thumb); > + } > + break; > + case 15: > + if (is64) { > + syndrome = syn_cp15_rrt_trap(1, 0xe, opc1, crm, rt, rt2, > + isread, s->thumb); > + } else { > + syndrome = syn_cp15_rt_trap(1, 0xe, opc1, opc2, crn, crm, > + rt, isread, s->thumb); > + } > + break; > + default: > + /* ARMv8 defines that only coprocessors 14 and 15 exist, > + * so this can only happen if this is an ARMv7 or earlier > CPU, > + * in which case the syndrome information won't actually be > + * guest visible. > + */ > + assert(!arm_feature(env, ARM_FEATURE_V8)); > + syndrome = syn_uncategorized(); > + break; > + } > + > gen_set_pc_im(s, s->pc); > tmpptr = tcg_const_ptr(ri); > - gen_helper_access_check_cp_reg(cpu_env, tmpptr); > + tcg_syn = tcg_const_i32(syndrome); > + gen_helper_access_check_cp_reg(cpu_env, tmpptr, tcg_syn); > tcg_temp_free_ptr(tmpptr); > + tcg_temp_free_i32(tcg_syn); > } > > /* Handle special cases first */ > -- > 1.9.1 > >