From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tom Rini Date: Fri, 15 Jan 2021 13:43:31 -0500 Subject: [PATCH 1/1] image: usage of value ~0UL for intrd_high In-Reply-To: References: <20210109180617.42030-1-xypron.glpk@gmx.de> <20210109185836.GJ2292@bill-the-cat> <20210109194004.GK2292@bill-the-cat> <20210109212301.GL2292@bill-the-cat> <5fa3e58e-1a72-030a-893b-bfbf0c97bd65@gmx.de> <20210110134302.GM2292@bill-the-cat> Message-ID: <20210115184331.GD9782@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 Sun, Jan 10, 2021 at 05:20:50PM +0100, Heinrich Schuchardt wrote: > On 1/10/21 2:43 PM, Tom Rini wrote: > > On Sun, Jan 10, 2021 at 01:05:14PM +0100, Heinrich Schuchardt wrote: > > > On 1/9/21 10:23 PM, Tom Rini wrote: > > > > On Sat, Jan 09, 2021 at 08:59:01PM +0100, Heinrich Schuchardt wrote: > > > > > Am 9. Januar 2021 20:40:04 MEZ schrieb Tom Rini : > > > > > > On Sat, Jan 09, 2021 at 08:33:40PM +0100, Heinrich Schuchardt wrote: > > > > > > > On 1/9/21 7:58 PM, Tom Rini wrote: > > > > > > > > On Sat, Jan 09, 2021 at 08:47:07PM +0200, Andy Shevchenko wrote: > > > > > > > > > On Sat, Jan 9, 2021 at 8:06 PM Heinrich Schuchardt > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > The comment for initrd_high in the coding and in README were > > > > > > contradicting > > > > > > > > > > and neither fully described what the coding does. > > > > > > > > > > > > > > > > > > > > Clarify the usage of the special value ~0UL for the environment > > > > > > variable > > > > > > > > > > initrd_high. > > > > > > > > > > > > > > > > > > All those F:s are hard to read in the comments and documentation > > > > > > and > > > > > > > > > typo prone. I would prefer to rephrase like "all 1:s value in 32- > > > > > > or > > > > > > > > > 64-bit format" or alike. > > > > > > > > > > > > > > > > If we're going to improve this we should also note it's discouraged > > > > > > > > unless you know for certain there will be no overlap and it's > > > > > > strongly > > > > > > > > discouraged in default environments. > > > > > > > > > > > > > > What exactly is discouraged? > > > > > > > > > > > > > > * setting initrd_high to a value != ~0? Here I would agree. > > > > > > > * setting intird_high to ~0? Why should we copy initrd to a > > > > > > > different place? Is it for some outdated Linux release? > > > > > > > > > > > > We should always default to allowing the initrd to be relocated because > > > > > > we can see (in many cases) overlap that will lead to failure to boot > > > > > > but > > > > > > this forces us to ignore that. Having good default load values means > > > > > > we > > > > > > don't have a problem here. > > > > > > > > > > We have an initrd that is already in memory. What could it overlap > > > > > with that is not already overwritten? > > > > > > > > Having the kernel and initrd too close in memory has the kernel BSS > > > > overwrite the initrd. This has happened time and time again before > > > > I went around making some platforms have reasonable (ie kernel early, > > > > ramdisk in lowmem but beyond where a kernel+bss can be, etc) defaults > > > > and pushing others to do the same. > > > > > > > > > Can you provide the text you want to see here? > > > > > > > > Off-hand, it should look more like the big comment block in > > > > include/configs/ti_armv7_common.h and reference the Linux booting on > > > > arm/arm64 documents while noting that other architectures have the same > > > > fundamental issues and their exact limits may or may not be as well > > > > documented. > > > > > > > > > > There is nothing in the include/configs/ti_armv7_common.h comments > > > requiring to relocate initrd. > > > > /* > > * We setup defaults based on constraints from the Linux kernel, which should > > * also be safe elsewhere. We have the default load at 32MB into DDR (for > > * the kernel), FDT above 128MB (the maximum location for the end of the > > * kernel), and the ramdisk 512KB above that (allowing for hopefully never > > * seen large trees). We say all of this must be within the first 256MB > > * as that will normally be within the kernel lowmem and thus visible via > > * bootm_size and we only run on platforms with 256MB or more of memory. > > * > > * As a temporary storage for DTBO blobs (which should be applied into DTB > > * blob), we use the location 15.5 MB above the ramdisk. If someone wants to > > * use ramdisk bigger than 15.5 MB, then DTBO can be loaded and applied to DTB > > * blob before loading the ramdisk, as DTBO location is only used as a temporary > > * storage, and can be re-used after 'fdt apply' command is done. > > */ > > I cannot see how this relates to initrd relocation. You cannot safely disable initrd relocation without ensuring it won't be locked in an unsafe location first. It's also _generally_ going to be noise in the boot sequence and not an advisable default. It should be documented as part of how to tune a system for optimal boot times. But given how often defaults are copy/pasted to the next platform without fully understanding why those values were picked is why what we use as default in one platform needs to be very carefully done. -- Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 659 bytes Desc: not available URL: