| Message ID | 20240830214730.1621-7-dpsmith@apertussolutions.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Boot modules for Hyperlaunch | expand |
On 30.08.2024 23:46, Daniel P. Smith wrote: > --- a/xen/arch/x86/setup.c > +++ b/xen/arch/x86/setup.c > @@ -632,7 +632,7 @@ static void __init noinline move_xen(void) > #undef BOOTSTRAP_MAP_LIMIT > > 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 struct boot_module *mods, As an array is meant, may I ask to switch to mods[] at this occasion? > @@ -642,20 +642,20 @@ 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)mods[i].early_mod->mod_start << PAGE_SHIFT; Similarly, may I ask to stop open-coding {,__}pfn_to_paddr() while transforming this? > @@ -1447,7 +1447,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p) > { > /* Don't overlap with modules. */ > end = consider_modules(s, e, reloc_size + mask, > - mod, boot_info->nr_mods, -1); > + boot_info->mods, boot_info->nr_mods, -1); > end &= ~mask; > } > else > @@ -1482,7 +1482,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p) > continue; > > /* Don't overlap with other modules (or Xen itself). */ > - end = consider_modules(s, e, size, mod, > + end = consider_modules(s, e, size, boot_info->mods, > boot_info->nr_mods + relocated, j); > > if ( highmem_start && end > highmem_start ) > @@ -1509,7 +1509,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p) > while ( !kexec_crash_area.start ) > { > /* Don't overlap with modules (or Xen itself). */ > - e = consider_modules(s, e, PAGE_ALIGN(kexec_crash_area.size), mod, > + e = consider_modules(s, e, PAGE_ALIGN(kexec_crash_area.size), boot_info->mods, > boot_info->nr_mods + relocated, -1); All of these show a meaningful increase of line lengths, up to the point of ending up with too long a line here. I really wonder if the variable name "boot_info" isn't too long for something that's going to be used quite frequently. Just "bi" maybe? Jan
On 04/09/2024 7:40 am, Jan Beulich wrote: > On 30.08.2024 23:46, Daniel P. Smith wrote: >> @@ -1447,7 +1447,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p) >> { >> /* Don't overlap with modules. */ >> end = consider_modules(s, e, reloc_size + mask, >> - mod, boot_info->nr_mods, -1); >> + boot_info->mods, boot_info->nr_mods, -1); >> end &= ~mask; >> } >> else >> @@ -1482,7 +1482,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p) >> continue; >> >> /* Don't overlap with other modules (or Xen itself). */ >> - end = consider_modules(s, e, size, mod, >> + end = consider_modules(s, e, size, boot_info->mods, >> boot_info->nr_mods + relocated, j); >> >> if ( highmem_start && end > highmem_start ) >> @@ -1509,7 +1509,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p) >> while ( !kexec_crash_area.start ) >> { >> /* Don't overlap with modules (or Xen itself). */ >> - e = consider_modules(s, e, PAGE_ALIGN(kexec_crash_area.size), mod, >> + e = consider_modules(s, e, PAGE_ALIGN(kexec_crash_area.size), boot_info->mods, >> boot_info->nr_mods + relocated, -1); > All of these show a meaningful increase of line lengths, up to the point of > ending up with too long a line here. I really wonder if the variable name > "boot_info" isn't too long for something that's going to be used quite > frequently. Just "bi" maybe? Actually I noticed that too. It's boot_info-> in setup.c, and bi-> everywhere else (with bm later too). We should just use "bi" uniformly even in setup.c (Or some other name, but I'm happy with bi here - it's very easily qualified by it's field names.) ~Andrew
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c index 28fdbf4d4c2b..8912956ee7f1 100644 --- a/xen/arch/x86/setup.c +++ b/xen/arch/x86/setup.c @@ -632,7 +632,7 @@ static void __init noinline move_xen(void) #undef BOOTSTRAP_MAP_LIMIT 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 struct boot_module *mods, unsigned int nr_mods, unsigned int this_mod) { unsigned int i; @@ -642,20 +642,20 @@ 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)mods[i].early_mod->mod_start << PAGE_SHIFT; + uint64_t end = start + PAGE_ALIGN(mods[i].early_mod->mod_end); if ( i == this_mod ) continue; if ( s < end && start < e ) { - end = consider_modules(end, e, size, mod + i + 1, + end = consider_modules(end, e, size, &mods[i + 1], nr_mods - i - 1, this_mod - i - 1); if ( end ) return end; - return consider_modules(s, start, size, mod + i + 1, + return consider_modules(s, start, size, &mods[i + 1], nr_mods - i - 1, this_mod - i - 1); } } @@ -1447,7 +1447,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p) { /* Don't overlap with modules. */ end = consider_modules(s, e, reloc_size + mask, - mod, boot_info->nr_mods, -1); + boot_info->mods, boot_info->nr_mods, -1); end &= ~mask; } else @@ -1482,7 +1482,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p) continue; /* Don't overlap with other modules (or Xen itself). */ - end = consider_modules(s, e, size, mod, + end = consider_modules(s, e, size, boot_info->mods, boot_info->nr_mods + relocated, j); if ( highmem_start && end > highmem_start ) @@ -1509,7 +1509,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p) while ( !kexec_crash_area.start ) { /* Don't overlap with modules (or Xen itself). */ - e = consider_modules(s, e, PAGE_ALIGN(kexec_crash_area.size), mod, + e = consider_modules(s, e, PAGE_ALIGN(kexec_crash_area.size), boot_info->mods, boot_info->nr_mods + relocated, -1); if ( s >= e ) break;
To start transitioning consider_modules() over to struct boot_module, begin with taking the array of struct boot_modules but use the temporary struct element early_mod. No functional change intended. Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com> --- xen/arch/x86/setup.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)