On 2017-04-05 08:15, Lukasz Majewski wrote: > Hi Stefan, > >> On 2017-04-04 01:23, Lukasz Majewski wrote: >> > Hi Stefan, >> > >> >> Hi Lukasz, >> >> >> >> On 2017-04-03 04:20, Lukasz Majewski wrote: >> >> > Hi Stefan, >> >> > >> >> > Thanks for your patch. Please allow me to share some ideas for >> >> > improvements. >> >> > >> >> >> From: Stefan Agner >> >> >> >> >> >> This patchset enables to boot elf binaries on secondary Cortex-M >> >> >> class cores available on i.MX 6SoloX/7Solo/7Dual. This makes >> >> >> handling and loading firmwares much more convinient since all >> >> >> information where the firmware has to be loaded to is contained >> >> >> in the elf headers. A typical usage looks like this: >> >> >> >> >> >> Colibri iMX7 # tftp ${loadaddr} firmware.elf && bootaux >> >> >> ${loadaddr} Using FEC0 device >> >> >> TFTP from server 192.168.10.1; our IP address is 192.168.10.2 >> >> >> Filename 'firmware.elf'. >> >> >> Load address: 0x80800000 >> >> >> Loading: ################################################## >> >> >> 88.3 KiB 5.4 MiB/s >> >> >> done >> >> >> Bytes transferred = 90424 (16138 hex) >> >> >> ## Starting auxiliary core at 0x1FFF8311 ... >> >> >> Colibri iMX7 # >> >> > >> >> > I can find some other platforms (not only IMX), which would >> >> > benefit from this code - the generic 'bootaux' command. >> >> > >> >> > One good example would to allow multiple binaries for different >> >> > SoC Cores (e.g. 2x Cortex-A8) to be loaded and started by u-boot. >> >> > >> >> > Hence, I'm wondering if you could make those patches usable for >> >> > other platforms as well? >> >> >> >> I don't think that this is a good idea. bootaux is meant for >> >> auxiliary cores, which often use a different architecture and are >> >> not cache coherent (hence the cache flushes). >> > >> > I do remember, that I saw similar "tiny" patch to add "just one" >> > ad-hoc command to do the same (conceptually) for TI SoC floating on >> > the u-boot mailing list. >> > >> > Please correct me if I'm wrong, but bootaux is IMX specific and does >> > work, which very likely, will be also required by other SoC vendors >> > (TI's Sitara is also equipped with Cortex-M4 and PRUSS). >> >> bootaux is currently IMX specific, and its currently supported binary >> format is M-class specific. >> >> > >> > Unification of such effort can save us all a lot of trouble in a >> > very near future. >> >> In OSS, you do not develop for the future. It gets developed when its >> here. > > I cannot agree here. When you perceive threat then you prepare for it. > >> >> > >> >> >> >> On SMP systems the main operating system normally starts the >> >> secondary core. >> > >> > I think that there are some conceptual similarities between loading >> > code to SMP core and Cortex M3. Of course some "tweaks" would be >> > needed. >> > >> >> There are conceptual similarities between a car and a truck, still, >> they are likely manufactured in a different assembly line, probably >> by a different producer. >> >> I guess in the end it really depends on where you draw the line: There >> are differences between loading code which will get executed by the >> primary code, loading code to execute explicitly on a SMP core, and >> loading code to execute on a remote processor. >> >> The first case is already well supported, and we need to keep support >> backward compatibility. >> >> The second case, IMHO, is a somewhat silly case: A SMP system usually >> gets driven by a single OS image, and that OS makes sure to initialize >> the cores (maybe with the help of firmware, such as the PSCI interface >> on ARM). There is no need for a boot loader command to execute a image >> on a secondary core explicitly... > > I do understand (and agree) with your point with SMP. > > But, I do know at least one use case when somebody would like to start > two binaries (those are bare metal programs, taking care of SoC setup, > IPC, etc). > > And maybe Linux with some FPGA based IP block already configured in > u-boot. > >> >> The third case is probably a case where we could start think about >> unification efforts. >> >> >> >> Otherwise, if you want to run them separately using U-Boot, >> >> maybe a new command such as bootsmp would be more suited. >> > >> > Hmm - I do think that it would be too much: >> > >> > - bootm for generic single core >> > - bootaux for IMX >> > - bootsmp for SMP (on IMX/TI/RK?) >> > - boot?? for ?? >> >> There is at least also bootz and bootelf. > > I will reply to the other thread regarding bootelf. > >> >> > >> > I would like to avoid adding new commands for doing conceptually the >> > same work. >> > >> > Even better, we could extend bootm to support the "multi binary" >> > case too, but IMHO it would be also correct to have "bootaux" to >> > handle generic binaries loading. >> > >> >> >> >> > >> >> >> >> >> >> Note that the bootaux command retrieved the entry point (PC) >> >> >> from the elf binary. >> >> > >> >> > Could you make this code flexible enough to handle not only elf >> >> > binaries, but also other data (e.g. FPGA bitstreams)? >> >> >> >> The code of bootaux is rather small, I don't see much value into >> >> stuff boot code for other peripherals into it. >> > >> > But I'm not asking to write support for other vendor's SoC/use >> > cases. >> > >> > I'm just wondering if you could write generic command (framework) to >> > support this use case and as an example add support for your IMX's >> > Cortex-M3/4. >> > >> >> Sure, mv arch/arm/imx-common/imx_bootaux.c cmd/, there is the >> framework :-) >> >> But this promotes the M class specific binary format to a generic >> format supported for all cores. >> >> > We would kill two birds with one stone - IMX is supported from the >> > very beginning and we do have generic code to extend it by other >> > vendors. >> > >> >> I don't know how FPGA >> >> bistream loading typically works, but I guess it is quite different >> >> from loading a firmware into RAM/SRAM and flush caches... >> > >> > FPGA quirks would be handled in arch/SoC specific code. >> > >> >> >> >> I am not against reuse and unification, I just feel that this is >> >> not really a case where we gain much. >> > >> > With generic "bootaux/bootm" command we can point other developers >> > who would like to add such booting code to the already available >> > framework. >> > >> > Also, we would prevent other "ad-hoc" commands adding to u-boot. >> > >> >> Extending bootm does not seem like a good idea. bootm is already >> rather complex, making it even more complex by supporting the >> auxiliary core case seems really not work well. >> >> Also, bootm supports lots of features which you probably never would >> use or test on a remote core (e.g. initramfs etc...) >> >> >> >> >> > >> >> > Maybe it would better to: >> >> > ------------------------- >> >> > >> >> > Embrace those binaries into FIT file (*.itb)? And allow multiple >> >> > binaries loading? I'm thinking of work similar to one already >> >> > did by Andre Przywara for SPL: >> >> > >> >> > "[PATCH v3 00/19] SPL: extend FIT loading support" posted on >> >> > 31.03.2017? >> >> > >> >> > In that way we would "open" many new use cases, and other >> >> > platforms (e.g. FPGA's, TI, etc) could benefit from it. >> >> > One solid use case is to load different Linux binaries (or/and >> >> > bare metal programs) to different SoC cores. >> >> > >> >> > The "meta data" (i.e. load address, data type, description), >> >> > could be extracted from the FIT format (the code is already in >> >> > u-boot). >> >> > >> >> > IMHO, this is very generic approach. >> >> >> >> I did some experiments with using FIT images for auxiliary core >> >> firmware, however, it did not add a lot of advantage over using >> >> elf: >> >> http://git.toradex.com/cgit/u-boot-toradex.git/commit/?h=2015.04-toradex-next&id=d1d416f272e840e8139aec911f89a70fe5523eb2 >> > >> > Unfortunately, not all use cases allow using ELF format. FIT gives >> > more flexibility: >> > >> > - ./doc/README.dfutftp -> different images loading >> > >> > - Andre's patch to load multiple binaries in SPL - [PATCH v3 00/19] >> > SPL: extend FIT loading support" >> > >> >> Are flexible, but very much U-Boot specific. And much more complex to >> support. >> >> >> >> >> Firmwares are already built and available in the elf file format. >> >> The Linux remoteproc framework, which is meant to handle this kind >> >> of cores too, supports elf firmware loading too, so supporting elf >> >> in U-Boot too is a nice alignment. Also using FIT adds an >> >> additional step when building firm wares... >> > >> > But this is all OK. >> > >> > The ELF binary would be wrapped in FIT (with e.g. "elf" property, >> > even 1 to 1 mapping). Then the 'bootaux/bootm' could parse ELF >> > (which is also generic). And then some "IMX specific" (arch/SoC) >> > code would be executed. >> > >> >> So we go from a nacked binary loaded directly to the place it has been >> linked to, to a double wrapped firmware file...? > > We would have elf binary file embedded into FIT file, these would bring > flexibility. > > FIT support is in place (u-boot/spl). > > In such a way you can use any binary in any format. > > But I must admit that we are going off-topic here..... > >> >> Not sure if user will appriciate that extra work and boot time. And >> developers the extra code. >> >> Maybe in a perfect world that just works, because you know, FIT is >> generic, and elf is generic... But that is just not how it works in >> practice. The existing FIT code is rather complex, supports lots of >> corner cases. The elf code supports different header types... Loading >> the elf sections (which use M4 specific addressing) needs address >> translation to get to suitable host addresses... > > Maybe I'm an idealist :-) > > One analogy comes to my mind between linux and u-boot. > > The proliferation of u-boot commands and linux board files. Why Linux > spend tremendous resources to remove board files and switch to device > tree? > I argue that is the right way of doing it: Do ad-hoc solutions, whatever makes sense, and once you understand the full breath of design space it is much easier to build a suitable framework. At that point, do the refactoring and build that suitable framework. If you design a framework without the understanding of the whole design space, you will end up with something which does not work well and needs lots of work-arounds etc... Of course, it is a bit different since both examples have outside facing impact (change to device tree as well as changes to U-Boot commands). -- Stefan >> >> > >> >> >> >> >> >> > >> >> >> Also all sections are translated to A7 addresses >> >> >> in order to properly load the firmware sections into the >> >> >> appropriate locations. >> >> > >> >> > This would require some tiny arch specific code. >> >> > >> >> >> >> Yes, but in my rather small patchset (164 additional lines) this is >> >> actually almost the bulk of changes :-) >> > >> > Yes, I fully understand you :-) - I do remember when I wanted to >> > add 3 lines (polarity inversion) to IMX PWM subsystem some time >> > ago ... :-) >> > >> >> Hehe, yeah that was a rather long episode. >> >> It was a bit different, though, the core framework was already there, >> it was mostly about adopting a particular driver to that framework. >> Building a new framework is a bit more involved... >> >> -- >> Stefan >> >> > Best regards, >> > Łukasz Majewski >> > >> >> >> >> -- >> >> Stefan >> >> >> >> >> Also cache flushes is taken care of, so that there is no >> >> >> manual dcache flush necessary anymore. >> >> >> >> >> >> The NXP FreeRTOS BSP already generates elf binaries which can be >> >> >> directly used with this elf binary support. >> >> >> >> >> >> The last patch adds bootaux support for Vybrid. >> >> >> >> >> >> >> >> >> Stefan Agner (3): >> >> >> imx: imx-common: move aux core image parsing to common code >> >> >> imx: imx-common: add elf firmware support >> >> >> ARM: vf610: add auxiliary core boot support >> >> >> >> >> >> arch/arm/cpu/armv7/mx6/soc.c | 30 +++++--- >> >> >> arch/arm/cpu/armv7/mx7/soc.c | 34 ++++++--- >> >> >> arch/arm/cpu/armv7/vf610/generic.c | 42 +++++++++++ >> >> >> arch/arm/imx-common/Kconfig | 2 +- >> >> >> arch/arm/imx-common/Makefile | 4 +- >> >> >> arch/arm/imx-common/imx_bootaux.c | 105 >> >> >> +++++++++++++++++++++++----- >> >> >> arch/arm/include/asm/arch-vf610/sys_proto.h | 8 +++ >> >> >> arch/arm/include/asm/imx-common/sys_proto.h | 6 ++ >> >> >> configs/colibri_vf_defconfig | 1 + 9 files >> >> >> changed, 198 insertions(+), 34 deletions(-) create mode 100644 >> >> >> arch/arm/include/asm/arch-vf610/sys_proto.h >> >> >> >> >> > >> >> > >> >> > >> >> > >> >> > Best regards, >> >> > >> >> > Lukasz Majewski >> >> > >> >> > -- >> >> > >> >> > DENX Software Engineering GmbH, Managing Director: Wolfgang >> >> > Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 >> >> > Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: >> >> > (+49)-8142-66989-80 Email: wd at denx.de >> > >> > >> > >> > >> > Best regards, >> > >> > Lukasz Majewski >> > >> > -- >> > >> > DENX Software Engineering GmbH, Managing Director: Wolfgang >> > Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, >> > Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: >> > wd at denx.de > > > > > Best regards, > > Lukasz Majewski > > -- > > DENX Software Engineering GmbH, Managing Director: Wolfgang Denk > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de