[U-Boot] [PATCH 20/20] tegra: config: nyan-big: Add options required by Chrome OS boot

[Stephen Warren <swarren@wwwdotorg.org>]

On 05/19/2015 12:01 PM, Simon Glass wrote:
> Hi Stephen,
>
> On 19 May 2015 at 09:41, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 05/18/2015 03:33 PM, Simon Glass wrote:
>>>
>>> Hi Stephen,
>>>
>>> On 15 May 2015 at 09:34, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>>
>>>> On 05/13/2015 07:56 AM, Simon Glass wrote:
>>>>>
>>>>>
>>>>> Hi Stephen,
>>>>>
>>>>> On 25 February 2015 at 16:31, Stephen Warren <swarren@wwwdotorg.org>
>>>>> wrote:
>>>>>>
>>>>>>
>>>>>> On 02/17/2015 03:29 PM, Simon Glass wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> We need to match the device tree in the FIT with the U-Boot model so
>>>>>>> we
>>>>>>> can automatically select the right device tree. Also adjust the load
>>>>>>> address
>>>>>>> so that the device tree is not in the way when a zImage kernel tries
>>>>>>> to
>>>>>>> extract itself.
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> We don't tend to use LOADADDR in any of the default boot scripts any
>>>>>> more.
>>>>>> Rather, we explicitly load files to a "semantic" location indicated by
>>>>>> one
>>>>>> of the following variables in tegra124-common.h:
>>>>>>
>>>>>> #define MEM_LAYOUT_ENV_SETTINGS \
>>>>>>            "scriptaddr=0x90000000\0" \
>>>>>>            "pxefile_addr_r=0x90100000\0" \
>>>>>>            "kernel_addr_r=0x81000000\0" \
>>>>>>            "fdt_addr_r=0x82000000\0" \
>>>>>>            "ramdisk_addr_r=0x82100000\0"
>>>>>>
>>>>>> Perhaps the ChromeOS boot scripts could be adjusted to use one/some of
>>>>>> those
>>>>>> variables?
>>>>>>
>>>>>> If the value of CONFIG_LOADADDR isn't appropriate, perhaps we should
>>>>>> fix
>>>>>> it
>>>>>> for all Tegra SoCs/boards?
>>>>>
>>>>>
>>>>>
>>>>> I forgot about this comment sorry. I had problems with the image
>>>>> overwriting itself. It is a bzImage inside a FIT so doesn't use the
>>>>> proper FIT decompression.
>>>>>
>>>>> Anyway I'd like to clarify what is meant by kernel_addr_r. Is that
>>>>> where the FIT is loaded or where the kernel will decompress to, or
>>>>> something else?
>>>>
>>>>
>>>>
>>>> kernel_addr_r is the address in RAM at which a kernel zImage is loaded by
>>>> config_distro_bootcmd.h scripts (and likely other scripts too). It's
>>>> usually
>>>> deliberately chosen to be a fair way into RAM, so that when the zImage
>>>> decompresses (to approx the start of RAM), the decompressed image doesn't
>>>> overlap the compressed image at kernel_addr_r. This avoids the kernel
>>>> decompressor having to move/copy the zImage somewhere else before copying
>>>> to
>>>> avoid any overlap during decompression.
>>>>
>>>> config_distro_bootcmd.h doesn't support FIT, so I haven't tried out
>>>> loading
>>>> FIT images. That said, if U-Boot simply jumps to the location of the
>>>> kernel
>>>> in the FIT image itself without any copying, I would expect everything to
>>>> work fine if you loaded a FIT image to kernel_addr_r. However, if the
>>>> processing of the FIT image requires the kernel to be copied out of the
>>>> FIT
>>>> image to some other location, you'd have to carefully choose that
>>>> location
>>>> so it didn't overlap anything. I'd strongly recommend avoiding any
>>>> unnecessary copies like that though, if at all possible, from the
>>>> perspective of both pointlessly wasted execution time and simplicity of
>>>> the
>>>> boot process. Perhaps storing a a kernel_noload image type inside the FIT
>>>> rather than a kernel image type might help there? Perhaps you want to
>>>> introduce a new standard variable that defines where FIT images are
>>>> loaded.
>>>
>>>
>>> I wonder what would be involved in adjusting config_distro_bootcmd to
>>> support FIT?
>>
>>
>> Well, it goes against the very idea of config_distro_bootcmd, which is to
>> provide a single standard mechanism that doesn't rely on any
>> bootloader-specific file formats etc. That mechanism is a raw zImage, a raw
>> initrd, and a plain text extlinux.cfg file to specify things like file
>> paths/names, command-line, etc.
>>
>> The boot.scr support there is legacy, and not something that should be built
>> upon going forward. So, that's not an argument for adding support for a
>> third mechanism!
>
> Do we need to adjust the mechanism? The only difference I see is that
> FIT brings the files together.

No, it's just fine as it is.

The benefit of the existing mechanism is precisely that nobody has to 
package the zImage/initrd/... together, whereas that appears to be 
precisely the purpose of FIT. The two schemes are by definition 
opposites of each-other.

>>> Would it make it simpler or harder? To me, we should be
>>> moving to using FIT with U-Boot as it is much more flexible and better
>>> designed from the ground up to provide the required features.
>>
>>
>> I disagree that FIT is a good idea, but that's a separate topic.
>>
>>> Anyway, normally FIT has a compressed kernel (e.g. with LZO), not a
>>> bzImage.
>>
>>
>> Do you mean FIT normally contains an originally uncompressed kernel (i.e. an
>> Image file in ARM land at least), but then compresses it itself as part of
>> FIT image generation? A bzImage is also a "compressed kernel".
>
> That's right, that's what I mean.
>
>>
>>> So it seems like we need an additional decompression address.
>>>
>>> I suppose we could always use malloc() instead... But perhaps a FIT
>>> load address makes sense. But then we don't really need a kernel load
>>> address do we? Shouldn't that be specified in the FIT itself?
>>
>>
>> A FIT load address does sound like it makes sense.
>>
>> If U-Boot copies the kernel image out of the FIT image to somewhere else,
>> the FIT image shouldn't specify the address to copy it to. This varies per
>> SoC, so if this address is hard-coded into FIT, it means the FIT image can't
>> be used on different SoCs (or perhaps even different boards, depending on
>> how differing RAM sizes work on various HW). This is why with
>> config_distro_bootcmd, all the addresses come from the U-Boot environment,
>> not hard-coded into a file on the disk. That way, the files are all generic
>> and can be used on various different systems that require different
>> addresses. It possibly makes sense for kernel_addr_r to be the destination
>> of that copy?
>
> Doesn't the kernel know where the kernel needs to be loaded / copied
> as part of the bzImage stuff?  From what I see at present this is
> hard-coded into the code in the kernel. So we need to put this correct
> address in the FIT, is that right?

No. The address is dynamically calculated by the kernel decompressor for 
any kernel with CONFIG_AUTO_ZRELADDR enabled. Any distro-provided kernel 
will have that option enabled, so that the kernel doesn't have to 
hard-code any addresses, and so that the same kernel image can run on 
multiple SoCs.

For kernels without CONFIG_AUTO_ZRELADDR enabled, everything should 
still work too, provided the decompression target address the kernel has 
hard-coded into it doesn't overlap stuff like the DTB or initrd.

> Are you thinking we should allow
> FIT to take an environment variable as a load address?

That would likely be required for it to be useful with 
CONFIG_AUTO_ZRELADDR, yes.

> If so, I feel it would make a lot of sense for the kernel to package
> up the FIT to avoid these issues.

The kernel community has already specifically rejected (multiple times 
IIRC) generating bootloader-specific image formats in the kernel build 
system.