> From: Borislav Petkov [mailto:[email protected]] > Sent: Wednesday, July 24, 2013 6:33 AM > On Tue, Jul 23, 2013 at 11:00:26PM +0200, Torsten Kaiser wrote: > > Extract common checks and initialisations from load_ucode_ap() and > > save_microcode_in_initrd_amd() to load_microcode_amd_early(). > > load_ucode_ap() gets a quick exit for !cpu, because for the BSP there > is > > already a different function dealing with its update. > > > > The original code already didn't anything, because without > load_microcode_amd() > > getting called apply_microcode_amd() could not do anything. > > > > Signed-off-by: Torsten Kaiser <[email protected]> > > > > --- a/arch/x86/kernel/microcode_amd_early.c 2013-07-22 > 06:22:32.000000000 +0200 > > +++ b/arch/x86/kernel/microcode_amd_early.c 2013-07-23 > 20:00:04.889508712 +0200 > > @@ -196,6 +196,23 @@ void __init load_ucode_amd_bsp(void) > > apply_ucode_in_initrd(cd.data, cd.size); > > } > > > > +static int load_microcode_amd_early(void) > > +{ > > + enum ucode_state ret; > > + void *ucode; > > + > > + if (ucode_loaded || !ucode_size || !initrd_start) > > + return 0; > > + > > + ucode = (void *)(initrd_start + ucode_offset); > > + ret = load_microcode_amd(0, ucode, ucode_size); > > + if (ret != UCODE_OK) > > + return -EINVAL; > > + > > + ucode_loaded = true; > > + return 0; > > +} > > + > > #ifdef CONFIG_X86_32 > > u8 amd_bsp_mpb[MPB_MAX_SIZE]; > > > > @@ -258,17 +275,13 @@ void load_ucode_amd_ap(void) > > > > collect_cpu_info_amd_early(&cpu_data(cpu), ucode_cpu_info + cpu); > > > > - if (cpu && !ucode_loaded) { > > - void *ucode; > > - > > - if (!ucode_size || !initrd_start) > > - return; > > + /* BSP via load_ucode_amd_bsp() */ > > + if (!cpu) > > + return; > > Ok, this is really misleading. Fenghua, what's the reason for calling > load_ucode_ap() on the BSP too? > > We have on the one hand: > > x86_64_start_kernel > |->load_ucode_bsp > > and on the other: > > x86_64_start_kernel > |-> x86_64_start_reservations > |-> start_kernel > |-> trap_init > |-> cpu_init > |-> load_ucode_ap() > > so we attempt to load the ucode twice on the BSP. >
You are right. Though the second time loading attempt will not find a valid/newer ucode patch to update. So this won't cause a real/serious problem. But we had better to fix this anyway. > IMO, we should do this in cpu_init: > > if (cpu) > load_ucode_ap(); > > no? This check won't work when CPU0 is hot added. So we need to find a better way to fix this. Thanks. -Fenghua
