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 <dpsm...@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

Reply via email to