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

[Stephen Warren]

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

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.