* [Qemu-trivial] [Qemu-devel][PATCH v3] hw/i386/pc: prompt not multboot or morden kernel image @ 2013-04-09 2:27 liguang 2013-04-09 4:36 ` [Qemu-trivial] [Qemu-devel] [PATCH " Jesse Larrew 0 siblings, 1 reply; 6+ messages in thread From: liguang @ 2013-04-09 2:27 UTC (permalink / raw) To: Stefan Hajnoczi, qemu-devel, qemu-trivial; +Cc: liguang 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. Signed-off-by: liguang <lig.fnst@cn.fujitsu.com> --- 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; } -- 1.7.2.5 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-trivial] [Qemu-devel] [PATCH v3] hw/i386/pc: prompt not multboot or morden kernel image 2013-04-09 2:27 [Qemu-trivial] [Qemu-devel][PATCH v3] hw/i386/pc: prompt not multboot or morden kernel image liguang @ 2013-04-09 4:36 ` Jesse Larrew 2013-04-09 5:05 ` li guang 0 siblings, 1 reply; 6+ messages in thread From: Jesse Larrew @ 2013-04-09 4:36 UTC (permalink / raw) To: liguang; +Cc: qemu-trivial, Anthony Liguori, qemu-devel 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 <lig.fnst@cn.fujitsu.com> > --- > 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). In that case, the error message above would also be misleading. 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. :) Jesse Larrew Software Engineer, KVM Team IBM Linux Technology Center Phone: (512) 973-2052 (T/L: 363-2052) jlarrew@linux.vnet.ibm.com ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-trivial] [Qemu-devel] [PATCH v3] hw/i386/pc: prompt not multboot or morden kernel image 2013-04-09 4:36 ` [Qemu-trivial] [Qemu-devel] [PATCH " Jesse Larrew @ 2013-04-09 5:05 ` li guang 2013-04-09 19:15 ` Jesse Larrew 0 siblings, 1 reply; 6+ messages in thread From: li guang @ 2013-04-09 5:05 UTC (permalink / raw) To: Jesse Larrew; +Cc: qemu-trivial, Anthony Liguori, qemu-devel 在 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 <lig.fnst@cn.fujitsu.com> > > --- > > 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") :-) > > 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 > > Jesse Larrew > Software Engineer, KVM Team > IBM Linux Technology Center > Phone: (512) 973-2052 (T/L: 363-2052) > jlarrew@linux.vnet.ibm.com > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-trivial] [Qemu-devel] [PATCH v3] hw/i386/pc: prompt not multboot or morden kernel image 2013-04-09 5:05 ` li guang @ 2013-04-09 19:15 ` Jesse Larrew 2013-04-11 2:18 ` li guang 0 siblings, 1 reply; 6+ messages in thread From: Jesse Larrew @ 2013-04-09 19:15 UTC (permalink / raw) To: li guang; +Cc: qemu-trivial, Anthony Liguori, qemu-devel 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 <lig.fnst@cn.fujitsu.com> >>> --- >>> 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) jlarrew@linux.vnet.ibm.com ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-trivial] [Qemu-devel] [PATCH v3] hw/i386/pc: prompt not multboot or morden kernel image 2013-04-09 19:15 ` Jesse Larrew @ 2013-04-11 2:18 ` li guang 2013-04-12 11:40 ` Stefan Hajnoczi 0 siblings, 1 reply; 6+ messages in thread From: li guang @ 2013-04-11 2:18 UTC (permalink / raw) To: Jesse Larrew; +Cc: qemu-trivial, Anthony Liguori, qemu-devel 在 2013-04-09二的 14:15 -0500，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 <lig.fnst@cn.fujitsu.com> > >>> --- > >>> 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) { maybe a good candidate, but, AFAIK, 0xAA55 can hardly be a magic for it's commonly used by many cases. > + fprintf(stderr, "qemu: '%s' is not an x86 kernel image.\n", > + kernel_filename); > + exit(1); > + } > } > protocol = 0; > } ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-trivial] [Qemu-devel] [PATCH v3] hw/i386/pc: prompt not multboot or morden kernel image 2013-04-11 2:18 ` li guang @ 2013-04-12 11:40 ` Stefan Hajnoczi 0 siblings, 0 replies; 6+ messages in thread From: Stefan Hajnoczi @ 2013-04-12 11:40 UTC (permalink / raw) To: li guang; +Cc: qemu-trivial, Anthony Liguori, Jesse Larrew, qemu-devel On Thu, Apr 11, 2013 at 10:18:56AM +0800, li guang wrote: > 在 2013-04-09二的 14:15 -0500，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 <lig.fnst@cn.fujitsu.com> > > >>> --- > > >>> 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) { > > maybe a good candidate, > but, AFAIK, 0xAA55 can hardly be a magic for > it's commonly used by many cases. The problem is that older kernel images don't have a real magic number to check. We cannot break compatibility by rejecting them now. Jesse's patch makes sense to me. Feel free to discuss more, I'll merge once there is consensus. Stefan ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-04-12 11:40 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-04-09 2:27 [Qemu-trivial] [Qemu-devel][PATCH v3] hw/i386/pc: prompt not multboot or morden kernel image liguang 2013-04-09 4:36 ` [Qemu-trivial] [Qemu-devel] [PATCH " Jesse Larrew 2013-04-09 5:05 ` li guang 2013-04-09 19:15 ` Jesse Larrew 2013-04-11 2:18 ` li guang 2013-04-12 11:40 ` Stefan Hajnoczi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).