lkml.org 
[lkml]   [2018]   [Nov]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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

\
 
 \ /
  Last update: 2018-11-22 08:35    [W:0.827 / U:0.076 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site