diff mbox

[1/2] multiboot: Support arbitrary number of modules.

Message ID 1261833226-26896-1-git-send-email-adam@os.inf.tu-dresden.de
State New
Headers show

Commit Message

Adam Lackorzynski Dec. 26, 2009, 1:13 p.m. UTC
Signed-off-by: Adam Lackorzynski <adam@os.inf.tu-dresden.de>
---
 hw/pc.c |  268 +++++++++++++++++++++++++++++++++++++++------------------------
 1 files changed, 167 insertions(+), 101 deletions(-)

Comments

Anthony Liguori Jan. 8, 2010, 4:35 p.m. UTC | #1
On 12/26/2009 07:13 AM, Adam Lackorzynski wrote:
> Signed-off-by: Adam Lackorzynski<adam@os.inf.tu-dresden.de>
>    

Applied.  Thanks.

Regards,

Anthony Liguori
> ---
>   hw/pc.c |  268 +++++++++++++++++++++++++++++++++++++++------------------------
>   1 files changed, 167 insertions(+), 101 deletions(-)
>
> diff --git a/hw/pc.c b/hw/pc.c
> index 83f8dd0..2dca777 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -51,6 +51,12 @@
>   /* Show multiboot debug output */
>   //#define DEBUG_MULTIBOOT
>
> +#ifdef DEBUG_MULTIBOOT
> +#define mb_debug(a...) fprintf(stderr, ## a)
> +#else
> +#define mb_debug(a...)
> +#endif
> +
>   #define BIOS_FILENAME "bios.bin"
>
>   #define PC_MAX_BIOS_SIZE (4 * 1024 * 1024)
> @@ -512,6 +518,85 @@ static long get_file_size(FILE *f)
>   #error multiboot struct needs to fit in 16 bit real mode
>   #endif
>
> +enum {
> +    /* Multiboot info */
> +    MBI_FLAGS       = 0,
> +    MBI_MEM_LOWER   = 4,
> +    MBI_MEM_UPPER   = 8,
> +    MBI_BOOT_DEVICE = 12,
> +    MBI_CMDLINE     = 16,
> +    MBI_MODS_COUNT  = 20,
> +    MBI_MODS_ADDR   = 24,
> +    MBI_MMAP_ADDR   = 48,
> +
> +    MBI_SIZE        = 88,
> +
> +    /* Multiboot modules */
> +    MB_MOD_START    = 0,
> +    MB_MOD_END      = 4,
> +    MB_MOD_CMDLINE  = 8,
> +
> +    MB_MOD_SIZE     = 16,
> +
> +    /* Region offsets */
> +    ADDR_E820_MAP = MULTIBOOT_STRUCT_ADDR + 0,
> +    ADDR_MBI      = ADDR_E820_MAP + 0x500,
> +
> +    /* Multiboot flags */
> +    MULTIBOOT_FLAGS_MEMORY      = 1<<  0,
> +    MULTIBOOT_FLAGS_BOOT_DEVICE = 1<<  1,
> +    MULTIBOOT_FLAGS_CMDLINE     = 1<<  2,
> +    MULTIBOOT_FLAGS_MODULES     = 1<<  3,
> +    MULTIBOOT_FLAGS_MMAP        = 1<<  6,
> +};
> +
> +typedef struct {
> +    /* buffer holding kernel, cmdlines and mb_infos */
> +    void *mb_buf;
> +    /* address in target */
> +    target_phys_addr_t mb_buf_phys;
> +    /* size of mb_buf in bytes */
> +    unsigned mb_buf_size;
> +    /* offset of mb-info's in bytes */
> +    target_phys_addr_t offset_mbinfo;
> +    /* offset in buffer for cmdlines in bytes */
> +    target_phys_addr_t offset_cmdlines;
> +    /* offset of modules in bytes */
> +    target_phys_addr_t offset_mods;
> +    /* available slots for mb modules infos */
> +    int mb_mods_avail;
> +    /* currently used slots of mb modules */
> +    int mb_mods_count;
> +} MultibootState;
> +
> +static uint32_t mb_add_cmdline(MultibootState *s, const char *cmdline)
> +{
> +    int len = strlen(cmdline) + 1;
> +    target_phys_addr_t p = s->offset_cmdlines;
> +
> +    pstrcpy((char *)s->mb_buf + p, len, cmdline);
> +    s->offset_cmdlines += len;
> +    return s->mb_buf_phys + p;
> +}
> +
> +static void mb_add_mod(MultibootState *s,
> +                       target_phys_addr_t start, target_phys_addr_t end,
> +                       target_phys_addr_t cmdline_phys)
> +{
> +    char *p;
> +    assert(s->mb_mods_count<  s->mb_mods_avail);
> +
> +    p = (char *)s->mb_buf + s->offset_mbinfo + MB_MOD_SIZE * s->mb_mods_count;
> +
> +    stl_p(p + MB_MOD_START,   start);
> +    stl_p(p + MB_MOD_END,     end);
> +    stl_p(p + MB_MOD_CMDLINE, cmdline_phys);
> +
> +    mb_debug("mod%02d: %08x - %08x\n", s->mb_mods_count, start, end);
> +
> +    s->mb_mods_count++;
> +}
> +
>   static int load_multiboot(void *fw_cfg,
>                             FILE *f,
>                             const char *kernel_filename,
> @@ -524,12 +609,8 @@ static int load_multiboot(void *fw_cfg,
>       uint32_t mh_entry_addr;
>       uint32_t mh_load_addr;
>       uint32_t mb_kernel_size;
> -    uint32_t mmap_addr = MULTIBOOT_STRUCT_ADDR;
> -    uint32_t mb_bootinfo = MULTIBOOT_STRUCT_ADDR + 0x500;
> -    uint32_t mb_mod_end;
> -    uint8_t bootinfo[0x500];
> -    uint32_t cmdline = 0x200;
> -    uint8_t *mb_kernel_data;
> +    MultibootState mbs;
> +    uint8_t bootinfo[MBI_SIZE];
>       uint8_t *mb_bootinfo_data;
>
>       /* Ok, let's see if it is a multiboot image.
> @@ -550,10 +631,9 @@ static int load_multiboot(void *fw_cfg,
>       if (!is_multiboot)
>           return 0; /* no multiboot */
>
> -#ifdef DEBUG_MULTIBOOT
> -    fprintf(stderr, "qemu: I believe we found a multiboot image!\n");
> -#endif
> +    mb_debug("qemu: I believe we found a multiboot image!\n");
>       memset(bootinfo, 0, sizeof(bootinfo));
> +    memset(&mbs, 0, sizeof(mbs));
>
>       if (flags&  0x00000004) { /* MULTIBOOT_HEADER_HAS_VBE */
>           fprintf(stderr, "qemu: multiboot knows VBE. we don't.\n");
> @@ -573,24 +653,18 @@ static int load_multiboot(void *fw_cfg,
>           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, mh_load_addr, mb_kernel_size) != mb_kernel_size) {
> +        mbs.mb_buf = qemu_malloc(mb_kernel_size);
> +        if (rom_copy(mbs.mb_buf, mh_load_addr, mb_kernel_size) != mb_kernel_size) {
>               fprintf(stderr, "Error while fetching elf kernel from rom\n");
>               exit(1);
>           }
>
> -#ifdef DEBUG_MULTIBOOT
> -        fprintf(stderr, "qemu: loading multiboot-elf kernel (%#x bytes) with entry %#zx\n",
> -                mb_kernel_size, (size_t)mh_entry_addr);
> -#endif
> +        mb_debug("qemu: loading multiboot-elf kernel (%#x bytes) with entry %#zx\n",
> +                  mb_kernel_size, (size_t)mh_entry_addr);
>       } else {
>           /* Valid if mh_flags sets MULTIBOOT_HEADER_HAS_ADDR. */
>           uint32_t mh_header_addr = ldl_p(header+i+12);
>           mh_load_addr = ldl_p(header+i+16);
> -#ifdef DEBUG_MULTIBOOT
> -        uint32_t mh_load_end_addr = ldl_p(header+i+20);
> -        uint32_t mh_bss_end_addr = ldl_p(header+i+24);
> -#endif
>           uint32_t mb_kernel_text_offset = i - (mh_header_addr - mh_load_addr);
>
>           mh_entry_addr = ldl_p(header+i+28);
> @@ -602,118 +676,110 @@ static int load_multiboot(void *fw_cfg,
>           uint32_t mh_height = ldl_p(header+i+40);
>           uint32_t mh_depth = ldl_p(header+i+44); */
>
> -#ifdef DEBUG_MULTIBOOT
> -        fprintf(stderr, "multiboot: mh_header_addr = %#x\n", mh_header_addr);
> -        fprintf(stderr, "multiboot: mh_load_addr = %#x\n", mh_load_addr);
> -        fprintf(stderr, "multiboot: mh_load_end_addr = %#x\n", mh_load_end_addr);
> -        fprintf(stderr, "multiboot: mh_bss_end_addr = %#x\n", mh_bss_end_addr);
> -        fprintf(stderr, "qemu: loading multiboot kernel (%#x bytes) at %#x\n",
> -                mb_kernel_size, mh_load_addr);
> -#endif
> +        mb_debug("multiboot: mh_header_addr = %#x\n", mh_header_addr);
> +        mb_debug("multiboot: mh_load_addr = %#x\n", mh_load_addr);
> +        mb_debug("multiboot: mh_load_end_addr = %#x\n", ldl_p(header+i+20));
> +        mb_debug("multiboot: mh_bss_end_addr = %#x\n", ldl_p(header+i+24));
> +        mb_debug("qemu: loading multiboot kernel (%#x bytes) at %#x\n",
> +                 mb_kernel_size, mh_load_addr);
>
> -        mb_kernel_data = qemu_malloc(mb_kernel_size);
> +        mbs.mb_buf = qemu_malloc(mb_kernel_size);
>           fseek(f, mb_kernel_text_offset, SEEK_SET);
> -        if (fread(mb_kernel_data, 1, mb_kernel_size, f) != mb_kernel_size) {
> +        if (fread(mbs.mb_buf, 1, mb_kernel_size, f) != mb_kernel_size) {
>               fprintf(stderr, "fread() failed\n");
>               exit(1);
>           }
>           fclose(f);
>       }
>
> -    /* blob size is only the kernel for now */
> -    mb_mod_end = mh_load_addr + mb_kernel_size;
> +    mbs.mb_buf_phys = mh_load_addr;
> +
> +    mbs.mb_buf_size = TARGET_PAGE_ALIGN(mb_kernel_size);
> +    mbs.offset_mbinfo = mbs.mb_buf_size;
> +
> +    /* Calculate space for cmdlines and mb_mods */
> +    mbs.mb_buf_size += strlen(kernel_filename) + 1;
> +    mbs.mb_buf_size += strlen(kernel_cmdline) + 1;
> +    if (initrd_filename) {
> +        const char *r = initrd_filename;
> +        mbs.mb_buf_size += strlen(r) + 1;
> +        mbs.mb_mods_avail = 1;
> +        while ((r = strchr(r, ','))) {
> +           mbs.mb_mods_avail++;
> +           r++;
> +        }
> +        mbs.mb_buf_size += MB_MOD_SIZE * mbs.mb_mods_avail;
> +    }
> +
> +    mbs.mb_buf_size = TARGET_PAGE_ALIGN(mbs.mb_buf_size);
> +
> +    /* enlarge mb_buf to hold cmdlines and mb-info structs */
> +    mbs.mb_buf          = qemu_realloc(mbs.mb_buf, mbs.mb_buf_size);
> +    mbs.offset_cmdlines = mbs.offset_mbinfo + mbs.mb_mods_avail * MB_MOD_SIZE;
>
> -    /* load modules */
> -    stl_p(bootinfo + 20, 0x0); /* mods_count */
>       if (initrd_filename) {
> -        uint32_t mb_mod_info = 0x100;
> -        uint32_t mb_mod_cmdline = 0x300;
> -        uint32_t mb_mod_start = mh_load_addr;
> -        uint32_t mb_mod_length = mb_kernel_size;
>           char *next_initrd;
> -        char *next_space;
> -        int mb_mod_count = 0;
> +
> +        mbs.offset_mods = mbs.mb_buf_size;
>
>           do {
> -            if (mb_mod_info + 16>  mb_mod_cmdline) {
> -                printf("WARNING: Too many modules loaded, aborting.\n");
> -                break;
> -            }
> +            char *next_space;
> +            uint32_t mb_mod_length;
> +            uint32_t offs = mbs.mb_buf_size;
>
>               next_initrd = strchr(initrd_filename, ',');
>               if (next_initrd)
>                   *next_initrd = '\0';
>               /* if a space comes after the module filename, treat everything
>                  after that as parameters */
> -            pstrcpy((char*)bootinfo + mb_mod_cmdline,
> -                    sizeof(bootinfo) - mb_mod_cmdline,
> -                    initrd_filename);
> -            stl_p(bootinfo + mb_mod_info + 8, mb_bootinfo + mb_mod_cmdline); /* string */
> -            mb_mod_cmdline += strlen(initrd_filename) + 1;
> -            if (mb_mod_cmdline>  sizeof(bootinfo)) {
> -                mb_mod_cmdline = sizeof(bootinfo);
> -                printf("WARNING: Too many module cmdlines loaded, aborting.\n");
> -                break;
> -            }
> +            target_phys_addr_t c = mb_add_cmdline(&mbs, initrd_filename);
>               if ((next_space = strchr(initrd_filename, ' ')))
>                   *next_space = '\0';
> -#ifdef DEBUG_MULTIBOOT
> -            printf("multiboot loading module: %s\n", initrd_filename);
> -#endif
> -            mb_mod_start = (mb_mod_start + mb_mod_length + (TARGET_PAGE_SIZE - 1))
> -&  (TARGET_PAGE_MASK);
> +            mb_debug("multiboot loading module: %s\n", initrd_filename);
>               mb_mod_length = get_image_size(initrd_filename);
>               if (mb_mod_length<  0) {
>                   fprintf(stderr, "failed to get %s image size\n", initrd_filename);
>                   exit(1);
>               }
> -            mb_mod_end = mb_mod_start + mb_mod_length;
> -            mb_mod_count++;
> -
> -            /* append module data at the end of last module */
> -            mb_kernel_data = qemu_realloc(mb_kernel_data,
> -                                          mb_mod_end - mh_load_addr);
> -            load_image(initrd_filename,
> -                       mb_kernel_data + mb_mod_start - mh_load_addr);
> -
> -            stl_p(bootinfo + mb_mod_info + 0, mb_mod_start);
> -            stl_p(bootinfo + mb_mod_info + 4, mb_mod_start + mb_mod_length);
> -            stl_p(bootinfo + mb_mod_info + 12, 0x0); /* reserved */
> -#ifdef DEBUG_MULTIBOOT
> -            printf("mod_start: %#x\nmod_end:   %#x\n", mb_mod_start,
> -                   mb_mod_start + mb_mod_length);
> -#endif
> +
> +            mbs.mb_buf_size = TARGET_PAGE_ALIGN(mb_mod_length + mbs.mb_buf_size);
> +            mbs.mb_buf = qemu_realloc(mbs.mb_buf, mbs.mb_buf_size);
> +
> +            load_image(initrd_filename, (unsigned char *)mbs.mb_buf + offs);
> +            mb_add_mod(&mbs, mbs.mb_buf_phys + offs,
> +                       mbs.mb_buf_phys + offs + mb_mod_length, c);
> +
> +            mb_debug("mod_start: %p\nmod_end:   %p\n  cmdline: %#x\n",
> +                     (char *)mbs.mb_buf + offs,
> +                     (char *)mbs.mb_buf + offs + mb_mod_length, c);
>               initrd_filename = next_initrd+1;
> -            mb_mod_info += 16;
>           } while (next_initrd);
> -        stl_p(bootinfo + 20, mb_mod_count); /* mods_count */
> -        stl_p(bootinfo + 24, mb_bootinfo + 0x100); /* mods_addr */
>       }
>
>       /* Commandline support */
> -    stl_p(bootinfo + 16, mb_bootinfo + cmdline);
> -    snprintf((char*)bootinfo + cmdline, 0x100, "%s %s",
> +    char kcmdline[strlen(kernel_filename) + strlen(kernel_cmdline) + 2];
> +    snprintf(kcmdline, sizeof(kcmdline), "%s %s",
>                kernel_filename, kernel_cmdline);
> +    stl_p(bootinfo + MBI_CMDLINE, mb_add_cmdline(&mbs, kcmdline));
>
> -    /* the kernel is where we want it to be now */
> -#define MULTIBOOT_FLAGS_MEMORY (1<<  0)
> -#define MULTIBOOT_FLAGS_BOOT_DEVICE (1<<  1)
> -#define MULTIBOOT_FLAGS_CMDLINE (1<<  2)
> -#define MULTIBOOT_FLAGS_MODULES (1<<  3)
> -#define MULTIBOOT_FLAGS_MMAP (1<<  6)
> -    stl_p(bootinfo, MULTIBOOT_FLAGS_MEMORY
> -                  | MULTIBOOT_FLAGS_BOOT_DEVICE
> -                  | MULTIBOOT_FLAGS_CMDLINE
> -                  | MULTIBOOT_FLAGS_MODULES
> -                  | MULTIBOOT_FLAGS_MMAP);
> -    stl_p(bootinfo + 4, 640); /* mem_lower */
> -    stl_p(bootinfo + 8, ram_size / 1024); /* mem_upper */
> -    stl_p(bootinfo + 12, 0x8001ffff); /* XXX: use the -boot switch? */
> -    stl_p(bootinfo + 48, mmap_addr); /* mmap_addr */
> +    stl_p(bootinfo + MBI_MODS_ADDR,  mbs.mb_buf_phys + mbs.offset_mbinfo);
> +    stl_p(bootinfo + MBI_MODS_COUNT, mbs.mb_mods_count); /* mods_count */
>
> -#ifdef DEBUG_MULTIBOOT
> -    fprintf(stderr, "multiboot: mh_entry_addr = %#x\n", mh_entry_addr);
> -#endif
> +    /* the kernel is where we want it to be now */
> +    stl_p(bootinfo + MBI_FLAGS, MULTIBOOT_FLAGS_MEMORY
> +                                | MULTIBOOT_FLAGS_BOOT_DEVICE
> +                                | MULTIBOOT_FLAGS_CMDLINE
> +                                | MULTIBOOT_FLAGS_MODULES
> +                                | MULTIBOOT_FLAGS_MMAP);
> +    stl_p(bootinfo + MBI_MEM_LOWER,   640);
> +    stl_p(bootinfo + MBI_MEM_UPPER,   ram_size / 1024);
> +    stl_p(bootinfo + MBI_BOOT_DEVICE, 0x8001ffff); /* XXX: use the -boot switch? */
> +    stl_p(bootinfo + MBI_MMAP_ADDR,   ADDR_E820_MAP);
> +
> +    mb_debug("multiboot: mh_entry_addr = %#x\n", mh_entry_addr);
> +    mb_debug("           mb_buf_phys   = %x\n", mbs.mb_buf_phys);
> +    mb_debug("           mod_start     = %x\n", mbs.mb_buf_phys + mbs.offset_mods);
> +    mb_debug("           mb_mods_count = %d\n", mbs.mb_mods_count);
>
>       /* save bootinfo off the stack */
>       mb_bootinfo_data = qemu_malloc(sizeof(bootinfo));
> @@ -722,11 +788,11 @@ static int load_multiboot(void *fw_cfg,
>       /* Pass variables to option rom */
>       fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ENTRY, mh_entry_addr);
>       fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ADDR, mh_load_addr);
> -    fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_SIZE, mb_mod_end - mh_load_addr);
> -    fw_cfg_add_bytes(fw_cfg, FW_CFG_KERNEL_DATA, mb_kernel_data,
> -                     mb_mod_end - mh_load_addr);
> +    fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_SIZE, mbs.mb_buf_size);
> +    fw_cfg_add_bytes(fw_cfg, FW_CFG_KERNEL_DATA,
> +                     mbs.mb_buf, mbs.mb_buf_size);
>
> -    fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_ADDR, mb_bootinfo);
> +    fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_ADDR, ADDR_MBI);
>       fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_SIZE, sizeof(bootinfo));
>       fw_cfg_add_bytes(fw_cfg, FW_CFG_INITRD_DATA, mb_bootinfo_data,
>                        sizeof(bootinfo));
>
diff mbox

Patch

diff --git a/hw/pc.c b/hw/pc.c
index 83f8dd0..2dca777 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -51,6 +51,12 @@ 
 /* Show multiboot debug output */
 //#define DEBUG_MULTIBOOT
 
+#ifdef DEBUG_MULTIBOOT
+#define mb_debug(a...) fprintf(stderr, ## a)
+#else
+#define mb_debug(a...)
+#endif
+
 #define BIOS_FILENAME "bios.bin"
 
 #define PC_MAX_BIOS_SIZE (4 * 1024 * 1024)
@@ -512,6 +518,85 @@  static long get_file_size(FILE *f)
 #error multiboot struct needs to fit in 16 bit real mode
 #endif
 
+enum {
+    /* Multiboot info */
+    MBI_FLAGS       = 0,
+    MBI_MEM_LOWER   = 4,
+    MBI_MEM_UPPER   = 8,
+    MBI_BOOT_DEVICE = 12,
+    MBI_CMDLINE     = 16,
+    MBI_MODS_COUNT  = 20,
+    MBI_MODS_ADDR   = 24,
+    MBI_MMAP_ADDR   = 48,
+
+    MBI_SIZE        = 88,
+
+    /* Multiboot modules */
+    MB_MOD_START    = 0,
+    MB_MOD_END      = 4,
+    MB_MOD_CMDLINE  = 8,
+
+    MB_MOD_SIZE     = 16,
+
+    /* Region offsets */
+    ADDR_E820_MAP = MULTIBOOT_STRUCT_ADDR + 0,
+    ADDR_MBI      = ADDR_E820_MAP + 0x500,
+
+    /* Multiboot flags */
+    MULTIBOOT_FLAGS_MEMORY      = 1 << 0,
+    MULTIBOOT_FLAGS_BOOT_DEVICE = 1 << 1,
+    MULTIBOOT_FLAGS_CMDLINE     = 1 << 2,
+    MULTIBOOT_FLAGS_MODULES     = 1 << 3,
+    MULTIBOOT_FLAGS_MMAP        = 1 << 6,
+};
+
+typedef struct {
+    /* buffer holding kernel, cmdlines and mb_infos */
+    void *mb_buf;
+    /* address in target */
+    target_phys_addr_t mb_buf_phys;
+    /* size of mb_buf in bytes */
+    unsigned mb_buf_size;
+    /* offset of mb-info's in bytes */
+    target_phys_addr_t offset_mbinfo;
+    /* offset in buffer for cmdlines in bytes */
+    target_phys_addr_t offset_cmdlines;
+    /* offset of modules in bytes */
+    target_phys_addr_t offset_mods;
+    /* available slots for mb modules infos */
+    int mb_mods_avail;
+    /* currently used slots of mb modules */
+    int mb_mods_count;
+} MultibootState;
+
+static uint32_t mb_add_cmdline(MultibootState *s, const char *cmdline)
+{
+    int len = strlen(cmdline) + 1;
+    target_phys_addr_t p = s->offset_cmdlines;
+
+    pstrcpy((char *)s->mb_buf + p, len, cmdline);
+    s->offset_cmdlines += len;
+    return s->mb_buf_phys + p;
+}
+
+static void mb_add_mod(MultibootState *s,
+                       target_phys_addr_t start, target_phys_addr_t end,
+                       target_phys_addr_t cmdline_phys)
+{
+    char *p;
+    assert(s->mb_mods_count < s->mb_mods_avail);
+
+    p = (char *)s->mb_buf + s->offset_mbinfo + MB_MOD_SIZE * s->mb_mods_count;
+
+    stl_p(p + MB_MOD_START,   start);
+    stl_p(p + MB_MOD_END,     end);
+    stl_p(p + MB_MOD_CMDLINE, cmdline_phys);
+
+    mb_debug("mod%02d: %08x - %08x\n", s->mb_mods_count, start, end);
+
+    s->mb_mods_count++;
+}
+
 static int load_multiboot(void *fw_cfg,
                           FILE *f,
                           const char *kernel_filename,
@@ -524,12 +609,8 @@  static int load_multiboot(void *fw_cfg,
     uint32_t mh_entry_addr;
     uint32_t mh_load_addr;
     uint32_t mb_kernel_size;
-    uint32_t mmap_addr = MULTIBOOT_STRUCT_ADDR;
-    uint32_t mb_bootinfo = MULTIBOOT_STRUCT_ADDR + 0x500;
-    uint32_t mb_mod_end;
-    uint8_t bootinfo[0x500];
-    uint32_t cmdline = 0x200;
-    uint8_t *mb_kernel_data;
+    MultibootState mbs;
+    uint8_t bootinfo[MBI_SIZE];
     uint8_t *mb_bootinfo_data;
 
     /* Ok, let's see if it is a multiboot image.
@@ -550,10 +631,9 @@  static int load_multiboot(void *fw_cfg,
     if (!is_multiboot)
         return 0; /* no multiboot */
 
-#ifdef DEBUG_MULTIBOOT
-    fprintf(stderr, "qemu: I believe we found a multiboot image!\n");
-#endif
+    mb_debug("qemu: I believe we found a multiboot image!\n");
     memset(bootinfo, 0, sizeof(bootinfo));
+    memset(&mbs, 0, sizeof(mbs));
 
     if (flags & 0x00000004) { /* MULTIBOOT_HEADER_HAS_VBE */
         fprintf(stderr, "qemu: multiboot knows VBE. we don't.\n");
@@ -573,24 +653,18 @@  static int load_multiboot(void *fw_cfg,
         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, mh_load_addr, mb_kernel_size) != mb_kernel_size) {
+        mbs.mb_buf = qemu_malloc(mb_kernel_size);
+        if (rom_copy(mbs.mb_buf, mh_load_addr, mb_kernel_size) != mb_kernel_size) {
             fprintf(stderr, "Error while fetching elf kernel from rom\n");
             exit(1);
         }
 
-#ifdef DEBUG_MULTIBOOT
-        fprintf(stderr, "qemu: loading multiboot-elf kernel (%#x bytes) with entry %#zx\n",
-                mb_kernel_size, (size_t)mh_entry_addr);
-#endif
+        mb_debug("qemu: loading multiboot-elf kernel (%#x bytes) with entry %#zx\n",
+                  mb_kernel_size, (size_t)mh_entry_addr);
     } else {
         /* Valid if mh_flags sets MULTIBOOT_HEADER_HAS_ADDR. */
         uint32_t mh_header_addr = ldl_p(header+i+12);
         mh_load_addr = ldl_p(header+i+16);
-#ifdef DEBUG_MULTIBOOT
-        uint32_t mh_load_end_addr = ldl_p(header+i+20);
-        uint32_t mh_bss_end_addr = ldl_p(header+i+24);
-#endif
         uint32_t mb_kernel_text_offset = i - (mh_header_addr - mh_load_addr);
 
         mh_entry_addr = ldl_p(header+i+28);
@@ -602,118 +676,110 @@  static int load_multiboot(void *fw_cfg,
         uint32_t mh_height = ldl_p(header+i+40);
         uint32_t mh_depth = ldl_p(header+i+44); */
 
-#ifdef DEBUG_MULTIBOOT
-        fprintf(stderr, "multiboot: mh_header_addr = %#x\n", mh_header_addr);
-        fprintf(stderr, "multiboot: mh_load_addr = %#x\n", mh_load_addr);
-        fprintf(stderr, "multiboot: mh_load_end_addr = %#x\n", mh_load_end_addr);
-        fprintf(stderr, "multiboot: mh_bss_end_addr = %#x\n", mh_bss_end_addr);
-        fprintf(stderr, "qemu: loading multiboot kernel (%#x bytes) at %#x\n",
-                mb_kernel_size, mh_load_addr);
-#endif
+        mb_debug("multiboot: mh_header_addr = %#x\n", mh_header_addr);
+        mb_debug("multiboot: mh_load_addr = %#x\n", mh_load_addr);
+        mb_debug("multiboot: mh_load_end_addr = %#x\n", ldl_p(header+i+20));
+        mb_debug("multiboot: mh_bss_end_addr = %#x\n", ldl_p(header+i+24));
+        mb_debug("qemu: loading multiboot kernel (%#x bytes) at %#x\n",
+                 mb_kernel_size, mh_load_addr);
 
-        mb_kernel_data = qemu_malloc(mb_kernel_size);
+        mbs.mb_buf = qemu_malloc(mb_kernel_size);
         fseek(f, mb_kernel_text_offset, SEEK_SET);
-        if (fread(mb_kernel_data, 1, mb_kernel_size, f) != mb_kernel_size) {
+        if (fread(mbs.mb_buf, 1, mb_kernel_size, f) != mb_kernel_size) {
             fprintf(stderr, "fread() failed\n");
             exit(1);
         }
         fclose(f);
     }
 
-    /* blob size is only the kernel for now */
-    mb_mod_end = mh_load_addr + mb_kernel_size;
+    mbs.mb_buf_phys = mh_load_addr;
+
+    mbs.mb_buf_size = TARGET_PAGE_ALIGN(mb_kernel_size);
+    mbs.offset_mbinfo = mbs.mb_buf_size;
+
+    /* Calculate space for cmdlines and mb_mods */
+    mbs.mb_buf_size += strlen(kernel_filename) + 1;
+    mbs.mb_buf_size += strlen(kernel_cmdline) + 1;
+    if (initrd_filename) {
+        const char *r = initrd_filename;
+        mbs.mb_buf_size += strlen(r) + 1;
+        mbs.mb_mods_avail = 1;
+        while ((r = strchr(r, ','))) {
+           mbs.mb_mods_avail++;
+           r++;
+        }
+        mbs.mb_buf_size += MB_MOD_SIZE * mbs.mb_mods_avail;
+    }
+
+    mbs.mb_buf_size = TARGET_PAGE_ALIGN(mbs.mb_buf_size);
+
+    /* enlarge mb_buf to hold cmdlines and mb-info structs */
+    mbs.mb_buf          = qemu_realloc(mbs.mb_buf, mbs.mb_buf_size);
+    mbs.offset_cmdlines = mbs.offset_mbinfo + mbs.mb_mods_avail * MB_MOD_SIZE;
 
-    /* load modules */
-    stl_p(bootinfo + 20, 0x0); /* mods_count */
     if (initrd_filename) {
-        uint32_t mb_mod_info = 0x100;
-        uint32_t mb_mod_cmdline = 0x300;
-        uint32_t mb_mod_start = mh_load_addr;
-        uint32_t mb_mod_length = mb_kernel_size;
         char *next_initrd;
-        char *next_space;
-        int mb_mod_count = 0;
+
+        mbs.offset_mods = mbs.mb_buf_size;
 
         do {
-            if (mb_mod_info + 16 > mb_mod_cmdline) {
-                printf("WARNING: Too many modules loaded, aborting.\n");
-                break;
-            }
+            char *next_space;
+            uint32_t mb_mod_length;
+            uint32_t offs = mbs.mb_buf_size;
 
             next_initrd = strchr(initrd_filename, ',');
             if (next_initrd)
                 *next_initrd = '\0';
             /* if a space comes after the module filename, treat everything
                after that as parameters */
-            pstrcpy((char*)bootinfo + mb_mod_cmdline,
-                    sizeof(bootinfo) - mb_mod_cmdline,
-                    initrd_filename);
-            stl_p(bootinfo + mb_mod_info + 8, mb_bootinfo + mb_mod_cmdline); /* string */
-            mb_mod_cmdline += strlen(initrd_filename) + 1;
-            if (mb_mod_cmdline > sizeof(bootinfo)) {
-                mb_mod_cmdline = sizeof(bootinfo);
-                printf("WARNING: Too many module cmdlines loaded, aborting.\n");
-                break;
-            }
+            target_phys_addr_t c = mb_add_cmdline(&mbs, initrd_filename);
             if ((next_space = strchr(initrd_filename, ' ')))
                 *next_space = '\0';
-#ifdef DEBUG_MULTIBOOT
-            printf("multiboot loading module: %s\n", initrd_filename);
-#endif
-            mb_mod_start = (mb_mod_start + mb_mod_length + (TARGET_PAGE_SIZE - 1))
-                         & (TARGET_PAGE_MASK);
+            mb_debug("multiboot loading module: %s\n", initrd_filename);
             mb_mod_length = get_image_size(initrd_filename);
             if (mb_mod_length < 0) {
                 fprintf(stderr, "failed to get %s image size\n", initrd_filename);
                 exit(1);
             }
-            mb_mod_end = mb_mod_start + mb_mod_length;
-            mb_mod_count++;
-
-            /* append module data at the end of last module */
-            mb_kernel_data = qemu_realloc(mb_kernel_data,
-                                          mb_mod_end - mh_load_addr);
-            load_image(initrd_filename,
-                       mb_kernel_data + mb_mod_start - mh_load_addr);
-
-            stl_p(bootinfo + mb_mod_info + 0, mb_mod_start);
-            stl_p(bootinfo + mb_mod_info + 4, mb_mod_start + mb_mod_length);
-            stl_p(bootinfo + mb_mod_info + 12, 0x0); /* reserved */
-#ifdef DEBUG_MULTIBOOT
-            printf("mod_start: %#x\nmod_end:   %#x\n", mb_mod_start,
-                   mb_mod_start + mb_mod_length);
-#endif
+
+            mbs.mb_buf_size = TARGET_PAGE_ALIGN(mb_mod_length + mbs.mb_buf_size);
+            mbs.mb_buf = qemu_realloc(mbs.mb_buf, mbs.mb_buf_size);
+
+            load_image(initrd_filename, (unsigned char *)mbs.mb_buf + offs);
+            mb_add_mod(&mbs, mbs.mb_buf_phys + offs,
+                       mbs.mb_buf_phys + offs + mb_mod_length, c);
+
+            mb_debug("mod_start: %p\nmod_end:   %p\n  cmdline: %#x\n",
+                     (char *)mbs.mb_buf + offs,
+                     (char *)mbs.mb_buf + offs + mb_mod_length, c);
             initrd_filename = next_initrd+1;
-            mb_mod_info += 16;
         } while (next_initrd);
-        stl_p(bootinfo + 20, mb_mod_count); /* mods_count */
-        stl_p(bootinfo + 24, mb_bootinfo + 0x100); /* mods_addr */
     }
 
     /* Commandline support */
-    stl_p(bootinfo + 16, mb_bootinfo + cmdline);
-    snprintf((char*)bootinfo + cmdline, 0x100, "%s %s",
+    char kcmdline[strlen(kernel_filename) + strlen(kernel_cmdline) + 2];
+    snprintf(kcmdline, sizeof(kcmdline), "%s %s",
              kernel_filename, kernel_cmdline);
+    stl_p(bootinfo + MBI_CMDLINE, mb_add_cmdline(&mbs, kcmdline));
 
-    /* the kernel is where we want it to be now */
-#define MULTIBOOT_FLAGS_MEMORY (1 << 0)
-#define MULTIBOOT_FLAGS_BOOT_DEVICE (1 << 1)
-#define MULTIBOOT_FLAGS_CMDLINE (1 << 2)
-#define MULTIBOOT_FLAGS_MODULES (1 << 3)
-#define MULTIBOOT_FLAGS_MMAP (1 << 6)
-    stl_p(bootinfo, MULTIBOOT_FLAGS_MEMORY
-                  | MULTIBOOT_FLAGS_BOOT_DEVICE
-                  | MULTIBOOT_FLAGS_CMDLINE
-                  | MULTIBOOT_FLAGS_MODULES
-                  | MULTIBOOT_FLAGS_MMAP);
-    stl_p(bootinfo + 4, 640); /* mem_lower */
-    stl_p(bootinfo + 8, ram_size / 1024); /* mem_upper */
-    stl_p(bootinfo + 12, 0x8001ffff); /* XXX: use the -boot switch? */
-    stl_p(bootinfo + 48, mmap_addr); /* mmap_addr */
+    stl_p(bootinfo + MBI_MODS_ADDR,  mbs.mb_buf_phys + mbs.offset_mbinfo);
+    stl_p(bootinfo + MBI_MODS_COUNT, mbs.mb_mods_count); /* mods_count */
 
-#ifdef DEBUG_MULTIBOOT
-    fprintf(stderr, "multiboot: mh_entry_addr = %#x\n", mh_entry_addr);
-#endif
+    /* the kernel is where we want it to be now */
+    stl_p(bootinfo + MBI_FLAGS, MULTIBOOT_FLAGS_MEMORY
+                                | MULTIBOOT_FLAGS_BOOT_DEVICE
+                                | MULTIBOOT_FLAGS_CMDLINE
+                                | MULTIBOOT_FLAGS_MODULES
+                                | MULTIBOOT_FLAGS_MMAP);
+    stl_p(bootinfo + MBI_MEM_LOWER,   640);
+    stl_p(bootinfo + MBI_MEM_UPPER,   ram_size / 1024);
+    stl_p(bootinfo + MBI_BOOT_DEVICE, 0x8001ffff); /* XXX: use the -boot switch? */
+    stl_p(bootinfo + MBI_MMAP_ADDR,   ADDR_E820_MAP);
+
+    mb_debug("multiboot: mh_entry_addr = %#x\n", mh_entry_addr);
+    mb_debug("           mb_buf_phys   = %x\n", mbs.mb_buf_phys);
+    mb_debug("           mod_start     = %x\n", mbs.mb_buf_phys + mbs.offset_mods);
+    mb_debug("           mb_mods_count = %d\n", mbs.mb_mods_count);
 
     /* save bootinfo off the stack */
     mb_bootinfo_data = qemu_malloc(sizeof(bootinfo));
@@ -722,11 +788,11 @@  static int load_multiboot(void *fw_cfg,
     /* Pass variables to option rom */
     fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ENTRY, mh_entry_addr);
     fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_ADDR, mh_load_addr);
-    fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_SIZE, mb_mod_end - mh_load_addr);
-    fw_cfg_add_bytes(fw_cfg, FW_CFG_KERNEL_DATA, mb_kernel_data,
-                     mb_mod_end - mh_load_addr);
+    fw_cfg_add_i32(fw_cfg, FW_CFG_KERNEL_SIZE, mbs.mb_buf_size);
+    fw_cfg_add_bytes(fw_cfg, FW_CFG_KERNEL_DATA,
+                     mbs.mb_buf, mbs.mb_buf_size);
 
-    fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_ADDR, mb_bootinfo);
+    fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_ADDR, ADDR_MBI);
     fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_SIZE, sizeof(bootinfo));
     fw_cfg_add_bytes(fw_cfg, FW_CFG_INITRD_DATA, mb_bootinfo_data,
                      sizeof(bootinfo));