Re: [boot-wrapper PATCH 04/12] Remove `flag_no_el3`

From: Andre Przywara <andre.przywara@arm.com>
To: Mark Rutland <mark.rutland@arm.com>
Cc: linux-arm-kernel@lists.infradead.org, Jaxson.Han@arm.com,
	Wei.Chen@arm.com
Subject: Re: [boot-wrapper PATCH 04/12] Remove `flag_no_el3`
Date: Fri, 30 Jul 2021 16:13:05 +0100	[thread overview]
Message-ID: <20210730161305.4064f576@slackpad.fritz.box> (raw)
In-Reply-To: <20210729152050.23635-5-mark.rutland@arm.com>

On Thu, 29 Jul 2021 16:20:42 +0100
Mark Rutland <mark.rutland@arm.com> wrote:

Hi,

> We set `flag_no_el3` when not booted at EL3 / monitor mode, and
> subsequently we use this to determine whether we need to drop exception
> level before entering Linux. As this can be derived from CurrentEL or
> CPSR, the flag itself is redundant, and we can defer the check until
> we're about to enter Linux.
> 
> In future this will allow more logic to be converted into C, where it
> will be easier to handle architectural variants.
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> ---
>  arch/aarch32/boot.S | 14 +++-----------
>  arch/aarch64/boot.S | 13 ++-----------
>  2 files changed, 5 insertions(+), 22 deletions(-)
> 
> diff --git a/arch/aarch32/boot.S b/arch/aarch32/boot.S
> index 2a85ad5..0bd1ca2 100644
> --- a/arch/aarch32/boot.S
> +++ b/arch/aarch32/boot.S
> @@ -31,9 +31,6 @@ ENTRY(_start)
>  	cmp	r0, #PSR_HYP
>  	bne	_switch_monitor

Can't this become "beq start_no_el3" now?

>  
> -	mov	r0, #1
> -	ldr	r1, =flag_no_el3
> -	str	r0, [r1]
>  	b	start_no_el3
>  
>  _switch_monitor:
> @@ -89,9 +86,9 @@ ENTRY(jump_kernel)
>  	ldr	lr, [r5], #4
>  	ldm	r5, {r0 - r2}
>  
> -	ldr	r4, =flag_no_el3
> -	ldr	r4, [r4]
> -	cmp	r4, #1
> +	mrs	r4, cpsr
> +	and	r4, #PSR_MODE_MASK
> +	cmp	r4, #PSR_MON

Is comparing explicitly against monitor mode the right thing? IIRC
normally we come out of reset in secure SVC, and this *is* EL3 (the
highest implemented exception level), from an ARMv8 perspective.
The old code did compare against HYP, which is probably what we want
and is also one of the few modes we are sure of being not EL3.

>  	bxeq	lr				@ no EL3
>  
>  	ldr	r4, =SPSR_KERNEL
> @@ -113,8 +110,3 @@ boot_vectors:
>  	b	.
>  	b	.
>  	b	.
> -
> -	.section .data
> -	.align 2
> -flag_no_el3:
> -	.long 0
> diff --git a/arch/aarch64/boot.S b/arch/aarch64/boot.S
> index 37759ce..fae0188 100644
> --- a/arch/aarch64/boot.S
> +++ b/arch/aarch64/boot.S
> @@ -28,10 +28,6 @@ _start:
>  	cmp	x0, #CURRENTEL_EL3
>  	b.eq	1f

Can't this become "b.ne start_no_el3" now?

Cheers,
Andre

>  
> -	mov	w0, #1
> -	ldr	x1, =flag_no_el3
> -	str	w0, [x1]
> -
>  	b	start_no_el3
>  
>  1:	mov	x0, #0x30			// RES1
> @@ -140,8 +136,8 @@ jump_kernel:
>  	bl	find_logical_id
>  	bl	setup_stack		// Reset stack pointer
>  
> -	ldr	w0, flag_no_el3
> -	cmp	w0, #0			// Prepare Z flag
> +	mrs	x0, CurrentEl
> +	cmp	w0, #CURRENTEL_EL3	// Prepare Z flag
>  
>  	mov	x0, x20
>  	mov	x1, x21
> @@ -164,8 +160,3 @@ jump_kernel:
>  	eret
>  
>  	.ltorg
> -
> -	.data
> -	.align 3
> -flag_no_el3:
> -	.long 0

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel