Skip to contentSkip to conversation

rpi: Adjust fdt_addr_r to a sane address

PATCH New Alexander Graf 2 participants 11 messages Apr 13, 2018 – May 24, 2018

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 rpi: Adjust fdt_addr_r to a sane address

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.

---
 include/configs/rpi.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
include/configs/rpi.h +1 -1
index 325e52a019..fcf7e0976b 100644
@@ -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" \
Signature · 1 line
2.12.3
#1 lore raw

PATCH 1/1 rpi: Adjust fdt_addr_r to a sane address

Hi Alexander,

On Fri, 13 Apr 2018 17:49:00 +0200
Alexander Graf <[email protected]> wrote:

[...]

Quoted 6 lines
include/configs/rpi.h +0 -0
index 325e52a019..fcf7e0976b 100644

include/configs/rpi.h line 130

Quoted 8 lines
include/configs/rpi.h +1 -1
@@ -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

#2 lore raw

PATCH 1/1 rpi: Adjust fdt_addr_r to a sane address

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(-)
Signature · 1 line
2.16.3
#3 lore raw

PATCH 1/2 rpi: Fix fdt_high & initrd_high for 64-bit builds

The magic value that disables relocation is dependent on the CPU word size, so the current 'ffffffff' is doing the wrong thing on aarch64.

---
 include/configs/rpi.h | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)
include/configs/rpi.h +10 -2
index 325e52a019..f1189a27f3 100644
@@ -91,6 +91,14 @@
 	"stdout=serial,vidconsole\0" \
 	"stderr=serial,vidconsole\0"
 
+#ifdef CONFIG_ARM64
+#define FDT_HIGH "ffffffffffffffff"
+#define INITRD_HIGH "ffffffffffffffff"
+#else
+#define FDT_HIGH "ffffffff"
+#define INITRD_HIGH "ffffffff"
+#endif
+
 /*
  * Memory layout for where various images get loaded by boot scripts:
  *
@@ -122,8 +130,8 @@
  *   for any boot script to be up to 1M, which is hopefully plenty.
  */
 #define ENV_MEM_LAYOUT_SETTINGS \
-	"fdt_high=ffffffff\0" \
-	"initrd_high=ffffffff\0" \
+	"fdt_high=" FDT_HIGH "\0" \
+	"initrd_high=" INITRD_HIGH "\0" \
 	"fdt_addr_r=0x00000100\0" \
 	"pxefile_addr_r=0x00100000\0" \
 	"kernel_addr_r=0x01000000\0" \
Signature · 1 line
2.16.3
#4 lore raw

PATCH 1/2 rpi: Fix fdt_high & initrd_high for 64-bit builds

Quoted 4 lines · Tuomas Tynkkynen

The magic value that disables relocation is dependent on the CPU word size, so the current 'ffffffff' is doing the wrong thing on aarch64.

The BCM283x series of SOCs is limited to 32bit address space, so I don't quite see why the current (int)-1 is wrong?

Alex

Quoted 35 lines
---
 include/configs/rpi.h | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)
include/configs/rpi.h +10 -2
index 325e52a019..f1189a27f3 100644
@@ -91,6 +91,14 @@
 	"stdout=serial,vidconsole\0" \
 	"stderr=serial,vidconsole\0"
 
+#ifdef CONFIG_ARM64
+#define FDT_HIGH "ffffffffffffffff"
+#define INITRD_HIGH "ffffffffffffffff"
+#else
+#define FDT_HIGH "ffffffff"
+#define INITRD_HIGH "ffffffff"
+#endif
+
 /*
  * Memory layout for where various images get loaded by boot scripts:
  *
@@ -122,8 +130,8 @@
  *   for any boot script to be up to 1M, which is hopefully plenty.
  */
 #define ENV_MEM_LAYOUT_SETTINGS \
-	"fdt_high=ffffffff\0" \
-	"initrd_high=ffffffff\0" \
+	"fdt_high=" FDT_HIGH "\0" \
+	"initrd_high=" INITRD_HIGH "\0" \
 	"fdt_addr_r=0x00000100\0" \
 	"pxefile_addr_r=0x00100000\0" \
 	"kernel_addr_r=0x01000000\0" \
#5 lore raw

PATCH 1/2 rpi: Fix fdt_high & initrd_high for 64-bit builds

Hi Alex,

Quoted 10 lines · On Thu, 24 May 2018 09:51:57 +0200 Alexander Graf <[email protected]> wrote:
Quoted 4 lines · Tuomas Tynkkynen

The magic value that disables relocation is dependent on the CPU word size, so the current 'ffffffff' is doing the wrong thing on aarch64.

The BCM283x series of SOCs is limited to 32bit address space, so I don't quite see why the current (int)-1 is wrong?

The comparison for the magic "don't relocate value" is done by parsing the variable as ulong and then comparing to ~0. So on 64-bit, ffffffff gets interpreted as literal 0xffffffff limit for the relocation (which I think in practice is the same as not specifying initrd_high at all since the end of DRAM is lower than that) instead.

#6 lore raw

PATCH 1/2 rpi: Fix fdt_high & initrd_high for 64-bit builds

Quoted 16 lines · Tuomas Tynkkynen

Hi Alex,

Quoted 10 lines · On Thu, 24 May 2018 09:51:57 +0200 Alexander Graf <[email protected]> wrote:
Quoted 4 lines · Tuomas Tynkkynen

The magic value that disables relocation is dependent on the CPU word size, so the current 'ffffffff' is doing the wrong thing on aarch64.

The BCM283x series of SOCs is limited to 32bit address space, so I don't quite see why the current (int)-1 is wrong?

Quoted 5 lines

The comparison for the magic "don't relocate value" is done by parsing the variable as ulong and then comparing to ~0. So on 64-bit, ffffffff gets interpreted as literal 0xffffffff limit for the relocation (which I think in practice is the same as not specifying initrd_high at all since the end of DRAM is lower than that) instead.

Ouch, that logic is terrible. But it means your patch is correct. I'll apply it.

Thanks,

Alex

#7 lore raw

PATCH 2/2 rpi: Change load addresses to make more room for the kernel & DTB

In reply to Tuomas Tynkkynen

As of Linux 4.16, a multiplatform AArch64 kernel with our distro config takes 26M. The current space reservation leaves only 17M for the kernel and if it goes over it, the initrd gets overwritten when loading the kernel from the filesystem.

A similar problem happens on ARMv7 with the DTBs taken from the downstream Raspberry Pi foundation kernel. I guess they compile them with DT overlay support enabled which grows them just enough.

Fix both of these problems by rewriting the memory map, which now allows kernels to be up to 36M and DTBs up to 1M. Also the comment block was kind of obsolete ever since the introduction of AArch64 support and the firmware-loaded DTB doesn't get placed at 0x100 anymore either, so that is fixed as well.

---
 include/configs/rpi.h | 61 ++++++++++++++++++++++++++++++---------------------
 1 file changed, 36 insertions(+), 25 deletions(-)
include/configs/rpi.h +36 -25
index f1189a27f3..425c0c2e6d 100644
@@ -104,39 +104,50 @@
  *
  * I suspect address 0 is used as the SMP pen on the RPi2, so avoid this.
  *
- * fdt_addr_r simply shouldn't overlap anything else. However, the RPi's
- *   binary firmware loads a DT to address 0x100, so we choose this address to
- *   match it. This allows custom boot scripts to pass this DT on to Linux
- *   simply by not over-writing the data at this address. When using U-Boot,
- *   U-Boot (and scripts it executes) typicaly ignore the DT loaded by the FW
- *   and loads its own DT from disk (triggered by boot.scr or extlinux.conf).
+ * Older versions of the boot firmware place the firmware-loaded DTB at 0x100,
+ * newer versions place it in high memory. So prevent U-Boot from doing its own
+ * DTB + initrd relocation so that we won't accidentally relocate the initrd
+ * over the firmware-loaded DTB and generally try to lay out things starting
+ * from the bottom of RAM.
  *
- * pxefile_addr_r can be pretty much anywhere that doesn't conflict with
- *   something else. Put it low in memory to avoid conflicts.
+ * kernel_addr_r has different constraints on ARM and Aarch64.  For 32-bit ARM,
+ * it must be within the first 128M of RAM in order for the kernel's
+ * CONFIG_AUTO_ZRELADDR option to work. The kernel itself will be decompressed
+ * to 0x8000 but the decompressor clobbers 0x4000-0x8000 as well. The
+ * decompressor also likes to relocate itself to right past the end of the
+ * decompressed kernel, so in total the sum of the compressed and and
+ * decompressed kernel needs to be reserved.
  *
- * kernel_addr_r must be within the first 128M of RAM in order for the
- *   kernel's CONFIG_AUTO_ZRELADDR option to work. Since the kernel will
- *   decompress itself to 0x8000 after the start of RAM, kernel_addr_r
- *   should not overlap that area, or the kernel will have to copy itself
- *   somewhere else before decompression. Similarly, the address of any other
- *   data passed to the kernel shouldn't overlap the start of RAM. Pushing
- *   this up to 16M allows for a sizable kernel to be decompressed below the
- *   compressed load address.
+ *   For Aarch64, the kernel image is uncompressed and must be loaded at
+ *   text_offset bytes (specified in the header of the Image) into a 2MB
+ *   boundary. The 'booti' command relocates the image if necessary. Linux uses
+ *   a default text_offset of 0x80000.  In summary, loading at 0x80000
+ *   satisfies all these constraints and reserving memory up to 0x02400000
+ *   permits fairly large (roughly 36M) kernels.
  *
- * scriptaddr can be pretty much anywhere that doesn't conflict with something
- *   else. Choosing 32M allows for the compressed kernel to be up to 16M.
+ * scriptaddr and pxefile_addr_r can be pretty much anywhere that doesn't
+ * conflict with something else. Reserving 1M for each of them at
+ * 0x02400000-0x02500000 and 0x02500000-0x02600000 should be plenty.
  *
- * ramdisk_addr_r simply shouldn't overlap anything else. Choosing 33M allows
- *   for any boot script to be up to 1M, which is hopefully plenty.
+ * On ARM, both the DTB and any possible initrd must be loaded such that they
+ * fit inside the lowmem mapping in Linux. In practice, this usually means not
+ * more than ~700M away from the start of the kernel image but this number can
+ * be larger OR smaller depending on e.g. the 'vmalloc=xxxM' command line
+ * parameter given to the kernel. So reserving memory from low to high
+ * satisfies this constraint again. Reserving 1M at 0x02600000-0x02700000 for
+ * the DTB leaves rest of the free RAM to the initrd starting at 0x02700000.
+ * Even with the smallest possible CPU-GPU memory split of the CPU getting
+ * only 64M, the remaining 25M starting at 0x02700000 should allow quite
+ * large initrds before they start colliding with U-Boot.
  */
 #define ENV_MEM_LAYOUT_SETTINGS \
 	"fdt_high=" FDT_HIGH "\0" \
 	"initrd_high=" INITRD_HIGH "\0" \
-	"fdt_addr_r=0x00000100\0" \
-	"pxefile_addr_r=0x00100000\0" \
-	"kernel_addr_r=0x01000000\0" \
-	"scriptaddr=0x02000000\0" \
-	"ramdisk_addr_r=0x02100000\0" \
+	"kernel_addr_r=0x00080000\0" \
+	"scriptaddr=0x02400000\0" \
+	"pxefile_addr_r=0x02500000\0" \
+	"fdt_addr_r=0x02600000\0" \
+	"ramdisk_addr_r=0x02700000\0"
 
 #define BOOT_TARGET_DEVICES(func) \
 	func(MMC, mmc, 0) \
Signature · 1 line
2.16.3
#8 lore raw

PATCH 1/1 rpi: Adjust fdt_addr_r to a sane address

In reply to Tuomas Tynkkynen
Quoted 6 lines · On Fri, 20 Apr 2018 13:03:47 +0300 Tuomas Tynkkynen <[email protected]> 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.

Quoted 4 lines

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

#9 lore raw

PATCH 1/1 rpi: Adjust fdt_addr_r to a sane address

In reply to Tuomas Tynkkynen
Quoted 41 lines · Tuomas Tynkkynen

Hi Alexander,

On Fri, 13 Apr 2018 17:49:00 +0200
Alexander Graf <[email protected]> wrote:

[...]

Quoted 6 lines
include/configs/rpi.h +0 -0
index 325e52a019..fcf7e0976b 100644

include/configs/rpi.h line 130

Quoted 8 lines
include/configs/rpi.h +1 -1
@@ -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.

Quoted 2 lines

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

#10 lore raw

PATCH 1/1 rpi: Adjust fdt_addr_r to a sane address

Hi Alexander,

Quoted 57 lines · On Thu, 24 May 2018 10:12:54 +0200 Alexander Graf <[email protected]> wrote:
Quoted 41 lines · Tuomas Tynkkynen

Hi Alexander,

On Fri, 13 Apr 2018 17:49:00 +0200
Alexander Graf <[email protected]> wrote:

[...]

Quoted 6 lines
include/configs/rpi.h +0 -0
index 325e52a019..fcf7e0976b 100644

include/configs/rpi.h line 130

Quoted 8 lines
include/configs/rpi.h +1 -1
@@ -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.

Quoted 2 lines

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
Quoted 8 lines

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.

Quoted 1 line

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

Good.

Quoted 4 lines

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.

Quoted 4 lines

Alex

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

Thanks,
Tuomas