diff mbox series

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

Message ID 20240830214730.1621-6-dpsmith@apertussolutions.com (mailing list archive)
State New
Headers show
Series Boot modules for Hyperlaunch | expand

Commit Message

Daniel P. Smith Aug. 30, 2024, 9:46 p.m. UTC
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(-)

Comments

Andrew Cooper Sept. 3, 2024, 11:29 p.m. UTC | #1
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
Jan Beulich Sept. 4, 2024, 6:33 a.m. UTC | #2
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
diff mbox series

Patch

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. */