[U-Boot] [PATCH] Align end of bss by 4 bytes.

[Trent Piepho]

On Thu, 16 Oct 2008, Selvamuthukumar wrote:
> Most of the bss initialization loop increments 4 bytes
> at a time. And the loop end is checked for an 'equal'
> condition. Make the bss end address aligned by 4, so
> that the loop will end as expected.

It's not really the end of bss that matters, but the alignment of the symbol
that is being used in the code that clears bss, e.g. _end or _bss_end or
whatever.

On Thu, 16 Oct 2008, Dan Malek wrote:
> for equal and make these assumptions.  The code should
> be corrected to determine the proper size of the bss
> space and zero accordingly.... or at least test for
> greater than or equal.

Just changing the test isn't really enough, it's necessary to also support
writing less than 4 bytes at once to get the tail correct.  This appears to be
usually written in asm, so it's not to keep is simple.  For exanple, is
already a patch for ppc4xx that does this:

commit 42ed33ffe135f618680f9d6e9712eb35a85bcb62
Author: Anatolij Gustschin <agustschin at t-online.de>
Date:   Wed Dec 5 17:43:20 2007 +0100

      Fix ppc4xx clear_bss() code

      ppc4xx clear_bss() fails if BSS segment size is not
      divisible by 4 without remainder. This patch provides
      fix for this problem.

This patch is rather more complex that just adding an ALIGN(4) to the linker
script.

I noticed some errors in this patch:

> +++ b/board/AtmarkTechno/suzaku/u-boot.lds
> @@ -62,6 +62,7 @@ SECTIONS
> 		__bss_start = .;
> 		*(.bss)
> 		__bss_end = .;
> +		. = ALIGN(4);

Does this board use __bss_end for it's clear code?  If so, you should move the
alignemnt up a line.

> +++ b/board/MAI/AmigaOneG3SE/u-boot.lds
> @@ -133,6 +133,7 @@ SECTIONS
>    *(.dynbss)
>    *(.bss)
>    *(COMMON)
> +   . = ALIGN(4);
>   }
>   _end = ALIGN(4) /*.*/ ;

_end is already aligned, you don't need to patch this one.  Or maybe it should
be changed to say "_end = ."?

>   PROVIDE (end = ALIGN(4) /*.*/);

IMHO, that is ugly, this looks nicer:
PROVIDE(end = _end);

> +++ b/board/atmel/atngw100/u-boot.lds
> @@ -67,6 +67,7 @@ SECTIONS
> 	.bss : {
> 		*(.bss)
> 		*(.bss.*)
> +		. = ALIGN(4);
> 	}
> 	. = ALIGN(8);
> 	_end = .;

They already have an align 8

> +++ b/board/atmel/atstk1000/u-boot.lds
> @@ -67,6 +67,7 @@ SECTIONS
> 	.bss (NOLOAD) : {
> 		*(.bss)
> 		*(.bss.*)
> +		. = ALIGN(4);
> 	}
> 	. = ALIGN(8);
> 	_end = .;

Same here.

> diff --git a/board/dave/PPChameleonEVB/u-boot.lds b/board/dave/PPChameleonEVB/u-boot.lds
> index e42c76f..289cbc2 100644
> --- a/board/dave/PPChameleonEVB/u-boot.lds
> +++ b/board/dave/PPChameleonEVB/u-boot.lds
> @@ -141,6 +141,7 @@ SECTIONS
>    *(.dynbss)
>    *(.bss)
>    *(COMMON)
> +   . = ALIGN(4);
>   }
>
>   ppcenv_assert = ASSERT(. < 0xFFFF8000, ".bss section too big, overlaps .ppcenv section. Please update your confguration: CFG_MONITOR_BASE, CFG_MONITOR_LEN and TEXT_BASE may need to be modified.");
    . = 0xFFFF8000;
    .ppcenv :
    {
      common/env_embedded.o(.ppcenv);
    }

    _end = . ;
    PROVIDE (end = .);

If this board uses _end to clear bss, you need to move the alignment to after
the ppcenv section.  Or maybe this board should move _end to before ppcenv? 
Or maybe it doesn't clear bss and _end isn't used?  However it works, what you
have changed isn't correct.

> +++ b/board/earthlcd/favr-32-ezkit/u-boot.lds
> @@ -65,6 +65,7 @@ SECTIONS
> 	.bss (NOLOAD) : {
> 		*(.bss)
> 		*(.bss.*)
> +		. = ALIGN(4);
> 	}
> 	. = ALIGN(8);
> 	_end = .;

Already has align 8.

> +++ b/board/esd/tasreg/u-boot.lds
> @@ -139,6 +139,7 @@ SECTIONS
>    *(COMMON)
>    . = ALIGN(4);
>    _ebss = .;
> +   . = ALIGN(4);
>   }
>   _end = . ;
>   PROVIDE (end = .);

Already has an align, doesn't need another.

> +++ b/board/genietv/u-boot.lds
> @@ -133,6 +133,7 @@ SECTIONS
>    *(.dynbss)
>    *(.bss)
>    *(COMMON)
> +   . = ALIGN(4);
>   }
>   . = ALIGN(256 * 1024);
>   .ppcenv	:

Already has alignment.  There is also no symbol like _end between bss and
ppcenv, so this board probably doesn't clear bss.

> +++ b/board/hymod/u-boot.lds
> @@ -136,6 +136,7 @@ SECTIONS
>    *(.dynbss)
>    *(.bss)
>    *(COMMON)
> +   . = ALIGN(4);
>   }
>   . = ALIGN(256 * 1024);
>   .ppcenv	:

Same as genietv

> +++ b/board/sh7763rdp/u-boot.lds
> @@ -98,6 +98,7 @@ SECTIONS
> 	{
> 		*(.bss)
> 		. = ALIGN(4);
> +		. = ALIGN(4);
> 	}
> 	PROVIDE (bss_end = .);

Already has alignment.

> +++ b/board/ssv/adnpesc1/u-boot.lds
> @@ -64,6 +64,7 @@ SECTIONS
> 	.bss (NOLOAD) :
> 	{
> 		*(.bss)
> +		. = ALIGN(4);
> 	}
> 	. = ALIGN(4);
> 	__bss_end = .;

Already has alignment.  Maybe delete the second one?

> +++ b/board/xilinx/ml401/u-boot.lds
> @@ -62,6 +62,7 @@ SECTIONS
> 		__bss_start = .;
> 		*(.bss)
> 		__bss_end = .;
> +		. = ALIGN(4);
> 	}
> 	__end = . ;
> }

__bss_end won't be aligned.  Maybe if the board doesn't need both __bss_end
and __end one them should be deleted?

> +++ b/board/xilinx/xupv2p/u-boot.lds
> @@ -62,6 +62,7 @@ SECTIONS
> 		__bss_start = .;
> 		*(.bss)
> 		__bss_end = .;
> +		. = ALIGN(4);
> 	}
> 	__end = . ;
> }

Another __bss_end non-alignemnt problem.