| Message ID | 1391183143-30724-10-git-send-email-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
On 5 February 2014 06:23, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote: > On Sat, Feb 1, 2014 at 1:45 AM, Peter Maydell <peter.maydell@linaro.org> wrote: >> Implement the MSR (immediate) instructions, which can update the >> PSTATE SP and DAIF fields. >> >> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> >> --- >> target-arm/cpu.h | 1 + >> target-arm/helper.h | 2 ++ >> target-arm/op_helper.c | 25 +++++++++++++++++++++++++ >> target-arm/translate-a64.c | 24 +++++++++++++++++++++++- >> 4 files changed, 51 insertions(+), 1 deletion(-) >> >> diff --git a/target-arm/cpu.h b/target-arm/cpu.h >> index 385cfcd..e66d464 100644 >> --- a/target-arm/cpu.h >> +++ b/target-arm/cpu.h >> @@ -426,6 +426,7 @@ int cpu_arm_handle_mmu_fault (CPUARMState *env, target_ulong address, int rw, >> #define PSTATE_Z (1U << 30) >> #define PSTATE_N (1U << 31) >> #define PSTATE_NZCV (PSTATE_N | PSTATE_Z | PSTATE_C | PSTATE_V) >> +#define PSTATE_DAIF (PSTATE_D | PSTATE_A | PSTATE_I | PSTATE_F) >> #define CACHED_PSTATE_BITS (PSTATE_NZCV) >> /* Mode values for AArch64 */ >> #define PSTATE_MODE_EL3h 13 >> diff --git a/target-arm/helper.h b/target-arm/helper.h >> index 71b8411..93a27ce 100644 >> --- a/target-arm/helper.h >> +++ b/target-arm/helper.h >> @@ -62,6 +62,8 @@ DEF_HELPER_2(get_cp_reg, i32, env, ptr) >> DEF_HELPER_3(set_cp_reg64, void, env, ptr, i64) >> DEF_HELPER_2(get_cp_reg64, i64, env, ptr) >> >> +DEF_HELPER_3(msr_i_pstate, void, env, i32, i32) >> + >> DEF_HELPER_2(get_r13_banked, i32, env, i32) >> DEF_HELPER_3(set_r13_banked, void, env, i32, i32) >> >> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c >> index a918e5b..c812a9f 100644 >> --- a/target-arm/op_helper.c >> +++ b/target-arm/op_helper.c >> @@ -313,6 +313,31 @@ uint64_t HELPER(get_cp_reg64)(CPUARMState *env, void *rip) >> return value; >> } >> >> +void HELPER(msr_i_pstate)(CPUARMState *env, uint32_t op, uint32_t imm) >> +{ >> + /* MSR_i to update PSTATE. This is OK from EL0 only if UMA is set. >> + * Note that SPSel is never OK from EL0; we rely on handle_msr_i() >> + * to catch that case at translate time. >> + */ >> + if (arm_current_pl(env) == 0 && !(env->cp15.c1_sys & SCTLR_UMA)) { >> + raise_exception(env, EXCP_UDEF); >> + } >> + >> + switch (op) { >> + case 0x05: /* SPSel */ >> + env->pstate = deposit32(env->pstate, 0, 1, imm); > > "0","1" hardcoded constants are a bit unfriendly. I guess the current > macro set doesnt define _SHIFT and _WIDTH definitions, should they be > added? > > FWIW, I have this macro in my tree which makes short work of defining > mask, shift and width constants as a one liner: > > /* Define SHIFT, LENGTH and MASK constants for a field within a register */ > > #define FIELD(reg, field, length, shift) \ > enum { reg ## _ ## field ## _SHIFT = (shift)}; \ > enum { reg ## _ ## field ## _LENGTH = (length)}; \ > enum { reg ## _ ## field ## _MASK = (((1ULL << (length)) - 1) \ > << (shift)) }; > > Usage would be something like FIELD(PSTATE, SPSEL, 1, 0) Hmm. I guess we could use some more consistent structure in defining bit macros. (reg, field, start, len) would be a better argument order though, to match the extract and deposit fns. >> + break; >> + case 0x1e: /* DAIFSet */ >> + env->pstate |= (imm << 6) & PSTATE_DAIF; >> + break; >> + case 0x1f: /* DAIFClear */ >> + env->pstate &= ~((imm << 6) & PSTATE_DAIF); > > I wonder whether deposit should be extended with and/or (with > existing) versions to allow for consistency in places like this. In > SPSel we get the nice deposit based implementation but with the logic > function change here were are stuck with open codedness. Set and > clearing and fields should be common enough tree wide to warrant it > perhaps. I dunno. Deposit is a function mostly because "clear old field to zeroes then insert new value" is complicated enough that it's easy to miscode it. Plain force-set and force-clear I think is simple enough (and not all that common) that I'm happy to opencode it. One point I need to think about a little more with the DAIF bits is whether we should be keeping them in one place for AArch32 and AArch64 -- at the moment an AArch32 core's IF bits are in cpsr. Having them in one location would make the cpu-exec.c code a bit simpler. thanks -- PMM
On 5 February 2014 06:23, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote: > On Sat, Feb 1, 2014 at 1:45 AM, Peter Maydell <peter.maydell@linaro.org> wrote: >> + switch (op) { >> + case 0x05: /* SPSel */ >> + env->pstate = deposit32(env->pstate, 0, 1, imm); > > "0","1" hardcoded constants are a bit unfriendly. I guess the current > macro set doesnt define _SHIFT and _WIDTH definitions, should they be > added? > > FWIW, I have this macro in my tree which makes short work of defining > mask, shift and width constants as a one liner: > > /* Define SHIFT, LENGTH and MASK constants for a field within a register */ > > #define FIELD(reg, field, length, shift) \ > enum { reg ## _ ## field ## _SHIFT = (shift)}; \ > enum { reg ## _ ## field ## _LENGTH = (length)}; \ > enum { reg ## _ ## field ## _MASK = (((1ULL << (length)) - 1) \ > << (shift)) }; > > Usage would be something like FIELD(PSTATE, SPSEL, 1, 0) So, I kind of like this, but I'm a bit reluctant to tie up this patchset in "add a new generic facility to bitops.h", and current handling for pstate/cpsr has a lot of hardcoded constants for bit positions. Is it OK if we do this as a separate cleanup, or do you feel strongly we should bite the bullet and do it as part of this series? thanks -- PMM
diff --git a/target-arm/cpu.h b/target-arm/cpu.h index 385cfcd..e66d464 100644 --- a/target-arm/cpu.h +++ b/target-arm/cpu.h @@ -426,6 +426,7 @@ int cpu_arm_handle_mmu_fault (CPUARMState *env, target_ulong address, int rw, #define PSTATE_Z (1U << 30) #define PSTATE_N (1U << 31) #define PSTATE_NZCV (PSTATE_N | PSTATE_Z | PSTATE_C | PSTATE_V) +#define PSTATE_DAIF (PSTATE_D | PSTATE_A | PSTATE_I | PSTATE_F) #define CACHED_PSTATE_BITS (PSTATE_NZCV) /* Mode values for AArch64 */ #define PSTATE_MODE_EL3h 13 diff --git a/target-arm/helper.h b/target-arm/helper.h index 71b8411..93a27ce 100644 --- a/target-arm/helper.h +++ b/target-arm/helper.h @@ -62,6 +62,8 @@ DEF_HELPER_2(get_cp_reg, i32, env, ptr) DEF_HELPER_3(set_cp_reg64, void, env, ptr, i64) DEF_HELPER_2(get_cp_reg64, i64, env, ptr) +DEF_HELPER_3(msr_i_pstate, void, env, i32, i32) + DEF_HELPER_2(get_r13_banked, i32, env, i32) DEF_HELPER_3(set_r13_banked, void, env, i32, i32) diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c index a918e5b..c812a9f 100644 --- a/target-arm/op_helper.c +++ b/target-arm/op_helper.c @@ -313,6 +313,31 @@ uint64_t HELPER(get_cp_reg64)(CPUARMState *env, void *rip) return value; } +void HELPER(msr_i_pstate)(CPUARMState *env, uint32_t op, uint32_t imm) +{ + /* MSR_i to update PSTATE. This is OK from EL0 only if UMA is set. + * Note that SPSel is never OK from EL0; we rely on handle_msr_i() + * to catch that case at translate time. + */ + if (arm_current_pl(env) == 0 && !(env->cp15.c1_sys & SCTLR_UMA)) { + raise_exception(env, EXCP_UDEF); + } + + switch (op) { + case 0x05: /* SPSel */ + env->pstate = deposit32(env->pstate, 0, 1, imm); + break; + case 0x1e: /* DAIFSet */ + env->pstate |= (imm << 6) & PSTATE_DAIF; + break; + case 0x1f: /* DAIFClear */ + env->pstate &= ~((imm << 6) & PSTATE_DAIF); + break; + default: + g_assert_not_reached(); + } +} + /* ??? Flag setting arithmetic is awkward because we need to do comparisons. The only way to do that in TCG is a conditional branch, which clobbers all our temporaries. For now implement these as helper functions. */ diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c index d938e5e..a942609 100644 --- a/target-arm/translate-a64.c +++ b/target-arm/translate-a64.c @@ -1106,7 +1106,29 @@ static void handle_sync(DisasContext *s, uint32_t insn, static void handle_msr_i(DisasContext *s, uint32_t insn, unsigned int op1, unsigned int op2, unsigned int crm) { - unsupported_encoding(s, insn); + int op = op1 << 3 | op2; + switch (op) { + case 0x05: /* SPSel */ + if (s->current_pl == 0) { + unallocated_encoding(s); + return; + } + /* fall through */ + case 0x1e: /* DAIFSet */ + case 0x1f: /* DAIFClear */ + { + TCGv_i32 tcg_imm = tcg_const_i32(crm); + TCGv_i32 tcg_op = tcg_const_i32(op); + gen_helper_msr_i_pstate(cpu_env, tcg_op, tcg_imm); + tcg_temp_free_i32(tcg_imm); + tcg_temp_free_i32(tcg_op); + s->is_jmp = DISAS_UPDATE; + break; + } + default: + unallocated_encoding(s); + return; + } } static void gen_get_nzcv(TCGv_i64 tcg_rt)
Implement the MSR (immediate) instructions, which can update the PSTATE SP and DAIF fields. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target-arm/cpu.h | 1 + target-arm/helper.h | 2 ++ target-arm/op_helper.c | 25 +++++++++++++++++++++++++ target-arm/translate-a64.c | 24 +++++++++++++++++++++++- 4 files changed, 51 insertions(+), 1 deletion(-)