On 21/10/2024 7:00 pm, Andrew Cooper wrote: > On 21/10/2024 1:45 am, Daniel P. Smith wrote: >> To allow a slow conversion of x86 over to struct boot_module, start with >> replacing all references to module_t mod, only in setup.c, to the mod element >> of struct boot_module. These serves twofold, first to allow the incremental >> transition from module_t fields to struct boot_module fields. The second is >> to >> allow the conversion of function definitions from taking module_t parameters >> to >> accepting struct boot_module as needed when a transitioned field will be >> accessed. >> >> Signed-off-by: Daniel P. Smith <dpsm...@apertussolutions.com> >> --- >> Changes since v6: >> - code style >> - switched to a local ref >> >> Changes since v5: >> - rewrote commit message >> - coding style changes >> - added comment for initial_images assignment >> --- >> xen/arch/x86/setup.c | 62 +++++++++++++++++++++++++------------------- >> 1 file changed, 35 insertions(+), 27 deletions(-) >> >> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c >> index 48809aa94451..b6d688f8fe5e 100644 >> --- a/xen/arch/x86/setup.c >> +++ b/xen/arch/x86/setup.c >> @@ -1364,15 +1364,19 @@ 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; >> + /* >> + * The field bi->mods[0].mod points to the first element of the module_t >> + * array. >> + */ >> + initial_images = bi->mods[0].mod; > This looks actively-dodgy. It might be correct, but its also not necessary. > > bi->mods[] is populated and both initial_images_nrpages() and > discard_initial_images() have a local bi-> pointer which they already > consume nr_module from, so you really can drop initial_images here in > the series. > > i.e. you want to pull patch 28 forward to ahead of of this one, and it > will reduce the churn through the series. > > But mostly, it removes a transient-WTF construct from the series, making > it easier to review.
In fact, having done this locally, both patches 27 and 28 disappear. Patches 29 and 30 are the ones which swap these two functions away from using the mod-> pointer, and they're basically the same. So yes, far less churn. ~Andrew
