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

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

[GitNoviceMikeH]

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?
Also added four V4T ARM CPU definitions - 720T, 920T, 922T and 940T.

---
 target-arm/cpu.c    | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 target-arm/cpu.h    |  1 +
 target-arm/helper.c | 11 ++++++++---
 3 files changed, 64 insertions(+), 3 deletions(-)

diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index cc6c6f3..1e81a81 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -699,6 +699,53 @@ static ObjectClass *arm_cpu_class_by_name(const char *cpu_model)
 /* CPU models. These are not needed for the AArch64 linux-user build. */
 #if !defined(CONFIG_USER_ONLY) || !defined(TARGET_AARCH64)
 
+static void arm720t_initfn(Object *obj)
+{
+    ARMCPU *cpu = ARM_CPU(obj);
+
+    cpu->dtb_compatible = "arm,arm720t";
+    set_feature(&cpu->env, ARM_FEATURE_V4T);
+    set_feature(&cpu->env, ARM_FEATURE_DUMMY_C15_REGS);
+    cpu->midr = 0x41807204;
+    cpu->reset_sctlr = 0x00090078;
+}
+
+static void arm920t_initfn(Object *obj)
+{
+    ARMCPU *cpu = ARM_CPU(obj);
+
+    cpu->dtb_compatible = "arm,arm920t";
+    set_feature(&cpu->env, ARM_FEATURE_V4T);
+    set_feature(&cpu->env, ARM_FEATURE_DUMMY_C15_REGS);
+    cpu->midr = 0x41129200;
+    cpu->ctr = 0x0d172172;
+    cpu->reset_sctlr = 0x00090078;
+}
+
+static void arm922t_initfn(Object *obj)
+{
+    ARMCPU *cpu = ARM_CPU(obj);
+
+    cpu->dtb_compatible = "arm,arm922t";
+    set_feature(&cpu->env, ARM_FEATURE_V4T);
+    set_feature(&cpu->env, ARM_FEATURE_DUMMY_C15_REGS);
+    cpu->midr = 0x41029220;
+    cpu->ctr = 0x0d132132;
+    cpu->reset_sctlr = 0x00090078;
+}
+
+static void arm940t_initfn(Object *obj)
+{
+    ARMCPU *cpu = ARM_CPU(obj);
+
+    cpu->dtb_compatible = "arm,arm940t";
+    set_feature(&cpu->env, ARM_FEATURE_V4T);
+    set_feature(&cpu->env, ARM_FEATURE_DUMMY_C15_REGS);
+    cpu->midr = 0x41029400;
+    cpu->ctr = 0x0f0f10f1;
+    cpu->reset_sctlr = 0x00090078;
+}
+
 static void arm926_initfn(Object *obj)
 {
     ARMCPU *cpu = ARM_CPU(obj);
@@ -933,6 +980,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 +1019,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;
@@ -1039,6 +1088,7 @@ static void cortex_a9_initfn(Object *obj)
     set_feature(&cpu->env, ARM_FEATURE_NEON);
     set_feature(&cpu->env, ARM_FEATURE_THUMB2EE);
     set_feature(&cpu->env, ARM_FEATURE_EL3);
+    set_feature(&cpu->env, ARM_FEATURE_SCTLR_TE_BIT);
     /* Note that A9 supports the MP extensions even for
      * A9UP and single-core A9MP (which are both different
      * and valid configurations; we don't model A9UP).
@@ -1107,6 +1157,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;
@@ -1330,6 +1381,10 @@ typedef struct ARMCPUInfo {
 
 static const ARMCPUInfo arm_cpus[] = {
 #if !defined(CONFIG_USER_ONLY) || !defined(TARGET_AARCH64)
+    { .name = "arm720t",     .initfn = arm720t_initfn },
+    { .name = "arm920t",     .initfn = arm920t_initfn },
+    { .name = "arm922t",     .initfn = arm922t_initfn },
+    { .name = "arm940t",     .initfn = arm940t_initfn },
     { .name = "arm926",      .initfn = arm926_initfn },
     { .name = "arm946",      .initfn = arm946_initfn },
     { .name = "arm1026",     .initfn = arm1026_initfn },
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 31825d3..f922da2 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, /* Thumb-exception bit in control register */
 };
 
 static inline int arm_feature(CPUARMState *env, int feature)
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 7df1f06..e8bd71e 100644
--- a/target-arm/helper.c
+++ 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: */
+    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

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

[Peter Crosthwaite]

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?

> 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,
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.

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

>
> ---
>  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
>
>

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
> <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
>>
>>
>

[Qemu-devel] [PATCH] ARM targets: added ARM_FEATURE for Thumb-exception bit in system control register

[Mike Haben]

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(-)

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;
      env->regs[15] = addr;
      cs->interrupt_request |= CPU_INTERRUPT_EXITTB;
-- 
1.9.1

Re: [Qemu-devel] [PATCH] ARM targets: added ARM_FEATURE for Thumb-exception bit in system control register

[Peter Maydell]

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

Re: [Qemu-devel] [PATCH] ARM targets: added ARM_FEATURE for Thumb-exception bit in system control register

[Mike Haben]

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
>

Re: [Qemu-devel] [PATCH] ARM targets: added ARM_FEATURE for Thumb-exception bit in system control register

[Peter Maydell]

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