[U-Boot] [PATCH] arm: vf610: move device tree after kernel image

[U-Boot] [PATCH] arm: vf610: move device tree after kernel image

[Stefan Agner]

Since the device tree relocation is disabled (fdt_high set to
0xffffffff), U-Boot keeps the device tree at its load address
0x81000000. The kernel uncompresses itself to 0x80008000 by
default, hence this limits the maximum (uncompressed) kernel
size to somewhat below 16MiB, otherwise the device tree gets
overwritten by the kernel data...

Move the device tree load address to 0x84000000 to avoid that
the device tree being overwritten by the kernel.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 include/configs/vf610twr.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/configs/vf610twr.h b/include/configs/vf610twr.h
index 324ba8f..1d15f35 100644
--- a/include/configs/vf610twr.h
+++ b/include/configs/vf610twr.h
@@ -129,7 +129,7 @@
 	"fdt_high=0xffffffff\0" \
 	"initrd_high=0xffffffff\0" \
 	"fdt_file=vf610-twr.dtb\0" \
-	"fdt_addr=0x81000000\0" \
+	"fdt_addr=0x84000000\0" \
 	"boot_fdt=try\0" \
 	"ip_dyn=yes\0" \
 	"mmcdev=" __stringify(CONFIG_SYS_MMC_ENV_DEV) "\0" \
-- 
2.6.1

[U-Boot] [PATCH] arm: vf610: move device tree after kernel image

[Albert ARIBAUD]

Hello Stefan,

On Tue, 13 Oct 2015 23:11:45 -0700, Stefan Agner <stefan@agner.ch>
wrote:
> Since the device tree relocation is disabled (fdt_high set to
> 0xffffffff), U-Boot keeps the device tree at its load address
> 0x81000000. The kernel uncompresses itself to 0x80008000 by
> default, hence this limits the maximum (uncompressed) kernel
> size to somewhat below 16MiB, otherwise the device tree gets
> overwritten by the kernel data...
> 
> Move the device tree load address to 0x84000000 to avoid that
> the device tree being overwritten by the kernel.

OOC, why is device tree relocation disabled? I'm asking because by
manually placing the device tree (or anything else) high in DDR, one
runs the risk of overwriting some of U-Boot's data.

Amicalement,
-- 
Albert.

[U-Boot] [PATCH] arm: vf610: move device tree after kernel image

[Albert ARIBAUD]

On Wed, 14 Oct 2015 10:36:42 +0200, Albert ARIBAUD
<albert.u.boot@aribaud.net> wrote:
> Hello Stefan,
> 
> On Tue, 13 Oct 2015 23:11:45 -0700, Stefan Agner <stefan@agner.ch>
> wrote:
> > Since the device tree relocation is disabled (fdt_high set to
> > 0xffffffff), U-Boot keeps the device tree at its load address
> > 0x81000000. The kernel uncompresses itself to 0x80008000 by
> > default, hence this limits the maximum (uncompressed) kernel
> > size to somewhat below 16MiB, otherwise the device tree gets
> > overwritten by the kernel data...
> > 
> > Move the device tree load address to 0x84000000 to avoid that
> > the device tree being overwritten by the kernel.
> 
> OOC, why is device tree relocation disabled? I'm asking because by
> manually placing the device tree (or anything else) high in DDR, one
> runs the risk of overwriting some of U-Boot's data.

Also, nitpick: this is not "vf610:" but "vf610twr:" (although some
other vf610 boards might need to perform the same fix, depending on the
size of their kernel) -- not that it requires a v2, mind you, the
committer could just fix the subject on-the-fly.

Amicalement,
-- 
Albert.

[U-Boot] [PATCH] arm: vf610: move device tree after kernel image

[Tom Rini]

On Tue, Oct 13, 2015 at 11:11:45PM -0700, Stefan Agner wrote:

> Since the device tree relocation is disabled (fdt_high set to
> 0xffffffff), U-Boot keeps the device tree at its load address
> 0x81000000. The kernel uncompresses itself to 0x80008000 by
> default, hence this limits the maximum (uncompressed) kernel
> size to somewhat below 16MiB, otherwise the device tree gets
> overwritten by the kernel data...
> 
> Move the device tree load address to 0x84000000 to avoid that
> the device tree being overwritten by the kernel.
> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
>  include/configs/vf610twr.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/configs/vf610twr.h b/include/configs/vf610twr.h
> index 324ba8f..1d15f35 100644
> --- a/include/configs/vf610twr.h
> +++ b/include/configs/vf610twr.h
> @@ -129,7 +129,7 @@
>  	"fdt_high=0xffffffff\0" \
>  	"initrd_high=0xffffffff\0" \
>  	"fdt_file=vf610-twr.dtb\0" \
> -	"fdt_addr=0x81000000\0" \
> +	"fdt_addr=0x84000000\0" \
>  	"boot_fdt=try\0" \
>  	"ip_dyn=yes\0" \
>  	"mmcdev=" __stringify(CONFIG_SYS_MMC_ENV_DEV) "\0" \

OK, this is all wrong :(  Please take a look at
include/configs/ti_armv7_common.h and DEFAULT_LINUX_BOOT_ENV (and the
giant comment block explaining the values).

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20151014/22d3c53f/attachment.sig>

[U-Boot] [PATCH] arm: vf610: move device tree after kernel image

[Stefan Agner]

Hi Tom,

On 2015-10-14 05:39, Tom Rini wrote:
> On Tue, Oct 13, 2015 at 11:11:45PM -0700, Stefan Agner wrote:
> 
>> Since the device tree relocation is disabled (fdt_high set to
>> 0xffffffff), U-Boot keeps the device tree at its load address
>> 0x81000000. The kernel uncompresses itself to 0x80008000 by
>> default, hence this limits the maximum (uncompressed) kernel
>> size to somewhat below 16MiB, otherwise the device tree gets
>> overwritten by the kernel data...
>>
>> Move the device tree load address to 0x84000000 to avoid that
>> the device tree being overwritten by the kernel.
>>
>> Signed-off-by: Stefan Agner <stefan@agner.ch>
>> ---
>>  include/configs/vf610twr.h | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/include/configs/vf610twr.h b/include/configs/vf610twr.h
>> index 324ba8f..1d15f35 100644
>> --- a/include/configs/vf610twr.h
>> +++ b/include/configs/vf610twr.h
>> @@ -129,7 +129,7 @@
>>  	"fdt_high=0xffffffff\0" \
>>  	"initrd_high=0xffffffff\0" \
>>  	"fdt_file=vf610-twr.dtb\0" \
>> -	"fdt_addr=0x81000000\0" \
>> +	"fdt_addr=0x84000000\0" \
>>  	"boot_fdt=try\0" \
>>  	"ip_dyn=yes\0" \
>>  	"mmcdev=" __stringify(CONFIG_SYS_MMC_ENV_DEV) "\0" \
> 
> OK, this is all wrong :(  Please take a look at
> include/configs/ti_armv7_common.h and DEFAULT_LINUX_BOOT_ENV (and the
> giant comment block explaining the values).

This default environment is quite old, and according to the log it came
from Timesys/Freescales initial BSP.

The end of the DDR is commonly used for the second Cortex-M4 core. Hence
I guess fdt_high=0xffffffff has been set to avoid that U-Boot relocates
the device tree into the DDR area used by the M4. Note that this board
only has 128MiB of RAM,

I see that the comments in ti_armv7_common.h is mostly about moving
stuff into the upper 128MiB, which we don't have. So, with these two
things in mind, what do you want me to do differently?

--
Stefan

[U-Boot] [PATCH] arm: vf610: move device tree after kernel image

[Tom Rini]

On Wed, Oct 14, 2015 at 09:05:33AM -0700, Stefan Agner wrote:
> Hi Tom,
> 
> On 2015-10-14 05:39, Tom Rini wrote:
> > On Tue, Oct 13, 2015 at 11:11:45PM -0700, Stefan Agner wrote:
> > 
> >> Since the device tree relocation is disabled (fdt_high set to
> >> 0xffffffff), U-Boot keeps the device tree at its load address
> >> 0x81000000. The kernel uncompresses itself to 0x80008000 by
> >> default, hence this limits the maximum (uncompressed) kernel
> >> size to somewhat below 16MiB, otherwise the device tree gets
> >> overwritten by the kernel data...
> >>
> >> Move the device tree load address to 0x84000000 to avoid that
> >> the device tree being overwritten by the kernel.
> >>
> >> Signed-off-by: Stefan Agner <stefan@agner.ch>
> >> ---
> >>  include/configs/vf610twr.h | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/include/configs/vf610twr.h b/include/configs/vf610twr.h
> >> index 324ba8f..1d15f35 100644
> >> --- a/include/configs/vf610twr.h
> >> +++ b/include/configs/vf610twr.h
> >> @@ -129,7 +129,7 @@
> >>  	"fdt_high=0xffffffff\0" \
> >>  	"initrd_high=0xffffffff\0" \
> >>  	"fdt_file=vf610-twr.dtb\0" \
> >> -	"fdt_addr=0x81000000\0" \
> >> +	"fdt_addr=0x84000000\0" \
> >>  	"boot_fdt=try\0" \
> >>  	"ip_dyn=yes\0" \
> >>  	"mmcdev=" __stringify(CONFIG_SYS_MMC_ENV_DEV) "\0" \
> > 
> > OK, this is all wrong :(  Please take a look at
> > include/configs/ti_armv7_common.h and DEFAULT_LINUX_BOOT_ENV (and the
> > giant comment block explaining the values).
> 
> This default environment is quite old, and according to the log it came
> from Timesys/Freescales initial BSP.
> 
> The end of the DDR is commonly used for the second Cortex-M4 core. Hence
> I guess fdt_high=0xffffffff has been set to avoid that U-Boot relocates
> the device tree into the DDR area used by the M4. Note that this board
> only has 128MiB of RAM,
> 
> I see that the comments in ti_armv7_common.h is mostly about moving
> stuff into the upper 128MiB, which we don't have. So, with these two
> things in mind, what do you want me to do differently?

So, we want to use bootm_size whenever possible as the way to tell
U-Boot "kernel, ramdisk (if present) and fdt (if present) need to exist
within thus chunk of memory".  So instead of saying
fdt_high/initrd_high=0xffffffff we leave those alone, set bootm_size to
howmuch we can set aside for those three things and let U-Boot sort out
making sure things don't overlap and if they need to be moved.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20151014/6f77ef41/attachment.sig>

[U-Boot] [PATCH] arm: vf610: move device tree after kernel image

[Stefan Agner]

On 2015-10-14 01:36, Albert ARIBAUD wrote:
> Hello Stefan,
> 
> On Tue, 13 Oct 2015 23:11:45 -0700, Stefan Agner <stefan@agner.ch>
> wrote:
>> Since the device tree relocation is disabled (fdt_high set to
>> 0xffffffff), U-Boot keeps the device tree at its load address
>> 0x81000000. The kernel uncompresses itself to 0x80008000 by
>> default, hence this limits the maximum (uncompressed) kernel
>> size to somewhat below 16MiB, otherwise the device tree gets
>> overwritten by the kernel data...
>>
>> Move the device tree load address to 0x84000000 to avoid that
>> the device tree being overwritten by the kernel.
> 
> OOC, why is device tree relocation disabled? I'm asking because by
> manually placing the device tree (or anything else) high in DDR, one
> runs the risk of overwriting some of U-Boot's data.

I guess this has been done for Cortex-M4 firmwares running from DDR. As
Tom pointed out, this can be archived in a nicer way using bootm_size.

--
Stefan

[U-Boot] [PATCH] arm: vf610: move device tree after kernel image

[Albert ARIBAUD]

Hello Stefan,

On Wed, 14 Oct 2015 10:45:57 -0700, Stefan Agner <stefan@agner.ch>
wrote:
> On 2015-10-14 01:36, Albert ARIBAUD wrote:
> > Hello Stefan,
> > 
> > On Tue, 13 Oct 2015 23:11:45 -0700, Stefan Agner <stefan@agner.ch>
> > wrote:
> >> Since the device tree relocation is disabled (fdt_high set to
> >> 0xffffffff), U-Boot keeps the device tree at its load address
> >> 0x81000000. The kernel uncompresses itself to 0x80008000 by
> >> default, hence this limits the maximum (uncompressed) kernel
> >> size to somewhat below 16MiB, otherwise the device tree gets
> >> overwritten by the kernel data...
> >>
> >> Move the device tree load address to 0x84000000 to avoid that
> >> the device tree being overwritten by the kernel.
> > 
> > OOC, why is device tree relocation disabled? I'm asking because by
> > manually placing the device tree (or anything else) high in DDR, one
> > runs the risk of overwriting some of U-Boot's data.
> 
> I guess this has been done for Cortex-M4 firmwares running from DDR. As
> Tom pointed out, this can be archived in a nicer way using bootm_size.

Thanks -- I've just seen Tom's answer. I guess I'll update pcm052
too...

> Stefan

Amicalement,
-- 
Albert.