This defines how multiple modules can be passed to a domain by packing them together into a "multiboot module" in a way very similar to the multiboot standard. An SIF_ flag is added to announce such package. This also adds a packing implementation to PV-GRUB. Signed-Off-By: Samuel Thibault <samuel.thibault@ens-lyon.org> diff -r d0b030008814 xen/include/public/xen.h --- a/xen/include/public/xen.h Fri Nov 27 08:09:26 2009 +0000 +++ b/xen/include/public/xen.h Sun Nov 29 13:17:38 2009 +0100 @@ -584,8 +584,35 @@ typedef struct start_info start_info_t; /* These flags are passed in the ''flags'' field of start_info_t. */ #define SIF_PRIVILEGED (1<<0) /* Is the domain privileged? */ #define SIF_INITDOMAIN (1<<1) /* Is this the initial control domain? */ +#define SIF_MULTIBOOT_MOD (1<<2) /* Is mod_start a multiboot module? */ #define SIF_PM_MASK (0xFF<<8) /* reserve 1 byte for xen-pm options */ +/* + * A multiboot module is a package containing modules very similar to a + * multiboot module array. The only differences are: + * - the array of module descriptors is by convention simply at the beginning + * of the multiboot module, + * - addresses in the module descriptors are based on the beginning of the + * multiboot module, + * - the number of modules is determined by a termination descriptor that has + * mod_start == 0. + * + * This permits to both build it statically and reference it in a configuration + * file, and let the PV guest easily rebase the addresses to virtual addresses + * and at the same time count the number of modules. + */ +struct xen_multiboot_mod_list +{ + /* Address of first byte of the module */ + uint32_t mod_start; + /* Address of last byte of the module (inclusive) */ + uint32_t mod_end; + /* Address of zero-terminated command line */ + uint32_t cmdline; + /* Unused, must be zero */ + uint32_t pad; +}; + typedef struct dom0_vga_console_info { uint8_t video_type; /* DOM0_VGA_CONSOLE_??? */ #define XEN_VGATYPE_TEXT_MODE_3 0x03 diff -r d0b030008814 stubdom/grub.patches/99minios --- a/stubdom/grub.patches/99minios Fri Nov 27 08:09:26 2009 +0000 +++ b/stubdom/grub.patches/99minios Sun Nov 29 13:17:38 2009 +0100 @@ -151,6 +151,14 @@ Index: grub/stage2/builtins.c /* print */ static int +@@ -2910,6 +2910,7 @@ + switch (kernel_type) + { + case KERNEL_TYPE_MULTIBOOT: ++ case KERNEL_TYPE_PV: + if (mb_cmdline + len + 1 > (char *) MB_CMDLINE_BUF + MB_CMDLINE_BUFLEN) + { + errnum = ERR_WONT_FIT; @@ -3776,6 +3802,7 @@ }; diff -r d0b030008814 stubdom/grub/config.h --- a/stubdom/grub/config.h Fri Nov 27 08:09:26 2009 +0000 +++ b/stubdom/grub/config.h Sun Nov 29 13:17:38 2009 +0100 @@ -5,7 +5,7 @@ #define debug _debug #define grub_halt(a) do_exit() #define printf grub_printf -void kexec(void *kernel, long kernel_size, void *module, long module_size, char *cmdline); +void kexec(void *kernel, long kernel_size, void *module, long module_size, char *cmdline, unsigned long flags); struct fbfront_dev *fb_open(void *fb, int width, int height, int depth); void fb_close(void); void pv_boot (void); diff -r d0b030008814 stubdom/grub/kexec.c --- a/stubdom/grub/kexec.c Fri Nov 27 08:09:26 2009 +0000 +++ b/stubdom/grub/kexec.c Sun Nov 29 13:17:38 2009 +0100 @@ -103,7 +103,7 @@ int kexec_allocate(struct xc_dom_image * return 0; } -void kexec(void *kernel, long kernel_size, void *module, long module_size, char *cmdline) +void kexec(void *kernel, long kernel_size, void *module, long module_size, char *cmdline, unsigned long flags) { struct xc_dom_image *dom; int rc; @@ -129,7 +129,7 @@ void kexec(void *kernel, long kernel_siz dom->ramdisk_blob = module; dom->ramdisk_size = module_size; - dom->flags = 0; + dom->flags = flags; dom->console_evtchn = start_info.console.domU.evtchn; dom->xenstore_evtchn = start_info.store_evtchn; diff -r d0b030008814 stubdom/grub/mini-os.c --- a/stubdom/grub/mini-os.c Fri Nov 27 08:09:26 2009 +0000 +++ b/stubdom/grub/mini-os.c Sun Nov 29 13:17:38 2009 +0100 @@ -173,6 +173,8 @@ load_file(char *name, void **ptr, long * void *kernel_image, *module_image; long kernel_size, module_size; char *kernel_arg, *module_arg; +void *multiboot_next_module; +struct xen_multiboot_mod_list *multiboot_next_module_header; kernel_t load_image (char *kernel, char *arg, kernel_t suggested_type, @@ -196,6 +198,8 @@ load_initrd (char *initrd) if (module_image) free(module_image); module_image = NULL; + multiboot_next_module = NULL; + multiboot_next_module_header = NULL; load_file (initrd, &module_image, &module_size); return ! errnum; } @@ -203,20 +207,76 @@ load_initrd (char *initrd) int load_module (char *module, char *arg) { - if (module_image) + void *new_module, *new_module_image; + long new_module_size, rounded_new_module_size; + + if (load_file (module, &new_module, &new_module_size)) + return 0; + if (strlen(arg) >= PAGE_SIZE) { + /* Too big module command line */ + errnum = ERR_WONT_FIT; + return 0; + } + rounded_new_module_size = (new_module_size + PAGE_SIZE - 1) & PAGE_MASK; + + if (module_image && !multiboot_next_module_header) { + /* Initrd already loaded, drop it */ free(module_image); - module_image = NULL; - load_file (module, &module_image, &module_size); - if (module_arg) - free(module_arg); - module_arg = strdup(arg); - return ! errnum; + if (module_arg) + free(module_arg); + module_image = NULL; + } + if (!module_image) + /* Reserve one page for the header */ + multiboot_next_module = (void*) PAGE_SIZE; + + /* Allocate more room for the new module plus its arg */ + new_module_image = realloc(module_image, + (multiboot_next_module - module_image) + rounded_new_module_size + PAGE_SIZE); + + /* Update pointers */ + multiboot_next_module += new_module_image - module_image; + multiboot_next_module_header = (void*) multiboot_next_module_header + (new_module_image - module_image); + module_image = new_module_image; + + if ((void*) (multiboot_next_module_header+1) - module_image > PAGE_SIZE) { + /* Too many modules */ + ERR_WONT_FIT; + return 0; + } + + /* Copy module */ + memcpy(multiboot_next_module, new_module, new_module_size); + multiboot_next_module_header->mod_start = multiboot_next_module - module_image; + multiboot_next_module_header->mod_end = multiboot_next_module_header->mod_start + new_module_size - 1; + multiboot_next_module += rounded_new_module_size; + + /* Copy cmdline */ + strcpy(multiboot_next_module, arg); + multiboot_next_module_header->cmdline = multiboot_next_module - module_image; + multiboot_next_module += PAGE_SIZE; + + /* Pad */ + multiboot_next_module_header->pad = 0; + + multiboot_next_module_header++; + + return 1; } void pv_boot (void) { - kexec(kernel_image, kernel_size, module_image, module_size, kernel_arg); + unsigned long flags = 0; + if (multiboot_next_module_header) { + /* Termination entry */ + multiboot_next_module_header->mod_start = 0; + /* Total size */ + module_size = multiboot_next_module - module_image; + /* It''s a multiboot module */ + flags |= SIF_MULTIBOOT_MOD; + } + kexec(kernel_image, kernel_size, module_image, module_size, kernel_arg, flags); } /* _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On Wed, 2009-12-09 at 14:01 +0000, Samuel Thibault wrote:> + * A multiboot module is a package containing modules very similar to > a > + * multiboot module array. The only differences are:I don''t really know the multiboot layout but is there a reason not to use it as is rather than defining a new version? I would imagine that using the standard layout could potentially lead to less Xen specific code on the guest side. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Samuel Thibault
2009-Dec-09 14:59 UTC
Re: [Xen-devel] [PATCH] multiboot-like module support
Ian Campbell, le Wed 09 Dec 2009 14:29:35 +0000, a écrit :> On Wed, 2009-12-09 at 14:01 +0000, Samuel Thibault wrote: > > + * A multiboot module is a package containing modules very similar to > > a > > + * multiboot module array. The only differences are: > > I don''t really know the multiboot layout but is there a reason not to > use it as is rather than defining a new version?It''d make the builder more tricky, as multiboot uses absolute physical addresses. That''d mean extending libxc in order to take the list of modules itself and build the table very late, during xc_dom_build_image(), when the target virtual address is known, while by using relative offsets, there is no need to, it''s self-contained. Relative offsets also permit to build a package even before submitting it to xm create (that''s how I''m using it atm actually). I''m not against it, it''s just that it is much less simple. One good thing however is that then the code building the table would be in libxc, shared by pv-grub and xend. The other difference is how to know the number of modules. In the multiboot standard, it''s simply set in the boot_info structure. That could be added to the start_info structure for instance.> I would imagine that using the standard layout could potentially lead to > less Xen specific code on the guest side.Well, yes, I''d just answer that there aren''t so many multiboot implementations anyway :) Samuel _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Hello, Samuel Thibault, le Wed 09 Dec 2009 15:59:15 +0100, a écrit :> It''d make the builder more tricky, as multiboot uses absolute > physical addresses.Here is a patch. This defines how multiple modules can be passed to a domain by packing them together into a "multiboot module" the same way as the multiboot standard. An SIF_ flag is added to announce such package. xc_dom_multiboot_mem is added to libxc to load such list of modules, used by PV-GRUB. Signed-Off-By: Samuel Thibault <samuel.thibault@ens-lyon.org> diff -r 8f304c003af4 stubdom/grub.patches/99minios --- a/stubdom/grub.patches/99minios Wed Dec 09 10:59:31 2009 +0000 +++ b/stubdom/grub.patches/99minios Fri Dec 11 03:40:30 2009 +0100 @@ -151,6 +151,14 @@ /* print */ static int +@@ -2910,6 +2910,7 @@ + switch (kernel_type) + { + case KERNEL_TYPE_MULTIBOOT: ++ case KERNEL_TYPE_PV: + if (mb_cmdline + len + 1 > (char *) MB_CMDLINE_BUF + MB_CMDLINE_BUFLEN) + { + errnum = ERR_WONT_FIT; @@ -3776,6 +3802,7 @@ }; diff -r 8f304c003af4 stubdom/grub/config.h --- a/stubdom/grub/config.h Wed Dec 09 10:59:31 2009 +0000 +++ b/stubdom/grub/config.h Fri Dec 11 03:40:30 2009 +0100 @@ -5,7 +5,7 @@ #define debug _debug #define grub_halt(a) do_exit() #define printf grub_printf -void kexec(void *kernel, long kernel_size, void *module, long module_size, char *cmdline); +void kexec(void *kernel, long kernel_size, char *cmdline, void *module, long module_size, int multiboot_number, void **multiboot_blobs, size_t *multiboot_sizes, char **multiboot_cmdlines); struct fbfront_dev *fb_open(void *fb, int width, int height, int depth); void fb_close(void); void pv_boot (void); diff -r 8f304c003af4 stubdom/grub/kexec.c --- a/stubdom/grub/kexec.c Wed Dec 09 10:59:31 2009 +0000 +++ b/stubdom/grub/kexec.c Fri Dec 11 03:40:30 2009 +0100 @@ -103,7 +103,7 @@ return 0; } -void kexec(void *kernel, long kernel_size, void *module, long module_size, char *cmdline) +void kexec(void *kernel, long kernel_size, char *cmdline, void *module, long module_size, int multiboot_number, void **multiboot_blobs, size_t *multiboot_sizes, char **multiboot_cmdlines) { struct xc_dom_image *dom; int rc; @@ -123,11 +123,11 @@ dom = xc_dom_allocate(cmdline, features); dom->allocate = kexec_allocate; - dom->kernel_blob = kernel; - dom->kernel_size = kernel_size; - - dom->ramdisk_blob = module; - dom->ramdisk_size = module_size; + xc_dom_kernel_mem(dom, kernel, kernel_size); + if (module) + xc_dom_ramdisk_mem(dom, module, module_size); + else if (multiboot_blobs) + xc_dom_multiboot_mem(dom, multiboot_number, multiboot_blobs, multiboot_sizes, multiboot_cmdlines); dom->flags = 0; dom->console_evtchn = start_info.console.domU.evtchn; diff -r 8f304c003af4 stubdom/grub/mini-os.c --- a/stubdom/grub/mini-os.c Wed Dec 09 10:59:31 2009 +0000 +++ b/stubdom/grub/mini-os.c Fri Dec 11 03:40:30 2009 +0100 @@ -172,7 +172,11 @@ void *kernel_image, *module_image; long kernel_size, module_size; -char *kernel_arg, *module_arg; +char *kernel_arg; +void **multiboot_blobs; +size_t *multiboot_sizes; +char **multiboot_cmdlines; +int multiboot_number; kernel_t load_image (char *kernel, char *arg, kernel_t suggested_type, @@ -196,6 +200,13 @@ if (module_image) free(module_image); module_image = NULL; + multiboot_number = 0; + free(multiboot_blobs); + multiboot_blobs = NULL; + free(multiboot_sizes); + multiboot_sizes = NULL; + free(multiboot_cmdlines); + multiboot_cmdlines = NULL; load_file (initrd, &module_image, &module_size); return ! errnum; } @@ -203,20 +214,28 @@ int load_module (char *module, char *arg) { - if (module_image) - free(module_image); - module_image = NULL; - load_file (module, &module_image, &module_size); - if (module_arg) - free(module_arg); - module_arg = strdup(arg); - return ! errnum; + void *module_blob; + long module_size; + + if (load_file (module, &module_blob, &module_size)) + return 0; + + multiboot_number++; + multiboot_blobs = realloc(multiboot_blobs, multiboot_number * sizeof(*multiboot_blobs)); + multiboot_sizes = realloc(multiboot_sizes, multiboot_number * sizeof(*multiboot_sizes)); + multiboot_cmdlines = realloc(multiboot_cmdlines, multiboot_number * sizeof(*multiboot_cmdlines)); + + multiboot_blobs[multiboot_number-1] = module_blob; + multiboot_sizes[multiboot_number-1] = module_size; + multiboot_cmdlines[multiboot_number-1] = arg; + + return 1; } void pv_boot (void) { - kexec(kernel_image, kernel_size, module_image, module_size, kernel_arg); + kexec(kernel_image, kernel_size, kernel_arg, module_image, module_size, multiboot_number, multiboot_blobs, multiboot_sizes, multiboot_cmdlines); } /* diff -r 8f304c003af4 tools/include/xen-foreign/reference.size --- a/tools/include/xen-foreign/reference.size Wed Dec 09 10:59:31 2009 +0000 +++ b/tools/include/xen-foreign/reference.size Fri Dec 11 03:40:30 2009 +0100 @@ -1,7 +1,7 @@ structs | x86_32 x86_64 ia64 -start_info | 1112 1168 1168 +start_info | 1116 1176 1176 trap_info | 8 16 - pt_fpreg | - - 16 cpu_user_regs | 68 200 - diff -r 8f304c003af4 tools/libxc/xc_dom.h --- a/tools/libxc/xc_dom.h Wed Dec 09 10:59:31 2009 +0000 +++ b/tools/libxc/xc_dom.h Fri Dec 11 03:40:30 2009 +0100 @@ -35,6 +35,10 @@ size_t kernel_size; void *ramdisk_blob; size_t ramdisk_size; + int multiboot_number; + void **multiboot_blobs; + char **multiboot_cmdlines; + size_t *multiboot_sizes; /* arguments and parameters */ char *cmdline; @@ -47,6 +51,7 @@ /* memory layout */ struct xc_dom_seg kernel_seg; struct xc_dom_seg ramdisk_seg; + struct xc_dom_seg multiboot_seg; struct xc_dom_seg p2m_seg; struct xc_dom_seg pgtables_seg; struct xc_dom_seg devicetree_seg; @@ -168,6 +173,10 @@ size_t memsize); int xc_dom_ramdisk_mem(struct xc_dom_image *dom, const void *mem, size_t memsize); +int xc_dom_multiboot_mem(struct xc_dom_image *dom, int multiboot_number, + void **multiboot_blobs, + size_t *multiboot_sizes, + char **multiboot_cmdlines); int xc_dom_parse_image(struct xc_dom_image *dom); struct xc_dom_arch *xc_dom_find_arch_hooks(char *guest_type); diff -r 8f304c003af4 tools/libxc/xc_dom_core.c --- a/tools/libxc/xc_dom_core.c Wed Dec 09 10:59:31 2009 +0000 +++ b/tools/libxc/xc_dom_core.c Fri Dec 11 03:40:30 2009 +0100 @@ -607,6 +607,17 @@ return 0; } +int xc_dom_multiboot_mem(struct xc_dom_image *dom, int number, + void **mems, size_t *memsizes, char **cmdlines) +{ + xc_dom_printf("%s: called\n", __FUNCTION__); + dom->multiboot_number = number; + dom->multiboot_blobs = mems; + dom->multiboot_sizes = memsizes; + dom->multiboot_cmdlines = cmdlines; + return 0; +} + int xc_dom_parse_image(struct xc_dom_image *dom) { int i; @@ -757,6 +768,54 @@ memcpy(ramdiskmap, dom->ramdisk_blob, dom->ramdisk_size); } + /* load multiboot modules */ + if ( dom->multiboot_blobs ) + { + void *multibootmap; + int i; + struct xen_multiboot_mod_list *module; + char *module_cmdline; + void *module_blob; + size_t totsize = 0; + size_t headers_size = 0; + size_t cmdlines_size = 0; + + for (i = 0; i < dom->multiboot_number; i++) { + totsize += ROUNDUP(dom->multiboot_sizes[i], page_size); + headers_size += sizeof(*module); + cmdlines_size += strlen(dom->multiboot_cmdlines[i]) + 1; + } + + totsize += ROUNDUP(headers_size + cmdlines_size, page_size); + + if (xc_dom_alloc_segment(dom, &dom->multiboot_seg, "multiboot", 0, totsize)) + goto err; + multibootmap = xc_dom_seg_to_ptr(dom, &dom->multiboot_seg); + + module = multibootmap; + module_cmdline = multibootmap + headers_size; + module_blob = multibootmap + headers_size + cmdlines_size; + + for (i = 0; i < dom->multiboot_number; i++) { + int len = strlen(dom->multiboot_cmdlines[i]) + 1; + memcpy(module_cmdline, dom->multiboot_cmdlines[i], len); + module->cmdline = (void*) module_cmdline - multibootmap + + dom->multiboot_seg.vstart - dom->parms.virt_base; + module_cmdline += len; + + memcpy(module_blob, dom->multiboot_blobs[i], + dom->multiboot_sizes[i]); + module->mod_start = (void*) module_blob - multibootmap + + dom->multiboot_seg.vstart - dom->parms.virt_base; + module->mod_end = module->mod_start + dom->multiboot_sizes[i] - 1; + module_blob += ROUNDUP(dom->multiboot_sizes[i], page_size); + + module->pad = 0; + + module++; + } + } + /* allocate other pages */ if ( dom->arch_hooks->alloc_magic_pages(dom) != 0 ) goto err; diff -r 8f304c003af4 tools/libxc/xc_dom_ia64.c --- a/tools/libxc/xc_dom_ia64.c Wed Dec 09 10:59:31 2009 +0000 +++ b/tools/libxc/xc_dom_ia64.c Fri Dec 11 03:40:30 2009 +0100 @@ -69,6 +69,17 @@ bp->initrd_start = start_info->mod_start; bp->initrd_size = start_info->mod_len; } + + if ( dom->multiboot_blobs ) + { + start_info->mod_start = dom->multiboot_seg.vstart; + start_info->mod_len = dom->multiboot_seg.vend - dom->multiboot_seg.vstart; + start_info->mods_count = dom->multiboot_number; + bp->initrd_start = start_info->mod_start; + bp->initrd_size = start_info->mod_len; + dom->flags |= SIF_MULTIBOOT_MOD; + } + bp->command_line = (dom->start_info_pfn << PAGE_SHIFT_IA64) + offsetof(start_info_t, cmd_line); if ( dom->cmdline ) diff -r 8f304c003af4 tools/libxc/xc_dom_x86.c --- a/tools/libxc/xc_dom_x86.c Wed Dec 09 10:59:31 2009 +0000 +++ b/tools/libxc/xc_dom_x86.c Fri Dec 11 03:40:30 2009 +0100 @@ -441,6 +441,14 @@ start_info->mod_len = dom->ramdisk_seg.vend - dom->ramdisk_seg.vstart; } + if ( dom->multiboot_blobs ) + { + start_info->mod_start = dom->multiboot_seg.vstart; + start_info->mod_len = dom->multiboot_seg.vend - dom->multiboot_seg.vstart; + start_info->mods_count = dom->multiboot_number; + dom->flags |= SIF_MULTIBOOT_MOD; + } + if ( dom->cmdline ) { strncpy((char *)start_info->cmd_line, dom->cmdline, MAX_GUEST_CMDLINE); @@ -481,6 +489,14 @@ start_info->mod_len = dom->ramdisk_seg.vend - dom->ramdisk_seg.vstart; } + if ( dom->multiboot_blobs ) + { + start_info->mod_start = dom->multiboot_seg.vstart; + start_info->mod_len = dom->multiboot_seg.vend - dom->multiboot_seg.vstart; + start_info->mods_count = dom->multiboot_number; + dom->flags |= SIF_MULTIBOOT_MOD; + } + if ( dom->cmdline ) { strncpy((char *)start_info->cmd_line, dom->cmdline, MAX_GUEST_CMDLINE); diff -r 8f304c003af4 xen/include/public/xen.h --- a/xen/include/public/xen.h Wed Dec 09 10:59:31 2009 +0000 +++ b/xen/include/public/xen.h Fri Dec 11 03:40:30 2009 +0100 @@ -572,6 +572,7 @@ /* The pfn range here covers both page table and p->m table frames. */ unsigned long first_p2m_pfn;/* 1st pfn forming initial P->M table. */ unsigned long nr_p2m_frames;/* # of pfns forming initial P->M table. */ + unsigned long mods_count; /* Number of multiboot modules. */ }; typedef struct start_info start_info_t; @@ -584,8 +585,25 @@ /* These flags are passed in the ''flags'' field of start_info_t. */ #define SIF_PRIVILEGED (1<<0) /* Is the domain privileged? */ #define SIF_INITDOMAIN (1<<1) /* Is this the initial control domain? */ +#define SIF_MULTIBOOT_MOD (1<<2) /* Is mod_start a multiboot module? */ #define SIF_PM_MASK (0xFF<<8) /* reserve 1 byte for xen-pm options */ +/* + * A multiboot module is a package containing modules like a multiboot module + * array. + */ +struct xen_multiboot_mod_list +{ + /* PHYSICAL address of first byte of the module */ + unsigned long mod_start; + /* PHYSICAL address of last byte of the module (inclusive) */ + unsigned long mod_end; + /* PHYSICAL address of zero-terminated command line */ + unsigned long cmdline; + /* Unused, must be zero */ + unsigned long pad; +}; + typedef struct dom0_vga_console_info { uint8_t video_type; /* DOM0_VGA_CONSOLE_??? */ #define XEN_VGATYPE_TEXT_MODE_3 0x03 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 11/12/2009 02:44, "Samuel Thibault" <samuel.thibault@ens-lyon.org> wrote:> Samuel Thibault, le Wed 09 Dec 2009 15:59:15 +0100, a écrit : >> It''d make the builder more tricky, as multiboot uses absolute >> physical addresses. > > Here is a patch.Using my metric of "which patch is way shorter, and doesn''t touch start_info", I like the old patch rather more. :-) -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On Fri, 2009-12-11 at 08:09 +0000, Keir Fraser wrote:> On 11/12/2009 02:44, "Samuel Thibault" <samuel.thibault@ens-lyon.org> wrote: > > > Samuel Thibault, le Wed 09 Dec 2009 15:59:15 +0100, a écrit : > >> It''d make the builder more tricky, as multiboot uses absolute > >> physical addresses. > > > > Here is a patch. > > Using my metric of "which patch is way shorter, and doesn''t touch > start_info", I like the old patch rather more. :-)I think I agree. I initially asked because I wondered what the reasons were for the difference in layout, but Samuel''s reasons seemed, well, reasonable ;-) This version doesn''t seem too bad though. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Ian Campbell, le Fri 11 Dec 2009 09:59:44 +0000, a écrit :> On Fri, 2009-12-11 at 08:09 +0000, Keir Fraser wrote: > > On 11/12/2009 02:44, "Samuel Thibault" <samuel.thibault@ens-lyon.org> wrote: > > > > > Samuel Thibault, le Wed 09 Dec 2009 15:59:15 +0100, a écrit : > > >> It''d make the builder more tricky, as multiboot uses absolute > > >> physical addresses. > > > > > > Here is a patch. > > > > Using my metric of "which patch is way shorter, and doesn''t touch > > start_info", I like the old patch rather more. :-) > > I think I agree. I initially asked because I wondered what the reasons > were for the difference in layout, but Samuel''s reasons seemed, well, > reasonable ;-) > > This version doesn''t seem too bad though.Yes, it didn''t go as badly as I thought. The addition of mods_count could perhaps be avoided by saying that the multiboot module table always ends with a NULL entry. That is still compliant with multiboot, it just makes the PV guest have to count the modules itself (but not have to rebase all the pointers). About "which patch is way shorter", it could actually be useful, for the first version, to move the code building the package into libxc, to share it with future xm support, and we end up with basically the same complexity. Samuel _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Samuel Thibault, le Fri 11 Dec 2009 11:29:23 +0100, a écrit :> Yes, it didn''t go as badly as I thought. The addition of mods_count > could perhaps be avoided by saying that the multiboot module table > always ends with a NULL entry. That is still compliant with multiboot, > it just makes the PV guest have to count the modules itself (but not > have to rebase all the pointers).i.e. the patch below Samuel This defines how multiple modules can be passed to a domain by packing them together into a "multiboot module" the same way as the multiboot standard. An SIF_ flag is added to announce such package. xc_dom_multiboot_mem is added to libxc to load such list of modules, used by PV-GRUB. Signed-Off-By: Samuel Thibault <samuel.thibault@ens-lyon.org> diff -r 8f304c003af4 stubdom/grub.patches/99minios --- a/stubdom/grub.patches/99minios Wed Dec 09 10:59:31 2009 +0000 +++ b/stubdom/grub.patches/99minios Sat Dec 12 00:18:05 2009 +0100 @@ -151,6 +151,14 @@ /* print */ static int +@@ -2910,6 +2910,7 @@ + switch (kernel_type) + { + case KERNEL_TYPE_MULTIBOOT: ++ case KERNEL_TYPE_PV: + if (mb_cmdline + len + 1 > (char *) MB_CMDLINE_BUF + MB_CMDLINE_BUFLEN) + { + errnum = ERR_WONT_FIT; @@ -3776,6 +3802,7 @@ }; diff -r 8f304c003af4 stubdom/grub/config.h --- a/stubdom/grub/config.h Wed Dec 09 10:59:31 2009 +0000 +++ b/stubdom/grub/config.h Sat Dec 12 00:18:05 2009 +0100 @@ -5,7 +5,7 @@ #define debug _debug #define grub_halt(a) do_exit() #define printf grub_printf -void kexec(void *kernel, long kernel_size, void *module, long module_size, char *cmdline); +void kexec(void *kernel, long kernel_size, char *cmdline, void *module, long module_size, int multiboot_number, void **multiboot_blobs, size_t *multiboot_sizes, char **multiboot_cmdlines); struct fbfront_dev *fb_open(void *fb, int width, int height, int depth); void fb_close(void); void pv_boot (void); diff -r 8f304c003af4 stubdom/grub/kexec.c --- a/stubdom/grub/kexec.c Wed Dec 09 10:59:31 2009 +0000 +++ b/stubdom/grub/kexec.c Sat Dec 12 00:18:05 2009 +0100 @@ -103,7 +103,7 @@ return 0; } -void kexec(void *kernel, long kernel_size, void *module, long module_size, char *cmdline) +void kexec(void *kernel, long kernel_size, char *cmdline, void *module, long module_size, int multiboot_number, void **multiboot_blobs, size_t *multiboot_sizes, char **multiboot_cmdlines) { struct xc_dom_image *dom; int rc; @@ -123,11 +123,11 @@ dom = xc_dom_allocate(cmdline, features); dom->allocate = kexec_allocate; - dom->kernel_blob = kernel; - dom->kernel_size = kernel_size; - - dom->ramdisk_blob = module; - dom->ramdisk_size = module_size; + xc_dom_kernel_mem(dom, kernel, kernel_size); + if (module) + xc_dom_ramdisk_mem(dom, module, module_size); + else if (multiboot_blobs) + xc_dom_multiboot_mem(dom, multiboot_number, multiboot_blobs, multiboot_sizes, multiboot_cmdlines); dom->flags = 0; dom->console_evtchn = start_info.console.domU.evtchn; diff -r 8f304c003af4 stubdom/grub/mini-os.c --- a/stubdom/grub/mini-os.c Wed Dec 09 10:59:31 2009 +0000 +++ b/stubdom/grub/mini-os.c Sat Dec 12 00:18:05 2009 +0100 @@ -172,7 +172,11 @@ void *kernel_image, *module_image; long kernel_size, module_size; -char *kernel_arg, *module_arg; +char *kernel_arg; +void **multiboot_blobs; +size_t *multiboot_sizes; +char **multiboot_cmdlines; +int multiboot_number; kernel_t load_image (char *kernel, char *arg, kernel_t suggested_type, @@ -196,6 +200,13 @@ if (module_image) free(module_image); module_image = NULL; + multiboot_number = 0; + free(multiboot_blobs); + multiboot_blobs = NULL; + free(multiboot_sizes); + multiboot_sizes = NULL; + free(multiboot_cmdlines); + multiboot_cmdlines = NULL; load_file (initrd, &module_image, &module_size); return ! errnum; } @@ -203,20 +214,28 @@ int load_module (char *module, char *arg) { - if (module_image) - free(module_image); - module_image = NULL; - load_file (module, &module_image, &module_size); - if (module_arg) - free(module_arg); - module_arg = strdup(arg); - return ! errnum; + void *module_blob; + long module_size; + + if (load_file (module, &module_blob, &module_size)) + return 0; + + multiboot_number++; + multiboot_blobs = realloc(multiboot_blobs, multiboot_number * sizeof(*multiboot_blobs)); + multiboot_sizes = realloc(multiboot_sizes, multiboot_number * sizeof(*multiboot_sizes)); + multiboot_cmdlines = realloc(multiboot_cmdlines, multiboot_number * sizeof(*multiboot_cmdlines)); + + multiboot_blobs[multiboot_number-1] = module_blob; + multiboot_sizes[multiboot_number-1] = module_size; + multiboot_cmdlines[multiboot_number-1] = arg; + + return 1; } void pv_boot (void) { - kexec(kernel_image, kernel_size, module_image, module_size, kernel_arg); + kexec(kernel_image, kernel_size, kernel_arg, module_image, module_size, multiboot_number, multiboot_blobs, multiboot_sizes, multiboot_cmdlines); } /* diff -r 8f304c003af4 tools/libxc/xc_dom.h --- a/tools/libxc/xc_dom.h Wed Dec 09 10:59:31 2009 +0000 +++ b/tools/libxc/xc_dom.h Sat Dec 12 00:18:05 2009 +0100 @@ -35,6 +35,10 @@ size_t kernel_size; void *ramdisk_blob; size_t ramdisk_size; + int multiboot_number; + void **multiboot_blobs; + char **multiboot_cmdlines; + size_t *multiboot_sizes; /* arguments and parameters */ char *cmdline; @@ -47,6 +51,7 @@ /* memory layout */ struct xc_dom_seg kernel_seg; struct xc_dom_seg ramdisk_seg; + struct xc_dom_seg multiboot_seg; struct xc_dom_seg p2m_seg; struct xc_dom_seg pgtables_seg; struct xc_dom_seg devicetree_seg; @@ -168,6 +173,10 @@ size_t memsize); int xc_dom_ramdisk_mem(struct xc_dom_image *dom, const void *mem, size_t memsize); +int xc_dom_multiboot_mem(struct xc_dom_image *dom, int multiboot_number, + void **multiboot_blobs, + size_t *multiboot_sizes, + char **multiboot_cmdlines); int xc_dom_parse_image(struct xc_dom_image *dom); struct xc_dom_arch *xc_dom_find_arch_hooks(char *guest_type); diff -r 8f304c003af4 tools/libxc/xc_dom_core.c --- a/tools/libxc/xc_dom_core.c Wed Dec 09 10:59:31 2009 +0000 +++ b/tools/libxc/xc_dom_core.c Sat Dec 12 00:18:05 2009 +0100 @@ -607,6 +607,17 @@ return 0; } +int xc_dom_multiboot_mem(struct xc_dom_image *dom, int number, + void **mems, size_t *memsizes, char **cmdlines) +{ + xc_dom_printf("%s: called\n", __FUNCTION__); + dom->multiboot_number = number; + dom->multiboot_blobs = mems; + dom->multiboot_sizes = memsizes; + dom->multiboot_cmdlines = cmdlines; + return 0; +} + int xc_dom_parse_image(struct xc_dom_image *dom) { int i; @@ -757,6 +768,56 @@ memcpy(ramdiskmap, dom->ramdisk_blob, dom->ramdisk_size); } + /* load multiboot modules */ + if ( dom->multiboot_blobs ) + { + void *multibootmap; + int i; + struct xen_multiboot_mod_list *module; + char *module_cmdline; + void *module_blob; + size_t totsize = 0; + size_t headers_size = 0; + size_t cmdlines_size = 0; + + for (i = 0; i < dom->multiboot_number; i++) { + totsize += ROUNDUP(dom->multiboot_sizes[i], page_size); + headers_size += sizeof(*module); + cmdlines_size += strlen(dom->multiboot_cmdlines[i]) + 1; + } + headers_size += sizeof(*module); + + totsize += ROUNDUP(headers_size + cmdlines_size, page_size); + + if (xc_dom_alloc_segment(dom, &dom->multiboot_seg, "multiboot", 0, totsize)) + goto err; + multibootmap = xc_dom_seg_to_ptr(dom, &dom->multiboot_seg); + + module = multibootmap; + module_cmdline = multibootmap + headers_size; + module_blob = multibootmap + headers_size + cmdlines_size; + + for (i = 0; i < dom->multiboot_number; i++) { + int len = strlen(dom->multiboot_cmdlines[i]) + 1; + memcpy(module_cmdline, dom->multiboot_cmdlines[i], len); + module->cmdline = (void*) module_cmdline - multibootmap + + dom->multiboot_seg.vstart - dom->parms.virt_base; + module_cmdline += len; + + memcpy(module_blob, dom->multiboot_blobs[i], + dom->multiboot_sizes[i]); + module->mod_start = (void*) module_blob - multibootmap + + dom->multiboot_seg.vstart - dom->parms.virt_base; + module->mod_end = module->mod_start + dom->multiboot_sizes[i] - 1; + module_blob += ROUNDUP(dom->multiboot_sizes[i], page_size); + + module->pad = 0; + + module++; + } + module->mod_start = 0; + } + /* allocate other pages */ if ( dom->arch_hooks->alloc_magic_pages(dom) != 0 ) goto err; diff -r 8f304c003af4 tools/libxc/xc_dom_ia64.c --- a/tools/libxc/xc_dom_ia64.c Wed Dec 09 10:59:31 2009 +0000 +++ b/tools/libxc/xc_dom_ia64.c Sat Dec 12 00:18:05 2009 +0100 @@ -69,6 +69,16 @@ bp->initrd_start = start_info->mod_start; bp->initrd_size = start_info->mod_len; } + + if ( dom->multiboot_blobs ) + { + start_info->mod_start = dom->multiboot_seg.vstart; + start_info->mod_len = dom->multiboot_seg.vend - dom->multiboot_seg.vstart; + bp->initrd_start = start_info->mod_start; + bp->initrd_size = start_info->mod_len; + dom->flags |= SIF_MULTIBOOT_MOD; + } + bp->command_line = (dom->start_info_pfn << PAGE_SHIFT_IA64) + offsetof(start_info_t, cmd_line); if ( dom->cmdline ) diff -r 8f304c003af4 tools/libxc/xc_dom_x86.c --- a/tools/libxc/xc_dom_x86.c Wed Dec 09 10:59:31 2009 +0000 +++ b/tools/libxc/xc_dom_x86.c Sat Dec 12 00:18:05 2009 +0100 @@ -441,6 +441,13 @@ start_info->mod_len = dom->ramdisk_seg.vend - dom->ramdisk_seg.vstart; } + if ( dom->multiboot_blobs ) + { + start_info->mod_start = dom->multiboot_seg.vstart; + start_info->mod_len = dom->multiboot_seg.vend - dom->multiboot_seg.vstart; + dom->flags |= SIF_MULTIBOOT_MOD; + } + if ( dom->cmdline ) { strncpy((char *)start_info->cmd_line, dom->cmdline, MAX_GUEST_CMDLINE); @@ -481,6 +488,13 @@ start_info->mod_len = dom->ramdisk_seg.vend - dom->ramdisk_seg.vstart; } + if ( dom->multiboot_blobs ) + { + start_info->mod_start = dom->multiboot_seg.vstart; + start_info->mod_len = dom->multiboot_seg.vend - dom->multiboot_seg.vstart; + dom->flags |= SIF_MULTIBOOT_MOD; + } + if ( dom->cmdline ) { strncpy((char *)start_info->cmd_line, dom->cmdline, MAX_GUEST_CMDLINE); diff -r 8f304c003af4 xen/include/public/xen.h --- a/xen/include/public/xen.h Wed Dec 09 10:59:31 2009 +0000 +++ b/xen/include/public/xen.h Sat Dec 12 00:18:05 2009 +0100 @@ -584,8 +584,27 @@ /* These flags are passed in the ''flags'' field of start_info_t. */ #define SIF_PRIVILEGED (1<<0) /* Is the domain privileged? */ #define SIF_INITDOMAIN (1<<1) /* Is this the initial control domain? */ +#define SIF_MULTIBOOT_MOD (1<<2) /* Is mod_start a multiboot module? */ #define SIF_PM_MASK (0xFF<<8) /* reserve 1 byte for xen-pm options */ +/* + * A multiboot module is a package containing modules like a multiboot module + * array. The only difference is that the module list always ends with an entry + * with mod_start set to zero, to let the guest know how many modules are + * provided. + */ +struct xen_multiboot_mod_list +{ + /* PHYSICAL address of first byte of the module */ + unsigned long mod_start; + /* PHYSICAL address of last byte of the module (inclusive) */ + unsigned long mod_end; + /* PHYSICAL address of zero-terminated command line */ + unsigned long cmdline; + /* Unused, must be zero */ + unsigned long pad; +}; + typedef struct dom0_vga_console_info { uint8_t video_type; /* DOM0_VGA_CONSOLE_??? */ #define XEN_VGATYPE_TEXT_MODE_3 0x03 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
So, what''s the preferred way? - either completely multiboot compliant, has to patch libxc and modify start_info to add the number of modules (my second patch) - multiboot compliant except the module table has to end with a NULL entry to let the domU compute the number of modules, has to patch libxc but leave start_info as is (my third patch). - multiboot-like, offsets have to be recomputed by the domU, no need to patch libxc for now, until somebody wants to add xend support (my first patch). Samuel _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Your original patch is checked in already, you know? -- Keir On 15/12/2009 20:42, "Samuel Thibault" <samuel.thibault@ens-lyon.org> wrote:> So, what''s the preferred way? > > - either completely multiboot compliant, has to patch libxc and modify > start_info to add the number of modules (my second patch) > - multiboot compliant except the module table has to end with a NULL > entry to let the domU compute the number of modules, has to patch > libxc but leave start_info as is (my third patch). > - multiboot-like, offsets have to be recomputed by the domU, no need to > patch libxc for now, until somebody wants to add xend support (my > first patch). > > Samuel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser, le Tue 15 Dec 2009 22:18:16 +0000, a écrit :> Your original patch is checked in already, you know?Ah, I didn''t know :) Cool! Samuel _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel