[U-Boot] rpi: Adjust fdt_addr_r to a sane address

Message ID 20180413154900.84208-1-agraf@suse.de
State Accepted, archived
Delegated to: Alexander Graf
Headers show
Series
  • [U-Boot] rpi: Adjust fdt_addr_r to a sane address
Related show

Commit Message

Alexander Graf April 13, 2018, 3:49 p.m.
Back in the old days, 0x100 was used as the address to pass the device tree
from firmware into the kernel. This has since changed to a more dynamic
location, so using 0x100 actually breaks more things than it helps with.

Let's move the device tree default location for distro boot to a more sane
place that gives us enough head room in low memory.

Reported-by: Tuomas Tynkkynen <tuomas@tuxera.com>
Signed-off-by: Alexander Graf <agraf@suse.de>
---
 include/configs/rpi.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Tuomas Tynkkynen April 14, 2018, 6:04 p.m. | #1
Hi Alexander,

On Fri, 13 Apr 2018 17:49:00 +0200
Alexander Graf <agraf@suse.de> wrote:

[...]
> 
> diff --git a/include/configs/rpi.h b/include/configs/rpi.h
> index 325e52a019..fcf7e0976b 100644
> --- a/include/configs/rpi.h
> +++ b/include/configs/rpi.h
> @@ -124,7 +124,7 @@
>  #define ENV_MEM_LAYOUT_SETTINGS \
>  	"fdt_high=ffffffff\0" \
>  	"initrd_high=ffffffff\0" \
> -	"fdt_addr_r=0x00000100\0" \
> +	"fdt_addr_r=0x01f00000\0" \
>  	"pxefile_addr_r=0x00100000\0" \
>  	"kernel_addr_r=0x01000000\0" \
>  	"scriptaddr=0x02000000\0" \

Note that above the #define is a larger comment block that needs to be
updated as well. Also the other addresses also need updatingfor bigger
kernels on AArch64: https://patchwork.ozlabs.org/patch/777725/

Though now I double-checked that the smallest possible GPU-CPU memory
split is actually 64MB for the CPU, not 128M. So maybe something like:

         "kernel_addr_r=0x00080000\0" \
         "fdt_addr_r=0x02400000\0" \
         "scriptaddr=0x02500000\0" \
         "pxefile_addr_r=0x02600000\0" \
         "ramdisk_addr_r=0x02700000\0"

which would allow a kernel up to 36M, 1M for dtb, script and pxe files
each, and at least 25M for the initrd. Also I think giving up with the
constraint of locating the zImage high enough so that the kernel
decompressor doesn't need to relocate itself can be dropped. If the
boot speed of their Raspi matters that much, probably they wouldn't use
U-Boot in the first place.

What is the address that the RPi firmware loads its device tree to? I
hope that we don't have to worry about the positioning of that too...

- Tuomas
Tuomas Tynkkynen April 20, 2018, 10:03 a.m. | #2
Hi Alexander,

What do you think of these patches? I haven't done testing with the
big kernels / DTBs yet, just that my previously-working kernel still
boots.

Tuomas Tynkkynen (2):
  rpi: Fix fdt_high & initrd_high for 64-bit builds
  rpi: Change load addresses to make more room for the kernel & DTB

 include/configs/rpi.h | 73 ++++++++++++++++++++++++++++++++-------------------
 1 file changed, 46 insertions(+), 27 deletions(-)
Tuomas Tynkkynen April 22, 2018, 7:41 p.m. | #3
On Fri, 20 Apr 2018 13:03:47 +0300
Tuomas Tynkkynen <tuomas@tuxera.com> wrote:

> Hi Alexander,
> 
> What do you think of these patches? I haven't done testing with the
> big kernels / DTBs yet, just that my previously-working kernel still
> boots.
> 

I've now verified that these two patches work as expected. I tested:

- RPi 1 running mainline kernel
- RPi 1 running downstream kernel
- RPi 3 running mainline kernel in 64-bit mode
- RPi 3 running mainline kernel in 32-bit mode
- RPi 3 running downstream kernel in 32-bit mode
- RPi 3B+ running downstream kernel in 32-bit mode

This was with the extlinux.conf distro boot. I don't know to what
extent these variables affect EFI boot.

> Tuomas Tynkkynen (2):
>   rpi: Fix fdt_high & initrd_high for 64-bit builds
>   rpi: Change load addresses to make more room for the kernel & DTB
>
Alexander Graf May 24, 2018, 8:12 a.m. | #4
On 14.04.18 20:04, Tuomas Tynkkynen wrote:
> Hi Alexander,
> 
> On Fri, 13 Apr 2018 17:49:00 +0200
> Alexander Graf <agraf@suse.de> wrote:
> 
> [...]
>>
>> diff --git a/include/configs/rpi.h b/include/configs/rpi.h
>> index 325e52a019..fcf7e0976b 100644
>> --- a/include/configs/rpi.h
>> +++ b/include/configs/rpi.h
>> @@ -124,7 +124,7 @@
>>  #define ENV_MEM_LAYOUT_SETTINGS \
>>  	"fdt_high=ffffffff\0" \
>>  	"initrd_high=ffffffff\0" \
>> -	"fdt_addr_r=0x00000100\0" \
>> +	"fdt_addr_r=0x01f00000\0" \
>>  	"pxefile_addr_r=0x00100000\0" \
>>  	"kernel_addr_r=0x01000000\0" \
>>  	"scriptaddr=0x02000000\0" \
> 
> Note that above the #define is a larger comment block that needs to be
> updated as well. Also the other addresses also need updatingfor bigger
> kernels on AArch64: https://patchwork.ozlabs.org/patch/777725/
> 
> Though now I double-checked that the smallest possible GPU-CPU memory
> split is actually 64MB for the CPU, not 128M. So maybe something like:
> 
>          "kernel_addr_r=0x00080000\0" \
>          "fdt_addr_r=0x02400000\0" \
>          "scriptaddr=0x02500000\0" \
>          "pxefile_addr_r=0x02600000\0" \
>          "ramdisk_addr_r=0x02700000\0"
> 
> which would allow a kernel up to 36M, 1M for dtb, script and pxe files
> each, and at least 25M for the initrd. Also I think giving up with the
> constraint of locating the zImage high enough so that the kernel
> decompressor doesn't need to relocate itself can be dropped. If the
> boot speed of their Raspi matters that much, probably they wouldn't use
> U-Boot in the first place.
> 
> What is the address that the RPi firmware loads its device tree to? I
> hope that we don't have to worry about the positioning of that too...

U-Boot> bdinfo
arch_number = 0x00000000
boot_params = 0x00000100
DRAM bank   = 0x00000000
-> start    = 0x00000000
-> size     = 0x3B400000
baudrate    = 115200 bps
TLB addr    = 0x3B3F0000
relocaddr   = 0x3B348000
reloc off   = 0x3B2C8000
irq_sp      = 0x3AF3E120
sp start    = 0x3AF3E120
Early malloc usage: 138 / 2000
fdt_blob = 000000003af3e130
U-Boot> print fdt_addr
fdt_addr=2effb300

So on boot the DT passed into U-Boot is either at ~750MB or close to the
top. I can't quite find any code that explains the difference in the two
variables. Either way, I guess firmware tries to put it reasonably high?

So as long as we keep the load addresses low enough, we should be safe.

Also fyi, I would like to switch to CONFIG_OF_BOARD by default, once I
added code that marks u-boot.bin as an "upstream kernel" for the RPi
firmware, because then newer RPi firmwares will pass us a fully upstream
compatible device tree [1] which we can then pass on to Linux as default.


Alex

[1] https://github.com/raspberrypi/firmware/issues/943
Tuomas Tynkkynen May 24, 2018, 11:07 a.m. | #5
Hi Alexander,

On Thu, 24 May 2018 10:12:54 +0200
Alexander Graf <agraf@suse.de> wrote:

> On 14.04.18 20:04, Tuomas Tynkkynen wrote:
> > Hi Alexander,
> > 
> > On Fri, 13 Apr 2018 17:49:00 +0200
> > Alexander Graf <agraf@suse.de> wrote:
> > 
> > [...]  
> >>
> >> diff --git a/include/configs/rpi.h b/include/configs/rpi.h
> >> index 325e52a019..fcf7e0976b 100644
> >> --- a/include/configs/rpi.h
> >> +++ b/include/configs/rpi.h
> >> @@ -124,7 +124,7 @@
> >>  #define ENV_MEM_LAYOUT_SETTINGS \
> >>  	"fdt_high=ffffffff\0" \
> >>  	"initrd_high=ffffffff\0" \
> >> -	"fdt_addr_r=0x00000100\0" \
> >> +	"fdt_addr_r=0x01f00000\0" \
> >>  	"pxefile_addr_r=0x00100000\0" \
> >>  	"kernel_addr_r=0x01000000\0" \
> >>  	"scriptaddr=0x02000000\0" \  
> > 
> > Note that above the #define is a larger comment block that needs to be
> > updated as well. Also the other addresses also need updatingfor bigger
> > kernels on AArch64: https://patchwork.ozlabs.org/patch/777725/
> > 
> > Though now I double-checked that the smallest possible GPU-CPU memory
> > split is actually 64MB for the CPU, not 128M. So maybe something like:
> > 
> >          "kernel_addr_r=0x00080000\0" \
> >          "fdt_addr_r=0x02400000\0" \
> >          "scriptaddr=0x02500000\0" \
> >          "pxefile_addr_r=0x02600000\0" \
> >          "ramdisk_addr_r=0x02700000\0"
> > 
> > which would allow a kernel up to 36M, 1M for dtb, script and pxe files
> > each, and at least 25M for the initrd. Also I think giving up with the
> > constraint of locating the zImage high enough so that the kernel
> > decompressor doesn't need to relocate itself can be dropped. If the
> > boot speed of their Raspi matters that much, probably they wouldn't use
> > U-Boot in the first place.
> > 
> > What is the address that the RPi firmware loads its device tree to? I
> > hope that we don't have to worry about the positioning of that too...  
> 
> U-Boot> bdinfo
> arch_number = 0x00000000
> boot_params = 0x00000100
> DRAM bank   = 0x00000000
> -> start    = 0x00000000
> -> size     = 0x3B400000  
> baudrate    = 115200 bps
> TLB addr    = 0x3B3F0000
> relocaddr   = 0x3B348000
> reloc off   = 0x3B2C8000
> irq_sp      = 0x3AF3E120
> sp start    = 0x3AF3E120
> Early malloc usage: 138 / 2000
> fdt_blob = 000000003af3e130
> U-Boot> print fdt_addr
> fdt_addr=2effb300
> 
> So on boot the DT passed into U-Boot is either at ~750MB or close to the
> top. I can't quite find any code that explains the difference in the two
> variables. Either way, I guess firmware tries to put it reasonably high?

I believe fdt_addr is what is passed to the kernel and fdt_blob is what
U-Boot's device model is using.

> So as long as we keep the load addresses low enough, we should be safe.

Good.

> Also fyi, I would like to switch to CONFIG_OF_BOARD by default, once I
> added code that marks u-boot.bin as an "upstream kernel" for the RPi
> firmware, because then newer RPi firmwares will pass us a fully upstream
> compatible device tree [1] which we can then pass on to Linux as default.

Makes sense, but I guess needs some documentation. Also for existing things
like whether enable_uart=1 is needed or not.

> 
> Alex
> 
> [1] https://github.com/raspberrypi/firmware/issues/943

Thanks,
Tuomas

Patch

diff --git a/include/configs/rpi.h b/include/configs/rpi.h
index 325e52a019..fcf7e0976b 100644
--- a/include/configs/rpi.h
+++ b/include/configs/rpi.h
@@ -124,7 +124,7 @@ 
 #define ENV_MEM_LAYOUT_SETTINGS \
 	"fdt_high=ffffffff\0" \
 	"initrd_high=ffffffff\0" \
-	"fdt_addr_r=0x00000100\0" \
+	"fdt_addr_r=0x01f00000\0" \
 	"pxefile_addr_r=0x00100000\0" \
 	"kernel_addr_r=0x01000000\0" \
 	"scriptaddr=0x02000000\0" \