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