diff mbox

ARM: OMAP5 / DRA7: Fix HYP mode boot for thumb2 build

Message ID 20170216000504.rkamjmm7dghkfs4h@squirrel.local (mailing list archive)
State New, archived
Headers show

Commit Message

Matthijs van Duin Feb. 16, 2017, 12:05 a.m. UTC
'adr' yields a data-pointer, not a function-pointer.

Fixes: 999f934de195 ("ARM: omap5/dra7xx: Enable booting secondary CPU in HYP mode")
Signed-off-by: Matthijs van Duin <matthijsvanduin@gmail.com>
---
 arch/arm/mach-omap2/omap-headsmp.S | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Lennart Sorensen Feb. 16, 2017, 8:03 p.m. UTC | #1
On Thu, Feb 16, 2017 at 01:05:04AM +0100, Matthijs van Duin wrote:
> 'adr' yields a data-pointer, not a function-pointer.
> 
> Fixes: 999f934de195 ("ARM: omap5/dra7xx: Enable booting secondary CPU in HYP mode")
> Signed-off-by: Matthijs van Duin <matthijsvanduin@gmail.com>
> ---
>  arch/arm/mach-omap2/omap-headsmp.S | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-omap2/omap-headsmp.S b/arch/arm/mach-omap2/omap-headsmp.S
> index fe36ce2734d4..4c6f14cf92a8 100644
> --- a/arch/arm/mach-omap2/omap-headsmp.S
> +++ b/arch/arm/mach-omap2/omap-headsmp.S
> @@ -17,6 +17,7 @@
>  
>  #include <linux/linkage.h>
>  #include <linux/init.h>
> +#include <asm/assembler.h>
>  
>  #include "omap44xx.h"
>  
> @@ -66,7 +67,7 @@ wait_2:	ldr	r2, =AUX_CORE_BOOT0_PA	@ read from AuxCoreBoot0
>  	cmp	r0, r4
>  	bne	wait_2
>  	ldr	r12, =API_HYP_ENTRY
> -	adr	r0, hyp_boot
> +	badr	r0, hyp_boot
>  	smc	#0
>  hyp_boot:
>  	b	omap_secondary_startup

I just based that on the code from the patch in
http://lists.infradead.org/pipermail/linux-arm-kernel/2013-November/213510.html
so if using badr is correct and adr is not, they certainly changing it
makes sense to me.  From what I can tell, using badr would make it work
correctly for a THUNB kernel build?  I certainly never tested that before
so it may very well be broken in that case.  I don't have access to the
hardware anymore to test it unfortunately.
Matthijs van Duin Feb. 16, 2017, 8:24 p.m. UTC | #2
On Thu, Feb 16, 2017 at 03:03:47PM -0500, Lennart Sorensen wrote:
> From what I can tell, using badr would make it work correctly for a
> THUMB kernel build?  I certainly never tested that before so it may
> very well be broken in that case.

It was.  My first try to boot a kernel in HYP mode resulted in:

[    0.156634] smp: Bringing up secondary CPUs ...
[    1.298996] CPU1: failed to come online

Replacing adr by badr fixes this, without any imapct on non-thumb builds
since there it simply expands to 'adr' again.

(I didn't know about this macro before, but it's all over the place in
arch/arm/kernel/*.S)

Matthijs
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lennart Sorensen Feb. 16, 2017, 8:57 p.m. UTC | #3
On Thu, Feb 16, 2017 at 09:24:48PM +0100, Matthijs van Duin wrote:
> On Thu, Feb 16, 2017 at 03:03:47PM -0500, Lennart Sorensen wrote:
> > From what I can tell, using badr would make it work correctly for a
> > THUMB kernel build?  I certainly never tested that before so it may
> > very well be broken in that case.
> 
> It was.  My first try to boot a kernel in HYP mode resulted in:
> 
> [    0.156634] smp: Bringing up secondary CPUs ...
> [    1.298996] CPU1: failed to come online
> 
> Replacing adr by badr fixes this, without any imapct on non-thumb builds
> since there it simply expands to 'adr' again.
> 
> (I didn't know about this macro before, but it's all over the place in
> arch/arm/kernel/*.S)

Well the macro certainly explains why it would work in ARM mode and not
THUMB mode.

Looks like a good fix then.
Tony Lindgren Feb. 16, 2017, 9:14 p.m. UTC | #4
* Lennart Sorensen <lsorense@csclub.uwaterloo.ca> [170216 12:59]:
> On Thu, Feb 16, 2017 at 09:24:48PM +0100, Matthijs van Duin wrote:
> > On Thu, Feb 16, 2017 at 03:03:47PM -0500, Lennart Sorensen wrote:
> > > From what I can tell, using badr would make it work correctly for a
> > > THUMB kernel build?  I certainly never tested that before so it may
> > > very well be broken in that case.
> > 
> > It was.  My first try to boot a kernel in HYP mode resulted in:
> > 
> > [    0.156634] smp: Bringing up secondary CPUs ...
> > [    1.298996] CPU1: failed to come online
> > 
> > Replacing adr by badr fixes this, without any imapct on non-thumb builds
> > since there it simply expands to 'adr' again.
> > 
> > (I didn't know about this macro before, but it's all over the place in
> > arch/arm/kernel/*.S)
> 
> Well the macro certainly explains why it would work in ARM mode and not
> THUMB mode.
> 
> Looks like a good fix then.

Applying into fixes thanks,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/arm/mach-omap2/omap-headsmp.S b/arch/arm/mach-omap2/omap-headsmp.S
index fe36ce2734d4..4c6f14cf92a8 100644
--- a/arch/arm/mach-omap2/omap-headsmp.S
+++ b/arch/arm/mach-omap2/omap-headsmp.S
@@ -17,6 +17,7 @@ 
 
 #include <linux/linkage.h>
 #include <linux/init.h>
+#include <asm/assembler.h>
 
 #include "omap44xx.h"
 
@@ -66,7 +67,7 @@  wait_2:	ldr	r2, =AUX_CORE_BOOT0_PA	@ read from AuxCoreBoot0
 	cmp	r0, r4
 	bne	wait_2
 	ldr	r12, =API_HYP_ENTRY
-	adr	r0, hyp_boot
+	badr	r0, hyp_boot
 	smc	#0
 hyp_boot:
 	b	omap_secondary_startup