ARM: keystone: remove hand-coded smc instruction

This lets us build keystone with CONFIG_THUMB2_KERNEL enabled, without the patch we get this error:

Error: cannot determine Thumb instruction size. Use .inst.n/.inst.w instead

This follows the same change for all other platforms done earlier.

Signed-off-by Arnd Bergmann <[email protected]>

Cc Santosh Shilimkar <[email protected]>

arch/arm/mach-keystone/platsmp.c +0 -0

index 630ab3bd..1800c33 100644

Quoted 16 lines · Arnd Bergmann

This lets us build keystone with CONFIG_THUMB2_KERNEL enabled, without the patch we get this error:

Error: cannot determine Thumb instruction size. Use .inst.n/.inst.w instead

This follows the same change for all other platforms done earlier.

Signed-off-by Arnd Bergmann <[email protected]>

Cc Santosh Shilimkar <[email protected]>

arch/arm/mach-keystone/platsmp.c +0 -0

index 630ab3bd..1800c33 100644

arch/arm/mach-keystone/platsmp.c static int __cpuinit keystone_smp_boot_secondary(unsigned int cpu, · line 40

Quoted 8 lines

arch/arm/mach-keystone/platsmp.c +1 -1

@@ -34,7 +34,7 @@ static int __cpuinit keystone_smp_boot_secondary(unsigned int cpu,
 		"mov    r0, #0\n"	/* power on cmd	*/
 		"mov    r1, %1\n"	/* cpu		*/
 		"mov    r2, %2\n"	/* start	*/
-		".inst  0xe1600070\n"	/* smc #0	*/
+		"smc	#0\n"		/* smc #0	*/
 		"mov    %0, r0\n"
 		: "=r" (error)
 		: "r"(cpu), "r"(start)

Ah, damm, now I get this error for building an ARMv6+ARMv7 combined kernel:

/tmp/ccSWHCik.s:55: Error: selected processor does not support ARM mode `smc #0'

I'll add a -march=armv7-a gcc flag as well.

Quoted 30 lines · Arnd Bergmann

Quoted 16 lines · Arnd Bergmann

This lets us build keystone with CONFIG_THUMB2_KERNEL enabled, without the patch we get this error:

Error: cannot determine Thumb instruction size. Use .inst.n/.inst.w instead

This follows the same change for all other platforms done earlier.

Signed-off-by Arnd Bergmann <[email protected]>

Cc Santosh Shilimkar <[email protected]>

arch/arm/mach-keystone/platsmp.c +0 -0

index 630ab3bd..1800c33 100644

arch/arm/mach-keystone/platsmp.c static int __cpuinit keystone_smp_boot_secondary(unsigned int cpu, · line 40

Quoted 8 lines

arch/arm/mach-keystone/platsmp.c +1 -1

@@ -34,7 +34,7 @@ static int __cpuinit keystone_smp_boot_secondary(unsigned int cpu,
             "mov    r0, #0\n"       /* power on cmd */
             "mov    r1, %1\n"       /* cpu          */
             "mov    r2, %2\n"       /* start        */
-            ".inst  0xe1600070\n"   /* smc #0       */
+            "smc    #0\n"           /* smc #0       */
             "mov    %0, r0\n"
             : "=r" (error)
             : "r"(cpu), "r"(start)

Ah, damm, now I get this error for building an ARMv6+ARMv7 combined kernel:

/tmp/ccSWHCik.s:55: Error: selected processor does not support ARM mode `smc #0'

Quoted 2 lines

I'll add a -march=armv7-a gcc flag as well.

I was just typing about the error. I started with smc first but then hit the below one which lead me to write an opcode. I couldn't compile c file for v7 only build and hence letf it in encoded format.

I will be moving this code to separate asm file along with other smc API support.

Quoted 37 lines · Santosh Shilimkar

Quoted 28 lines · Arnd Bergmann

Quoted 16 lines · Arnd Bergmann

This lets us build keystone with CONFIG_THUMB2_KERNEL enabled, without the patch we get this error:

Error: cannot determine Thumb instruction size. Use .inst.n/.inst.w instead

This follows the same change for all other platforms done earlier.

Signed-off-by Arnd Bergmann <[email protected]>

Cc Santosh Shilimkar <[email protected]>

arch/arm/mach-keystone/platsmp.c +0 -0

index 630ab3bd..1800c33 100644

arch/arm/mach-keystone/platsmp.c static int __cpuinit keystone_smp_boot_secondary(unsigned int cpu, · line 40

Quoted 8 lines

arch/arm/mach-keystone/platsmp.c +1 -1

@@ -34,7 +34,7 @@ static int __cpuinit keystone_smp_boot_secondary(unsigned int cpu,
             "mov    r0, #0\n"       /* power on cmd */
             "mov    r1, %1\n"       /* cpu          */
             "mov    r2, %2\n"       /* start        */
-            ".inst  0xe1600070\n"   /* smc #0       */
+            "smc    #0\n"           /* smc #0       */
             "mov    %0, r0\n"
             : "=r" (error)
             : "r"(cpu), "r"(start)

Ah, damm, now I get this error for building an ARMv6+ARMv7 combined kernel:

Quoted 4 lines

/tmp/ccSWHCik.s:55: Error: selected processor does not support ARM mode `smc #0'

I'll add a -march=armv7-a gcc flag as well.

I was just typing about the error. I started with smc first but then hit the below one which lead me to write an opcode. I couldn't compile c file for v7 only build and hence letf it in encoded format.

Quoted 3 lines

I will be moving this code to separate asm file along with other smc API support.

Ok, I'll wait for a patch from you then. My second attempt to fix it up also didn't work.

Quoted 1 line

I was curious how you will fix that for a c file. Just to be clear, I was planning to do that in 3.11-rcx/3.12 time. Let me know if it needs to be done earlier than that.

Quoted 4 lines · Santosh Shilimkar

Quoted 1 line

I was curious how you will fix that for a c file. Just to be clear, I was planning to do that in 3.11-rcx/3.12 time. Let me know if it needs to be done earlier than that.

It breaks randconfig builds on arm-soc at the moment, so I'd like the fix as early as possible for 3.11.

From 05d6a5b6cad624fb3791e8c1f8eb7c774f0790d9 Mon Sep 17 00:00:00 2001

Quoted 9 lines · Arnd Bergmann

Quoted 4 lines · Santosh Shilimkar

Quoted 1 line

I was curious how you will fix that for a c file. Just to be clear, I was planning to do that in 3.11-rcx/3.12 time. Let me know if it needs to be done earlier than that.

It breaks randconfig builds on arm-soc at the moment, so I'd like the fix as early as possible for 3.11.

Ok, fix is at end of the email. Let me know if it makes to pass both the builds now. I have build and boot tested both ARM and THUMB2 builds on Keystone board.

Regards,
Santosh

From 05d6a5b6cad624fb3791e8c1f8eb7c774f0790d9 Mon Sep 17 00:00:00 2001
From: Santosh Shilimkar <[email protected]>
Date: Fri, 21 Jun 2013 18:35:32 -0400
Subject: [PATCH] ARM: keystone: Move CPU bringup code to dedicated asm file

Because of inline asm usage in platsmp.c, smc instruction creates build failure for ARM V6+V7 build where as using instruction encoding for smc breaks the thumb2 build.

So move the code snippet to separate asm file and mark it with 'armv7-a$(plus_sec)' to avoid any build issues.

Cc Arnd Bergmann <[email protected]>

Signed-off-by Santosh Shilimkar <[email protected]>

---
 arch/arm/mach-keystone/Makefile   |    6 +++++-
 arch/arm/mach-keystone/keystone.h |    6 ++++++
 arch/arm/mach-keystone/platsmp.c  |   15 +++------------
 arch/arm/mach-keystone/smc.S      |   29 +++++++++++++++++++++++++++++
 4 files changed, 43 insertions(+), 13 deletions(-)
 create mode 100644 arch/arm/mach-keystone/smc.S

arch/arm/mach-keystone/Makefile +5 -1

index 3f6b8ab..ddc52b0 100644
@@ -1,2 +1,6 @@
-obj-y					:= keystone.o
+obj-y					:= keystone.o smc.o
+
+plus_sec := $(call as-instr,.arch_extension sec,+sec)
+AFLAGS_smc.o				:=-Wa,-march=armv7-a$(plus_sec)
+
 obj-$(CONFIG_SMP)			+= platsmp.o

arch/arm/mach-keystone/keystone.h +6 -0

index 43a1b47..60bef9d 100644
@@ -11,7 +11,13 @@
 #ifndef __KEYSTONE_H__
 #define __KEYSTONE_H__
 
+#define KEYSTONE_MON_CPU_UP_IDX		0x00
+
+#ifndef __ASSEMBLER__
+
 extern struct smp_operations keystone_smp_ops;
 extern void secondary_startup(void);
+extern u32 keystone_cpu_smc(u32 command, u32 cpu, u32 addr);
 
+#endif /* __ASSEMBLER__ */
 #endif /* __KEYSTONE_H__ */

arch/arm/mach-keystone/platsmp.c +3 -12

index 630ab3b..1d4181e 100644
@@ -30,18 +30,9 @@ static int __cpuinit keystone_smp_boot_secondary(unsigned int cpu,
 	pr_debug("keystone-smp: booting cpu %d, vector %08lx\n",
 		 cpu, start);
 
-	asm volatile (
-		"mov    r0, #0\n"	/* power on cmd	*/
-		"mov    r1, %1\n"	/* cpu		*/
-		"mov    r2, %2\n"	/* start	*/
-		".inst  0xe1600070\n"	/* smc #0	*/
-		"mov    %0, r0\n"
-		: "=r" (error)
-		: "r"(cpu), "r"(start)
-		: "cc", "r0", "r1", "r2", "memory"
-	);
-
-	pr_debug("keystone-smp: monitor returned %d\n", error);
+	error = keystone_cpu_smc(KEYSTONE_MON_CPU_UP_IDX, cpu, start);
+	if (error)
+		pr_err("CPU %d bringup failed with %d\n", cpu, error);
 
 	return error;
 }

arch/arm/mach-keystone/smc.S new file +13 -0

new file mode 100644
index 0000000..9b9e4f7
@@ -0,0 +1,29 @@
+/*
+ * Keystone Secure APIs
+ *
+ * Copyright (C) 2013 Texas Instruments, Inc.
+ * 	Santosh Shilimkar <[email protected]>
+ *
+ * This program is free software,you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/linkage.h>
+

Quoted 33 lines · Santosh Shilimkar

Quoted 9 lines · Arnd Bergmann

Quoted 4 lines · Santosh Shilimkar

Quoted 1 line

I was curious how you will fix that for a c file. Just to be clear, I was planning to do that in 3.11-rcx/3.12 time. Let me know if it needs to be done earlier than that.

It breaks randconfig builds on arm-soc at the moment, so I'd like the fix as early as possible for 3.11.

Ok, fix is at end of the email. Let me know if it makes to pass both the builds now. I have build and boot tested both ARM and THUMB2 builds on Keystone board.

Regards,
Santosh

From 05d6a5b6cad624fb3791e8c1f8eb7c774f0790d9 Mon Sep 17 00:00:00 2001
From: Santosh Shilimkar <[email protected]>
Date: Fri, 21 Jun 2013 18:35:32 -0400
Subject: [PATCH] ARM: keystone: Move CPU bringup code to dedicated asm file

Because of inline asm usage in platsmp.c, smc instruction creates build failure for ARM V6+V7 build where as using instruction encoding for smc breaks the thumb2 build.

So move the code snippet to separate asm file and mark it with 'armv7-a$(plus_sec)' to avoid any build issues.

Cc Arnd Bergmann <[email protected]>

Signed-off-by Santosh Shilimkar <[email protected]>

---

Quoted 19 lines

arch/arm/mach-keystone/smc.S new file +13 -0

new file mode 100644
index 0000000..9b9e4f7
@@ -0,0 +1,29 @@
+/*
+ * Keystone Secure APIs
+ *
+ * Copyright (C) 2013 Texas Instruments, Inc.
+ * 	Santosh Shilimkar <[email protected]>
+ *
+ * This program is free software,you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/linkage.h>
+

arch/arm/mach-keystone/smc.S line 23

Quoted 10 lines

arch/arm/mach-keystone/smc.S new file +10 -0

+/**
+ * u32 keystone_cpu_smc(u32 command, u32 cpu, u32 addr)
+ *
+ * Low level CPU monitor API
+ * @command:	Monitor command.
+ * @cpu:	CPU Number
+ * @addr:	Kernel jump address for boot CPU
+ *
+ * Return: Non zero value on failure
+ */

Oops, looks like I missed the final mail on this thread. Ignore my previous mail.

I still think it would be a good idea to try to consolidate all these trivial SMC wrappers, but this remains debatable.

Anyway, this looks like it should work, except:

arch/arm/mach-keystone/smc.S line 27

Quoted 4 lines

arch/arm/mach-keystone/smc.S new file +4 -0

+ENTRY(keystone_cpu_smc)
+	stmfd   sp!, {r4-r12, lr}
+	smc	#0
+	dsb

What's this DSB for? (You didn't have it in the inline asm version)

arch/arm/mach-keystone/smc.S line 29

Quoted 2 lines

arch/arm/mach-keystone/smc.S new file +2 -0

+	ldmfd   sp!, {r4-r12, pc}
+ENDPROC(keystone_cpu_smc)

r12 is caller-save btw; you don't need to preserve it.

Quoted 34 lines · Dave Martin

Quoted 33 lines · Santosh Shilimkar

Quoted 9 lines · Arnd Bergmann

Quoted 4 lines · Santosh Shilimkar

Quoted 1 line

I was curious how you will fix that for a c file. Just to be clear, I was planning to do that in 3.11-rcx/3.12 time. Let me know if it needs to be done earlier than that.

It breaks randconfig builds on arm-soc at the moment, so I'd like the fix as early as possible for 3.11.

Ok, fix is at end of the email. Let me know if it makes to pass both the builds now. I have build and boot tested both ARM and THUMB2 builds on Keystone board.

Regards,
Santosh

From 05d6a5b6cad624fb3791e8c1f8eb7c774f0790d9 Mon Sep 17 00:00:00 2001
From: Santosh Shilimkar <[email protected]>
Date: Fri, 21 Jun 2013 18:35:32 -0400
Subject: [PATCH] ARM: keystone: Move CPU bringup code to dedicated asm file

Because of inline asm usage in platsmp.c, smc instruction creates build failure for ARM V6+V7 build where as using instruction encoding for smc breaks the thumb2 build.

So move the code snippet to separate asm file and mark it with 'armv7-a$(plus_sec)' to avoid any build issues.

Cc Arnd Bergmann <[email protected]>

Signed-off-by Santosh Shilimkar <[email protected]>

---

[..]

Quoted 40 lines

Quoted 19 lines

arch/arm/mach-keystone/smc.S new file +13 -0

new file mode 100644
index 0000000..9b9e4f7
@@ -0,0 +1,29 @@
+/*
+ * Keystone Secure APIs
+ *
+ * Copyright (C) 2013 Texas Instruments, Inc.
+ * 	Santosh Shilimkar <[email protected]>
+ *
+ * This program is free software,you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/linkage.h>
+

arch/arm/mach-keystone/smc.S line 23

Quoted 10 lines

arch/arm/mach-keystone/smc.S new file +10 -0

+/**
+ * u32 keystone_cpu_smc(u32 command, u32 cpu, u32 addr)
+ *
+ * Low level CPU monitor API
+ * @command:	Monitor command.
+ * @cpu:	CPU Number
+ * @addr:	Kernel jump address for boot CPU
+ *
+ * Return: Non zero value on failure
+ */

Oops, looks like I missed the final mail on this thread. Ignore my previous mail.

I still think it would be a good idea to try to consolidate all these trivial SMC wrappers, but this remains debatable.

Anyway, this looks like it should work, except:

Quoted 7 lines

arch/arm/mach-keystone/smc.S line 27

Quoted 4 lines

arch/arm/mach-keystone/smc.S new file +4 -0

+ENTRY(keystone_cpu_smc)
+	stmfd   sp!, {r4-r12, lr}
+	smc	#0
+	dsb

What's this DSB for? (You didn't have it in the inline asm version)

Just to drain the write buffer before resuming on non-secure side.

I actually added it while moving it to asm file.

Quoted 5 lines

arch/arm/mach-keystone/smc.S line 29

Quoted 2 lines

arch/arm/mach-keystone/smc.S new file +2 -0

+	ldmfd   sp!, {r4-r12, pc}
+ENDPROC(keystone_cpu_smc)

r12 is caller-save btw; you don't need to preserve it.

Indeed. Will update it while adding some more SMC APIs. Its not harmful as such for now.

Quoted 85 lines · Santosh Shilimkar

Quoted 34 lines · Dave Martin

Quoted 33 lines · Santosh Shilimkar

Quoted 9 lines · Arnd Bergmann

Quoted 4 lines · Santosh Shilimkar

Quoted 1 line

I was curious how you will fix that for a c file. Just to be clear, I was planning to do that in 3.11-rcx/3.12 time. Let me know if it needs to be done earlier than that.

It breaks randconfig builds on arm-soc at the moment, so I'd like the fix as early as possible for 3.11.

Ok, fix is at end of the email. Let me know if it makes to pass both the builds now. I have build and boot tested both ARM and THUMB2 builds on Keystone board.

Regards,
Santosh

From 05d6a5b6cad624fb3791e8c1f8eb7c774f0790d9 Mon Sep 17 00:00:00 2001
From: Santosh Shilimkar <[email protected]>
Date: Fri, 21 Jun 2013 18:35:32 -0400
Subject: [PATCH] ARM: keystone: Move CPU bringup code to dedicated asm file

Because of inline asm usage in platsmp.c, smc instruction creates build failure for ARM V6+V7 build where as using instruction encoding for smc breaks the thumb2 build.

So move the code snippet to separate asm file and mark it with 'armv7-a$(plus_sec)' to avoid any build issues.

Cc Arnd Bergmann <[email protected]>

Signed-off-by Santosh Shilimkar <[email protected]>

---

[..]

Quoted 47 lines

Quoted 19 lines

arch/arm/mach-keystone/smc.S new file +13 -0

new file mode 100644
index 0000000..9b9e4f7
@@ -0,0 +1,29 @@
+/*
+ * Keystone Secure APIs
+ *
+ * Copyright (C) 2013 Texas Instruments, Inc.
+ * 	Santosh Shilimkar <[email protected]>
+ *
+ * This program is free software,you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/linkage.h>
+

arch/arm/mach-keystone/smc.S line 23

Quoted 10 lines

arch/arm/mach-keystone/smc.S new file +10 -0

+/**
+ * u32 keystone_cpu_smc(u32 command, u32 cpu, u32 addr)
+ *
+ * Low level CPU monitor API
+ * @command:	Monitor command.
+ * @cpu:	CPU Number
+ * @addr:	Kernel jump address for boot CPU
+ *
+ * Return: Non zero value on failure
+ */

Oops, looks like I missed the final mail on this thread. Ignore my previous mail.

I still think it would be a good idea to try to consolidate all these trivial SMC wrappers, but this remains debatable.

Anyway, this looks like it should work, except:

arch/arm/mach-keystone/smc.S line 27

Quoted 4 lines

arch/arm/mach-keystone/smc.S new file +4 -0

+ENTRY(keystone_cpu_smc)
+	stmfd   sp!, {r4-r12, lr}
+	smc	#0
+	dsb

What's this DSB for? (You didn't have it in the inline asm version)

Quoted 1 line

Just to drain the write buffer before resuming on non-secure side.

Why do you need to do that?

Quoted 7 lines

I actually added it while moving it to asm file.

Quoted 5 lines

arch/arm/mach-keystone/smc.S line 29

Quoted 2 lines

arch/arm/mach-keystone/smc.S new file +2 -0

+	ldmfd   sp!, {r4-r12, pc}
+ENDPROC(keystone_cpu_smc)

r12 is caller-save btw; you don't need to preserve it.

Quoted 2 lines

Indeed. Will update it while adding some more SMC APIs. Its not harmful as such for now.

If you could change that as soon as you make another modification to this file, that would be appreciated. These code snippets get cut and pasted recklessly.

Quoted 86 lines · Dave Martin

Quoted 85 lines · Santosh Shilimkar

Quoted 34 lines · Dave Martin

Quoted 33 lines · Santosh Shilimkar

Quoted 9 lines · Arnd Bergmann

Quoted 4 lines · Santosh Shilimkar

Quoted 1 line

I was curious how you will fix that for a c file. Just to be clear, I was planning to do that in 3.11-rcx/3.12 time. Let me know if it needs to be done earlier than that.

It breaks randconfig builds on arm-soc at the moment, so I'd like the fix as early as possible for 3.11.

Ok, fix is at end of the email. Let me know if it makes to pass both the builds now. I have build and boot tested both ARM and THUMB2 builds on Keystone board.

Regards,
Santosh

From 05d6a5b6cad624fb3791e8c1f8eb7c774f0790d9 Mon Sep 17 00:00:00 2001
From: Santosh Shilimkar <[email protected]>
Date: Fri, 21 Jun 2013 18:35:32 -0400
Subject: [PATCH] ARM: keystone: Move CPU bringup code to dedicated asm file

Because of inline asm usage in platsmp.c, smc instruction creates build failure for ARM V6+V7 build where as using instruction encoding for smc breaks the thumb2 build.

So move the code snippet to separate asm file and mark it with 'armv7-a$(plus_sec)' to avoid any build issues.

Cc Arnd Bergmann <[email protected]>

Signed-off-by Santosh Shilimkar <[email protected]>

---

[..]

Quoted 47 lines

Quoted 19 lines

arch/arm/mach-keystone/smc.S new file +13 -0

new file mode 100644
index 0000000..9b9e4f7
@@ -0,0 +1,29 @@
+/*
+ * Keystone Secure APIs
+ *
+ * Copyright (C) 2013 Texas Instruments, Inc.
+ * 	Santosh Shilimkar <[email protected]>
+ *
+ * This program is free software,you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/linkage.h>
+

arch/arm/mach-keystone/smc.S line 23

Quoted 10 lines

arch/arm/mach-keystone/smc.S new file +10 -0

+/**
+ * u32 keystone_cpu_smc(u32 command, u32 cpu, u32 addr)
+ *
+ * Low level CPU monitor API
+ * @command:	Monitor command.
+ * @cpu:	CPU Number
+ * @addr:	Kernel jump address for boot CPU
+ *
+ * Return: Non zero value on failure
+ */

Oops, looks like I missed the final mail on this thread. Ignore my previous mail.

I still think it would be a good idea to try to consolidate all these trivial SMC wrappers, but this remains debatable.

Anyway, this looks like it should work, except:

arch/arm/mach-keystone/smc.S line 27

Quoted 4 lines

arch/arm/mach-keystone/smc.S new file +4 -0

+ENTRY(keystone_cpu_smc)
+	stmfd   sp!, {r4-r12, lr}
+	smc	#0
+	dsb

What's this DSB for? (You didn't have it in the inline asm version)

Quoted 4 lines

Quoted 1 line

Just to drain the write buffer before resuming on non-secure side.

Why do you need to do that?

To commit any secure side pending writes. I don't remember exactly the issues but I remember facing issues in power management sequencing with SMC calls in between. That time a dsb did the trick. In fact I use to keep 1 before SMC and 1 after.

Quoted 37 lines · Santosh Shilimkar

Quoted 36 lines · Dave Martin

Quoted 35 lines · Santosh Shilimkar

Quoted 34 lines · Dave Martin

Quoted 33 lines · Santosh Shilimkar

Quoted 9 lines · Arnd Bergmann

Quoted 4 lines · Santosh Shilimkar

Quoted 1 line

I was curious how you will fix that for a c file. Just to be clear, I was planning to do that in 3.11-rcx/3.12 time. Let me know if it needs to be done earlier than that.

It breaks randconfig builds on arm-soc at the moment, so I'd like the fix as early as possible for 3.11.

Ok, fix is at end of the email. Let me know if it makes to pass both the builds now. I have build and boot tested both ARM and THUMB2 builds on Keystone board.

Regards,
Santosh

From 05d6a5b6cad624fb3791e8c1f8eb7c774f0790d9 Mon Sep 17 00:00:00 2001
From: Santosh Shilimkar <[email protected]>
Date: Fri, 21 Jun 2013 18:35:32 -0400
Subject: [PATCH] ARM: keystone: Move CPU bringup code to dedicated asm file

Because of inline asm usage in platsmp.c, smc instruction creates build failure for ARM V6+V7 build where as using instruction encoding for smc breaks the thumb2 build.

So move the code snippet to separate asm file and mark it with 'armv7-a$(plus_sec)' to avoid any build issues.

Cc Arnd Bergmann <[email protected]>

Signed-off-by Santosh Shilimkar <[email protected]>

---

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

Quoted 9 lines

Anyway, this looks like it should work, except:

arch/arm/mach-keystone/smc.S line 27

Quoted 4 lines

arch/arm/mach-keystone/smc.S new file +4 -0

+ENTRY(keystone_cpu_smc)
+	stmfd   sp!, {r4-r12, lr}
+	smc	#0
+	dsb

What's this DSB for? (You didn't have it in the inline asm version)

Quoted 8 lines

Quoted 4 lines

Quoted 1 line

Just to drain the write buffer before resuming on non-secure side.

Why do you need to do that?

To commit any secure side pending writes. I don't remember exactly the issues but I remember facing issues in power management sequencing with SMC calls in between. That time a dsb did the trick. In fact I use to keep 1 before SMC and 1 after.

Can we at least have a comment (as recommended in
Documentation/development-process/4.Coding)?

We don't want other people reading this and concluding that SMC always needs barriers -- in general, this shouldn't be necessary and could indicate bugs lurking somewhere.

Quoted 30 lines · Arnd Bergmann

Quoted 16 lines · Arnd Bergmann

This lets us build keystone with CONFIG_THUMB2_KERNEL enabled, without the patch we get this error:

Error: cannot determine Thumb instruction size. Use .inst.n/.inst.w instead

This follows the same change for all other platforms done earlier.

Signed-off-by Arnd Bergmann <[email protected]>

Cc Santosh Shilimkar <[email protected]>

arch/arm/mach-keystone/platsmp.c +0 -0

index 630ab3bd..1800c33 100644

arch/arm/mach-keystone/platsmp.c static int __cpuinit keystone_smp_boot_secondary(unsigned int cpu, · line 40

Quoted 8 lines

arch/arm/mach-keystone/platsmp.c +1 -1

@@ -34,7 +34,7 @@ static int __cpuinit keystone_smp_boot_secondary(unsigned int cpu,
 		"mov    r0, #0\n"	/* power on cmd	*/
 		"mov    r1, %1\n"	/* cpu		*/
 		"mov    r2, %2\n"	/* start	*/
-		".inst  0xe1600070\n"	/* smc #0	*/
+		"smc	#0\n"		/* smc #0	*/
 		"mov    %0, r0\n"
 		: "=r" (error)
 		: "r"(cpu), "r"(start)

Ah, damm, now I get this error for building an ARMv6+ARMv7 combined kernel:

/tmp/ccSWHCik.s:55: Error: selected processor does not support ARM mode `smc #0'

Quoted 2 lines

I'll add a -march=armv7-a gcc flag as well.

I was just typing about the error. I started with smc first but then hit the below one which lead me to write an opcode. I couldn't compile c file for v7 only build and hence letf it in encoded format.

I will be moving this code to separate asm file along with other smc API support.

Quoted 31 lines · Santosh Shilimkar

Quoted 30 lines · Arnd Bergmann

Quoted 16 lines · Arnd Bergmann

This lets us build keystone with CONFIG_THUMB2_KERNEL enabled, without the patch we get this error:

Error: cannot determine Thumb instruction size. Use .inst.n/.inst.w instead

This follows the same change for all other platforms done earlier.

Signed-off-by Arnd Bergmann <[email protected]>

Cc Santosh Shilimkar <[email protected]>

arch/arm/mach-keystone/platsmp.c +0 -0

index 630ab3bd..1800c33 100644

arch/arm/mach-keystone/platsmp.c static int __cpuinit keystone_smp_boot_secondary(unsigned int cpu, · line 40

Quoted 8 lines

arch/arm/mach-keystone/platsmp.c +1 -1

@@ -34,7 +34,7 @@ static int __cpuinit keystone_smp_boot_secondary(unsigned int cpu,
 		"mov    r0, #0\n"	/* power on cmd	*/
 		"mov    r1, %1\n"	/* cpu		*/
 		"mov    r2, %2\n"	/* start	*/
-		".inst  0xe1600070\n"	/* smc #0	*/
+		"smc	#0\n"		/* smc #0	*/
 		"mov    %0, r0\n"
 		: "=r" (error)
 		: "r"(cpu), "r"(start)

Ah, damm, now I get this error for building an ARMv6+ARMv7 combined kernel:

/tmp/ccSWHCik.s:55: Error: selected processor does not support ARM mode `smc #0'

Quoted 8 lines

Quoted 2 lines

I'll add a -march=armv7-a gcc flag as well.

I was just typing about the error. I started with smc first but then hit the below one which lead me to write an opcode. I couldn't compile c file for v7 only build and hence letf it in encoded format.

I will be moving this code to separate asm file along with other smc API support.

I think the correct way to fix this is to use ".arch_extension sec" and "smc" in the C inline asm, and put

CFLAGS_platsmp.o := -march=armv7-a

in arch/arm/mach-keystone/Makefile

This approach is followed in a few other places.

Or put the SMC in a separate .S file with suitable AFLAGS if not on a performance-critical hot path (for boot_secondary() it doesn't sounds like a problem).