[PATCH v2] common: fdt: Remove additional 4k space for fdt allocation

[Heinrich Schuchardt]

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.soma at 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?

Best regards

Heinrich