[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 04/21] target-arm: Provide correct syndrome i
From: |
Peter Crosthwaite |
Subject: |
Re: [Qemu-devel] [PATCH v4 04/21] target-arm: Provide correct syndrome information for cpreg access traps |
Date: |
Mon, 17 Mar 2014 13:05:26 +1000 |
On Fri, Mar 7, 2014 at 5:32 AM, Peter Maydell <address@hidden> 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 <address@hidden>
> ---
> 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 276f3a9..7f23cb8 100644
> --- a/target-arm/helper.h
> +++ b/target-arm/helper.h
> @@ -57,7 +57,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..2c0db20 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 = 0,
> + EC_WFX_TRAP = 1,
> + EC_CP15RTTRAP = 3,
> + EC_CP15RRTTRAP = 4,
> + EC_CP14RTTRAP = 5,
> + EC_CP14DTTRAP = 6,
> + EC_ADVSIMDFPACCESSTRAP = 7,
> + EC_FPIDTRAP = 8,
> + EC_CP14RRTTRAP = 0xc,
> + EC_ILLEGALSTATE = 0xe,
> + 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,
> +};
> +
Can we space out the constants to a consistent tab stop for readability?
> +#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 1458cd3..bef2cf6 100644
> --- a/target-arm/op_helper.c
> +++ b/target-arm/op_helper.c
> @@ -274,17 +274,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;
Can TCG just deposit this directly and unconditionally straight to the
env to avoid the extra syndrome arg?
Regards,
Peter
> + 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 156fda2..a4f9258 100644
> --- a/target-arm/translate-a64.c
> +++ b/target-arm/translate-a64.c
> @@ -1239,10 +1239,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 4a53313..9a81222 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -6843,10 +6843,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.0
>
>
- [Qemu-devel] [PATCH v4 16/21] target-arm: Implement SP_EL0, SP_EL1, (continued)
- [Qemu-devel] [PATCH v4 16/21] target-arm: Implement SP_EL0, SP_EL1, Peter Maydell, 2014/03/06
- [Qemu-devel] [PATCH v4 19/21] target-arm: Implement AArch64 EL1 exception handling, Peter Maydell, 2014/03/06
- [Qemu-devel] [PATCH v4 18/21] target-arm: Move arm_log_exception() into internals.h, Peter Maydell, 2014/03/06
- [Qemu-devel] [PATCH v4 21/21] hw/arm/virt: Add support for Cortex-A57, Peter Maydell, 2014/03/06
- [Qemu-devel] [PATCH v4 04/21] target-arm: Provide correct syndrome information for cpreg access traps, Peter Maydell, 2014/03/06
- Re: [Qemu-devel] [PATCH v4 04/21] target-arm: Provide correct syndrome information for cpreg access traps,
Peter Crosthwaite <=
- [Qemu-devel] [PATCH v4 10/21] target-arm: Add v8 mmu translation support, Peter Maydell, 2014/03/06
- [Qemu-devel] [PATCH v4 11/21] target-arm: Don't mention PMU in debug feature register, Peter Maydell, 2014/03/06
- [Qemu-devel] [PATCH v4 06/21] target-arm: Provide syndrome information for MMU faults, Peter Maydell, 2014/03/06