Re: [PATCH] x86/boot: Load microcode much earlier on boot

[Jan Beulich <jbeulich@xxxxxxxx>]

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 <jbeulich@xxxxxxxx>

I'm surprised though that you didn't comment at all on the other aspect
I raised.

Jan