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. > >> >> 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... 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 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...? 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... > >> >> >> > >> >> 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