Patchwork Initial support for ARM V6-M

login
register
mail settings
Submitter Alex_Rozenman@mentor.com
Date Dec. 13, 2011, 5:44 p.m.
Message ID <1323798248.18910.3.camel@petersburg>
Download mbox | patch
Permalink /patch/131153/
State New
Headers show

Comments

Alex_Rozenman@mentor.com - Dec. 13, 2011, 5:44 p.m.
Hi,

Please review / discuss / push the attched patch. It adds initial
support for V6-M for ARM and Cortex-M0.

Best regards,
Alex Rozenman
Peter Maydell - Dec. 13, 2011, 6:05 p.m.
On 13 December 2011 17:44, Alex Rozenman <Alex_Rozenman@mentor.com> wrote:
> Please review / discuss / push the attched patch. It adds initial support
> for V6-M for ARM and Cortex-M0.

Please don't submit patches as attachments (see
http://wiki.qemu.org/Contribute/SubmitAPatch for other guidelines).

This patch is pretty obviously missing a lot. It makes a start
on the instruction level differences, but v6M also has significant
differences in the memory mapped system registers, NVIC, etc, which
this patch doesn't try to deal with at all. There are also CPU model
differences like a smaller set of supported MRS/MSR system registers,
the M0 having no user/privileged distinction, etc.

The v6M Architecture Reference Manual has an appendix on the v6M-to-v7M
differences, which is probably worth working through.

-- PMM
Andreas Färber - Dec. 13, 2011, 6:11 p.m.
Hi,

Am 13.12.2011 18:44, schrieb Alex Rozenman:
> Please review / discuss / push the attched patch. It adds initial
> support for V6-M for ARM and Cortex-M0.

Please send this as an inline patch (git send-email) and cc Peter.

Some comments:

Please rebase against Peter's target-arm branch:

http://git.linaro.org/gitweb?p=people/pmaydell/qemu-arm.git;a=shortlog;h=refs/heads/target-arm.next

You can drop V4T and V5 features then.

IS_V7M() does not seem to be used anywhere yet.

Looks good otherwise AFAICT.

Andreas
Richard Henderson - Dec. 13, 2011, 6:19 p.m.
On 12/13/2011 09:44 AM, Alex Rozenman wrote:
> +            if (IS_V6M(env))
> +                goto illegal_op;

Also run your patch through scripts/checkpatch.pl.  Which should have
told you that you need braces here and elsewhere.


r~
Peter Maydell - Dec. 13, 2011, 11:13 p.m.
On 13 December 2011 18:11, Andreas Färber <afaerber@suse.de> wrote:
> Please rebase against Peter's target-arm branch:
>
> http://git.linaro.org/gitweb?p=people/pmaydell/qemu-arm.git;a=shortlog;h=refs/heads/target-arm.next

Note that this .next branch rebases, so treat it with the appropriate
degree of caution.

(I've submitted a pullreq for it anyway, so hopefully those changes
will go into master shortly.)

-- PMM
Alex_Rozenman@mentor.com - Dec. 14, 2011, 9:55 a.m.
Hi Peter,

Thank you for your comments. I'll submit a new patch. I am aware about
architertural differences b/w v6m and v7m, but many of them is less
relevant for the existing QEMU code (pririty bits for example).

Alex

On Tue, 2011-12-13 at 18:05 +0000, Peter Maydell wrote:
> On 13 December 2011 17:44, Alex Rozenman <Alex_Rozenman@mentor.com> wrote:
> > Please review / discuss / push the attched patch. It adds initial support
> > for V6-M for ARM and Cortex-M0.
> 
> Please don't submit patches as attachments (see
> http://wiki.qemu.org/Contribute/SubmitAPatch for other guidelines).
> 
> This patch is pretty obviously missing a lot. It makes a start
> on the instruction level differences, but v6M also has significant
> differences in the memory mapped system registers, NVIC, etc, which
> this patch doesn't try to deal with at all. There are also CPU model
> differences like a smaller set of supported MRS/MSR system registers,
> the M0 having no user/privileged distinction, etc.
> 
> The v6M Architecture Reference Manual has an appendix on the v6M-to-v7M
> differences, which is probably worth working through.
> 
> -- PMM
Peter Maydell - Dec. 14, 2011, 10:06 a.m.
On 14 December 2011 09:55, Alex Rozenman <Alex_Rozenman@mentor.com> wrote:
> Thank you for your comments. I'll submit a new patch. I am aware about
> architertural differences b/w v6m and v7m, but many of them is less
> relevant for the existing QEMU code (pririty bits for example).

My concern really is that at the moment we claim to support v7M
but it's pretty broken, which causes a steady trickle of people who
come along and point out that it doesn't actually work properly.
If we're going to add v6M then it needs to be a reasonably complete
and correct implementation.

-- PMM

Patch

From ff09b4ef970cde304c170c6e0a7678d2486b63e6 Mon Sep 17 00:00:00 2001
From: Alex Rozenman <Alex_Rozenman@mentor.com>
Date: Tue, 13 Dec 2011 19:28:39 +0200
Subject: [PATCH] Initial support for ARM V6-M.

Add an initial support for ARM V6-M architecture including
ARM Cortex-M0 core.

Signed-off-by: Alex Rozenman <Alex_Rozenman@mentor.com>
---
 target-arm/cpu.h       |    5 +++++
 target-arm/helper.c    |    8 ++++++++
 target-arm/translate.c |   26 ++++++++++++++++++++++++++
 3 files changed, 39 insertions(+), 0 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index c4d742f..3559c70 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -400,6 +400,10 @@  void cpu_arm_set_cp_io(CPUARMState *env, int cpnum,
    conventional cores (ie. Application or Realtime profile).  */
 
 #define IS_M(env) arm_feature(env, ARM_FEATURE_M)
+#define IS_V6M(env) (IS_M(env) && arm_feature(env, ARM_FEATURE_V6) && \
+                     !arm_feature(env, ARM_FEATURE_V7))
+#define IS_V7M(env) (IS_M(env) && arm_feature(env, ARM_FEATURE_V7))
+
 #define ARM_CPUID(env) (env->cp15.c0_cpuid)
 
 #define ARM_CPUID_ARM1026     0x4106a262
@@ -427,6 +431,7 @@  void cpu_arm_set_cp_io(CPUARMState *env, int cpnum,
 #define ARM_CPUID_ARM11MPCORE 0x410fb022
 #define ARM_CPUID_CORTEXA8    0x410fc080
 #define ARM_CPUID_CORTEXA9    0x410fc090
+#define ARM_CPUID_CORTEXM0    0x410cc200
 #define ARM_CPUID_CORTEXM3    0x410fc231
 #define ARM_CPUID_ANY         0xffffffff
 
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 3fe5822..cd1b157 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -186,6 +186,13 @@  static void cpu_reset_model_id(CPUARMState *env, uint32_t id)
         env->cp15.c0_ccsid[1] = 0x200fe015; /* 16k L1 icache. */
         env->cp15.c1_sys = 0x00c50078;
         break;
+    case ARM_CPUID_CORTEXM0:
+        set_feature(env, ARM_FEATURE_V4T);
+        set_feature(env, ARM_FEATURE_V5);
+        set_feature(env, ARM_FEATURE_V6);
+        set_feature(env, ARM_FEATURE_THUMB2);
+        set_feature(env, ARM_FEATURE_M);
+        break;
     case ARM_CPUID_CORTEXM3:
         set_feature(env, ARM_FEATURE_V4T);
         set_feature(env, ARM_FEATURE_V5);
@@ -425,6 +432,7 @@  static const struct arm_cpu_t arm_cpu_names[] = {
     { ARM_CPUID_ARM1136_R2, "arm1136-r2"},
     { ARM_CPUID_ARM1176, "arm1176"},
     { ARM_CPUID_ARM11MPCORE, "arm11mpcore"},
+    { ARM_CPUID_CORTEXM0, "cortex-m0"},
     { ARM_CPUID_CORTEXM3, "cortex-m3"},
     { ARM_CPUID_CORTEXA8, "cortex-a8"},
     { ARM_CPUID_CORTEXA9, "cortex-a9"},
diff --git a/target-arm/translate.c b/target-arm/translate.c
index f91553a..26afe5e 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -8141,6 +8141,28 @@  static int disas_thumb2_insn(CPUState *env, DisasContext *s, uint16_t insn_hw1)
         ARCH(6T2);
     }
 
+    if (IS_V6M(env)) {
+        /* Allowed only: BL, DMB, DSB, ISB, MRS, MSR. */
+        switch (insn & 0xfffffff0) {
+        case 0xf3bf8f40: /* dsb */
+        case 0xf3bf8f50: /* dmb */
+        case 0xf3bf8f60: /* isb */
+            return 0; /* Do not emulate. */
+        default: {
+            int ok = 0;
+            if ((insn & 0xfff0f000) == 0xf3808000) /* mrs */
+                ok = 1;
+            else if ((insn & 0xfffff000) == 0xf3ef8000) /* msr */
+                ok = 1;
+            else if ((insn & 0xf800d000) == 0xf000d000) /* bl */
+                ok = 1;
+            if (!ok)
+                goto illegal_op;
+            break;
+        }
+        }
+    }
+
     rn = (insn >> 16) & 0xf;
     rs = (insn >> 12) & 0xf;
     rd = (insn >> 8) & 0xf;
@@ -9659,6 +9681,8 @@  static void disas_thumb_insn(CPUState *env, DisasContext *s)
             break;
 
         case 1: case 3: case 9: case 11: /* czb */
+            if (IS_V6M(env))
+                goto illegal_op;
             rm = insn & 7;
             tmp = load_reg(s, rm);
             s->condlabel = gen_new_label();
@@ -9680,6 +9704,8 @@  static void disas_thumb_insn(CPUState *env, DisasContext *s)
                 break;
             }
             /* If Then.  */
+            if (IS_V6M(env))
+                goto illegal_op;
             s->condexec_cond = (insn >> 4) & 0xe;
             s->condexec_mask = insn & 0x1f;
             /* No actual code generated for this insn, just setup state.  */
-- 
1.7.8