| Message ID | 55EAD1F8.60107@virgin.net |
|---|---|
| State | New |
| Headers | show |
On 5 September 2015 at 12:28, Mike Haben <mike.haben@virgin.net> wrote: > Most ARM cores switch unconditionally to ARM mode when an exception occurs; > some Cortex variants have a "Thumb-exception enable" bit in the system > control register that allows an unconditional switch to Thumb mode instead > when handling exceptions. The presence of this bit seems unrelated to the > version of instruction set, and some earlier cores use the same bit (30) in > the control register for a completely different purpose, so seems sensible > to handle it as yet another ARM feature. I think that SCTLR.TE is an ARMv7 feature -- it is documented in the v7 ARM ARM, and in the v6 ARM ARM the bit is UNP/SBZP. And the CPUs you've set your new feature bit on in this patch are exactly the v7 CPUs. So I think that we should just change the existing guard (which requires FEATURE_V4T) to require FEATURE_V7 instead). You're right that we need to specifically squash env->thumb to false in the no-feature-present case, though. thanks -- PMM
Hi Peter, You're quite right, on reading some more I see the correspondence with V7. However... while reading up on the Cortex-M3/4/7, I also found "Only Thumb and Thumb-2 instruction sets are supported in Cortex-M architectures, but the legacy 32-bit ARM instruction set isn't supported". Ugh - to avoid storing up a problem for the future, I think I better think it out again! best regards, Mike H. On 05/09/15 14:02, Peter Maydell wrote: > On 5 September 2015 at 12:28, Mike Haben <mike.haben@virgin.net> wrote: >> Most ARM cores switch unconditionally to ARM mode when an exception occurs; >> some Cortex variants have a "Thumb-exception enable" bit in the system >> control register that allows an unconditional switch to Thumb mode instead >> when handling exceptions. The presence of this bit seems unrelated to the >> version of instruction set, and some earlier cores use the same bit (30) in >> the control register for a completely different purpose, so seems sensible >> to handle it as yet another ARM feature. > > I think that SCTLR.TE is an ARMv7 feature -- it is documented > in the v7 ARM ARM, and in the v6 ARM ARM the bit is UNP/SBZP. > And the CPUs you've set your new feature bit on in this patch > are exactly the v7 CPUs. > > So I think that we should just change the existing guard > (which requires FEATURE_V4T) to require FEATURE_V7 instead). > You're right that we need to specifically squash env->thumb > to false in the no-feature-present case, though. > > thanks > -- PMM >
On 5 September 2015 at 14:38, Mike Haben <mike.haben@virgin.net> wrote: > Hi Peter, > You're quite right, on reading some more I see the correspondence with V7. > However... while reading up on the Cortex-M3/4/7, I also found > "Only Thumb and Thumb-2 instruction sets are supported in Cortex-M > architectures, but the legacy 32-bit ARM instruction set isn't supported". > Ugh - to avoid storing up a problem for the future, I think I better think > it out again! M profile exception handling is completely different to A/R profile, and does not use this function at all (it is done via arm_v7m_cpu_do_interrupt()). So that isn't a problem. (In fact M profile doesn't even have an SCTLR register.) thanks -- PMM
diff --git a/target-arm/cpu.c b/target-arm/cpu.c index cc6c6f3..9c96fe1 100644 --- a/target-arm/cpu.c +++ b/target-arm/cpu.c @@ -933,6 +933,7 @@ static void cortex_r5_initfn(Object *obj) set_feature(&cpu->env, ARM_FEATURE_ARM_DIV); set_feature(&cpu->env, ARM_FEATURE_V7MP); set_feature(&cpu->env, ARM_FEATURE_MPU); + set_feature(&cpu->env, ARM_FEATURE_SCTLR_TE_BIT); cpu->midr = 0x411fc153; /* r1p3 */ cpu->id_pfr0 = 0x0131; cpu->id_pfr1 = 0x001; @@ -971,6 +972,7 @@ static void cortex_a8_initfn(Object *obj) set_feature(&cpu->env, ARM_FEATURE_THUMB2EE); set_feature(&cpu->env, ARM_FEATURE_DUMMY_C15_REGS); set_feature(&cpu->env, ARM_FEATURE_EL3); + set_feature(&cpu->env, ARM_FEATURE_SCTLR_TE_BIT); cpu->midr = 0x410fc080; cpu->reset_fpsid = 0x410330c0; cpu->mvfr0 = 0x11110222; @@ -1045,6 +1047,7 @@ static void cortex_a9_initfn(Object *obj) */ set_feature(&cpu->env, ARM_FEATURE_V7MP); set_feature(&cpu->env, ARM_FEATURE_CBAR); + set_feature(&cpu->env, ARM_FEATURE_SCTLR_TE_BIT); cpu->midr = 0x410fc090; cpu->reset_fpsid = 0x41033090; cpu->mvfr0 = 0x11110222; @@ -1107,6 +1110,7 @@ static void cortex_a15_initfn(Object *obj) set_feature(&cpu->env, ARM_FEATURE_CBAR_RO); set_feature(&cpu->env, ARM_FEATURE_LPAE); set_feature(&cpu->env, ARM_FEATURE_EL3); + set_feature(&cpu->env, ARM_FEATURE_SCTLR_TE_BIT); cpu->kvm_target = QEMU_KVM_ARM_TARGET_CORTEX_A15; cpu->midr = 0x412fc0f1; cpu->reset_fpsid = 0x410430f0; diff --git a/target-arm/cpu.h b/target-arm/cpu.h index 31825d3..30d49b0 100644 --- a/target-arm/cpu.h +++ b/target-arm/cpu.h @@ -905,6 +905,7 @@ enum arm_features { ARM_FEATURE_V8_SHA256, /* implements SHA256 part of v8 Crypto Extensions */ ARM_FEATURE_V8_PMULL, /* implements PMULL part of v8 Crypto Extensions */ ARM_FEATURE_THUMB_DSP, /* DSP insns supported in the Thumb encodings */ + ARM_FEATURE_SCTLR_TE_BIT, /* Control register bit 30 is Thumb-exception */ }; static inline int arm_feature(CPUARMState *env, int feature) diff --git a/target-arm/helper.c b/target-arm/helper.c index 7df1f06..c1c50da 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -5658,11 +5658,18 @@ void arm_cpu_do_interrupt(CPUState *cs) /* Switch to the new mode, and to the correct instruction set. */ env->uncached_cpsr = (env->uncached_cpsr & ~CPSR_M) | new_mode; env->daif |= mask; - /* this is a lie, as the was no c1_sys on V4T/V5, but who cares - * and we should just guard the thumb mode on V4 */ - if (arm_feature(env, ARM_FEATURE_V4T)) { - env->thumb = (A32_BANKED_CURRENT_REG_GET(env, sctlr) & SCTLR_TE) != 0; - } + + /* Most ARM cores switch unconditionally to ARM mode when an exception + * occurs: + */ + env->thumb = false; + /* ...but certain cores have a Thumb-exception enable bit in the system + * control register: + */ + if (arm_feature(env, ARM_FEATURE_SCTLR_TE_BIT)) { + env->thumb = (A32_BANKED_CURRENT_REG_GET(env, sctlr) & SCTLR_TE) != 0; + } + env->regs[14] = env->regs[15] + offset;
Most ARM cores switch unconditionally to ARM mode when an exception occurs; some Cortex variants have a "Thumb-exception enable" bit in the system control register that allows an unconditional switch to Thumb mode instead when handling exceptions. The presence of this bit seems unrelated to the version of instruction set, and some earlier cores use the same bit (30) in the control register for a completely different purpose, so seems sensible to handle it as yet another ARM feature. Signed-off-by: Mike Haben <mike.haben@virgin.net> --- target-arm/cpu.c | 4 ++++ target-arm/cpu.h | 1 + target-arm/helper.c | 17 ++++++++++++----- 3 files changed, 17 insertions(+), 5 deletions(-) env->regs[15] = addr; cs->interrupt_request |= CPU_INTERRUPT_EXITTB;