[Qemu-devel] i386: don't require elf64 for multiboot kernel

Gonzo FWS posted 1 patch 5 years, 4 months ago
Test asan passed
Test checkpatch passed
Test docker-quick@centos7 passed
Test docker-mingw@fedora passed
Test docker-clang@ubuntu passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/HE1P190MB00739AD5A654DAA3FE9B4662DEAF0@HE1P190MB0073.EURP190.PROD.OUTLOOK.COM
hw/i386/multiboot.c | 5 -----
1 file changed, 5 deletions(-)
[Qemu-devel] i386: don't require elf64 for multiboot kernel
Posted by Gonzo FWS 5 years, 4 months ago
Right now IncludeOS on x86_64 must use a chainloader for multiboot support. The chainloader is an ELF32 kernel that loads the real ELF64 kernel and jumps to it. As long as the ELF has the .multiboot section and conforms to the spec, meaning _start is be a 32-bit entry, it should be fine.

By removing the extra check in multiboot.c, we can also boot ELF64 files. As can be seen here:
https://cloud.fwsnet.net/index.php/s/XrkBkC8zy7MLa9p

Signed-off-by: Alf-André Walla <fwsgonzo@hotmail.com>
---
 hw/i386/multiboot.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/hw/i386/multiboot.c b/hw/i386/multiboot.c
index 1a4344f5fc..d07ebf3361 100644
--- a/hw/i386/multiboot.c
+++ b/hw/i386/multiboot.c
@@ -194,11 +194,6 @@ int load_multiboot(FWCfgState *fw_cfg,
         int kernel_size;
         fclose(f);

-        if (((struct elf64_hdr*)header)->e_machine == EM_X86_64) {
-            error_report("Cannot load x86-64 image, give a 32bit one.");
-            exit(1);
-        }
-
         kernel_size = load_elf(kernel_filename, NULL, NULL, &elf_entry,
                                &elf_low, &elf_high, 0, I386_ELF_MACHINE,
                                0, 0);
--
2.17.1

Re: [Qemu-devel] i386: don't require elf64 for multiboot kernel
Posted by Thomas Huth 5 years, 4 months ago
On 2018-12-04 17:55, Gonzo FWS wrote:
> Right now IncludeOS on x86_64 must use a chainloader for multiboot support. The chainloader is an ELF32 kernel that loads the real ELF64 kernel and jumps to it. As long as the ELF has the .multiboot section and conforms to the spec, meaning _start is be a 32-bit entry, it should be fine.
> 
> By removing the extra check in multiboot.c, we can also boot ELF64 files. As can be seen here:
> https://cloud.fwsnet.net/index.php/s/XrkBkC8zy7MLa9p
> 
> Signed-off-by: Alf-André Walla <fwsgonzo@hotmail.com>
> ---
>  hw/i386/multiboot.c | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/hw/i386/multiboot.c b/hw/i386/multiboot.c
> index 1a4344f5fc..d07ebf3361 100644
> --- a/hw/i386/multiboot.c
> +++ b/hw/i386/multiboot.c
> @@ -194,11 +194,6 @@ int load_multiboot(FWCfgState *fw_cfg,
>          int kernel_size;
>          fclose(f);
> 
> -        if (((struct elf64_hdr*)header)->e_machine == EM_X86_64) {
> -            error_report("Cannot load x86-64 image, give a 32bit one.");
> -            exit(1);
> -        }
> -

Looks like the check has once been introduced explicitly:

https://git.qemu.org/?p=qemu.git;a=commitdiff;h=9696846600cac4bd0dfd6835

Maybe we should at least print out a warn_report() instead, so that
people who try to boot 64-bit kernel without 32-bit entry point still
get an idea why it is not working in that case?

  Thomas


PS: Please make sure to CC: the i386 maintainers when sending patches
that touch files in the hw/i386/ directory, thanks!

Re: [Qemu-devel] i386: don't require elf64 for multiboot kernel
Posted by Paolo Bonzini 5 years, 4 months ago
On 05/12/18 06:35, Thomas Huth wrote:
> On 2018-12-04 17:55, Gonzo FWS wrote:
>> Right now IncludeOS on x86_64 must use a chainloader for multiboot
>> support. The chainloader is an ELF32 kernel that loads the real
>> ELF64 kernel and jumps to it. As long as the ELF has the .multiboot
>> section and conforms to the spec, meaning _start is be a 32-bit
>> entry, it should be fine.
>> 
>> By removing the extra check in multiboot.c, we can also boot ELF64
>> files. As can be seen here: 
>> https://cloud.fwsnet.net/index.php/s/XrkBkC8zy7MLa9p
>> 
>> Signed-off-by: Alf-André Walla <fwsgonzo@hotmail.com>

I think a 64-bit multiboot specification doesn't exist, but if it
existed, it would require an EM_X86_64 ELF file to be booted already in
long mode.  I don't think it's a good idea to allow booting into an
EM_X86_64 ELF file with the processor in 32-bit mode.

Paolo

Re: [Qemu-devel] i386: don't require elf64 for multiboot kernel
Posted by Gonzo FWS 5 years, 4 months ago
When I attach a GRUB loader, it doesn't check ELFCLASS at all. It just boots the kernel as expected in protected mode. Only Qemu does the extra check that I know of, but I cannot force your opinions on this. We can still continue to use a chainloader to avoid this problem. (Or worse, build a ELFCLASS32 64-bit kernel. Just kidding, of course.)

-gonzo
________________________________
From: Paolo Bonzini <pbonzini@redhat.com>
Sent: Wednesday, December 5, 2018 1:20 PM
To: Thomas Huth; Gonzo FWS; qemu-devel@nongnu.org
Cc: Richard Henderson; Eduardo Habkost; Michael S. Tsirkin; Marcel Apfelbaum
Subject: Re: [Qemu-devel] i386: don't require elf64 for multiboot kernel

On 05/12/18 06:35, Thomas Huth wrote:
> On 2018-12-04 17:55, Gonzo FWS wrote:
>> Right now IncludeOS on x86_64 must use a chainloader for multiboot
>> support. The chainloader is an ELF32 kernel that loads the real
>> ELF64 kernel and jumps to it. As long as the ELF has the .multiboot
>> section and conforms to the spec, meaning _start is be a 32-bit
>> entry, it should be fine.
>>
>> By removing the extra check in multiboot.c, we can also boot ELF64
>> files. As can be seen here:
>> https://cloud.fwsnet.net/index.php/s/XrkBkC8zy7MLa9p
>>
>> Signed-off-by: Alf-André Walla <fwsgonzo@hotmail.com>

I think a 64-bit multiboot specification doesn't exist, but if it
existed, it would require an EM_X86_64 ELF file to be booted already in
long mode.  I don't think it's a good idea to allow booting into an
EM_X86_64 ELF file with the processor in 32-bit mode.

Paolo