Re: [Qemu-devel] [FOR 0.12][PATCH] Fix loading of ELF multiboot kernels

[Markus Armbruster]

Kevin Wolf <address@hidden> writes:

> The multiboot implementation assumed that there is only one program header
> (which contains the entry point) and that the entry point is at the start of
> the code. This doesn't hold true generally and caused too little data to be
> loaded.

Out of curiosity: does this affect images people actually use?
Examples?

> Fix the loading code to pass the whole loaded data to the Multiboot Option 
> ROM.
>
> Signed-off-by: Kevin Wolf <address@hidden>
> ---
>  hw/loader.c |    2 --
>  hw/pc.c     |   10 ++++++----
>  2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/hw/loader.c b/hw/loader.c
> index 2d7a2c4..4c6981f 100644
> --- a/hw/loader.c
> +++ b/hw/loader.c
> @@ -718,8 +718,6 @@ int rom_copy(uint8_t *dest, target_phys_addr_t addr, 
> size_t size)
>      QTAILQ_FOREACH(rom, &roms, next) {
>          if (rom->max)
>              continue;
> -        if (rom->min > addr)
> -            continue;
>          if (rom->min + rom->romsize < addr)
>              continue;
>          if (rom->min > end)

I don't understand this hunk.

> diff --git a/hw/pc.c b/hw/pc.c
> index 8c1b7ea..fcebe3d 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -560,19 +560,21 @@ static int load_multiboot(void *fw_cfg,
>      }
>      if (!(flags & 0x00010000)) { /* MULTIBOOT_HEADER_HAS_ADDR */
>          uint64_t elf_entry;
> +        uint64_t elf_low, elf_high;
>          int kernel_size;
>          fclose(f);
> -        kernel_size = load_elf(kernel_filename, 0, &elf_entry, NULL, NULL,
> +        kernel_size = load_elf(kernel_filename, 0, &elf_entry, &elf_low, 
> &elf_high,
>                                 0, ELF_MACHINE, 0);
>          if (kernel_size < 0) {
>              fprintf(stderr, "Error while loading elf kernel\n");
>              exit(1);
>          }
> -        mh_load_addr = mh_entry_addr = elf_entry;
> -        mb_kernel_size = kernel_size;
> +        mh_load_addr = elf_low;
> +        mb_kernel_size = elf_high - elf_low;
> +        mh_entry_addr = elf_entry;
>  
>          mb_kernel_data = qemu_malloc(mb_kernel_size);
> -        if (rom_copy(mb_kernel_data, elf_entry, kernel_size) != kernel_size) 
> {
> +        if (rom_copy(mb_kernel_data, mh_load_addr, mb_kernel_size) != 
> mb_kernel_size) {
>              fprintf(stderr, "Error while fetching elf kernel from rom\n");
>              exit(1);
>          }

I get this part, and it looks good.