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

[Philippe Mathieu-Daudé]

Hi Alexey,

On 07/21/2017 01:19 AM, Alexey Kardashevskiy wrote:
> This reverts c8e1158cf611 "elf-loader: warn about invalid endianness"
> as it produces a useless message every time an LE kernel image is
> passed via -kernel on a ppc64-pseries machine. The pseries machine
> already checks for ELF_LOAD_WRONG_ENDIAN and tries with big_endian=0.
>
> Signed-off-by: Alexey Kardashevskiy <address@hidden>
> ---
>   hw/core/loader.c | 1 -
>   1 file changed, 1 deletion(-)
>
> diff --git a/hw/core/loader.c b/hw/core/loader.c
> index c17ace0a2e..e5e8cbb638 100644
> --- a/hw/core/loader.c
> +++ b/hw/core/loader.c
> @@ -480,7 +480,6 @@ int load_elf_ram(const char *filename,
>       }
>   
>       if (target_data_order != e_ident[EI_DATA]) {
> -        fprintf(stderr, "%s: wrong endianness\n", filename);
>           ret = ELF_LOAD_WRONG_ENDIAN;
>           goto fail;
>       }

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