On 20.11.2024 12:24, Andrew Cooper wrote:
> On 20/11/2024 10:44 am, Jan Beulich wrote:
>> On 19.11.2024 22:57, Andrew Cooper wrote:
>>> Following commit cd7cc5320bb2 ("x86/boot: add start and size fields to
>>> struct
>>> boot_module"), bootstrap_map*() works as soon as boot_info is populated.
>> I'm struggling with following where this connection is coming from.
>
> Specifically, the final hunk:
>
>> @@ -1416,12 +1420,6 @@ void asmlinkage __init noreturn
>> __start_xen(void) if ( bi->mods[i].start & (PAGE_SIZE - 1) )
>> panic("Bootloader didn't honor module alignment request\n"); - /* - *
>> TODO: load ucode earlier once multiboot modules become accessible - *
>> at an earlier stage. - */ - early_microcode_init(bi); - if (
>> xen_phys_start ) { struct boot_module *xen = &bi->mods[bi->nr_modules];
>
> The context with panic() used to read:
>
> panic("Bootloader didn't honor module alignment request\n");
> bi->mods[i].mod->mod_end -= bi->mods[i].mod->mod_start;
> bi->mods[i].mod->mod_start >>= PAGE_SHIFT;
>
> and calling bootstrap_map() prior to that mapped junk because the start
> is wrong by PAGE_SHIFT.
>
> This is why the TODO was inserted the last time around when we couldn't
> move loading as early as wanted.
Hmm, I see. It wasn't really that they were inaccessible, it merely was
that adjustments of internally maintained data would have been needed.
Which could have been as easy as instantiating a local mod variable,
fill it with suitably adjusted data, and pass an address thereof to
bootstrap_map().
In any event:
Reviewed-by: Jan Beulich <[email protected]>
I'm surprised though that you didn't comment at all on the other aspect
I raised.
Jan
