diff mbox series

[v2,1/3] mx6sabresd: Fix U-Boot corruption after saving the environment

Message ID 20240202160405.4126192-1-festevam@gmail.com
State Accepted
Commit 41fdfae064f9c5934af83f5c76a95d1e0c910d76
Delegated to: Fabio Estevam
Headers show
Series [v2,1/3] mx6sabresd: Fix U-Boot corruption after saving the environment | expand

Commit Message

Fabio Estevam Feb. 2, 2024, 4:04 p.m. UTC 
From: Fabio Estevam <festevam@denx.de>

U-Boot binary has grown in such a way that it goes beyond the reserved
area for the environment variables.
    
Running "saveenv" and rebooting the board causes U-Boot to hang because
of this overlap.
    
Fix this problem by selecting CONFIG_LTO so that the U-Boot proper
size can be reduced.
    
Also, to prevent this same problem to happen in the future, use
CONFIG_BOARD_SIZE_LIMIT, which can detect the overlap in build-time.
    
CONFIG_BOARD_SIZE_LIMIT is calculated as follows:

CONFIG_BOARD_SIZE_LIMIT = CONFIG_ENV_OFFSET - u-boot-img.dtb offset
CONFIG_BOARD_SIZE_LIMIT = 0xc000 - 69 * 1024
CONFIG_BOARD_SIZE_LIMIT = 715766

Signed-off-by: Fabio Estevam <festevam@denx.de>
---
Changes since v1:
- Select LTO. (Tom)

 configs/mx6sabresd_defconfig | 3 +++
 1 file changed, 3 insertions(+)

Comments

Tom Rini Feb. 2, 2024, 4:06 p.m. UTC | #1 
On Fri, Feb 02, 2024 at 01:04:03PM -0300, Fabio Estevam wrote:

> From: Fabio Estevam <festevam@denx.de>
> 
> U-Boot binary has grown in such a way that it goes beyond the reserved
> area for the environment variables.
>     
> Running "saveenv" and rebooting the board causes U-Boot to hang because
> of this overlap.
>     
> Fix this problem by selecting CONFIG_LTO so that the U-Boot proper
> size can be reduced.
>     
> Also, to prevent this same problem to happen in the future, use
> CONFIG_BOARD_SIZE_LIMIT, which can detect the overlap in build-time.
>     
> CONFIG_BOARD_SIZE_LIMIT is calculated as follows:
> 
> CONFIG_BOARD_SIZE_LIMIT = CONFIG_ENV_OFFSET - u-boot-img.dtb offset
> CONFIG_BOARD_SIZE_LIMIT = 0xc000 - 69 * 1024
> CONFIG_BOARD_SIZE_LIMIT = 715766
> 
> Signed-off-by: Fabio Estevam <festevam@denx.de>

Thanks!

Reviewed-by: Tom Rini <trini@konsulko.com>
Igor Opaniuk Feb. 2, 2024, 8:34 p.m. UTC | #2 
On Fri, Feb 2, 2024 at 5:04 PM Fabio Estevam <festevam@gmail.com> wrote:
>
> From: Fabio Estevam <festevam@denx.de>
>
> U-Boot binary has grown in such a way that it goes beyond the reserved
> area for the environment variables.
>
> Running "saveenv" and rebooting the board causes U-Boot to hang because
> of this overlap.
>
> Fix this problem by selecting CONFIG_LTO so that the U-Boot proper
> size can be reduced.
>
> Also, to prevent this same problem to happen in the future, use
> CONFIG_BOARD_SIZE_LIMIT, which can detect the overlap in build-time.
>
> CONFIG_BOARD_SIZE_LIMIT is calculated as follows:
>
> CONFIG_BOARD_SIZE_LIMIT = CONFIG_ENV_OFFSET - u-boot-img.dtb offset
> CONFIG_BOARD_SIZE_LIMIT = 0xc000 - 69 * 1024
> CONFIG_BOARD_SIZE_LIMIT = 715766
>
> Signed-off-by: Fabio Estevam <festevam@denx.de>
> ---
> Changes since v1:
> - Select LTO. (Tom)
>
>  configs/mx6sabresd_defconfig | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/configs/mx6sabresd_defconfig b/configs/mx6sabresd_defconfig
> index a90efe4a7786..d4fb55622cf1 100644
> --- a/configs/mx6sabresd_defconfig
> +++ b/configs/mx6sabresd_defconfig
> @@ -21,6 +21,9 @@ CONFIG_SPL_SERIAL=y
>  CONFIG_SPL=y
>  CONFIG_SPL_LIBDISK_SUPPORT=y
>  CONFIG_PCI=y
> +CONFIG_LTO=y
> +CONFIG_HAS_BOARD_SIZE_LIMIT=y
> +CONFIG_BOARD_SIZE_LIMIT=715766
>  CONFIG_FIT=y
>  CONFIG_SPL_FIT_PRINT=y
>  CONFIG_SPL_LOAD_FIT=y
> --
> 2.34.1
>
Reviewed-by: Igor Opaniuk <igor.opaniuk@foundries.io>

btw, from my experience, the same issue will probably occur again in future,
especially when the current U-Boot size is already close to the defined limit
(yes, with this patch it will be reported during compile time, but still).

Have you considered adjusting the boot image layout instead
(moving CONFIG_ENV_OFFSET, so unlikely
U-Boot image will overlap again) to tackle feature problems?
Igor Opaniuk Feb. 2, 2024, 9:16 p.m. UTC | #3 
On Fri, Feb 2, 2024 at 9:34 PM Igor Opaniuk <igor.opaniuk@foundries.io> wrote:
>
> On Fri, Feb 2, 2024 at 5:04 PM Fabio Estevam <festevam@gmail.com> wrote:
> >
> > From: Fabio Estevam <festevam@denx.de>
> >
> > U-Boot binary has grown in such a way that it goes beyond the reserved
> > area for the environment variables.
> >
> > Running "saveenv" and rebooting the board causes U-Boot to hang because
> > of this overlap.
> >
> > Fix this problem by selecting CONFIG_LTO so that the U-Boot proper
> > size can be reduced.
> >
> > Also, to prevent this same problem to happen in the future, use
> > CONFIG_BOARD_SIZE_LIMIT, which can detect the overlap in build-time.
> >
> > CONFIG_BOARD_SIZE_LIMIT is calculated as follows:
> >
> > CONFIG_BOARD_SIZE_LIMIT = CONFIG_ENV_OFFSET - u-boot-img.dtb offset
> > CONFIG_BOARD_SIZE_LIMIT = 0xc000 - 69 * 1024
> > CONFIG_BOARD_SIZE_LIMIT = 715766
> >
> > Signed-off-by: Fabio Estevam <festevam@denx.de>
> > ---
> > Changes since v1:
> > - Select LTO. (Tom)
> >
> >  configs/mx6sabresd_defconfig | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/configs/mx6sabresd_defconfig b/configs/mx6sabresd_defconfig
> > index a90efe4a7786..d4fb55622cf1 100644
> > --- a/configs/mx6sabresd_defconfig
> > +++ b/configs/mx6sabresd_defconfig
> > @@ -21,6 +21,9 @@ CONFIG_SPL_SERIAL=y
> >  CONFIG_SPL=y
> >  CONFIG_SPL_LIBDISK_SUPPORT=y
> >  CONFIG_PCI=y
> > +CONFIG_LTO=y
> > +CONFIG_HAS_BOARD_SIZE_LIMIT=y
> > +CONFIG_BOARD_SIZE_LIMIT=715766
> >  CONFIG_FIT=y
> >  CONFIG_SPL_FIT_PRINT=y
> >  CONFIG_SPL_LOAD_FIT=y
> > --
> > 2.34.1
> >
> Reviewed-by: Igor Opaniuk <igor.opaniuk@foundries.io>
>
> btw, from my experience, the same issue will probably occur again in future,
> especially when the current U-Boot size is already close to the defined limit
> (yes, with this patch it will be reported during compile time, but still).
>
> Have you considered adjusting the boot image layout instead
> (moving CONFIG_ENV_OFFSET, so unlikely
> U-Boot image will overlap again) to tackle feature problems?
I've just checked the previous patch version, where
you addressed that exactly with CONFIG_ENV_OFFSET change and
following suggestions/objections from Tom, so sorry for the noise :)

>
> --
> Best regards - Freundliche Grüsse - Meilleures salutations
>
> Igor Opaniuk
> Senior Software Engineer, Embedded & Security
> E: igor.opaniuk@foundries.io
> W: www.foundries.io
Fabio Estevam Feb. 8, 2024, 2:34 p.m. UTC | #4 
On Fri, Feb 2, 2024 at 1:04 PM Fabio Estevam <festevam@gmail.com> wrote:
>
> From: Fabio Estevam <festevam@denx.de>
>
> U-Boot binary has grown in such a way that it goes beyond the reserved
> area for the environment variables.
>
> Running "saveenv" and rebooting the board causes U-Boot to hang because
> of this overlap.
>
> Fix this problem by selecting CONFIG_LTO so that the U-Boot proper
> size can be reduced.
>
> Also, to prevent this same problem to happen in the future, use
> CONFIG_BOARD_SIZE_LIMIT, which can detect the overlap in build-time.
>
> CONFIG_BOARD_SIZE_LIMIT is calculated as follows:
>
> CONFIG_BOARD_SIZE_LIMIT = CONFIG_ENV_OFFSET - u-boot-img.dtb offset
> CONFIG_BOARD_SIZE_LIMIT = 0xc000 - 69 * 1024
> CONFIG_BOARD_SIZE_LIMIT = 715766
>
> Signed-off-by: Fabio Estevam <festevam@denx.de>

Applied all, thanks.
diff mbox series

Patch

diff --git a/configs/mx6sabresd_defconfig b/configs/mx6sabresd_defconfig
index a90efe4a7786..d4fb55622cf1 100644
--- a/configs/mx6sabresd_defconfig
+++ b/configs/mx6sabresd_defconfig
@@ -21,6 +21,9 @@  CONFIG_SPL_SERIAL=y
 CONFIG_SPL=y
 CONFIG_SPL_LIBDISK_SUPPORT=y
 CONFIG_PCI=y
+CONFIG_LTO=y
+CONFIG_HAS_BOARD_SIZE_LIMIT=y
+CONFIG_BOARD_SIZE_LIMIT=715766
 CONFIG_FIT=y
 CONFIG_SPL_FIT_PRINT=y
 CONFIG_SPL_LOAD_FIT=y