| Message ID | 20241006214956.24339-8-dpsmith@apertussolutions.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Boot modules for Hyperlaunch | expand |
On 2024-10-06 17:49, Daniel P. Smith wrote: > The purpose of struct boot_module is to encapsulate the state of boot module as > it is processed by Xen. Locating boot module state struct boot_module reduces > the number of global variables as well as the number of state variables that > must be passed around. It also lays the groundwork for hyperlaunch mult-domain > construction, where multiple instances of state variables like headroom will be > needed. > > Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com> > --- > xen/arch/x86/include/asm/bootinfo.h | 5 +++++ > xen/arch/x86/setup.c | 23 ++++++++++++++--------- > 2 files changed, 19 insertions(+), 9 deletions(-) > > diff --git a/xen/arch/x86/include/asm/bootinfo.h b/xen/arch/x86/include/asm/bootinfo.h > index d19473d8941e..c7e6b4ebf0da 100644 > --- a/xen/arch/x86/include/asm/bootinfo.h > +++ b/xen/arch/x86/include/asm/bootinfo.h > @@ -17,6 +17,11 @@ > struct boot_module { > /* Transitionary only */ > module_t *mod; > + /* > + * A boot module may contain a compressed kernel that Xen will need space > + * reserved, into which it will be decompressed. Maybe "Extra space, before the module data, for compressed kernel modules to be decompressed into." And some ascii art could help: [ headroom ][ compressed data ] <decompression> [ decompressed data ] (Not sure how to create a down arrow...) > + */ > + unsigned long headroom; > }; > > /* > diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c > index ba9f110d98c6..dd82ca3d43e2 100644 > --- a/xen/arch/x86/setup.c > +++ b/xen/arch/x86/setup.c > @@ -1012,7 +1012,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p) > struct boot_info *bi; > multiboot_info_t *mbi; > module_t *mod; > - unsigned long nr_pages, raw_max_page, modules_headroom, module_map[1]; > + unsigned long nr_pages, raw_max_page, module_map[1]; > int i, j, e820_warn = 0, bytes = 0; > unsigned long eb_start, eb_end; > bool acpi_boot_table_init_done = false, relocated = false; > @@ -1371,7 +1371,10 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p) > mod[bi->nr_modules].mod_end = __2M_rwdata_end - _stext; > } > > - modules_headroom = bzimage_headroom(bootstrap_map(mod), mod->mod_end); > + bi->mods[0].headroom = > + bzimage_headroom(bootstrap_map(bi->mods[0].mod), > + bi->mods[0].mod->mod_end); > + > bootstrap_map(NULL); > > #ifndef highmem_start > @@ -1456,8 +1459,10 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p) > * decompressor overheads of mod[0] (the dom0 kernel). When we > * move mod[0], we incorporate this as extra space at the start. > */ > - unsigned long headroom = j ? 0 : modules_headroom; > - unsigned long size = PAGE_ALIGN(headroom + mod[j].mod_end); > + struct boot_module *bm = &bi->mods[j]; > + unsigned long size; > + > + size = PAGE_ALIGN(bm->headroom + mod[j].mod_end); Just do unsigned long size = PAGE_ALIGN(bm->headroom + mod[j].mod_end); ? > > if ( mod[j].reserved ) > continue; The rest looks good. Regards, Jason
On 10/7/24 14:55, Jason Andryuk wrote: > On 2024-10-06 17:49, Daniel P. Smith wrote: >> The purpose of struct boot_module is to encapsulate the state of boot >> module as >> it is processed by Xen. Locating boot module state struct boot_module >> reduces >> the number of global variables as well as the number of state >> variables that >> must be passed around. It also lays the groundwork for hyperlaunch >> mult-domain >> construction, where multiple instances of state variables like >> headroom will be >> needed. >> >> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com> >> --- >> xen/arch/x86/include/asm/bootinfo.h | 5 +++++ >> xen/arch/x86/setup.c | 23 ++++++++++++++--------- >> 2 files changed, 19 insertions(+), 9 deletions(-) >> >> diff --git a/xen/arch/x86/include/asm/bootinfo.h >> b/xen/arch/x86/include/asm/bootinfo.h >> index d19473d8941e..c7e6b4ebf0da 100644 >> --- a/xen/arch/x86/include/asm/bootinfo.h >> +++ b/xen/arch/x86/include/asm/bootinfo.h >> @@ -17,6 +17,11 @@ >> struct boot_module { >> /* Transitionary only */ >> module_t *mod; >> + /* >> + * A boot module may contain a compressed kernel that Xen will >> need space >> + * reserved, into which it will be decompressed. > > Maybe "Extra space, before the module data, for compressed kernel > modules to be decompressed into." I will rework it with your suggestions. > And some ascii art could help: > > [ headroom ][ compressed data ] > <decompression> > [ decompressed data ] > > (Not sure how to create a down arrow...) Yes, in fact I would just show the three states like this: At boot: [ compressed kernel ] After boot module relocation: [ estimated headroom + PAGE_SIZE rounding ][ compressed kernel ] After kernel decompression: [ decompressed kernel ][ unused rounding ] >> + */ >> + unsigned long headroom; >> }; >> /* >> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c >> index ba9f110d98c6..dd82ca3d43e2 100644 >> --- a/xen/arch/x86/setup.c >> +++ b/xen/arch/x86/setup.c >> @@ -1012,7 +1012,7 @@ void asmlinkage __init noreturn >> __start_xen(unsigned long mbi_p) >> struct boot_info *bi; >> multiboot_info_t *mbi; >> module_t *mod; >> - unsigned long nr_pages, raw_max_page, modules_headroom, >> module_map[1]; >> + unsigned long nr_pages, raw_max_page, module_map[1]; >> int i, j, e820_warn = 0, bytes = 0; >> unsigned long eb_start, eb_end; >> bool acpi_boot_table_init_done = false, relocated = false; >> @@ -1371,7 +1371,10 @@ void asmlinkage __init noreturn >> __start_xen(unsigned long mbi_p) >> mod[bi->nr_modules].mod_end = __2M_rwdata_end - _stext; >> } >> - modules_headroom = bzimage_headroom(bootstrap_map(mod), >> mod->mod_end); >> + bi->mods[0].headroom = >> + bzimage_headroom(bootstrap_map(bi->mods[0].mod), >> + bi->mods[0].mod->mod_end); >> + >> bootstrap_map(NULL); >> #ifndef highmem_start >> @@ -1456,8 +1459,10 @@ void asmlinkage __init noreturn >> __start_xen(unsigned long mbi_p) >> * decompressor overheads of mod[0] (the dom0 kernel). >> When we >> * move mod[0], we incorporate this as extra space at >> the start. >> */ >> - unsigned long headroom = j ? 0 : modules_headroom; >> - unsigned long size = PAGE_ALIGN(headroom + mod[j].mod_end); >> + struct boot_module *bm = &bi->mods[j]; >> + unsigned long size; >> + >> + size = PAGE_ALIGN(bm->headroom + mod[j].mod_end); > > Just do > unsigned long size = PAGE_ALIGN(bm->headroom + > mod[j].mod_end); > ? yep, not sure why I even split it. v/r, dps
diff --git a/xen/arch/x86/include/asm/bootinfo.h b/xen/arch/x86/include/asm/bootinfo.h index d19473d8941e..c7e6b4ebf0da 100644 --- a/xen/arch/x86/include/asm/bootinfo.h +++ b/xen/arch/x86/include/asm/bootinfo.h @@ -17,6 +17,11 @@ struct boot_module { /* Transitionary only */ module_t *mod; + /* + * A boot module may contain a compressed kernel that Xen will need space + * reserved, into which it will be decompressed. + */ + unsigned long headroom; }; /* diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c index ba9f110d98c6..dd82ca3d43e2 100644 --- a/xen/arch/x86/setup.c +++ b/xen/arch/x86/setup.c @@ -1012,7 +1012,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p) struct boot_info *bi; multiboot_info_t *mbi; module_t *mod; - unsigned long nr_pages, raw_max_page, modules_headroom, module_map[1]; + unsigned long nr_pages, raw_max_page, module_map[1]; int i, j, e820_warn = 0, bytes = 0; unsigned long eb_start, eb_end; bool acpi_boot_table_init_done = false, relocated = false; @@ -1371,7 +1371,10 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p) mod[bi->nr_modules].mod_end = __2M_rwdata_end - _stext; } - modules_headroom = bzimage_headroom(bootstrap_map(mod), mod->mod_end); + bi->mods[0].headroom = + bzimage_headroom(bootstrap_map(bi->mods[0].mod), + bi->mods[0].mod->mod_end); + bootstrap_map(NULL); #ifndef highmem_start @@ -1456,8 +1459,10 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p) * decompressor overheads of mod[0] (the dom0 kernel). When we * move mod[0], we incorporate this as extra space at the start. */ - unsigned long headroom = j ? 0 : modules_headroom; - unsigned long size = PAGE_ALIGN(headroom + mod[j].mod_end); + struct boot_module *bm = &bi->mods[j]; + unsigned long size; + + size = PAGE_ALIGN(bm->headroom + mod[j].mod_end); if ( mod[j].reserved ) continue; @@ -1470,14 +1475,14 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p) continue; if ( s < end && - (headroom || + (bm->headroom || ((end - size) >> PAGE_SHIFT) > mod[j].mod_start) ) { - move_memory(end - size + headroom, + move_memory(end - size + bm->headroom, (uint64_t)mod[j].mod_start << PAGE_SHIFT, mod[j].mod_end); mod[j].mod_start = (end - size) >> PAGE_SHIFT; - mod[j].mod_end += headroom; + mod[j].mod_end += bm->headroom; mod[j].reserved = 1; } } @@ -1504,7 +1509,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p) #endif } - if ( modules_headroom && !mod->reserved ) + if ( bi->mods[0].headroom && !mod->reserved ) panic("Not enough memory to relocate the dom0 kernel image\n"); for ( i = 0; i < bi->nr_modules; ++i ) { @@ -2056,7 +2061,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p) * 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. */ - dom0 = create_dom0(mod, modules_headroom, + dom0 = create_dom0(mod, bi->mods[0].headroom, initrdidx < bi->nr_modules ? mod + initrdidx : NULL, kextra, bi->loader); if ( !dom0 )
The purpose of struct boot_module is to encapsulate the state of boot module as it is processed by Xen. Locating boot module state struct boot_module reduces the number of global variables as well as the number of state variables that must be passed around. It also lays the groundwork for hyperlaunch mult-domain construction, where multiple instances of state variables like headroom will be needed. Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com> --- xen/arch/x86/include/asm/bootinfo.h | 5 +++++ xen/arch/x86/setup.c | 23 ++++++++++++++--------- 2 files changed, 19 insertions(+), 9 deletions(-)