diff mbox series

[v6,07/44] x86/boot: move headroom to boot modules

Message ID 20241017170325.3842-8-dpsmith@apertussolutions.com (mailing list archive)
State Superseded
Headers show
Series Boot modules for Hyperlaunch | expand

Commit Message

Daniel P. Smith Oct. 17, 2024, 5:02 p.m. UTC
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>
---
Changes since v5:
- reword and expand comment on headroom
- consolidated declaration and assignment
---
 xen/arch/x86/include/asm/bootinfo.h | 13 +++++++++++++
 xen/arch/x86/setup.c                | 21 ++++++++++++---------
 2 files changed, 25 insertions(+), 9 deletions(-)

Comments

Jason Andryuk Oct. 17, 2024, 9:22 p.m. UTC | #1
On 2024-10-17 13:02, 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>

Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
Andrew Cooper Oct. 17, 2024, 9:39 p.m. UTC | #2
On 17/10/2024 6:02 pm, Daniel P. Smith wrote:
> diff --git a/xen/arch/x86/include/asm/bootinfo.h b/xen/arch/x86/include/asm/bootinfo.h
> index d19473d8941e..5fe4f1387d03 100644
> --- a/xen/arch/x86/include/asm/bootinfo.h
> +++ b/xen/arch/x86/include/asm/bootinfo.h
> @@ -17,6 +17,19 @@
>  struct boot_module {
>      /* Transitionary only */
>      module_t *mod;

Blank line here.

I know it falls back out later, but it's very important for legibility
to be able to tell at a glance which comment belongs to which item.

~Andrew

> +    /*
> +     * A boot module may contain a compressed kernel that will require
> +     * additional space, before the module data, into which the kernel will be
> +     * decompressed.
> +     *
> +     * Memory layout 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;
>  };
>  
>  /*
>
Daniel P. Smith Oct. 18, 2024, 12:19 a.m. UTC | #3
On 10/17/24 17:39, Andrew Cooper wrote:
> On 17/10/2024 6:02 pm, Daniel P. Smith wrote:
>> diff --git a/xen/arch/x86/include/asm/bootinfo.h b/xen/arch/x86/include/asm/bootinfo.h
>> index d19473d8941e..5fe4f1387d03 100644
>> --- a/xen/arch/x86/include/asm/bootinfo.h
>> +++ b/xen/arch/x86/include/asm/bootinfo.h
>> @@ -17,6 +17,19 @@
>>   struct boot_module {
>>       /* Transitionary only */
>>       module_t *mod;
> 
> Blank line here.
> 
> I know it falls back out later, but it's very important for legibility
> to be able to tell at a glance which comment belongs to which item.

Ack.

v/r,
dps
diff mbox series

Patch

diff --git a/xen/arch/x86/include/asm/bootinfo.h b/xen/arch/x86/include/asm/bootinfo.h
index d19473d8941e..5fe4f1387d03 100644
--- a/xen/arch/x86/include/asm/bootinfo.h
+++ b/xen/arch/x86/include/asm/bootinfo.h
@@ -17,6 +17,19 @@ 
 struct boot_module {
     /* Transitionary only */
     module_t *mod;
+    /*
+     * A boot module may contain a compressed kernel that will require
+     * additional space, before the module data, into which the kernel will be
+     * decompressed.
+     *
+     * Memory layout 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 10934bf31d7f..11e1027d72dd 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,8 @@  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 = PAGE_ALIGN(bm->headroom + mod[j].mod_end);
 
             if ( mod[j].reserved )
                 continue;
@@ -1470,14 +1473,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 +1507,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 +2059,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 )