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

[Mike Haben]

On 03/09/15 17:58, Peter Crosthwaite wrote:
> Hi Mike,
>
> On Thu, Sep 3, 2015 at 2:27 AM, GitNoviceMikeH
> <address@hidden> wrote:
> > From: GitNoviceMikeH <address@hidden>
> >
> > 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