ARM: zImage: add support for ARMv7-M

arch/arm/boot/compressed/head.S __hyp_reentry_vectors: · line 1298

Quoted 7 lines · Joachim Eastwood

  ARM(          mov     pc, r4  )               @ call kernel
- THUMB(                bx      r4      )               @ entry point is always ARM

arch/arm/boot/compressed/head.S +4 -0

+#ifdef CONFIG_CPU_V7M
+               add     r4, r4, #1              @ enter in Thumb mode for ARMv7M
+#endif
+ THUMB(                bx      r4      )               @ entry point is always ARM for non ARMv7M CPUs

I think it would be much nicer to avoid sprinkling #ifdefs here. We already have the ARM() and THUMB() macros to deal with the two cases we support, which are booting in ARM mode vs ARMv7-A with THUMB2 mode. We can probably add another macro like this to deal with the ARMv7-M case that does not have ARM mode.

Quoted 14 lines · Arnd Bergmann

arch/arm/boot/compressed/head.S __hyp_reentry_vectors: · line 1298

Quoted 7 lines · Joachim Eastwood

  ARM(          mov     pc, r4  )               @ call kernel
- THUMB(                bx      r4      )               @ entry point is always ARM

arch/arm/boot/compressed/head.S +4 -0

+#ifdef CONFIG_CPU_V7M
+               add     r4, r4, #1              @ enter in Thumb mode for ARMv7M
+#endif
+ THUMB(                bx      r4      )               @ entry point is always ARM for non ARMv7M CPUs

I think it would be much nicer to avoid sprinkling #ifdefs here. We already have the ARM() and THUMB() macros to deal with the two cases we support, which are booting in ARM mode vs ARMv7-A with THUMB2 mode. We can probably add another macro like this to deal with the ARMv7-M case that does not have ARM mode.

Well, I guess I could make a THUMBONLY macro or something like that. But I think that the macro would only be useful in the case you quoted above. The other ifdefs in the patch are either for large blocks or removing code. So I don't think it would improve the patch a great deal.

btw, do you have better name suggestion than THUMBONLY?

Quoted 22 lines · Joachim Eastwood

Quoted 14 lines · Arnd Bergmann

arch/arm/boot/compressed/head.S __hyp_reentry_vectors: · line 1298

Quoted 7 lines · Joachim Eastwood

  ARM(          mov     pc, r4  )               @ call kernel
- THUMB(                bx      r4      )               @ entry point is always ARM

arch/arm/boot/compressed/head.S +4 -0

+#ifdef CONFIG_CPU_V7M
+               add     r4, r4, #1              @ enter in Thumb mode for ARMv7M
+#endif
+ THUMB(                bx      r4      )               @ entry point is always ARM for non ARMv7M CPUs

I think it would be much nicer to avoid sprinkling #ifdefs here. We already have the ARM() and THUMB() macros to deal with the two cases we support, which are booting in ARM mode vs ARMv7-A with THUMB2 mode. We can probably add another macro like this to deal with the ARMv7-M case that does not have ARM mode.

Well, I guess I could make a THUMBONLY macro or something like that. But I think that the macro would only be useful in the case you quoted above. The other ifdefs in the patch are either for large blocks or removing code. So I don't think it would improve the patch a great deal.

Quoted 1 line

btw, do you have better name suggestion than THUMBONLY?

It looks like you would need the reverse as well, !THUMBONLY. What about M_CLASS and A_CLASS (with or without underscore and maybe AR_CLASS if we ever need to differentiate between A and R)?

Quoted 25 lines · Catalin Marinas

Quoted 23 lines · Joachim Eastwood

Quoted 14 lines · Arnd Bergmann

arch/arm/boot/compressed/head.S __hyp_reentry_vectors: · line 1298

Quoted 7 lines · Joachim Eastwood

  ARM(          mov     pc, r4  )               @ call kernel
- THUMB(                bx      r4      )               @ entry point is always ARM

arch/arm/boot/compressed/head.S +4 -0

+#ifdef CONFIG_CPU_V7M
+               add     r4, r4, #1              @ enter in Thumb mode for ARMv7M
+#endif
+ THUMB(                bx      r4      )               @ entry point is always ARM for non ARMv7M CPUs

I think it would be much nicer to avoid sprinkling #ifdefs here. We already have the ARM() and THUMB() macros to deal with the two cases we support, which are booting in ARM mode vs ARMv7-A with THUMB2 mode. We can probably add another macro like this to deal with the ARMv7-M case that does not have ARM mode.

Well, I guess I could make a THUMBONLY macro or something like that. But I think that the macro would only be useful in the case you quoted above. The other ifdefs in the patch are either for large blocks or removing code. So I don't think it would improve the patch a great deal.

btw, do you have better name suggestion than THUMBONLY?

Quoted 3 lines

It looks like you would need the reverse as well, !THUMBONLY. What about M_CLASS and A_CLASS (with or without underscore and maybe AR_CLASS if we ever need to differentiate between A and R)?

Yes, that would remove more of the ifdefs.
With M_CLASS/A_CLASS macros I could also wrap ".arch armv7-m" and ".arm" directives. But A_CLASS might look a bit strange when used for ARM CPUs that are not called Cortex.

Quoted 3 lines · Joachim Eastwood

.align
- .arm @ Always enter in ARM state
+ AR_CLASS( .arm ) @ Always enter in ARM state for AR class

Nitpick: A/R class (or classes).

arch/arm/boot/compressed/head.S __common_mmu_cache_on: · line 798

Quoted 2 lines

arch/arm/boot/compressed/head.S +2 -0

+#elif defined(CONFIG_CPU_V7M)
+             ldr     r9, =0xe000ed00         @ CPUID register address

Another nitpick: can you use some macro? We define them in arch/arm/include/asm/v7m.h, so you could write the above as (untested):

ldr r9, =BASEADDR_V7M_SCB + V7M_SCB_CPUID

This patch makes it possible to enter zImage in Thumb mode for ARMv7M (Cortex-M) CPUs that does not support ARM mode. The kernel entry is also made in Thumb mode.

Signed-off-by Joachim Eastwood <[email protected]>

---
Hi,

Updated patch per comments from Catalin Marinas.

Changes
v3: Use defines for ARMv7M CPU registers.
v2: Introduce AR_CLASS/M_CLASS macros. This reduces the amount
of ifdefs needed in compressed/head.S.

Successfully tested on NXP LPC4357 (Cortex-M4).

 arch/arm/boot/compressed/head.S | 19 +++++++++++++++----
 arch/arm/include/asm/unified.h  |  8 ++++++++
 2 files changed, 23 insertions(+), 4 deletions(-)

arch/arm/boot/compressed/head.S +4 -1

index 413fd94b5301..cd27090625fe 100644
@@ -10,8 +10,11 @@
  */
 #include <linux/linkage.h>
 #include <asm/assembler.h>
+#include <asm/v7m.h>
+
+ AR_CLASS(	.arch	armv7-a	)
+ M_CLASS(	.arch	armv7-m	)
 
-	.arch	armv7-a
 /*
  * Debugging stuff
  *

Hello Joachim,

Quoted 34 lines · Joachim Eastwood

This patch makes it possible to enter zImage in Thumb mode for ARMv7M (Cortex-M) CPUs that does not support ARM mode. The kernel entry is also made in Thumb mode.

Signed-off-by Joachim Eastwood <[email protected]>

---
Hi,

Updated patch per comments from Catalin Marinas.

Changes
v3: Use defines for ARMv7M CPU registers.
v2: Introduce AR_CLASS/M_CLASS macros. This reduces the amount
of ifdefs needed in compressed/head.S.

Successfully tested on NXP LPC4357 (Cortex-M4).

 arch/arm/boot/compressed/head.S | 19 +++++++++++++++----
 arch/arm/include/asm/unified.h  |  8 ++++++++
 2 files changed, 23 insertions(+), 4 deletions(-)

arch/arm/boot/compressed/head.S +4 -0

index 413fd94b5301..cd27090625fe 100644
@@ -10,8 +10,11 @@
  */
 #include <linux/linkage.h>
 #include <asm/assembler.h>
+#include <asm/v7m.h>
+
+ AR_CLASS(   .arch   armv7-a )
+ M_CLASS(    .arch   armv7-m )
 

arch/arm/boot/compressed/head.S line 120

Quoted 10 lines

arch/arm/boot/compressed/head.S +1 -2

-     .arch   armv7-a
 /*
  * Debugging stuff
  *

@@ -114,7 +117,7 @@
  * sort out different calling conventions
  */
              .align
-             .arm                            @ Always enter in ARM state
+ AR_CLASS(   .arm    )               @ Always enter in ARM state for A/R classes

Semantically we don't want .arm when CPU_THUMBONLY is enabled. At least currently this is equivalent with !AR_CLASS. Maybe this is worth to be pointed out in a comment?

arch/arm/boot/compressed/head.S start: · line 142

Quoted 11 lines

arch/arm/boot/compressed/head.S +1 -0

 start:
              .type   start,#function
              .rept   7

@@ -133,6 +136,7 @@ start:
  THUMB(              .thumb                  )
 1:
  ARM_BE8(    setend  be )                    @ go BE8 if compiled for BE8
+#ifndef CONFIG_CPU_V7M
              mrs     r9, cpsr
 #ifdef CONFIG_ARM_VIRT_EXT
              bl      __hyp_stub_install      @ get into SVC mode, reversibly

I think you need to be more precious here. The following is #ifdef'd out:
#endif

                mov     r7, r1                  @ save architecture ID
                mov     r8, r2                  @ save atags pointer

/*
* Booting from Angel - need to enter SVC mode and disable
* FIQs/IRQs (numeric definitions from angel arm.h source).
* We only do this if we were in user mode on entry. */
mrs r2, cpsr @ get current mode
tst r2, #3 @ not user?
bne not_angel
mov r0, #0x17 @ angel_SWIreason_EnterSVC
ARM( swi 0x123456 ) @ angel_SWI_ARM
THUMB( svc 0xab ) @ angel_SWI_THUMB
not_angel:

arch/arm/boot/compressed/head.S not_angel: · line 162

Quoted 5 lines

arch/arm/boot/compressed/head.S +1 -0

@@ -155,6 +159,7 @@ not_angel:
              safe_svcmode_maskall r0
              msr     spsr_cxsf, r9           @ Save the CPU boot mode in
                                              @ SPSR
+#endif

At least saving r1 and r2 should be preserved, shouldn't it? I wonder how this could result in a working kernel because r7 and r8 are passed to Image which are not initialized with your patch.

Quoted 6 lines · Uwe Kleine-K?nig

Quoted 6 lines

arch/arm/boot/compressed/head.S +0 -0

index 413fd94b5301..cd27090625fe 100644
[...]

@@ -114,7 +117,7 @@

Quoted 8 lines

arch/arm/boot/compressed/head.S line 120

Quoted 5 lines

arch/arm/boot/compressed/head.S +1 -1

  * sort out different calling conventions
  */
              .align
-             .arm                            @ Always enter in ARM state
+ AR_CLASS(   .arm    )               @ Always enter in ARM state for A/R classes

Semantically we don't want .arm when CPU_THUMBONLY is enabled. At least currently this is equivalent with !AR_CLASS. Maybe this is worth to be pointed out in a comment?

Won't THUMBONLY always mean !AR_CLASS?

Do you have a particular comment in mind?

This patch makes it possible to enter zImage in Thumb mode for ARMv7M

(Cortex-M) CPUs that does not support ARM mode. The kernel entry is also made in Thumb mode.

Signed-off-by Joachim Eastwood <[email protected]>

---
Hi,

Updated patch with comments from Uwe Kleine-K?nig.

Changes
v4: Preserve r1/r2. Add comment about class A/R.
v3: Use defines for ARMv7M CPU registers.
v2: Introduce AR_CLASS/M_CLASS macros. This reduces the amount
of ifdefs needed in compressed/head.S.

Successfully tested on NXP LPC4357 (Cortex-M4).

regards,
Joachim Eastwood

 arch/arm/boot/compressed/head.S | 28 ++++++++++++++++++++++------
 arch/arm/include/asm/unified.h  |  8 ++++++++
 2 files changed, 30 insertions(+), 6 deletions(-)

arch/arm/boot/compressed/head.S +17 -4

index 413fd94b5301..0afcbb1e1d7d 100644
@@ -10,8 +10,11 @@
  */
 #include <linux/linkage.h>
 #include <asm/assembler.h>
+#include <asm/v7m.h>
+
+ AR_CLASS(	.arch	armv7-a	)
+ M_CLASS(	.arch	armv7-m	)
 
-	.arch	armv7-a
 /*
  * Debugging stuff
  *

@@ -114,7 +117,12 @@
  * sort out different calling conventions
  */
 		.align
-		.arm				@ Always enter in ARM state
+		/*
+		 * Always enter in ARM state for CPUs that support the ARM ISA.
+		 * As of today (2014) that's exactly the members of the A and R
+		 * classes.
+		 */
+ AR_CLASS(	.arm	)
 start:
 		.type	start,#function
 		.rept	7

@@ -132,14 +140,15 @@ start:
 
  THUMB(		.thumb			)
 1:
- ARM_BE8(	setend	be )			@ go BE8 if compiled for BE8
-		mrs	r9, cpsr
+ ARM_BE8(	setend	be		)	@ go BE8 if compiled for BE8
+ AR_CLASS(	mrs	r9, cpsr	)
 #ifdef CONFIG_ARM_VIRT_EXT
 		bl	__hyp_stub_install	@ get into SVC mode, reversibly
 #endif
 		mov	r7, r1			@ save architecture ID
 		mov	r8, r2			@ save atags pointer
 
+#ifndef CONFIG_CPU_V7M
 		/*
 		 * Booting from Angel - need to enter SVC mode and disable
 		 * FIQs/IRQs (numeric definitions from angel arm.h source).

@@ -155,6 +164,7 @@ not_angel:
 		safe_svcmode_maskall r0
 		msr	spsr_cxsf, r9		@ Save the CPU boot mode in
 						@ SPSR
+#endif
 		/*
 		 * Note that some cache flushing and other stuff may
 		 * be needed here - is there an Angel SWI call for this?

@@ -790,6 +800,9 @@ __common_mmu_cache_on:
 call_cache_fn:	adr	r12, proc_types
 #ifdef CONFIG_CPU_CP15
 		mrc	p15, 0, r9, c0, c0	@ get processor ID
+#elif defined(CONFIG_CPU_V7M)
+		ldr	r9, =BASEADDR_V7M_SCB
+		ldr	r9, [r9, V7M_SCB_CPUID]
 #else
 		ldr	r9, =CONFIG_PROCESSOR_ID
 #endif