On 11/27/2018 07:31 PM, Simon Goldschmidt wrote: > On Tue, Nov 27, 2018 at 4:47 PM Marek Vasut <marek.va...@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.va...@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.va...@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...
Thanks for all that work :-) -- Best regards, Marek Vasut _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
