Re: [Qemu-trivial] [Qemu-devel] [PATCH v3] hw/i386/pc: prompt not multboot or morden kernel image

[Jesse Larrew]

On 04/09/2013 12:05 AM, li guang wrote:
> 在 2013-04-08一的 23:36 -0500，Jesse Larrew写道：
>> On 04/08/2013 09:27 PM, liguang wrote:
>>> if head magic is missing or wrong unexpectedly, we'd
>>> better to prompt memssage for this.
>>> e.g.
>>> I make a mistake to boot a vmlinuz for MIPS(which
>>> I think it's for x86) like this:
>>> qemu-system-x86_64 -kernel vmlinuz -initrd demord
>>> then qemu report:
>>> "qemu: linux kernel too old to load a ram disk"
>>> that's misleading.
>>>
>>
>> Yes, that message would definitely be misleading in this case.
>>
>>> Signed-off-by: liguang <address@hidden>
>>> ---
>>>  hw/i386/pc.c |    2 ++
>>>  1 files changed, 2 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>>> index ebbf059..6b29c3f 100644
>>> --- a/hw/i386/pc.c
>>> +++ b/hw/i386/pc.c
>>> @@ -686,6 +686,8 @@ static void load_linux(void *fw_cfg,
>>>          if (load_multiboot(fw_cfg, f, kernel_filename, initrd_filename,
>>>                             kernel_cmdline, kernel_size, header)) {
>>>              return;
>>> +        } else {
>>> +            fprintf(stderr, "warn: invalid multiboot or modern kernel 
>>> image\n");
>>>          }
>>>          protocol = 0;
>>>      }
>>>
>>
>> My impression from reading through the code is that this branch can be
>> legitimately triggered by an older kernel (protocol < 0x200). 
> 
> right.
> 
>> In that case,the error message above would also be misleading.
> 
> the warning message is saying: hey, we prefer modern kernel image
> or multiboot, but you specify neither, what are you doing?
> do you want to boot an old(obsolete) kernel?
> 
> or, add a line like fprintf(stderr, "do you boot an old kernel?\n") :-)
> 

Ah, I understand now. Thanks for explaining that. :)

>>
>> I think a better solution might be to simply validate that the arch
>> specified in the ELF header matches the arch that qemu is emulating.
>>  I'll look into this more tomorrow morning. :)
> 
> that's fine
> 

Looking at Documentation/x86/boot.txt in the kernel source, it looks
like there is a separate word of header magic that all x86 kernels should
have -- even the oldest images. Adding a separate check for that magic
might be a more complete solution:

---
 hw/i386/pc.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 8d75b34..e120427 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -686,6 +686,14 @@ static void load_linux(void *fw_cfg,
         if (load_multiboot(fw_cfg, f, kernel_filename, initrd_filename,
                            kernel_cmdline, kernel_size, header)) {
             return;
+        } else {
+            /* This is the closest thing old Linux kernels have to a
+               magic number. */
+            if (lduw_p(header+0x01FE) != 0xAA55) {
+                fprintf(stderr, "qemu: '%s' is not an x86 kernel image.\n",
+                        kernel_filename);
+                exit(1);
+            }
         }
         protocol = 0;
     }
-- 
1.7.11.7

Jesse Larrew
Software Engineer, KVM Team
IBM Linux Technology Center
Phone: (512) 973-2052 (T/L: 363-2052)
address@hidden