Re: [Qemu-devel] [PATCH] Added specific ARM_FEATURE for Thumb-exception enable bit

[Mike Haben <mike.haben@virgin.net>]

On 03/09/15 17:58, Peter Crosthwaite wrote:
> Hi Mike,
>
> On Thu, Sep 3, 2015 at 2:27 AM, GitNoviceMikeH
> <mike.haben_at_virgin.net@mike-desktop> wrote:
>> From: GitNoviceMikeH <mike.haben@virgin.net>
>>
>> Most ARM cores switch unconditionally to ARM mode when an exception occurs;
>> a few (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, so seems sensible to handle it as yet another
>> ARM feature?
>
> What is the earliest ARM ARM the bit appears in?

As far as I can tell, it only appears in Cortex cores - don't know which 
of these is the earliest.  920T and 940T use bit 30 of SCTLR for 
"not-FastBus", a feature completely unrelated to Thumb-exception.

>
>> Also added four V4T ARM CPU definitions - 720T, 920T, 922T and 940T.
>
> This should be at least three patches. One to add the new ARM_FEATURE,

Seriously, >=3 patches? Could we live with two (one to add the V4 CPUs, 
one to properly implement Thumb-exception on just the Cortex CPUs)?

> then one to add the feature to existing CPUs (A9 and firends), then
> add the new CPUs you want. For acceptance you also need to sign off
> the patch(es) with your real name and email. Git config --global
> user.name and user.email and pass the -s flag to git commit. You also
> need to fix your sending email.

OK, my mistake - first time using git-send-email.

>
> What is your use case for the new CPUs? do you use these CPUs with a
> particular board?
>

My immediate use-case was to run some test/example code written for a 
920T, to compare QEMU against SoCLib; seemed sensible to add other 
common V4 cores at the same time.

>>
>> ---
>>   target-arm/cpu.c    | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   target-arm/cpu.h    |  1 +
>>   target-arm/helper.c | 11 ++++++++---
>>   3 files changed, 64 insertions(+), 3 deletions(-)
> ...
>> +++ b/target-arm/helper.c
>> @@ -5658,11 +5658,16 @@ 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)) {
>> +
>> +    /* Most ARM cores switch unconditionally to ARM mode when an exception
>> +     * occurs: */
>
> /* comment style
>   * is like this
>   */
>
> Regards,
> Peter
>
>> +    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;
>>       env->regs[15] = addr;
>>       cs->interrupt_request |= CPU_INTERRUPT_EXITTB;
>> --
>> 1.9.1
>>
>>
>