Hi On Tue, Aug 8, 2017 at 8:04 AM, Kevin Wolf <kw...@redhat.com> wrote: > Am 04.08.2017 um 06:53 hat Anatol Pomozov geschrieben: >> Hi Kevin >> >> Thanks for the information. >> >> So I sounds like we do want multiboot to load all sections regardless >> of its segments info. To achieve it we need to read sections headers >> and load all section that were not loaded yet. >> >> I have a working implementation here >> https://github.com/anatol/qemu/commit/26773cf4f1f30b2d0d3fd89cce024f8e9c5603c5 >> Tested it with my multiboot OS image. The target iterates over all >> sections and prints their names/address. The output result is the same >> for QEMU and for VmWare+GRUB. >> >> Let me know if this idea looks good so I can send this patch to qemu >> maillist. > > I'm not sure if I'll find the time to review this in detail, but I'll > just give it a quick look. > > You seem to attempt to add support for 64 bit ELF binaries. The > Multiboot standard doesn't explicitly say that this is forbidden, but > everything in it expects 32 bit binaries and I don't think GRUB (as the > reference implementation for Multiboot) accepts 64 bit ELFs either. The > boot state is 32 bit protected mode in any case. So unless I'm mistaken > on GRUB's behaviour, I'd rather not support 64 bit ELFs.
Grub actually does support ELF64 loading with multiboot. Here is the GRUB code that implements it https://github.com/coreos/grub/blob/master/grub-core/loader/multiboot.c#L210 It would be great if QEMU behaved similar way. Actually I've been using qemu with ELF64 multiboot for a while and it works great. > > You shouldn't look at ELF headers at all if MULTIBOOT_AOUT_KLUDGE is > given. In this case, the kernel image is to be treated as a flat binary > and section headers can't be expected. I think your code breaks non-ELF > kernels. Ok, will revert this part back. > > As for loading the section headers, duplicating ELF parser code may be > functionally correct, but probably not the best way to implement things. > As I wrote in an earlier email, load_elf() already parses all the > information that you need. You really just need to store it somewhere, > which would probably just mean adding a new parameter to load_elf() and > the parser could then store a copy of the data there if it's non-NULL. Loading section headers is actually a small part of my change. It also: - calculates memory needed to load all sections into memory - allocates memory - loads sections This "load all sections into memory" functionality is very multiboot specific. But if you think if load_elf() is better place for it then I can look at moving it to load_elf().