From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Daniel Kiper <daniel.kiper@oracle.com>, xen-devel@lists.xenproject.org
Cc: jgross@suse.com, keir@xen.org, ian.campbell@citrix.com,
stefano.stabellini@eu.citrix.com, ross.philipson@citrix.com,
roy.franz@linaro.org, ning.sun@intel.com, jbeulich@suse.com,
qiaowei.ren@intel.com, richard.l.maliszewski@intel.com,
gang.wei@intel.com, fu.wei@linaro.org
Subject: Re: [PATCH for-xen-4.5 v4 12/18] x86: move modules data from mbi to boot_info and remove mbi
Date: Fri, 17 Oct 2014 23:35:44 +0100 [thread overview]
Message-ID: <544199C0.60600@citrix.com> (raw)
In-Reply-To: <1413555132-22138-13-git-send-email-daniel.kiper@oracle.com>
On 17/10/2014 15:12, Daniel Kiper wrote:
This patch is still conflating two things (moving modules from one
scheme to another), and clearing up the compat mbi notion. They should
be split apart, which would aid the clarity of the modules change.
> Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
> ---
> xen/arch/x86/boot/x86_64.S | 10 +--
> xen/arch/x86/boot_info.c | 27 +++-----
> xen/arch/x86/domain_build.c | 24 +++----
> xen/arch/x86/efi/efi-boot.h | 30 +++------
> xen/arch/x86/microcode.c | 39 +++++------
> xen/arch/x86/setup.c | 140 +++++++++++++++++++--------------------
> xen/common/efi/boot.c | 1 -
> xen/common/efi/runtime.c | 4 ++
> xen/include/asm-x86/boot_info.h | 7 ++
> xen/include/asm-x86/setup.h | 10 +--
> xen/include/xsm/xsm.h | 16 +++--
> xen/xsm/xsm_core.c | 6 +-
> xen/xsm/xsm_policy.c | 10 +--
> 13 files changed, 153 insertions(+), 171 deletions(-)
>
> diff --git a/xen/arch/x86/boot/x86_64.S b/xen/arch/x86/boot/x86_64.S
> index 2305b56..500e0d7 100644
> --- a/xen/arch/x86/boot/x86_64.S
> +++ b/xen/arch/x86/boot/x86_64.S
> @@ -29,18 +29,12 @@
> test %ebx,%ebx
> jnz start_secondary
>
> - /* Initialize the Multiboot info struct. */
> - mov mbd_pa(%rip),%edi
> - call __init_mbi
> - pushq %rax
> -
> /* Initialize the boot_info. */
> mov mbd_pa(%rip),%edi
Thinking about this, having got this far through the series. Why cant
__init_boot_inf() be a parameterless function? mbd is available to it
straight from C, which would reduce the amount of parameter-passing in
asm code.
> call __init_boot_info
>
> - /* Pass off the Multiboot info struct and the boot_info to C land. */
> - popq %rdi
> - movq %rax,%rsi
> + /* Pass off the boot_info to C land. */
> + movq %rax,%rdi
> call __start_xen
> ud2 /* Force a panic (invalid opcode). */
>
> diff --git a/xen/arch/x86/boot_info.c b/xen/arch/x86/boot_info.c
> index 9e4af78..081453e 100644
> --- a/xen/arch/x86/boot_info.c
> +++ b/xen/arch/x86/boot_info.c
> @@ -37,8 +37,6 @@ extern struct e820entry e820map[];
> extern unsigned int e820nr;
> extern unsigned int lowmem_kb, highmem_kb;
>
> -static multiboot_info_t __read_mostly mbi;
> -
> static boot_info_t __read_mostly boot_info_mb = {
> .boot_loader_name = "UNKNOWN",
> .cmdline = NULL,
> @@ -46,6 +44,8 @@ static boot_info_t __read_mostly boot_info_mb = {
> .mem_upper = 0,
> .e820map_nr = 0,
> .e820map = NULL,
> + .mods_nr = 0,
> + .mods = NULL,
> .warn_msg = NULL,
> .err_msg = NULL
> };
> @@ -130,26 +130,12 @@ static void __init init_mmap(boot_info_t *boot_info, mbd_t *mbd)
> boot_info->e820map = e820_raw;
> }
>
> -unsigned long __init __init_mbi(u32 mbd_pa)
> +boot_info_t __init *__init_boot_info(u32 mbd_pa)
> {
> mbd_t *mbd = __va(mbd_pa);
>
> enable_bsp_exception_support();
>
> - if ( mbd->mods_nr )
> - {
> - mbi.flags |= MBI_MODULES;
> - mbi.mods_count = mbd->mods_nr;
> - mbi.mods_addr = mbd->mods;
> - }
> -
> - return (unsigned long)&mbi;
> -}
> -
> -boot_info_t __init *__init_boot_info(u32 mbd_pa)
> -{
> - mbd_t *mbd = __va(mbd_pa);
> -
> if ( mbd->boot_loader_name )
> boot_info_mb.boot_loader_name = __va(mbd->boot_loader_name);
>
> @@ -158,5 +144,12 @@ boot_info_t __init *__init_boot_info(u32 mbd_pa)
>
> init_mmap(&boot_info_mb, mbd);
>
> + if ( boot_info_mb.err_msg )
> + goto err;
> +
> + boot_info_mb.mods_nr = mbd->mods_nr;
> + boot_info_mb.mods = __va(mbd->mods);
> +
> +err:
> return &boot_info_mb;
> }
> diff --git a/xen/arch/x86/domain_build.c b/xen/arch/x86/domain_build.c
> index 7a6afea..2737b06 100644
> --- a/xen/arch/x86/domain_build.c
> +++ b/xen/arch/x86/domain_build.c
> @@ -751,9 +751,9 @@ static __init void setup_pv_physmap(struct domain *d, unsigned long pgtbl_pfn,
>
> int __init construct_dom0(
> struct domain *d,
> - const module_t *image, unsigned long image_headroom,
> - module_t *initrd,
> - void *(*bootstrap_map)(const module_t *),
> + const boot_module_t *image, unsigned long image_headroom,
> + boot_module_t *initrd,
> + void *(*bootstrap_map)(const boot_module_t *),
> char *cmdline)
> {
> int i, cpu, rc, compatible, compat32, order, machine;
> @@ -770,9 +770,9 @@ int __init construct_dom0(
> struct vcpu *v = d->vcpu[0];
> unsigned long long value;
> char *image_base = bootstrap_map(image);
> - unsigned long image_len = image->mod_end;
> + unsigned long image_len = image->end;
> char *image_start = image_base + image_headroom;
> - unsigned long initrd_len = initrd ? initrd->mod_end : 0;
> + unsigned long initrd_len = initrd ? initrd->end : 0;
> l4_pgentry_t *l4tab = NULL, *l4start = NULL;
> l3_pgentry_t *l3tab = NULL, *l3start = NULL;
> l2_pgentry_t *l2tab = NULL, *l2start = NULL;
> @@ -987,7 +987,7 @@ int __init construct_dom0(
> initrd_pfn = vinitrd_start ?
> (vinitrd_start - v_start) >> PAGE_SHIFT :
> d->tot_pages;
> - initrd_mfn = mfn = initrd->mod_start;
> + initrd_mfn = mfn = initrd->start;
> count = PFN_UP(initrd_len);
> if ( d->arch.physaddr_bitsize &&
> ((mfn + count - 1) >> (d->arch.physaddr_bitsize - PAGE_SHIFT)) )
> @@ -1002,12 +1002,12 @@ int __init construct_dom0(
> free_domheap_pages(page, order);
> page += 1UL << order;
> }
> - memcpy(page_to_virt(page), mfn_to_virt(initrd->mod_start),
> + memcpy(page_to_virt(page), mfn_to_virt(initrd->start),
> initrd_len);
> - mpt_alloc = (paddr_t)initrd->mod_start << PAGE_SHIFT;
> + mpt_alloc = (paddr_t)initrd->start << PAGE_SHIFT;
> init_domheap_pages(mpt_alloc,
> mpt_alloc + PAGE_ALIGN(initrd_len));
> - initrd->mod_start = initrd_mfn = page_to_mfn(page);
> + initrd->start = initrd_mfn = page_to_mfn(page);
> }
> else
> {
> @@ -1015,7 +1015,7 @@ int __init construct_dom0(
> if ( assign_pages(d, mfn_to_page(mfn++), 0, 0) )
> BUG();
> }
> - initrd->mod_end = 0;
> + initrd->end = 0;
> }
>
> printk("PHYSICAL MEMORY ARRANGEMENT:\n"
> @@ -1026,7 +1026,7 @@ int __init construct_dom0(
> nr_pages - d->tot_pages);
> if ( initrd )
> {
> - mpt_alloc = (paddr_t)initrd->mod_start << PAGE_SHIFT;
> + mpt_alloc = (paddr_t)initrd->start << PAGE_SHIFT;
> printk("\n Init. ramdisk: %"PRIpaddr"->%"PRIpaddr,
> mpt_alloc, mpt_alloc + initrd_len);
> }
> @@ -1281,7 +1281,7 @@ int __init construct_dom0(
> if ( pfn >= initrd_pfn )
> {
> if ( pfn < initrd_pfn + PFN_UP(initrd_len) )
> - mfn = initrd->mod_start + (pfn - initrd_pfn);
> + mfn = initrd->start + (pfn - initrd_pfn);
> else
> mfn -= PFN_UP(initrd_len);
> }
> diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
> index 96e758c..d8b30c1 100644
> --- a/xen/arch/x86/efi/efi-boot.h
> +++ b/xen/arch/x86/efi/efi-boot.h
> @@ -10,10 +10,6 @@
> #include <asm/processor.h>
>
> static struct file __initdata ucode;
> -static multiboot_info_t __initdata mbi = {
> - .flags = MBI_MODULES
> -};
> -static module_t __initdata mb_modules[3];
>
> static void __init edd_put_string(u8 *dst, size_t n, const char *src)
> {
> @@ -254,8 +250,7 @@ static void __init noreturn efi_arch_post_exit_boot(void)
> [cs] "ir" (__HYPERVISOR_CS),
> [ds] "r" (__HYPERVISOR_DS),
> [stkoff] "i" (STACK_SIZE - sizeof(struct cpu_info)),
> - "D" (__va(&mbi))
> - "S" (__va(&boot_info_efi))
> + "D" (__va(&boot_info_efi))
> : "memory" );
> for( ; ; ); /* not reached */
> }
> @@ -273,7 +268,7 @@ static void __init efi_arch_cfg_file_late(EFI_FILE_HANDLE dir_handle, char *sect
> name.s = get_value(&cfg, "global", "ucode");
> if ( name.s )
> {
> - microcode_set_module(mbi.mods_count);
> + microcode_set_module(boot_info_efi.mods_nr);
> split_string(name.s);
> read_file(dir_handle, s2w(&name), &ucode, NULL);
> efi_bs->FreePool(name.w);
> @@ -303,12 +298,6 @@ static void __init efi_arch_handle_cmdline(CHAR16 *image_name,
> else
> name.s = "xen";
> place_string_char(&boot_info_efi.cmdline, name.s);
> -
> - /*
> - * These must not be initialized statically, since the value must
> - * not get relocated when processing base relocations later.
> - */
> - mbi.mods_addr = (long)mb_modules;
> }
>
> static void __init efi_arch_edd(void)
> @@ -593,16 +582,17 @@ static void __init efi_arch_handle_module(struct file *file, const CHAR16 *name,
>
> /*
> * If options are provided, put them in
> - * mb_modules[mbi.mods_count].string after the filename, with a space
> - * separating them. place_string_u32() prepends strings and adds separating
> + * boot_info_efi.mods[boot_info_efi.mods_nr].cmdline
> + * after the filename, with a space separating them.
> + * place_string_u32() prepends strings and adds separating
> * spaces, so the call order is reversed.
> */
> if ( options )
> - place_string_u32(&mb_modules[mbi.mods_count].string, options);
> - place_string_u32(&mb_modules[mbi.mods_count].string, local_name.s);
> - mb_modules[mbi.mods_count].mod_start = file->addr >> PAGE_SHIFT;
> - mb_modules[mbi.mods_count].mod_end = file->size;
> - ++mbi.mods_count;
> + place_string_u32(&boot_info_efi.mods[boot_info_efi.mods_nr].cmdline, options);
> + place_string_u32(&boot_info_efi.mods[boot_info_efi.mods_nr].cmdline, local_name.s);
> + boot_info_efi.mods[boot_info_efi.mods_nr].start = file->addr >> PAGE_SHIFT;
> + boot_info_efi.mods[boot_info_efi.mods_nr].end = file->size;
end is surely start + size (give or take a fencepost) ? How did the
pre-existing code work?
> + ++boot_info_efi.mods_nr;
> efi_bs->FreePool(ptr);
> }
>
> diff --git a/xen/arch/x86/microcode.c b/xen/arch/x86/microcode.c
> index 091d5d1..0293eba 100644
> --- a/xen/arch/x86/microcode.c
> +++ b/xen/arch/x86/microcode.c
> @@ -40,8 +40,8 @@
> #include <asm/setup.h>
> #include <asm/microcode.h>
>
> -static module_t __initdata ucode_mod;
> -static void *(*__initdata ucode_mod_map)(const module_t *);
> +static boot_module_t __initdata ucode_mod;
> +static void *(*__initdata ucode_mod_map)(const boot_module_t *);
> static signed int __initdata ucode_mod_idx;
> static bool_t __initdata ucode_mod_forced;
> static cpumask_t __initdata init_mask;
> @@ -94,10 +94,9 @@ custom_param("ucode", parse_ucode);
>
> void __init microcode_scan_module(
> unsigned long *module_map,
> - const multiboot_info_t *mbi,
> - void *(*bootmap)(const module_t *))
> + const boot_info_t *boot_info,
> + void *(*bootmap)(const boot_module_t *))
> {
> - module_t *mod = (module_t *)__va(mbi->mods_addr);
> uint64_t *_blob_start;
> unsigned long _blob_size;
> struct cpio_data cd;
> @@ -119,13 +118,13 @@ void __init microcode_scan_module(
> /*
> * Try all modules and see whichever could be the microcode blob.
> */
> - for ( i = 1 /* Ignore dom0 kernel */; i < mbi->mods_count; i++ )
> + for ( i = 1 /* Ignore dom0 kernel */; i < boot_info->mods_nr; i++ )
> {
> if ( !test_bit(i, module_map) )
> continue;
>
> - _blob_start = bootmap(&mod[i]);
> - _blob_size = mod[i].mod_end;
> + _blob_start = bootmap(&boot_info->mods[i]);
> + _blob_size = boot_info->mods[i].end;
> if ( !_blob_start )
> {
> printk("Could not map multiboot module #%d (size: %ld)\n",
> @@ -165,21 +164,19 @@ err:
> }
> void __init microcode_grab_module(
> unsigned long *module_map,
> - const multiboot_info_t *mbi,
> - void *(*map)(const module_t *))
> + const boot_info_t *boot_info,
> + void *(*map)(const boot_module_t *))
> {
> - module_t *mod = (module_t *)__va(mbi->mods_addr);
> -
> if ( ucode_mod_idx < 0 )
> - ucode_mod_idx += mbi->mods_count;
> - if ( ucode_mod_idx <= 0 || ucode_mod_idx >= mbi->mods_count ||
> + ucode_mod_idx += boot_info->mods_nr;
> + if ( ucode_mod_idx <= 0 || ucode_mod_idx >= boot_info->mods_nr ||
> !__test_and_clear_bit(ucode_mod_idx, module_map) )
> goto scan;
> - ucode_mod = mod[ucode_mod_idx];
> + ucode_mod = boot_info->mods[ucode_mod_idx];
> ucode_mod_map = map;
> scan:
> if ( ucode_scan )
> - microcode_scan_module(module_map, mbi, map);
> + microcode_scan_module(module_map, boot_info, map);
> }
>
> const struct microcode_ops *microcode_ops;
> @@ -345,7 +342,7 @@ int microcode_update(XEN_GUEST_HANDLE_PARAM(const_void) buf, unsigned long len)
> static void __init _do_microcode_update(unsigned long data)
> {
> void *_data = (void *)data;
> - size_t len = ucode_blob.size ? ucode_blob.size : ucode_mod.mod_end;
> + size_t len = ucode_blob.size ? ucode_blob.size : ucode_mod.end;
>
> microcode_update_cpu(_data, len);
> cpumask_set_cpu(smp_processor_id(), &init_mask);
> @@ -360,7 +357,7 @@ static int __init microcode_init(void)
> if ( !microcode_ops )
> return 0;
>
> - if ( !ucode_mod.mod_end && !ucode_blob.size )
> + if ( !ucode_mod.end && !ucode_blob.size )
> return 0;
>
> data = ucode_blob.size ? ucode_blob.data : ucode_mod_map(&ucode_mod);
> @@ -414,7 +411,7 @@ static int __init microcode_presmp_init(void)
> {
> if ( microcode_ops )
> {
> - if ( ucode_mod.mod_end || ucode_blob.size )
> + if ( ucode_mod.end || ucode_blob.size )
> {
> void *data;
> size_t len;
> @@ -427,7 +424,7 @@ static int __init microcode_presmp_init(void)
> }
> else
> {
> - len = ucode_mod.mod_end;
> + len = ucode_mod.end;
> data = ucode_mod_map(&ucode_mod);
> }
> if ( data )
> @@ -447,7 +444,7 @@ static int __init microcode_presmp_init(void)
> ucode_blob.data = NULL;
> }
> else
> - ucode_mod.mod_end = 0;
> + ucode_mod.end = 0;
> }
> }
>
> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index 32d9a3a..6417419 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -12,7 +12,6 @@
> #include <xen/console.h>
> #include <xen/serial.h>
> #include <xen/trace.h>
> -#include <xen/multiboot.h>
> #include <xen/domain_page.h>
> #include <xen/version.h>
> #include <xen/gdbstub.h>
> @@ -140,7 +139,7 @@ static void __init parse_acpi_param(char *s)
> }
> }
>
> -static const module_t *__initdata initial_images;
> +static const boot_module_t *__initdata initial_images;
> static unsigned int __initdata nr_initial_images;
>
> unsigned long __init initial_images_nrpages(void)
> @@ -149,7 +148,7 @@ unsigned long __init initial_images_nrpages(void)
> unsigned int i;
>
> for ( nr = i = 0; i < nr_initial_images; ++i )
> - nr += PFN_UP(initial_images[i].mod_end);
> + nr += PFN_UP(initial_images[i].end);
>
> return nr;
> }
> @@ -160,10 +159,10 @@ void __init discard_initial_images(void)
>
> for ( i = 0; i < nr_initial_images; ++i )
> {
> - uint64_t start = (uint64_t)initial_images[i].mod_start << PAGE_SHIFT;
> + uint64_t start = (uint64_t)initial_images[i].start << PAGE_SHIFT;
>
> init_domheap_pages(start,
> - start + PAGE_ALIGN(initial_images[i].mod_end));
> + start + PAGE_ALIGN(initial_images[i].end));
> }
>
> nr_initial_images = 0;
> @@ -264,14 +263,14 @@ static void __init normalise_cpu_order(void)
> * Ensure a given physical memory range is present in the bootstrap mappings.
> * Use superpage mappings to ensure that pagetable memory needn't be allocated.
> */
> -static void *__init bootstrap_map(const module_t *mod)
> +static void *__init bootstrap_map(const boot_module_t *mod)
> {
> static unsigned long __initdata map_cur = BOOTSTRAP_MAP_BASE;
> uint64_t start, end, mask = (1L << L2_PAGETABLE_SHIFT) - 1;
> void *ret;
>
> if ( system_state != SYS_STATE_early_boot )
> - return mod ? mfn_to_virt(mod->mod_start) : NULL;
> + return mod ? mfn_to_virt(mod->start) : NULL;
>
> if ( !mod )
> {
> @@ -280,8 +279,8 @@ static void *__init bootstrap_map(const module_t *mod)
> return NULL;
> }
>
> - start = (uint64_t)mod->mod_start << PAGE_SHIFT;
> - end = start + mod->mod_end;
> + start = (uint64_t)mod->start << PAGE_SHIFT;
> + end = start + mod->end;
> if ( start >= end )
> return NULL;
>
> @@ -311,25 +310,25 @@ static void *__init move_memory(
>
> while ( size )
> {
> - module_t mod;
> + boot_module_t mod;
> unsigned int soffs = src & mask;
> unsigned int doffs = dst & mask;
> unsigned int sz;
> void *d, *s;
>
> - mod.mod_start = (src - soffs) >> PAGE_SHIFT;
> - mod.mod_end = soffs + size;
> - if ( mod.mod_end > blksz )
> - mod.mod_end = blksz;
> - sz = mod.mod_end - soffs;
> + mod.start = (src - soffs) >> PAGE_SHIFT;
> + mod.end = soffs + size;
> + if ( mod.end > blksz )
> + mod.end = blksz;
> + sz = mod.end - soffs;
> s = bootstrap_map(&mod);
>
> - mod.mod_start = (dst - doffs) >> PAGE_SHIFT;
> - mod.mod_end = doffs + size;
> - if ( mod.mod_end > blksz )
> - mod.mod_end = blksz;
> - if ( sz > mod.mod_end - doffs )
> - sz = mod.mod_end - doffs;
> + mod.start = (dst - doffs) >> PAGE_SHIFT;
> + mod.end = doffs + size;
> + if ( mod.end > blksz )
> + mod.end = blksz;
> + if ( sz > mod.end - doffs )
> + sz = mod.end - doffs;
> d = bootstrap_map(&mod);
>
> memmove(d + doffs, s + soffs, sz);
> @@ -348,7 +347,7 @@ static void *__init move_memory(
> }
>
> static uint64_t __init consider_modules(
> - uint64_t s, uint64_t e, uint32_t size, const module_t *mod,
> + uint64_t s, uint64_t e, uint32_t size, const boot_module_t *mod,
> unsigned int nr_mods, unsigned int this_mod)
> {
> unsigned int i;
> @@ -358,8 +357,8 @@ static uint64_t __init consider_modules(
>
> for ( i = 0; i < nr_mods ; ++i )
> {
> - uint64_t start = (uint64_t)mod[i].mod_start << PAGE_SHIFT;
> - uint64_t end = start + PAGE_ALIGN(mod[i].mod_end);
> + uint64_t start = (uint64_t)mod[i].start << PAGE_SHIFT;
> + uint64_t end = start + PAGE_ALIGN(mod[i].end);
>
> if ( i == this_mod )
> continue;
> @@ -549,12 +548,10 @@ void __init enable_bsp_exception_support(void)
> sort_exception_tables();
> }
>
> -void __init noreturn __start_xen(unsigned long mbi_p, boot_info_t *boot_info_ptr)
> +void __init noreturn __start_xen(boot_info_t *boot_info_ptr)
> {
> char *cmdline, *kextra;
> unsigned int initrdidx, domcr_flags = DOMCRF_s3_integrity;
> - multiboot_info_t *mbi = (multiboot_info_t *)mbi_p;
> - module_t *mod = (module_t *)__va(mbi->mods_addr);
> unsigned long nr_pages, raw_max_page, modules_headroom, *module_map;
> int i, j;
> bool_t acpi_boot_table_init_done = 0;
> @@ -672,7 +669,7 @@ void __init noreturn __start_xen(unsigned long mbi_p, boot_info_t *boot_info_ptr
> bootsym(boot_edd_info_nr));
>
> /* Check that we have at least one Multiboot module. */
> - if ( !(mbi->flags & MBI_MODULES) || (mbi->mods_count == 0) )
> + if ( !boot_info->mods_nr )
> panic("dom0 kernel not specified. Check bootloader configuration.");
>
> if ( ((unsigned long)cpu0_stack & (STACK_SIZE-1)) != 0 )
> @@ -707,8 +704,8 @@ void __init noreturn __start_xen(unsigned long mbi_p, boot_info_t *boot_info_ptr
> set_kexec_crash_area_size((u64)nr_pages << PAGE_SHIFT);
> kexec_reserve_area(&boot_e820);
>
> - initial_images = mod;
> - nr_initial_images = mbi->mods_count;
> + nr_initial_images = boot_info->mods_nr;
> + initial_images = boot_info->mods;
>
> /*
> * Iterate backwards over all superpage-aligned RAM regions.
> @@ -723,16 +720,15 @@ void __init noreturn __start_xen(unsigned long mbi_p, boot_info_t *boot_info_ptr
> * we can relocate the dom0 kernel and other multiboot modules. Also, on
> * x86/64, we relocate Xen to higher memory.
> */
> - for ( i = 0; !efi_enabled && i < mbi->mods_count; i++ )
> + for ( i = 0; !efi_enabled && i < boot_info->mods_nr; i++ )
> {
> - if ( mod[i].mod_start & (PAGE_SIZE - 1) )
> + if ( boot_info->mods[i].start & (PAGE_SIZE - 1) )
> panic("Bootloader didn't honor module alignment request.");
> - mod[i].mod_end -= mod[i].mod_start;
> - mod[i].mod_start >>= PAGE_SHIFT;
> - mod[i].reserved = 0;
> + boot_info->mods[i].end -= boot_info->mods[i].start;
> + boot_info->mods[i].start >>= PAGE_SHIFT;
> }
>
> - modules_headroom = bzimage_headroom(bootstrap_map(mod), mod->mod_end);
> + modules_headroom = bzimage_headroom(bootstrap_map(boot_info->mods), boot_info->mods->end);
The old code was playing a little fast-and-loose with pointers vs arrays.
Please convert to the more-appropriate mods[0] as you are transforming
the code anyway, which helps highlight the bzimage_headroom() check
applies to the dom0 kernel only.
> bootstrap_map(NULL);
>
> #ifndef highmem_start
> @@ -773,7 +769,7 @@ void __init noreturn __start_xen(unsigned long mbi_p, boot_info_t *boot_info_ptr
> {
> /* Don't overlap with modules. */
> end = consider_modules(s, e, reloc_size + mask,
> - mod, mbi->mods_count, -1);
> + boot_info->mods, boot_info->mods_nr, -1);
> end &= ~mask;
> }
> else
> @@ -861,36 +857,36 @@ void __init noreturn __start_xen(unsigned long mbi_p, boot_info_t *boot_info_ptr
> }
>
> /* Is the region suitable for relocating the multiboot modules? */
> - for ( j = mbi->mods_count - 1; j >= 0; j-- )
> + for ( j = boot_info->mods_nr - 1; j >= 0; j-- )
> {
> unsigned long headroom = j ? 0 : modules_headroom;
> - unsigned long size = PAGE_ALIGN(headroom + mod[j].mod_end);
> + unsigned long size = PAGE_ALIGN(headroom + boot_info->mods[j].end);
>
> - if ( mod[j].reserved )
> + if ( boot_info->mods[j].relocated )
> continue;
>
> /* Don't overlap with other modules. */
> - end = consider_modules(s, e, size, mod, mbi->mods_count, j);
> + end = consider_modules(s, e, size, boot_info->mods, boot_info->mods_nr, j);
>
> if ( highmem_start && end > highmem_start )
> continue;
>
> if ( s < end &&
> (headroom ||
> - ((end - size) >> PAGE_SHIFT) > mod[j].mod_start) )
> + ((end - size) >> PAGE_SHIFT) > boot_info->mods[j].start) )
> {
> move_memory(end - size + headroom,
> - (uint64_t)mod[j].mod_start << PAGE_SHIFT,
> - mod[j].mod_end, 0);
> - mod[j].mod_start = (end - size) >> PAGE_SHIFT;
> - mod[j].mod_end += headroom;
> - mod[j].reserved = 1;
> + (uint64_t)boot_info->mods[j].start << PAGE_SHIFT,
> + boot_info->mods[j].end, 0);
> + boot_info->mods[j].start = (end - size) >> PAGE_SHIFT;
> + boot_info->mods[j].end += headroom;
> + boot_info->mods[j].relocated = 1;
> }
> }
>
> /* Don't overlap with modules. */
> e = consider_modules(s, e, PAGE_ALIGN(kexec_crash_area.size),
> - mod, mbi->mods_count, -1);
> + boot_info->mods, boot_info->mods_nr, -1);
> if ( !kexec_crash_area.start && (s < e) )
> {
> e = (e - kexec_crash_area.size) & PAGE_MASK;
> @@ -898,18 +894,18 @@ void __init noreturn __start_xen(unsigned long mbi_p, boot_info_t *boot_info_ptr
> }
> }
>
> - if ( modules_headroom && !mod->reserved )
> + if ( modules_headroom && !boot_info->mods->relocated )
> panic("Not enough memory to relocate the dom0 kernel image.");
> - for ( i = 0; i < mbi->mods_count; ++i )
> + for ( i = 0; i < boot_info->mods_nr; ++i )
> {
> - uint64_t s = (uint64_t)mod[i].mod_start << PAGE_SHIFT;
> + uint64_t s = (uint64_t)boot_info->mods[i].start << PAGE_SHIFT;
>
> - reserve_e820_ram(&boot_e820, s, s + PAGE_ALIGN(mod[i].mod_end));
> + reserve_e820_ram(&boot_e820, s, s + PAGE_ALIGN(boot_info->mods[i].end));
> }
>
> if ( !xen_phys_start )
> panic("Not enough memory to relocate Xen.");
> - reserve_e820_ram(&boot_e820, efi_enabled ? mbi->mem_upper : __pa(&_start),
> + reserve_e820_ram(&boot_e820, efi_enabled ? boot_info->mem_upper : __pa(&_start),
> __pa(&_end));
>
> /* Late kexec reservation (dynamic start address). */
> @@ -955,10 +951,10 @@ void __init noreturn __start_xen(unsigned long mbi_p, boot_info_t *boot_info_ptr
> ASSERT(j);
> }
> map_e = boot_e820.map[j].addr + boot_e820.map[j].size;
> - for ( j = 0; j < mbi->mods_count; ++j )
> + for ( j = 0; j < boot_info->mods_nr; ++j )
> {
> - uint64_t end = pfn_to_paddr(mod[j].mod_start) +
> - mod[j].mod_end;
> + uint64_t end = pfn_to_paddr(boot_info->mods[j].start) +
> + boot_info->mods[j].end;
>
> if ( map_e < end )
> map_e = end;
> @@ -1031,13 +1027,13 @@ void __init noreturn __start_xen(unsigned long mbi_p, boot_info_t *boot_info_ptr
> }
> }
>
> - for ( i = 0; i < mbi->mods_count; ++i )
> + for ( i = 0; i < boot_info->mods_nr; ++i )
> {
> - set_pdx_range(mod[i].mod_start,
> - mod[i].mod_start + PFN_UP(mod[i].mod_end));
> - map_pages_to_xen((unsigned long)mfn_to_virt(mod[i].mod_start),
> - mod[i].mod_start,
> - PFN_UP(mod[i].mod_end), PAGE_HYPERVISOR);
> + set_pdx_range(boot_info->mods[i].start,
> + boot_info->mods[i].start + PFN_UP(boot_info->mods[i].end));
> + map_pages_to_xen((unsigned long)mfn_to_virt(boot_info->mods[i].start),
> + boot_info->mods[i].start,
> + PFN_UP(boot_info->mods[i].end), PAGE_HYPERVISOR);
> }
>
> if ( kexec_crash_area.size )
> @@ -1191,13 +1187,13 @@ void __init noreturn __start_xen(unsigned long mbi_p, boot_info_t *boot_info_ptr
>
> init_IRQ();
>
> - module_map = xmalloc_array(unsigned long, BITS_TO_LONGS(mbi->mods_count));
> - bitmap_fill(module_map, mbi->mods_count);
> + module_map = xmalloc_array(unsigned long, BITS_TO_LONGS(boot_info->mods_nr));
> + bitmap_fill(module_map, boot_info->mods_nr);
> __clear_bit(0, module_map); /* Dom0 kernel is always first */
>
> - xsm_multiboot_init(module_map, mbi, bootstrap_map);
> + xsm_multiboot_init(module_map, boot_info, bootstrap_map);
>
> - microcode_grab_module(module_map, mbi, bootstrap_map);
> + microcode_grab_module(module_map, boot_info, bootstrap_map);
>
> timer_init();
>
> @@ -1302,7 +1298,7 @@ void __init noreturn __start_xen(unsigned long mbi_p, boot_info_t *boot_info_ptr
> dom0->target = NULL;
>
> /* Grab the DOM0 command line. */
> - cmdline = (char *)(mod[0].string ? __va(mod[0].string) : NULL);
> + cmdline = (char *)(boot_info->mods[0].cmdline ? __va(boot_info->mods[0].cmdline) : NULL);
> if ( (cmdline != NULL) || (kextra != NULL) )
> {
> static char __initdata dom0_cmdline[MAX_GUEST_CMDLINE];
> @@ -1334,8 +1330,8 @@ void __init noreturn __start_xen(unsigned long mbi_p, boot_info_t *boot_info_ptr
> if ( xen_cpuidle )
> xen_processor_pmbits |= XEN_PROCESSOR_PM_CX;
>
> - initrdidx = find_first_bit(module_map, mbi->mods_count);
> - if ( bitmap_weight(module_map, mbi->mods_count) > 1 )
> + initrdidx = find_first_bit(module_map, boot_info->mods_nr);
> + if ( bitmap_weight(module_map, boot_info->mods_nr) > 1 )
> printk(XENLOG_WARNING
> "Multiple initrd candidates, picking module #%u\n",
> initrdidx);
> @@ -1352,9 +1348,9 @@ void __init noreturn __start_xen(unsigned long mbi_p, boot_info_t *boot_info_ptr
> * We're going to setup domain0 using the module(s) that we stashed safely
> * above our heap. The second module, if present, is an initrd ramdisk.
> */
> - if ( construct_dom0(dom0, mod, modules_headroom,
> - (initrdidx > 0) && (initrdidx < mbi->mods_count)
> - ? mod + initrdidx : NULL,
> + if ( construct_dom0(dom0, boot_info->mods, modules_headroom,
&boot_info->mods[0] please.
> + (initrdidx > 0) && (initrdidx < boot_info->mods_nr)
> + ? boot_info->mods + initrdidx : NULL,
> bootstrap_map, cmdline) != 0)
> panic("Could not set up DOM0 guest OS");
>
> diff --git a/xen/common/efi/boot.c b/xen/common/efi/boot.c
> index f272171..43f2939 100644
> --- a/xen/common/efi/boot.c
> +++ b/xen/common/efi/boot.c
> @@ -10,7 +10,6 @@
> #include <xen/keyhandler.h>
> #include <xen/lib.h>
> #include <xen/mm.h>
> -#include <xen/multiboot.h>
> #include <xen/pci_regs.h>
> #include <xen/pfn.h>
> #if EFI_PAGE_SIZE != PAGE_SIZE
> diff --git a/xen/common/efi/runtime.c b/xen/common/efi/runtime.c
> index 03c6658..1b786a1 100644
> --- a/xen/common/efi/runtime.c
> +++ b/xen/common/efi/runtime.c
> @@ -56,6 +56,8 @@ const struct efi_pci_rom *__read_mostly efi_pci_roms;
> #ifndef CONFIG_ARM /* TODO - disabled until implemented on ARM */
> extern struct e820entry e820map[];
>
> +static boot_module_t __read_mostly boot_info_mods[3] = {};
You can drop the braces, as this is static.
~Andrew
> +
> boot_info_t __read_mostly boot_info_efi = {
> .boot_loader_name = "EFI",
> .cmdline = NULL,
> @@ -63,6 +65,8 @@ boot_info_t __read_mostly boot_info_efi = {
> .mem_upper = 0,
> .e820map_nr = 0,
> .e820map = e820map,
> + .mods_nr = 0,
> + .mods = boot_info_mods,
> .warn_msg = NULL,
> .err_msg = NULL
> };
> diff --git a/xen/include/asm-x86/boot_info.h b/xen/include/asm-x86/boot_info.h
> index a882c0c..c0c92b4 100644
> --- a/xen/include/asm-x86/boot_info.h
> +++ b/xen/include/asm-x86/boot_info.h
> @@ -22,6 +22,7 @@
> #include <xen/types.h>
>
> #include <asm/e820.h>
> +#include <asm/mbd.h>
>
> /*
> * Define boot_info type. It will be used to define variable which in turn
> @@ -58,6 +59,12 @@ typedef struct {
> */
> struct e820entry *e820map;
>
> + /* Number of modules. */
> + unsigned int mods_nr;
> +
> + /* Pointer to modules description. */
> + boot_module_t *mods;
> +
> /*
> * Info about warning occurred during boot_info initialization.
> * NULL if everything went OK.
> diff --git a/xen/include/asm-x86/setup.h b/xen/include/asm-x86/setup.h
> index d051ee6..f60cf41 100644
> --- a/xen/include/asm-x86/setup.h
> +++ b/xen/include/asm-x86/setup.h
> @@ -1,7 +1,7 @@
> #ifndef __X86_SETUP_H_
> #define __X86_SETUP_H_
>
> -#include <xen/multiboot.h>
> +#include <asm/boot_info.h>
>
> extern unsigned long xenheap_initial_phys_start;
>
> @@ -29,9 +29,9 @@ void vesa_mtrr_init(void);
>
> int construct_dom0(
> struct domain *d,
> - const module_t *kernel, unsigned long kernel_headroom,
> - module_t *initrd,
> - void *(*bootstrap_map)(const module_t *),
> + const boot_module_t *kernel, unsigned long kernel_headroom,
> + boot_module_t *initrd,
> + void *(*bootstrap_map)(const boot_module_t *),
> char *cmdline);
>
> unsigned long initial_images_nrpages(void);
> @@ -40,7 +40,7 @@ void discard_initial_images(void);
> int xen_in_range(unsigned long mfn);
>
> void microcode_grab_module(
> - unsigned long *, const multiboot_info_t *, void *(*)(const module_t *));
> + unsigned long *, const boot_info_t *, void *(*)(const boot_module_t *));
>
> extern uint8_t kbd_shift_flags;
>
> diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
> index 4ce089f..9723e59 100644
> --- a/xen/include/xsm/xsm.h
> +++ b/xen/include/xsm/xsm.h
> @@ -16,7 +16,9 @@
> #define __XSM_H__
>
> #include <xen/sched.h>
> -#include <xen/multiboot.h>
> +#ifndef CONFIG_ARM /* TODO - disabled until implemented on ARM */
> +#include <asm/boot_info.h>
> +#endif
>
> typedef void xsm_op_t;
> DEFINE_XEN_GUEST_HANDLE(xsm_op_t);
> @@ -671,11 +673,11 @@ static inline int xsm_ioport_mapping (xsm_default_t def, struct domain *d, uint3
>
> #ifdef CONFIG_MULTIBOOT
> extern int xsm_multiboot_init(unsigned long *module_map,
> - const multiboot_info_t *mbi,
> - void *(*bootstrap_map)(const module_t *));
> + const boot_info_t *boot_info,
> + void *(*bootstrap_map)(const boot_module_t *));
> extern int xsm_multiboot_policy_init(unsigned long *module_map,
> - const multiboot_info_t *mbi,
> - void *(*bootstrap_map)(const module_t *));
> + const boot_info_t *boot_info,
> + void *(*bootstrap_map)(const boot_module_t *));
> #endif
>
> #ifdef HAS_DEVICE_TREE
> @@ -695,8 +697,8 @@ extern void xsm_fixup_ops(struct xsm_operations *ops);
>
> #ifdef CONFIG_MULTIBOOT
> static inline int xsm_multiboot_init (unsigned long *module_map,
> - const multiboot_info_t *mbi,
> - void *(*bootstrap_map)(const module_t *))
> + const boot_info_t *boot_info,
> + void *(*bootstrap_map)(const boot_module_t *))
> {
> return 0;
> }
> diff --git a/xen/xsm/xsm_core.c b/xen/xsm/xsm_core.c
> index 0ac6d03..39b7ff6 100644
> --- a/xen/xsm/xsm_core.c
> +++ b/xen/xsm/xsm_core.c
> @@ -60,8 +60,8 @@ static int __init xsm_core_init(void)
>
> #ifdef CONFIG_MULTIBOOT
> int __init xsm_multiboot_init(unsigned long *module_map,
> - const multiboot_info_t *mbi,
> - void *(*bootstrap_map)(const module_t *))
> + const boot_info_t *boot_info,
> + void *(*bootstrap_map)(const boot_module_t *))
> {
> int ret = 0;
>
> @@ -69,7 +69,7 @@ int __init xsm_multiboot_init(unsigned long *module_map,
>
> if ( XSM_MAGIC )
> {
> - ret = xsm_multiboot_policy_init(module_map, mbi, bootstrap_map);
> + ret = xsm_multiboot_policy_init(module_map, boot_info, bootstrap_map);
> if ( ret )
> {
> bootstrap_map(NULL);
> diff --git a/xen/xsm/xsm_policy.c b/xen/xsm/xsm_policy.c
> index 6e0bb78..aa30c21 100644
> --- a/xen/xsm/xsm_policy.c
> +++ b/xen/xsm/xsm_policy.c
> @@ -33,11 +33,11 @@ u32 __initdata policy_size = 0;
>
> #ifdef CONFIG_MULTIBOOT
> int __init xsm_multiboot_policy_init(unsigned long *module_map,
> - const multiboot_info_t *mbi,
> - void *(*bootstrap_map)(const module_t *))
> + const boot_info_t *boot_info,
> + void *(*bootstrap_map)(const boot_module_t *))
> {
> int i;
> - module_t *mod = (module_t *)__va(mbi->mods_addr);
> + boot_module_t *mod = (boot_module_t *)__va(boot_info->mods);
> int rc = 0;
> u32 *_policy_start;
> unsigned long _policy_len;
> @@ -46,13 +46,13 @@ int __init xsm_multiboot_policy_init(unsigned long *module_map,
> * Try all modules and see whichever could be the binary policy.
> * Adjust module_map for the module that is the binary policy.
> */
> - for ( i = mbi->mods_count-1; i >= 1; i-- )
> + for ( i = boot_info->mods_nr-1; i >= 1; i-- )
> {
> if ( !test_bit(i, module_map) )
> continue;
>
> _policy_start = bootstrap_map(mod + i);
> - _policy_len = mod[i].mod_end;
> + _policy_len = mod[i].end;
>
> if ( (xsm_magic_t)(*_policy_start) == XSM_MAGIC )
> {
next prev parent reply other threads:[~2014-10-17 22:35 UTC|newest]
Thread overview: 61+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-10-17 14:11 [PATCH for-xen-4.5 v4 00/18] xen: Break multiboot (v1) dependency and add multiboot2 support Daniel Kiper
2014-10-17 14:11 ` [PATCH for-xen-4.5 v4 01/18] xen/makefile: clean target should remove xen.efi binary Daniel Kiper
2014-10-17 14:11 ` [PATCH for-xen-4.5 v4 02/18] x86/boot: fix reloc.S build dependencies Daniel Kiper
2014-10-17 14:51 ` Jan Beulich
2014-10-17 16:10 ` Daniel Kiper
2014-10-17 16:22 ` Jan Beulich
2014-10-17 14:56 ` Andrew Cooper
2014-10-17 15:10 ` Jan Beulich
2014-10-17 14:11 ` [PATCH for-xen-4.5 v4 03/18] x86: define cmdline_cook() loader_name argument as a const Daniel Kiper
2014-10-17 14:11 ` [PATCH for-xen-4.5 v4 04/18] x86/boot: use constant in head.S instead of hardcoded value Daniel Kiper
2014-10-17 15:00 ` Andrew Cooper
2014-10-17 15:52 ` Daniel Kiper
2014-10-17 16:18 ` Jan Beulich
2014-10-17 16:22 ` Daniel Kiper
2014-10-20 8:00 ` Jan Beulich
2014-10-17 14:11 ` [PATCH for-xen-4.5 v4 05/18] x86/boot/reloc: create generic alloc and copy functions Daniel Kiper
2014-10-17 16:04 ` Andrew Cooper
2014-10-17 17:11 ` Daniel Kiper
2014-10-17 17:22 ` Andrew Cooper
2014-10-17 14:11 ` [PATCH for-xen-4.5 v4 06/18] x86: introduce MultiBoot Data (MBD) type Daniel Kiper
2014-10-17 17:14 ` Andrew Cooper
2014-10-17 14:12 ` [PATCH for-xen-4.5 v4 07/18] x86/efi: add place_string_u32() function Daniel Kiper
2014-10-17 14:12 ` [PATCH for-xen-4.5 v4 08/18] x86: introduce boot_info structure Daniel Kiper
2014-10-17 20:55 ` Andrew Cooper
2014-10-17 14:12 ` [PATCH for-xen-4.5 v4 09/18] x86: move boot_loader_name from mbi to boot_info Daniel Kiper
2014-10-17 21:05 ` Andrew Cooper
2014-10-17 14:12 ` [PATCH for-xen-4.5 v4 10/18] x86: move cmdline " Daniel Kiper
2014-10-17 21:27 ` Andrew Cooper
2014-10-17 14:12 ` [PATCH for-xen-4.5 v4 11/18] x86: move legacy BIOS memory map stuff " Daniel Kiper
2014-10-17 22:08 ` Andrew Cooper
2014-10-17 14:12 ` [PATCH for-xen-4.5 v4 12/18] x86: move modules data from mbi to boot_info and remove mbi Daniel Kiper
2014-10-17 22:35 ` Andrew Cooper [this message]
2014-10-20 8:38 ` Jan Beulich
2014-10-17 14:12 ` [PATCH for-xen-4.5 v4 13/18] x86: move EFI memory map stuff to boot_info Daniel Kiper
2014-10-17 14:12 ` [PATCH for-xen-4.5 v4 14/18] x86: move MPS, ACPI and SMBIOS data " Daniel Kiper
2014-10-17 22:51 ` Andrew Cooper
2014-10-17 14:12 ` [PATCH for-xen-4.5 v4 15/18] x86: move video " Daniel Kiper
2014-10-17 22:55 ` Andrew Cooper
2014-10-17 14:12 ` [PATCH for-xen-4.5 v4 16/18] x86: move HDD " Daniel Kiper
2014-10-17 22:57 ` Andrew Cooper
2014-10-17 14:12 ` [PATCH for-xen-4.5 v4 17/18] x86/boot: use %ecx instead of %eax Daniel Kiper
2014-10-17 14:12 ` [PATCH for-xen-4.5 v4 18/18] xen/x86: add multiboot2 protocol support Daniel Kiper
2014-10-17 23:13 ` Andrew Cooper
2014-10-17 14:42 ` [PATCH for-xen-4.5 v4 00/18] xen: Break multiboot (v1) dependency and add multiboot2 support Jan Beulich
2014-10-17 15:49 ` Daniel Kiper
2014-10-23 10:19 ` Jan Beulich
2014-10-23 11:08 ` Andrew Cooper
2014-10-23 14:57 ` Daniel Kiper
2014-10-23 15:26 ` Jan Beulich
2014-10-23 15:50 ` Daniel Kiper
2014-10-23 16:04 ` Jan Beulich
2014-10-23 17:55 ` konrad wilk
2014-10-24 9:09 ` Jan Beulich
2014-10-23 15:55 ` Andrew Cooper
2014-10-23 18:04 ` konrad wilk
2014-10-23 21:55 ` Andrew Cooper
2014-10-24 7:07 ` Daniel Kiper
2014-10-23 11:14 ` Stefano Stabellini
2014-10-23 11:33 ` Jan Beulich
2014-10-17 18:02 ` Roy Franz
2014-10-27 11:09 ` Daniel Kiper
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=544199C0.60600@citrix.com \
--to=andrew.cooper3@citrix.com \
--cc=daniel.kiper@oracle.com \
--cc=fu.wei@linaro.org \
--cc=gang.wei@intel.com \
--cc=ian.campbell@citrix.com \
--cc=jbeulich@suse.com \
--cc=jgross@suse.com \
--cc=keir@xen.org \
--cc=ning.sun@intel.com \
--cc=qiaowei.ren@intel.com \
--cc=richard.l.maliszewski@intel.com \
--cc=ross.philipson@citrix.com \
--cc=roy.franz@linaro.org \
--cc=stefano.stabellini@eu.citrix.com \
--cc=xen-devel@lists.xenproject.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.