[PATCH] board_init: Do not reserve MALLOC_F area on stack if non-zero MALLOC_F_ADDR

[PATCH] board_init: Do not reserve MALLOC_F area on stack if non-zero MALLOC_F_ADDR

[Marek Vasut]

In case the MALLOC_F_ADDR is set to non-zero value, the early malloc area is
not going to be placed just below stack top, but elsewhere. Do not reserve
MALLOC_F bytes in this case, as that wastes stack space and may even cause
insufficient stack space in SPL.

This functionality is particularly useful on i.MX8M, where the insufficient
stack space can be triggered.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Albert ARIBAUD <albert.u.boot@aribaud.net>
Cc: Fabio Estevam <festevam@denx.de>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Simon Glass <sjg@chromium.org>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Thomas Chou <thomas@wytron.com.tw>
Cc: Tom Rini <trini@konsulko.com>
---
 common/init/board_init.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/common/init/board_init.c b/common/init/board_init.c
index eab5ee13953..6a550261778 100644
--- a/common/init/board_init.c
+++ b/common/init/board_init.c
@@ -78,8 +78,10 @@ __weak void board_init_f_init_stack_protection(void)
 ulong board_init_f_alloc_reserve(ulong top)
 {
 	/* Reserve early malloc arena */
+#ifndef CONFIG_MALLOC_F_ADDR
 #if CONFIG_VAL(SYS_MALLOC_F_LEN)
 	top -= CONFIG_VAL(SYS_MALLOC_F_LEN);
+#endif
 #endif
 	/* LAST : reserve GD (rounded up to a multiple of 16 bytes) */
 	top = rounddown(top-sizeof(struct global_data), 16);
-- 
2.35.1

Re: [PATCH] board_init: Do not reserve MALLOC_F area on stack if non-zero MALLOC_F_ADDR

[Simon Glass]

Hi Marek,

On Sat, 25 Jun 2022 at 11:58, Marek Vasut <marex@denx.de> wrote:
>
> In case the MALLOC_F_ADDR is set to non-zero value, the early malloc area is
> not going to be placed just below stack top, but elsewhere. Do not reserve
> MALLOC_F bytes in this case, as that wastes stack space and may even cause
> insufficient stack space in SPL.
>
> This functionality is particularly useful on i.MX8M, where the insufficient
> stack space can be triggered.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Albert ARIBAUD <albert.u.boot@aribaud.net>
> Cc: Fabio Estevam <festevam@denx.de>
> Cc: Peng Fan <peng.fan@nxp.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Thomas Chou <thomas@wytron.com.tw>
> Cc: Tom Rini <trini@konsulko.com>
> ---
>  common/init/board_init.c | 2 ++
>  1 file changed, 2 insertions(+)

Please can you migrate the option to Kconfig first? I suspect that
will simplify the logic and avoid the #ifndef

>
> diff --git a/common/init/board_init.c b/common/init/board_init.c
> index eab5ee13953..6a550261778 100644
> --- a/common/init/board_init.c
> +++ b/common/init/board_init.c
> @@ -78,8 +78,10 @@ __weak void board_init_f_init_stack_protection(void)
>  ulong board_init_f_alloc_reserve(ulong top)
>  {
>         /* Reserve early malloc arena */
> +#ifndef CONFIG_MALLOC_F_ADDR
>  #if CONFIG_VAL(SYS_MALLOC_F_LEN)
>         top -= CONFIG_VAL(SYS_MALLOC_F_LEN);
> +#endif
>  #endif
>         /* LAST : reserve GD (rounded up to a multiple of 16 bytes) */
>         top = rounddown(top-sizeof(struct global_data), 16);
> --
> 2.35.1
>

Regards,
Simon

Re: [PATCH] board_init: Do not reserve MALLOC_F area on stack if non-zero MALLOC_F_ADDR

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 2276 bytes --]

On Thu, Jun 30, 2022 at 04:06:22AM -0600, Simon Glass wrote:
> Hi Marek,
> 
> On Sat, 25 Jun 2022 at 11:58, Marek Vasut <marex@denx.de> wrote:
> >
> > In case the MALLOC_F_ADDR is set to non-zero value, the early malloc area is
> > not going to be placed just below stack top, but elsewhere. Do not reserve
> > MALLOC_F bytes in this case, as that wastes stack space and may even cause
> > insufficient stack space in SPL.
> >
> > This functionality is particularly useful on i.MX8M, where the insufficient
> > stack space can be triggered.
> >
> > Signed-off-by: Marek Vasut <marex@denx.de>
> > Cc: Albert ARIBAUD <albert.u.boot@aribaud.net>
> > Cc: Fabio Estevam <festevam@denx.de>
> > Cc: Peng Fan <peng.fan@nxp.com>
> > Cc: Simon Glass <sjg@chromium.org>
> > Cc: Stefano Babic <sbabic@denx.de>
> > Cc: Thomas Chou <thomas@wytron.com.tw>
> > Cc: Tom Rini <trini@konsulko.com>
> > ---
> >  common/init/board_init.c | 2 ++
> >  1 file changed, 2 insertions(+)
> 
> Please can you migrate the option to Kconfig first? I suspect that
> will simplify the logic and avoid the #ifndef

That was my first reaction as well.   But, I'm not so sure after looking
at things more.  An issue is that CONFIG_MALLOC_F_ADDR should probably
be CONFIG_SPL_MALLOC_F_ADDR as it's only used in SPL.  But it's also not
an either/or around CONFIG_SPL_SYS_MALLOC_F_LEN.

It would be good to move to Kconfig (but I also don't see a common
default for the handful of SoCs using it), and maybe a !CONFIG_VAL()
test instead of ifndef.

> 
> >
> > diff --git a/common/init/board_init.c b/common/init/board_init.c
> > index eab5ee13953..6a550261778 100644
> > --- a/common/init/board_init.c
> > +++ b/common/init/board_init.c
> > @@ -78,8 +78,10 @@ __weak void board_init_f_init_stack_protection(void)
> >  ulong board_init_f_alloc_reserve(ulong top)
> >  {
> >         /* Reserve early malloc arena */
> > +#ifndef CONFIG_MALLOC_F_ADDR
> >  #if CONFIG_VAL(SYS_MALLOC_F_LEN)
> >         top -= CONFIG_VAL(SYS_MALLOC_F_LEN);
> > +#endif
> >  #endif
> >         /* LAST : reserve GD (rounded up to a multiple of 16 bytes) */
> >         top = rounddown(top-sizeof(struct global_data), 16);
> > --
> > 2.35.1
> >
> 
> Regards,
> Simon

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH] board_init: Do not reserve MALLOC_F area on stack if non-zero MALLOC_F_ADDR

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 829 bytes --]

On Sat, Jun 25, 2022 at 07:58:24PM +0200, Marek Vasut wrote:

> In case the MALLOC_F_ADDR is set to non-zero value, the early malloc area is
> not going to be placed just below stack top, but elsewhere. Do not reserve
> MALLOC_F bytes in this case, as that wastes stack space and may even cause
> insufficient stack space in SPL.
> 
> This functionality is particularly useful on i.MX8M, where the insufficient
> stack space can be triggered.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Albert ARIBAUD <albert.u.boot@aribaud.net>
> Cc: Fabio Estevam <festevam@denx.de>
> Cc: Peng Fan <peng.fan@nxp.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Thomas Chou <thomas@wytron.com.tw>
> Cc: Tom Rini <trini@konsulko.com>

Applied to u-boot/next, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH] board_init: Do not reserve MALLOC_F area on stack if non-zero MALLOC_F_ADDR

[Marek Vasut]

On 6/30/22 15:36, Tom Rini wrote:

Hi,

[...]

>>> In case the MALLOC_F_ADDR is set to non-zero value, the early malloc area is
>>> not going to be placed just below stack top, but elsewhere. Do not reserve
>>> MALLOC_F bytes in this case, as that wastes stack space and may even cause
>>> insufficient stack space in SPL.
>>>
>>> This functionality is particularly useful on i.MX8M, where the insufficient
>>> stack space can be triggered.

[...]

>>>   common/init/board_init.c | 2 ++
>>>   1 file changed, 2 insertions(+)
>>
>> Please can you migrate the option to Kconfig first? I suspect that
>> will simplify the logic and avoid the #ifndef
> 
> That was my first reaction as well.   But, I'm not so sure after looking
> at things more.  An issue is that CONFIG_MALLOC_F_ADDR should probably
> be CONFIG_SPL_MALLOC_F_ADDR as it's only used in SPL.  But it's also not
> an either/or around CONFIG_SPL_SYS_MALLOC_F_LEN.
> 
> It would be good to move to Kconfig (but I also don't see a common
> default for the handful of SoCs using it), and maybe a !CONFIG_VAL()
> test instead of ifndef.

See
[PATCH] board_init: Convert CONFIG_MALLOC_F_ADDR to Kconfig