Re: [patch 23/24] x86/speculation: Enable PRCTL mode for spectre_v2_app2app
* Thomas Gleixner wrote:
> Now that all prerequisites are in place:
>
> - Add the prctl command line option
>
> - Default the 'auto' mode to 'prctl'
>
> - When SMT state changes, update the static key which controls the
>conditional STIBP evaluation on context switch.
>
> - At init update the static key which controls the conditional IBPB
>evaluation on context switch.
>
> Signed-off-by: Thomas Gleixner
> ---
> Documentation/admin-guide/kernel-parameters.txt |5 ++
> arch/x86/kernel/cpu/bugs.c | 46
> +---
> 2 files changed, 45 insertions(+), 6 deletions(-)
>
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -4246,7 +4246,10 @@
> by spectre_v2=off
> auto- Kernel selects the mitigation depending on
> the available CPU features and vulnerability.
> - Default is off.
> + Default is prctl.
> + prctl - Indirect branch speculation is enabled, but
> + mitigation can be enabled via prctl per
> thread.
> + The mitigation control state is inherited on
> fork.
Please change the order of the last two entries, i.e. make 'auto' the
last one. This is the pattern we use in other places plus we refer to
'prctl' before we document it.
> static const struct {
> @@ -270,6 +272,7 @@ static const struct {
> { "auto", SPECTRE_V2_APP2APP_CMD_AUTO,false },
> { "off",SPECTRE_V2_APP2APP_CMD_NONE,false },
> { "on", SPECTRE_V2_APP2APP_CMD_FORCE, true },
> + { "prctl", SPECTRE_V2_APP2APP_CMD_PRCTL, false },
Might make sense to order them in a consistent fashion as well.
> + /*
> + * If STIBP is not available or SMT is not possible clear the STIPB
> + * mode.
> + */
> + if (!smt_possible || !boot_cpu_has(X86_FEATURE_STIBP))
> + mode = SPECTRE_V2_APP2APP_NONE;
Another nit: please match order of the comments to how the condition is
written in the code.
> +/* Update the static key controlling the evaluation of TIF_SPEC_IB */
> +static void update_indir_branch_cond(void)
> +{
> + if (!IS_ENABLED(CONFIG_SMP))
> + return;
> +
> + if (sched_smt_active())
> + static_branch_enable(_to_cond_stibp);
> + else
> + static_branch_disable(_to_cond_stibp);
So in the !SMP case sched_smt_active() is already doing the right thing:
static inline bool sched_smt_active(void) { return false; }
I.e. couldn't we just remove the extra CONFIG_SMP condition?
This would simplify the code with some very minor expense on !SMP.
Thanks,
Ingo
Re: [patch 23/24] x86/speculation: Enable PRCTL mode for spectre_v2_app2app
* Thomas Gleixner wrote:
> Now that all prerequisites are in place:
>
> - Add the prctl command line option
>
> - Default the 'auto' mode to 'prctl'
>
> - When SMT state changes, update the static key which controls the
>conditional STIBP evaluation on context switch.
>
> - At init update the static key which controls the conditional IBPB
>evaluation on context switch.
>
> Signed-off-by: Thomas Gleixner
> ---
> Documentation/admin-guide/kernel-parameters.txt |5 ++
> arch/x86/kernel/cpu/bugs.c | 46
> +---
> 2 files changed, 45 insertions(+), 6 deletions(-)
>
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -4246,7 +4246,10 @@
> by spectre_v2=off
> auto- Kernel selects the mitigation depending on
> the available CPU features and vulnerability.
> - Default is off.
> + Default is prctl.
> + prctl - Indirect branch speculation is enabled, but
> + mitigation can be enabled via prctl per
> thread.
> + The mitigation control state is inherited on
> fork.
Please change the order of the last two entries, i.e. make 'auto' the
last one. This is the pattern we use in other places plus we refer to
'prctl' before we document it.
> static const struct {
> @@ -270,6 +272,7 @@ static const struct {
> { "auto", SPECTRE_V2_APP2APP_CMD_AUTO,false },
> { "off",SPECTRE_V2_APP2APP_CMD_NONE,false },
> { "on", SPECTRE_V2_APP2APP_CMD_FORCE, true },
> + { "prctl", SPECTRE_V2_APP2APP_CMD_PRCTL, false },
Might make sense to order them in a consistent fashion as well.
> + /*
> + * If STIBP is not available or SMT is not possible clear the STIPB
> + * mode.
> + */
> + if (!smt_possible || !boot_cpu_has(X86_FEATURE_STIBP))
> + mode = SPECTRE_V2_APP2APP_NONE;
Another nit: please match order of the comments to how the condition is
written in the code.
> +/* Update the static key controlling the evaluation of TIF_SPEC_IB */
> +static void update_indir_branch_cond(void)
> +{
> + if (!IS_ENABLED(CONFIG_SMP))
> + return;
> +
> + if (sched_smt_active())
> + static_branch_enable(_to_cond_stibp);
> + else
> + static_branch_disable(_to_cond_stibp);
So in the !SMP case sched_smt_active() is already doing the right thing:
static inline bool sched_smt_active(void) { return false; }
I.e. couldn't we just remove the extra CONFIG_SMP condition?
This would simplify the code with some very minor expense on !SMP.
Thanks,
Ingo
[patch 23/24] x86/speculation: Enable PRCTL mode for spectre_v2_app2app
Now that all prerequisites are in place:
- Add the prctl command line option
- Default the 'auto' mode to 'prctl'
- When SMT state changes, update the static key which controls the
conditional STIBP evaluation on context switch.
- At init update the static key which controls the conditional IBPB
evaluation on context switch.
Signed-off-by: Thomas Gleixner
---
Documentation/admin-guide/kernel-parameters.txt |5 ++
arch/x86/kernel/cpu/bugs.c | 46 +---
2 files changed, 45 insertions(+), 6 deletions(-)
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -4246,7 +4246,10 @@
by spectre_v2=off
auto- Kernel selects the mitigation depending on
the available CPU features and vulnerability.
- Default is off.
+ Default is prctl.
+ prctl - Indirect branch speculation is enabled, but
+ mitigation can be enabled via prctl per
thread.
+ The mitigation control state is inherited on
fork.
Not specifying this option is equivalent to
spectre_v2_app2app=auto.
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -255,11 +255,13 @@ enum spectre_v2_app2app_cmd {
SPECTRE_V2_APP2APP_CMD_NONE,
SPECTRE_V2_APP2APP_CMD_AUTO,
SPECTRE_V2_APP2APP_CMD_FORCE,
+ SPECTRE_V2_APP2APP_CMD_PRCTL,
};
static const char *spectre_v2_app2app_strings[] = {
[SPECTRE_V2_APP2APP_NONE] = "App-App Vulnerable",
[SPECTRE_V2_APP2APP_STRICT] = "App-App Mitigation: STIBP
protection",
+ [SPECTRE_V2_APP2APP_PRCTL] = "App-App Mitigation: STIBP via prctl",
};
static const struct {
@@ -270,6 +272,7 @@ static const struct {
{ "auto", SPECTRE_V2_APP2APP_CMD_AUTO,false },
{ "off",SPECTRE_V2_APP2APP_CMD_NONE,false },
{ "on", SPECTRE_V2_APP2APP_CMD_FORCE, true },
+ { "prctl", SPECTRE_V2_APP2APP_CMD_PRCTL, false },
};
static void __init spec_v2_app_print_cond(const char *reason, bool secure)
@@ -324,12 +327,15 @@ spectre_v2_app2app_select_mitigation(enu
smt_possible = false;
switch (spectre_v2_parse_app2app_cmdline(v2_cmd)) {
- case SPECTRE_V2_APP2APP_CMD_AUTO:
case SPECTRE_V2_APP2APP_CMD_NONE:
goto set_mode;
case SPECTRE_V2_APP2APP_CMD_FORCE:
mode = SPECTRE_V2_APP2APP_STRICT;
break;
+ case SPECTRE_V2_APP2APP_CMD_AUTO:
+ case SPECTRE_V2_APP2APP_CMD_PRCTL:
+ mode = SPECTRE_V2_APP2APP_PRCTL;
+ break;
}
/* Initialize Indirect Branch Prediction Barrier */
@@ -340,6 +346,9 @@ spectre_v2_app2app_select_mitigation(enu
case SPECTRE_V2_APP2APP_STRICT:
static_branch_enable(_to_always_ibpb);
break;
+ case SPECTRE_V2_APP2APP_PRCTL:
+ static_branch_enable(_to_cond_ibpb);
+ break;
default:
break;
}
@@ -352,6 +361,12 @@ spectre_v2_app2app_select_mitigation(enu
if (spectre_v2_enabled == SPECTRE_V2_IBRS_ENHANCED)
return;
+ /*
+* If STIBP is not available or SMT is not possible clear the STIPB
+* mode.
+*/
+ if (!smt_possible || !boot_cpu_has(X86_FEATURE_STIBP))
+ mode = SPECTRE_V2_APP2APP_NONE;
set_mode:
spectre_v2_app2app = mode;
/* Only print the STIBP mode when SMT possible */
@@ -552,6 +567,18 @@ static void update_stibp_strict(void)
on_each_cpu(update_stibp_msr, NULL, 1);
}
+/* Update the static key controlling the evaluation of TIF_SPEC_IB */
+static void update_indir_branch_cond(void)
+{
+ if (!IS_ENABLED(CONFIG_SMP))
+ return;
+
+ if (sched_smt_active())
+ static_branch_enable(_to_cond_stibp);
+ else
+ static_branch_disable(_to_cond_stibp);
+}
+
void arch_smt_update(void)
{
/* Enhanced IBRS makes using STIBP unnecessary. No update required. */
@@ -567,6 +594,7 @@ void arch_smt_update(void)
update_stibp_strict();
break;
case SPECTRE_V2_APP2APP_PRCTL:
+ update_indir_branch_cond();
break;
}
@@ -1044,17 +1072,25 @@ static char *stibp_state(void)
case SPECTRE_V2_APP2APP_STRICT:
return ", STIBP: forced";
case SPECTRE_V2_APP2APP_PRCTL:
- return "";
+ return ", STIBP: opt-in";
}
return "";
}
static char *ibpb_state(void)
{
- if
[patch 23/24] x86/speculation: Enable PRCTL mode for spectre_v2_app2app
Now that all prerequisites are in place:
- Add the prctl command line option
- Default the 'auto' mode to 'prctl'
- When SMT state changes, update the static key which controls the
conditional STIBP evaluation on context switch.
- At init update the static key which controls the conditional IBPB
evaluation on context switch.
Signed-off-by: Thomas Gleixner
---
Documentation/admin-guide/kernel-parameters.txt |5 ++
arch/x86/kernel/cpu/bugs.c | 46 +---
2 files changed, 45 insertions(+), 6 deletions(-)
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -4246,7 +4246,10 @@
by spectre_v2=off
auto- Kernel selects the mitigation depending on
the available CPU features and vulnerability.
- Default is off.
+ Default is prctl.
+ prctl - Indirect branch speculation is enabled, but
+ mitigation can be enabled via prctl per
thread.
+ The mitigation control state is inherited on
fork.
Not specifying this option is equivalent to
spectre_v2_app2app=auto.
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -255,11 +255,13 @@ enum spectre_v2_app2app_cmd {
SPECTRE_V2_APP2APP_CMD_NONE,
SPECTRE_V2_APP2APP_CMD_AUTO,
SPECTRE_V2_APP2APP_CMD_FORCE,
+ SPECTRE_V2_APP2APP_CMD_PRCTL,
};
static const char *spectre_v2_app2app_strings[] = {
[SPECTRE_V2_APP2APP_NONE] = "App-App Vulnerable",
[SPECTRE_V2_APP2APP_STRICT] = "App-App Mitigation: STIBP
protection",
+ [SPECTRE_V2_APP2APP_PRCTL] = "App-App Mitigation: STIBP via prctl",
};
static const struct {
@@ -270,6 +272,7 @@ static const struct {
{ "auto", SPECTRE_V2_APP2APP_CMD_AUTO,false },
{ "off",SPECTRE_V2_APP2APP_CMD_NONE,false },
{ "on", SPECTRE_V2_APP2APP_CMD_FORCE, true },
+ { "prctl", SPECTRE_V2_APP2APP_CMD_PRCTL, false },
};
static void __init spec_v2_app_print_cond(const char *reason, bool secure)
@@ -324,12 +327,15 @@ spectre_v2_app2app_select_mitigation(enu
smt_possible = false;
switch (spectre_v2_parse_app2app_cmdline(v2_cmd)) {
- case SPECTRE_V2_APP2APP_CMD_AUTO:
case SPECTRE_V2_APP2APP_CMD_NONE:
goto set_mode;
case SPECTRE_V2_APP2APP_CMD_FORCE:
mode = SPECTRE_V2_APP2APP_STRICT;
break;
+ case SPECTRE_V2_APP2APP_CMD_AUTO:
+ case SPECTRE_V2_APP2APP_CMD_PRCTL:
+ mode = SPECTRE_V2_APP2APP_PRCTL;
+ break;
}
/* Initialize Indirect Branch Prediction Barrier */
@@ -340,6 +346,9 @@ spectre_v2_app2app_select_mitigation(enu
case SPECTRE_V2_APP2APP_STRICT:
static_branch_enable(_to_always_ibpb);
break;
+ case SPECTRE_V2_APP2APP_PRCTL:
+ static_branch_enable(_to_cond_ibpb);
+ break;
default:
break;
}
@@ -352,6 +361,12 @@ spectre_v2_app2app_select_mitigation(enu
if (spectre_v2_enabled == SPECTRE_V2_IBRS_ENHANCED)
return;
+ /*
+* If STIBP is not available or SMT is not possible clear the STIPB
+* mode.
+*/
+ if (!smt_possible || !boot_cpu_has(X86_FEATURE_STIBP))
+ mode = SPECTRE_V2_APP2APP_NONE;
set_mode:
spectre_v2_app2app = mode;
/* Only print the STIBP mode when SMT possible */
@@ -552,6 +567,18 @@ static void update_stibp_strict(void)
on_each_cpu(update_stibp_msr, NULL, 1);
}
+/* Update the static key controlling the evaluation of TIF_SPEC_IB */
+static void update_indir_branch_cond(void)
+{
+ if (!IS_ENABLED(CONFIG_SMP))
+ return;
+
+ if (sched_smt_active())
+ static_branch_enable(_to_cond_stibp);
+ else
+ static_branch_disable(_to_cond_stibp);
+}
+
void arch_smt_update(void)
{
/* Enhanced IBRS makes using STIBP unnecessary. No update required. */
@@ -567,6 +594,7 @@ void arch_smt_update(void)
update_stibp_strict();
break;
case SPECTRE_V2_APP2APP_PRCTL:
+ update_indir_branch_cond();
break;
}
@@ -1044,17 +1072,25 @@ static char *stibp_state(void)
case SPECTRE_V2_APP2APP_STRICT:
return ", STIBP: forced";
case SPECTRE_V2_APP2APP_PRCTL:
- return "";
+ return ", STIBP: opt-in";
}
return "";
}
static char *ibpb_state(void)
{
- if
