On 30/08/2024 10:46 pm, Daniel P. Smith wrote: > From: Christopher Clark <[email protected]> > > 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 <[email protected]> > + * Copyright (c) 2024 Apertus Solutions, LLC > + * Author: Daniel P. Smith <[email protected]> > + */ > + > +#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
