All of lore.kernel.org
 help / color / mirror / Atom feed
From: Anthony Liguori <anthony@codemonkey.ws>
To: Adam Lackorzynski <adam@os.inf.tu-dresden.de>
Cc: qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH 1/2] multiboot: Support arbitrary number of modules.
Date: Fri, 08 Jan 2010 10:35:54 -0600	[thread overview]
Message-ID: <4B475EEA.1030801@codemonkey.ws> (raw)
In-Reply-To: <1261833226-26896-1-git-send-email-adam@os.inf.tu-dresden.de>

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));
>    

       reply	other threads:[~2010-01-08 16:36 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1261833226-26896-1-git-send-email-adam@os.inf.tu-dresden.de>
2010-01-08 16:35 ` Anthony Liguori [this message]
2009-12-18 18:57 [Qemu-devel] [PATCH 1/2] multiboot: Support arbitrary number of modules Adam Lackorzynski
  -- strict thread matches above, loose matches on Subject: below --
2009-11-30 22:25 [Qemu-devel] [PATCH 1/2] multiboot: Fix module loading and setting of mmap Adam Lackorzynski
2009-11-30 22:26 ` [Qemu-devel] [PATCH 2/2] multiboot: Support arbitrary number of modules Adam Lackorzynski
2009-12-03 20:26   ` Anthony Liguori
2009-12-03 22:19     ` [Qemu-devel] [PATCH 1/2] " Adam Lackorzynski
2009-12-03 23:24       ` [Qemu-devel] " Anthony Liguori
2009-12-06 12:52         ` [Qemu-devel] " Adam Lackorzynski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4B475EEA.1030801@codemonkey.ws \
    --to=anthony@codemonkey.ws \
    --cc=adam@os.inf.tu-dresden.de \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.