Messages in this thread | | | Date | Thu, 22 Nov 2018 08:34:47 +0100 | From | Ingo Molnar <> | Subject | Re: [patch 21/24] x86/speculation: Prepare arch_smt_update() for PRCTL mode |
| |
* Thomas Gleixner <tglx@linutronix.de> wrote:
> The upcoming fine grained per task STIBP control needs to be updated on CPU > hotplug as well. > > Split out the code which controls the strict mode so the prctl control code > can be added later. > > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> > --- > arch/x86/kernel/cpu/bugs.c | 46 ++++++++++++++++++++++++--------------------- > 1 file changed, 25 insertions(+), 21 deletions(-) > > --- a/arch/x86/kernel/cpu/bugs.c > +++ b/arch/x86/kernel/cpu/bugs.c > @@ -531,40 +531,44 @@ static void __init spectre_v2_select_mit > arch_smt_update(); > } > > -static bool stibp_needed(void) > +static void update_stibp_msr(void *info) > { > - /* Enhanced IBRS makes using STIBP unnecessary. */ > - if (spectre_v2_enabled == SPECTRE_V2_IBRS_ENHANCED) > - return false; > - > - /* Check for strict app2app mitigation mode */ > - return spectre_v2_app2app == SPECTRE_V2_APP2APP_STRICT; > + wrmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base); > }
Does Sparse or other tooling warn about unused function parameters? If yes then it might make sense to mark it __used?
> > -static void update_stibp_msr(void *info) > +/* Update x86_spec_ctrl_base in case SMT state changed. */ > +static void update_stibp_strict(void) > { > - wrmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base); > + u64 mask = x86_spec_ctrl_base & ~SPEC_CTRL_STIBP; > + > + if (sched_smt_active()) > + mask |= SPEC_CTRL_STIBP; > + > + if (mask == x86_spec_ctrl_base) > + return; > + > + pr_info("Spectre v2 cross-process SMT mitigation: %s STIBP\n", > + mask & SPEC_CTRL_STIBP ? "Enabling" : "Disabling"); > + x86_spec_ctrl_base = mask; > + on_each_cpu(update_stibp_msr, NULL, 1); > } > > void arch_smt_update(void) > { > - u64 mask; > - > - if (!stibp_needed()) > + /* Enhanced IBRS makes using STIBP unnecessary. No update required. */ > + if (spectre_v2_enabled == SPECTRE_V2_IBRS_ENHANCED) > return; > > mutex_lock(&spec_ctrl_mutex); > > - mask = x86_spec_ctrl_base & ~SPEC_CTRL_STIBP; > - if (sched_smt_active()) > - mask |= SPEC_CTRL_STIBP; > - > - if (mask != x86_spec_ctrl_base) { > - pr_info("Spectre v2 cross-process SMT mitigation: %s STIBP\n", > - mask & SPEC_CTRL_STIBP ? "Enabling" : "Disabling"); > - x86_spec_ctrl_base = mask; > - on_each_cpu(update_stibp_msr, NULL, 1); > + switch (spectre_v2_app2app) { > + case SPECTRE_V2_APP2APP_NONE: > + break; > + case SPECTRE_V2_APP2APP_STRICT: > + update_stibp_strict(); > + break; > }
So I'm wondering, shouldn't firmware_restrict_branch_speculation_start()/_end() also enable/disable STIBP? It already enabled/disables IBRS.
Thanks,
Ingo
| |