linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Andre Przywara <andre.przywara@arm.com>
To: Jaxson Han <jaxson.han@arm.com>
Cc: mark.rutland@arm.com, linux-arm-kernel@lists.infradead.org,
	wei.chen@arm.com
Subject: Re: [boot-wrapper PATCH v2 2/8] aarch64: Rename labels and prepare for lower EL booting
Date: Mon, 24 May 2021 10:32:24 +0100	[thread overview]
Message-ID: <20210524103224.564dbfb2@slackpad.fritz.box> (raw)
In-Reply-To: <20210521104807.138269-3-jaxson.han@arm.com>

On Fri, 21 May 2021 18:48:01 +0800
Jaxson Han <jaxson.han@arm.com> wrote:

> Prepare for booting from lower EL. Rename *_el3 relavant labels with
> *_el_max and *_no_el3 with *_keep_el. Since the original _no_el3 means
> "We neither do init sequence at this highest EL nor drop to lower EL
> when entering to kernel", we rename it with _keep_el to make it more
> clear for lower EL initialisation.

Thanks for splitting out this patch, I verified that most of it is
indeed just renames, apart from the things below:

> 
> Signed-off-by: Jaxson Han <jaxson.han@arm.com>
> ---
>  arch/aarch64/boot.S            | 33 ++++++++++++++++++++++-----------
>  arch/aarch64/include/asm/cpu.h |  3 +++
>  arch/aarch64/psci.S            | 13 +++++++------
>  arch/aarch64/spin.S            |  8 ++++----
>  4 files changed, 36 insertions(+), 21 deletions(-)
> 
> diff --git a/arch/aarch64/boot.S b/arch/aarch64/boot.S
> index a9264de..e4f5f3d 100644
> --- a/arch/aarch64/boot.S
> +++ b/arch/aarch64/boot.S
> @@ -12,7 +12,7 @@
>  	.section .init
>  
>  	.globl	_start
> -	.globl jump_kernel
> +	.globl	jump_kernel
>  
>  _start:
>  	cpuid	x0, x1
> @@ -22,20 +22,31 @@ _start:
>  	bl	setup_stack
>  
>  	/*
> -	 * EL3 initialisation
> +	 * Boot sequence
> +	 * If CurrentEL == EL3, then goto EL3 initialisation and drop to
> +	 *   lower EL before entering the kernel.
> +	 * Else, no initialisation and keep the current EL before
> +	 *   entering the kernel.
>  	 */
>  	mrs	x0, CurrentEL
>  	cmp	x0, #CURRENTEL_EL3
> -	b.eq	1f
> +	beq	el3_init
>  
> +	/*
> +	 * We stay in the current EL for entering the kernel
> +	 */
>  	mov	w0, #1
> -	ldr	x1, =flag_no_el3
> +	ldr	x1, =flag_keep_el
>  	str	w0, [x1]
>  
>  	bl	setup_stack
> -	b	start_no_el3
> +	b	start_keep_el
>  
> -1:	mov	x0, #0x30			// RES1
> +	/*
> +	 * EL3 initialisation
> +	 */
> +el3_init:
> +	mov	x0, #0x30			// RES1
>  	orr	x0, x0, #(1 << 0)		// Non-secure EL1
>  	orr	x0, x0, #(1 << 8)		// HVC enable
>  
> @@ -114,7 +125,7 @@ _start:
>  
>  	bl	gic_secure_init
>  
> -	b	start_el3
> +	b	start_el_max
>  
>  err_invalid_id:
>  	b	.
> @@ -141,7 +152,7 @@ jump_kernel:
>  	bl	find_logical_id
>  	bl	setup_stack		// Reset stack pointer
>  
> -	ldr	w0, flag_no_el3
> +	ldr	w0, flag_keep_el
>  	cmp	w0, #0			// Prepare Z flag
>  
>  	mov	x0, x20
> @@ -150,9 +161,9 @@ jump_kernel:
>  	mov	x3, x23
>  
>  	b.eq	1f
> -	br	x19			// No EL3
> +	br	x19			// Keep current EL
>  
> -1:	mov	x4, #SPSR_KERNEL
> +1:	ldr	w4, #SPSR_KERNEL

This looks both premature and wrong: The value of SPSR_KERNEL doesn't
change here, so doesn't need any change in this instruction. Plus: ldr
without '=' requires an address, not a value, and this in fact already
breaks compilation:
arch/aarch64/boot.S:166: Error: immediate value must be a multiple of 4 at operand 2 -- `ldr w4,#((1<<8)|(1<<9)|(1<<7)|(1<<6)|(9<<0))'

So please remove this change, and also the changes in cpu.h below, and
introduce them only when they are actually needed.

Apart from the rest is then really just renames.

Cheers,
Andre


>  
>  	/*
>  	 * If bit 0 of the kernel address is set, we're entering in AArch32
> @@ -168,5 +179,5 @@ jump_kernel:
>  
>  	.data
>  	.align 3
> -flag_no_el3:
> +flag_keep_el:
>  	.long 0
> diff --git a/arch/aarch64/include/asm/cpu.h b/arch/aarch64/include/asm/cpu.h
> index ccb5397..2b3a0a4 100644
> --- a/arch/aarch64/include/asm/cpu.h
> +++ b/arch/aarch64/include/asm/cpu.h
> @@ -11,6 +11,7 @@
>  
>  #define MPIDR_ID_BITS		0xff00ffffff
>  
> +#define CURRENTEL_EL2		(2 << 2)
>  #define CURRENTEL_EL3		(3 << 2)
>  
>  /*
> @@ -24,6 +25,7 @@
>  #define SPSR_I			(1 << 7)	/* IRQ masked */
>  #define SPSR_F			(1 << 6)	/* FIQ masked */
>  #define SPSR_T			(1 << 5)	/* Thumb */
> +#define SPSR_EL1H		(5 << 0)	/* EL1 Handler mode */
>  #define SPSR_EL2H		(9 << 0)	/* EL2 Handler mode */
>  #define SPSR_HYP		(0x1a << 0)	/* M[3:0] = hyp, M[4] = AArch32 */
>  
> @@ -42,6 +44,7 @@
>  #else
>  #define SCTLR_EL1_RESET		SCTLR_EL1_RES1
>  #define SPSR_KERNEL		(SPSR_A | SPSR_D | SPSR_I | SPSR_F | SPSR_EL2H)
> +#define SPSR_KERNEL_EL1		(SPSR_A | SPSR_D | SPSR_I | SPSR_F | SPSR_EL1H)
>  #endif
>  
>  #ifndef __ASSEMBLY__
> diff --git a/arch/aarch64/psci.S b/arch/aarch64/psci.S
> index 01ebe7d..ae02fd6 100644
> --- a/arch/aarch64/psci.S
> +++ b/arch/aarch64/psci.S
> @@ -45,8 +45,8 @@ vector:
>  
>  	.text
>  
> -	.globl start_no_el3
> -	.globl start_el3
> +	.globl start_keep_el
> +	.globl start_el_max
>  
>  err_exception:
>  	b err_exception
> @@ -101,7 +101,7 @@ smc_exit:
>  	eret
>  
>  
> -start_el3:
> +start_el_max:
>  	ldr	x0, =vector
>  	bl	setup_vector
>  
> @@ -111,10 +111,11 @@ start_el3:
>  	b	psci_first_spin
>  
>  /*
> - * This PSCI implementation requires EL3. Without EL3 we'll only boot the
> - * primary cpu, all others will be trapped in an infinite loop.
> + * This PSCI implementation requires the highest EL(EL3 or Armv8-R EL2).
> + * Without the highest EL, we'll only boot the primary cpu, all others
> + * will be trapped in an infinite loop.
>   */
> -start_no_el3:
> +start_keep_el:
>  	cpuid	x0, x1
>  	bl	find_logical_id
>  	cbz	x0, psci_first_spin
> diff --git a/arch/aarch64/spin.S b/arch/aarch64/spin.S
> index 72603cf..533177c 100644
> --- a/arch/aarch64/spin.S
> +++ b/arch/aarch64/spin.S
> @@ -11,11 +11,11 @@
>  
>  	.text
>  
> -	.globl start_no_el3
> -	.globl start_el3
> +	.globl start_keep_el
> +	.globl start_el_max
>  
> -start_el3:
> -start_no_el3:
> +start_el_max:
> +start_keep_el:
>  	cpuid	x0, x1
>  	bl	find_logical_id
>  


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

  reply	other threads:[~2021-05-24 17:13 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-21 10:47 [boot-wrapper PATCH v2 0/8] Add Armv8-R AArch64 support Jaxson Han
2021-05-21 10:48 ` [boot-wrapper PATCH v2 1/8] Decouple V2M_SYS config by auto-detect dtb node Jaxson Han
2021-05-21 10:48 ` [boot-wrapper PATCH v2 2/8] aarch64: Rename labels and prepare for lower EL booting Jaxson Han
2021-05-24  9:32   ` Andre Przywara [this message]
2021-05-25  1:54     ` Jaxson Han
2021-05-21 10:48 ` [boot-wrapper PATCH v2 3/8] aarch64: Remove the redundant setup_stack Jaxson Han
2021-05-24  9:32   ` Andre Przywara
2021-05-21 10:48 ` [boot-wrapper PATCH v2 4/8] aarch64: Prepare for EL1 booting Jaxson Han
2021-05-24  9:32   ` Andre Przywara
2021-05-21 10:48 ` [boot-wrapper PATCH v2 5/8] aarch64: Prepare for lower EL booting Jaxson Han
2021-05-24  9:33   ` Andre Przywara
2021-05-21 10:48 ` [boot-wrapper PATCH v2 6/8] gic-v3: Prepare for gicv3 with EL2 Jaxson Han
2021-05-24  9:33   ` Andre Przywara
2021-05-21 10:48 ` [boot-wrapper PATCH v2 7/8] aarch64: Prepare for booting " Jaxson Han
2021-05-24  9:33   ` Andre Przywara
2021-05-21 10:48 ` [boot-wrapper PATCH v2 8/8] aarch64: Introduce EL2 boot code for Armv8-R AArch64 Jaxson Han
2021-05-24 10:19   ` Andre Przywara
2021-05-25  1:59     ` Jaxson Han

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210524103224.564dbfb2@slackpad.fritz.box \
    --to=andre.przywara@arm.com \
    --cc=jaxson.han@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=mark.rutland@arm.com \
    --cc=wei.chen@arm.com \
    --subject='Re: [boot-wrapper PATCH v2 2/8] aarch64: Rename labels and prepare for lower EL booting' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).