On 6/12/20 4:56 PM, Heinrich Schuchardt wrote: > On 6/12/20 9:28 PM, Stephen Warren wrote: >> On 6/11/20 5:10 AM, Michal Simek wrote: >>> From: Ashok Reddy Soma <ashok.reddy.s...@xilinx.com> >>> >>> There is no technical reason to add additional 4k space for FDT. This space >>> is completely unused and just increase memory requirements. This is >>> problematic on systems with limited memory resources as Xilinx Zynq >>> CSE/ZynqMP mini and Versal mini configurations. >> >> I tried to work out where this additional space came from, but can't >> find any obvious clues why it's there. >> >> The extra 4k was originally introduced in x86-specific code by: >> >> commit f697d528caba0c30382b7269fd36f1111e51810d >> x86: Support relocation of FDT on start-up >> +int copy_fdt_to_ram(void) >> + fdt_size = ALIGN(fdt_totalsize(gd->fdt_blob) + 0x1000, 32); >> >> ... and later copied to generic code by: >> >> commit 1938f4a5b62fc03c52b47697a89b2bb47b77c90c (HEAD) >> Introduce generic pre-relocation board_f.c >> +static int reserve_fdt(void) >> + gd->fdt_size = ALIGN(fdt_totalsize(gd->fdt_blob) + >> 0x1000, 32); >> >> However, there's no obvious comment re: why 4k was added. I wonder if >> it's anything to do with 4k==page size and hence different MMU >> permissions for the DTB and any data following it? That's purely a guess >> though, since I don't see anything in those patches messing with the MMU> >> any differently for the DTB. > > Target $(obj)/%.dtb.S in scripts/Makefile.lib only guarantees 16 byte > alignment of embedded device trees. It does not make any sense to > provide extra 4KiB in the case of CONFIG_OF_EMBED=y and not for > CONFIG_OF_EMBED=n. > > I do not see a need for using ALIGN for the fdt size. > >> So I guess if this patch passes testing, it's good:-) > > Which test will check that we do not have any buffer overruns due to > increasing the device tree size before we copy the device tree in a boot > command?
We'd need to boot U-Boot on all the boards it supports. In other words, apply it early during a release cycle and watch for any fallout when people test it I guess.