Messages in this thread Patch in this message | | | Date | Fri, 22 Sep 2023 15:42:00 +0200 | From | Borislav Petkov <> | Subject | Re: [patch V3 18/30] x86/microcode: Handle "nosmt" correctly |
| |
Just textual nitpicks. Otherwise looks nice.
On Tue, Sep 12, 2023 at 09:58:12AM +0200, Thomas Gleixner wrote: > From: Thomas Gleixner <tglx@linutronix.de> > > On CPUs where microcode loading is not NMI safe the SMT sibling which is > parked in one of the play_dead() variants, these parked CPUs still react > on NMIs.
s/, these parked CPUs still react/still reacts/.
Simpler.
> So if a NMI hits while the primary thread updates the microcode > the resulting behaviour is undefined. The default play_dead() > implementation on modern CPUs is using MWAIT, which is not guaranteed to > be safe against an microcode update which affects MWAIT.
s/an //
> +/* > + * Ensure that all required CPUs which are present and have been booted > + * once are online. > + * > + * To pass this check, all primary threads must be online. > + * > + * If the microcode load is not safe against NMI then all SMT threads > + * must be online as well because they still react on NMI when they are
s/react on NMI/react to NMIs/
> + * soft-offlined and parked in one of the play_dead() variants. So if a > + * NMI hits while the primary thread updates the microcode the resulting > + * behaviour is undefined. The default play_dead() implementation on > + * modern CPUs is using MWAIT, which is also not guaranteed to be safe
s/is using/uses/
> --- a/arch/x86/kernel/cpu/microcode/internal.h > +++ b/arch/x86/kernel/cpu/microcode/internal.h > @@ -20,18 +20,17 @@ enum ucode_state { > > struct microcode_ops { > enum ucode_state (*request_microcode_fw)(int cpu, struct device *dev); > - > void (*microcode_fini_cpu)(int cpu); > > /* > - * The generic 'microcode_core' part guarantees that > - * the callbacks below run on a target cpu when they > - * are being called. > + * The generic 'microcode_core' part guarantees that the callbacks > + * below run on a target cpu when they are being called.
s/cpu/CPU/
while at it.
> * See also the "Synchronization" section in microcode_core.c. > */ > - enum ucode_state (*apply_microcode)(int cpu); > - int (*collect_cpu_info)(int cpu, struct cpu_signature *csig); > - void (*finalize_late_load)(int result); > + enum ucode_state (*apply_microcode)(int cpu); > + int (*collect_cpu_info)(int cpu, struct cpu_signature *csig); > + void (*finalize_late_load)(int result); > + unsigned int nmi_safe : 1; > };
-- Regards/Gruss, Boris.
https://people.kernel.org/tglx/notes-about-netiquette
| |