From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NTHoj-0006Sw-HM for qemu-devel@nongnu.org; Fri, 08 Jan 2010 11:36:05 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1NTHoe-0006Pz-AH for qemu-devel@nongnu.org; Fri, 08 Jan 2010 11:36:05 -0500 Received: from [199.232.76.173] (port=50144 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NTHoe-0006Pu-6m for qemu-devel@nongnu.org; Fri, 08 Jan 2010 11:36:00 -0500 Received: from mail-yx0-f188.google.com ([209.85.210.188]:60841) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NTHod-0008M2-7E for qemu-devel@nongnu.org; Fri, 08 Jan 2010 11:35:59 -0500 Received: by yxe26 with SMTP id 26so18824129yxe.4 for ; Fri, 08 Jan 2010 08:35:57 -0800 (PST) Message-ID: <4B475EEA.1030801@codemonkey.ws> Date: Fri, 08 Jan 2010 10:35:54 -0600 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH 1/2] multiboot: Support arbitrary number of modules. References: <1261833226-26896-1-git-send-email-adam@os.inf.tu-dresden.de> In-Reply-To: <1261833226-26896-1-git-send-email-adam@os.inf.tu-dresden.de> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Adam Lackorzynski Cc: qemu-devel@nongnu.org On 12/26/2009 07:13 AM, Adam Lackorzynski wrote: > Signed-off-by: Adam Lackorzynski > 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)); >