[PATCH 1/1] image: usage of value ~0UL for intrd_high

[Heinrich Schuchardt]

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 <trini at konsulko.com>:
>>>>> 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
>>>>> <xypron.glpk at gmx.de> 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.

Best regards

Heinrich

>
> Now, it's I gather not clear that we're NOT setting initrd_high here
> (nor fdt_high) because we're setting reasonable defaults for everyone to
> use, so long as they have enough memory.
>