Re: [PATCH 2/2] ARM: setup_c: avoid clearing BSS twice

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

Hi,

On 2/26/26 13:23, Sascha Hauer wrote:
> Add a bss_cleared variable that is set after the first call to
> setup_c(). On subsequent calls, the BSS clear is skipped to avoid
> wiping already-initialized data.
> 
> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  arch/arm/cpu/common.c    |  2 ++
>  arch/arm/cpu/setupc_32.S | 10 +++++++++-
>  arch/arm/cpu/setupc_64.S | 10 ++++++++--
>  3 files changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/cpu/common.c b/arch/arm/cpu/common.c
> index e86c89f808..40742894c2 100644
> --- a/arch/arm/cpu/common.c
> +++ b/arch/arm/cpu/common.c
> @@ -77,6 +77,8 @@ int __pure cpu_architecture(void)
>  }
>  #endif
>  
> +int bss_cleared;

This will go into BSS, so you implicitly assume BSS to be already
zero as this is accessed first before BSS initialization.

I suggest, along with moving it into the .data section, that you
also move it into the assembly file, so the consumer is directly
next to it:

       .pushsection .data
       .align 2
setup_c_done:
       .word 0
       .popsection

> +
>  extern int __boot_cpu_mode;
>  
>  int boot_cpu_mode(void)
> diff --git a/arch/arm/cpu/setupc_32.S b/arch/arm/cpu/setupc_32.S
> index c2c3f97528..d923a8f621 100644
> --- a/arch/arm/cpu/setupc_32.S
> +++ b/arch/arm/cpu/setupc_32.S
> @@ -7,16 +7,24 @@
>  .section .text.setupc
>  
>  /*
> - * setup_c: clear bss
> + * setup_c: clear bss if not yet done
>   */
>  ENTRY(setup_c)
> +	ldr	r0, =bss_cleared
> +	ldr	r1, [r0]
> +	cmp	r1, #0
> +	bne	1f			/* skip if already done */
>  	mov	r4, lr

Move this instruction to the start, so ...

>  	ldr	r0, =__bss_start
>  	mov	r1, #0
>  	ldr	r2, =__bss_stop
>  	sub	r2, r2, r0
>  	bl	__memset		/* clear bss */
> +	ldr	r0, =bss_cleared
> +	mov	r1, #1
> +	str	r1, [r0]		/* mark bss cleared */
>  	ret	r4
> +1:	ret	lr

This instruction can be dropped and 1: just points at the existing
ret r4?

>  ENDPROC(setup_c)
>  
>  /*
> diff --git a/arch/arm/cpu/setupc_64.S b/arch/arm/cpu/setupc_64.S
> index fd95187a04..8262deb512 100644
> --- a/arch/arm/cpu/setupc_64.S
> +++ b/arch/arm/cpu/setupc_64.S
> @@ -7,17 +7,23 @@
>  .section .text.setupc
>  
>  /*
> - * setup_c: clear bss
> + * setup_c: clear bss if not yet done
>   */
>  ENTRY(setup_c)
> +	adr_l	x0, bss_cleared
> +	ldr	w1, [x0]
> +	cbnz	w1, 1f			/* skip if already done */
>  	mov	x15, x30
>  	adr_l	x0, __bss_start
>  	mov	x1, #0
>  	adr_l	x2, __bss_stop
>  	sub	x2, x2, x0
>  	bl	__memset		/* clear bss */
> +	adr_l	x0, bss_cleared
> +	mov	w1, #1
> +	str	w1, [x0]		/* mark bss cleared */
>  	mov	x30, x15
> -	ret
> +1:	ret
>  ENDPROC(setup_c)
>  
>  /*
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |