[PATCH v5 3/9] board_f: Correct stack reservation

[Tom Rini]

On Sun, Dec 01, 2024 at 08:28:05AM -0700, Simon Glass wrote:

> The reserve_stack_aligned() function already ensures that the resulting
> address is aligned to a 16-byte boundary. The comment seems to suggest
> that 16 is passed reserve_stack_aligned() to make it aligned.
> 
> Change the value to 0, since the stack can start at the current address,
> if it is suitably aligned already.
> 
> Signed-off-by: Simon Glass <sjg at chromium.org>

Er:
/*
 * reserve after start_addr_sp the requested size and make the stack pointer
 * 16-byte aligned, this alignment is needed for cast on the reserved memory
 * ref = x86_64 ABI: https://reviews.llvm.org/D30049: 16 bytes
 *     = ARMv8 Instruction Set Overview: quad word, 16 bytes
 */
static unsigned long reserve_stack_aligned(size_t size)
{
        return ALIGN_DOWN(gd->start_addr_sp - size, 16);
}

is what the code is today.

> ---
> 
> (no changes since v1)
> 
>  common/board_f.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/common/board_f.c b/common/board_f.c
> index 98dc2591e1d..677e37d93c0 100644
> --- a/common/board_f.c
> +++ b/common/board_f.c
> @@ -601,7 +601,7 @@ __weak int arch_reserve_stacks(void)
>  static int reserve_stacks(void)
>  {
>  	/* make stack pointer 16-byte aligned */
> -	gd->start_addr_sp = reserve_stack_aligned(16);
> +	gd->start_addr_sp = reserve_stack_aligned(0);
>  
>  	/*
>  	 * let the architecture-specific code tailor gd->start_addr_sp and

And this is the last call to reserve_stack_aligned(). So, this is going
to save us between 16 and 0 bytes of memory. If we're going to make a
change here we need to comment in the code more what we're up to at this
point, and the commit message should be more authoritatively worded too.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20241202/7e2868cc/attachment.sig>