[U-Boot] [PATCH] ARM: rmobile: Convert to bootm_size

[Simon Goldschmidt]

On 27.11.2018 14:09, Marek Vasut wrote:
> On 11/27/2018 01:33 PM, Simon Goldschmidt wrote:
>> On Tue, Nov 27, 2018 at 1:25 PM Marek Vasut <marek.vasut at gmail.com> wrote:
>>> On 11/27/2018 08:03 AM, Simon Goldschmidt wrote:
>>>> On Tue, Nov 27, 2018 at 1:11 AM Marek Vasut <marek.vasut at gmail.com> wrote:
>>>>> Convert all Renesas R-Car boards to bootm_size of 256 MiB and drop both
>>>>> fdt_high and initrd_high. This change implies that the FDT and initrd
>>>>> will always be copied into the first 256 MiB of RAM instead of being
>>>>> used in place, which can cause various kinds of inobvious problems.
>>>>>
>>>>> The simpler problems include FDT or initrd being overwritten or being
>>>>> used from unaligned addresses, especially on ARM64. The overhead of
>>>>> copying the FDT to aligned location is negligible and these problems
>>>>> go away, so the benefit is significant.
>>>>>
>>>>> Regarding alignment problems with fitImage. The alignment of DT properties
>>>>> is always 32 bits, which implies that the alignment of the "data" property
>>>>> in fitImage is also 32 bits. The /incbin/ syntax plays no role here. The
>>>>> kernel expects all elements, including DT and initrd, to be aligned to
>>>>> 64 bits on ARM64, thus using them in place may not be possible. Using the
>>>>> bootm_size assures correct alignment, again with negligible overhead.
>>>> In my opinion, all of these raw addresses defined in scripts or config
>>>> should be removed: They are probably vulnerable to overwriting
>>>> themselves as they only provide an address, not a range.
>>> This is not an address, it's size. And this one at least assures that
>>> the first 256 MiB are reserved for the kernel/FDT/initrd during bootm time.
>> Sorry I did not express myself clear enough. I meant that "fdt_high"
>> and "initrd_high" are bad because they contain an address only, not a
>> range. The 'bootm_size' thing is much better!
> Well the fdt_high and intrd_high can also contain a special ~0 value,
> which says "use the fdt/initrd in place", which is dangerous.
>
>>>> Just out of curiosity: is it required to put fdt and initrd into the
>>>> first 256 MiB or is this just some 'random' limit to ensure we use lmb
>>>> but don't overwrite U-Boot (text, heap, stack, etc)? Because if so, my
>>>> series to fix the recent CVE issues improves lmb to not overwrite
>>>> U-Boot and other reserved addresses and you might be able to remove
>>>> 'bootm_size', too. The improved lmb code would just allocate an
>>>> aligned address somewhere in the available RAM.
>>> It's just the first 256 MiB from the beginning, so there's enough space
>>> between that and U-Boot on all these boards.
>> Of course. I wanted to know if it would be good enough if U-Boot would
>> just put it somewhere without overwriting things or do you really need
>> them in the first 256 MiB? Because the revised lmb code would make
>> sure there's nothing overwritten, so there would be no need to trim at
>> 256 MiB.
> You can put them anywhere, you just need to meet the alignment
> requirements. Can the new LMB code help somehow with that ? And if so, how ?

My additions to the LMB code should only ensure nothing gets overwritten 
so you don't have to limit boom_size to 256MiB (but use the complete RAM 
when bootm_size is not set).
Alignment does not change but should already be OK with LMB as you use it?

Simon