[U-Boot] [PATCH 0/3] imx: bootaux elf firmware support

[Stefan Agner]

On 2017-04-04 02:22, Marek Vasut wrote:
> On 04/04/2017 02:02 AM, Stefan Agner wrote:
> [...]
>>>>> Admitedly, I didn't look at the patch, but if you want to boot ad-hoc
>>>>> cores, you can very well also boot secondary cores on the current CPU
>>>>> complex with the same command. Why not ?
>>>>
>>>> Sure, it could be done. I just feel it is not the right design.
>>>>
>>>> Auxiliary cores have usually a different view to memory, this is why I
>>>> had to add the get_host_mapping callback in the elf loader code to let
>>>> architecture dependent code translate to host addresses. SMP systems
>>>> don't need that.
>>>>
>>>> Also flush caches is not necessary on some cache coherent CPU's
>>>> (depending on how your cache coherence between I and D cache looks
>>>> like).
>>>
>>> So SMP is just a reduced special-case of this , yes ?
>>
>> Yeah, I guess you can get away with dummy callback implementation and a
>> performance hit due to cash flushes.
> 
> Or you can abstract things out ?
> 

There is one callback to arch for translation and one for cache flush,
what more can I abstract out?

>>>> Creating a new command like bootaux comes with very few overhead.
>>>
>>> The overhead is the new command, we already have many ad-hoc commands.
>>>
>>
>> Agreed, and I really wished that this got discussed when that command
>> initially got added. I brought it up back then...
>> https://lists.denx.de/pipermail/u-boot/2016-January/240323.html
>>
>> It seemed to be acceptable to just add this ad hoc command, with some
>> "random binary format" support back then...
> 
> Based on that discussion, I only see that noone opposed, but I don't see
> any agreement.
> 

Maybe I need to word my emails a bit stronger, but with that email I
actually tried to oppose:
https://lists.denx.de/pipermail/u-boot/2016-January/240989.html

>> Ok, it is not entirely
>> random, since it is the format of a binary how it ends up in
>> microcontrollers execute in place flash (stack pointer and reset vector
>> are the two first words)....
> 
> I thought this command was starting the core by loading data to RAM ,
> not flash ?
> 

Ok, maybe I am a bit unclear here:

bootaux currently supports a Cortex-M specific binary format only. The
binary format starts with a Cortex-M class vector table. The vector
tables first vector is the "reset vector".

In a regular microcontroller, that binary gets flashed on a NOR flash
which is mapped at 0x0 for the CPU. The CPU has no "boot ROM", the CPU
starts by calling the reset vector. So when NXP defined how the bootaux
command should look like, they just took that binary format which was
laying around and implemented a U-Boot command around it.

So this is the history of the binary format. And my point here is, that
the binary format supported by bootaux is _very_ Cortex-M class
specific.



>> However, making this ad hoc command now
>> generic really feels weird to me, since we would end up supporting that
>> format for A class CPUs etc... bootaux is really suited for auxiliary
>> M-class cores on ARM, as it is right now. Maybe we should have named it
>> bootm ;-)
> 
> We always try to avoid one-off hacks, so it's not weird. I still don't
> quite understand how it is a problem to abstract things out . I am not
> asking you to support CA, but to avoid CM specific implementation which
> cannot be extended to support CA or even MIPS/Nios2/etc .
> 

Again, why would you support a Cortex-M class specific file format for
MIPS/Nios2 etc...?

bootaux is M4 specific. We can through it away, and start over, but then
we should call the command differently.


>>>> This are the reasons why I feel creating a new command for a SMP boot
>>>> case makes more sense. We can still reuse functions which are very
>>>> similar by moving them into some common location, where it makes sense.
>>>>
>>>>>
>>>>> Also, I think this might come useful when booting stuff like "Altera
>>>>> Sparrow" ...
>>>>
>>>> I am not familiar with that architecture, what kind of core does it
>>>> provide which needs to be booted by U-Boot?
>>>
>>> The secondary ARM core in the SoCFPGA C-A9 complex or Nios2 core in the
>>> FPGA.
>>
>> In my thinking, the Nios2 core seems like such a remote processor well
>> suited for the bootaux command. For the secondary A9, I would create a
>> new command.
> 
> Uh, why, that does not make any sense to me. Both the random core in
> FPGA and the secondary CA9 core have almost the same bringup sequence.
> What is so different about this stuff ?
> 

A cache coherent core and a non-cache coherent core remote core
somewhere in the SoC is very much different IMHO.

Lets compare how you bring up a A class CPU, or cache coherent SMP style
CPU in general:

1. Load code into RAM
2. Some cache flushing might be necessary due to I/D-cache incoherency
3. Write entry point into some architecture specific register
4. kick off the CPU by writing another architecture specific register

M class/remote CPU

1. Load code in RAM, depending on image format translate core specific
memory map to host memory map
2. Flush D-cache
3. (Potentially set up IOMMU, not in the NXP case though)
4. Write entry point into some architecture specific register
5. kick off the CPU by writing another architecture specific register

>From what I can see, everything is different other than "Load code in
RAM" which is image specific. If the image format is complex, we
certainly should factor that out to avoid code duplication.

>> If we want to support the two with the same command, we already have a
>> first problem: How do we address them? Of course, we could add just a
>> index or something, but this would already break backward compatibility
>> of the bootaux command.
> 
> So we already accepted a command which has shit commandline semantics
> and we're stuck with it , hrm. You can always specify a default core

Add shitty binary file format to the list.

> index for those two iMX boards and it's done. Or just fix the semantics
> in their default environment, which is not awesome, but I doubt this is
> wildly used yet.

I guess my general point is, bootaux is already broken in two ways:
Command line interface does not allow for extensibility, the only format
supported right now is a binary format which is not generic.

My patchset was an attempt to improve the situation to give it at least
a decent elf format support.

Note that there is also bootelf, where the elf headers have been
introduced. Reusing that command seems impractical since it is meant to
boot on the primary CPU. My first attempt was reusing at least the load
function, but it seemed impractical due to memory map translation and
since the load function is rather short. If it helps for the acceptance
of this patchset, I still could try to make that happen.

Making the bootaux something completely new, e.g. a generic command
under cmd/*, with CPU enumeration capabilities and (pluggable?) image
support is something I am not willing to build.

--
Stefan