Re: [Qemu-devel] [PATCH qemu] Revert "elf-loader: warn about invalid endianness"

[Alexey Kardashevskiy <aik@ozlabs.ru>]

On 21/07/17 18:05, Philippe Mathieu-Daudé wrote:
> On 07/21/2017 04:30 AM, Alexey Kardashevskiy wrote:
>> On 21/07/17 16:48, Philippe Mathieu-Daudé wrote:
>>> I submitted this patch because I spent some time debugging while QEMU was
>>> failing silently using a MIPS kernel image which used to work, after
>>> realizing I was in an incorrect build_dir using qemu-system-mipsel to load
>>> a big endian image and felt stupid [1]. This dumb error can happen to other
>>> people so I added this warning here.
>>
>> Been there too. This is why we try loading images twice in pseries.
>>
>>> I was not aware of the ELF_LOAD_WRONG_ENDIAN related code, and at least the
>>> MIPS arch is not using it.
>>> As I can see in MAINTAINERS, sPAPR is "Supported" meaning "Someone is
>>> actually paid to look after this", while there is no such paid person for
>>> the MIPS part.
>>> It seems each arch had a different way to load images and hw/core/loader.c
>>> was an effort to merge common code but mostly "Supported" arch are using
>>> it.
>>
>> afaict MIPS calls load_elf() in 4 places, each checks for the return value
>> and already prints the message, how come that that message is not enough?
>> What would probably make sense here is if MIPS also printed the return code
>> from load_elf() but in any case it is easily visible from gdb.
>>
>>
>>> While your revert does fixes your sPAPR warning issue, looking at the
>>> problem roots I think the correct fix is to improve the MIPS port and
>>> eventually the less loved archs to unify the loader.c calls and avoid such
>>> problems.
>>> I don't object reverting this patch for 2.10 and improve the loader.c usage
>>> during 2.11 cycle, I only wonder if this is another corporate/hobbyist>
>>> conflict of interest with corporate crushing on hobbyist instead of
>>
>> Come on mate...
> 
> https://www.quora.com/Why-do-the-French-complain-all-the-time
> 
> :D

Good one :)


What about the reverting patch, is it going anywhere? Thanks.


> 
>>
>>
>>> helping, motivating contribution improving common code usage.
>>>
>>> Cc'ed MIPS and loader.c maintainers (both "Maintained" and not
>>> "Supported").
>>>
>>> Phil.
>>>
>>> [1] http://lists.nongnu.org/archive/html/qemu-devel/2017-06/msg05926.html


-- 
Alexey