Re: u-boot places device-tree in bss

On 14/12/2025 21:57, Yuri Honegger wrote:
>
>> Am 14.12.2025 um 21:39 schrieb Nick Hudson <[email protected]>:
>>
>> On 14/12/2025 18:56, Yuri Honegger wrote:
>>> Hello,
>>> When the memory address for the device tree isn't provided to u-boot, u-boot
>>> decides where to place the device tree by itself. For me, it chooses to place it
>>> right after the code, overlapping the bss section. This means the device tree
>>> gets overwritten when clearing the bss and a bit later, NetBSD rightfully panics
>>> because of a corrupt device tree.
>>> Since u-boot leaves some space between the code and the device tree, it works
>>> with a small kernel, but as soon as I enable more features and bss grows, I run
>>> into this issue.
>> I'm a little surprised that NetBSD's GENERIC_V5 is bloated compared to your u-boot's idea of how big the kernel is, but not completely.
> I think u-boot might only considers text + other initialized segments and ignores bss when allocating the memory, then round up or something like that. I’ll have to investigate it more thoroughly.
Not sure how this works. From a quick glance at usr.bin/mkubootimage I 
don't think the bss is even registered in the netbsd.ub image.
>>> I can work around this by giving u-boot a boot.scr that manually places the
>>> device tree, but I'm wondering if this could be fixed in NetBSD.
>>> We could add a panic if we encounter this situation. That would be a bit more
>>> helpful than the current error:
>>> [ 1.0000000] panic: fdt_check_header failed: FDT_ERR_BADMAGIC
>> Sure... something like the attached. Oh, looking at arm32_bootmem_init it's a bit more complicated, but you get the idea.
> It almost works. One issue is that the fdt address is physical memory while the bss start/end addresses are virtual memory.
>
> In my case:
> FDT VA: 0x42b93000 PA: 0x42b93000
> BSS VA: 0xc05de580 PA: 0x425de580
>
> Once you account for that it it catches the issue for me. I’ve also changed to lower bound comparison to <=.
>
>
yeah, I realised I was probably mixing VA and PA. thanks for dealing 
with that.
> I don’t quite get your point about arm32_bootmem_init. When we reach it, we’ve already copied the fdt using fdt_open_into so we don’t need the original fdt anymore.

I mean something like the attached.

? sys/arch/evbarm/conf/DEBUG64
? sys/arch/evbarm/conf/GENERIC.new
? sys/arch/evbarm/conf/GENERIC64_UVMHIST
? sys/arch/evbarm/conf/VIRTNVMM64
? sys/arch/evbarm/conf/files.fdt_armv5
Index: sys/arch/evbarm/fdt/fdt_machdep.c
===================================================================
RCS file: /cvsroot/src/sys/arch/evbarm/fdt/fdt_machdep.c,v
retrieving revision 1.112
diff -u -p -r1.112 fdt_machdep.c
--- sys/arch/evbarm/fdt/fdt_machdep.c	4 Oct 2025 03:26:40 -0000	1.112
+++ sys/arch/evbarm/fdt/fdt_machdep.c	15 Dec 2025 07:09:57 -0000
@@ -30,6 +30,7 @@
 __KERNEL_RCSID(0, "$NetBSD: fdt_machdep.c,v 1.112 2025/10/04 03:26:40 thorpej Exp $");
 
 #include "opt_arm_debug.h"
+#include "opt_arm_start.h"
 #include "opt_bootconfig.h"
 #include "opt_cpuoptions.h"
 #include "opt_ddb.h"
@@ -257,10 +258,24 @@ initarm(void *arg)
 {
 	const struct fdt_platform *plat;
 	uint64_t memory_start, memory_end;
+	extern int KERNEL_BASE_virt[];
+	extern char const __start__init_memory[];
+	extern char const __stop__init_memory[] __weak;
 
 	/* set temporally to work printf()/panic() even before consinit() */
 	cn_tab = &earlycons;
 
+	vaddr_t kstartva = trunc_page((vaddr_t)KERNEL_BASE_virt);
+#if defined(__HAVE_GENERIC_START)
+	vaddr_t kendva = round_page((vaddr_t)__stop__init_memory);
+#else
+	extern char _end[];
+	vaddr_t kendva = round_page((vaddr_t)_end);
+#endif
+	vaddr_t fdt_va = KERN_PHYSTOV((paddr_t)fdt_addr_r);
+	if (kstartva <= fdt_va && fdt_va < kendva)
+		panic("FDT placed within loaded kernel");
+
 	/* Load FDT */
 	int error = fdt_check_header(fdt_addr_r);
 	if (error != 0)
@@ -411,9 +426,6 @@ initarm(void *arg)
 	 * Now we have APs started the pages used for stacks and L1PT can
 	 * be given to uvm
 	 */
-	extern char const __start__init_memory[];
-	extern char const __stop__init_memory[] __weak;
-
 	if (&__start__init_memory[0] != &__stop__init_memory[0]) {
 		const paddr_t spa = KERN_VTOPHYS((vaddr_t)__start__init_memory);
 		const paddr_t epa = KERN_VTOPHYS((vaddr_t)__stop__init_memory);