[PATCH v4 00/44] Boot modules for Hyperlaunch

[PATCH v4 00/44] Boot modules for Hyperlaunch

[Daniel P. Smith]

The Boot Modules for Hyperlaunch series is an effort to split out preliminary
changes necessary for the introduction of the Hyperlaunch domain builder
logic. These preliminary changes revolve around introducing the struct
boot_module and struct boot_domain structures. This includes converting the
dom0 construction path to use these structures. These abstractions lay the
groundwork to transform and extend the dom0 construction logic into a limited,
but general domain builder.

The splitting of Hyperlaunch into a pair of series was twofold, to reduce the
effort in reviewing a much larger series, and to reduce the effort in handling
the knock-on effects to the construction logic from requested review changes.

A note on v4:

For v4, two significant direction changes occurred. First, the series was
pulled back from attempting to be in common and instead focused on being
purely an x86 capability. Second, the changes were broken down into much
smaller change sets and ordered to provide a more ordered evolution of the
code. To the most extent possible, it was attempted to ensure all v3 comments
were transcribed accordingly with the scope change.

Much thanks to AMD for supporting this work.

Documentation on Hyperlaunch:
https://wiki.xenproject.org/wiki/Hyperlaunch

Original Hyperlaunch v1 patch series:
https://lists.xenproject.org/archives/html/xen-devel/2022-07/msg00345.html

V/r,
Daniel P. Smith

Changes since v3:
- reduced scope to x86 only
- broke changes into a smaller chunks with a linear progression
- concerns about deconflicting with Arm deferred
- conversion from mb1 to boot modules no longer attempted at entry points
- the temporary conversion function is now the permenant means to convert
- incorporated suggestion from Andy Cooper for handling bootstrap_map

Changes since v2:
- combined v2 patches 7 and 8 for common review
- rebased the v2 series onto the current tip of staging (sorry)
- fixed the placement of the patch changelogs
- provided the changes description in the cover letter

Changes since v1:
- the v2 and v3 series implement functionality from v1 patches 2-4
    - v2 series objective is to enable efficient patch review in support
      of merging the functionality into the hypervisor. It implements a
      subset of the v1 series, incorporating changes from community
      feedback.
- the bootstrap map is made accessible early in the v2 series via both
  multiboot and boot module arguments until later in the series where
  multiboot use is retired. This allows for incremental conversion across
  several patches from multiboot to boot modules.
- the 32-bit x86 boot environment header is removed, and changes are
  made to allow the new common bootinfo headers to be used instead.
- Arm and RISC-V architecture bootinfo headers are added to ensure that
  builds on those architectures can complete correctly.
- The KConfig patch to set the maximum number of boot modules allowed
  is not included in this series, replaced with a static maximum define.

Andrew Cooper (1):
  x86/boot: split bootstrap_map_addr() out of bootstrap_map()

Christopher Clark (1):
  x86/boot: move x86 boot module counting into a new boot_info struct

Daniel P. Smith (42):
  x86/boot: move boot loader name to boot info
  x86/boot: move cmdline to boot info
  x86/boot: move mmap info to boot info
  x86/boot: introduce struct boot_module
  x86/boot: convert consider_modules to struct boot_module
  x86/boot: move headroom to boot modules
  x86/boot: convert setup.c mod refs to early_mod
  x86/boot: introduce boot module types
  x86/boot: introduce boot module flags
  x86/boot: add start and size fields to struct boot_module
  x86/boot: update struct boot_module on module relocation
  x86/boot: transition relocation calculations to struct boot_module
  x86/boot: introduce boot module interator
  x86/boot: introduce consumed flag for struct boot_module
  x86/boot: convert microcode loading to consume struct boot_info
  x86/boot: convert late microcode loading to struct boot_module
  x86/boot: use consumed boot module flag for microcode
  x86/boot: convert xsm policy loading to struct boot_module
  x86/boot: convert ramdisk locating to struct boot_module
  x86/boot: remove module_map usage from microcode loading
  x86/boot: remove module_map usage from xsm policy loading
  x86/boot: remove module_map usage by ramdisk loading
  x86/boot: convert create_dom0 to use boot info
  x86/boot: convert construct_dom0 to use struct boot_module
  x86/boot: relocate kextra into boot info
  x86/boot: add cmdline to struct boot_module
  x86/boot: convert dom0_construct_pv image param to struct boot_module
  x86/boot: convert dom0_construct_pv initrd param to struct boot_module
  x86/boot: convert dom0_construct_pvh to struct boot_module
  x86/boot: convert pvh_load_kernel to struct boot_module
  x86/boot: convert initial_images to struct boot_module
  x86/boot: drop the use of initial_images unit global
  x86/boot: remove usage of mod_end by discard_initial_images
  x86/boot: remove remaining early_mod references
  x86/boot: remove early_mod from struct boot_module
  x86/boot: introduce boot domain
  x86/boot: introduce domid field to struct boot_domain
  x86/boot: add cmdline to struct boot_domain
  x86/boot: add struct domain to struct boot_domain
  x86/boot: convert construct_dom0 to struct boot_domain
  x86/boot: convert dom0_construct_pv to struct boot_domain
  x86/boot: convert dom0_construct_pvh to struct boot_domain

 xen/arch/x86/cpu/microcode/core.c     |  78 +++----
 xen/arch/x86/dom0_build.c             |  21 +-
 xen/arch/x86/hvm/dom0_build.c         |  55 +++--
 xen/arch/x86/include/asm/bootdomain.h |  37 +++
 xen/arch/x86/include/asm/bootinfo.h   |  83 +++++++
 xen/arch/x86/include/asm/dom0_build.h |  11 +-
 xen/arch/x86/include/asm/microcode.h  |  12 +-
 xen/arch/x86/include/asm/setup.h      |   9 +-
 xen/arch/x86/pv/dom0_build.c          |  38 ++--
 xen/arch/x86/setup.c                  | 316 ++++++++++++++++----------
 xen/include/xsm/xsm.h                 |  14 +-
 xen/xsm/xsm_core.c                    |  15 +-
 xen/xsm/xsm_policy.c                  |  18 +-
 13 files changed, 442 insertions(+), 265 deletions(-)
 create mode 100644 xen/arch/x86/include/asm/bootdomain.h
 create mode 100644 xen/arch/x86/include/asm/bootinfo.h

-- 
2.30.2

[PATCH v4 01/44] x86/boot: move x86 boot module counting into a new boot_info struct

[Daniel P. Smith]

From: Christopher Clark <christopher.w.clark@gmail.com>

An initial step towards a non-multiboot internal representation of boot
modules for common code, starting with x86 setup and converting the fields
that are accessed for the startup calculations.

Introduce a new header, <xen/asm/bootinfo.h>, and populate it with a new
boot_info structure initially containing a count of the number of boot
modules.

No functional change intended.

Signed-off-by: Christopher Clark <christopher.w.clark@gmail.com>
Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/arch/x86/include/asm/bootinfo.h | 25 +++++++++++++
 xen/arch/x86/setup.c                | 58 +++++++++++++++++------------
 2 files changed, 59 insertions(+), 24 deletions(-)
 create mode 100644 xen/arch/x86/include/asm/bootinfo.h

diff --git a/xen/arch/x86/include/asm/bootinfo.h b/xen/arch/x86/include/asm/bootinfo.h
new file mode 100644
index 000000000000..e850f80d26a7
--- /dev/null
+++ b/xen/arch/x86/include/asm/bootinfo.h
@@ -0,0 +1,25 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (c) 2024 Christopher Clark <christopher.w.clark@gmail.com>
+ * Copyright (c) 2024 Apertus Solutions, LLC
+ * Author: Daniel P. Smith <dpsmith@apertussolutions.com>
+ */
+
+#ifndef __XEN_X86_BOOTINFO_H__
+#define __XEN_X86_BOOTINFO_H__
+
+struct boot_info {
+    unsigned int nr_mods;
+};
+
+#endif
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index eee20bb1753c..dd94ee2e736b 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -32,6 +32,7 @@
 #include <compat/xen.h>
 #endif
 #include <xen/bitops.h>
+#include <asm/bootinfo.h>
 #include <asm/smp.h>
 #include <asm/processor.h>
 #include <asm/mpspec.h>
@@ -276,7 +277,16 @@ static int __init cf_check parse_acpi_param(const char *s)
 custom_param("acpi", parse_acpi_param);
 
 static const module_t *__initdata initial_images;
-static unsigned int __initdata nr_initial_images;
+static struct boot_info __initdata *boot_info;
+
+static void __init multiboot_to_bootinfo(multiboot_info_t *mbi)
+{
+    static struct boot_info __initdata info;
+
+    info.nr_mods = mbi->mods_count;
+
+    boot_info = &info;
+}
 
 unsigned long __init initial_images_nrpages(nodeid_t node)
 {
@@ -285,7 +295,7 @@ unsigned long __init initial_images_nrpages(nodeid_t node)
     unsigned long nr;
     unsigned int i;
 
-    for ( nr = i = 0; i < nr_initial_images; ++i )
+    for ( nr = i = 0; i < boot_info->nr_mods; ++i )
     {
         unsigned long start = initial_images[i].mod_start;
         unsigned long end = start + PFN_UP(initial_images[i].mod_end);
@@ -301,7 +311,7 @@ void __init discard_initial_images(void)
 {
     unsigned int i;
 
-    for ( i = 0; i < nr_initial_images; ++i )
+    for ( i = 0; i < boot_info->nr_mods; ++i )
     {
         uint64_t start = (uint64_t)initial_images[i].mod_start << PAGE_SHIFT;
 
@@ -309,7 +319,7 @@ void __init discard_initial_images(void)
                            start + PAGE_ALIGN(initial_images[i].mod_end));
     }
 
-    nr_initial_images = 0;
+    boot_info->nr_mods = 0;
     initial_images = NULL;
 }
 
@@ -1034,9 +1044,10 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
         mod = __va(mbi->mods_addr);
     }
 
+    multiboot_to_bootinfo(mbi);
+
     loader = (mbi->flags & MBI_LOADERNAME) ? __va(mbi->boot_loader_name)
                                            : "unknown";
-
     /* Parse the command-line options. */
     if ( mbi->flags & MBI_CMDLINE )
         cmdline = cmdline_cook(__va(mbi->cmdline), loader);
@@ -1141,18 +1152,18 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
            bootsym(boot_edd_info_nr));
 
     /* Check that we have at least one Multiboot module. */
-    if ( !(mbi->flags & MBI_MODULES) || (mbi->mods_count == 0) )
+    if ( !(mbi->flags & MBI_MODULES) || (boot_info->nr_mods == 0) )
         panic("dom0 kernel not specified. Check bootloader configuration\n");
 
     /* Check that we don't have a silly number of modules. */
-    if ( mbi->mods_count > sizeof(module_map) * 8 )
+    if ( boot_info->nr_mods > sizeof(module_map) * 8 )
     {
-        mbi->mods_count = sizeof(module_map) * 8;
+        boot_info->nr_mods = sizeof(module_map) * 8;
         printk("Excessive multiboot modules - using the first %u only\n",
-               mbi->mods_count);
+               boot_info->nr_mods);
     }
 
-    bitmap_fill(module_map, mbi->mods_count);
+    bitmap_fill(module_map, boot_info->nr_mods);
     __clear_bit(0, module_map); /* Dom0 kernel is always first */
 
     if ( pvh_boot )
@@ -1325,9 +1336,8 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
     kexec_reserve_area();
 
     initial_images = mod;
-    nr_initial_images = mbi->mods_count;
 
-    for ( i = 0; !efi_enabled(EFI_LOADER) && i < mbi->mods_count; i++ )
+    for ( i = 0; !efi_enabled(EFI_LOADER) && i < boot_info->nr_mods; i++ )
     {
         if ( mod[i].mod_start & (PAGE_SIZE - 1) )
             panic("Bootloader didn't honor module alignment request\n");
@@ -1351,8 +1361,8 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
          * respective reserve_e820_ram() invocation below. No need to
          * query efi_boot_mem_unused() here, though.
          */
-        mod[mbi->mods_count].mod_start = virt_to_mfn(_stext);
-        mod[mbi->mods_count].mod_end = __2M_rwdata_end - _stext;
+        mod[boot_info->nr_mods].mod_start = virt_to_mfn(_stext);
+        mod[boot_info->nr_mods].mod_end = __2M_rwdata_end - _stext;
     }
 
     modules_headroom = bzimage_headroom(bootstrap_map(mod), mod->mod_end);
@@ -1412,7 +1422,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, mbi->mods_count, -1);
+                                   mod, boot_info->nr_mods, -1);
             end &= ~mask;
         }
         else
@@ -1433,7 +1443,7 @@ 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-- )
+        for ( j = boot_info->nr_mods - 1; j >= 0; j-- )
         {
             /*
              * 'headroom' is a guess for the decompressed size and
@@ -1448,7 +1458,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
 
             /* Don't overlap with other modules (or Xen itself). */
             end = consider_modules(s, e, size, mod,
-                                   mbi->mods_count + relocated, j);
+                                   boot_info->nr_mods + relocated, j);
 
             if ( highmem_start && end > highmem_start )
                 continue;
@@ -1475,7 +1485,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
         {
             /* Don't overlap with modules (or Xen itself). */
             e = consider_modules(s, e, PAGE_ALIGN(kexec_crash_area.size), mod,
-                                 mbi->mods_count + relocated, -1);
+                                 boot_info->nr_mods + relocated, -1);
             if ( s >= e )
                 break;
             if ( e > kexec_crash_area_limit )
@@ -1490,7 +1500,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
 
     if ( modules_headroom && !mod->reserved )
         panic("Not enough memory to relocate the dom0 kernel image\n");
-    for ( i = 0; i < mbi->mods_count; ++i )
+    for ( i = 0; i < boot_info->nr_mods; ++i )
     {
         uint64_t s = (uint64_t)mod[i].mod_start << PAGE_SHIFT;
 
@@ -1570,7 +1580,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
                     ASSERT(j);
                 }
                 map_e = boot_e820.map[j].addr + boot_e820.map[j].size;
-                for ( j = 0; j < mbi->mods_count; ++j )
+                for ( j = 0; j < boot_info->nr_mods; ++j )
                 {
                     uint64_t end = pfn_to_paddr(mod[j].mod_start) +
                                    mod[j].mod_end;
@@ -1645,7 +1655,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
         }
     }
 
-    for ( i = 0; i < mbi->mods_count; ++i )
+    for ( i = 0; i < boot_info->nr_mods; ++i )
     {
         set_pdx_range(mod[i].mod_start,
                       mod[i].mod_start + PFN_UP(mod[i].mod_end));
@@ -2032,8 +2042,8 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
            cpu_has_nx ? XENLOG_INFO : XENLOG_WARNING "Warning: ",
            cpu_has_nx ? "" : "not ");
 
-    initrdidx = find_first_bit(module_map, mbi->mods_count);
-    if ( bitmap_weight(module_map, mbi->mods_count) > 1 )
+    initrdidx = find_first_bit(module_map, boot_info->nr_mods);
+    if ( bitmap_weight(module_map, boot_info->nr_mods) > 1 )
         printk(XENLOG_WARNING
                "Multiple initrd candidates, picking module #%u\n",
                initrdidx);
@@ -2043,7 +2053,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
      * above our heap. The second module, if present, is an initrd ramdisk.
      */
     dom0 = create_dom0(mod, modules_headroom,
-                       initrdidx < mbi->mods_count ? mod + initrdidx : NULL,
+                       initrdidx < boot_info->nr_mods ? mod + initrdidx : NULL,
                        kextra, loader);
     if ( !dom0 )
         panic("Could not set up DOM0 guest OS\n");
-- 
2.30.2

[PATCH v4 02/44] x86/boot: move boot loader name to boot info

[Daniel P. Smith]

Transition the incoming boot loader name to be held in struct boot_info.

No functional change intended.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/arch/x86/include/asm/bootinfo.h |  2 ++
 xen/arch/x86/setup.c                | 15 ++++++++-------
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/xen/arch/x86/include/asm/bootinfo.h b/xen/arch/x86/include/asm/bootinfo.h
index e850f80d26a7..e69feb1bb8be 100644
--- a/xen/arch/x86/include/asm/bootinfo.h
+++ b/xen/arch/x86/include/asm/bootinfo.h
@@ -10,6 +10,8 @@
 
 struct boot_info {
     unsigned int nr_mods;
+
+    const char *boot_loader_name;
 };
 
 #endif
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index dd94ee2e736b..432b7d1701e4 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -285,6 +285,9 @@ static void __init multiboot_to_bootinfo(multiboot_info_t *mbi)
 
     info.nr_mods = mbi->mods_count;
 
+    info.boot_loader_name = (mbi->flags & MBI_LOADERNAME) ?
+                            __va(mbi->boot_loader_name) : "unknown";
+
     boot_info = &info;
 }
 
@@ -993,7 +996,7 @@ static struct domain *__init create_dom0(const module_t *image,
 
 void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
 {
-    const char *memmap_type = NULL, *loader, *cmdline = "";
+    const char *memmap_type = NULL, *cmdline = "";
     char *kextra;
     void *bsp_stack;
     struct cpu_info *info = get_cpu_info(), *bsp_info;
@@ -1046,11 +1049,9 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
 
     multiboot_to_bootinfo(mbi);
 
-    loader = (mbi->flags & MBI_LOADERNAME) ? __va(mbi->boot_loader_name)
-                                           : "unknown";
     /* Parse the command-line options. */
     if ( mbi->flags & MBI_CMDLINE )
-        cmdline = cmdline_cook(__va(mbi->cmdline), loader);
+        cmdline = cmdline_cook(__va(mbi->cmdline), boot_info->boot_loader_name);
 
     if ( (kextra = strstr(cmdline, " -- ")) != NULL )
     {
@@ -1091,7 +1092,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
     if ( pvh_boot )
         pvh_print_info();
 
-    printk("Bootloader: %s\n", loader);
+    printk("Bootloader: %s\n", boot_info->boot_loader_name);
 
     printk("Command line: %s\n", cmdline);
 
@@ -1184,7 +1185,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
         l3_bootmap[l3_table_offset(BOOTSTRAP_MAP_BASE)] =
             l3e_from_paddr(__pa(l2_bootmap), __PAGE_HYPERVISOR);
 
-        memmap_type = loader;
+        memmap_type = boot_info->boot_loader_name;
     }
     else if ( efi_enabled(EFI_BOOT) )
         memmap_type = "EFI";
@@ -2054,7 +2055,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
      */
     dom0 = create_dom0(mod, modules_headroom,
                        initrdidx < boot_info->nr_mods ? mod + initrdidx : NULL,
-                       kextra, loader);
+                       kextra, boot_info->boot_loader_name);
     if ( !dom0 )
         panic("Could not set up DOM0 guest OS\n");
 
-- 
2.30.2

[PATCH v4 03/44] x86/boot: move cmdline to boot info

[Daniel P. Smith]

Transition Xen's command line to being held in struct boot_info.

No functional change intended.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/arch/x86/include/asm/bootinfo.h |  1 +
 xen/arch/x86/setup.c                | 21 +++++++++++++--------
 2 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/xen/arch/x86/include/asm/bootinfo.h b/xen/arch/x86/include/asm/bootinfo.h
index e69feb1bb8be..d2ca077d2356 100644
--- a/xen/arch/x86/include/asm/bootinfo.h
+++ b/xen/arch/x86/include/asm/bootinfo.h
@@ -12,6 +12,7 @@ struct boot_info {
     unsigned int nr_mods;
 
     const char *boot_loader_name;
+    const char *cmdline;
 };
 
 #endif
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 432b7d1701e4..a945fa10555f 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -276,6 +276,8 @@ static int __init cf_check parse_acpi_param(const char *s)
 }
 custom_param("acpi", parse_acpi_param);
 
+static const char *cmdline_cook(const char *p, const char *loader_name);
+
 static const module_t *__initdata initial_images;
 static struct boot_info __initdata *boot_info;
 
@@ -288,6 +290,13 @@ static void __init multiboot_to_bootinfo(multiboot_info_t *mbi)
     info.boot_loader_name = (mbi->flags & MBI_LOADERNAME) ?
                             __va(mbi->boot_loader_name) : "unknown";
 
+    /* Parse the command-line options. */
+    if ( mbi->flags & MBI_CMDLINE )
+        info.cmdline = cmdline_cook(__va(mbi->cmdline),
+                                    info.boot_loader_name);
+    else
+        info.cmdline = "";
+
     boot_info = &info;
 }
 
@@ -996,7 +1005,7 @@ static struct domain *__init create_dom0(const module_t *image,
 
 void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
 {
-    const char *memmap_type = NULL, *cmdline = "";
+    const char *memmap_type = NULL;
     char *kextra;
     void *bsp_stack;
     struct cpu_info *info = get_cpu_info(), *bsp_info;
@@ -1049,11 +1058,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
 
     multiboot_to_bootinfo(mbi);
 
-    /* Parse the command-line options. */
-    if ( mbi->flags & MBI_CMDLINE )
-        cmdline = cmdline_cook(__va(mbi->cmdline), boot_info->boot_loader_name);
-
-    if ( (kextra = strstr(cmdline, " -- ")) != NULL )
+    if ( (kextra = strstr(boot_info->cmdline, " -- ")) != NULL )
     {
         /*
          * Options after ' -- ' separator belong to dom0.
@@ -1064,7 +1069,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
         kextra += 3;
         while ( kextra[1] == ' ' ) kextra++;
     }
-    cmdline_parse(cmdline);
+    cmdline_parse(boot_info->cmdline);
 
     /* Must be after command line argument parsing and before
      * allocing any xenheap structures wanted in lower memory. */
@@ -1094,7 +1099,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
 
     printk("Bootloader: %s\n", boot_info->boot_loader_name);
 
-    printk("Command line: %s\n", cmdline);
+    printk("Command line: %s\n", boot_info->cmdline);
 
     printk("Xen image load base address: %#lx\n", xen_phys_start);
     if ( hypervisor_name )
-- 
2.30.2

[PATCH v4 04/44] x86/boot: move mmap info to boot info

[Daniel P. Smith]

Transition the memory map info to be held in struct boot_info.

No functional change intended.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/arch/x86/include/asm/bootinfo.h |  5 +++++
 xen/arch/x86/setup.c                | 12 +++++++++---
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/include/asm/bootinfo.h b/xen/arch/x86/include/asm/bootinfo.h
index d2ca077d2356..e785ed1c5982 100644
--- a/xen/arch/x86/include/asm/bootinfo.h
+++ b/xen/arch/x86/include/asm/bootinfo.h
@@ -8,11 +8,16 @@
 #ifndef __XEN_X86_BOOTINFO_H__
 #define __XEN_X86_BOOTINFO_H__
 
+#include <xen/types.h>
+
 struct boot_info {
     unsigned int nr_mods;
 
     const char *boot_loader_name;
     const char *cmdline;
+
+    paddr_t mmap_addr;
+    uint32_t mmap_length;
 };
 
 #endif
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index a945fa10555f..c6b45ced00ae 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -297,6 +297,12 @@ static void __init multiboot_to_bootinfo(multiboot_info_t *mbi)
     else
         info.cmdline = "";
 
+    if ( mbi->flags & MBI_MEMMAP )
+    {
+        info.mmap_addr = mbi->mmap_addr;
+        info.mmap_length = mbi->mmap_length;
+    }
+
     boot_info = &info;
 }
 
@@ -1200,13 +1206,13 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
     {
         memmap_type = "Xen-e820";
     }
-    else if ( mbi->flags & MBI_MEMMAP )
+    else if ( boot_info->mmap_addr )
     {
         memmap_type = "Multiboot-e820";
-        while ( bytes < mbi->mmap_length &&
+        while ( bytes < boot_info->mmap_length &&
                 e820_raw.nr_map < ARRAY_SIZE(e820_raw.map) )
         {
-            memory_map_t *map = __va(mbi->mmap_addr + bytes);
+            memory_map_t *map = __va(boot_info->mmap_addr + bytes);
 
             /*
              * This is a gross workaround for a BIOS bug. Some bootloaders do
-- 
2.30.2

[PATCH v4 05/44] x86/boot: introduce struct boot_module

[Daniel P. Smith]

This will introduce a new struct boot_module to provide a rich state
representation around modules provided by the boot loader. Support is for 64
boot modules, one held in reserve for Xen, and up to 63 can be provided by the
boot loader. The array of struct boot_modules will be accessible via a
reference held in struct boot_info.

A temporary `early_mod` parameter is included in struct boot_module to ease the
transition from using Multiboot v1 structures over to struct boot_module. Once
the transition is complete, the parameter will be dropped from the structure.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/arch/x86/include/asm/bootinfo.h |  6 ++++++
 xen/arch/x86/setup.c                | 19 ++++++++++++++++---
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/include/asm/bootinfo.h b/xen/arch/x86/include/asm/bootinfo.h
index e785ed1c5982..844262495962 100644
--- a/xen/arch/x86/include/asm/bootinfo.h
+++ b/xen/arch/x86/include/asm/bootinfo.h
@@ -8,10 +8,16 @@
 #ifndef __XEN_X86_BOOTINFO_H__
 #define __XEN_X86_BOOTINFO_H__
 
+#include <xen/multiboot.h>
 #include <xen/types.h>
 
+struct boot_module {
+    module_t *early_mod;
+};
+
 struct boot_info {
     unsigned int nr_mods;
+    struct boot_module *mods;
 
     const char *boot_loader_name;
     const char *cmdline;
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index c6b45ced00ae..28fdbf4d4c2b 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -278,12 +278,17 @@ custom_param("acpi", parse_acpi_param);
 
 static const char *cmdline_cook(const char *p, const char *loader_name);
 
+/* Max number of boot modules a bootloader can provide in addition to Xen */
+#define MAX_NR_BOOTMODS 63
+
 static const module_t *__initdata initial_images;
 static struct boot_info __initdata *boot_info;
 
-static void __init multiboot_to_bootinfo(multiboot_info_t *mbi)
+static void __init multiboot_to_bootinfo(multiboot_info_t *mbi, module_t *mods)
 {
     static struct boot_info __initdata info;
+    static struct boot_module __initdata boot_mods[MAX_NR_BOOTMODS + 1];
+    unsigned int i;
 
     info.nr_mods = mbi->mods_count;
 
@@ -303,6 +308,14 @@ static void __init multiboot_to_bootinfo(multiboot_info_t *mbi)
         info.mmap_length = mbi->mmap_length;
     }
 
+    info.mods = boot_mods;
+
+    for ( i=0; i < info.nr_mods; i++ )
+        boot_mods[i].early_mod = &mods[i];
+
+    /* map the last mb module for xen entry */
+    boot_mods[info.nr_mods].early_mod = &mods[info.nr_mods];
+
     boot_info = &info;
 }
 
@@ -1062,7 +1075,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
         mod = __va(mbi->mods_addr);
     }
 
-    multiboot_to_bootinfo(mbi);
+    multiboot_to_bootinfo(mbi, mod);
 
     if ( (kextra = strstr(boot_info->cmdline, " -- ")) != NULL )
     {
@@ -1164,7 +1177,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
            bootsym(boot_edd_info_nr));
 
     /* Check that we have at least one Multiboot module. */
-    if ( !(mbi->flags & MBI_MODULES) || (boot_info->nr_mods == 0) )
+    if ( boot_info->nr_mods == 0 )
         panic("dom0 kernel not specified. Check bootloader configuration\n");
 
     /* Check that we don't have a silly number of modules. */
-- 
2.30.2

[PATCH v4 06/44] x86/boot: convert consider_modules to struct boot_module

[Daniel P. Smith]

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(-)

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;
-- 
2.30.2

[PATCH v4 07/44] x86/boot: move headroom to boot modules

[Daniel P. Smith]

The purpose of struct boot_module is to encapsulate the state of boot modules.
Doing locates boot module state with its respective boot module, reduces
globals and multiple state variables being passed around. It also lays the
ground work for hyperlaunch where more multiple instances of these state
variables like headroom will be needed.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/arch/x86/include/asm/bootinfo.h |  1 +
 xen/arch/x86/setup.c                | 22 +++++++++++++---------
 2 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/xen/arch/x86/include/asm/bootinfo.h b/xen/arch/x86/include/asm/bootinfo.h
index 844262495962..3e0e36df096b 100644
--- a/xen/arch/x86/include/asm/bootinfo.h
+++ b/xen/arch/x86/include/asm/bootinfo.h
@@ -13,6 +13,7 @@
 
 struct boot_module {
     module_t *early_mod;
+    unsigned long headroom;
 };
 
 struct boot_info {
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 8912956ee7f1..fd6cc7fac907 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1031,7 +1031,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
     unsigned int initrdidx, num_parked = 0;
     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;
@@ -1390,7 +1390,9 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
         mod[boot_info->nr_mods].mod_end = __2M_rwdata_end - _stext;
     }
 
-    modules_headroom = bzimage_headroom(bootstrap_map(mod), mod->mod_end);
+    boot_info->mods[0].headroom = bzimage_headroom(
+                        bootstrap_map(boot_info->mods[0].early_mod),
+                        boot_info->mods[0].early_mod->mod_end);
     bootstrap_map(NULL);
 
 #ifndef highmem_start
@@ -1475,8 +1477,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 = &boot_info->mods[j];
+            unsigned long size;
+
+            size = PAGE_ALIGN(bm->headroom + mod[j].mod_end);
 
             if ( mod[j].reserved )
                 continue;
@@ -1489,14 +1493,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;
             }
         }
@@ -1523,7 +1527,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
 #endif
     }
 
-    if ( modules_headroom && !mod->reserved )
+    if ( boot_info->mods[0].headroom && !mod->reserved )
         panic("Not enough memory to relocate the dom0 kernel image\n");
     for ( i = 0; i < boot_info->nr_mods; ++i )
     {
@@ -2077,7 +2081,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, boot_info->mods[0].headroom,
                        initrdidx < boot_info->nr_mods ? mod + initrdidx : NULL,
                        kextra, boot_info->boot_loader_name);
     if ( !dom0 )
-- 
2.30.2

[PATCH v4 08/44] x86/boot: convert setup.c mod refs to early_mod

[Daniel P. Smith]

To allow a slow conversion of x86 over to struct boot_module, start with
replacing all references to struct mod to the early_mod element of struct
boot_module. These serves twofold, first to allow the incremental transition
from struct mod fields to struct boot_module fields.  The second is to allow
the conversion of function definitions from taking struct mod parameters to
accepting struct boot_module as needed when a transitioned field will be
accessed.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/arch/x86/setup.c | 61 ++++++++++++++++++++++++--------------------
 1 file changed, 34 insertions(+), 27 deletions(-)

diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index fd6cc7fac907..82a4375683d2 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1360,15 +1360,15 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
     set_kexec_crash_area_size((u64)nr_pages << PAGE_SHIFT);
     kexec_reserve_area();
 
-    initial_images = mod;
+    initial_images = boot_info->mods[0].early_mod;
 
     for ( i = 0; !efi_enabled(EFI_LOADER) && i < boot_info->nr_mods; i++ )
     {
-        if ( mod[i].mod_start & (PAGE_SIZE - 1) )
+        if ( boot_info->mods[i].early_mod->mod_start & (PAGE_SIZE - 1) )
             panic("Bootloader didn't honor module alignment request\n");
-        mod[i].mod_end -= mod[i].mod_start;
-        mod[i].mod_start >>= PAGE_SHIFT;
-        mod[i].reserved = 0;
+        boot_info->mods[i].early_mod->mod_end -= boot_info->mods[i].early_mod->mod_start;
+        boot_info->mods[i].early_mod->mod_start >>= PAGE_SHIFT;
+        boot_info->mods[i].early_mod->reserved = 0;
     }
 
     /*
@@ -1379,6 +1379,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
 
     if ( xen_phys_start )
     {
+        int idx = boot_info->nr_mods;
         relocated = true;
 
         /*
@@ -1386,8 +1387,8 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
          * respective reserve_e820_ram() invocation below. No need to
          * query efi_boot_mem_unused() here, though.
          */
-        mod[boot_info->nr_mods].mod_start = virt_to_mfn(_stext);
-        mod[boot_info->nr_mods].mod_end = __2M_rwdata_end - _stext;
+        boot_info->mods[idx].early_mod->mod_start = virt_to_mfn(_stext);
+        boot_info->mods[idx].early_mod->mod_end = __2M_rwdata_end - _stext;
     }
 
     boot_info->mods[0].headroom = bzimage_headroom(
@@ -1480,9 +1481,9 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
             struct boot_module *bm = &boot_info->mods[j];
             unsigned long size;
 
-            size = PAGE_ALIGN(bm->headroom + mod[j].mod_end);
+            size = PAGE_ALIGN(bm->headroom + bm->early_mod->mod_end);
 
-            if ( mod[j].reserved )
+            if ( boot_info->mods[j].early_mod->reserved )
                 continue;
 
             /* Don't overlap with other modules (or Xen itself). */
@@ -1494,14 +1495,14 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
 
             if ( s < end &&
                  (bm->headroom ||
-                  ((end - size) >> PAGE_SHIFT) > mod[j].mod_start) )
+                  ((end - size) >> PAGE_SHIFT) > bm->early_mod->mod_start) )
             {
                 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 += bm->headroom;
-                mod[j].reserved = 1;
+                            (uint64_t)bm->early_mod->mod_start << PAGE_SHIFT,
+                            bm->early_mod->mod_end);
+                bm->early_mod->mod_start = (end - size) >> PAGE_SHIFT;
+                bm->early_mod->mod_end += bm->headroom;
+                bm->early_mod->reserved = 1;
             }
         }
 
@@ -1527,13 +1528,15 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
 #endif
     }
 
-    if ( boot_info->mods[0].headroom && !mod->reserved )
+    if ( boot_info->mods[0].headroom && !boot_info->mods[0].early_mod->reserved )
         panic("Not enough memory to relocate the dom0 kernel image\n");
     for ( i = 0; i < boot_info->nr_mods; ++i )
     {
-        uint64_t s = (uint64_t)mod[i].mod_start << PAGE_SHIFT;
+        uint64_t s = (uint64_t)boot_info->mods[i].early_mod->mod_start
+                        << PAGE_SHIFT;
 
-        reserve_e820_ram(&boot_e820, s, s + PAGE_ALIGN(mod[i].mod_end));
+        reserve_e820_ram(&boot_e820, s,
+                         s + PAGE_ALIGN(boot_info->mods[i].early_mod->mod_end));
     }
 
     if ( !xen_phys_start )
@@ -1611,8 +1614,9 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
                 map_e = boot_e820.map[j].addr + boot_e820.map[j].size;
                 for ( j = 0; j < boot_info->nr_mods; ++j )
                 {
-                    uint64_t end = pfn_to_paddr(mod[j].mod_start) +
-                                   mod[j].mod_end;
+                    uint64_t end = pfn_to_paddr(
+                                   boot_info->mods[j].early_mod->mod_start) +
+                                   boot_info->mods[j].early_mod->mod_end;
 
                     if ( map_e < end )
                         map_e = end;
@@ -1686,11 +1690,13 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
 
     for ( i = 0; i < boot_info->nr_mods; ++i )
     {
-        set_pdx_range(mod[i].mod_start,
-                      mod[i].mod_start + PFN_UP(mod[i].mod_end));
-        map_pages_to_xen((unsigned long)mfn_to_virt(mod[i].mod_start),
-                         _mfn(mod[i].mod_start),
-                         PFN_UP(mod[i].mod_end), PAGE_HYPERVISOR);
+        set_pdx_range(boot_info->mods[i].early_mod->mod_start,
+                      boot_info->mods[i].early_mod->mod_start +
+                      PFN_UP(boot_info->mods[i].early_mod->mod_end));
+        map_pages_to_xen(
+            (unsigned long)mfn_to_virt(boot_info->mods[i].early_mod->mod_start),
+            _mfn(boot_info->mods[i].early_mod->mod_start),
+            PFN_UP(boot_info->mods[i].early_mod->mod_end), PAGE_HYPERVISOR);
     }
 
 #ifdef CONFIG_KEXEC
@@ -2081,8 +2087,9 @@ 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, boot_info->mods[0].headroom,
-                       initrdidx < boot_info->nr_mods ? mod + initrdidx : NULL,
+    dom0 = create_dom0(boot_info->mods[0].early_mod, boot_info->mods[0].headroom,
+                       initrdidx < boot_info->nr_mods ?
+                            boot_info->mods[initrdidx].early_mod : NULL,
                        kextra, boot_info->boot_loader_name);
     if ( !dom0 )
         panic("Could not set up DOM0 guest OS\n");
-- 
2.30.2

[PATCH v4 09/44] x86/boot: split bootstrap_map_addr() out of bootstrap_map()

[Daniel P. Smith]

From: Andrew Cooper <andrew.cooper3@citrix.com>

Using an interface based on addresses directly, not modules.

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/arch/x86/include/asm/setup.h |  1 +
 xen/arch/x86/setup.c             | 19 +++++++++++++------
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/xen/arch/x86/include/asm/setup.h b/xen/arch/x86/include/asm/setup.h
index d75589178b91..15dcb62cb5ac 100644
--- a/xen/arch/x86/include/asm/setup.h
+++ b/xen/arch/x86/include/asm/setup.h
@@ -34,6 +34,7 @@ void setup_io_bitmap(struct domain *d);
 
 unsigned long initial_images_nrpages(nodeid_t node);
 void discard_initial_images(void);
+void *bootstrap_map_addr(uint64_t start, uint64_t end);
 void *bootstrap_map(const module_t *mod);
 
 int remove_xen_ranges(struct rangeset *r);
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 82a4375683d2..d4f557b4c50d 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -436,24 +436,22 @@ static void __init normalise_cpu_order(void)
  * Ensure a given physical memory range is present in the bootstrap mappings.
  * Use superpage mappings to ensure that pagetable memory needn't be allocated.
  */
-void *__init bootstrap_map(const module_t *mod)
+void *__init bootstrap_map_addr(uint64_t start, uint64_t end)
 {
     static unsigned long __initdata map_cur = BOOTSTRAP_MAP_BASE;
-    uint64_t start, end, mask = (1L << L2_PAGETABLE_SHIFT) - 1;
+    uint64_t mask = (1L << L2_PAGETABLE_SHIFT) - 1;
     void *ret;
 
     if ( system_state != SYS_STATE_early_boot )
-        return mod ? mfn_to_virt(mod->mod_start) : NULL;
+        return end ? maddr_to_virt(start) : NULL;
 
-    if ( !mod )
+    if ( !end )
     {
         destroy_xen_mappings(BOOTSTRAP_MAP_BASE, BOOTSTRAP_MAP_LIMIT);
         map_cur = BOOTSTRAP_MAP_BASE;
         return NULL;
     }
 
-    start = (uint64_t)mod->mod_start << PAGE_SHIFT;
-    end = start + mod->mod_end;
     if ( start >= end )
         return NULL;
 
@@ -469,6 +467,15 @@ void *__init bootstrap_map(const module_t *mod)
     return ret;
 }
 
+void *__init bootstrap_map(const module_t *mod)
+{
+    if ( !mod )
+        return bootstrap_map_addr(0, 0);
+
+    return bootstrap_map_addr(pfn_to_paddr(mod->mod_start),
+                              pfn_to_paddr(mod->mod_start) + mod->mod_end);
+}
+
 static void __init move_memory(
     uint64_t dst, uint64_t src, unsigned int size)
 {
-- 
2.30.2

[PATCH v4 10/44] x86/boot: introduce boot module types

[Daniel P. Smith]

This commit introduces module types of xen, kernel, and ramdisk to allow boot
module detect code to tag the purpose of a boot module. This reduces the need
for hard coded order assumptions and global variables to be used by consumers
of boot modules, such as domain construction.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/arch/x86/include/asm/bootinfo.h | 9 +++++++++
 xen/arch/x86/setup.c                | 3 +++
 2 files changed, 12 insertions(+)

diff --git a/xen/arch/x86/include/asm/bootinfo.h b/xen/arch/x86/include/asm/bootinfo.h
index 3e0e36df096b..1b1b640f83f7 100644
--- a/xen/arch/x86/include/asm/bootinfo.h
+++ b/xen/arch/x86/include/asm/bootinfo.h
@@ -11,9 +11,18 @@
 #include <xen/multiboot.h>
 #include <xen/types.h>
 
+/* Boot module binary type / purpose */
+enum bootmod_type {
+    BOOTMOD_UNKNOWN,
+    BOOTMOD_XEN,
+    BOOTMOD_KERNEL,
+    BOOTMOD_RAMDISK,
+};
+
 struct boot_module {
     module_t *early_mod;
     unsigned long headroom;
+    enum bootmod_type type;
 };
 
 struct boot_info {
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index d4f557b4c50d..8d5450c981a8 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -314,6 +314,7 @@ static void __init multiboot_to_bootinfo(multiboot_info_t *mbi, module_t *mods)
         boot_mods[i].early_mod = &mods[i];
 
     /* map the last mb module for xen entry */
+    boot_mods[info.nr_mods].type = BOOTMOD_XEN;
     boot_mods[info.nr_mods].early_mod = &mods[info.nr_mods];
 
     boot_info = &info;
@@ -1197,6 +1198,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
 
     bitmap_fill(module_map, boot_info->nr_mods);
     __clear_bit(0, module_map); /* Dom0 kernel is always first */
+    boot_info->mods[0].type = BOOTMOD_KERNEL;
 
     if ( pvh_boot )
     {
@@ -2085,6 +2087,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
            cpu_has_nx ? "" : "not ");
 
     initrdidx = find_first_bit(module_map, boot_info->nr_mods);
+    boot_info->mods[initrdidx].type = BOOTMOD_RAMDISK;
     if ( bitmap_weight(module_map, boot_info->nr_mods) > 1 )
         printk(XENLOG_WARNING
                "Multiple initrd candidates, picking module #%u\n",
-- 
2.30.2

[PATCH v4 11/44] x86/boot: introduce boot module flags

[Daniel P. Smith]

The existing startup code employs various ad-hoc state tracking about certain
boot module types by each area of the code. A boot module flags is added to
enable tracking these different states.  The first state to be transition by
this commit is module relocation.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/arch/x86/include/asm/bootinfo.h | 4 ++++
 xen/arch/x86/setup.c                | 8 ++++----
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/include/asm/bootinfo.h b/xen/arch/x86/include/asm/bootinfo.h
index 1b1b640f83f7..baf6d74db754 100644
--- a/xen/arch/x86/include/asm/bootinfo.h
+++ b/xen/arch/x86/include/asm/bootinfo.h
@@ -23,6 +23,10 @@ struct boot_module {
     module_t *early_mod;
     unsigned long headroom;
     enum bootmod_type type;
+
+    uint32_t flags;
+#define BOOTMOD_FLAG_X86_RELOCATED     (1U << 0)
+
 };
 
 struct boot_info {
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 8d5450c981a8..47e4fcc2a8ce 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1377,7 +1377,6 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
             panic("Bootloader didn't honor module alignment request\n");
         boot_info->mods[i].early_mod->mod_end -= boot_info->mods[i].early_mod->mod_start;
         boot_info->mods[i].early_mod->mod_start >>= PAGE_SHIFT;
-        boot_info->mods[i].early_mod->reserved = 0;
     }
 
     /*
@@ -1492,7 +1491,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
 
             size = PAGE_ALIGN(bm->headroom + bm->early_mod->mod_end);
 
-            if ( boot_info->mods[j].early_mod->reserved )
+            if ( boot_info->mods[j].flags & BOOTMOD_FLAG_X86_RELOCATED )
                 continue;
 
             /* Don't overlap with other modules (or Xen itself). */
@@ -1511,7 +1510,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
                             bm->early_mod->mod_end);
                 bm->early_mod->mod_start = (end - size) >> PAGE_SHIFT;
                 bm->early_mod->mod_end += bm->headroom;
-                bm->early_mod->reserved = 1;
+                bm->flags |= BOOTMOD_FLAG_X86_RELOCATED;
             }
         }
 
@@ -1537,7 +1536,8 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
 #endif
     }
 
-    if ( boot_info->mods[0].headroom && !boot_info->mods[0].early_mod->reserved )
+    if ( boot_info->mods[0].headroom &&
+         !(boot_info->mods[0].flags & BOOTMOD_FLAG_X86_RELOCATED) )
         panic("Not enough memory to relocate the dom0 kernel image\n");
     for ( i = 0; i < boot_info->nr_mods; ++i )
     {
-- 
2.30.2

[PATCH v4 12/44] x86/boot: add start and size fields to struct boot_module

[Daniel P. Smith]

This commit introduces the start and size fields to struct boot_module and adds
a corresponding bootstrap mapping function, bootstrap_map_bm.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/arch/x86/include/asm/bootinfo.h |  2 ++
 xen/arch/x86/include/asm/setup.h    |  2 ++
 xen/arch/x86/setup.c                | 13 +++++++++++++
 3 files changed, 17 insertions(+)

diff --git a/xen/arch/x86/include/asm/bootinfo.h b/xen/arch/x86/include/asm/bootinfo.h
index baf6d74db754..37132afb4e6a 100644
--- a/xen/arch/x86/include/asm/bootinfo.h
+++ b/xen/arch/x86/include/asm/bootinfo.h
@@ -27,6 +27,8 @@ struct boot_module {
     uint32_t flags;
 #define BOOTMOD_FLAG_X86_RELOCATED     (1U << 0)
 
+    paddr_t start;
+    size_t size;
 };
 
 struct boot_info {
diff --git a/xen/arch/x86/include/asm/setup.h b/xen/arch/x86/include/asm/setup.h
index 15dcb62cb5ac..165ca744ba34 100644
--- a/xen/arch/x86/include/asm/setup.h
+++ b/xen/arch/x86/include/asm/setup.h
@@ -2,6 +2,7 @@
 #define __X86_SETUP_H_
 
 #include <xen/multiboot.h>
+#include <asm/bootinfo.h>
 #include <asm/numa.h>
 
 extern const char __2M_text_start[], __2M_text_end[];
@@ -36,6 +37,7 @@ unsigned long initial_images_nrpages(nodeid_t node);
 void discard_initial_images(void);
 void *bootstrap_map_addr(uint64_t start, uint64_t end);
 void *bootstrap_map(const module_t *mod);
+void *bootstrap_map_bm(const struct boot_module *bm);
 
 int remove_xen_ranges(struct rangeset *r);
 
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 47e4fcc2a8ce..021c5699f86c 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -311,8 +311,13 @@ static void __init multiboot_to_bootinfo(multiboot_info_t *mbi, module_t *mods)
     info.mods = boot_mods;
 
     for ( i=0; i < info.nr_mods; i++ )
+    {
         boot_mods[i].early_mod = &mods[i];
 
+        boot_mods[i].start = (paddr_t)mods[i].mod_start;
+        boot_mods[i].size = mods[i].mod_end - mods[i].mod_start;
+    }
+
     /* map the last mb module for xen entry */
     boot_mods[info.nr_mods].type = BOOTMOD_XEN;
     boot_mods[info.nr_mods].early_mod = &mods[info.nr_mods];
@@ -477,6 +482,14 @@ void *__init bootstrap_map(const module_t *mod)
                               pfn_to_paddr(mod->mod_start) + mod->mod_end);
 }
 
+void *__init bootstrap_map_bm(const struct boot_module *bm)
+{
+    if ( !bm )
+        return bootstrap_map_addr(0, 0);
+
+    return bootstrap_map_addr(bm->start, bm->start + bm->size);
+}
+
 static void __init move_memory(
     uint64_t dst, uint64_t src, unsigned int size)
 {
-- 
2.30.2

[PATCH v4 13/44] x86/boot: update struct boot_module on module relocation

[Daniel P. Smith]

When a boot module is relocated, ensure struct boot_module start and size
fields are updated along with early_mod.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/arch/x86/setup.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 021c5699f86c..27517d24b2ea 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1408,8 +1408,10 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
          * respective reserve_e820_ram() invocation below. No need to
          * query efi_boot_mem_unused() here, though.
          */
-        boot_info->mods[idx].early_mod->mod_start = virt_to_mfn(_stext);
-        boot_info->mods[idx].early_mod->mod_end = __2M_rwdata_end - _stext;
+        boot_info->mods[idx].start = boot_info->mods[idx].early_mod->mod_start
+                                   = virt_to_mfn(_stext);
+        boot_info->mods[idx].size = boot_info->mods[idx].early_mod->mod_end
+                                  = __2M_rwdata_end - _stext;
     }
 
     boot_info->mods[0].headroom = bzimage_headroom(
-- 
2.30.2

[PATCH v4 14/44] x86/boot: transition relocation calculations to struct boot_module

[Daniel P. Smith]

Use struct boot_module fields, start and size, when calculating the relocation
address and size. It also ensures that early_mod references are kept in sync.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/arch/x86/setup.c | 33 +++++++++++++++------------------
 1 file changed, 15 insertions(+), 18 deletions(-)

diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 27517d24b2ea..0b6bde9ce7e3 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1504,7 +1504,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
             struct boot_module *bm = &boot_info->mods[j];
             unsigned long size;
 
-            size = PAGE_ALIGN(bm->headroom + bm->early_mod->mod_end);
+            size = PAGE_ALIGN(bm->headroom + bm->size);
 
             if ( boot_info->mods[j].flags & BOOTMOD_FLAG_X86_RELOCATED )
                 continue;
@@ -1518,13 +1518,12 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
 
             if ( s < end &&
                  (bm->headroom ||
-                  ((end - size) >> PAGE_SHIFT) > bm->early_mod->mod_start) )
+                  paddr_to_pfn(end - size) > paddr_to_pfn(bm->start)) )
             {
-                move_memory(end - size + bm->headroom,
-                            (uint64_t)bm->early_mod->mod_start << PAGE_SHIFT,
-                            bm->early_mod->mod_end);
-                bm->early_mod->mod_start = (end - size) >> PAGE_SHIFT;
-                bm->early_mod->mod_end += bm->headroom;
+                move_memory(end - size + bm->headroom, bm->start, bm->size);
+                bm->start = (end - size);
+                bm->early_mod->mod_start = paddr_to_pfn(bm->start);
+                bm->size = bm->early_mod->mod_end += bm->headroom;
                 bm->flags |= BOOTMOD_FLAG_X86_RELOCATED;
             }
         }
@@ -1556,11 +1555,10 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
         panic("Not enough memory to relocate the dom0 kernel image\n");
     for ( i = 0; i < boot_info->nr_mods; ++i )
     {
-        uint64_t s = (uint64_t)boot_info->mods[i].early_mod->mod_start
-                        << PAGE_SHIFT;
+        uint64_t s = (uint64_t)boot_info->mods[i].start;
 
         reserve_e820_ram(&boot_e820, s,
-                         s + PAGE_ALIGN(boot_info->mods[i].early_mod->mod_end));
+                         s + PAGE_ALIGN(boot_info->mods[i].size));
     }
 
     if ( !xen_phys_start )
@@ -1638,9 +1636,8 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
                 map_e = boot_e820.map[j].addr + boot_e820.map[j].size;
                 for ( j = 0; j < boot_info->nr_mods; ++j )
                 {
-                    uint64_t end = pfn_to_paddr(
-                                   boot_info->mods[j].early_mod->mod_start) +
-                                   boot_info->mods[j].early_mod->mod_end;
+                    uint64_t end = boot_info->mods[j].start +
+                                   boot_info->mods[j].size;
 
                     if ( map_e < end )
                         map_e = end;
@@ -1714,13 +1711,13 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
 
     for ( i = 0; i < boot_info->nr_mods; ++i )
     {
-        set_pdx_range(boot_info->mods[i].early_mod->mod_start,
-                      boot_info->mods[i].early_mod->mod_start +
+        set_pdx_range(paddr_to_pfn(boot_info->mods[i].start),
+                      paddr_to_pfn(boot_info->mods[i].start) +
                       PFN_UP(boot_info->mods[i].early_mod->mod_end));
         map_pages_to_xen(
-            (unsigned long)mfn_to_virt(boot_info->mods[i].early_mod->mod_start),
-            _mfn(boot_info->mods[i].early_mod->mod_start),
-            PFN_UP(boot_info->mods[i].early_mod->mod_end), PAGE_HYPERVISOR);
+            (unsigned long)maddr_to_virt(boot_info->mods[i].start),
+            maddr_to_mfn(boot_info->mods[i].start),
+            PFN_UP(boot_info->mods[i].size), PAGE_HYPERVISOR);
     }
 
 #ifdef CONFIG_KEXEC
-- 
2.30.2

[PATCH v4 15/44] x86/boot: introduce boot module interator

[Daniel P. Smith]

Provide an iterator to go through boot module array searching based on type.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/arch/x86/include/asm/bootinfo.h | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/include/asm/bootinfo.h b/xen/arch/x86/include/asm/bootinfo.h
index 37132afb4e6a..99f8c9b83b25 100644
--- a/xen/arch/x86/include/asm/bootinfo.h
+++ b/xen/arch/x86/include/asm/bootinfo.h
@@ -42,8 +42,24 @@ struct boot_info {
     uint32_t mmap_length;
 };
 
-#endif
+static inline int __init next_boot_module_index(
+    const struct boot_info *bi, enum bootmod_type t, int offset)
+{
+    int i;
+
+    for ( i = offset; i < bi->nr_mods; i++ )
+    {
+        if ( bi->mods[i].type == t )
+            return i;
+    }
+
+    return -1;
+}
 
+#define first_boot_module_index(bi, t)              \
+    next_boot_module_index(bi, t, 0)
+
+#endif
 /*
  * Local variables:
  * mode: C
-- 
2.30.2

[PATCH v4 16/44] x86/boot: introduce consumed flag for struct boot_module

[Daniel P. Smith]

Allow the tracking of when a boot module has been consumed by a handler in the
hypervisor independent of when it is claimed. The instances where the
hypervisor does nothing beyond claiming, the dom0 kernel, dom0 ramdisk, and a
placeholder for itself, are updated as being consumed at the time of being
claimed.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/arch/x86/include/asm/bootinfo.h | 1 +
 xen/arch/x86/setup.c                | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/xen/arch/x86/include/asm/bootinfo.h b/xen/arch/x86/include/asm/bootinfo.h
index 99f8c9b83b25..740308693629 100644
--- a/xen/arch/x86/include/asm/bootinfo.h
+++ b/xen/arch/x86/include/asm/bootinfo.h
@@ -26,6 +26,7 @@ struct boot_module {
 
     uint32_t flags;
 #define BOOTMOD_FLAG_X86_RELOCATED     (1U << 0)
+#define BOOTMOD_FLAG_X86_CONSUMED      (1U << 1)
 
     paddr_t start;
     size_t size;
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 0b6bde9ce7e3..da75dfce083d 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -320,6 +320,7 @@ static void __init multiboot_to_bootinfo(multiboot_info_t *mbi, module_t *mods)
 
     /* map the last mb module for xen entry */
     boot_mods[info.nr_mods].type = BOOTMOD_XEN;
+    boot_mods[info.nr_mods].flags |= BOOTMOD_FLAG_X86_CONSUMED;
     boot_mods[info.nr_mods].early_mod = &mods[info.nr_mods];
 
     boot_info = &info;
@@ -1212,6 +1213,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
     bitmap_fill(module_map, boot_info->nr_mods);
     __clear_bit(0, module_map); /* Dom0 kernel is always first */
     boot_info->mods[0].type = BOOTMOD_KERNEL;
+    boot_info->mods[0].flags |= BOOTMOD_FLAG_X86_CONSUMED;
 
     if ( pvh_boot )
     {
@@ -2100,6 +2102,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
 
     initrdidx = find_first_bit(module_map, boot_info->nr_mods);
     boot_info->mods[initrdidx].type = BOOTMOD_RAMDISK;
+    boot_info->mods[initrdidx].flags |= BOOTMOD_FLAG_X86_CONSUMED;
     if ( bitmap_weight(module_map, boot_info->nr_mods) > 1 )
         printk(XENLOG_WARNING
                "Multiple initrd candidates, picking module #%u\n",
-- 
2.30.2

[PATCH v4 17/44] x86/boot: convert microcode loading to consume struct boot_info

[Daniel P. Smith]

Convert the microcode loading functions to take struct boot_info, and then
using struct boot_module to map and check for microcode. To keep the changes
focused, continue using the struct mod to hold the reference to the microcode
that is used by the late microcode logic.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/arch/x86/cpu/microcode/core.c    | 37 +++++++++++++---------------
 xen/arch/x86/include/asm/bootinfo.h  |  1 +
 xen/arch/x86/include/asm/microcode.h | 14 ++++++-----
 xen/arch/x86/setup.c                 |  4 +--
 4 files changed, 28 insertions(+), 28 deletions(-)

diff --git a/xen/arch/x86/cpu/microcode/core.c b/xen/arch/x86/cpu/microcode/core.c
index 8a9e744489b9..653a725173ba 100644
--- a/xen/arch/x86/cpu/microcode/core.c
+++ b/xen/arch/x86/cpu/microcode/core.c
@@ -35,6 +35,7 @@
 #include <xen/watchdog.h>
 
 #include <asm/apic.h>
+#include <asm/bootinfo.h>
 #include <asm/cpu-policy.h>
 #include <asm/nmi.h>
 #include <asm/processor.h>
@@ -152,10 +153,8 @@ static int __init cf_check parse_ucode(const char *s)
 custom_param("ucode", parse_ucode);
 
 static void __init microcode_scan_module(
-    unsigned long *module_map,
-    const multiboot_info_t *mbi)
+    unsigned long *module_map, const struct boot_info *bi)
 {
-    module_t *mod = (module_t *)__va(mbi->mods_addr);
     uint64_t *_blob_start;
     unsigned long _blob_size;
     struct cpio_data cd;
@@ -177,16 +176,16 @@ static void __init microcode_scan_module(
     /*
      * Try all modules and see whichever could be the microcode blob.
      */
-    for ( i = 1 /* Ignore dom0 kernel */; i < mbi->mods_count; i++ )
+    for ( i = 1 /* Ignore dom0 kernel */; i < bi->nr_mods; i++ )
     {
         if ( !test_bit(i, module_map) )
             continue;
 
-        _blob_start = bootstrap_map(&mod[i]);
-        _blob_size = mod[i].mod_end;
+        _blob_start = bootstrap_map_bm(&bi->mods[i]);
+        _blob_size = bi->mods[i].size;
         if ( !_blob_start )
         {
-            printk("Could not map multiboot module #%d (size: %ld)\n",
+            printk("Could not map boot module #%d (size: %ld)\n",
                    i, _blob_size);
             continue;
         }
@@ -204,20 +203,18 @@ static void __init microcode_scan_module(
 }
 
 static void __init microcode_grab_module(
-    unsigned long *module_map,
-    const multiboot_info_t *mbi)
+    unsigned long *module_map, const struct boot_info *bi)
 {
-    module_t *mod = (module_t *)__va(mbi->mods_addr);
-
     if ( ucode_mod_idx < 0 )
-        ucode_mod_idx += mbi->mods_count;
-    if ( ucode_mod_idx <= 0 || ucode_mod_idx >= mbi->mods_count ||
+        ucode_mod_idx += bi->nr_mods;
+    if ( ucode_mod_idx <= 0 || ucode_mod_idx >= bi->nr_mods ||
          !__test_and_clear_bit(ucode_mod_idx, module_map) )
         goto scan;
-    ucode_mod = mod[ucode_mod_idx];
+    bi->mods[ucode_mod_idx].type = BOOTMOD_MICROCODE;
+    ucode_mod = *bi->mods[ucode_mod_idx].early_mod;
 scan:
     if ( ucode_scan )
-        microcode_scan_module(module_map, mbi);
+        microcode_scan_module(module_map, bi);
 }
 
 static struct microcode_ops __ro_after_init ucode_ops;
@@ -805,8 +802,8 @@ static int __init early_update_cache(const void *data, size_t len)
     return rc;
 }
 
-int __init microcode_init_cache(unsigned long *module_map,
-                                const struct multiboot_info *mbi)
+int __init microcode_init_cache(
+    unsigned long *module_map, const struct boot_info *bi)
 {
     int rc = 0;
 
@@ -815,7 +812,7 @@ int __init microcode_init_cache(unsigned long *module_map,
 
     if ( ucode_scan )
         /* Need to rescan the modules because they might have been relocated */
-        microcode_scan_module(module_map, mbi);
+        microcode_scan_module(module_map, bi);
 
     if ( ucode_mod.mod_end )
         rc = early_update_cache(bootstrap_map(&ucode_mod),
@@ -862,7 +859,7 @@ static int __init early_microcode_update_cpu(void)
 }
 
 int __init early_microcode_init(unsigned long *module_map,
-                                const struct multiboot_info *mbi)
+                                const struct boot_info *bi)
 {
     const struct cpuinfo_x86 *c = &boot_cpu_data;
     int rc = 0;
@@ -905,7 +902,7 @@ int __init early_microcode_init(unsigned long *module_map,
         return -ENODEV;
     }
 
-    microcode_grab_module(module_map, mbi);
+    microcode_grab_module(module_map, bi);
 
     if ( ucode_mod.mod_end || ucode_blob.size )
         rc = early_microcode_update_cpu();
diff --git a/xen/arch/x86/include/asm/bootinfo.h b/xen/arch/x86/include/asm/bootinfo.h
index 740308693629..1cef48457c66 100644
--- a/xen/arch/x86/include/asm/bootinfo.h
+++ b/xen/arch/x86/include/asm/bootinfo.h
@@ -17,6 +17,7 @@ enum bootmod_type {
     BOOTMOD_XEN,
     BOOTMOD_KERNEL,
     BOOTMOD_RAMDISK,
+    BOOTMOD_MICROCODE,
 };
 
 struct boot_module {
diff --git a/xen/arch/x86/include/asm/microcode.h b/xen/arch/x86/include/asm/microcode.h
index 57c08205d475..02fdb56bb82b 100644
--- a/xen/arch/x86/include/asm/microcode.h
+++ b/xen/arch/x86/include/asm/microcode.h
@@ -4,6 +4,8 @@
 #include <xen/types.h>
 #include <xen/percpu.h>
 
+#include <asm/bootinfo.h>
+
 #include <public/xen.h>
 
 struct multiboot_info;
@@ -22,12 +24,12 @@ struct cpu_signature {
 DECLARE_PER_CPU(struct cpu_signature, cpu_sig);
 
 void microcode_set_module(unsigned int idx);
-int microcode_update(XEN_GUEST_HANDLE(const_void) buf,
-                     unsigned long len, unsigned int flags);
-int early_microcode_init(unsigned long *module_map,
-                         const struct multiboot_info *mbi);
-int microcode_init_cache(unsigned long *module_map,
-                         const struct multiboot_info *mbi);
+int microcode_update(
+    XEN_GUEST_HANDLE(const_void) buf, unsigned long len, unsigned int flags);
+int early_microcode_init(
+    unsigned long *module_map, const struct boot_info *bi);
+int microcode_init_cache(
+    unsigned long *module_map, const struct boot_info *bi);
 int microcode_update_one(void);
 
 #endif /* ASM_X86__MICROCODE_H */
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index da75dfce083d..6495ab393576 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1398,7 +1398,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
      * TODO: load ucode earlier once multiboot modules become accessible
      * at an earlier stage.
      */
-    early_microcode_init(module_map, mbi);
+    early_microcode_init(module_map, boot_info);
 
     if ( xen_phys_start )
     {
@@ -1952,7 +1952,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
 
     timer_init();
 
-    microcode_init_cache(module_map, mbi); /* Needs xmalloc() */
+    microcode_init_cache(module_map, boot_info); /* Needs xmalloc() */
 
     tsx_init(); /* Needs microcode.  May change HLE/RTM feature bits. */
 
-- 
2.30.2

[PATCH v4 18/44] x86/boot: convert late microcode loading to struct boot_module

[Daniel P. Smith]

Remove the use of struct mod to hold the reference for the microcode,
converting the code to work with a struct boot_module.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/arch/x86/cpu/microcode/core.c | 31 +++++++++++++++----------------
 1 file changed, 15 insertions(+), 16 deletions(-)

diff --git a/xen/arch/x86/cpu/microcode/core.c b/xen/arch/x86/cpu/microcode/core.c
index 653a725173ba..cedb03beea54 100644
--- a/xen/arch/x86/cpu/microcode/core.c
+++ b/xen/arch/x86/cpu/microcode/core.c
@@ -28,7 +28,6 @@
 #include <xen/err.h>
 #include <xen/guest_access.h>
 #include <xen/init.h>
-#include <xen/multiboot.h>
 #include <xen/param.h>
 #include <xen/spinlock.h>
 #include <xen/stop_machine.h>
@@ -59,7 +58,7 @@
  */
 #define MICROCODE_UPDATE_TIMEOUT_US 1000000
 
-static module_t __initdata ucode_mod;
+static struct boot_module __initdata ucode_mod;
 static signed int __initdata ucode_mod_idx;
 static bool __initdata ucode_mod_forced;
 static unsigned int nr_cores;
@@ -93,7 +92,7 @@ struct ucode_mod_blob {
 
 static struct ucode_mod_blob __initdata ucode_blob;
 /*
- * By default we will NOT parse the multiboot modules to see if there is
+ * By default we will NOT parse the boot modules to see if there is
  * cpio image with the microcode images.
  */
 static bool __initdata ucode_scan;
@@ -198,7 +197,7 @@ static void __init microcode_scan_module(
             ucode_blob.data = cd.data;
             break;
         }
-        bootstrap_map(NULL);
+        bootstrap_map_bm(NULL);
     }
 }
 
@@ -211,7 +210,7 @@ static void __init microcode_grab_module(
          !__test_and_clear_bit(ucode_mod_idx, module_map) )
         goto scan;
     bi->mods[ucode_mod_idx].type = BOOTMOD_MICROCODE;
-    ucode_mod = *bi->mods[ucode_mod_idx].early_mod;
+    ucode_mod = bi->mods[ucode_mod_idx];
 scan:
     if ( ucode_scan )
         microcode_scan_module(module_map, bi);
@@ -749,14 +748,14 @@ static int __init cf_check microcode_init(void)
      */
     if ( ucode_blob.size )
     {
-        bootstrap_map(NULL);
+        bootstrap_map_bm(NULL);
         ucode_blob.size = 0;
         ucode_blob.data = NULL;
     }
-    else if ( ucode_mod.mod_end )
+    else if ( ucode_mod.size )
     {
-        bootstrap_map(NULL);
-        ucode_mod.mod_end = 0;
+        bootstrap_map_bm(NULL);
+        ucode_mod.size = 0;
     }
 
     return 0;
@@ -814,9 +813,9 @@ int __init microcode_init_cache(
         /* Need to rescan the modules because they might have been relocated */
         microcode_scan_module(module_map, bi);
 
-    if ( ucode_mod.mod_end )
-        rc = early_update_cache(bootstrap_map(&ucode_mod),
-                                ucode_mod.mod_end);
+    if ( ucode_mod.size )
+        rc = early_update_cache(bootstrap_map_bm(&ucode_mod),
+                                ucode_mod.size);
     else if ( ucode_blob.size )
         rc = early_update_cache(ucode_blob.data, ucode_blob.size);
 
@@ -835,10 +834,10 @@ static int __init early_microcode_update_cpu(void)
         len = ucode_blob.size;
         data = ucode_blob.data;
     }
-    else if ( ucode_mod.mod_end )
+    else if ( ucode_mod.size )
     {
-        len = ucode_mod.mod_end;
-        data = bootstrap_map(&ucode_mod);
+        len = ucode_mod.size;
+        data = bootstrap_map_bm(&ucode_mod);
     }
 
     if ( !data )
@@ -904,7 +903,7 @@ int __init early_microcode_init(unsigned long *module_map,
 
     microcode_grab_module(module_map, bi);
 
-    if ( ucode_mod.mod_end || ucode_blob.size )
+    if ( ucode_mod.size || ucode_blob.size )
         rc = early_microcode_update_cpu();
 
     /*
-- 
2.30.2

[PATCH v4 19/44] x86/boot: use consumed boot module flag for microcode

[Daniel P. Smith]

To track if the microcode boot module was loaded, a copy of the boot module is
kept. The size element of this copy is set to zero as the indicator that the
microcode was loaded. A side effect is that the modules have to be rescanned to
find the boot module post-relocation, so the cache copy can be created.

Use the consumed boot module flag to track the loading of the microcode boot
module. This removes the need to manipulate the boot module size element, no
longer requiring the copy, thus allowing it to be replaced by a reference. As a
result it is no longer necessary to rescan the boot modules after relocation
has occurred.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/arch/x86/cpu/microcode/core.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/xen/arch/x86/cpu/microcode/core.c b/xen/arch/x86/cpu/microcode/core.c
index cedb03beea54..83dd8bbe9fb1 100644
--- a/xen/arch/x86/cpu/microcode/core.c
+++ b/xen/arch/x86/cpu/microcode/core.c
@@ -58,7 +58,7 @@
  */
 #define MICROCODE_UPDATE_TIMEOUT_US 1000000
 
-static struct boot_module __initdata ucode_mod;
+static struct boot_module __initdata *ucode_mod;
 static signed int __initdata ucode_mod_idx;
 static bool __initdata ucode_mod_forced;
 static unsigned int nr_cores;
@@ -210,7 +210,7 @@ static void __init microcode_grab_module(
          !__test_and_clear_bit(ucode_mod_idx, module_map) )
         goto scan;
     bi->mods[ucode_mod_idx].type = BOOTMOD_MICROCODE;
-    ucode_mod = bi->mods[ucode_mod_idx];
+    ucode_mod = &bi->mods[ucode_mod_idx];
 scan:
     if ( ucode_scan )
         microcode_scan_module(module_map, bi);
@@ -752,10 +752,10 @@ static int __init cf_check microcode_init(void)
         ucode_blob.size = 0;
         ucode_blob.data = NULL;
     }
-    else if ( ucode_mod.size )
+    else if ( ucode_mod && !(ucode_mod->flags & BOOTMOD_FLAG_X86_CONSUMED) )
     {
         bootstrap_map_bm(NULL);
-        ucode_mod.size = 0;
+        ucode_mod->flags |= BOOTMOD_FLAG_X86_CONSUMED;
     }
 
     return 0;
@@ -809,14 +809,14 @@ int __init microcode_init_cache(
     if ( !ucode_ops.apply_microcode )
         return -ENODEV;
 
-    if ( ucode_scan )
-        /* Need to rescan the modules because they might have been relocated */
+    /* Scan if microcode was not detected earlier */
+    if ( !ucode_mod )
         microcode_scan_module(module_map, bi);
 
-    if ( ucode_mod.size )
-        rc = early_update_cache(bootstrap_map_bm(&ucode_mod),
-                                ucode_mod.size);
-    else if ( ucode_blob.size )
+    if ( ucode_mod && !(ucode_mod->flags & BOOTMOD_FLAG_X86_CONSUMED) )
+        rc = early_update_cache(bootstrap_map_bm(ucode_mod),
+                                ucode_mod->size);
+    else if ( ucode_mod && ucode_blob.size )
         rc = early_update_cache(ucode_blob.data, ucode_blob.size);
 
     return rc;
@@ -834,10 +834,10 @@ static int __init early_microcode_update_cpu(void)
         len = ucode_blob.size;
         data = ucode_blob.data;
     }
-    else if ( ucode_mod.size )
+    else if ( ucode_mod && !(ucode_mod->flags & BOOTMOD_FLAG_X86_CONSUMED) )
     {
-        len = ucode_mod.size;
-        data = bootstrap_map_bm(&ucode_mod);
+        len = ucode_mod->size;
+        data = bootstrap_map_bm(ucode_mod);
     }
 
     if ( !data )
@@ -903,7 +903,7 @@ int __init early_microcode_init(unsigned long *module_map,
 
     microcode_grab_module(module_map, bi);
 
-    if ( ucode_mod.size || ucode_blob.size )
+    if ( ucode_mod || ucode_blob.size )
         rc = early_microcode_update_cpu();
 
     /*
-- 
2.30.2

[PATCH v4 20/44] x86/boot: convert xsm policy loading to struct boot_module

[Daniel P. Smith]

Iterate through the unclaimed struct boot_module to see if any are an XSM FLASK
policy. If one is located, mark it as an xsm policy.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/arch/x86/include/asm/bootinfo.h |  1 +
 xen/arch/x86/setup.c                |  2 +-
 xen/include/xsm/xsm.h               | 11 +++++++----
 xen/xsm/xsm_core.c                  | 13 +++++++++++--
 xen/xsm/xsm_policy.c                | 15 ++++++++-------
 5 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/xen/arch/x86/include/asm/bootinfo.h b/xen/arch/x86/include/asm/bootinfo.h
index 1cef48457c66..1a325f66b3ba 100644
--- a/xen/arch/x86/include/asm/bootinfo.h
+++ b/xen/arch/x86/include/asm/bootinfo.h
@@ -18,6 +18,7 @@ enum bootmod_type {
     BOOTMOD_KERNEL,
     BOOTMOD_RAMDISK,
     BOOTMOD_MICROCODE,
+    BOOTMOD_XSM_POLICY,
 };
 
 struct boot_module {
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 6495ab393576..28b39d23f644 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1878,7 +1878,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
     mmio_ro_ranges = rangeset_new(NULL, "r/o mmio ranges",
                                   RANGESETF_prettyprint_hex);
 
-    xsm_multiboot_init(module_map, mbi);
+    xsm_multiboot_init(module_map, boot_info);
 
     /*
      * IOMMU-related ACPI table parsing may require some of the system domains
diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
index 627c0d2731af..9e511ef8878c 100644
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -17,7 +17,10 @@
 
 #include <xen/alternative-call.h>
 #include <xen/sched.h>
-#include <xen/multiboot.h>
+
+#ifdef CONFIG_MULTIBOOT
+#include <asm/bootinfo.h>
+#endif
 
 /* policy magic number (defined by XSM_MAGIC) */
 typedef uint32_t xsm_magic_t;
@@ -779,9 +782,9 @@ static inline int xsm_argo_send(const struct domain *d, const struct domain *t)
 
 #ifdef CONFIG_MULTIBOOT
 int xsm_multiboot_init(
-    unsigned long *module_map, const multiboot_info_t *mbi);
+    unsigned long *module_map, const struct boot_info *bi);
 int xsm_multiboot_policy_init(
-    unsigned long *module_map, const multiboot_info_t *mbi,
+    unsigned long *module_map, const struct boot_info *bi,
     void **policy_buffer, size_t *policy_size);
 #endif
 
@@ -829,7 +832,7 @@ static const inline struct xsm_ops *silo_init(void)
 
 #ifdef CONFIG_MULTIBOOT
 static inline int xsm_multiboot_init (
-    unsigned long *module_map, const multiboot_info_t *mbi)
+    unsigned long *module_map, const struct boot_info *bi)
 {
     return 0;
 }
diff --git a/xen/xsm/xsm_core.c b/xen/xsm/xsm_core.c
index eaa028109bde..9f87ec4f6754 100644
--- a/xen/xsm/xsm_core.c
+++ b/xen/xsm/xsm_core.c
@@ -21,6 +21,7 @@
 #ifdef CONFIG_XSM
 
 #ifdef CONFIG_MULTIBOOT
+#include <asm/bootinfo.h>
 #include <asm/setup.h>
 #endif
 
@@ -140,7 +141,7 @@ static int __init xsm_core_init(const void *policy_buffer, size_t policy_size)
 
 #ifdef CONFIG_MULTIBOOT
 int __init xsm_multiboot_init(
-    unsigned long *module_map, const multiboot_info_t *mbi)
+    unsigned long *module_map, const struct boot_info *bi)
 {
     int ret = 0;
     void *policy_buffer = NULL;
@@ -150,7 +151,7 @@ int __init xsm_multiboot_init(
 
     if ( XSM_MAGIC )
     {
-        ret = xsm_multiboot_policy_init(module_map, mbi, &policy_buffer,
+        ret = xsm_multiboot_policy_init(module_map, bi, &policy_buffer,
                                         &policy_size);
         if ( ret )
         {
@@ -161,6 +162,14 @@ int __init xsm_multiboot_init(
     }
 
     ret = xsm_core_init(policy_buffer, policy_size);
+    if ( ret == 0 )
+    {
+        int idx = first_boot_module_index(bi, BOOTMOD_XSM_POLICY);
+
+        /* If the policy was loaded from a boot module, mark it consumed */
+        if ( idx >= 0 )
+            bi->mods[idx].flags |= BOOTMOD_FLAG_X86_CONSUMED;
+    }
     bootstrap_map(NULL);
 
     return 0;
diff --git a/xen/xsm/xsm_policy.c b/xen/xsm/xsm_policy.c
index 8dafbc93810f..e4c94afd108d 100644
--- a/xen/xsm/xsm_policy.c
+++ b/xen/xsm/xsm_policy.c
@@ -21,6 +21,7 @@
 #include <xsm/xsm.h>
 #ifdef CONFIG_MULTIBOOT
 #include <xen/multiboot.h>
+#include <asm/bootinfo.h>
 #include <asm/setup.h>
 #endif
 #include <xen/bitops.h>
@@ -31,11 +32,10 @@
 
 #ifdef CONFIG_MULTIBOOT
 int __init xsm_multiboot_policy_init(
-    unsigned long *module_map, const multiboot_info_t *mbi,
+    unsigned long *module_map, const struct boot_info *bi,
     void **policy_buffer, size_t *policy_size)
 {
     int i;
-    module_t *mod = (module_t *)__va(mbi->mods_addr);
     int rc = 0;
     u32 *_policy_start;
     unsigned long _policy_len;
@@ -44,13 +44,13 @@ int __init xsm_multiboot_policy_init(
      * Try all modules and see whichever could be the binary policy.
      * Adjust module_map for the module that is the binary policy.
      */
-    for ( i = mbi->mods_count-1; i >= 1; i-- )
+    for ( i = bi->nr_mods-1; i >= 1; i-- )
     {
-        if ( !test_bit(i, module_map) )
+        if ( bi->mods[i].type != BOOTMOD_UNKNOWN )
             continue;
 
-        _policy_start = bootstrap_map(mod + i);
-        _policy_len   = mod[i].mod_end;
+        _policy_start = bootstrap_map_bm(&bi->mods[i]);
+        _policy_len   = bi->mods[i].size;
 
         if ( (xsm_magic_t)(*_policy_start) == XSM_MAGIC )
         {
@@ -61,11 +61,12 @@ int __init xsm_multiboot_policy_init(
                    _policy_len,_policy_start);
 
             __clear_bit(i, module_map);
+            bi->mods[i].type = BOOTMOD_XSM_POLICY;
             break;
 
         }
 
-        bootstrap_map(NULL);
+        bootstrap_map_bm(NULL);
     }
 
     return rc;
-- 
2.30.2

[PATCH v4 21/44] x86/boot: convert ramdisk locating to struct boot_module

[Daniel P. Smith]

Locate the first unclaimed struct boot_module and mark it as ramdisk. If there
are any remaining unclaimed struct boot_module instances, report to the
console.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/arch/x86/setup.c | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 28b39d23f644..189c45302fab 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -2100,20 +2100,30 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
            cpu_has_nx ? XENLOG_INFO : XENLOG_WARNING "Warning: ",
            cpu_has_nx ? "" : "not ");
 
-    initrdidx = find_first_bit(module_map, boot_info->nr_mods);
-    boot_info->mods[initrdidx].type = BOOTMOD_RAMDISK;
-    boot_info->mods[initrdidx].flags |= BOOTMOD_FLAG_X86_CONSUMED;
-    if ( bitmap_weight(module_map, boot_info->nr_mods) > 1 )
-        printk(XENLOG_WARNING
-               "Multiple initrd candidates, picking module #%u\n",
-               initrdidx);
+    /*
+     * At this point all capabilities that consume boot modules should have
+     * claimed their boot modules. Find the first unclaimed boot module and
+     * claim it as the initrd ramdisk. Do a second search to see if there are
+     * any remaining unclaimed boot modules, and report them as unusued initrd
+     * candidates.
+     */
+    initrdidx = first_boot_module_index(boot_info, BOOTMOD_UNKNOWN);
+    if ( initrdidx >= 0 )
+    {
+        boot_info->mods[initrdidx].type = BOOTMOD_RAMDISK;
+        boot_info->mods[initrdidx].flags |= BOOTMOD_FLAG_X86_CONSUMED;
+        if ( first_boot_module_index(boot_info, BOOTMOD_UNKNOWN) >= 0 )
+            printk(XENLOG_WARNING
+                   "Multiple initrd candidates, picking module #%u\n",
+                   initrdidx);
+    }
 
     /*
      * 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(boot_info->mods[0].early_mod, boot_info->mods[0].headroom,
-                       initrdidx < boot_info->nr_mods ?
+                       (initrdidx >= 0 && initrdidx < boot_info->nr_mods) ?
                             boot_info->mods[initrdidx].early_mod : NULL,
                        kextra, boot_info->boot_loader_name);
     if ( !dom0 )
-- 
2.30.2

[PATCH v4 22/44] x86/boot: remove module_map usage from microcode loading

[Daniel P. Smith]

With all consumers of module_map converted, remove usage of it
by the microcode loading logic.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/arch/x86/cpu/microcode/core.c    | 22 +++++++++-------------
 xen/arch/x86/include/asm/microcode.h |  6 ++----
 xen/arch/x86/setup.c                 |  4 ++--
 3 files changed, 13 insertions(+), 19 deletions(-)

diff --git a/xen/arch/x86/cpu/microcode/core.c b/xen/arch/x86/cpu/microcode/core.c
index 83dd8bbe9fb1..a465a676d176 100644
--- a/xen/arch/x86/cpu/microcode/core.c
+++ b/xen/arch/x86/cpu/microcode/core.c
@@ -151,8 +151,7 @@ static int __init cf_check parse_ucode(const char *s)
 }
 custom_param("ucode", parse_ucode);
 
-static void __init microcode_scan_module(
-    unsigned long *module_map, const struct boot_info *bi)
+static void __init microcode_scan_module(const struct boot_info *bi)
 {
     uint64_t *_blob_start;
     unsigned long _blob_size;
@@ -177,7 +176,7 @@ static void __init microcode_scan_module(
      */
     for ( i = 1 /* Ignore dom0 kernel */; i < bi->nr_mods; i++ )
     {
-        if ( !test_bit(i, module_map) )
+        if ( bi->mods[i].type != BOOTMOD_UNKNOWN )
             continue;
 
         _blob_start = bootstrap_map_bm(&bi->mods[i]);
@@ -201,19 +200,18 @@ static void __init microcode_scan_module(
     }
 }
 
-static void __init microcode_grab_module(
-    unsigned long *module_map, const struct boot_info *bi)
+static void __init microcode_grab_module(const struct boot_info *bi)
 {
     if ( ucode_mod_idx < 0 )
         ucode_mod_idx += bi->nr_mods;
     if ( ucode_mod_idx <= 0 || ucode_mod_idx >= bi->nr_mods ||
-         !__test_and_clear_bit(ucode_mod_idx, module_map) )
+         (bi->mods[ucode_mod_idx].type != BOOTMOD_UNKNOWN) )
         goto scan;
     bi->mods[ucode_mod_idx].type = BOOTMOD_MICROCODE;
     ucode_mod = &bi->mods[ucode_mod_idx];
 scan:
     if ( ucode_scan )
-        microcode_scan_module(module_map, bi);
+        microcode_scan_module(bi);
 }
 
 static struct microcode_ops __ro_after_init ucode_ops;
@@ -801,8 +799,7 @@ static int __init early_update_cache(const void *data, size_t len)
     return rc;
 }
 
-int __init microcode_init_cache(
-    unsigned long *module_map, const struct boot_info *bi)
+int __init microcode_init_cache(const struct boot_info *bi)
 {
     int rc = 0;
 
@@ -811,7 +808,7 @@ int __init microcode_init_cache(
 
     /* Scan if microcode was not detected earlier */
     if ( !ucode_mod )
-        microcode_scan_module(module_map, bi);
+        microcode_scan_module(bi);
 
     if ( ucode_mod && !(ucode_mod->flags & BOOTMOD_FLAG_X86_CONSUMED) )
         rc = early_update_cache(bootstrap_map_bm(ucode_mod),
@@ -857,8 +854,7 @@ static int __init early_microcode_update_cpu(void)
     return microcode_update_cpu(patch);
 }
 
-int __init early_microcode_init(unsigned long *module_map,
-                                const struct boot_info *bi)
+int __init early_microcode_init(const struct boot_info *bi)
 {
     const struct cpuinfo_x86 *c = &boot_cpu_data;
     int rc = 0;
@@ -901,7 +897,7 @@ int __init early_microcode_init(unsigned long *module_map,
         return -ENODEV;
     }
 
-    microcode_grab_module(module_map, bi);
+    microcode_grab_module(bi);
 
     if ( ucode_mod || ucode_blob.size )
         rc = early_microcode_update_cpu();
diff --git a/xen/arch/x86/include/asm/microcode.h b/xen/arch/x86/include/asm/microcode.h
index 02fdb56bb82b..6abd6c1ff510 100644
--- a/xen/arch/x86/include/asm/microcode.h
+++ b/xen/arch/x86/include/asm/microcode.h
@@ -26,10 +26,8 @@ DECLARE_PER_CPU(struct cpu_signature, cpu_sig);
 void microcode_set_module(unsigned int idx);
 int microcode_update(
     XEN_GUEST_HANDLE(const_void) buf, unsigned long len, unsigned int flags);
-int early_microcode_init(
-    unsigned long *module_map, const struct boot_info *bi);
-int microcode_init_cache(
-    unsigned long *module_map, const struct boot_info *bi);
+int early_microcode_init(const struct boot_info *bi);
+int microcode_init_cache(const struct boot_info *bi);
 int microcode_update_one(void);
 
 #endif /* ASM_X86__MICROCODE_H */
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 189c45302fab..541a956923d3 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1398,7 +1398,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
      * TODO: load ucode earlier once multiboot modules become accessible
      * at an earlier stage.
      */
-    early_microcode_init(module_map, boot_info);
+    early_microcode_init(boot_info);
 
     if ( xen_phys_start )
     {
@@ -1952,7 +1952,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
 
     timer_init();
 
-    microcode_init_cache(module_map, boot_info); /* Needs xmalloc() */
+    microcode_init_cache(boot_info); /* Needs xmalloc() */
 
     tsx_init(); /* Needs microcode.  May change HLE/RTM feature bits. */
 
-- 
2.30.2

[PATCH v4 23/44] x86/boot: remove module_map usage from xsm policy loading

[Daniel P. Smith]

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/arch/x86/setup.c  | 2 +-
 xen/include/xsm/xsm.h | 9 +++------
 xen/xsm/xsm_core.c    | 6 ++----
 xen/xsm/xsm_policy.c  | 5 +----
 4 files changed, 7 insertions(+), 15 deletions(-)

diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 541a956923d3..90c2ddb3728a 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1878,7 +1878,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
     mmio_ro_ranges = rangeset_new(NULL, "r/o mmio ranges",
                                   RANGESETF_prettyprint_hex);
 
-    xsm_multiboot_init(module_map, boot_info);
+    xsm_multiboot_init(boot_info);
 
     /*
      * IOMMU-related ACPI table parsing may require some of the system domains
diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h
index 9e511ef8878c..3f05d09880d8 100644
--- a/xen/include/xsm/xsm.h
+++ b/xen/include/xsm/xsm.h
@@ -781,11 +781,9 @@ static inline int xsm_argo_send(const struct domain *d, const struct domain *t)
 #endif /* XSM_NO_WRAPPERS */
 
 #ifdef CONFIG_MULTIBOOT
-int xsm_multiboot_init(
-    unsigned long *module_map, const struct boot_info *bi);
+int xsm_multiboot_init(const struct boot_info *bi);
 int xsm_multiboot_policy_init(
-    unsigned long *module_map, const struct boot_info *bi,
-    void **policy_buffer, size_t *policy_size);
+    const struct boot_info *bi, void **policy_buffer, size_t *policy_size);
 #endif
 
 #ifdef CONFIG_HAS_DEVICE_TREE
@@ -831,8 +829,7 @@ static const inline struct xsm_ops *silo_init(void)
 #include <xsm/dummy.h>
 
 #ifdef CONFIG_MULTIBOOT
-static inline int xsm_multiboot_init (
-    unsigned long *module_map, const struct boot_info *bi)
+static inline int xsm_multiboot_init(const struct boot_info *bi)
 {
     return 0;
 }
diff --git a/xen/xsm/xsm_core.c b/xen/xsm/xsm_core.c
index 9f87ec4f6754..f816c94fb5aa 100644
--- a/xen/xsm/xsm_core.c
+++ b/xen/xsm/xsm_core.c
@@ -140,8 +140,7 @@ static int __init xsm_core_init(const void *policy_buffer, size_t policy_size)
 }
 
 #ifdef CONFIG_MULTIBOOT
-int __init xsm_multiboot_init(
-    unsigned long *module_map, const struct boot_info *bi)
+int __init xsm_multiboot_init(const struct boot_info *bi)
 {
     int ret = 0;
     void *policy_buffer = NULL;
@@ -151,8 +150,7 @@ int __init xsm_multiboot_init(
 
     if ( XSM_MAGIC )
     {
-        ret = xsm_multiboot_policy_init(module_map, bi, &policy_buffer,
-                                        &policy_size);
+        ret = xsm_multiboot_policy_init(bi, &policy_buffer, &policy_size);
         if ( ret )
         {
             bootstrap_map(NULL);
diff --git a/xen/xsm/xsm_policy.c b/xen/xsm/xsm_policy.c
index e4c94afd108d..ac09dd808b27 100644
--- a/xen/xsm/xsm_policy.c
+++ b/xen/xsm/xsm_policy.c
@@ -32,8 +32,7 @@
 
 #ifdef CONFIG_MULTIBOOT
 int __init xsm_multiboot_policy_init(
-    unsigned long *module_map, const struct boot_info *bi,
-    void **policy_buffer, size_t *policy_size)
+    const struct boot_info *bi, void **policy_buffer, size_t *policy_size)
 {
     int i;
     int rc = 0;
@@ -42,7 +41,6 @@ int __init xsm_multiboot_policy_init(
 
     /*
      * Try all modules and see whichever could be the binary policy.
-     * Adjust module_map for the module that is the binary policy.
      */
     for ( i = bi->nr_mods-1; i >= 1; i-- )
     {
@@ -60,7 +58,6 @@ int __init xsm_multiboot_policy_init(
             printk("Policy len %#lx, start at %p.\n",
                    _policy_len,_policy_start);
 
-            __clear_bit(i, module_map);
             bi->mods[i].type = BOOTMOD_XSM_POLICY;
             break;
 
-- 
2.30.2

[PATCH v4 24/44] x86/boot: remove module_map usage by ramdisk loading

[Daniel P. Smith]

The ramdisk loading is the last user of module_map, remove
its usage and any remaining remnants of module_map.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/arch/x86/setup.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 90c2ddb3728a..5f42d1049110 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1053,7 +1053,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
     unsigned int initrdidx, num_parked = 0;
     multiboot_info_t *mbi;
     module_t *mod;
-    unsigned long nr_pages, raw_max_page, module_map[1];
+    unsigned long nr_pages, raw_max_page;
     int i, j, e820_warn = 0, bytes = 0;
     unsigned long eb_start, eb_end;
     bool acpi_boot_table_init_done = false, relocated = false;
@@ -1203,15 +1203,14 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
         panic("dom0 kernel not specified. Check bootloader configuration\n");
 
     /* Check that we don't have a silly number of modules. */
-    if ( boot_info->nr_mods > sizeof(module_map) * 8 )
+    if ( boot_info->nr_mods > MAX_NR_BOOTMODS + 1 )
     {
-        boot_info->nr_mods = sizeof(module_map) * 8;
+        boot_info->nr_mods = MAX_NR_BOOTMODS + 1;
         printk("Excessive multiboot modules - using the first %u only\n",
                boot_info->nr_mods);
     }
 
-    bitmap_fill(module_map, boot_info->nr_mods);
-    __clear_bit(0, module_map); /* Dom0 kernel is always first */
+    /* Dom0 kernel is always first */
     boot_info->mods[0].type = BOOTMOD_KERNEL;
     boot_info->mods[0].flags |= BOOTMOD_FLAG_X86_CONSUMED;
 
-- 
2.30.2

[PATCH v4 25/44] x86/boot: convert create_dom0 to use boot info

[Daniel P. Smith]

This commit changes create_dom0 to no longer take the individual components and
take struct boot_info instead. Internally, it is changed to locate the kernel
and ramdisk details from struct boot_info.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/arch/x86/setup.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 5f42d1049110..6a613c4847b0 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -946,10 +946,8 @@ static unsigned int __init copy_bios_e820(struct e820entry *map, unsigned int li
     return n;
 }
 
-static struct domain *__init create_dom0(const module_t *image,
-                                         unsigned long headroom,
-                                         module_t *initrd, const char *kextra,
-                                         const char *loader)
+static struct domain *__init create_dom0(const struct boot_info *bi,
+                                         const char *kextra)
 {
     static char __initdata cmdline[MAX_GUEST_CMDLINE];
 
@@ -964,9 +962,21 @@ static struct domain *__init create_dom0(const module_t *image,
             .misc_flags = opt_dom0_msr_relaxed ? XEN_X86_MSR_RELAXED : 0,
         },
     };
+    int headroom, mod_idx = first_boot_module_index(bi, BOOTMOD_RAMDISK);
+    module_t *image, *initrd;
     struct domain *d;
     domid_t domid;
 
+    /* Map boot_module to mb1 module for dom0 */
+    image = bi->mods[0].early_mod;
+    headroom = bi->mods[0].headroom;
+
+    /* Map boot_module to mb1 module for initrd */
+    if ( mod_idx < 0 )
+        initrd = NULL;
+    else
+        initrd = bi->mods[mod_idx].early_mod;
+
     if ( opt_dom0_pvh )
     {
         dom0_cfg.flags |= (XEN_DOMCTL_CDF_hvm |
@@ -995,7 +1005,8 @@ static struct domain *__init create_dom0(const module_t *image,
     if ( image->string || kextra )
     {
         if ( image->string )
-            safe_strcpy(cmdline, cmdline_cook(__va(image->string), loader));
+            safe_strcpy(cmdline, cmdline_cook(__va(image->string),
+                        bi->boot_loader_name));
 
         if ( kextra )
             /* kextra always includes exactly one leading space. */
@@ -2121,10 +2132,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(boot_info->mods[0].early_mod, boot_info->mods[0].headroom,
-                       (initrdidx >= 0 && initrdidx < boot_info->nr_mods) ?
-                            boot_info->mods[initrdidx].early_mod : NULL,
-                       kextra, boot_info->boot_loader_name);
+    dom0 = create_dom0(boot_info, kextra);
     if ( !dom0 )
         panic("Could not set up DOM0 guest OS\n");
 
-- 
2.30.2

[PATCH v4 26/44] x86/boot: convert construct_dom0 to use struct boot_module

[Daniel P. Smith]

The construct_dom0 function is converted to consume struct boot_module
instances for the kernel and ramdisk. With this change, it is no longer
necessary for the internal use of struct mod by create_dom0, so they are
changed to struct boot_module.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/arch/x86/dom0_build.c        | 10 +++++-----
 xen/arch/x86/include/asm/setup.h |  6 ++----
 xen/arch/x86/setup.c             | 17 ++++++++---------
 3 files changed, 15 insertions(+), 18 deletions(-)

diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
index 8d56705a0861..fb1b7e5d11cf 100644
--- a/xen/arch/x86/dom0_build.c
+++ b/xen/arch/x86/dom0_build.c
@@ -13,6 +13,7 @@
 #include <xen/softirq.h>
 
 #include <asm/amd.h>
+#include <asm/bootinfo.h>
 #include <asm/dom0_build.h>
 #include <asm/guest.h>
 #include <asm/hpet.h>
@@ -596,9 +597,8 @@ int __init dom0_setup_permissions(struct domain *d)
     return rc;
 }
 
-int __init construct_dom0(struct domain *d, const module_t *image,
-                          unsigned long image_headroom, module_t *initrd,
-                          const char *cmdline)
+int __init construct_dom0(struct domain *d, const struct boot_module *image,
+                          struct boot_module *initrd, const char *cmdline)
 {
     int rc;
 
@@ -610,9 +610,9 @@ int __init construct_dom0(struct domain *d, const module_t *image,
     process_pending_softirqs();
 
     if ( is_hvm_domain(d) )
-        rc = dom0_construct_pvh(d, image, image_headroom, initrd, cmdline);
+        rc = dom0_construct_pvh(d, image->early_mod, image->headroom, initrd->early_mod, cmdline);
     else if ( is_pv_domain(d) )
-        rc = dom0_construct_pv(d, image, image_headroom, initrd, cmdline);
+        rc = dom0_construct_pv(d, image->early_mod, image->headroom, initrd->early_mod, cmdline);
     else
         panic("Cannot construct Dom0. No guest interface available\n");
 
diff --git a/xen/arch/x86/include/asm/setup.h b/xen/arch/x86/include/asm/setup.h
index 165ca744ba34..19e2ad95b523 100644
--- a/xen/arch/x86/include/asm/setup.h
+++ b/xen/arch/x86/include/asm/setup.h
@@ -27,10 +27,8 @@ void subarch_init_memory(void);
 void init_IRQ(void);
 
 int construct_dom0(
-    struct domain *d,
-    const module_t *image, unsigned long image_headroom,
-    module_t *initrd,
-    const char *cmdline);
+    struct domain *d, const struct boot_module *image,
+    struct boot_module *initrd, const char *cmdline);
 void setup_io_bitmap(struct domain *d);
 
 unsigned long initial_images_nrpages(nodeid_t node);
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 6a613c4847b0..f58eb21d00d7 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -962,20 +962,19 @@ static struct domain *__init create_dom0(const struct boot_info *bi,
             .misc_flags = opt_dom0_msr_relaxed ? XEN_X86_MSR_RELAXED : 0,
         },
     };
-    int headroom, mod_idx = first_boot_module_index(bi, BOOTMOD_RAMDISK);
-    module_t *image, *initrd;
+    int mod_idx = first_boot_module_index(bi, BOOTMOD_RAMDISK);
+    struct boot_module *image, *initrd;
     struct domain *d;
     domid_t domid;
 
     /* Map boot_module to mb1 module for dom0 */
-    image = bi->mods[0].early_mod;
-    headroom = bi->mods[0].headroom;
+    image = &bi->mods[0];
 
     /* Map boot_module to mb1 module for initrd */
     if ( mod_idx < 0 )
         initrd = NULL;
     else
-        initrd = bi->mods[mod_idx].early_mod;
+        initrd = &bi->mods[mod_idx];
 
     if ( opt_dom0_pvh )
     {
@@ -1002,10 +1001,10 @@ static struct domain *__init create_dom0(const struct boot_info *bi,
         panic("Error creating d%uv0\n", domid);
 
     /* Grab the DOM0 command line. */
-    if ( image->string || kextra )
+    if ( image->early_mod->string || kextra )
     {
-        if ( image->string )
-            safe_strcpy(cmdline, cmdline_cook(__va(image->string),
+        if ( image->early_mod->string )
+            safe_strcpy(cmdline, cmdline_cook(__va(image->early_mod->string),
                         bi->boot_loader_name));
 
         if ( kextra )
@@ -1040,7 +1039,7 @@ static struct domain *__init create_dom0(const struct boot_info *bi,
         write_cr4(read_cr4() & ~X86_CR4_SMAP);
     }
 
-    if ( construct_dom0(d, image, headroom, initrd, cmdline) != 0 )
+    if ( construct_dom0(d, image, initrd, cmdline) != 0 )
         panic("Could not construct domain 0\n");
 
     if ( cpu_has_smap )
-- 
2.30.2

[PATCH v4 27/44] x86/boot: relocate kextra into boot info

[Daniel P. Smith]

Move kextra into struct boot_info, thus no longer needed to be passed as a
parameter to create_dom0.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/arch/x86/include/asm/bootinfo.h |  2 ++
 xen/arch/x86/setup.c                | 12 ++++++------
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/xen/arch/x86/include/asm/bootinfo.h b/xen/arch/x86/include/asm/bootinfo.h
index 1a325f66b3ba..06b3ecaf7bec 100644
--- a/xen/arch/x86/include/asm/bootinfo.h
+++ b/xen/arch/x86/include/asm/bootinfo.h
@@ -41,6 +41,8 @@ struct boot_info {
     const char *boot_loader_name;
     const char *cmdline;
 
+    const char *kextra;
+
     paddr_t mmap_addr;
     uint32_t mmap_length;
 };
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index f58eb21d00d7..76f25e9d620c 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -946,8 +946,7 @@ static unsigned int __init copy_bios_e820(struct e820entry *map, unsigned int li
     return n;
 }
 
-static struct domain *__init create_dom0(const struct boot_info *bi,
-                                         const char *kextra)
+static struct domain *__init create_dom0(const struct boot_info *bi)
 {
     static char __initdata cmdline[MAX_GUEST_CMDLINE];
 
@@ -1001,15 +1000,15 @@ static struct domain *__init create_dom0(const struct boot_info *bi,
         panic("Error creating d%uv0\n", domid);
 
     /* Grab the DOM0 command line. */
-    if ( image->early_mod->string || kextra )
+    if ( image->early_mod->string || bi->kextra )
     {
         if ( image->early_mod->string )
             safe_strcpy(cmdline, cmdline_cook(__va(image->early_mod->string),
                         bi->boot_loader_name));
 
-        if ( kextra )
+        if ( bi->kextra )
             /* kextra always includes exactly one leading space. */
-            safe_strcat(cmdline, kextra);
+            safe_strcat(cmdline, bi->kextra);
 
         /* Append any extra parameters. */
         if ( skip_ioapic_setup && !strstr(cmdline, "noapic") )
@@ -1119,6 +1118,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
         *kextra = '\0';
         kextra += 3;
         while ( kextra[1] == ' ' ) kextra++;
+        boot_info->kextra = kextra;
     }
     cmdline_parse(boot_info->cmdline);
 
@@ -2131,7 +2131,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(boot_info, kextra);
+    dom0 = create_dom0(boot_info);
     if ( !dom0 )
         panic("Could not set up DOM0 guest OS\n");
 
-- 
2.30.2

[PATCH v4 28/44] x86/boot: add cmdline to struct boot_module

[Daniel P. Smith]

Add a char pointer field, cmdline, to struct boot_module to hold the address
pointed to by the string field of struct mod. This removes the need to use the
early_mod field to get to the dom0 kernel command line.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/arch/x86/include/asm/bootinfo.h | 2 ++
 xen/arch/x86/setup.c                | 9 ++++++---
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/include/asm/bootinfo.h b/xen/arch/x86/include/asm/bootinfo.h
index 06b3ecaf7bec..edd5c435d032 100644
--- a/xen/arch/x86/include/asm/bootinfo.h
+++ b/xen/arch/x86/include/asm/bootinfo.h
@@ -30,6 +30,8 @@ struct boot_module {
 #define BOOTMOD_FLAG_X86_RELOCATED     (1U << 0)
 #define BOOTMOD_FLAG_X86_CONSUMED      (1U << 1)
 
+    const char *cmdline;
+
     paddr_t start;
     size_t size;
 };
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 76f25e9d620c..8ba32c6b6bfa 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -314,6 +314,8 @@ static void __init multiboot_to_bootinfo(multiboot_info_t *mbi, module_t *mods)
     {
         boot_mods[i].early_mod = &mods[i];
 
+        boot_mods[i].cmdline = (char *)(paddr_t)mods[i].string;
+
         boot_mods[i].start = (paddr_t)mods[i].mod_start;
         boot_mods[i].size = mods[i].mod_end - mods[i].mod_start;
     }
@@ -1000,10 +1002,11 @@ static struct domain *__init create_dom0(const struct boot_info *bi)
         panic("Error creating d%uv0\n", domid);
 
     /* Grab the DOM0 command line. */
-    if ( image->early_mod->string || bi->kextra )
+    if ( image->cmdline || bi->kextra )
     {
-        if ( image->early_mod->string )
-            safe_strcpy(cmdline, cmdline_cook(__va(image->early_mod->string),
+        if ( image->cmdline )
+            safe_strcpy(cmdline,
+                        cmdline_cook(__va((unsigned long)image->cmdline),
                         bi->boot_loader_name));
 
         if ( bi->kextra )
-- 
2.30.2

[PATCH v4 29/44] x86/boot: convert dom0_construct_pv image param to struct boot_module

[Daniel P. Smith]

This changes the type for the image parameter of dom0_construct_pv to be struct
boot_module. Removing the usage of early_mod field for kernel module.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/arch/x86/dom0_build.c             | 2 +-
 xen/arch/x86/include/asm/dom0_build.h | 7 +++----
 xen/arch/x86/pv/dom0_build.c          | 9 ++++-----
 3 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
index fb1b7e5d11cf..42043230faff 100644
--- a/xen/arch/x86/dom0_build.c
+++ b/xen/arch/x86/dom0_build.c
@@ -612,7 +612,7 @@ int __init construct_dom0(struct domain *d, const struct boot_module *image,
     if ( is_hvm_domain(d) )
         rc = dom0_construct_pvh(d, image->early_mod, image->headroom, initrd->early_mod, cmdline);
     else if ( is_pv_domain(d) )
-        rc = dom0_construct_pv(d, image->early_mod, image->headroom, initrd->early_mod, cmdline);
+        rc = dom0_construct_pv(d, image, initrd->early_mod, cmdline);
     else
         panic("Cannot construct Dom0. No guest interface available\n");
 
diff --git a/xen/arch/x86/include/asm/dom0_build.h b/xen/arch/x86/include/asm/dom0_build.h
index 107c1ff98367..a1f36f7d360d 100644
--- a/xen/arch/x86/include/asm/dom0_build.h
+++ b/xen/arch/x86/include/asm/dom0_build.h
@@ -13,10 +13,9 @@ unsigned long dom0_compute_nr_pages(struct domain *d,
                                     unsigned long initrd_len);
 int dom0_setup_permissions(struct domain *d);
 
-int dom0_construct_pv(struct domain *d, const module_t *image,
-                      unsigned long image_headroom,
-                      module_t *initrd,
-                      const char *cmdline);
+int dom0_construct_pv(
+    struct domain *d, const struct boot_module *image,
+    module_t *initrd, const char *cmdline);
 
 int dom0_construct_pvh(struct domain *d, const module_t *image,
                        unsigned long image_headroom,
diff --git a/xen/arch/x86/pv/dom0_build.c b/xen/arch/x86/pv/dom0_build.c
index 57e58a02e707..db908ddb78b5 100644
--- a/xen/arch/x86/pv/dom0_build.c
+++ b/xen/arch/x86/pv/dom0_build.c
@@ -355,8 +355,7 @@ static struct page_info * __init alloc_chunk(struct domain *d,
 }
 
 int __init dom0_construct_pv(struct domain *d,
-                             const module_t *image,
-                             unsigned long image_headroom,
+                             const struct boot_module *image,
                              module_t *initrd,
                              const char *cmdline)
 {
@@ -374,9 +373,9 @@ int __init dom0_construct_pv(struct domain *d,
     unsigned int flush_flags = 0;
     start_info_t *si;
     struct vcpu *v = d->vcpu[0];
-    void *image_base = bootstrap_map(image);
-    unsigned long image_len = image->mod_end;
-    void *image_start = image_base + image_headroom;
+    void *image_base = bootstrap_map_bm(image);
+    unsigned long image_len = image->size;
+    void *image_start = image_base + image->headroom;
     unsigned long initrd_len = initrd ? initrd->mod_end : 0;
     l4_pgentry_t *l4tab = NULL, *l4start = NULL;
     l3_pgentry_t *l3tab = NULL, *l3start = NULL;
-- 
2.30.2

[PATCH v4 30/44] x86/boot: convert dom0_construct_pv initrd param to struct boot_module

[Daniel P. Smith]

This changes the type for the initrd parameter of dom0_construct_pv to be struct
boot_module. This conversion requires several adjustments throughout dom0_construct_pv
to account for the type change. Removes the usage of early_mod field for ramdisk module.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/arch/x86/dom0_build.c             |  2 +-
 xen/arch/x86/include/asm/dom0_build.h |  2 +-
 xen/arch/x86/pv/dom0_build.c          | 19 ++++++++++---------
 3 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
index 42043230faff..c39588b346b0 100644
--- a/xen/arch/x86/dom0_build.c
+++ b/xen/arch/x86/dom0_build.c
@@ -612,7 +612,7 @@ int __init construct_dom0(struct domain *d, const struct boot_module *image,
     if ( is_hvm_domain(d) )
         rc = dom0_construct_pvh(d, image->early_mod, image->headroom, initrd->early_mod, cmdline);
     else if ( is_pv_domain(d) )
-        rc = dom0_construct_pv(d, image, initrd->early_mod, cmdline);
+        rc = dom0_construct_pv(d, image, initrd, cmdline);
     else
         panic("Cannot construct Dom0. No guest interface available\n");
 
diff --git a/xen/arch/x86/include/asm/dom0_build.h b/xen/arch/x86/include/asm/dom0_build.h
index a1f36f7d360d..2ce5ea3851af 100644
--- a/xen/arch/x86/include/asm/dom0_build.h
+++ b/xen/arch/x86/include/asm/dom0_build.h
@@ -15,7 +15,7 @@ int dom0_setup_permissions(struct domain *d);
 
 int dom0_construct_pv(
     struct domain *d, const struct boot_module *image,
-    module_t *initrd, const char *cmdline);
+    struct boot_module *initrd, const char *cmdline);
 
 int dom0_construct_pvh(struct domain *d, const module_t *image,
                        unsigned long image_headroom,
diff --git a/xen/arch/x86/pv/dom0_build.c b/xen/arch/x86/pv/dom0_build.c
index db908ddb78b5..dce62d76e41e 100644
--- a/xen/arch/x86/pv/dom0_build.c
+++ b/xen/arch/x86/pv/dom0_build.c
@@ -356,7 +356,7 @@ static struct page_info * __init alloc_chunk(struct domain *d,
 
 int __init dom0_construct_pv(struct domain *d,
                              const struct boot_module *image,
-                             module_t *initrd,
+                             struct boot_module *initrd,
                              const char *cmdline)
 {
     int i, rc, order, machine;
@@ -376,7 +376,7 @@ int __init dom0_construct_pv(struct domain *d,
     void *image_base = bootstrap_map_bm(image);
     unsigned long image_len = image->size;
     void *image_start = image_base + image->headroom;
-    unsigned long initrd_len = initrd ? initrd->mod_end : 0;
+    unsigned long initrd_len = initrd ? initrd->size : 0;
     l4_pgentry_t *l4tab = NULL, *l4start = NULL;
     l3_pgentry_t *l3tab = NULL, *l3start = NULL;
     l2_pgentry_t *l2tab = NULL, *l2start = NULL;
@@ -612,7 +612,7 @@ int __init dom0_construct_pv(struct domain *d,
         initrd_pfn = vinitrd_start ?
                      (vinitrd_start - v_start) >> PAGE_SHIFT :
                      domain_tot_pages(d);
-        initrd_mfn = mfn = initrd->mod_start;
+        initrd_mfn = mfn = initrd->start >> PAGE_SHIFT;
         count = PFN_UP(initrd_len);
         if ( d->arch.physaddr_bitsize &&
              ((mfn + count - 1) >> (d->arch.physaddr_bitsize - PAGE_SHIFT)) )
@@ -627,12 +627,13 @@ int __init dom0_construct_pv(struct domain *d,
                     free_domheap_pages(page, order);
                     page += 1UL << order;
                 }
-            memcpy(page_to_virt(page), mfn_to_virt(initrd->mod_start),
+            memcpy(page_to_virt(page), maddr_to_virt(initrd->start),
                    initrd_len);
-            mpt_alloc = (paddr_t)initrd->mod_start << PAGE_SHIFT;
+            mpt_alloc = initrd->start;
             init_domheap_pages(mpt_alloc,
                                mpt_alloc + PAGE_ALIGN(initrd_len));
-            initrd->mod_start = initrd_mfn = mfn_x(page_to_mfn(page));
+            initrd_mfn = mfn_x(page_to_mfn(page));
+            initrd->start = initrd_mfn << PAGE_SHIFT;
         }
         else
         {
@@ -640,7 +641,7 @@ int __init dom0_construct_pv(struct domain *d,
                 if ( assign_pages(mfn_to_page(_mfn(mfn++)), 1, d, 0) )
                     BUG();
         }
-        initrd->mod_end = 0;
+        initrd->size = initrd->early_mod->mod_end = 0;
 
         iommu_memory_setup(d, "initrd", mfn_to_page(_mfn(initrd_mfn)),
                            PFN_UP(initrd_len), &flush_flags);
@@ -654,7 +655,7 @@ int __init dom0_construct_pv(struct domain *d,
                nr_pages - domain_tot_pages(d));
     if ( initrd )
     {
-        mpt_alloc = (paddr_t)initrd->mod_start << PAGE_SHIFT;
+        mpt_alloc = initrd->start;
         printk("\n Init. ramdisk: %"PRIpaddr"->%"PRIpaddr,
                mpt_alloc, mpt_alloc + initrd_len);
     }
@@ -882,7 +883,7 @@ int __init dom0_construct_pv(struct domain *d,
         if ( pfn >= initrd_pfn )
         {
             if ( pfn < initrd_pfn + PFN_UP(initrd_len) )
-                mfn = initrd->mod_start + (pfn - initrd_pfn);
+                mfn = (initrd->start >> PAGE_SHIFT) + (pfn - initrd_pfn);
             else
                 mfn -= PFN_UP(initrd_len);
         }
-- 
2.30.2

[PATCH v4 31/44] x86/boot: convert dom0_construct_pvh to struct boot_module

[Daniel P. Smith]

This changes both the kernel and ramdisk parameters over to struct
boot_module.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/arch/x86/dom0_build.c             |  2 +-
 xen/arch/x86/hvm/dom0_build.c         | 11 +++++------
 xen/arch/x86/include/asm/dom0_build.h |  7 +++----
 3 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
index c39588b346b0..71b2e3afc1a1 100644
--- a/xen/arch/x86/dom0_build.c
+++ b/xen/arch/x86/dom0_build.c
@@ -610,7 +610,7 @@ int __init construct_dom0(struct domain *d, const struct boot_module *image,
     process_pending_softirqs();
 
     if ( is_hvm_domain(d) )
-        rc = dom0_construct_pvh(d, image->early_mod, image->headroom, initrd->early_mod, cmdline);
+        rc = dom0_construct_pvh(d, image, initrd, cmdline);
     else if ( is_pv_domain(d) )
         rc = dom0_construct_pv(d, image, initrd, cmdline);
     else
diff --git a/xen/arch/x86/hvm/dom0_build.c b/xen/arch/x86/hvm/dom0_build.c
index f3eddb684686..1579034ebfc0 100644
--- a/xen/arch/x86/hvm/dom0_build.c
+++ b/xen/arch/x86/hvm/dom0_build.c
@@ -1285,10 +1285,9 @@ static void __hwdom_init pvh_setup_mmcfg(struct domain *d)
     }
 }
 
-int __init dom0_construct_pvh(struct domain *d, const module_t *image,
-                              unsigned long image_headroom,
-                              module_t *initrd,
-                              const char *cmdline)
+int __init dom0_construct_pvh(
+    struct domain *d, const struct boot_module *image,
+    struct boot_module *initrd, const char *cmdline)
 {
     paddr_t entry, start_info;
     int rc;
@@ -1332,8 +1331,8 @@ int __init dom0_construct_pvh(struct domain *d, const module_t *image,
         return rc;
     }
 
-    rc = pvh_load_kernel(d, image, image_headroom, initrd, bootstrap_map(image),
-                         cmdline, &entry, &start_info);
+    rc = pvh_load_kernel(d, image->early_mod, image->headroom, initrd->early_mod,
+                         bootstrap_map_bm(image), cmdline, &entry, &start_info);
     if ( rc )
     {
         printk("Failed to load Dom0 kernel\n");
diff --git a/xen/arch/x86/include/asm/dom0_build.h b/xen/arch/x86/include/asm/dom0_build.h
index 2ce5ea3851af..8f7b37f3d308 100644
--- a/xen/arch/x86/include/asm/dom0_build.h
+++ b/xen/arch/x86/include/asm/dom0_build.h
@@ -17,10 +17,9 @@ int dom0_construct_pv(
     struct domain *d, const struct boot_module *image,
     struct boot_module *initrd, const char *cmdline);
 
-int dom0_construct_pvh(struct domain *d, const module_t *image,
-                       unsigned long image_headroom,
-                       module_t *initrd,
-                       const char *cmdline);
+int dom0_construct_pvh(
+    struct domain *d, const struct boot_module *image,
+    struct boot_module *initrd, const char *cmdline);
 
 unsigned long dom0_paging_pages(const struct domain *d,
                                 unsigned long nr_pages);
-- 
2.30.2

[PATCH v4 32/44] x86/boot: convert pvh_load_kernel to struct boot_module

[Daniel P. Smith]

This changes both the kernel and ramdisk parameters over to struct boot_module.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/arch/x86/hvm/dom0_build.c | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/xen/arch/x86/hvm/dom0_build.c b/xen/arch/x86/hvm/dom0_build.c
index 1579034ebfc0..b359f3d98376 100644
--- a/xen/arch/x86/hvm/dom0_build.c
+++ b/xen/arch/x86/hvm/dom0_build.c
@@ -642,15 +642,14 @@ static bool __init check_and_adjust_load_address(
     return true;
 }
 
-static int __init pvh_load_kernel(struct domain *d, const module_t *image,
-                                  unsigned long image_headroom,
-                                  module_t *initrd, void *image_base,
-                                  const char *cmdline, paddr_t *entry,
-                                  paddr_t *start_info_addr)
+static int __init pvh_load_kernel(
+    struct domain *d, const struct boot_module *image,
+    struct boot_module *initrd, void *image_base,
+    const char *cmdline, paddr_t *entry, paddr_t *start_info_addr)
 {
-    void *image_start = image_base + image_headroom;
-    unsigned long image_len = image->mod_end;
-    unsigned long initrd_len = initrd ? initrd->mod_end : 0;
+    void *image_start = image_base + image->headroom;
+    unsigned long image_len = image->size;
+    unsigned long initrd_len = initrd ? initrd->size : 0;
     struct elf_binary elf;
     struct elf_dom_parms parms;
     paddr_t last_addr;
@@ -725,7 +724,7 @@ static int __init pvh_load_kernel(struct domain *d, const module_t *image,
 
     if ( initrd != NULL )
     {
-        rc = hvm_copy_to_guest_phys(last_addr, mfn_to_virt(initrd->mod_start),
+        rc = hvm_copy_to_guest_phys(last_addr, maddr_to_virt(initrd->start),
                                     initrd_len, v);
         if ( rc )
         {
@@ -736,9 +735,9 @@ static int __init pvh_load_kernel(struct domain *d, const module_t *image,
         mod.paddr = last_addr;
         mod.size = initrd_len;
         last_addr += ROUNDUP(initrd_len, elf_64bit(&elf) ? 8 : 4);
-        if ( initrd->string )
+        if ( initrd->cmdline )
         {
-            char *str = __va(initrd->string);
+            char *str = __va((unsigned long)initrd->cmdline);
             size_t len = strlen(str) + 1;
 
             rc = hvm_copy_to_guest_phys(last_addr, str, len, v);
@@ -1331,8 +1330,8 @@ int __init dom0_construct_pvh(
         return rc;
     }
 
-    rc = pvh_load_kernel(d, image->early_mod, image->headroom, initrd->early_mod,
-                         bootstrap_map_bm(image), cmdline, &entry, &start_info);
+    rc = pvh_load_kernel(d, image, initrd, bootstrap_map_bm(image), cmdline,
+                         &entry, &start_info);
     if ( rc )
     {
         printk("Failed to load Dom0 kernel\n");
-- 
2.30.2

[PATCH v4 33/44] x86/boot: convert initial_images to struct boot_module

[Daniel P. Smith]

The variable initial_images is used for tracking the boot modules passed in by
the boot loader. Convert to a struct boot_module and adjust the code that uses
it accordingly.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/arch/x86/setup.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 8ba32c6b6bfa..77c0833dd441 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -281,7 +281,7 @@ static const char *cmdline_cook(const char *p, const char *loader_name);
 /* Max number of boot modules a bootloader can provide in addition to Xen */
 #define MAX_NR_BOOTMODS 63
 
-static const module_t *__initdata initial_images;
+static const struct boot_module *__initdata initial_images;
 static struct boot_info __initdata *boot_info;
 
 static void __init multiboot_to_bootinfo(multiboot_info_t *mbi, module_t *mods)
@@ -337,8 +337,9 @@ unsigned long __init initial_images_nrpages(nodeid_t node)
 
     for ( nr = i = 0; i < boot_info->nr_mods; ++i )
     {
-        unsigned long start = initial_images[i].mod_start;
-        unsigned long end = start + PFN_UP(initial_images[i].mod_end);
+        unsigned long start = initial_images[i].early_mod->mod_start;
+        unsigned long end = start +
+                            PFN_UP(initial_images[i].early_mod->mod_end);
 
         if ( end > node_start && node_end > start )
             nr += min(node_end, end) - max(node_start, start);
@@ -353,10 +354,12 @@ void __init discard_initial_images(void)
 
     for ( i = 0; i < boot_info->nr_mods; ++i )
     {
-        uint64_t start = (uint64_t)initial_images[i].mod_start << PAGE_SHIFT;
+        uint64_t start =
+            (uint64_t)initial_images[i].early_mod->mod_start << PAGE_SHIFT;
 
         init_domheap_pages(start,
-                           start + PAGE_ALIGN(initial_images[i].mod_end));
+                           start +
+                           PAGE_ALIGN(initial_images[i].early_mod->mod_end));
     }
 
     boot_info->nr_mods = 0;
@@ -1396,7 +1399,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
     set_kexec_crash_area_size((u64)nr_pages << PAGE_SHIFT);
     kexec_reserve_area();
 
-    initial_images = boot_info->mods[0].early_mod;
+    initial_images = boot_info->mods;
 
     for ( i = 0; !efi_enabled(EFI_LOADER) && i < boot_info->nr_mods; i++ )
     {
-- 
2.30.2

[PATCH v4 34/44] x86/boot: drop the use of initial_images unit global

[Daniel P. Smith]

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/arch/x86/setup.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 77c0833dd441..977b68098694 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -281,7 +281,6 @@ static const char *cmdline_cook(const char *p, const char *loader_name);
 /* Max number of boot modules a bootloader can provide in addition to Xen */
 #define MAX_NR_BOOTMODS 63
 
-static const struct boot_module *__initdata initial_images;
 static struct boot_info __initdata *boot_info;
 
 static void __init multiboot_to_bootinfo(multiboot_info_t *mbi, module_t *mods)
@@ -337,9 +336,9 @@ unsigned long __init initial_images_nrpages(nodeid_t node)
 
     for ( nr = i = 0; i < boot_info->nr_mods; ++i )
     {
-        unsigned long start = initial_images[i].early_mod->mod_start;
+        unsigned long start = boot_info->mods[i].early_mod->mod_start;
         unsigned long end = start +
-                            PFN_UP(initial_images[i].early_mod->mod_end);
+                            PFN_UP(boot_info->mods[i].early_mod->mod_end);
 
         if ( end > node_start && node_end > start )
             nr += min(node_end, end) - max(node_start, start);
@@ -355,15 +354,14 @@ void __init discard_initial_images(void)
     for ( i = 0; i < boot_info->nr_mods; ++i )
     {
         uint64_t start =
-            (uint64_t)initial_images[i].early_mod->mod_start << PAGE_SHIFT;
+            (uint64_t)boot_info->mods[i].early_mod->mod_start << PAGE_SHIFT;
 
         init_domheap_pages(start,
                            start +
-                           PAGE_ALIGN(initial_images[i].early_mod->mod_end));
+                           PAGE_ALIGN(boot_info->mods[i].early_mod->mod_end));
     }
 
     boot_info->nr_mods = 0;
-    initial_images = NULL;
 }
 
 static void __init init_idle_domain(void)
@@ -1399,8 +1397,6 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
     set_kexec_crash_area_size((u64)nr_pages << PAGE_SHIFT);
     kexec_reserve_area();
 
-    initial_images = boot_info->mods;
-
     for ( i = 0; !efi_enabled(EFI_LOADER) && i < boot_info->nr_mods; i++ )
     {
         if ( boot_info->mods[i].early_mod->mod_start & (PAGE_SIZE - 1) )
-- 
2.30.2

[PATCH v4 35/44] x86/boot: remove usage of mod_end by discard_initial_images

[Daniel P. Smith]

This eliminates usage of early_mod by discard_initial_images

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/arch/x86/pv/dom0_build.c | 2 +-
 xen/arch/x86/setup.c         | 6 ++----
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/xen/arch/x86/pv/dom0_build.c b/xen/arch/x86/pv/dom0_build.c
index dce62d76e41e..a403e23b2f93 100644
--- a/xen/arch/x86/pv/dom0_build.c
+++ b/xen/arch/x86/pv/dom0_build.c
@@ -641,7 +641,7 @@ int __init dom0_construct_pv(struct domain *d,
                 if ( assign_pages(mfn_to_page(_mfn(mfn++)), 1, d, 0) )
                     BUG();
         }
-        initrd->size = initrd->early_mod->mod_end = 0;
+        initrd->size = 0;
 
         iommu_memory_setup(d, "initrd", mfn_to_page(_mfn(initrd_mfn)),
                            PFN_UP(initrd_len), &flush_flags);
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 977b68098694..81610405a53c 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -353,12 +353,10 @@ void __init discard_initial_images(void)
 
     for ( i = 0; i < boot_info->nr_mods; ++i )
     {
-        uint64_t start =
-            (uint64_t)boot_info->mods[i].early_mod->mod_start << PAGE_SHIFT;
+        uint64_t start = boot_info->mods[i].start;
 
         init_domheap_pages(start,
-                           start +
-                           PAGE_ALIGN(boot_info->mods[i].early_mod->mod_end));
+                           start + PAGE_ALIGN(boot_info->mods[i].size));
     }
 
     boot_info->nr_mods = 0;
-- 
2.30.2

[PATCH v4 36/44] x86/boot: remove remaining early_mod references

[Daniel P. Smith]

Any direct usages of struct mod have been transitioned, remove the remaining
references to early_mod fields.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/arch/x86/setup.c | 28 +++++++++++-----------------
 1 file changed, 11 insertions(+), 17 deletions(-)

diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 81610405a53c..46f6bf82fe63 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -336,9 +336,8 @@ unsigned long __init initial_images_nrpages(nodeid_t node)
 
     for ( nr = i = 0; i < boot_info->nr_mods; ++i )
     {
-        unsigned long start = boot_info->mods[i].early_mod->mod_start;
-        unsigned long end = start +
-                            PFN_UP(boot_info->mods[i].early_mod->mod_end);
+        unsigned long start = boot_info->mods[i].start;
+        unsigned long end = start + PFN_UP(boot_info->mods[i].size);
 
         if ( end > node_start && node_end > start )
             nr += min(node_end, end) - max(node_start, start);
@@ -665,8 +664,8 @@ static uint64_t __init consider_modules(
 
     for ( i = 0; i < nr_mods ; ++i )
     {
-        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);
+        uint64_t start = (uint64_t)mods[i].start;
+        uint64_t end = start + PAGE_ALIGN(mods[i].size);
 
         if ( i == this_mod )
             continue;
@@ -1397,10 +1396,8 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
 
     for ( i = 0; !efi_enabled(EFI_LOADER) && i < boot_info->nr_mods; i++ )
     {
-        if ( boot_info->mods[i].early_mod->mod_start & (PAGE_SIZE - 1) )
+        if ( boot_info->mods[i].start & (PAGE_SIZE - 1) )
             panic("Bootloader didn't honor module alignment request\n");
-        boot_info->mods[i].early_mod->mod_end -= boot_info->mods[i].early_mod->mod_start;
-        boot_info->mods[i].early_mod->mod_start >>= PAGE_SHIFT;
     }
 
     /*
@@ -1419,15 +1416,13 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
          * respective reserve_e820_ram() invocation below. No need to
          * query efi_boot_mem_unused() here, though.
          */
-        boot_info->mods[idx].start = boot_info->mods[idx].early_mod->mod_start
-                                   = virt_to_mfn(_stext);
-        boot_info->mods[idx].size = boot_info->mods[idx].early_mod->mod_end
-                                  = __2M_rwdata_end - _stext;
+        boot_info->mods[idx].start = virt_to_maddr(_stext);
+        boot_info->mods[idx].size = __2M_rwdata_end - _stext;
     }
 
     boot_info->mods[0].headroom = bzimage_headroom(
-                        bootstrap_map(boot_info->mods[0].early_mod),
-                        boot_info->mods[0].early_mod->mod_end);
+                        bootstrap_map_bm(&boot_info->mods[0]),
+                        boot_info->mods[0].size);
     bootstrap_map(NULL);
 
 #ifndef highmem_start
@@ -1533,8 +1528,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
             {
                 move_memory(end - size + bm->headroom, bm->start, bm->size);
                 bm->start = (end - size);
-                bm->early_mod->mod_start = paddr_to_pfn(bm->start);
-                bm->size = bm->early_mod->mod_end += bm->headroom;
+                bm->size += bm->headroom;
                 bm->flags |= BOOTMOD_FLAG_X86_RELOCATED;
             }
         }
@@ -1724,7 +1718,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
     {
         set_pdx_range(paddr_to_pfn(boot_info->mods[i].start),
                       paddr_to_pfn(boot_info->mods[i].start) +
-                      PFN_UP(boot_info->mods[i].early_mod->mod_end));
+                      PFN_UP(boot_info->mods[i].size));
         map_pages_to_xen(
             (unsigned long)maddr_to_virt(boot_info->mods[i].start),
             maddr_to_mfn(boot_info->mods[i].start),
-- 
2.30.2

[PATCH v4 37/44] x86/boot: remove early_mod from struct boot_module

[Daniel P. Smith]

With all references to early_mod fields removed, remove the
early_mod field from struct boot_module.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/arch/x86/include/asm/bootinfo.h | 2 --
 xen/arch/x86/setup.c                | 3 ---
 2 files changed, 5 deletions(-)

diff --git a/xen/arch/x86/include/asm/bootinfo.h b/xen/arch/x86/include/asm/bootinfo.h
index edd5c435d032..59755ef42e53 100644
--- a/xen/arch/x86/include/asm/bootinfo.h
+++ b/xen/arch/x86/include/asm/bootinfo.h
@@ -8,7 +8,6 @@
 #ifndef __XEN_X86_BOOTINFO_H__
 #define __XEN_X86_BOOTINFO_H__
 
-#include <xen/multiboot.h>
 #include <xen/types.h>
 
 /* Boot module binary type / purpose */
@@ -22,7 +21,6 @@ enum bootmod_type {
 };
 
 struct boot_module {
-    module_t *early_mod;
     unsigned long headroom;
     enum bootmod_type type;
 
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 46f6bf82fe63..a6fbaecee9ed 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -311,8 +311,6 @@ static void __init multiboot_to_bootinfo(multiboot_info_t *mbi, module_t *mods)
 
     for ( i=0; i < info.nr_mods; i++ )
     {
-        boot_mods[i].early_mod = &mods[i];
-
         boot_mods[i].cmdline = (char *)(paddr_t)mods[i].string;
 
         boot_mods[i].start = (paddr_t)mods[i].mod_start;
@@ -322,7 +320,6 @@ static void __init multiboot_to_bootinfo(multiboot_info_t *mbi, module_t *mods)
     /* map the last mb module for xen entry */
     boot_mods[info.nr_mods].type = BOOTMOD_XEN;
     boot_mods[info.nr_mods].flags |= BOOTMOD_FLAG_X86_CONSUMED;
-    boot_mods[info.nr_mods].early_mod = &mods[info.nr_mods];
 
     boot_info = &info;
 }
-- 
2.30.2

[PATCH v4 38/44] x86/boot: introduce boot domain

[Daniel P. Smith]

To begin moving toward allowing the hypervisor to construct more than one
domain at boot, a container is needed for a domain's build information.

Introduce a new header, <xen/asm/bootdomain.h>, that contains the initial
struct boot_domain that encapsulate the build information for a domain.

No functional change intended.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/arch/x86/include/asm/bootdomain.h | 28 +++++++++++++++++++++++++++
 xen/arch/x86/include/asm/bootinfo.h   |  7 +++++++
 xen/arch/x86/setup.c                  | 27 +++++++++++---------------
 3 files changed, 46 insertions(+), 16 deletions(-)
 create mode 100644 xen/arch/x86/include/asm/bootdomain.h

diff --git a/xen/arch/x86/include/asm/bootdomain.h b/xen/arch/x86/include/asm/bootdomain.h
new file mode 100644
index 000000000000..4285223ac5ab
--- /dev/null
+++ b/xen/arch/x86/include/asm/bootdomain.h
@@ -0,0 +1,28 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (c) 2024 Apertus Solutions, LLC
+ * Author: Daniel P. Smith <dpsmith@apertussolutions.com>
+ * Copyright (c) 2024 Christopher Clark <christopher.w.clark@gmail.com>
+ */
+
+#ifndef __XEN_X86_BOOTDOMAIN_H__
+#define __XEN_X86_BOOTDOMAIN_H__
+
+struct boot_module;
+
+struct boot_domain {
+    struct boot_module *kernel;
+    struct boot_module *ramdisk;
+};
+
+#endif
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/x86/include/asm/bootinfo.h b/xen/arch/x86/include/asm/bootinfo.h
index 59755ef42e53..b135aaebd25e 100644
--- a/xen/arch/x86/include/asm/bootinfo.h
+++ b/xen/arch/x86/include/asm/bootinfo.h
@@ -10,6 +10,11 @@
 
 #include <xen/types.h>
 
+#include <asm/bootdomain.h>
+
+/* Max number of boot domains that Xen can construct */
+#define MAX_NR_BOOTDOMS 1
+
 /* Boot module binary type / purpose */
 enum bootmod_type {
     BOOTMOD_UNKNOWN,
@@ -45,6 +50,8 @@ struct boot_info {
 
     paddr_t mmap_addr;
     uint32_t mmap_length;
+
+    struct boot_domain *domains;
 };
 
 static inline int __init next_boot_module_index(
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index a6fbaecee9ed..77abed637ff2 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -287,6 +287,7 @@ static void __init multiboot_to_bootinfo(multiboot_info_t *mbi, module_t *mods)
 {
     static struct boot_info __initdata info;
     static struct boot_module __initdata boot_mods[MAX_NR_BOOTMODS + 1];
+    static struct boot_domain __initdata boot_doms[MAX_NR_BOOTDOMS];
     unsigned int i;
 
     info.nr_mods = mbi->mods_count;
@@ -321,6 +322,8 @@ static void __init multiboot_to_bootinfo(multiboot_info_t *mbi, module_t *mods)
     boot_mods[info.nr_mods].type = BOOTMOD_XEN;
     boot_mods[info.nr_mods].flags |= BOOTMOD_FLAG_X86_CONSUMED;
 
+    info.domains = boot_doms;
+
     boot_info = &info;
 }
 
@@ -958,20 +961,10 @@ static struct domain *__init create_dom0(const struct boot_info *bi)
             .misc_flags = opt_dom0_msr_relaxed ? XEN_X86_MSR_RELAXED : 0,
         },
     };
-    int mod_idx = first_boot_module_index(bi, BOOTMOD_RAMDISK);
-    struct boot_module *image, *initrd;
+    struct boot_domain *bd = &bi->domains[0];
     struct domain *d;
     domid_t domid;
 
-    /* Map boot_module to mb1 module for dom0 */
-    image = &bi->mods[0];
-
-    /* Map boot_module to mb1 module for initrd */
-    if ( mod_idx < 0 )
-        initrd = NULL;
-    else
-        initrd = &bi->mods[mod_idx];
-
     if ( opt_dom0_pvh )
     {
         dom0_cfg.flags |= (XEN_DOMCTL_CDF_hvm |
@@ -997,11 +990,11 @@ static struct domain *__init create_dom0(const struct boot_info *bi)
         panic("Error creating d%uv0\n", domid);
 
     /* Grab the DOM0 command line. */
-    if ( image->cmdline || bi->kextra )
+    if ( bd->kernel->cmdline || bi->kextra )
     {
-        if ( image->cmdline )
-            safe_strcpy(cmdline,
-                        cmdline_cook(__va((unsigned long)image->cmdline),
+        if ( bd->kernel->cmdline )
+            safe_strcpy(cmdline, cmdline_cook(
+                        __va((unsigned long)bd->kernel->cmdline),
                         bi->boot_loader_name));
 
         if ( bi->kextra )
@@ -1036,7 +1029,7 @@ static struct domain *__init create_dom0(const struct boot_info *bi)
         write_cr4(read_cr4() & ~X86_CR4_SMAP);
     }
 
-    if ( construct_dom0(d, image, initrd, cmdline) != 0 )
+    if ( construct_dom0(d, bd->kernel, bd->ramdisk, cmdline) != 0 )
         panic("Could not construct domain 0\n");
 
     if ( cpu_has_smap )
@@ -1221,6 +1214,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
     /* Dom0 kernel is always first */
     boot_info->mods[0].type = BOOTMOD_KERNEL;
     boot_info->mods[0].flags |= BOOTMOD_FLAG_X86_CONSUMED;
+    boot_info->domains[0].kernel = &boot_info->mods[0];
 
     if ( pvh_boot )
     {
@@ -2112,6 +2106,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
     {
         boot_info->mods[initrdidx].type = BOOTMOD_RAMDISK;
         boot_info->mods[initrdidx].flags |= BOOTMOD_FLAG_X86_CONSUMED;
+        boot_info->domains[0].ramdisk = &boot_info->mods[initrdidx];
         if ( first_boot_module_index(boot_info, BOOTMOD_UNKNOWN) >= 0 )
             printk(XENLOG_WARNING
                    "Multiple initrd candidates, picking module #%u\n",
-- 
2.30.2

[PATCH v4 39/44] x86/boot: introduce domid field to struct boot_domain

[Daniel P. Smith]

Add a domid field to struct boot_domain to hold the assigned domain id for the
domain. During initialization, ensure all instances of struct boot_domain have
the invalid domid to ensure that the domid must be set either by convention or
configuration.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/arch/x86/include/asm/bootdomain.h |  2 ++
 xen/arch/x86/setup.c                  | 12 +++++++-----
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/xen/arch/x86/include/asm/bootdomain.h b/xen/arch/x86/include/asm/bootdomain.h
index 4285223ac5ab..d6264d554dba 100644
--- a/xen/arch/x86/include/asm/bootdomain.h
+++ b/xen/arch/x86/include/asm/bootdomain.h
@@ -11,6 +11,8 @@
 struct boot_module;
 
 struct boot_domain {
+    domid_t domid;
+
     struct boot_module *kernel;
     struct boot_module *ramdisk;
 };
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 77abed637ff2..43b588d4d105 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -322,6 +322,9 @@ static void __init multiboot_to_bootinfo(multiboot_info_t *mbi, module_t *mods)
     boot_mods[info.nr_mods].type = BOOTMOD_XEN;
     boot_mods[info.nr_mods].flags |= BOOTMOD_FLAG_X86_CONSUMED;
 
+    for ( i=0; i < MAX_NR_BOOTDOMS; i++ )
+        boot_doms[i].domid = DOMID_INVALID;
+
     info.domains = boot_doms;
 
     boot_info = &info;
@@ -963,7 +966,6 @@ static struct domain *__init create_dom0(const struct boot_info *bi)
     };
     struct boot_domain *bd = &bi->domains[0];
     struct domain *d;
-    domid_t domid;
 
     if ( opt_dom0_pvh )
     {
@@ -979,15 +981,15 @@ static struct domain *__init create_dom0(const struct boot_info *bi)
         dom0_cfg.flags |= XEN_DOMCTL_CDF_iommu;
 
     /* Create initial domain.  Not d0 for pvshim. */
-    domid = get_initial_domain_id();
-    d = domain_create(domid, &dom0_cfg, pv_shim ? 0 : CDF_privileged);
+    bd->domid = get_initial_domain_id();
+    d = domain_create(bd->domid, &dom0_cfg, pv_shim ? 0 : CDF_privileged);
     if ( IS_ERR(d) )
-        panic("Error creating d%u: %ld\n", domid, PTR_ERR(d));
+        panic("Error creating d%u: %ld\n", bd->domid, PTR_ERR(d));
 
     init_dom0_cpuid_policy(d);
 
     if ( alloc_dom0_vcpu0(d) == NULL )
-        panic("Error creating d%uv0\n", domid);
+        panic("Error creating d%uv0\n", bd->domid);
 
     /* Grab the DOM0 command line. */
     if ( bd->kernel->cmdline || bi->kextra )
-- 
2.30.2

[PATCH v4 40/44] x86/boot: add cmdline to struct boot_domain

[Daniel P. Smith]

Add a container for the "cooked" command line for a domain.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/arch/x86/include/asm/bootdomain.h |  4 ++++
 xen/arch/x86/setup.c                  | 18 ++++++++----------
 2 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/xen/arch/x86/include/asm/bootdomain.h b/xen/arch/x86/include/asm/bootdomain.h
index d6264d554dba..00f7d9267965 100644
--- a/xen/arch/x86/include/asm/bootdomain.h
+++ b/xen/arch/x86/include/asm/bootdomain.h
@@ -8,9 +8,13 @@
 #ifndef __XEN_X86_BOOTDOMAIN_H__
 #define __XEN_X86_BOOTDOMAIN_H__
 
+#include <public/xen.h>
+
 struct boot_module;
 
 struct boot_domain {
+    char cmdline[MAX_GUEST_CMDLINE];
+
     domid_t domid;
 
     struct boot_module *kernel;
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 43b588d4d105..2c1aa1475c4e 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -951,8 +951,6 @@ static unsigned int __init copy_bios_e820(struct e820entry *map, unsigned int li
 
 static struct domain *__init create_dom0(const struct boot_info *bi)
 {
-    static char __initdata cmdline[MAX_GUEST_CMDLINE];
-
     struct xen_domctl_createdomain dom0_cfg = {
         .flags = IS_ENABLED(CONFIG_TBOOT) ? XEN_DOMCTL_CDF_s3_integrity : 0,
         .max_evtchn_port = -1,
@@ -995,17 +993,17 @@ static struct domain *__init create_dom0(const struct boot_info *bi)
     if ( bd->kernel->cmdline || bi->kextra )
     {
         if ( bd->kernel->cmdline )
-            safe_strcpy(cmdline, cmdline_cook(
+            safe_strcpy(bd->cmdline, cmdline_cook(
                         __va((unsigned long)bd->kernel->cmdline),
                         bi->boot_loader_name));
 
         if ( bi->kextra )
             /* kextra always includes exactly one leading space. */
-            safe_strcat(cmdline, bi->kextra);
+            safe_strcat(bd->cmdline, bi->kextra);
 
         /* Append any extra parameters. */
-        if ( skip_ioapic_setup && !strstr(cmdline, "noapic") )
-            safe_strcat(cmdline, " noapic");
+        if ( skip_ioapic_setup && !strstr(bd->cmdline, "noapic") )
+            safe_strcat(bd->cmdline, " noapic");
 
         if ( (strlen(acpi_param) == 0) && acpi_disabled )
         {
@@ -1013,10 +1011,10 @@ static struct domain *__init create_dom0(const struct boot_info *bi)
             safe_strcpy(acpi_param, "off");
         }
 
-        if ( (strlen(acpi_param) != 0) && !strstr(cmdline, "acpi=") )
+        if ( (strlen(acpi_param) != 0) && !strstr(bd->cmdline, "acpi=") )
         {
-            safe_strcat(cmdline, " acpi=");
-            safe_strcat(cmdline, acpi_param);
+            safe_strcat(bd->cmdline, " acpi=");
+            safe_strcat(bd->cmdline, acpi_param);
         }
     }
 
@@ -1031,7 +1029,7 @@ static struct domain *__init create_dom0(const struct boot_info *bi)
         write_cr4(read_cr4() & ~X86_CR4_SMAP);
     }
 
-    if ( construct_dom0(d, bd->kernel, bd->ramdisk, cmdline) != 0 )
+    if ( construct_dom0(d, bd->kernel, bd->ramdisk, bd->cmdline) != 0 )
         panic("Could not construct domain 0\n");
 
     if ( cpu_has_smap )
-- 
2.30.2

[PATCH v4 41/44] x86/boot: add struct domain to struct boot_domain

[Daniel P. Smith]

Store a reference to the created domain in struct boot_domain.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/arch/x86/include/asm/bootdomain.h |  3 +++
 xen/arch/x86/setup.c                  | 15 +++++++--------
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/xen/arch/x86/include/asm/bootdomain.h b/xen/arch/x86/include/asm/bootdomain.h
index 00f7d9267965..2322c459e36a 100644
--- a/xen/arch/x86/include/asm/bootdomain.h
+++ b/xen/arch/x86/include/asm/bootdomain.h
@@ -10,6 +10,7 @@
 
 #include <public/xen.h>
 
+struct domain;
 struct boot_module;
 
 struct boot_domain {
@@ -19,6 +20,8 @@ struct boot_domain {
 
     struct boot_module *kernel;
     struct boot_module *ramdisk;
+
+    struct domain *d;
 };
 
 #endif
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 2c1aa1475c4e..872d51310628 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -963,7 +963,6 @@ static struct domain *__init create_dom0(const struct boot_info *bi)
         },
     };
     struct boot_domain *bd = &bi->domains[0];
-    struct domain *d;
 
     if ( opt_dom0_pvh )
     {
@@ -980,13 +979,13 @@ static struct domain *__init create_dom0(const struct boot_info *bi)
 
     /* Create initial domain.  Not d0 for pvshim. */
     bd->domid = get_initial_domain_id();
-    d = domain_create(bd->domid, &dom0_cfg, pv_shim ? 0 : CDF_privileged);
-    if ( IS_ERR(d) )
-        panic("Error creating d%u: %ld\n", bd->domid, PTR_ERR(d));
+    bd->d = domain_create(bd->domid, &dom0_cfg, pv_shim ? 0 : CDF_privileged);
+    if ( IS_ERR(bd->d) )
+        panic("Error creating d%u: %ld\n", bd->domid, PTR_ERR(bd->d));
 
-    init_dom0_cpuid_policy(d);
+    init_dom0_cpuid_policy(bd->d);
 
-    if ( alloc_dom0_vcpu0(d) == NULL )
+    if ( alloc_dom0_vcpu0(bd->d) == NULL )
         panic("Error creating d%uv0\n", bd->domid);
 
     /* Grab the DOM0 command line. */
@@ -1029,7 +1028,7 @@ static struct domain *__init create_dom0(const struct boot_info *bi)
         write_cr4(read_cr4() & ~X86_CR4_SMAP);
     }
 
-    if ( construct_dom0(d, bd->kernel, bd->ramdisk, bd->cmdline) != 0 )
+    if ( construct_dom0(bd->d, bd->kernel, bd->ramdisk, bd->cmdline) != 0 )
         panic("Could not construct domain 0\n");
 
     if ( cpu_has_smap )
@@ -1038,7 +1037,7 @@ static struct domain *__init create_dom0(const struct boot_info *bi)
         cr4_pv32_mask |= X86_CR4_SMAP;
     }
 
-    return d;
+    return bd->d;
 }
 
 /* How much of the directmap is prebuilt at compile time. */
-- 
2.30.2

[PATCH v4 42/44] x86/boot: convert construct_dom0 to struct boot_domain

[Daniel P. Smith]

A struct boot_domain now encapsulates the domain reference, kernel, ramdisk,
and command line for the domain being constructed. As a result of this
encapsulation, construct_dom0 can now take a single struct boot_domain instead
of these four parameters.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/arch/x86/dom0_build.c        | 19 +++++++++----------
 xen/arch/x86/include/asm/setup.h |  4 +---
 xen/arch/x86/setup.c             |  2 +-
 3 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
index 71b2e3afc1a1..e552f2e9abef 100644
--- a/xen/arch/x86/dom0_build.c
+++ b/xen/arch/x86/dom0_build.c
@@ -597,22 +597,21 @@ int __init dom0_setup_permissions(struct domain *d)
     return rc;
 }
 
-int __init construct_dom0(struct domain *d, const struct boot_module *image,
-                          struct boot_module *initrd, const char *cmdline)
+int __init construct_dom0(struct boot_domain *bd)
 {
     int rc;
 
     /* Sanity! */
-    BUG_ON(!pv_shim && d->domain_id != 0);
-    BUG_ON(d->vcpu[0] == NULL);
-    BUG_ON(d->vcpu[0]->is_initialised);
+    BUG_ON(!pv_shim && bd->d->domain_id != 0);
+    BUG_ON(bd->d->vcpu[0] == NULL);
+    BUG_ON(bd->d->vcpu[0]->is_initialised);
 
     process_pending_softirqs();
 
-    if ( is_hvm_domain(d) )
-        rc = dom0_construct_pvh(d, image, initrd, cmdline);
-    else if ( is_pv_domain(d) )
-        rc = dom0_construct_pv(d, image, initrd, cmdline);
+    if ( is_hvm_domain(bd->d) )
+        rc = dom0_construct_pvh(bd->d, bd->kernel, bd->ramdisk, bd->cmdline);
+    else if ( is_pv_domain(bd->d) )
+        rc = dom0_construct_pv(bd->d, bd->kernel, bd->ramdisk, bd->cmdline);
     else
         panic("Cannot construct Dom0. No guest interface available\n");
 
@@ -620,7 +619,7 @@ int __init construct_dom0(struct domain *d, const struct boot_module *image,
         return rc;
 
     /* Sanity! */
-    BUG_ON(!d->vcpu[0]->is_initialised);
+    BUG_ON(!bd->d->vcpu[0]->is_initialised);
 
     return 0;
 }
diff --git a/xen/arch/x86/include/asm/setup.h b/xen/arch/x86/include/asm/setup.h
index 19e2ad95b523..8f54b429b954 100644
--- a/xen/arch/x86/include/asm/setup.h
+++ b/xen/arch/x86/include/asm/setup.h
@@ -26,9 +26,7 @@ void subarch_init_memory(void);
 
 void init_IRQ(void);
 
-int construct_dom0(
-    struct domain *d, const struct boot_module *image,
-    struct boot_module *initrd, const char *cmdline);
+int construct_dom0(struct boot_domain *d);
 void setup_io_bitmap(struct domain *d);
 
 unsigned long initial_images_nrpages(nodeid_t node);
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 872d51310628..5d48ebea51d5 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1028,7 +1028,7 @@ static struct domain *__init create_dom0(const struct boot_info *bi)
         write_cr4(read_cr4() & ~X86_CR4_SMAP);
     }
 
-    if ( construct_dom0(bd->d, bd->kernel, bd->ramdisk, bd->cmdline) != 0 )
+    if ( construct_dom0(bd) != 0 )
         panic("Could not construct domain 0\n");
 
     if ( cpu_has_smap )
-- 
2.30.2

[PATCH v4 43/44] x86/boot: convert dom0_construct_pv to struct boot_domain

[Daniel P. Smith]

With construct_dom0 consuming struct boot_domain, continue passing the
structure down to dom0_construct_pv.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/arch/x86/dom0_build.c             |  2 +-
 xen/arch/x86/include/asm/dom0_build.h |  5 ++--
 xen/arch/x86/pv/dom0_build.c          | 36 +++++++++++++--------------
 3 files changed, 20 insertions(+), 23 deletions(-)

diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
index e552f2e9abef..8beb33032940 100644
--- a/xen/arch/x86/dom0_build.c
+++ b/xen/arch/x86/dom0_build.c
@@ -611,7 +611,7 @@ int __init construct_dom0(struct boot_domain *bd)
     if ( is_hvm_domain(bd->d) )
         rc = dom0_construct_pvh(bd->d, bd->kernel, bd->ramdisk, bd->cmdline);
     else if ( is_pv_domain(bd->d) )
-        rc = dom0_construct_pv(bd->d, bd->kernel, bd->ramdisk, bd->cmdline);
+        rc = dom0_construct_pv(bd);
     else
         panic("Cannot construct Dom0. No guest interface available\n");
 
diff --git a/xen/arch/x86/include/asm/dom0_build.h b/xen/arch/x86/include/asm/dom0_build.h
index 8f7b37f3d308..60e9cb21f14d 100644
--- a/xen/arch/x86/include/asm/dom0_build.h
+++ b/xen/arch/x86/include/asm/dom0_build.h
@@ -4,6 +4,7 @@
 #include <xen/libelf.h>
 #include <xen/sched.h>
 
+#include <asm/bootinfo.h>
 #include <asm/setup.h>
 
 extern unsigned int dom0_memflags;
@@ -13,9 +14,7 @@ unsigned long dom0_compute_nr_pages(struct domain *d,
                                     unsigned long initrd_len);
 int dom0_setup_permissions(struct domain *d);
 
-int dom0_construct_pv(
-    struct domain *d, const struct boot_module *image,
-    struct boot_module *initrd, const char *cmdline);
+int dom0_construct_pv(struct boot_domain *bd);
 
 int dom0_construct_pvh(
     struct domain *d, const struct boot_module *image,
diff --git a/xen/arch/x86/pv/dom0_build.c b/xen/arch/x86/pv/dom0_build.c
index a403e23b2f93..82dc0c379f27 100644
--- a/xen/arch/x86/pv/dom0_build.c
+++ b/xen/arch/x86/pv/dom0_build.c
@@ -354,10 +354,7 @@ static struct page_info * __init alloc_chunk(struct domain *d,
     return page;
 }
 
-int __init dom0_construct_pv(struct domain *d,
-                             const struct boot_module *image,
-                             struct boot_module *initrd,
-                             const char *cmdline)
+int __init dom0_construct_pv(struct boot_domain *bd)
 {
     int i, rc, order, machine;
     bool compatible, compat;
@@ -372,11 +369,12 @@ int __init dom0_construct_pv(struct domain *d,
     struct page_info *page = NULL;
     unsigned int flush_flags = 0;
     start_info_t *si;
-    struct vcpu *v = d->vcpu[0];
-    void *image_base = bootstrap_map_bm(image);
-    unsigned long image_len = image->size;
-    void *image_start = image_base + image->headroom;
-    unsigned long initrd_len = initrd ? initrd->size : 0;
+    struct domain *d = bd->d;
+    struct vcpu *v = bd->d->vcpu[0];
+    void *image_base = bootstrap_map_bm(bd->kernel);
+    unsigned long image_len = bd->kernel->size;
+    void *image_start = image_base + bd->kernel->headroom;
+    unsigned long initrd_len = bd->ramdisk ? bd->ramdisk->size : 0;
     l4_pgentry_t *l4tab = NULL, *l4start = NULL;
     l3_pgentry_t *l3tab = NULL, *l3start = NULL;
     l2_pgentry_t *l2tab = NULL, *l2start = NULL;
@@ -612,7 +610,7 @@ int __init dom0_construct_pv(struct domain *d,
         initrd_pfn = vinitrd_start ?
                      (vinitrd_start - v_start) >> PAGE_SHIFT :
                      domain_tot_pages(d);
-        initrd_mfn = mfn = initrd->start >> PAGE_SHIFT;
+        initrd_mfn = mfn = bd->ramdisk->start >> PAGE_SHIFT;
         count = PFN_UP(initrd_len);
         if ( d->arch.physaddr_bitsize &&
              ((mfn + count - 1) >> (d->arch.physaddr_bitsize - PAGE_SHIFT)) )
@@ -627,13 +625,13 @@ int __init dom0_construct_pv(struct domain *d,
                     free_domheap_pages(page, order);
                     page += 1UL << order;
                 }
-            memcpy(page_to_virt(page), maddr_to_virt(initrd->start),
+            memcpy(page_to_virt(page), maddr_to_virt(bd->ramdisk->start),
                    initrd_len);
-            mpt_alloc = initrd->start;
+            mpt_alloc = bd->ramdisk->start;
             init_domheap_pages(mpt_alloc,
                                mpt_alloc + PAGE_ALIGN(initrd_len));
             initrd_mfn = mfn_x(page_to_mfn(page));
-            initrd->start = initrd_mfn << PAGE_SHIFT;
+            bd->ramdisk->start = initrd_mfn << PAGE_SHIFT;
         }
         else
         {
@@ -641,7 +639,7 @@ int __init dom0_construct_pv(struct domain *d,
                 if ( assign_pages(mfn_to_page(_mfn(mfn++)), 1, d, 0) )
                     BUG();
         }
-        initrd->size = 0;
+        bd->ramdisk->size = 0;
 
         iommu_memory_setup(d, "initrd", mfn_to_page(_mfn(initrd_mfn)),
                            PFN_UP(initrd_len), &flush_flags);
@@ -653,9 +651,9 @@ int __init dom0_construct_pv(struct domain *d,
     if ( domain_tot_pages(d) < nr_pages )
         printk(" (%lu pages to be allocated)",
                nr_pages - domain_tot_pages(d));
-    if ( initrd )
+    if ( bd->ramdisk )
     {
-        mpt_alloc = initrd->start;
+        mpt_alloc = bd->ramdisk->start;
         printk("\n Init. ramdisk: %"PRIpaddr"->%"PRIpaddr,
                mpt_alloc, mpt_alloc + initrd_len);
     }
@@ -883,7 +881,7 @@ int __init dom0_construct_pv(struct domain *d,
         if ( pfn >= initrd_pfn )
         {
             if ( pfn < initrd_pfn + PFN_UP(initrd_len) )
-                mfn = (initrd->start >> PAGE_SHIFT) + (pfn - initrd_pfn);
+                mfn = (bd->ramdisk->start >> PAGE_SHIFT) + (pfn - initrd_pfn);
             else
                 mfn -= PFN_UP(initrd_len);
         }
@@ -953,8 +951,8 @@ int __init dom0_construct_pv(struct domain *d,
     }
 
     memset(si->cmd_line, 0, sizeof(si->cmd_line));
-    if ( cmdline != NULL )
-        strlcpy((char *)si->cmd_line, cmdline, sizeof(si->cmd_line));
+    if ( bd->cmdline[0] != '\0' )
+        strlcpy((char *)si->cmd_line, bd->cmdline, sizeof(si->cmd_line));
 
 #ifdef CONFIG_VIDEO
     if ( !pv_shim && fill_console_start_info((void *)(si + 1)) )
-- 
2.30.2

[PATCH v4 44/44] x86/boot: convert dom0_construct_pvh to struct boot_domain

[Daniel P. Smith]

With construct_dom0 consuming struct boot_domain, continue passing the
structure down to dom0_construct_pvh.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/arch/x86/dom0_build.c             |  2 +-
 xen/arch/x86/hvm/dom0_build.c         | 31 +++++++++++++--------------
 xen/arch/x86/include/asm/dom0_build.h |  4 +---
 3 files changed, 17 insertions(+), 20 deletions(-)

diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
index 8beb33032940..6a21fd46d5a3 100644
--- a/xen/arch/x86/dom0_build.c
+++ b/xen/arch/x86/dom0_build.c
@@ -609,7 +609,7 @@ int __init construct_dom0(struct boot_domain *bd)
     process_pending_softirqs();
 
     if ( is_hvm_domain(bd->d) )
-        rc = dom0_construct_pvh(bd->d, bd->kernel, bd->ramdisk, bd->cmdline);
+        rc = dom0_construct_pvh(bd);
     else if ( is_pv_domain(bd->d) )
         rc = dom0_construct_pv(bd);
     else
diff --git a/xen/arch/x86/hvm/dom0_build.c b/xen/arch/x86/hvm/dom0_build.c
index b359f3d98376..13e17550efa5 100644
--- a/xen/arch/x86/hvm/dom0_build.c
+++ b/xen/arch/x86/hvm/dom0_build.c
@@ -1284,25 +1284,23 @@ static void __hwdom_init pvh_setup_mmcfg(struct domain *d)
     }
 }
 
-int __init dom0_construct_pvh(
-    struct domain *d, const struct boot_module *image,
-    struct boot_module *initrd, const char *cmdline)
+int __init dom0_construct_pvh(const struct boot_domain *bd)
 {
     paddr_t entry, start_info;
     int rc;
 
-    printk(XENLOG_INFO "*** Building a PVH Dom%d ***\n", d->domain_id);
+    printk(XENLOG_INFO "*** Building a PVH Dom%d ***\n", bd->domid);
 
-    if ( is_hardware_domain(d) )
+    if ( is_hardware_domain(bd->d) )
     {
         /*
          * Setup permissions early so that calls to add MMIO regions to the
          * p2m as part of vPCI setup don't fail due to permission checks.
          */
-        rc = dom0_setup_permissions(d);
+        rc = dom0_setup_permissions(bd->d);
         if ( rc )
         {
-            printk("%pd unable to setup permissions: %d\n", d, rc);
+            printk("%pd unable to setup permissions: %d\n", bd->d, rc);
             return rc;
         }
     }
@@ -1312,25 +1310,26 @@ int __init dom0_construct_pvh(
      * initialization so the iommu code can fetch the MMCFG regions used by the
      * domain.
      */
-    pvh_setup_mmcfg(d);
+    pvh_setup_mmcfg(bd->d);
 
     /*
      * Craft dom0 physical memory map and set the paging allocation. This must
      * be done before the iommu initializion, since iommu initialization code
      * will likely add mappings required by devices to the p2m (ie: RMRRs).
      */
-    pvh_init_p2m(d);
+    pvh_init_p2m(bd->d);
 
-    iommu_hwdom_init(d);
+    iommu_hwdom_init(bd->d);
 
-    rc = pvh_populate_p2m(d);
+    rc = pvh_populate_p2m(bd->d);
     if ( rc )
     {
         printk("Failed to setup Dom0 physical memory map\n");
         return rc;
     }
 
-    rc = pvh_load_kernel(d, image, initrd, bootstrap_map_bm(image), cmdline,
+    rc = pvh_load_kernel(bd->d, bd->kernel, bd->ramdisk,
+                         bootstrap_map_bm(bd->kernel), bd->cmdline,
                          &entry, &start_info);
     if ( rc )
     {
@@ -1338,14 +1337,14 @@ int __init dom0_construct_pvh(
         return rc;
     }
 
-    rc = pvh_setup_cpus(d, entry, start_info);
+    rc = pvh_setup_cpus(bd->d, entry, start_info);
     if ( rc )
     {
         printk("Failed to setup Dom0 CPUs: %d\n", rc);
         return rc;
     }
 
-    rc = pvh_setup_acpi(d, start_info);
+    rc = pvh_setup_acpi(bd->d, start_info);
     if ( rc )
     {
         printk("Failed to setup Dom0 ACPI tables: %d\n", rc);
@@ -1354,8 +1353,8 @@ int __init dom0_construct_pvh(
 
     if ( opt_dom0_verbose )
     {
-        printk("Dom%u memory map:\n", d->domain_id);
-        print_e820_memory_map(d->arch.e820, d->arch.nr_e820);
+        printk("Dom%u memory map:\n", bd->domid);
+        print_e820_memory_map(bd->d->arch.e820, bd->d->arch.nr_e820);
     }
 
     return 0;
diff --git a/xen/arch/x86/include/asm/dom0_build.h b/xen/arch/x86/include/asm/dom0_build.h
index 60e9cb21f14d..adbe90bfd034 100644
--- a/xen/arch/x86/include/asm/dom0_build.h
+++ b/xen/arch/x86/include/asm/dom0_build.h
@@ -16,9 +16,7 @@ int dom0_setup_permissions(struct domain *d);
 
 int dom0_construct_pv(struct boot_domain *bd);
 
-int dom0_construct_pvh(
-    struct domain *d, const struct boot_module *image,
-    struct boot_module *initrd, const char *cmdline);
+int dom0_construct_pvh(const struct boot_domain *bd);
 
 unsigned long dom0_paging_pages(const struct domain *d,
                                 unsigned long nr_pages);
-- 
2.30.2

Re: [PATCH v4 00/44] Boot modules for Hyperlaunch

[Daniel P. Smith]

On 8/30/24 17:46, Daniel P. Smith wrote:
> The Boot Modules for Hyperlaunch series is an effort to split out preliminary
> changes necessary for the introduction of the Hyperlaunch domain builder
> logic. These preliminary changes revolve around introducing the struct
> boot_module and struct boot_domain structures. This includes converting the
> dom0 construction path to use these structures. These abstractions lay the
> groundwork to transform and extend the dom0 construction logic into a limited,
> but general domain builder.
> 
> The splitting of Hyperlaunch into a pair of series was twofold, to reduce the
> effort in reviewing a much larger series, and to reduce the effort in handling
> the knock-on effects to the construction logic from requested review changes.
> 
> A note on v4:
> 
> For v4, two significant direction changes occurred. First, the series was
> pulled back from attempting to be in common and instead focused on being
> purely an x86 capability. Second, the changes were broken down into much
> smaller change sets and ordered to provide a more ordered evolution of the
> code. To the most extent possible, it was attempted to ensure all v3 comments
> were transcribed accordingly with the scope change.

As an FYI, after posting I realized I was not on the tip of staging, I 
have a v5 ready, which is at the tip of staging and corrects a small 
type issue with initrdidx in start_xen() and is 100% on CI. I'm holding 
off on posting back-to-back revisions of a large series, which would 
only results in a few code gyrations with no logical impact that would 
not result in different review than would have occurred on v4.

V/r,
DPS

Re: [PATCH v4 01/44] x86/boot: move x86 boot module counting into a new boot_info struct

[Alejandro Vallejo]

I haven't read the entire series yet, but here's my .02 so far

On Fri Aug 30, 2024 at 10:46 PM BST, Daniel P. Smith wrote:
> From: Christopher Clark <christopher.w.clark@gmail.com>
>
> An initial step towards a non-multiboot internal representation of boot
> modules for common code, starting with x86 setup and converting the fields
> that are accessed for the startup calculations.
>
> Introduce a new header, <xen/asm/bootinfo.h>, and populate it with a new
> boot_info structure initially containing a count of the number of boot
> modules.
>
> No functional change intended.
>
> Signed-off-by: Christopher Clark <christopher.w.clark@gmail.com>
> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
> ---
>  xen/arch/x86/include/asm/bootinfo.h | 25 +++++++++++++
>  xen/arch/x86/setup.c                | 58 +++++++++++++++++------------
>  2 files changed, 59 insertions(+), 24 deletions(-)
>  create mode 100644 xen/arch/x86/include/asm/bootinfo.h
>
> diff --git a/xen/arch/x86/include/asm/bootinfo.h b/xen/arch/x86/include/asm/bootinfo.h
> new file mode 100644
> index 000000000000..e850f80d26a7
> --- /dev/null
> +++ b/xen/arch/x86/include/asm/bootinfo.h
> @@ -0,0 +1,25 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (c) 2024 Christopher Clark <christopher.w.clark@gmail.com>
> + * Copyright (c) 2024 Apertus Solutions, LLC
> + * Author: Daniel P. Smith <dpsmith@apertussolutions.com>
> + */
> +
> +#ifndef __XEN_X86_BOOTINFO_H__
> +#define __XEN_X86_BOOTINFO_H__
> +

This struct would benefit from a comment stating what it's for and how it's
meant to be used. At a glance it seems like it's meant to be serve as a
boot-protocol agnostic representation of boot-parameters, used as a generic
means of information handover. Which would imply multiboot_info is parsed onto
it when booting from multiboot and is synthesised from scratch in other cases
(e.g: direct EFI?).

> +struct boot_info {
> +    unsigned int nr_mods;

It's imo better to treat this as an ABI. That would allow using this layer as a
boot protocol in itself (which I'm guessing is the objective? I haven't gotten
that far in the series). If so, this would need to be a fixed-width uintN_t.

Same with other fields in follow-up patches.

> +};
> +
> +#endif
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * tab-width: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index eee20bb1753c..dd94ee2e736b 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -32,6 +32,7 @@
>  #include <compat/xen.h>
>  #endif
>  #include <xen/bitops.h>
> +#include <asm/bootinfo.h>
>  #include <asm/smp.h>
>  #include <asm/processor.h>
>  #include <asm/mpspec.h>
> @@ -276,7 +277,16 @@ static int __init cf_check parse_acpi_param(const char *s)
>  custom_param("acpi", parse_acpi_param);
>  
>  static const module_t *__initdata initial_images;
> -static unsigned int __initdata nr_initial_images;
> +static struct boot_info __initdata *boot_info;
> +
> +static void __init multiboot_to_bootinfo(multiboot_info_t *mbi)

If this function returned boot_info instead and the caller made the
assignment then it would be possible to unit-test/fuzz it.

It also fits a bit more nicely with the usual implications of that function
name pattern, I think.

> +{
> +    static struct boot_info __initdata info;
> +
> +    info.nr_mods = mbi->mods_count;

Shouldn't this be gated on MBI_MODULES being set?

   info.nr_mods = (mbi->flags & MBI_MODULES) ? mbi->mods_count : 0;

> +
> +    boot_info = &info;
> +}
>  
>  unsigned long __init initial_images_nrpages(nodeid_t node)
>  {
> @@ -285,7 +295,7 @@ unsigned long __init initial_images_nrpages(nodeid_t node)
>      unsigned long nr;
>      unsigned int i;
>  
> -    for ( nr = i = 0; i < nr_initial_images; ++i )
> +    for ( nr = i = 0; i < boot_info->nr_mods; ++i )
>      {
>          unsigned long start = initial_images[i].mod_start;
>          unsigned long end = start + PFN_UP(initial_images[i].mod_end);
> @@ -301,7 +311,7 @@ void __init discard_initial_images(void)
>  {
>      unsigned int i;
>  
> -    for ( i = 0; i < nr_initial_images; ++i )
> +    for ( i = 0; i < boot_info->nr_mods; ++i )
>      {
>          uint64_t start = (uint64_t)initial_images[i].mod_start << PAGE_SHIFT;
>  
> @@ -309,7 +319,7 @@ void __init discard_initial_images(void)
>                             start + PAGE_ALIGN(initial_images[i].mod_end));
>      }
>  
> -    nr_initial_images = 0;
> +    boot_info->nr_mods = 0;

Out of curiosity, why is this required?

>      initial_images = NULL;
>  }
>  
> @@ -1034,9 +1044,10 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
>          mod = __va(mbi->mods_addr);
>      }
>  
> +    multiboot_to_bootinfo(mbi);
> +
>      loader = (mbi->flags & MBI_LOADERNAME) ? __va(mbi->boot_loader_name)
>                                             : "unknown";
> -

Stray newline removal?

>      /* Parse the command-line options. */
>      if ( mbi->flags & MBI_CMDLINE )
>          cmdline = cmdline_cook(__va(mbi->cmdline), loader);
> @@ -1141,18 +1152,18 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
>             bootsym(boot_edd_info_nr));
>  
>      /* Check that we have at least one Multiboot module. */
> -    if ( !(mbi->flags & MBI_MODULES) || (mbi->mods_count == 0) )
> +    if ( !(mbi->flags & MBI_MODULES) || (boot_info->nr_mods == 0) )

With MBI_MODULES accounted for during conversion, the first part of the
conditional can be ellided and you could simply do:

    if ( !boot_info->nr_mods )
        panic(...)

Also, could we move this to multiboot_to_bootinfo()? It'd contain these sorts
of boot argument checks to a much more self contained function and help check
at the point of assignment, preventing misuse.

>          panic("dom0 kernel not specified. Check bootloader configuration\n");
>  
>      /* Check that we don't have a silly number of modules. */

> -    if ( mbi->mods_count > sizeof(module_map) * 8 )
> +    if ( boot_info->nr_mods > sizeof(module_map) * 8 )

Like above, this check would be much more neatly contained where boot_info
is created, imo.

>      {
> -        mbi->mods_count = sizeof(module_map) * 8;
> +        boot_info->nr_mods = sizeof(module_map) * 8;
>          printk("Excessive multiboot modules - using the first %u only\n",

Does the comment need adjusting too to make it more general? As in
s/multiboot/boot.

> -               mbi->mods_count);
> +               boot_info->nr_mods);
>      }
>  
> -    bitmap_fill(module_map, mbi->mods_count);
> +    bitmap_fill(module_map, boot_info->nr_mods);
>      __clear_bit(0, module_map); /* Dom0 kernel is always first */
>  
>      if ( pvh_boot )

Cheers,
Alejandro

Re: [PATCH v4 01/44] x86/boot: move x86 boot module counting into a new boot_info struct

[Andrew Cooper]

On 30/08/2024 10:46 pm, Daniel P. Smith wrote:
> From: Christopher Clark <christopher.w.clark@gmail.com>
>
> An initial step towards a non-multiboot internal representation of boot
> modules for common code, starting with x86 setup and converting the fields
> that are accessed for the startup calculations.
>
> Introduce a new header, <xen/asm/bootinfo.h>, and populate it with a new

Just <asm/bootinfo.h>, which matches the code.

> diff --git a/xen/arch/x86/include/asm/bootinfo.h b/xen/arch/x86/include/asm/bootinfo.h
> new file mode 100644
> index 000000000000..e850f80d26a7
> --- /dev/null
> +++ b/xen/arch/x86/include/asm/bootinfo.h
> @@ -0,0 +1,25 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (c) 2024 Christopher Clark <christopher.w.clark@gmail.com>
> + * Copyright (c) 2024 Apertus Solutions, LLC
> + * Author: Daniel P. Smith <dpsmith@apertussolutions.com>
> + */
> +
> +#ifndef __XEN_X86_BOOTINFO_H__
> +#define __XEN_X86_BOOTINFO_H__
> +

There ought to be a short description of what boot_info is, even if it's
only "Xen's local representation of information provided by the
bootloader/environment."

> +struct boot_info {
> +    unsigned int nr_mods;

For the sake of 3 letters, please can this be nr_modules.  I've run sed
over the top of the v5 branch and it doesn't change line wrapping
anywhere, but it is a legibility improvement IMO.

> +};
> +
> +#endif

#endif /* __XEN_X86_BOOTINFO_H__ */

It very quickly get to not being in the same few lines as the #ifndef.

> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index eee20bb1753c..dd94ee2e736b 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -276,7 +277,16 @@ static int __init cf_check parse_acpi_param(const char *s)
>  custom_param("acpi", parse_acpi_param);
>  
>  static const module_t *__initdata initial_images;
> -static unsigned int __initdata nr_initial_images;
> +static struct boot_info __initdata *boot_info;
> +
> +static void __init multiboot_to_bootinfo(multiboot_info_t *mbi)
> +{
> +    static struct boot_info __initdata info;
> +
> +    info.nr_mods = mbi->mods_count;
> +
> +    boot_info = &info;
> +}

Having a global pointer set only to this private structure is weird.
Even this:

    static struct boot_info __initdata boot_info[1];

lets you keep -> notation, but removes one level of indirection.

> @@ -1034,9 +1044,10 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
>          mod = __va(mbi->mods_addr);
>      }
>  
> +    multiboot_to_bootinfo(mbi);
> +
>      loader = (mbi->flags & MBI_LOADERNAME) ? __va(mbi->boot_loader_name)
>                                             : "unknown";
> -

Stray line removal.  (should be in patch 2 to minimise churn.)

~Andrew

Re: [PATCH v4 01/44] x86/boot: move x86 boot module counting into a new boot_info struct

[Andrew Cooper]

On 30/08/2024 10:46 pm, Daniel P. Smith wrote:
> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index eee20bb1753c..dd94ee2e736b 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -1034,9 +1044,10 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
>          mod = __va(mbi->mods_addr);
>      }
>  
> +    multiboot_to_bootinfo(mbi);

Actually, peeking ahead to the end of the series, we've got this:

void __start_xen(unsigned long mbi_p)
{
    ...
    multiboot_info_t *mbi;
    module_t *mod;
    ...

    if ( pvh_boot )
    {
        ASSERT(mbi_p == 0);
        pvh_init(&mbi, &mod);
    }
    else
    {
        mbi = __va(mbi_p);
        mod = __va(mbi->mods_addr);
    }

    multiboot_to_bootinfo(mbi, mod);


which are the sum total of the mbi and mod pointers.  Worse, pvh_init()
is transforming the PVH into into MB1 info, just to be transformed
immediately to BI.

I expect this is work for the end of the series (I can't think of a nice
way to disentangle it earlier), but could we end up with something more
like:

    if ( pvh_boot )
    {
        ASSERT(mbi_p == 0);
        pvh_fill_boot_info();
    }
    else
    {
        multiboot_info_t *mbi = __va(mbi_p);

        multiboot_fill_boot_info(mbi, __va(mbi->mods_addr));
    }

?

Or perhaps even just pass mbi_p in, and have multiboot_fill_boot_info()
do the __va()'s itself.

If so, we probably want to make a naming and possibly prototype
difference in this patch.

~Andrew

Re: [PATCH v4 02/44] x86/boot: move boot loader name to boot info

[Andrew Cooper]

On 30/08/2024 10:46 pm, Daniel P. Smith wrote:
> Transition the incoming boot loader name to be held in struct boot_info.
>
> No functional change intended.
>
> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
> ---
>  xen/arch/x86/include/asm/bootinfo.h |  2 ++
>  xen/arch/x86/setup.c                | 15 ++++++++-------
>  2 files changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/xen/arch/x86/include/asm/bootinfo.h b/xen/arch/x86/include/asm/bootinfo.h
> index e850f80d26a7..e69feb1bb8be 100644
> --- a/xen/arch/x86/include/asm/bootinfo.h
> +++ b/xen/arch/x86/include/asm/bootinfo.h
> @@ -10,6 +10,8 @@
>  
>  struct boot_info {
>      unsigned int nr_mods;
> +
> +    const char *boot_loader_name;

Simply loader, matching the __setup_xen() variable you dropped, will be
fine.

> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index dd94ee2e736b..432b7d1701e4 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -2054,7 +2055,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
>       */
>      dom0 = create_dom0(mod, modules_headroom,
>                         initrdidx < boot_info->nr_mods ? mod + initrdidx : NULL,
> -                       kextra, loader);
> +                       kextra, boot_info->boot_loader_name);

Do I want to know why create_dom0() cares about our bootloader?  I'm
sure the answer is no.

~Andrew

Re: [PATCH v4 03/44] x86/boot: move cmdline to boot info

[Andrew Cooper]

On 30/08/2024 10:46 pm, Daniel P. Smith wrote:
> Transition Xen's command line to being held in struct boot_info.
>
> No functional change intended.
>
> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>

Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index 432b7d1701e4..a945fa10555f 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -1049,11 +1058,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
>  
>      multiboot_to_bootinfo(mbi);
>  
> -    /* Parse the command-line options. */
> -    if ( mbi->flags & MBI_CMDLINE )
> -        cmdline = cmdline_cook(__va(mbi->cmdline), boot_info->boot_loader_name);
> -
> -    if ( (kextra = strstr(cmdline, " -- ")) != NULL )
> +    if ( (kextra = strstr(boot_info->cmdline, " -- ")) != NULL )
>      {
>          /*
>           * Options after ' -- ' separator belong to dom0.
> @@ -1064,7 +1069,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
>          kextra += 3;
>          while ( kextra[1] == ' ' ) kextra++;
>      }
> -    cmdline_parse(cmdline);
> +    cmdline_parse(boot_info->cmdline);

It would be nice to get this kextra handling out of __start_xen(), but
I'm not entirely sure how.

It shouldn't live in multiboot_fill_boot_info() if that's going to be
split for pvh, yet it really ought to live with the other editing of
bi->cmdline.

Something that is very subtle is that the *kextra = '\0' between these
two hunks ends up truncating bi->cmdline.

Perhaps best to leave it alone until inspiration strikes.

~Andrew

Re: [PATCH v4 04/44] x86/boot: move mmap info to boot info

[Andrew Cooper]

On 30/08/2024 10:46 pm, Daniel P. Smith wrote:
> Transition the memory map info to be held in struct boot_info.
>
> No functional change intended.
>
> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
> ---
>  xen/arch/x86/include/asm/bootinfo.h |  5 +++++
>  xen/arch/x86/setup.c                | 12 +++++++++---
>  2 files changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/xen/arch/x86/include/asm/bootinfo.h b/xen/arch/x86/include/asm/bootinfo.h
> index d2ca077d2356..e785ed1c5982 100644
> --- a/xen/arch/x86/include/asm/bootinfo.h
> +++ b/xen/arch/x86/include/asm/bootinfo.h
> @@ -8,11 +8,16 @@
>  #ifndef __XEN_X86_BOOTINFO_H__
>  #define __XEN_X86_BOOTINFO_H__
>  
> +#include <xen/types.h>
> +
>  struct boot_info {
>      unsigned int nr_mods;
>  
>      const char *boot_loader_name;
>      const char *cmdline;
> +
> +    paddr_t mmap_addr;
> +    uint32_t mmap_length;

memmap please.

> @@ -1200,13 +1206,13 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
>      {
>          memmap_type = "Xen-e820";
>      }
> -    else if ( mbi->flags & MBI_MEMMAP )
> +    else if ( boot_info->mmap_addr )
>      {
>          memmap_type = "Multiboot-e820";
> -        while ( bytes < mbi->mmap_length &&
> +        while ( bytes < boot_info->mmap_length &&
>                  e820_raw.nr_map < ARRAY_SIZE(e820_raw.map) )
>          {
> -            memory_map_t *map = __va(mbi->mmap_addr + bytes);
> +            memory_map_t *map = __va(boot_info->mmap_addr + bytes);
>  
>              /*
>               * This is a gross workaround for a BIOS bug. Some bootloaders do

This is some very gnarly logic.  pvh_init() plays with e820_raw behind
the scenes and doesn't set MBI_MEMMAP.

Perhaps for later cleanup too, this logic wants folding into the new
multiboot_fill_boot_info() and leave __start_xen().

~Andrew

Re: [PATCH v4 05/44] x86/boot: introduce struct boot_module

[Andrew Cooper]

On 30/08/2024 10:46 pm, Daniel P. Smith wrote:
> diff --git a/xen/arch/x86/include/asm/bootinfo.h b/xen/arch/x86/include/asm/bootinfo.h
> index e785ed1c5982..844262495962 100644
> --- a/xen/arch/x86/include/asm/bootinfo.h
> +++ b/xen/arch/x86/include/asm/bootinfo.h
> @@ -8,10 +8,16 @@
>  #ifndef __XEN_X86_BOOTINFO_H__
>  #define __XEN_X86_BOOTINFO_H__
>  
> +#include <xen/multiboot.h>
>  #include <xen/types.h>
>  
> +struct boot_module {
> +    module_t *early_mod;

This could do with a /* Transitionary only */ comment.  In this patch
it's not too bad, but it does get worse as new fields are added, before
being removed.

I'd also drop the "early_" part.  I know it's the initial_images array
we're converting, but "early_" doesn't convey any extra meaning, and it
makes a number of lines get quite hairy.

> +};
> +
>  struct boot_info {
>      unsigned int nr_mods;
> +    struct boot_module *mods;

struct boot_module modules[MAX_NR_BOOTMODS + 1];

Probably at the end of the structure.  In turn it ...

>  
>      const char *boot_loader_name;
>      const char *cmdline;
> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index c6b45ced00ae..28fdbf4d4c2b 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -278,12 +278,17 @@ custom_param("acpi", parse_acpi_param);
>  
>  static const char *cmdline_cook(const char *p, const char *loader_name);
>  
> +/* Max number of boot modules a bootloader can provide in addition to Xen */
> +#define MAX_NR_BOOTMODS 63
> +
>  static const module_t *__initdata initial_images;
>  static struct boot_info __initdata *boot_info;
>  
> -static void __init multiboot_to_bootinfo(multiboot_info_t *mbi)
> +static void __init multiboot_to_bootinfo(multiboot_info_t *mbi, module_t *mods)
>  {
>      static struct boot_info __initdata info;
> +    static struct boot_module __initdata boot_mods[MAX_NR_BOOTMODS + 1];

... drops this static.

> +    unsigned int i;
>  
>      info.nr_mods = mbi->mods_count;
>  
> @@ -303,6 +308,14 @@ static void __init multiboot_to_bootinfo(multiboot_info_t *mbi)
>          info.mmap_length = mbi->mmap_length;
>      }
>  
> +    info.mods = boot_mods;
> +
> +    for ( i=0; i < info.nr_mods; i++ )

i = 0

> +        boot_mods[i].early_mod = &mods[i];
> +
> +    /* map the last mb module for xen entry */
> +    boot_mods[info.nr_mods].early_mod = &mods[info.nr_mods];

The comment is good, but note how this is just one extra iteration of
the loop, (so use <= for the bound).

~Andew

Re: [PATCH v4 07/44] x86/boot: move headroom to boot modules

[Andrew Cooper]

On 30/08/2024 10:46 pm, Daniel P. Smith wrote:
> diff --git a/xen/arch/x86/include/asm/bootinfo.h b/xen/arch/x86/include/asm/bootinfo.h
> index 844262495962..3e0e36df096b 100644
> --- a/xen/arch/x86/include/asm/bootinfo.h
> +++ b/xen/arch/x86/include/asm/bootinfo.h
> @@ -13,6 +13,7 @@
>  
>  struct boot_module {
>      module_t *early_mod;
> +    unsigned long headroom;

This needs a comment explaining what it's for.  Perhaps crib from ...

> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index 8912956ee7f1..fd6cc7fac907 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -1475,8 +1477,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.
>               */

... here, while also editing to to prevent it going stale.

It is this patch which stops modules_headroom being strictly the dom0
kernel.

~Andrew

Re: [PATCH v4 13/44] x86/boot: update struct boot_module on module relocation

[Andrew Cooper]

On 30/08/2024 10:46 pm, Daniel P. Smith wrote:
> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index 021c5699f86c..27517d24b2ea 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -1408,8 +1408,10 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
>           * respective reserve_e820_ram() invocation below. No need to
>           * query efi_boot_mem_unused() here, though.
>           */
> -        boot_info->mods[idx].early_mod->mod_start = virt_to_mfn(_stext);
> -        boot_info->mods[idx].early_mod->mod_end = __2M_rwdata_end - _stext;
> +        boot_info->mods[idx].start = boot_info->mods[idx].early_mod->mod_start
> +                                   = virt_to_mfn(_stext);
> +        boot_info->mods[idx].size = boot_info->mods[idx].early_mod->mod_end
> +                                  = __2M_rwdata_end - _stext;

MISRA objects to using a = b = c; syntax, and we're being asked to take
it out elsewhere.

It would be best to make local start/size variables, and the resulting
code will have less churn through the rest of the series.

~Andrew

Re: [PATCH v4 08/44] x86/boot: convert setup.c mod refs to early_mod

[Andrew Cooper]

On 30/08/2024 10:46 pm, Daniel P. Smith wrote:
> @@ -1379,6 +1379,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
>  
>      if ( xen_phys_start )
>      {
> +        int idx = boot_info->nr_mods;

unsigned int, but I'd be tempted to name it xen, so the hunk below is a
bit more intuitive to read.

~Andrew

Re: [PATCH v4 01/44] x86/boot: move x86 boot module counting into a new boot_info struct

[Jan Beulich]

On 02.09.2024 15:47, Alejandro Vallejo wrote:
> On Fri Aug 30, 2024 at 10:46 PM BST, Daniel P. Smith wrote:
>> @@ -309,7 +319,7 @@ void __init discard_initial_images(void)
>>                             start + PAGE_ALIGN(initial_images[i].mod_end));
>>      }
>>  
>> -    nr_initial_images = 0;
>> +    boot_info->nr_mods = 0;
> 
> Out of curiosity, why is this required?

Together with ...

>>      initial_images = NULL;

... this - to prevent undue access attempts later on. IOW just to be on
the safe side, aiui.

Jan

Re: [PATCH v4 04/44] x86/boot: move mmap info to boot info

[Jan Beulich]

On 30.08.2024 23:46, Daniel P. Smith wrote:
> --- a/xen/arch/x86/include/asm/bootinfo.h
> +++ b/xen/arch/x86/include/asm/bootinfo.h
> @@ -8,11 +8,16 @@
>  #ifndef __XEN_X86_BOOTINFO_H__
>  #define __XEN_X86_BOOTINFO_H__
>  
> +#include <xen/types.h>
> +
>  struct boot_info {
>      unsigned int nr_mods;
>  
>      const char *boot_loader_name;
>      const char *cmdline;
> +
> +    paddr_t mmap_addr;
> +    uint32_t mmap_length;

Why would this need to be a fixed-width type, unless we went in the direction
of what Alejandro mentioned in reply to patch 1? IOW at least we want to be
consistent with which kind of types are used here.

Jan

Re: [PATCH v4 01/44] x86/boot: move x86 boot module counting into a new boot_info struct

[Jan Beulich]

On 30.08.2024 23:46, Daniel P. Smith wrote:
> @@ -276,7 +277,16 @@ static int __init cf_check parse_acpi_param(const char *s)
>  custom_param("acpi", parse_acpi_param);
>  
>  static const module_t *__initdata initial_images;
> -static unsigned int __initdata nr_initial_images;
> +static struct boot_info __initdata *boot_info;
> +
> +static void __init multiboot_to_bootinfo(multiboot_info_t *mbi)

Pointer-to-const please.

Jan

Re: [PATCH v4 05/44] x86/boot: introduce struct boot_module

[Jan Beulich]

On 30.08.2024 23:46, Daniel P. Smith wrote:
> @@ -303,6 +308,14 @@ static void __init multiboot_to_bootinfo(multiboot_info_t *mbi)
>          info.mmap_length = mbi->mmap_length;
>      }
>  
> +    info.mods = boot_mods;
> +
> +    for ( i=0; i < info.nr_mods; i++ )
> +        boot_mods[i].early_mod = &mods[i];
> +
> +    /* map the last mb module for xen entry */

Nit: Comment style.

Jan

Re: [PATCH v4 06/44] x86/boot: convert consider_modules to struct boot_module

[Jan Beulich]

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

Re: [PATCH v4 07/44] x86/boot: move headroom to boot modules

[Jan Beulich]

On 30.08.2024 23:46, Daniel P. Smith wrote:
> The purpose of struct boot_module is to encapsulate the state of boot modules.
> Doing locates boot module state with its respective boot module, reduces

I'm struggling with the start of this sentence.

> @@ -1390,7 +1390,9 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
>          mod[boot_info->nr_mods].mod_end = __2M_rwdata_end - _stext;
>      }
>  
> -    modules_headroom = bzimage_headroom(bootstrap_map(mod), mod->mod_end);
> +    boot_info->mods[0].headroom = bzimage_headroom(
> +                        bootstrap_map(boot_info->mods[0].early_mod),
> +                        boot_info->mods[0].early_mod->mod_end);

Nit: This is badly indented. Either

    boot_info->mods[0].headroom = bzimage_headroom(
        bootstrap_map(boot_info->mods[0].early_mod),
        boot_info->mods[0].early_mod->mod_end);

or

    boot_info->mods[0].headroom =
        bzimage_headroom(
            bootstrap_map(boot_info->mods[0].early_mod),
            boot_info->mods[0].early_mod->mod_end);

or

    boot_info->mods[0].headroom =
        bzimage_headroom(bootstrap_map(boot_info->mods[0].early_mod),
                         boot_info->mods[0].early_mod->mod_end);

Even shortening "boot_info" will not avoid some line wrapping here, as it
looks.

Jan

Re: [PATCH v4 08/44] x86/boot: convert setup.c mod refs to early_mod

[Jan Beulich]

On 30.08.2024 23:46, Daniel P. Smith wrote:
> To allow a slow conversion of x86 over to struct boot_module, start with
> replacing all references to struct mod to the early_mod element of struct
> boot_module. These serves twofold, first to allow the incremental transition
> from struct mod fields to struct boot_module fields.  The second is to allow
> the conversion of function definitions from taking struct mod parameters to
> accepting struct boot_module as needed when a transitioned field will be
> accessed.

Yet earlier it was mentioned that early_mod is a transitory name. Will all of
this then need touching a 2nd time?

Jan

Re: [PATCH v4 09/44] x86/boot: split bootstrap_map_addr() out of bootstrap_map()

[Jan Beulich]

On 30.08.2024 23:46, Daniel P. Smith wrote:
> From: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> Using an interface based on addresses directly, not modules.
> 
> No functional change.

Okay, a mechanical transformation. But what's the goal?

> --- a/xen/arch/x86/include/asm/setup.h
> +++ b/xen/arch/x86/include/asm/setup.h
> @@ -34,6 +34,7 @@ void setup_io_bitmap(struct domain *d);
>  
>  unsigned long initial_images_nrpages(nodeid_t node);
>  void discard_initial_images(void);
> +void *bootstrap_map_addr(uint64_t start, uint64_t end);

Better paddr_t?

Jan

Re: [PATCH v4 06/44] x86/boot: convert consider_modules to struct boot_module

[Andrew Cooper]

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

Re: [PATCH v4 09/44] x86/boot: split bootstrap_map_addr() out of bootstrap_map()

[Andrew Cooper]

On 04/09/2024 7:49 am, Jan Beulich wrote:
> On 30.08.2024 23:46, Daniel P. Smith wrote:
>> From: Andrew Cooper <andrew.cooper3@citrix.com>
>>
>> Using an interface based on addresses directly, not modules.
>>
>> No functional change.
> Okay, a mechanical transformation. But what's the goal?

Its used by patch 12 which adds boostrap_map_bm(), but does want to be
reordered later in the series to immediately before it's used.


This is a patch of mine from a prior cleanup attempt.

I'm still itching to get rid of the chunking and backwards memcpy in
move_module(), but I'll pick the work back up again when it won't
collide with this series.

~Andrew

Re: [PATCH v4 01/44] x86/boot: move x86 boot module counting into a new boot_info struct

[Daniel P. Smith]

On 9/2/24 09:47, Alejandro Vallejo wrote:
> I haven't read the entire series yet, but here's my .02 so far
> 
> On Fri Aug 30, 2024 at 10:46 PM BST, Daniel P. Smith wrote:
>> From: Christopher Clark <christopher.w.clark@gmail.com>
>>
>> An initial step towards a non-multiboot internal representation of boot
>> modules for common code, starting with x86 setup and converting the fields
>> that are accessed for the startup calculations.
>>
>> Introduce a new header, <xen/asm/bootinfo.h>, and populate it with a new
>> boot_info structure initially containing a count of the number of boot
>> modules.
>>
>> No functional change intended.
>>
>> Signed-off-by: Christopher Clark <christopher.w.clark@gmail.com>
>> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
>> ---
>>   xen/arch/x86/include/asm/bootinfo.h | 25 +++++++++++++
>>   xen/arch/x86/setup.c                | 58 +++++++++++++++++------------
>>   2 files changed, 59 insertions(+), 24 deletions(-)
>>   create mode 100644 xen/arch/x86/include/asm/bootinfo.h
>>
>> diff --git a/xen/arch/x86/include/asm/bootinfo.h b/xen/arch/x86/include/asm/bootinfo.h
>> new file mode 100644
>> index 000000000000..e850f80d26a7
>> --- /dev/null
>> +++ b/xen/arch/x86/include/asm/bootinfo.h
>> @@ -0,0 +1,25 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>> +/*
>> + * Copyright (c) 2024 Christopher Clark <christopher.w.clark@gmail.com>
>> + * Copyright (c) 2024 Apertus Solutions, LLC
>> + * Author: Daniel P. Smith <dpsmith@apertussolutions.com>
>> + */
>> +
>> +#ifndef __XEN_X86_BOOTINFO_H__
>> +#define __XEN_X86_BOOTINFO_H__
>> +
> 
> This struct would benefit from a comment stating what it's for and how it's
> meant to be used. At a glance it seems like it's meant to be serve as a
> boot-protocol agnostic representation of boot-parameters, used as a generic
> means of information handover. Which would imply multiboot_info is parsed onto
> it when booting from multiboot and is synthesised from scratch in other cases
> (e.g: direct EFI?).

Yes, some inline documentation can be added.

>> +struct boot_info {
>> +    unsigned int nr_mods;
> 
> It's imo better to treat this as an ABI. That would allow using this layer as a
> boot protocol in itself (which I'm guessing is the objective? I haven't gotten
> that far in the series). If so, this would need to be a fixed-width uintN_t.
> 
> Same with other fields in follow-up patches.

The intent is to provide a clean internal abstraction around the boot 
material provided to Xen and not as an external boot protocol. The 
follow-on series to come will build upon this to introduce a 
representation of domains to be constructed at boot. A side goal with 
later introduced fields is to use proper xen types for the fields, eg. 
paddr_t, and mfn_t.

>> +};
>> +
>> +#endif
>> +
>> +/*
>> + * Local variables:
>> + * mode: C
>> + * c-file-style: "BSD"
>> + * c-basic-offset: 4
>> + * tab-width: 4
>> + * indent-tabs-mode: nil
>> + * End:
>> + */
>> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
>> index eee20bb1753c..dd94ee2e736b 100644
>> --- a/xen/arch/x86/setup.c
>> +++ b/xen/arch/x86/setup.c
>> @@ -32,6 +32,7 @@
>>   #include <compat/xen.h>
>>   #endif
>>   #include <xen/bitops.h>
>> +#include <asm/bootinfo.h>
>>   #include <asm/smp.h>
>>   #include <asm/processor.h>
>>   #include <asm/mpspec.h>
>> @@ -276,7 +277,16 @@ static int __init cf_check parse_acpi_param(const char *s)
>>   custom_param("acpi", parse_acpi_param);
>>   
>>   static const module_t *__initdata initial_images;
>> -static unsigned int __initdata nr_initial_images;
>> +static struct boot_info __initdata *boot_info;
>> +
>> +static void __init multiboot_to_bootinfo(multiboot_info_t *mbi)
> 
> If this function returned boot_info instead and the caller made the
> assignment then it would be possible to unit-test/fuzz it.
> 
> It also fits a bit more nicely with the usual implications of that function
> name pattern, I think.

As the larger capability continues to be developed, it is becoming 
necessary to get access to the reference from other areas of the code. 
Currently, I just move boot_info outside the function and exported the 
reference. I still have as an open question if the declaration should be 
static and access is obtained through an accessor function(s). I'm open 
to suggestions here.

>> +{
>> +    static struct boot_info __initdata info;
>> +
>> +    info.nr_mods = mbi->mods_count;
> 
> Shouldn't this be gated on MBI_MODULES being set?
> 
>     info.nr_mods = (mbi->flags & MBI_MODULES) ? mbi->mods_count : 0;

Yes.

>> +
>> +    boot_info = &info;
>> +}
>>   
>>   unsigned long __init initial_images_nrpages(nodeid_t node)
>>   {
>> @@ -285,7 +295,7 @@ unsigned long __init initial_images_nrpages(nodeid_t node)
>>       unsigned long nr;
>>       unsigned int i;
>>   
>> -    for ( nr = i = 0; i < nr_initial_images; ++i )
>> +    for ( nr = i = 0; i < boot_info->nr_mods; ++i )
>>       {
>>           unsigned long start = initial_images[i].mod_start;
>>           unsigned long end = start + PFN_UP(initial_images[i].mod_end);
>> @@ -301,7 +311,7 @@ void __init discard_initial_images(void)
>>   {
>>       unsigned int i;
>>   
>> -    for ( i = 0; i < nr_initial_images; ++i )
>> +    for ( i = 0; i < boot_info->nr_mods; ++i )
>>       {
>>           uint64_t start = (uint64_t)initial_images[i].mod_start << PAGE_SHIFT;
>>   
>> @@ -309,7 +319,7 @@ void __init discard_initial_images(void)
>>                              start + PAGE_ALIGN(initial_images[i].mod_end));
>>       }
>>   
>> -    nr_initial_images = 0;
>> +    boot_info->nr_mods = 0;
> 
> Out of curiosity, why is this required?

At this point, the boot modules have been "consumed" and are no longer 
safe to access. This ensures that if for some reason any module access 
code gets invoked that attempts to walk the modules, this will ensure
it will result in no access.

>>       initial_images = NULL;
>>   }
>>   
>> @@ -1034,9 +1044,10 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
>>           mod = __va(mbi->mods_addr);
>>       }
>>   
>> +    multiboot_to_bootinfo(mbi);
>> +
>>       loader = (mbi->flags & MBI_LOADERNAME) ? __va(mbi->boot_loader_name)
>>                                              : "unknown";
>> -
> 
> Stray newline removal?
> 
>>       /* Parse the command-line options. */
>>       if ( mbi->flags & MBI_CMDLINE )
>>           cmdline = cmdline_cook(__va(mbi->cmdline), loader);
>> @@ -1141,18 +1152,18 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
>>              bootsym(boot_edd_info_nr));
>>   
>>       /* Check that we have at least one Multiboot module. */
>> -    if ( !(mbi->flags & MBI_MODULES) || (mbi->mods_count == 0) )
>> +    if ( !(mbi->flags & MBI_MODULES) || (boot_info->nr_mods == 0) )
> 
> With MBI_MODULES accounted for during conversion, the first part of the
> conditional can be ellided and you could simply do:
> 
>      if ( !boot_info->nr_mods )
>          panic(...)

This eventually happens, the goal in this revision was to slowly unhook 
the mbi usage as it is pervasive and so the reviewers can ensure we are 
correctly replacing all of its usage. With that said, I can look to see 
if it is reasonable to add its usage here.

> Also, could we move this to multiboot_to_bootinfo()? It'd contain these sorts
> of boot argument checks to a much more self contained function and help check
> at the point of assignment, preventing misuse.

These checks could be moved up into that function, but then you would 
lose the ability to print messages since the conversion function is 
invoked before serial is initialized.

>>           panic("dom0 kernel not specified. Check bootloader configuration\n");
>>   
>>       /* Check that we don't have a silly number of modules. */
> 
>> -    if ( mbi->mods_count > sizeof(module_map) * 8 )
>> +    if ( boot_info->nr_mods > sizeof(module_map) * 8 )
> 
> Like above, this check would be much more neatly contained where boot_info
> is created, imo.

Again, you would lose the ability to print the error message.

>>       {
>> -        mbi->mods_count = sizeof(module_map) * 8;
>> +        boot_info->nr_mods = sizeof(module_map) * 8;
>>           printk("Excessive multiboot modules - using the first %u only\n",
> 
> Does the comment need adjusting too to make it more general? As in
> s/multiboot/boot.

I don't see why the message couldn't be generalized.

>> -               mbi->mods_count);
>> +               boot_info->nr_mods);
>>       }
>>   
>> -    bitmap_fill(module_map, mbi->mods_count);
>> +    bitmap_fill(module_map, boot_info->nr_mods);
>>       __clear_bit(0, module_map); /* Dom0 kernel is always first */
>>   
>>       if ( pvh_boot )
> 
> Cheers,
> Alejandro

Thanks for the review.

v/r
dps

Re: [PATCH v4 01/44] x86/boot: move x86 boot module counting into a new boot_info struct

[Daniel P. Smith]

On 9/3/24 18:24, Andrew Cooper wrote:
> On 30/08/2024 10:46 pm, Daniel P. Smith wrote:
>> From: Christopher Clark <christopher.w.clark@gmail.com>
>>
>> An initial step towards a non-multiboot internal representation of boot
>> modules for common code, starting with x86 setup and converting the fields
>> that are accessed for the startup calculations.
>>
>> Introduce a new header, <xen/asm/bootinfo.h>, and populate it with a new
> 
> Just <asm/bootinfo.h>, which matches the code.

Ack.

>> diff --git a/xen/arch/x86/include/asm/bootinfo.h b/xen/arch/x86/include/asm/bootinfo.h
>> new file mode 100644
>> index 000000000000..e850f80d26a7
>> --- /dev/null
>> +++ b/xen/arch/x86/include/asm/bootinfo.h
>> @@ -0,0 +1,25 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>> +/*
>> + * Copyright (c) 2024 Christopher Clark <christopher.w.clark@gmail.com>
>> + * Copyright (c) 2024 Apertus Solutions, LLC
>> + * Author: Daniel P. Smith <dpsmith@apertussolutions.com>
>> + */
>> +
>> +#ifndef __XEN_X86_BOOTINFO_H__
>> +#define __XEN_X86_BOOTINFO_H__
>> +
> 
> There ought to be a short description of what boot_info is, even if it's
> only "Xen's local representation of information provided by the
> bootloader/environment."

Yes, will be adding a description.

>> +struct boot_info {
>> +    unsigned int nr_mods;
> 
> For the sake of 3 letters, please can this be nr_modules.  I've run sed
> over the top of the v5 branch and it doesn't change line wrapping
> anywhere, but it is a legibility improvement IMO.

I'm okay with that.

>> +};
>> +
>> +#endif
> 
> #endif /* __XEN_X86_BOOTINFO_H__ */
> 
> It very quickly get to not being in the same few lines as the #ifndef.

Ack.

>> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
>> index eee20bb1753c..dd94ee2e736b 100644
>> --- a/xen/arch/x86/setup.c
>> +++ b/xen/arch/x86/setup.c
>> @@ -276,7 +277,16 @@ static int __init cf_check parse_acpi_param(const char *s)
>>   custom_param("acpi", parse_acpi_param);
>>   
>>   static const module_t *__initdata initial_images;
>> -static unsigned int __initdata nr_initial_images;
>> +static struct boot_info __initdata *boot_info;
>> +
>> +static void __init multiboot_to_bootinfo(multiboot_info_t *mbi)
>> +{
>> +    static struct boot_info __initdata info;
>> +
>> +    info.nr_mods = mbi->mods_count;
>> +
>> +    boot_info = &info;
>> +}
> 
> Having a global pointer set only to this private structure is weird.
> Even this:
> 
>      static struct boot_info __initdata boot_info[1];
> 
> lets you keep -> notation, but removes one level of indirection.

Further work has pushed this into being a global, at least in the 
yet-to-be public work. The question is whether the allocation should 
just move out to the unit level or use a level of in direction with an
accessor function.

>> @@ -1034,9 +1044,10 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
>>           mod = __va(mbi->mods_addr);
>>       }
>>   
>> +    multiboot_to_bootinfo(mbi);
>> +
>>       loader = (mbi->flags & MBI_LOADERNAME) ? __va(mbi->boot_loader_name)
>>                                              : "unknown";
>> -
> 
> Stray line removal.  (should be in patch 2 to minimise churn.)

Ack.

> ~Andrew

Thanks!

v/r,
dps

Re: [PATCH v4 01/44] x86/boot: move x86 boot module counting into a new boot_info struct

[Daniel P. Smith]

On 9/3/24 18:35, Andrew Cooper wrote:
> On 30/08/2024 10:46 pm, Daniel P. Smith wrote:
>> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
>> index eee20bb1753c..dd94ee2e736b 100644
>> --- a/xen/arch/x86/setup.c
>> +++ b/xen/arch/x86/setup.c
>> @@ -1034,9 +1044,10 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
>>           mod = __va(mbi->mods_addr);
>>       }
>>   
>> +    multiboot_to_bootinfo(mbi);
> 
> Actually, peeking ahead to the end of the series, we've got this:
> 
> void __start_xen(unsigned long mbi_p)
> {
>      ...
>      multiboot_info_t *mbi;
>      module_t *mod;
>      ...
> 
>      if ( pvh_boot )
>      {
>          ASSERT(mbi_p == 0);
>          pvh_init(&mbi, &mod);
>      }
>      else
>      {
>          mbi = __va(mbi_p);
>          mod = __va(mbi->mods_addr);
>      }
> 
>      multiboot_to_bootinfo(mbi, mod);
> 
> 
> which are the sum total of the mbi and mod pointers.  Worse, pvh_init()
> is transforming the PVH into into MB1 info, just to be transformed
> immediately to BI.
> 
> I expect this is work for the end of the series (I can't think of a nice
> way to disentangle it earlier), but could we end up with something more
> like:
> 
>      if ( pvh_boot )
>      {
>          ASSERT(mbi_p == 0);
>          pvh_fill_boot_info();
>      }
>      else
>      {
>          multiboot_info_t *mbi = __va(mbi_p);
> 
>          multiboot_fill_boot_info(mbi, __va(mbi->mods_addr));
>      }
> 
> ?
> 
> Or perhaps even just pass mbi_p in, and have multiboot_fill_boot_info()
> do the __va()'s itself.
> 
> If so, we probably want to make a naming and possibly prototype
> difference in this patch.

Let me combine this with some of Alejandro's comments. What if I were to 
reshape it to look like this,

/*
  * This level of indirection may not be desired, dropping it is not an
  * issue. I am proposing it because there is going to be a need to
  * access the instance from distant unit files.
  */
struct boot_info __init *get_boot_info(void)
{
     static struct boot_info __initdata info;

     return &info;
}

static struct boot_info __init *multiboot_fill_boot_info(
     unsigned long mbi_p)
{
     struct boot_info *bi = get_boot_info();
     multiboot_info_t *mbi = __va(mbi_p);
     module_t mods = __va(mbi->mods_addr);

     ...

     return bi;
}

, then in the PVH boot code

struct boot_info __init *pvh_fill_boot_info(void)
{
     struct boot_info *bi = get_boot_info();

     ...

     return bi;
}

, and then something similar for efi.

What does everyone think?

v/r,
dps

Re: [PATCH v4 01/44] x86/boot: move x86 boot module counting into a new boot_info struct

[Daniel P. Smith]

V/r,
Daniel P. Smith
Apertus Solutions, LLC

On 9/4/24 02:31, Jan Beulich wrote:
> On 30.08.2024 23:46, Daniel P. Smith wrote:
>> @@ -276,7 +277,16 @@ static int __init cf_check parse_acpi_param(const char *s)
>>   custom_param("acpi", parse_acpi_param);
>>   
>>   static const module_t *__initdata initial_images;
>> -static unsigned int __initdata nr_initial_images;
>> +static struct boot_info __initdata *boot_info;
>> +
>> +static void __init multiboot_to_bootinfo(multiboot_info_t *mbi)
> 
> Pointer-to-const please.

Provided it does not change based on other feedback, you are correct 
that it should be const.

v/r,
dps

Re: [PATCH v4 02/44] x86/boot: move boot loader name to boot info

[Daniel P. Smith]

On 9/3/24 18:41, Andrew Cooper wrote:
> On 30/08/2024 10:46 pm, Daniel P. Smith wrote:
>> Transition the incoming boot loader name to be held in struct boot_info.
>>
>> No functional change intended.
>>
>> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
>> ---
>>   xen/arch/x86/include/asm/bootinfo.h |  2 ++
>>   xen/arch/x86/setup.c                | 15 ++++++++-------
>>   2 files changed, 10 insertions(+), 7 deletions(-)
>>
>> diff --git a/xen/arch/x86/include/asm/bootinfo.h b/xen/arch/x86/include/asm/bootinfo.h
>> index e850f80d26a7..e69feb1bb8be 100644
>> --- a/xen/arch/x86/include/asm/bootinfo.h
>> +++ b/xen/arch/x86/include/asm/bootinfo.h
>> @@ -10,6 +10,8 @@
>>   
>>   struct boot_info {
>>       unsigned int nr_mods;
>> +
>> +    const char *boot_loader_name;
> 
> Simply loader, matching the __setup_xen() variable you dropped, will be
> fine.

Yep, can do.

>> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
>> index dd94ee2e736b..432b7d1701e4 100644
>> --- a/xen/arch/x86/setup.c
>> +++ b/xen/arch/x86/setup.c
>> @@ -2054,7 +2055,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
>>        */
>>       dom0 = create_dom0(mod, modules_headroom,
>>                          initrdidx < boot_info->nr_mods ? mod + initrdidx : NULL,
>> -                       kextra, loader);
>> +                       kextra, boot_info->boot_loader_name);
> 
> Do I want to know why create_dom0() cares about our bootloader?  I'm
> sure the answer is no.

Because in cmdline_cook(), you have this:

/*
  * PVH, our EFI loader, and GRUB2 don't include image name as first
  * item on command line.
  */
if ( xen_guest || efi_enabled(EFI_LOADER) || loader_is_grub2(loader_name) )
         return p;

Later in the series, all of this is no longer passed but it is still 
used by accessing the struct boot_info instance.

v/r,
dps

Re: [PATCH v4 03/44] x86/boot: move cmdline to boot info

[Daniel P. Smith]

On 9/3/24 19:04, Andrew Cooper wrote:
> On 30/08/2024 10:46 pm, Daniel P. Smith wrote:
>> Transition Xen's command line to being held in struct boot_info.
>>
>> No functional change intended.
>>
>> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
> 
> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

Thank you.

>> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
>> index 432b7d1701e4..a945fa10555f 100644
>> --- a/xen/arch/x86/setup.c
>> +++ b/xen/arch/x86/setup.c
>> @@ -1049,11 +1058,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
>>   
>>       multiboot_to_bootinfo(mbi);
>>   
>> -    /* Parse the command-line options. */
>> -    if ( mbi->flags & MBI_CMDLINE )
>> -        cmdline = cmdline_cook(__va(mbi->cmdline), boot_info->boot_loader_name);
>> -
>> -    if ( (kextra = strstr(cmdline, " -- ")) != NULL )
>> +    if ( (kextra = strstr(boot_info->cmdline, " -- ")) != NULL )
>>       {
>>           /*
>>            * Options after ' -- ' separator belong to dom0.
>> @@ -1064,7 +1069,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
>>           kextra += 3;
>>           while ( kextra[1] == ' ' ) kextra++;
>>       }
>> -    cmdline_parse(cmdline);
>> +    cmdline_parse(boot_info->cmdline);
> 
> It would be nice to get this kextra handling out of __start_xen(), but
> I'm not entirely sure how.
> 
> It shouldn't live in multiboot_fill_boot_info() if that's going to be
> split for pvh, yet it really ought to live with the other editing of
> bi->cmdline.
> 
> Something that is very subtle is that the *kextra = '\0' between these
> two hunks ends up truncating bi->cmdline.
> 
> Perhaps best to leave it alone until inspiration strikes.

One thought is to move all the logic into a function and make it the 
responsibility of fill function(s) to call it, since each entry point 
has its own unique way for the xen command line to be passed in.

v/r,
dps

Re: [PATCH v4 04/44] x86/boot: move mmap info to boot info

[Daniel P. Smith]

On 9/3/24 19:18, Andrew Cooper wrote:
> On 30/08/2024 10:46 pm, Daniel P. Smith wrote:
>> Transition the memory map info to be held in struct boot_info.
>>
>> No functional change intended.
>>
>> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
>> ---
>>   xen/arch/x86/include/asm/bootinfo.h |  5 +++++
>>   xen/arch/x86/setup.c                | 12 +++++++++---
>>   2 files changed, 14 insertions(+), 3 deletions(-)
>>
>> diff --git a/xen/arch/x86/include/asm/bootinfo.h b/xen/arch/x86/include/asm/bootinfo.h
>> index d2ca077d2356..e785ed1c5982 100644
>> --- a/xen/arch/x86/include/asm/bootinfo.h
>> +++ b/xen/arch/x86/include/asm/bootinfo.h
>> @@ -8,11 +8,16 @@
>>   #ifndef __XEN_X86_BOOTINFO_H__
>>   #define __XEN_X86_BOOTINFO_H__
>>   
>> +#include <xen/types.h>
>> +
>>   struct boot_info {
>>       unsigned int nr_mods;
>>   
>>       const char *boot_loader_name;
>>       const char *cmdline;
>> +
>> +    paddr_t mmap_addr;
>> +    uint32_t mmap_length;
> 
> memmap please.

Ack.

>> @@ -1200,13 +1206,13 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
>>       {
>>           memmap_type = "Xen-e820";
>>       }
>> -    else if ( mbi->flags & MBI_MEMMAP )
>> +    else if ( boot_info->mmap_addr )
>>       {
>>           memmap_type = "Multiboot-e820";
>> -        while ( bytes < mbi->mmap_length &&
>> +        while ( bytes < boot_info->mmap_length &&
>>                   e820_raw.nr_map < ARRAY_SIZE(e820_raw.map) )
>>           {
>> -            memory_map_t *map = __va(mbi->mmap_addr + bytes);
>> +            memory_map_t *map = __va(boot_info->mmap_addr + bytes);
>>   
>>               /*
>>                * This is a gross workaround for a BIOS bug. Some bootloaders do
> 
> This is some very gnarly logic.  pvh_init() plays with e820_raw behind
> the scenes and doesn't set MBI_MEMMAP.
> 
> Perhaps for later cleanup too, this logic wants folding into the new
> multiboot_fill_boot_info() and leave __start_xen().

I can add another patch that focuses on moving this to 
multiboot_fill_boot_info(). If there is also a transition to 
pvh_fill_boot_info(), then the question I have is, should this be better
served as a separate function similar to my proposal with the command 
line parsing?

v/r,
dps

Re: [PATCH v4 04/44] x86/boot: move mmap info to boot info

[Daniel P. Smith]

On 9/4/24 02:26, Jan Beulich wrote:
> On 30.08.2024 23:46, Daniel P. Smith wrote:
>> --- a/xen/arch/x86/include/asm/bootinfo.h
>> +++ b/xen/arch/x86/include/asm/bootinfo.h
>> @@ -8,11 +8,16 @@
>>   #ifndef __XEN_X86_BOOTINFO_H__
>>   #define __XEN_X86_BOOTINFO_H__
>>   
>> +#include <xen/types.h>
>> +
>>   struct boot_info {
>>       unsigned int nr_mods;
>>   
>>       const char *boot_loader_name;
>>       const char *cmdline;
>> +
>> +    paddr_t mmap_addr;
>> +    uint32_t mmap_length;
> 
> Why would this need to be a fixed-width type, unless we went in the direction
> of what Alejandro mentioned in reply to patch 1? IOW at least we want to be
> consistent with which kind of types are used here.

At of right now, I would prefer not to go down the path of a boot 
protocol, but I am willing to have the discussion if a majority pushes 
for it. Unless that happens, I will switch this to size_t.

v/r,
dps

Re: [PATCH v4 05/44] x86/boot: introduce struct boot_module

[Daniel P. Smith]

On 9/3/24 19:29, Andrew Cooper wrote:
> On 30/08/2024 10:46 pm, Daniel P. Smith wrote:
>> diff --git a/xen/arch/x86/include/asm/bootinfo.h b/xen/arch/x86/include/asm/bootinfo.h
>> index e785ed1c5982..844262495962 100644
>> --- a/xen/arch/x86/include/asm/bootinfo.h
>> +++ b/xen/arch/x86/include/asm/bootinfo.h
>> @@ -8,10 +8,16 @@
>>   #ifndef __XEN_X86_BOOTINFO_H__
>>   #define __XEN_X86_BOOTINFO_H__
>>   
>> +#include <xen/multiboot.h>
>>   #include <xen/types.h>
>>   
>> +struct boot_module {
>> +    module_t *early_mod;
> 
> This could do with a /* Transitionary only */ comment.  In this patch
> it's not too bad, but it does get worse as new fields are added, before
> being removed.

Yep, can add a comment.

> I'd also drop the "early_" part.  I know it's the initial_images array
> we're converting, but "early_" doesn't convey any extra meaning, and it
> makes a number of lines get quite hairy.

I can drop it.

>> +};
>> +
>>   struct boot_info {
>>       unsigned int nr_mods;
>> +    struct boot_module *mods;
> 
> struct boot_module modules[MAX_NR_BOOTMODS + 1];
> 
> Probably at the end of the structure.  In turn it ...

I can move it here, though just to be clear, are you suggesting that it 
is kept at the end of the structure as more fields are added.

>>   
>>       const char *boot_loader_name;
>>       const char *cmdline;
>> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
>> index c6b45ced00ae..28fdbf4d4c2b 100644
>> --- a/xen/arch/x86/setup.c
>> +++ b/xen/arch/x86/setup.c
>> @@ -278,12 +278,17 @@ custom_param("acpi", parse_acpi_param);
>>   
>>   static const char *cmdline_cook(const char *p, const char *loader_name);
>>   
>> +/* Max number of boot modules a bootloader can provide in addition to Xen */
>> +#define MAX_NR_BOOTMODS 63
>> +
>>   static const module_t *__initdata initial_images;
>>   static struct boot_info __initdata *boot_info;
>>   
>> -static void __init multiboot_to_bootinfo(multiboot_info_t *mbi)
>> +static void __init multiboot_to_bootinfo(multiboot_info_t *mbi, module_t *mods)
>>   {
>>       static struct boot_info __initdata info;
>> +    static struct boot_module __initdata boot_mods[MAX_NR_BOOTMODS + 1];
> 
> ... drops this static.

Will be dropped.

>> +    unsigned int i;
>>   
>>       info.nr_mods = mbi->mods_count;
>>   
>> @@ -303,6 +308,14 @@ static void __init multiboot_to_bootinfo(multiboot_info_t *mbi)
>>           info.mmap_length = mbi->mmap_length;
>>       }
>>   
>> +    info.mods = boot_mods;
>> +
>> +    for ( i=0; i < info.nr_mods; i++ )
> 
> i = 0

Ack.

>> +        boot_mods[i].early_mod = &mods[i];
>> +
>> +    /* map the last mb module for xen entry */
>> +    boot_mods[info.nr_mods].early_mod = &mods[info.nr_mods];
> 
> The comment is good, but note how this is just one extra iteration of
> the loop, (so use <= for the bound).

I will move the comment above the loop and adjust the condition.

v/r,
dps

Re: [PATCH v4 05/44] x86/boot: introduce struct boot_module

[Daniel P. Smith]

On 9/4/24 02:33, Jan Beulich wrote:
> On 30.08.2024 23:46, Daniel P. Smith wrote:
>> @@ -303,6 +308,14 @@ static void __init multiboot_to_bootinfo(multiboot_info_t *mbi)
>>           info.mmap_length = mbi->mmap_length;
>>       }
>>   
>> +    info.mods = boot_mods;
>> +
>> +    for ( i=0; i < info.nr_mods; i++ )
>> +        boot_mods[i].early_mod = &mods[i];
>> +
>> +    /* map the last mb module for xen entry */
> 
> Nit: Comment style.

Ack, will fix as part of relocation per Andy's comment.

v/r,
dps

Re: [PATCH v4 06/44] x86/boot: convert consider_modules to struct boot_module

[Daniel P. Smith]

On 9/4/24 02:40, Jan Beulich wrote:
> 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?

Sure.

>> @@ -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?

Yep, I can convert it. I was trying to be conscious of this, and you 
should see it in other places.

>> @@ -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?

Yes, in fact, my apologies as this appears to be a comment you made from 
a previous review. The suggestion from Alejandro will make this easier 
since it will become a local variable to __start_xen().

v/r,
dps

Re: [PATCH v4 06/44] x86/boot: convert consider_modules to struct boot_module

[Daniel P. Smith]

On 9/4/24 06:41, Andrew Cooper wrote:
> 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.)

Yes, I did make it "bi" as it was passed around, and will move setup.c 
to that as well. As for "bm", I will be moving the array into struct 
boot_info. When I do, is there a desire to see the element name to be 
"bm" or "bms"?

v/r,
dps

Re: [PATCH v4 07/44] x86/boot: move headroom to boot modules

[Daniel P. Smith]

On 9/3/24 19:40, Andrew Cooper wrote:
> On 30/08/2024 10:46 pm, Daniel P. Smith wrote:
>> diff --git a/xen/arch/x86/include/asm/bootinfo.h b/xen/arch/x86/include/asm/bootinfo.h
>> index 844262495962..3e0e36df096b 100644
>> --- a/xen/arch/x86/include/asm/bootinfo.h
>> +++ b/xen/arch/x86/include/asm/bootinfo.h
>> @@ -13,6 +13,7 @@
>>   
>>   struct boot_module {
>>       module_t *early_mod;
>> +    unsigned long headroom;
> 
> This needs a comment explaining what it's for.  Perhaps crib from ...

Ack.

>> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
>> index 8912956ee7f1..fd6cc7fac907 100644
>> --- a/xen/arch/x86/setup.c
>> +++ b/xen/arch/x86/setup.c
>> @@ -1475,8 +1477,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.
>>                */
> 
> ... here, while also editing to to prevent it going stale.
> 
> It is this patch which stops modules_headroom being strictly the dom0
> kernel.

Correct, because once multi-domain construction is introduced, there 
will be more than one kernel needing headroom for extraction.

v/r,
dps

Re: [PATCH v4 07/44] x86/boot: move headroom to boot modules

[Daniel P. Smith]

On 9/4/24 02:45, Jan Beulich wrote:
> On 30.08.2024 23:46, Daniel P. Smith wrote:
>> The purpose of struct boot_module is to encapsulate the state of boot modules.
>> Doing locates boot module state with its respective boot module, reduces
> 
> I'm struggling with the start of this sentence.

Yep, grammar check failed to catch. Will fix.

>> @@ -1390,7 +1390,9 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
>>           mod[boot_info->nr_mods].mod_end = __2M_rwdata_end - _stext;
>>       }
>>   
>> -    modules_headroom = bzimage_headroom(bootstrap_map(mod), mod->mod_end);
>> +    boot_info->mods[0].headroom = bzimage_headroom(
>> +                        bootstrap_map(boot_info->mods[0].early_mod),
>> +                        boot_info->mods[0].early_mod->mod_end);
> 
> Nit: This is badly indented. Either
> 
>      boot_info->mods[0].headroom = bzimage_headroom(
>          bootstrap_map(boot_info->mods[0].early_mod),
>          boot_info->mods[0].early_mod->mod_end);
> 
> or
> 
>      boot_info->mods[0].headroom =
>          bzimage_headroom(
>              bootstrap_map(boot_info->mods[0].early_mod),
>              boot_info->mods[0].early_mod->mod_end);
> 
> or
> 
>      boot_info->mods[0].headroom =
>          bzimage_headroom(bootstrap_map(boot_info->mods[0].early_mod),
>                           boot_info->mods[0].early_mod->mod_end);
> 
> Even shortening "boot_info" will not avoid some line wrapping here, as it
> looks.

I would lean towards the latter, as I find it the most clear. With the 
shortening of "boot_info" and dropping "early_" per Andy, It might bring 
it up one line. Will see when I do it.

v/r,
dps

Re: [PATCH v4 08/44] x86/boot: convert setup.c mod refs to early_mod

[Daniel P. Smith]

On 9/3/24 19:50, Andrew Cooper wrote:
> On 30/08/2024 10:46 pm, Daniel P. Smith wrote:
>> @@ -1379,6 +1379,7 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
>>   
>>       if ( xen_phys_start )
>>       {
>> +        int idx = boot_info->nr_mods;
> 
> unsigned int, but I'd be tempted to name it xen, so the hunk below is a
> bit more intuitive to read.

Ack on both parts.

v/r,
dps

Re: [PATCH v4 08/44] x86/boot: convert setup.c mod refs to early_mod

[Daniel P. Smith]

On 9/4/24 02:47, Jan Beulich wrote:
> On 30.08.2024 23:46, Daniel P. Smith wrote:
>> To allow a slow conversion of x86 over to struct boot_module, start with
>> replacing all references to struct mod to the early_mod element of struct
>> boot_module. These serves twofold, first to allow the incremental transition
>> from struct mod fields to struct boot_module fields.  The second is to allow
>> the conversion of function definitions from taking struct mod parameters to
>> accepting struct boot_module as needed when a transitioned field will be
>> accessed.
> 
> Yet earlier it was mentioned that early_mod is a transitory name. Will all of
> this then need touching a 2nd time?

Correct, but a lot of these references are parameters to functions who 
then call other functions that are passed the reference, turtles all the 
way down. To ease from having to wholesale convert every function down 
when switching to struct boot_module, a methodical approach was taken. 
To ensure there isn't an mix of using both the mods[] array and 
bm->early_mod, I did a wholesale switch which allows the immediate drop 
of the mods[] array. This way, as the last usage of early_mod is 
removed, then it is known that it can be dropped from struct boot_module.

The need for early_mod is that starting with __start_xen(), a commit can 
be done for each function with the conversion to accepting struct 
boot_module. The function is changed such that any accesses to address 
and size at that level to use struct boot_module directly. Any function 
calls within the function being converted that take an instance of 
module_t, would be handed bm->early_mod. The next commit would then 
descend into the next level, doing the same conversion.

The only other approach is to have a commit for the conversion of each 
function call in __start_xen() that accepts module_t along with the 
entire call chain under that function. That gets ugly very quickly.

v/r,
dps

Re: [PATCH v4 09/44] x86/boot: split bootstrap_map_addr() out of bootstrap_map()

[Daniel P. Smith]

On 9/4/24 02:49, Jan Beulich wrote:
> On 30.08.2024 23:46, Daniel P. Smith wrote:
>> From: Andrew Cooper <andrew.cooper3@citrix.com>
>>
>> Using an interface based on addresses directly, not modules.
>>
>> No functional change.
> 
> Okay, a mechanical transformation. But what's the goal?

I would defer to Andy's reply.

>> --- a/xen/arch/x86/include/asm/setup.h
>> +++ b/xen/arch/x86/include/asm/setup.h
>> @@ -34,6 +34,7 @@ void setup_io_bitmap(struct domain *d);
>>   
>>   unsigned long initial_images_nrpages(nodeid_t node);
>>   void discard_initial_images(void);
>> +void *bootstrap_map_addr(uint64_t start, uint64_t end);
> 
> Better paddr_t?

I don't see why not. It was Andy's patch, so unless he has an objection, 
I can change it.

v/r,
dps

Re: [PATCH v4 09/44] x86/boot: split bootstrap_map_addr() out of bootstrap_map()

[Daniel P. Smith]

On 9/4/24 06:47, Andrew Cooper wrote:
> On 04/09/2024 7:49 am, Jan Beulich wrote:
>> On 30.08.2024 23:46, Daniel P. Smith wrote:
>>> From: Andrew Cooper <andrew.cooper3@citrix.com>
>>>
>>> Using an interface based on addresses directly, not modules.
>>>
>>> No functional change.
>> Okay, a mechanical transformation. But what's the goal?
> 
> Its used by patch 12 which adds boostrap_map_bm(), but does want to be
> reordered later in the series to immediately before it's used.

I can reorder the series to make these this happen just before patch 12 
and ensure that they aren't introduced until the first usage of 
boostrap_map_bm().

v/r,
dps

Re: [PATCH v4 13/44] x86/boot: update struct boot_module on module relocation

[Daniel P. Smith]

On 9/3/24 19:48, Andrew Cooper wrote:
> On 30/08/2024 10:46 pm, Daniel P. Smith wrote:
>> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
>> index 021c5699f86c..27517d24b2ea 100644
>> --- a/xen/arch/x86/setup.c
>> +++ b/xen/arch/x86/setup.c
>> @@ -1408,8 +1408,10 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
>>            * respective reserve_e820_ram() invocation below. No need to
>>            * query efi_boot_mem_unused() here, though.
>>            */
>> -        boot_info->mods[idx].early_mod->mod_start = virt_to_mfn(_stext);
>> -        boot_info->mods[idx].early_mod->mod_end = __2M_rwdata_end - _stext;
>> +        boot_info->mods[idx].start = boot_info->mods[idx].early_mod->mod_start
>> +                                   = virt_to_mfn(_stext);
>> +        boot_info->mods[idx].size = boot_info->mods[idx].early_mod->mod_end
>> +                                  = __2M_rwdata_end - _stext;
> 
> MISRA objects to using a = b = c; syntax, and we're being asked to take
> it out elsewhere.
> 
> It would be best to make local start/size variables, and the resulting
> code will have less churn through the rest of the series.

I will drop the "a = b =c;" syntax and update appropriately.

v/r,
dps