| Message ID | 20240426140140.465506-2-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86/boot: Untangling | expand |
On 26.04.2024 16:01, Andrew Cooper wrote: > modules_headroom is a misleading name as it applies strictly to mod[0] only, > and the movement loop is deeply unintuitive and completely undocumented. > > Provide help to whomever needs to look at this code next. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Jan Beulich <jbeulich@suse.com>
On 2024-04-26 10:01, Andrew Cooper wrote: > modules_headroom is a misleading name as it applies strictly to mod[0] only, > and the movement loop is deeply unintuitive and completely undocumented. > > Provide help to whomever needs to look at this code next. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> > --- > CC: Jan Beulich <JBeulich@suse.com> > CC: Roger Pau Monné <roger.pau@citrix.com> > CC: Stefano Stabellini <sstabellini@kernel.org> > CC: Daniel Smith <dpsmith@apertussolutions.com> > CC: Christopher Clark <christopher.w.clark@gmail.com> > --- > xen/arch/x86/setup.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c > index caf235c6286d..59907fae095f 100644 > --- a/xen/arch/x86/setup.c > +++ b/xen/arch/x86/setup.c > @@ -1432,6 +1432,11 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p) > /* Is the region suitable for relocating the multiboot modules? */ > for ( j = mbi->mods_count - 1; j >= 0; j-- ) > { > + /* > + * 'headroom' is a guess for the decompressed size and > + * decompressor overheads of mod[0] (the dom0 kernel). When we > + * move mod[0], we incorperate this as extra space at the start. incorporate With that: Reviewed-by: Jason Andryuk <jason.andryuk@gmail.com> Thanks, Jason > + */ > unsigned long headroom = j ? 0 : modules_headroom; > unsigned long size = PAGE_ALIGN(headroom + mod[j].mod_end); >
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c index caf235c6286d..59907fae095f 100644 --- a/xen/arch/x86/setup.c +++ b/xen/arch/x86/setup.c @@ -1432,6 +1432,11 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p) /* Is the region suitable for relocating the multiboot modules? */ for ( j = mbi->mods_count - 1; j >= 0; j-- ) { + /* + * 'headroom' is a guess for the decompressed size and + * decompressor overheads of mod[0] (the dom0 kernel). When we + * move mod[0], we incorperate this as extra space at the start. + */ unsigned long headroom = j ? 0 : modules_headroom; unsigned long size = PAGE_ALIGN(headroom + mod[j].mod_end);
modules_headroom is a misleading name as it applies strictly to mod[0] only, and the movement loop is deeply unintuitive and completely undocumented. Provide help to whomever needs to look at this code next. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Roger Pau Monné <roger.pau@citrix.com> CC: Stefano Stabellini <sstabellini@kernel.org> CC: Daniel Smith <dpsmith@apertussolutions.com> CC: Christopher Clark <christopher.w.clark@gmail.com> --- xen/arch/x86/setup.c | 5 +++++ 1 file changed, 5 insertions(+)