[2/3] doc: environment: Expand on fdt_addr, initrd_addr and loadaddr

- Explain why fdt_addr and initrd_addr should not be set to disable
  relocation normally.
- Provide some advice on the typical loadaddr default value.

Signed-off-by: Tom Rini <trini@konsulko.com>
---
 doc/usage/environment.rst | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

On Mon, 20 Jun 2022 at 08:32, Tom Rini <trini@konsulko.com> wrote:
>
> - Explain why fdt_addr and initrd_addr should not be set to disable
>   relocation normally.
> - Provide some advice on the typical loadaddr default value.
>
> Signed-off-by: Tom Rini <trini@konsulko.com>
> ---
>  doc/usage/environment.rst | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

On 6/20/22 16:31, Tom Rini wrote:
> - Explain why fdt_addr and initrd_addr should not be set to disable
>    relocation normally.
> - Provide some advice on the typical loadaddr default value.
>
> Signed-off-by: Tom Rini <trini@konsulko.com>
> ---
>   doc/usage/environment.rst | 15 ++++++++++++---
>   1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/doc/usage/environment.rst b/doc/usage/environment.rst
> index a3f23d4e4e22..a9a4702632d2 100644
> --- a/doc/usage/environment.rst
> +++ b/doc/usage/environment.rst
> @@ -204,7 +204,9 @@ fdt_high
>       to work it must reside in writable memory, have
>       sufficient padding on the end of it for u-boot to
>       add the information it needs into it, and the memory
> -    must be accessible by the kernel.
> +    must be accessible by the kernel.  This usage is strongly discouraged
> +    however as it also stops U-Boot from ensuring the device tree starting
> +    address is properly aligned and a misaligned tree will cause OS failures.

fdt_addr_r is typically 4 byte aligned. Is there a bug in some part of
the code?

>
>   fdtcontroladdr
>       if set this is the address of the control flattened
> @@ -240,14 +242,21 @@ initrd_high
>       memory. In this case U-Boot will NOT COPY the
>       ramdisk at all. This may be useful to reduce the
>       boot time on your system, but requires that this
> -    feature is supported by your Linux kernel.
> +    feature is supported by your Linux kernel.  This usage however requires
> +    that the user ensure that there will be no overlap with other parts of the

It is not the user but the developer who define $kernel_addr_r,
$initr_addr_r, and $fdt_addr_r.

Relocating initrd does not stop the user from loading the kernel to
initrd_high. The user is always responsible for avoiding overlap. In
this regard there is nothing special about initrd_high=~0UL.

> +    iamge such as the Linux kernel BSS.  It should not be enabled by default

%s/iamge/image

> +    and only done as part of optimizing a deployment.

Relocating initrd and fdt is using boot time. On many boards it is
simply superfluous. On these boards I cannot see any reason not to
disable it by default.

Best regards

Heinrich

>
>   ipaddr
>       IP address; needed for tftpboot command
>
>   loadaddr
>       Default load address for commands like "bootp",
> -    "rarpboot", "tftpboot", "loadb" or "diskboot"
> +    "rarpboot", "tftpboot", "loadb" or "diskboot".  Note that the optimal
> +    default values here will vary between architectures.  On 32bit ARM for
> +    example, some offset from start of memory is used as the Linux kernel
> +    zImage has a self decompressor and it's best if we stay out of where that
> +    will be working.
>
>   loads_echo
>       see CONFIG_LOADS_ECHO

On Sun, Jul 10, 2022 at 01:02:28PM +0200, Heinrich Schuchardt wrote:
> On 6/20/22 16:31, Tom Rini wrote:
> > - Explain why fdt_addr and initrd_addr should not be set to disable
> >    relocation normally.
> > - Provide some advice on the typical loadaddr default value.
> > 
> > Signed-off-by: Tom Rini <trini@konsulko.com>
> > ---
> >   doc/usage/environment.rst | 15 ++++++++++++---
> >   1 file changed, 12 insertions(+), 3 deletions(-)
> > 
> > diff --git a/doc/usage/environment.rst b/doc/usage/environment.rst
> > index a3f23d4e4e22..a9a4702632d2 100644
> > --- a/doc/usage/environment.rst
> > +++ b/doc/usage/environment.rst
> > @@ -204,7 +204,9 @@ fdt_high
> >       to work it must reside in writable memory, have
> >       sufficient padding on the end of it for u-boot to
> >       add the information it needs into it, and the memory
> > -    must be accessible by the kernel.
> > +    must be accessible by the kernel.  This usage is strongly discouraged
> > +    however as it also stops U-Boot from ensuring the device tree starting
> > +    address is properly aligned and a misaligned tree will cause OS failures.
> 
> fdt_addr_r is typically 4 byte aligned. Is there a bug in some part of
> the code?

It must be 8 byte aligned.  And we don't know the alignment inside of
FIT images.  Do not disable device tree relocation except under the most
calculated of circumstances.

> >   fdtcontroladdr
> >       if set this is the address of the control flattened
> > @@ -240,14 +242,21 @@ initrd_high
> >       memory. In this case U-Boot will NOT COPY the
> >       ramdisk at all. This may be useful to reduce the
> >       boot time on your system, but requires that this
> > -    feature is supported by your Linux kernel.
> > +    feature is supported by your Linux kernel.  This usage however requires
> > +    that the user ensure that there will be no overlap with other parts of the
> 
> It is not the user but the developer who define $kernel_addr_r,
> $initr_addr_r, and $fdt_addr_r.

No, it can be either.  See stackoverflow and assorted blog posts.
Person who picked up an SBC and just wants the thing to work will find
various posts saying to do ... something ... here.

> Relocating initrd does not stop the user from loading the kernel to
> initrd_high. The user is always responsible for avoiding overlap. In
> this regard there is nothing special about initrd_high=~0UL.

And we're responsible for providing reasonable defaults.  Do not disable
initrd relocation by default as it gains little and can break the OS.

> > +    iamge such as the Linux kernel BSS.  It should not be enabled by default
> 
> %s/iamge/image

Please fix when applying.

> > +    and only done as part of optimizing a deployment.
> 
> Relocating initrd and fdt is using boot time. On many boards it is
> simply superfluous. On these boards I cannot see any reason not to
> disable it by default.

The way to avoid "superfluous" time here is to set reasonable default
locations, not disabling relocation.

Please note this is what the policy is, not something new.