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

Stephen Warren
2015-05-20T01:44:20Z

On 05/19/2015 05:27 PM, Simon Glass wrote:
> +Tom Rini in case he is interested...
>
> Hi Stephen,
>
> On 19 May 2015 at 15:36, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> 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.
>
> Really? Well zImage is a packaged kernel isn't it? That part of it is
> definitely opposite. But other than that I don't see why a group of
> separate files is so different from packaging them together.

The zImage packaging is a kernel-internal function implemented (both
compression and decompression) fully as part of the kernel build
process. Code/... that consumes the kernel doesn't even know that such
packaging happens; you can just run make in the kernel tree, and you get
a zImage.

However, if you want a FIT or uImage, you need to run additional tools
beyond a plain "make" in the kernel tree.

>>>>> 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.
>
> Form the help:
>
> "ZRELADDR is the physical address where the decompressed kernel image
> will be placed. If AUTO_ZRELADDR is selected, the address will be
> determined at run-time by masking the current IP with 0xf8000000. This
> assumes the zImage being placed in the first 128MB from start of
> memory."
>
> It is dynamically calculated based on the kernel being loaded to a
> reasonable place. But presumably the kernel itself actually ends up at
> a fixed TEXTBASE. Unless you are using ASLR iwc you have ELF
> relocations anyway. Or do I have this wrong?

The location the kernel is decompressed to is dynamic; it isn't fixed to
TEXTBASE. AUTO_ZRELADDR calculates this location. IIRC (it's been a
while since I looked at how this is implemented) the kernel code is
either position-independent (early boot code prior to MMU setup) or runs
in a virtual address space with the MMU configuration hiding the dynamic
nature of the run-time-generated physical address.

> So what I am saying is that the kernel could make a FIT with TEXTBASE
> set correctly.

I don't believe so. Well, for a single platform or set of similar
platforms, yes it could. For a completely generic multi-platform kernel
it could not.

>>> 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.
>
> Will await your thoughts on the above before commenting further.
>
>>
>>> 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.
>
> Doesn't' the kernel still support 'make uImage'? Anyway, if the
> information is available we don't need the kernel to support it
> itself. It would just be easier...

That target exists *but* since it runs "mkimage -T kernel" (rather than
e.g. -T kernel_noload), you must specify a fixed address for the uImage
to be unpacked into. This makes the build target useless for
multi-platform kernels, since the set of legal "load" addresses differs
between platforms. Multi-plaform kernels are pretty much all anyone
cares about these days at least for ARM Linux. IIRC there were some
discussions and patches proposed to solve some of this, but they were
rejected because the kernel maintainers didn't want the kernel build
process to support external bootloader-specific packaging steps. I
believe it was even proposed to remove the build target, but that
doesn't seem to have happened.