Re: [Qemu-devel] [RFC PATCH] elf loader: exit if incompatible architecture is detected

[Alexey Kardashevskiy]

On 01/21/2014 02:11 AM, Alexander Graf wrote:
> 
> On 07.01.2014, at 05:35, Alexey Kardashevskiy <address@hidden> wrote:
> 
>> If we know for sure that the image in "-kernel" is an ELF and we know its
>> architecture and it is not supported by the current QEMU, there is no
>> point to continue trying booting this image so let's exit once we deteced
>> this fact.
>>
>> Signed-off-by: Alexey Kardashevskiy <address@hidden>
> 
> How about we just remove non-ELF loading from -kernel on -M pseries?


We are fine with that, never tried non-elf anyway, I'll cook another patch
for that. I suppose I do exit(), just one level up, in
spapr_machine:init(), correct?


> 
> 
> Alex
> 
>> ---
>>
>>
>> One of our users tried an X86 image with qemu-system-ppc64. Instead of
>> printing some reasonable message (which is possible in this case as the image
>> is ELF), QEMU (spapr.c) simply copied the image in RAM as a raw image and
>> SLOF failed to boot from it.
>>
>> The patch fixes the issue but there are still questions.
>>
>> 1. Do we need more sophisticated error checking here? Return -2 instead of 
>> exit(1)
>> and do exit(1) few levels up?
>>
>> 2. The patch does not handle x86's vmlinuz case - these images are not ELFs
>> but "Linux kernel x86 boot executable bzImage" and QEMU does not parse them.
>> As a result, SLOF crashes with the registers dump. Do we really care to 
>> handle this?
>>
>>
>> ---
>> include/hw/elf_ops.h | 13 +++++++++----
>> 1 file changed, 9 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/hw/elf_ops.h b/include/hw/elf_ops.h
>> index acc701e..6bcc61f 100644
>> --- a/include/hw/elf_ops.h
>> +++ b/include/hw/elf_ops.h
>> @@ -212,21 +212,21 @@ static int glue(load_elf, SZ)(const char *name, int fd,
>>         case EM_PPC64:
>>             if (EM_PPC64 != ehdr.e_machine)
>>                 if (EM_PPC != ehdr.e_machine)
>> -                    goto fail;
>> +                    goto arch_fail;
>>             break;
>>         case EM_X86_64:
>>             if (EM_X86_64 != ehdr.e_machine)
>>                 if (EM_386 != ehdr.e_machine)
>> -                    goto fail;
>> +                    goto arch_fail;
>>             break;
>>         case EM_MICROBLAZE:
>>             if (EM_MICROBLAZE != ehdr.e_machine)
>>                 if (EM_MICROBLAZE_OLD != ehdr.e_machine)
>> -                    goto fail;
>> +                    goto arch_fail;
>>             break;
>>         default:
>>             if (elf_machine != ehdr.e_machine)
>> -                goto fail;
>> +                goto arch_fail;
>>     }
>>
>>     if (pentry)
>> @@ -306,4 +306,9 @@ static int glue(load_elf, SZ)(const char *name, int fd,
>>     g_free(data);
>>     g_free(phdr);
>>     return -1;
>> +
>> +arch_fail:
>> +    fprintf(stderr, "qemu: could not load arch-incompatible kernel '%s'\n",
>> +            name);
>> +    exit(1);
>> }
>> -- 
>> 1.8.4.rc4
>>
> 


-- 
Alexey