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

[Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>]

On Tue, Nov 27, 2018 at 4:47 PM Marek Vasut <marek.vasut@gmail.com> wrote:
>
> On 11/27/2018 04:26 PM, Simon Goldschmidt wrote:
> > 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@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@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?
>
> If I can use the entire RAM (except U-Boot and fitImage), that'd be
> nice. What change do I need to do ?

I don't know yet, sorry. I basically asked this question to find out
about the usage of 'bootm_size'. It's not really documented and I
couldn't find out it's full meaning yet. Because e.g. it is set to 16
MiB for socfpga gen5, which sounds a little low...

From reading the code, doesn't it already work when leaving out
'bootm_size'? (And leaving out 'bootm_mapsize' as well and not
defining CONFIG_SYS_BOOTMAPSZ?)

But I don't really know, finding that out and making it work is one of
my goals for that series I'm working on. The series started with
allowing all subitems in a FIT to be uncompressed but I found some
issues there and it grows...

Regards,
Simon