Re: [PATCH v4 0/7] elf: add better bootm support

[Oleksij Rempel <linux@rempel-privat.de>]

[-- Attachment #1.1.1: Type: text/plain, Size: 8279 bytes --]

Hi Clement,

This changes are needed to make your patch partially work on my system:
https://github.com/olerem/barebox/commit/90ad4f3cd8cbcfd361ff780bcfce1573ae73053d

I found following issues:
- the elf->list was not initiated
- my system has only 64MiB RAM and only 8MiB is available for barebox
alloc. Since you patch is reading complete file in to alloc, i was not
able to start kernel which was already copied to alloc by:
cp /mnt/tftp/kernel .
This is still not fixed, but with some sanity checks the user is
notified about it.
- lseek(fd, 0, SEEK_SET); is not working on tftp mount point. This was
the reason why elf header actually passed check, but had a trash in
elf->buf later.

On systems with limited amount of RAM, it would be better to read the
header and request RAM regions on first stand, and then directly read
them from file in to the requested regions.

And there is one more thing, by using "dryrun" function (boot(m) -d), i
would expect to be able to see reserved regions with "iomem" command.
Currently it is not the case.

Am 10.05.20 um 13:31 schrieb Clément Leger:
> Hi Oleksij,
> 
> ----- On 10 May, 2020, at 06:31, Oleksij Rempel o.rempel@pengutronix.de wrote:
> 
>> Hi Clement,
>>
>> just in case it makes a difference. I load this file over tftp (boot
>> net)
> 
> I manage to load your elf file on kvx using bootm (I do not have net boot)
> and here is what I get:
> 
> --------------------------------------------
> Board: KONIC 200 (K200)
> malloc space: 0x110306050 -> 0x200000000 (size 3.7 GiB)
> export: No such file or directory
> 
> Hit any to stop autoboot:    3
> barebox:/ bootm env/ore-linux-dpt-module 
> Entry Point:  80980000
> 
> Loading ELF 'env/ore-linux-dpt-module'
> Loading phdr to 0x0000000080980000 (2740416 bytes)
> ...
> --------------------------------------------
> 
> So one segment is loaded which is correct according to readelf.
> elf file is correct and can be loaded using bootm.
> 
> Could you try loading it using bootm directly ?
> This will confirm if it's related to net boot or not (although I don't
> know what are the underlying differences).
> 
> Thanks,
> 
> Clément
> 
>>
>> On Sat, May 09, 2020 at 09:24:55PM +0200, Clément Leger wrote:
>>> Hi Oleksij,
>>>
>>> ----- On 9 May, 2020, at 18:51, Oleksij Rempel o.rempel@pengutronix.de wrote:
>>>
>>>> On Sat, May 09, 2020 at 04:51:42PM +0200, Oleksij Rempel wrote:
>>>>> Hi Clement,
>>>>>
>>>>> suddenly it is still exploding. I'll try to investigate the reason.
>>>>
>>>> common/elf.c:
>>>>  load_elf_image_phdr()
>>>>    elf_hdr_e_phnum() <-- returns 0
>>>>    for (i = 0; i < elf_hdr_e_phnum(elf, buf) ; ++i) {
>>>>      ... so we will never get here.
>>>>
>>>> Probably we should add a sanity check here and be more verbose in this
>>>> case.
>>>
>>> There is something odd because your elf is correct and has 2 program
>>> headers. So this should not be a problem (but a sanity check makes
>>> sense !).
>>>
>>> I suspect that the elf size computation is wrong with your elf file
>>> and leads to incompletely loaded elf file but I tried to compute
>>> it using the sequence of code used in barebox and everything seems ok.
>>> Maybe there is something with the endianess/elfclass going wrong.
>>> I tried again on kvx to be sure I did not made a mistake but it works
>>> and the elf entry address is correctly used.
>>> I'll try to add handling for different endianess and load your elf
>>> file on kvx to debug that.
>>>
>>>>
>>>> here is my elf image:
>>>> https://github.com/olerem/barebox/blob/new-elf/ore-linux-dpt-module
>>>
>>> I succeeded in building Distrokit images but I can't get it under barebox.
>>> There is only 4M oof RAM and each time I tries to modify it, it won't boot
>>> anymore.
>>> I tried integrating the kernel (and your eore-linux-dpt-module elf) in
>>> the environment but it also won't boot. Do you have any idea on how to
>>> do it ?
>>> Even if the kernel won't start under qemu , at least I'll be able to
>>> understand what is wrong.
>>>
>>> Thanks for testing,
>>>
>>> Clément
>>>
>>>>
>>>>> On Fri, May 08, 2020 at 07:04:04PM +0200, Clement Leger wrote:
>>>>>> Currently, when booting an elf file using "bootm /dev/mtdx", bootm will
>>>>>> simply pass the file to the bootm and the read done on it will read the
>>>>>> entire flash partition. This series starts by some cleanup and then add an
>>>>>> elf_open function to load the elf file size only based on the elf header.
>>>>>> A special handling for the elf file is also added in bootm data to allow
>>>>>> using directly the elf file structure. Finally the mips bootm is modified
>>>>>> to use bootm_load_os directly instead of manual elf loading.
>>>>>>
>>>>>> Compilation for both mips and arm has been tested but run on qemu-malta was not
>>>>>> possible. Changing the MALLOC_SIZE to allow loading a kernel always lead to a
>>>>>> non-bootable system. Changes have been tested on kvx architecture for which
>>>>>> bootm support has been added and will be submitted.
>>>>>>
>>>>>> Changes v3 -> v4
>>>>>>  - Fix init of elf entry address to be used by bootm_load_elf
>>>>>>
>>>>>> Changes v2 -> v3
>>>>>>  - Integrate elf loading in bootm_load_os
>>>>>>  - Add patch to remove now unused elf_load_image/elf_release_image
>>>>>>  - Use malloc instead of xmalloc and check return value
>>>>>>
>>>>>> Changes v1 -> v2
>>>>>>  - Add BOOTM_ELF config to select elf support and add checks in code
>>>>>>  - Add an elf_get_mem_size function to avoid computing elf size in bootm.c
>>>>>>  - Use xmalloc and read_full in elf_open instead of xzalloc/read
>>>>>>  - Fix data->elf NULL reset
>>>>>>  - Remove elf struct entirely from mips bootm code
>>>>>>
>>>>>> Clement Leger (7):
>>>>>>   common: elf: add computation of elf boundaries
>>>>>>   common: elf: fix warning on 32 bits architectures
>>>>>>   common: elf: split init to be reused from other function
>>>>>>   common: elf: add elf_open, elf_close and elf_load
>>>>>>   common: bootm: add support for elf file loading
>>>>>>   mips: lib: bootm: use bootm elf loading capabilities
>>>>>>   common: elf: remove elf_load_image/elf_release_image
>>>>>>
>>>>>>  arch/mips/lib/bootm.c |  31 +++++-------
>>>>>>  common/Kconfig        |   8 +++
>>>>>>  common/bootm.c        |  33 +++++++++++++
>>>>>>  common/elf.c          | 111 +++++++++++++++++++++++++++++++++++-------
>>>>>>  include/bootm.h       |   3 ++
>>>>>>  include/elf.h         |  16 +++++-
>>>>>>  6 files changed, 163 insertions(+), 39 deletions(-)
>>>>>>
>>>>>> --
>>>>>> 2.17.1
>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> barebox mailing list
>>>>>> barebox@lists.infradead.org
>>>>>> http://lists.infradead.org/mailman/listinfo/barebox
>>>>>>
>>>>>
>>>>> --
>>>>> Pengutronix e.K.                           |                             |
>>>>> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
>>>>> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
>>>>> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
>>>>
>>>>
>>>>
>>>> --
>>>> Pengutronix e.K.                           |                             |
>>>> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
>>>> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
>>>> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
>>>
>>> _______________________________________________
>>> barebox mailing list
>>> barebox@lists.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/barebox
>>
>> --
>> Pengutronix e.K.                           |                             |
>> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
>> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
>> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
> 


-- 
Regards,
Oleksij


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 149 bytes --]

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox