From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tom Rini Date: Wed, 28 Nov 2018 07:19:27 -0500 Subject: [U-Boot] [PATCH] ARM: rmobile: Convert to bootm_size In-Reply-To: References: <20181127001054.29623-1-marek.vasut+renesas@gmail.com> <8cd3e7ee-f5dd-61f3-082c-feec2777141f@gmail.com> <97181119-9bd1-8f37-7b1e-0624d077e571@gmail.com> <55912eb8-c0a7-9b5f-0dc1-f71a497ae564@gmail.com> <93b1f711-86ec-3b12-195f-6e447c1425ad@gmail.com> <20181128013121.GH11247@bill-the-cat> Message-ID: <20181128121927.GK11247@bill-the-cat> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Wed, Nov 28, 2018 at 09:46:43AM +0100, Simon Goldschmidt wrote: > On Wed, Nov 28, 2018 at 2:31 AM Tom Rini wrote: > > > > On Tue, Nov 27, 2018 at 07:31:24PM +0100, Simon Goldschmidt wrote: > > > On Tue, Nov 27, 2018 at 4:47 PM Marek Vasut 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 > > > > >>> wrote: > > > > >>>> On 11/27/2018 08:03 AM, Simon Goldschmidt wrote: > > > > >>>>> On Tue, Nov 27, 2018 at 1:11 AM Marek Vasut > > > > >>>>> 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?) > > > > It's all a little funny, yes. common/image.c is where all of this gets > > laid out and we keep everything being passed to bootm (whatever image > > type it may be) between bootm_low (either set explicitly or start of > > first bank of DRAM) and bootm_size (default ends up basically being all > > of DRAM) and then we do lmb stuff. > > > > The main reason, as I replied just now earlier in the thread, is that > > for Linux we need to make sure everything is in "lowmem" and rather than > > disabling relocation (which leads to people seeing fail to boots when > > something overlaps something else, over time, and they spend a bunch of > > time debugging) via initrd_high/fdt_high=0xffffffff SoCs should instead > > set a reasonable constraint on bootm_size so that we can make sure > > nothing overlaps, when we can do that. > > Right, I spent some time debugging this, too when starting with FIT > images containing Kernel, FDT, initrd and FPGA image... > > I wasn't aware of the requirement to have everything in "lowmem" for > Linux though (I'll even have to check what exactly the lowmem range is > for my platform/Kernel). And without this requirement, I just failed > to see why we need "boom_size" when it also works with defaulting to > just find a free memory block just somewhere in the DRAM. JFYI, the general lowmem limit is 768MB, but since it can be configured at build-time at least, it can actually get tiny (and off the top of my head, I don't think 256MB is as small as it can be, but rather a I think reasonable assumption on my part given the trade-offs in-kernel). -- Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: not available URL: