diff mbox

[U-Boot,1/6] sniper: Move PXE file address to a viable location

Message ID 1458991204-22798-1-git-send-email-contact@paulk.fr
State Changes Requested
Delegated to: Tom Rini
Headers show

Commit Message

Paul Kocialkowski March 26, 2016, 11:19 a.m. UTC
This moves the PXE file address out of the bounds of the U-Boot image (which is
loaded at 0x80100000).

This also moves the ramdisk address to a better-looking aligned location.

Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
---
 include/configs/sniper.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Tom Rini March 26, 2016, 6:59 p.m. UTC | #1
On Sat, Mar 26, 2016 at 12:19:59PM +0100, Paul Kocialkowski wrote:
> This moves the PXE file address out of the bounds of the U-Boot image (which is
> loaded at 0x80100000).
> 
> This also moves the ramdisk address to a better-looking aligned location.
> 
> Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> ---
>  include/configs/sniper.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/configs/sniper.h b/include/configs/sniper.h
> index 171a8c4..2598af1 100644
> --- a/include/configs/sniper.h
> +++ b/include/configs/sniper.h
> @@ -243,8 +243,8 @@
>  	"loadaddr=0x82000000\0" \
>  	"fdt_addr_r=0x88000000\0" \
>  	"fdtaddr=0x88000000\0" \
> -	"ramdisk_addr_r=0x88080000\0" \
> -	"pxefile_addr_r=0x80100000\0" \
> +	"ramdisk_addr_r=0x84000000\0" \
> +	"pxefile_addr_r=0x86000000\0" \
>  	"scriptaddr=0x80000000\0" \
>  	"bootm_size=0x10000000\0" \
>  	"boot_mmc_dev=0\0" \

I strongly discourage these locations.  Quoting myself from
ti_armv7_common.h:
/*
 * We setup defaults based on constraints from the Linux kernel, which should
 * also be safe elsewhere.  We have the default load at 32MB into DDR (for
 * the kernel), FDT above 128MB (the maximum location for the end of the
 * kernel), and the ramdisk 512KB above that (allowing for hopefully never
 * seen large trees).  We say all of this must be within the first 256MB
 * as that will normally be within the kernel lowmem and thus visible via
 * bootm_size and we only run on platforms with 256MB or more of memory.
 */

Now, have you actually run into a problem with overwriting U-Boot?
Since U-Boot will run-time relocate, this shouldn't be a problem in
practice?  The minor games we play with locations here are due to Falcon
Mode where the BSS in question here is the one for SPL and we don't want
those two to conflict.
Paul Kocialkowski March 28, 2016, 11:53 a.m. UTC | #2
Le samedi 26 mars 2016 à 14:59 -0400, Tom Rini a écrit :
> On Sat, Mar 26, 2016 at 12:19:59PM +0100, Paul Kocialkowski wrote:
> > 
> > This moves the PXE file address out of the bounds of the U-Boot image (which
> > is
> > loaded at 0x80100000).
> > 
> > This also moves the ramdisk address to a better-looking aligned location.
> > 
> > Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
> > ---
> >  include/configs/sniper.h | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/include/configs/sniper.h b/include/configs/sniper.h
> > index 171a8c4..2598af1 100644
> > --- a/include/configs/sniper.h
> > +++ b/include/configs/sniper.h
> > @@ -243,8 +243,8 @@
> >  	"loadaddr=0x82000000\0" \
> >  	"fdt_addr_r=0x88000000\0" \
> >  	"fdtaddr=0x88000000\0" \
> > -	"ramdisk_addr_r=0x88080000\0" \
> > -	"pxefile_addr_r=0x80100000\0" \
> > +	"ramdisk_addr_r=0x84000000\0" \
> > +	"pxefile_addr_r=0x86000000\0" \
> >  	"scriptaddr=0x80000000\0" \
> >  	"bootm_size=0x10000000\0" \
> >  	"boot_mmc_dev=0\0" \
> I strongly discourage these locations.  Quoting myself from
> ti_armv7_common.h:
> /*
>  * We setup defaults based on constraints from the Linux kernel, which should
>  * also be safe elsewhere.  We have the default load at 32MB into DDR (for
>  * the kernel), FDT above 128MB (the maximum location for the end of the
>  * kernel), and the ramdisk 512KB above that (allowing for hopefully never
>  * seen large trees).  We say all of this must be within the first 256MB
>  * as that will normally be within the kernel lowmem and thus visible via
>  * bootm_size and we only run on platforms with 256MB or more of memory.
>  */

Thanks for the clarification, I think the addresses make sense as they are then.

> Now, have you actually run into a problem with overwriting U-Boot?

Actually, I had forgotten that U-Boot relocates. This address only caught my eye
as being equal to CONFIG_SYS_TEXT_BASE when going through the header, but I
didn't investigate enough.

There is indeed no run-time problem, thanks to relocation. I'll simply drop
these patches.

> Since U-Boot will run-time relocate, this shouldn't be a problem in
> practice?  The minor games we play with locations here are due to Falcon
> Mode where the BSS in question here is the one for SPL and we don't want
> those two to conflict.
diff mbox

Patch

diff --git a/include/configs/sniper.h b/include/configs/sniper.h
index 171a8c4..2598af1 100644
--- a/include/configs/sniper.h
+++ b/include/configs/sniper.h
@@ -243,8 +243,8 @@ 
 	"loadaddr=0x82000000\0" \
 	"fdt_addr_r=0x88000000\0" \
 	"fdtaddr=0x88000000\0" \
-	"ramdisk_addr_r=0x88080000\0" \
-	"pxefile_addr_r=0x80100000\0" \
+	"ramdisk_addr_r=0x84000000\0" \
+	"pxefile_addr_r=0x86000000\0" \
 	"scriptaddr=0x80000000\0" \
 	"bootm_size=0x10000000\0" \
 	"boot_mmc_dev=0\0" \