Kevin Wolf <kw...@redhat.com> 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 <kw...@redhat.com>
> ---
> 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.
