Re: [Qemu-devel] How to make ELF headers/symbol sections available for multiboot?

[Anatol Pomozov]

Hi

On Tue, Aug 8, 2017 at 8:04 AM, Kevin Wolf <address@hidden> 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().