[BOOT-WRAPPER v2 08/10] Simplify spin logic

[Andre Przywara]

On Mon, 12 Aug 2024 11:15:53 +0100
Mark Rutland <mark.rutland at arm.com> wrote:

> The logic for initial boot is more complicated than it needs to be,
> with both first_spin() having a special case for CPU0 that requires an
> additional argument to be passed to spin().
> 
> Simplify this by moving the special-case logic for CPU0 into
> first_spin(). This removes the need to provide a dummy mailbox for CPU0
> to spin on, simplfiies callers of first_spin() and spin(), which no
> longer need to pass a dummy mailbox or 'is_entry' for CPU0.

Ah, that's a nice one! Indeed this looks much cleaner now. The change
looks good to me, just some small thing below:

> Signed-off-by: Mark Rutland <mark.rutland at arm.com>
> Acked-by: Marc Zyngier <maz at kernel.org>
> Cc: Akos Denke <akos.denke at arm.com>
> Cc: Andre Przywara <andre.przywara at arm.com>
> Cc: Luca Fancellu <luca.fancellu at arm.com>
> ---
>  arch/aarch64/spin.S | 11 +----------
>  common/boot.c       | 20 ++++++++------------
>  common/psci.c       |  2 +-
>  include/boot.h      |  2 +-
>  4 files changed, 11 insertions(+), 24 deletions(-)
> 
> diff --git a/arch/aarch64/spin.S b/arch/aarch64/spin.S
> index 375f732..a7879d4 100644
> --- a/arch/aarch64/spin.S
> +++ b/arch/aarch64/spin.S
> @@ -23,15 +23,6 @@ ASM_FUNC(start_bootmethod)
>  	 * Primary CPU (x0 = 0) jumps to kernel, the other ones wait for an
>  	 * address to appear in mbox

I think this comment is a bit out of place now, either remove it entirely,
or hint that first_spin will take care of the difference between mbox and
kernel entrypoint.

Regardless:

Reviewed-by: Andre Przywara <andre.przywara at arm.com>

Cheers,
Andre

>  	 */
> -	adr	x3, mbox
> -	adr	x4, kernel_address
> -	cmp	x0, #0
> -	csel	x1, x3, x4, ne
> +	adr	x1, mbox
>  	mov	x2, #0
>  	bl	first_spin
> -
> -	.align 3
> -kernel_address:
> -	.long 0
> -
> -	.ltorg
> diff --git a/common/boot.c b/common/boot.c
> index 29d53a4..4417649 100644
> --- a/common/boot.c
> +++ b/common/boot.c
> @@ -27,7 +27,7 @@ const unsigned long id_table[] = { CPU_IDS };
>   * @invalid: value of an invalid address, 0 or -1 depending on the boot method
>   * @is_entry: when true, pass boot parameters to the kernel, instead of 0
>   */
> -void __noreturn spin(unsigned long *mbox, unsigned long invalid, int is_entry)
> +void __noreturn spin(unsigned long *mbox, unsigned long invalid)
>  {
>  	unsigned long addr = invalid;
>  
> @@ -36,13 +36,6 @@ void __noreturn spin(unsigned long *mbox, unsigned long invalid, int is_entry)
>  		addr = *mbox;
>  	}
>  
> -	if (is_entry)
> -#ifdef KERNEL_32
> -		jump_kernel(addr, 0, ~0, (unsigned long)&dtb, 0);
> -#else
> -		jump_kernel(addr, (unsigned long)&dtb, 0, 0, 0);
> -#endif
> -
>  	jump_kernel(addr, 0, 0, 0, 0);
>  
>  	unreachable();
> @@ -60,12 +53,15 @@ void __noreturn first_spin(unsigned int cpu, unsigned long *mbox,
>  			   unsigned long invalid)
>  {
>  	if (cpu == 0) {
> -		*mbox = (unsigned long)&entrypoint;
> -		sevl();
> -		spin(mbox, invalid, 1);
> +		unsigned long addr = (unsigned long)&entrypoint;
> +#ifdef KERNEL_32
> +		jump_kernel(addr, 0, ~0, (unsigned long)&dtb, 0);
> +#else
> +		jump_kernel(addr, (unsigned long)&dtb, 0, 0, 0);
> +#endif
>  	} else {
>  		*mbox = invalid;
> -		spin(mbox, invalid, 0);
> +		spin(mbox, invalid);
>  	}
>  
>  	unreachable();
> diff --git a/common/psci.c b/common/psci.c
> index 5ae4255..19cc315 100644
> --- a/common/psci.c
> +++ b/common/psci.c
> @@ -57,7 +57,7 @@ static int psci_cpu_off(void)
>  
>  	branch_table[cpu] = PSCI_ADDR_INVALID;
>  
> -	spin(branch_table + cpu, PSCI_ADDR_INVALID, 0);
> +	spin(branch_table + cpu, PSCI_ADDR_INVALID);
>  
>  	unreachable();
>  }
> diff --git a/include/boot.h b/include/boot.h
> index 459d1d5..18b805d 100644
> --- a/include/boot.h
> +++ b/include/boot.h
> @@ -12,7 +12,7 @@
>  #include <compiler.h>
>  #include <stdbool.h>
>  
> -void __noreturn spin(unsigned long *mbox, unsigned long invalid, int is_entry);
> +void __noreturn spin(unsigned long *mbox, unsigned long invalid);
>  
>  void __noreturn first_spin(unsigned int cpu, unsigned long *mbox,
>  			   unsigned long invalid_addr);