[PATCH] arm: boot/compressed: fix decompressor header layout for v7-M

[Dave Martin]

On Fri, May 19, 2017 at 03:59:59PM +0100, Ard Biesheuvel wrote:
> As reported by Patrice, the header layout of the decompressor is
> incorrect when building for v7-M. In this case, the __nop macro
> resolves to 'mov r0, r0', which is emitted as a narrow encoding,
> resulting in the header data fields to end up at lower offsets than
> required.
> 
> Given the variety of targets we need to support with the same code,
> the startup sequence is a bit of a jumble, and uses instructions
> and macros whose encoding width cannot be specified (badr), or only
> exists in a narrow encoding (bx)
> 
> So force the use of a wide encoding in __nop, and replace the start
> sequence with a simple jump to the label marking the start of code,
> preceded by a Thumb2 mode switch if required (using explicit wide
> encodings where appropriate). The label itself can be moved to the
> start of code [where it belongs] due to the larger range of branch
> instructions as compared to adr instructions.
> 
> Reported-by: Patrice CHOTARD <patrice.chotard at st.com>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel at linaro.org>
> ---
>  arch/arm/boot/compressed/efi-header.S |  4 +---
>  arch/arm/boot/compressed/head.S       | 14 +++++++-------
>  2 files changed, 8 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/arm/boot/compressed/efi-header.S b/arch/arm/boot/compressed/efi-header.S
> index 9d5dc4fda3c1..3f7d1b74c5e0 100644
> --- a/arch/arm/boot/compressed/efi-header.S
> +++ b/arch/arm/boot/compressed/efi-header.S
> @@ -17,14 +17,12 @@
>  		@ there.
>  		.inst	'M' | ('Z' << 8) | (0x1310 << 16)   @ tstne r0, #0x4d000
>  #else
> -		mov	r0, r0
> +		W(mov)	r0, r0
>  #endif
>  		.endm
>  
>  		.macro	__EFI_HEADER
>  #ifdef CONFIG_EFI_STUB
> -		b	__efi_start
> -
>  		.set	start_offset, __efi_start - start
>  		.org	start + 0x3c
>  		@
> diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
> index 7c711ba61417..8c336a6da210 100644
> --- a/arch/arm/boot/compressed/head.S
> +++ b/arch/arm/boot/compressed/head.S
> @@ -130,19 +130,19 @@ start:
>  		.rept	7
>  		__nop
>  		.endr
> -   ARM(		mov	r0, r0		)
> -   ARM(		b	1f		)
> - THUMB(		badr	r12, 1f		)
> - THUMB(		bx	r12		)
> +      ARM(	b	1f		)
> + AR_CLASS(	sub	pc, pc, #3	)	@ A/R: switch to Thumb2 mode
> +  M_CLASS(	nop.w			)	@ M: already in Thumb2 mode
> +    THUMB(	.thumb			)
> +    THUMB(	b.w	1f		)

It's not so easy to see that this always assembles to two words, or that
the "sub pc, pc, #3" is assembled but not executed in an ARM kernel.

Spelling it out might be more readable:

   ARM(			mov	r0, r0		)
   ARM(			mov	r0, r0		)

 THUMB(	AR_CLASS(	sub	pc, pc, #3	))
 THUMB(	M_CLASS(	nop.w			))
 THUMB(			.thumb			)
 THUMB(			b.w	1f		)

But I guess it works either way.

Cheers
---Dave

>  
>  		.word	_magic_sig	@ Magic numbers to help the loader
>  		.word	_magic_start	@ absolute load/run zImage address
>  		.word	_magic_end	@ zImage end address
>  		.word	0x04030201	@ endianness flag
>  
> - THUMB(		.thumb			)
> -1:		__EFI_HEADER
> -
> +		__EFI_HEADER
> +1:

[...]