[2/2] ARM: boot: Replace open-coded nop with macro
diff mbox series

Message ID 20190709130301.1916-2-linus.walleij@linaro.org
State Mainlined
Commit 6583d8298e5981876603b35926b5832f7e99a122
Headers show
Series
  • [1/2] ARM: boot: Explain the 8 nops
Related show

Commit Message

Linus Walleij July 9, 2019, 1:03 p.m. UTC
This open-coded nop as mov r0, r0 is a development history
artifact.

First commit b11fe38883d1
("ARM: 6663/1: make Thumb2 kernel entry point more similar
to the ARM one") moved the code around so that the nops
would come before the conditional thumb instructions, as it
turned out that some boot loaders were patching the initial
nop instructions in the kernel. At this point it is clear
that all mov r0,r0 are open-coded nops.

Then commit 81a0bc39ea19 ("ARM: add UEFI stub support")
moved things around and defined __nop for EFI support and
missed this open-coded nop.

commit 06a4b6d009a1
("ARM: 8677/1: boot/compressed: fix decompressor header
layout for v7-M") makes all invocations of __nop be wide,
but that is fine, because this is what we want: the
mov r0,r0 is inside ifndef CONFIG_THUMB2_KERNEL.

Cc: Nicolas Pitre <nico@fluxnic.net>
Cc: Roy Franz <roy.franz@cavium.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 arch/arm/boot/compressed/head.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Nicolas Pitre July 9, 2019, 3:02 p.m. UTC | #1
On Tue, 9 Jul 2019, Linus Walleij wrote:

> This open-coded nop as mov r0, r0 is a development history
> artifact.
> 
> First commit b11fe38883d1
> ("ARM: 6663/1: make Thumb2 kernel entry point more similar
> to the ARM one") moved the code around so that the nops
> would come before the conditional thumb instructions, as it
> turned out that some boot loaders were patching the initial
> nop instructions in the kernel. At this point it is clear
> that all mov r0,r0 are open-coded nops.
> 
> Then commit 81a0bc39ea19 ("ARM: add UEFI stub support")
> moved things around and defined __nop for EFI support and
> missed this open-coded nop.
> 
> commit 06a4b6d009a1
> ("ARM: 8677/1: boot/compressed: fix decompressor header
> layout for v7-M") makes all invocations of __nop be wide,
> but that is fine, because this is what we want: the
> mov r0,r0 is inside ifndef CONFIG_THUMB2_KERNEL.
> 
> Cc: Nicolas Pitre <nico@fluxnic.net>
> Cc: Roy Franz <roy.franz@cavium.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

For both patches:

Acked-by: Nicolas Pitre <nico@fluxnic.net>



> ---
>  arch/arm/boot/compressed/head.S | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
> index 544450c90673..93dffed0ac6e 100644
> --- a/arch/arm/boot/compressed/head.S
> +++ b/arch/arm/boot/compressed/head.S
> @@ -169,7 +169,7 @@ start:
>  		__nop
>  		.endr
>  #ifndef CONFIG_THUMB2_KERNEL
> -		mov	r0, r0
> +		__nop
>  #else
>   AR_CLASS(	sub	pc, pc, #3	)	@ A/R: switch to Thumb2 mode
>    M_CLASS(	nop.w			)	@ M: already in Thumb2 mode
> -- 
> 2.21.0
> 
>
Roy Franz July 12, 2019, 12:43 a.m. UTC | #2
On 7/9/19 6:03 AM, Linus Walleij wrote:
> External Email
> 
> ----------------------------------------------------------------------
> This open-coded nop as mov r0, r0 is a development history
> artifact.
> 
> First commit b11fe38883d1
> ("ARM: 6663/1: make Thumb2 kernel entry point more similar
> to the ARM one") moved the code around so that the nops
> would come before the conditional thumb instructions, as it
> turned out that some boot loaders were patching the initial
> nop instructions in the kernel. At this point it is clear
> that all mov r0,r0 are open-coded nops.
> 
> Then commit 81a0bc39ea19 ("ARM: add UEFI stub support")
> moved things around and defined __nop for EFI support and
> missed this open-coded nop.
> 
> commit 06a4b6d009a1
> ("ARM: 8677/1: boot/compressed: fix decompressor header
> layout for v7-M") makes all invocations of __nop be wide,
> but that is fine, because this is what we want: the
> mov r0,r0 is inside ifndef CONFIG_THUMB2_KERNEL.
> 
> Cc: Nicolas Pitre <nico@fluxnic.net>
> Cc: Roy Franz <roy.franz@cavium.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

For both patches
Acked-by: Roy Franz <rfranz@marvell.com>

> ---
>   arch/arm/boot/compressed/head.S | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
> index 544450c90673..93dffed0ac6e 100644
> --- a/arch/arm/boot/compressed/head.S
> +++ b/arch/arm/boot/compressed/head.S
> @@ -169,7 +169,7 @@ start:
>   		__nop
>   		.endr
>   #ifndef CONFIG_THUMB2_KERNEL
> -		mov	r0, r0
> +		__nop
>   #else
>    AR_CLASS(	sub	pc, pc, #3	)	@ A/R: switch to Thumb2 mode
>     M_CLASS(	nop.w			)	@ M: already in Thumb2 mode
>

Patch
diff mbox series

diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
index 544450c90673..93dffed0ac6e 100644
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -169,7 +169,7 @@  start:
 		__nop
 		.endr
 #ifndef CONFIG_THUMB2_KERNEL
-		mov	r0, r0
+		__nop
 #else
  AR_CLASS(	sub	pc, pc, #3	)	@ A/R: switch to Thumb2 mode
   M_CLASS(	nop.w			)	@ M: already in Thumb2 mode