Re: [Qemu-devel] [PATCH] multiboot: Fix module loading and setting of mmap

[Alexander Graf]

Adam Lackorzynski wrote:
> Hi,
>
> I need to following to fix multiboot booting for me.
>
> Should linuxboot.bin and multiboot.bin end up in pc-bios instead of
> pc-bios/optionrom?
>
>
> Signed-off-by: Adam Lackorzynski <address@hidden>
> ---
>  hw/pc.c                       |    4 ++--
>  pc-bios/optionrom/multiboot.S |    5 ++++-
>  2 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/hw/pc.c b/hw/pc.c
> index 7c791c4..1c8f95c 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -661,7 +661,7 @@ static int load_multiboot(void *fw_cfg,
>  
>              /* append module data at the end of last module */
>              mb_kernel_data = qemu_realloc(mb_kernel_data,
> -                                          mh_load_addr - mb_mod_end);
> +                                          mb_mod_end - mh_load_addr);
>   

Right. I wonder why it worked for me before?

>              load_image(initrd_filename,
>                         mb_kernel_data + mb_mod_start - mh_load_addr);
>  
> @@ -720,7 +720,7 @@ static int load_multiboot(void *fw_cfg,
>      fw_cfg_add_bytes(fw_cfg, FW_CFG_INITRD_DATA, mb_bootinfo_data,
>                       sizeof(bootinfo));
>  
> -    option_rom[nb_option_roms] = "multiboot.bin";
> +    option_rom[nb_option_roms] = "optionrom/multiboot.bin";
>   

This is wrong. The .bin file should be in pc-bios. It just doesn't get
copied over as part of the build process, but instead relies on the
committer to make sure the binary is updated on every source change.

>      nb_option_roms++;
>  
>      return 1; /* yes, we are multiboot */
> diff --git a/pc-bios/optionrom/multiboot.S b/pc-bios/optionrom/multiboot.S
> index be5c9fc..9131837 100644
> --- a/pc-bios/optionrom/multiboot.S
> +++ b/pc-bios/optionrom/multiboot.S
> @@ -62,6 +62,9 @@ run_multiboot:
>       add             %eax, %ebx
>       movl            %ebx, %gs:GS_GDT_DESC + 2
>  
> +     xor             %eax, %eax
> +     mov             %eax, %es
> +
>   

Ack. %es could be in any state here.

>       /* Read the bootinfo struct into RAM */
>       read_fw_blob(FW_CFG_INITRD)
>  
> @@ -71,7 +74,7 @@ run_multiboot:
>       mov             %ax, %fs
>  
>       /* ES = mmap_addr */
> -     mov             %eax, %fs:0x48
> +     mov             %fs:48, %eax
>   

Right...


Thanks for catching those!

Alex