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 <s...@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

Attachment: signature.asc
Description: PGP signature

Reply via email to