x86/speculation: Remedy the STIBP/IBPB overhead
Subsystems (7)
v2 cover x86/speculation: Remedy the STIBP/IBPB overhead
Thats hopefully the final version of this. Changes since V1:
- Renamed the command line option and related code to spectre_v2_user= as suggested by Josh.
- Thought more about the back to back optimization and finally left the IBPB code in switch_mm().
It still removes the ptrace check for the always IBPB case. That's substantial overhead for dubious value now that the default is conditional (prctl/seccomp) IBPB.
- Added two options which allow conditional STIBP and IBPB always mode.
- Addressed the review comments
Documentation is still work in progress. Thanks Andi for providing the first draft for it.
Still based on tip.git x86/pti as it has been discussed to remove the minimal RETPOLINE bandaid from stable kernels as well.
It's avaiable from git:
git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git WIP.x86/pti
Thanks,
tglx
8<------------------------
Documentation/admin-guide/kernel-parameters.txt | 56 ++ Documentation/userspace-api/spec_ctrl.rst | 9 arch/x86/Kconfig | 8 arch/x86/include/asm/msr-index.h | 5 arch/x86/include/asm/nospec-branch.h | 14 arch/x86/include/asm/spec-ctrl.h | 18 arch/x86/include/asm/switch_to.h | 3 arch/x86/include/asm/thread_info.h | 18 arch/x86/include/asm/tlbflush.h | 8 arch/x86/kernel/cpu/bugs.c | 520 ++++++++++++++++++------ arch/x86/kernel/process.c | 79 ++- arch/x86/kernel/process.h | 39 + arch/x86/kernel/process_32.c | 10 arch/x86/kernel/process_64.c | 10 arch/x86/mm/tlb.c | 109 +++-- include/linux/ptrace.h | 17 include/linux/sched.h | 9 include/linux/sched/smt.h | 20 include/uapi/linux/prctl.h | 1 kernel/cpu.c | 15 kernel/ptrace.c | 10 kernel/sched/core.c | 19 kernel/sched/sched.h | 4 tools/include/uapi/linux/prctl.h | 1 24 files changed, 745 insertions(+), 257 deletions(-)
PATCH v2 1/28 x86/speculation: Update the TIF_SSBD comment
"Reduced Data Speculation" is an obsolete term. The correct new name is "Speculative store bypass disable" - which is abbreviated into SSBD.
--- arch/x86/include/asm/thread_info.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/x86/include/asm/thread_info.h +1 -1
@@ -79,7 +79,7 @@ struct thread_info {#define TIF_SIGPENDING 2/* signal pending */ #define TIF_NEED_RESCHED 3/* rescheduling necessary */ #define TIF_SINGLESTEP 4/* reenable singlestep on user return*/ -#define TIF_SSBD 5/* Reduced data speculation */ +#define TIF_SSBD 5/* Speculative store bypass disable */ #define TIF_SYSCALL_EMU 6/* syscall emulation active */ #define TIF_SYSCALL_AUDIT 7/* syscall auditing active */ #define TIF_SECCOMP 8/* secure computing */
tip-bot for Tim Chen
merge bot[tip:x86/pti] x86/speculation: Update the TIF_SSBD comment
x86/speculation: Update the TIF_SSBD comment
Commit-ID: 8eb729b77faf83ac4c1f363a9ad68d042415f24c Gitweb: https://git.kernel.org/tip/8eb729b77faf83ac4c1f363a9ad68d042415f24c Author: Tim Chen <[email protected]> AuthorDate: Sun, 25 Nov 2018 19:33:29 +0100 Committer: Thomas Gleixner <[email protected]> CommitDate: Wed, 28 Nov 2018 11:57:04 +0100
x86/speculation: Update the TIF_SSBD comment
"Reduced Data Speculation" is an obsolete term. The correct new name is "Speculative store bypass disable" - which is abbreviated into SSBD.
--- arch/x86/include/asm/thread_info.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/x86/include/asm/thread_info.h +1 -1
@@ -79,7 +79,7 @@ struct thread_info {#define TIF_SIGPENDING 2/* signal pending */ #define TIF_NEED_RESCHED 3/* rescheduling necessary */ #define TIF_SINGLESTEP 4/* reenable singlestep on user return*/ -#define TIF_SSBD 5/* Reduced data speculation */ +#define TIF_SSBD 5/* Speculative store bypass disable */ #define TIF_SYSCALL_EMU 6/* syscall emulation active */ #define TIF_SYSCALL_AUDIT 7/* syscall auditing active */ #define TIF_SECCOMP 8/* secure computing */
Re: [patch V2 01/28] x86/speculation: Update the TIF_SSBD comment
In reply to Thomas GleixnerQuoted 5 lines · Thomas Gleixner
"Reduced Data Speculation" is an obsolete term. The correct new name is "Speculative store bypass disable" - which is abbreviated into SSBD.
Signed-off-by Tim Chen <[email protected]>Signed-off-by Thomas Gleixner <[email protected]>
Thank you!
Quoted 18 lines
--- arch/x86/include/asm/thread_info.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)arch/x86/include/asm/thread_info.h +1
-1@@ -79,7 +79,7 @@ struct thread_info {#define TIF_SIGPENDING 2/* signal pending */ #define TIF_NEED_RESCHED 3/* rescheduling necessary */ #define TIF_SINGLESTEP 4/* reenable singlestep on user return*/ -#define TIF_SSBD 5/* Reduced data speculation */ +#define TIF_SSBD 5/* Speculative store bypass disable */ #define TIF_SYSCALL_EMU 6/* syscall emulation active */ #define TIF_SYSCALL_AUDIT 7/* syscall auditing active */ #define TIF_SECCOMP 8/* secure computing */
PATCH v2 2/28 x86/speculation: Clean up spectre_v2_parse_cmdline()
In reply to Thomas GleixnerRemove the unnecessary 'else' statement in spectre_v2_parse_cmdline() to save an indentation level.
--- arch/x86/kernel/cpu/bugs.c | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-)
arch/x86/kernel/cpu/bugs.c +13 -14
@@ -276,22 +276,21 @@ static enum spectre_v2_mitigation_cmd __if (cmdline_find_option_bool (boot_command_line,"nospectre_v2" ))return SPECTRE_V2_CMD_NONE ; -else { - ret =cmdline_find_option (boot_command_line,"spectre_v2" , arg,sizeof (arg)); -if (ret <0 ) -return SPECTRE_V2_CMD_AUTO ; -for (i =0 ; i <ARRAY_SIZE (mitigation_options); i++) { -if (!match_option (arg, ret, mitigation_options[i].option)) -continue ; - cmd = mitigation_options[i].cmd; -break ; - } + ret =cmdline_find_option (boot_command_line,"spectre_v2" , arg,sizeof (arg)); +if (ret <0 ) +return SPECTRE_V2_CMD_AUTO ; -if (i >=ARRAY_SIZE (mitigation_options)) { -pr_err ("unknown option (%s). Switching to AUTO select\n" , arg); -return SPECTRE_V2_CMD_AUTO ; - } +for (i =0 ; i <ARRAY_SIZE (mitigation_options); i++) { +if (!match_option (arg, ret, mitigation_options[i].option)) +continue ; + cmd = mitigation_options[i].cmd; +break ; + } + +if (i >=ARRAY_SIZE (mitigation_options)) { +pr_err ("unknown option (%s). Switching to AUTO select\n" , arg); +return SPECTRE_V2_CMD_AUTO ; } if ((cmd == SPECTRE_V2_CMD_RETPOLINE ||
tip-bot for Tim Chen
merge bot[tip:x86/pti] x86/speculation: Clean up spectre_v2_parse_cmdline()
x86/speculation: Clean up spectre_v2_parse_cmdline()
Commit-ID: 24848509aa55eac39d524b587b051f4e86df3c12 Gitweb: https://git.kernel.org/tip/24848509aa55eac39d524b587b051f4e86df3c12 Author: Tim Chen <[email protected]> AuthorDate: Sun, 25 Nov 2018 19:33:30 +0100 Committer: Thomas Gleixner <[email protected]> CommitDate: Wed, 28 Nov 2018 11:57:04 +0100
x86/speculation: Clean up spectre_v2_parse_cmdline()
Remove the unnecessary 'else' statement in spectre_v2_parse_cmdline() to save an indentation level.
--- arch/x86/kernel/cpu/bugs.c | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-)
arch/x86/kernel/cpu/bugs.c +13 -14
@@ -276,22 +276,21 @@ static enum spectre_v2_mitigation_cmd __init spectre_v2_parse_cmdline(void)if (cmdline_find_option_bool (boot_command_line,"nospectre_v2" ))return SPECTRE_V2_CMD_NONE ; -else { - ret =cmdline_find_option (boot_command_line,"spectre_v2" , arg,sizeof (arg)); -if (ret <0 ) -return SPECTRE_V2_CMD_AUTO ; -for (i =0 ; i <ARRAY_SIZE (mitigation_options); i++) { -if (!match_option (arg, ret, mitigation_options[i].option)) -continue ; - cmd = mitigation_options[i].cmd; -break ; - } + ret =cmdline_find_option (boot_command_line,"spectre_v2" , arg,sizeof (arg)); +if (ret <0 ) +return SPECTRE_V2_CMD_AUTO ; -if (i >=ARRAY_SIZE (mitigation_options)) { -pr_err ("unknown option (%s). Switching to AUTO select\n" , arg); -return SPECTRE_V2_CMD_AUTO ; - } +for (i =0 ; i <ARRAY_SIZE (mitigation_options); i++) { +if (!match_option (arg, ret, mitigation_options[i].option)) +continue ; + cmd = mitigation_options[i].cmd; +break ; + } + +if (i >=ARRAY_SIZE (mitigation_options)) { +pr_err ("unknown option (%s). Switching to AUTO select\n" , arg); +return SPECTRE_V2_CMD_AUTO ; } if ((cmd == SPECTRE_V2_CMD_RETPOLINE ||
Re: [patch V2 02/28] x86/speculation: Clean up spectre_v2_parse_cmdline()
In reply to Thomas GleixnerQuoted 5 lines · Thomas Gleixner
Remove the unnecessary 'else' statement in
spectre_v2_parse_cmdline()to save an indentation level.Signed-off-by Tim Chen <[email protected]>Signed-off-by Thomas Gleixner <[email protected]>
Thank you!
Quoted 45 lines
--- arch/x86/kernel/cpu/bugs.c | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-)arch/x86/kernel/cpu/bugs.c +13
-14@@ -276,22 +276,21 @@ static enum spectre_v2_mitigation_cmd __if (cmdline_find_option_bool (boot_command_line,"nospectre_v2" ))return SPECTRE_V2_CMD_NONE ; -else { - ret =cmdline_find_option (boot_command_line,"spectre_v2" , arg,sizeof (arg)); -if (ret <0 ) -return SPECTRE_V2_CMD_AUTO ; -for (i =0 ; i <ARRAY_SIZE (mitigation_options); i++) { -if (!match_option (arg, ret, mitigation_options[i].option)) -continue ; - cmd = mitigation_options[i].cmd; -break ; - } + ret =cmdline_find_option (boot_command_line,"spectre_v2" , arg,sizeof (arg)); +if (ret <0 ) +return SPECTRE_V2_CMD_AUTO ; -if (i >=ARRAY_SIZE (mitigation_options)) { -pr_err ("unknown option (%s). Switching to AUTO select\n" , arg); -return SPECTRE_V2_CMD_AUTO ; - } +for (i =0 ; i <ARRAY_SIZE (mitigation_options); i++) { +if (!match_option (arg, ret, mitigation_options[i].option)) +continue ; + cmd = mitigation_options[i].cmd; +break ; + } + +if (i >=ARRAY_SIZE (mitigation_options)) { +pr_err ("unknown option (%s). Switching to AUTO select\n" , arg); +return SPECTRE_V2_CMD_AUTO ; } if ((cmd == SPECTRE_V2_CMD_RETPOLINE ||
PATCH v2 3/28 x86/speculation: Remove unnecessary ret variable in cpu_show_common()
In reply to Thomas Gleixner--- arch/x86/kernel/cpu/bugs.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)
arch/x86/kernel/cpu/bugs.c +1 -4
@@ -847,8 +847,6 @@ static ssize_t l1tf_show_state(char *bufstatic ssize_t cpu_show_common (struct device *dev,struct device_attribute *attr,char *buf,unsigned int bug) { -int ret; -if (!boot_cpu_has_bug (bug))return sprintf (buf,"Not affected\n" );
@@ -866,13 +864,12 @@ static ssize_t cpu_show_common(struct dereturn sprintf (buf,"Mitigation: __user pointer sanitization\n" );case X86_BUG_SPECTRE_V2 : - ret =sprintf (buf,"%s%s%s%s%s%s\n" , spectre_v2_strings[spectre_v2_enabled], +return sprintf (buf,"%s%s%s%s%s%s\n" , spectre_v2_strings[spectre_v2_enabled],boot_cpu_has (X86_FEATURE_USE_IBPB ) ?", IBPB" :"" ,boot_cpu_has (X86_FEATURE_USE_IBRS_FW ) ?", IBRS_FW" :"" , (x86_spec_ctrl_base &SPEC_CTRL_STIBP ) ?", STIBP" :"" ,boot_cpu_has (X86_FEATURE_RSB_CTXSW ) ?", RSB filling" :"" ,spectre_v2_module_string ()); -return ret;case X86_BUG_SPEC_STORE_BYPASS :return sprintf (buf,"%s\n" , ssb_strings[ssb_mode]);
tip-bot for Tim Chen
merge bot[tip:x86/pti] x86/speculation: Remove unnecessary ret variable in cpu_show_common()
x86/speculation: Remove unnecessary ret variable in cpu_show_common()
Commit-ID: b86bda0426853bfe8a3506c7d2a5b332760ae46b Gitweb: https://git.kernel.org/tip/b86bda0426853bfe8a3506c7d2a5b332760ae46b Author: Tim Chen <[email protected]> AuthorDate: Sun, 25 Nov 2018 19:33:31 +0100 Committer: Thomas Gleixner <[email protected]> CommitDate: Wed, 28 Nov 2018 11:57:05 +0100
x86/speculation: Remove unnecessary ret variable in cpu_show_common()
--- arch/x86/kernel/cpu/bugs.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)
arch/x86/kernel/cpu/bugs.c +1 -4
@@ -847,8 +847,6 @@ static ssize_t l1tf_show_state(char *buf)static ssize_t cpu_show_common (struct device *dev,struct device_attribute *attr,char *buf,unsigned int bug) { -int ret; -if (!boot_cpu_has_bug (bug))return sprintf (buf,"Not affected\n" );
@@ -866,13 +864,12 @@ static ssize_t cpu_show_common(struct device *dev, struct device_attribute *attrreturn sprintf (buf,"Mitigation: __user pointer sanitization\n" );case X86_BUG_SPECTRE_V2 : - ret =sprintf (buf,"%s%s%s%s%s%s\n" , spectre_v2_strings[spectre_v2_enabled], +return sprintf (buf,"%s%s%s%s%s%s\n" , spectre_v2_strings[spectre_v2_enabled],boot_cpu_has (X86_FEATURE_USE_IBPB ) ?", IBPB" :"" ,boot_cpu_has (X86_FEATURE_USE_IBRS_FW ) ?", IBRS_FW" :"" , (x86_spec_ctrl_base &SPEC_CTRL_STIBP ) ?", STIBP" :"" ,boot_cpu_has (X86_FEATURE_RSB_CTXSW ) ?", RSB filling" :"" ,spectre_v2_module_string ()); -return ret;case X86_BUG_SPEC_STORE_BYPASS :return sprintf (buf,"%s\n" , ssb_strings[ssb_mode]);
Re: [patch V2 03/28] x86/speculation: Remove unnecessary ret variable in cpu_show_common()
In reply to Thomas GleixnerQuoted 2 lines · Thomas Gleixner
Signed-off-by Tim Chen <[email protected]>Signed-off-by Thomas Gleixner <[email protected]>
Thank you!
Quoted 33 lines
--- arch/x86/kernel/cpu/bugs.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)arch/x86/kernel/cpu/bugs.c +1
-4@@ -847,8 +847,6 @@ static ssize_t l1tf_show_state(char *bufstatic ssize_t cpu_show_common (struct device *dev,struct device_attribute *attr,char *buf,unsigned int bug) { -int ret; -if (!boot_cpu_has_bug (bug))return sprintf (buf,"Not affected\n" );@@ -866,13 +864,12 @@ static ssize_t cpu_show_common(struct dereturn sprintf (buf,"Mitigation: __user pointer sanitization\n" );case X86_BUG_SPECTRE_V2 : - ret =sprintf (buf,"%s%s%s%s%s%s\n" , spectre_v2_strings[spectre_v2_enabled], +return sprintf (buf,"%s%s%s%s%s%s\n" , spectre_v2_strings[spectre_v2_enabled],boot_cpu_has (X86_FEATURE_USE_IBPB ) ?", IBPB" :"" ,boot_cpu_has (X86_FEATURE_USE_IBRS_FW ) ?", IBRS_FW" :"" , (x86_spec_ctrl_base &SPEC_CTRL_STIBP ) ?", STIBP" :"" ,boot_cpu_has (X86_FEATURE_RSB_CTXSW ) ?", RSB filling" :"" ,spectre_v2_module_string ()); -return ret;case X86_BUG_SPEC_STORE_BYPASS :return sprintf (buf,"%s\n" , ssb_strings[ssb_mode]);
PATCH v2 4/28 x86/speculation: Reorganize cpu_show_common()
In reply to Thomas GleixnerThe Spectre V2 printout in cpu_show_common() handles conditionals for the various mitigation methods directly in the sprintf() argument list. That's hard to read and will become unreadable if more complex decisions need to be made for a particular method.
Move the conditionals for STIBP and IBPB string selection into helper functions, so they can be extended later on.
--- arch/x86/kernel/cpu/bugs.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-)
arch/x86/kernel/cpu/bugs.c +18 -2
@@ -844,6 +844,22 @@ static ssize_t l1tf_show_state(char *buf }#endif +static char *stibp_state (void ) +{ +if (x86_spec_ctrl_base &SPEC_CTRL_STIBP ) +return ", STIBP" ; +else +return "" ; +} + +static char *ibpb_state (void ) +{ +if (boot_cpu_has (X86_FEATURE_USE_IBPB )) +return ", IBPB" ; +else +return "" ; +} +static ssize_t cpu_show_common (struct device *dev,struct device_attribute *attr,char *buf,unsigned int bug) {
@@ -865,9 +881,9 @@ static ssize_t cpu_show_common(struct decase X86_BUG_SPECTRE_V2 :return sprintf (buf,"%s%s%s%s%s%s\n" , spectre_v2_strings[spectre_v2_enabled], -boot_cpu_has (X86_FEATURE_USE_IBPB ) ?", IBPB" :"" , +ibpb_state (),boot_cpu_has (X86_FEATURE_USE_IBRS_FW ) ?", IBRS_FW" :"" , - (x86_spec_ctrl_base &SPEC_CTRL_STIBP ) ?", STIBP" :"" , +stibp_state (),boot_cpu_has (X86_FEATURE_RSB_CTXSW ) ?", RSB filling" :"" ,spectre_v2_module_string ());
Borislav Petkov
maintainerRe: [patch V2 04/28] x86/speculation: Reorganize cpu_show_common()
Quoted 8 lines · Thomas Gleixner
The Spectre V2 printout in
cpu_show_common()handles conditionals for the various mitigation methods directly in thesprintf()argument list. That's hard to read and will become unreadable if more complex decisions need to be made for a particular method.Move the conditionals for STIBP and IBPB string selection into helper functions, so they can be extended later on.
Quoted 6 lines
Signed-off-by Tim Chen <[email protected]>Signed-off-by Thomas Gleixner <[email protected]>--- arch/x86/kernel/cpu/bugs.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-)
Just a nitpick:
That subject should probably be more like:
"x86/speculation: Move cpu_show_common() string generation in separate functions"
or so, as it is not really reorganizing - just carving out functionality.
Signature · 4 lines
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.tip-bot for Tim Chen
merge bot[tip:x86/pti] x86/speculation: Move STIPB/IBPB string conditionals out of cpu_show_common()
In reply to Thomas Gleixnerx86/speculation: Move STIPB/IBPB string conditionals out of cpu_show_common()
Commit-ID: a8f76ae41cd633ac00be1b3019b1eb4741be3828 Gitweb: https://git.kernel.org/tip/a8f76ae41cd633ac00be1b3019b1eb4741be3828 Author: Tim Chen <[email protected]> AuthorDate: Sun, 25 Nov 2018 19:33:32 +0100 Committer: Thomas Gleixner <[email protected]> CommitDate: Wed, 28 Nov 2018 11:57:05 +0100
x86/speculation: Move STIPB/IBPB string conditionals out of cpu_show_common()
The Spectre V2 printout in cpu_show_common() handles conditionals for the various mitigation methods directly in the sprintf() argument list. That's hard to read and will become unreadable if more complex decisions need to be made for a particular method.
Move the conditionals for STIBP and IBPB string selection into helper functions, so they can be extended later on.
--- arch/x86/kernel/cpu/bugs.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-)
arch/x86/kernel/cpu/bugs.c +18 -2
@@ -844,6 +844,22 @@ static ssize_t l1tf_show_state(char *buf) }#endif +static char *stibp_state (void ) +{ +if (x86_spec_ctrl_base &SPEC_CTRL_STIBP ) +return ", STIBP" ; +else +return "" ; +} + +static char *ibpb_state (void ) +{ +if (boot_cpu_has (X86_FEATURE_USE_IBPB )) +return ", IBPB" ; +else +return "" ; +} +static ssize_t cpu_show_common (struct device *dev,struct device_attribute *attr,char *buf,unsigned int bug) {
@@ -865,9 +881,9 @@ static ssize_t cpu_show_common(struct device *dev, struct device_attribute *attrcase X86_BUG_SPECTRE_V2 :return sprintf (buf,"%s%s%s%s%s%s\n" , spectre_v2_strings[spectre_v2_enabled], -boot_cpu_has (X86_FEATURE_USE_IBPB ) ?", IBPB" :"" , +ibpb_state (),boot_cpu_has (X86_FEATURE_USE_IBRS_FW ) ?", IBRS_FW" :"" , - (x86_spec_ctrl_base &SPEC_CTRL_STIBP ) ?", STIBP" :"" , +stibp_state (),boot_cpu_has (X86_FEATURE_RSB_CTXSW ) ?", RSB filling" :"" ,spectre_v2_module_string ());
Re: [patch V2 04/28] x86/speculation: Reorganize cpu_show_common()
In reply to Thomas GleixnerQuoted 8 lines · Thomas Gleixner
The Spectre V2 printout in
cpu_show_common()handles conditionals for the various mitigation methods directly in thesprintf()argument list. That's hard to read and will become unreadable if more complex decisions need to be made for a particular method.Move the conditionals for STIBP and IBPB string selection into helper functions, so they can be extended later on.
Yeeey!
Thank you!
Quoted 46 lines
Signed-off-by Tim Chen <[email protected]>Signed-off-by Thomas Gleixner <[email protected]>--- arch/x86/kernel/cpu/bugs.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-)arch/x86/kernel/cpu/bugs.c +18
-2@@ -844,6 +844,22 @@ static ssize_t l1tf_show_state(char *buf }#endif +static char *stibp_state (void ) +{ +if (x86_spec_ctrl_base &SPEC_CTRL_STIBP ) +return ", STIBP" ; +else +return "" ; +} + +static char *ibpb_state (void ) +{ +if (boot_cpu_has (X86_FEATURE_USE_IBPB )) +return ", IBPB" ; +else +return "" ; +} +static ssize_t cpu_show_common (struct device *dev,struct device_attribute *attr,char *buf,unsigned int bug) {@@ -865,9 +881,9 @@ static ssize_t cpu_show_common(struct decase X86_BUG_SPECTRE_V2 :return sprintf (buf,"%s%s%s%s%s%s\n" , spectre_v2_strings[spectre_v2_enabled], -boot_cpu_has (X86_FEATURE_USE_IBPB ) ?", IBPB" :"" , +ibpb_state (),boot_cpu_has (X86_FEATURE_USE_IBRS_FW ) ?", IBRS_FW" :"" , - (x86_spec_ctrl_base &SPEC_CTRL_STIBP ) ?", STIBP" :"" , +stibp_state (),boot_cpu_has (X86_FEATURE_RSB_CTXSW ) ?", RSB filling" :"" ,spectre_v2_module_string ());
PATCH v2 5/28 x86/speculation: Disable STIBP when enhanced IBRS is in use
In reply to Thomas GleixnerIf enhanced IBRS is active, STIBP is redundant for mitigating Spectre v2 user space exploits from hyperthread sibling.
Disable STIBP when enhanced IBRS is used.
--- arch/x86/kernel/cpu/bugs.c | 7 +++++++ 1 file changed, 7 insertions(+)
arch/x86/kernel/cpu/bugs.c +7 -0
@@ -321,6 +321,10 @@ static bool stibp_needed(void)if (spectre_v2_enabled ==SPECTRE_V2_NONE )return false; +/* Enhanced IBRS makes using STIBP unnecessary. */ +if (spectre_v2_enabled ==SPECTRE_V2_IBRS_ENHANCED ) +return false; +if (!boot_cpu_has (X86_FEATURE_STIBP ))return false;
@@ -846,6 +850,9 @@ static ssize_t l1tf_show_state(char *bufstatic char *stibp_state (void ) { +if (spectre_v2_enabled ==SPECTRE_V2_IBRS_ENHANCED ) +return "" ; +if (x86_spec_ctrl_base &SPEC_CTRL_STIBP )return ", STIBP" ;else
tip-bot for Tim Chen
merge bot[tip:x86/pti] x86/speculation: Disable STIBP when enhanced IBRS is in use
x86/speculation: Disable STIBP when enhanced IBRS is in use
Commit-ID: 34bce7c9690b1d897686aac89604ba7adc365556 Gitweb: https://git.kernel.org/tip/34bce7c9690b1d897686aac89604ba7adc365556 Author: Tim Chen <[email protected]> AuthorDate: Sun, 25 Nov 2018 19:33:33 +0100 Committer: Thomas Gleixner <[email protected]> CommitDate: Wed, 28 Nov 2018 11:57:05 +0100
x86/speculation: Disable STIBP when enhanced IBRS is in use
If enhanced IBRS is active, STIBP is redundant for mitigating Spectre v2 user space exploits from hyperthread sibling.
Disable STIBP when enhanced IBRS is used.
--- arch/x86/kernel/cpu/bugs.c | 7 +++++++ 1 file changed, 7 insertions(+)
arch/x86/kernel/cpu/bugs.c +7 -0
@@ -321,6 +321,10 @@ static bool stibp_needed(void)if (spectre_v2_enabled ==SPECTRE_V2_NONE )return false; +/* Enhanced IBRS makes using STIBP unnecessary. */ +if (spectre_v2_enabled ==SPECTRE_V2_IBRS_ENHANCED ) +return false; +if (!boot_cpu_has (X86_FEATURE_STIBP ))return false;
@@ -846,6 +850,9 @@ static ssize_t l1tf_show_state(char *buf)static char *stibp_state (void ) { +if (spectre_v2_enabled ==SPECTRE_V2_IBRS_ENHANCED ) +return "" ; +if (x86_spec_ctrl_base &SPEC_CTRL_STIBP )return ", STIBP" ;else
Re: [patch V2 05/28] x86/speculation: Disable STIBP when enhanced IBRS is in use
In reply to Thomas GleixnerQuoted 4 lines · Thomas Gleixner
If enhanced IBRS is active, STIBP is redundant for mitigating Spectre v2 user space exploits from hyperthread sibling.
Disable STIBP when enhanced IBRS is used.
Thank you!
Quoted 33 lines
Signed-off-by Tim Chen <[email protected]>Signed-off-by Thomas Gleixner <[email protected]>--- arch/x86/kernel/cpu/bugs.c | 7 +++++++ 1 file changed, 7 insertions(+)arch/x86/kernel/cpu/bugs.c +7
-0@@ -321,6 +321,10 @@ static bool stibp_needed(void)if (spectre_v2_enabled ==SPECTRE_V2_NONE )return false; +/* Enhanced IBRS makes using STIBP unnecessary. */ +if (spectre_v2_enabled ==SPECTRE_V2_IBRS_ENHANCED ) +return false; +if (!boot_cpu_has (X86_FEATURE_STIBP ))return false;@@ -846,6 +850,9 @@ static ssize_t l1tf_show_state(char *bufstatic char *stibp_state (void ) { +if (spectre_v2_enabled ==SPECTRE_V2_IBRS_ENHANCED ) +return "" ; +if (x86_spec_ctrl_base &SPEC_CTRL_STIBP )return ", STIBP" ;else
PATCH v2 6/28 x86/speculation: Rename SSBD update functions
In reply to Thomas GleixnerDuring context switch, the SSBD bit in SPEC_CTRL MSR is updated according to changes of the TIF_SSBD flag in the current and next running task.
Currently, only the bit controlling speculative store bypass disable in SPEC_CTRL MSR is updated and the related update functions all have "speculative_store" or "ssb" in their names.
For enhanced mitigation control other bits in SPEC_CTRL MSR need to be updated as well, which makes the SSB names inadequate.
Rename the "speculative_store*" functions to a more generic name.
--- arch/x86/include/asm/spec-ctrl.h | 6 +++--- arch/x86/kernel/cpu/bugs.c | 4 ++-- arch/x86/kernel/process.c | 12 ++++++------ 3 files changed, 11 insertions(+), 11 deletions(-)
arch/x86/include/asm/spec-ctrl.h +3 -3
@@ -70,11 +70,11 @@ extern void speculative_store_bypass_ht_static inline void speculative_store_bypass_ht_init (void ) { }#endif -extern void speculative_store_bypass_update (unsigned long tif); +extern void speculation_ctrl_update (unsigned long tif); -static inline void speculative_store_bypass_update_current (void ) +static inline void speculation_ctrl_update_current (void ) { -speculative_store_bypass_update (current_thread_info ()->flags); +speculation_ctrl_update (current_thread_info ()->flags); }#endif
arch/x86/kernel/cpu/bugs.c +2 -2
@@ -200,7 +200,7 @@ x86_virt_spec_ctrl(u64 guest_spec_ctrl, tif = setguest ?ssbd_spec_ctrl_to_tif (guestval) :ssbd_spec_ctrl_to_tif (hostval); -speculative_store_bypass_update (tif); +speculation_ctrl_update (tif); } }EXPORT_SYMBOL_GPL (x86_virt_spec_ctrl);
@@ -632,7 +632,7 @@ static int ssb_prctl_set(struct task_str* mitigation until it is next scheduled. */ if (task == current && update) -speculative_store_bypass_update_current (); +speculation_ctrl_update_current ();return 0 ; }
arch/x86/kernel/process.c +6 -6
@@ -395,27 +395,27 @@ static __always_inline void amd_set_ssb_wrmsrl (MSR_AMD64_VIRT_SPEC_CTRL ,ssbd_tif_to_spec_ctrl (tifn)); } -static __always_inline void intel_set_ssb_state(unsigned long tifn) +static __always_inline void spec_ctrl_update_msr(unsigned long tifn) {u64 msr = x86_spec_ctrl_base |ssbd_tif_to_spec_ctrl (tifn);wrmsrl (MSR_IA32_SPEC_CTRL , msr); } -static __always_inline void __speculative_store_bypass_update(unsigned long tifn) +static __always_inline void __speculation_ctrl_update(unsigned long tifn) {if (static_cpu_has (X86_FEATURE_VIRT_SSBD ))amd_set_ssb_virt_state (tifn);else if (static_cpu_has (X86_FEATURE_LS_CFG_SSBD ))amd_set_core_ssb_state (tifn);else -intel_set_ssb_state (tifn); +spec_ctrl_update_msr (tifn); } -void speculative_store_bypass_update (unsigned long tif) +void speculation_ctrl_update (unsigned long tif) {preempt_disable (); -__speculative_store_bypass_update (tif); +__speculation_ctrl_update (tif);preempt_enable (); }
@@ -452,7 +452,7 @@ void __switch_to_xtra(struct task_structset_cpuid_faulting (!!(tifn & _TIF_NOCPUID));if ((tifp ^ tifn) & _TIF_SSBD) -__speculative_store_bypass_update (tifn); +__speculation_ctrl_update (tifn); }/*
Borislav Petkov
maintainerRe: [patch V2 06/28] x86/speculation: Rename SSBD update functions
Quoted 12 lines · Thomas Gleixner
During context switch, the SSBD bit in
SPEC_CTRLMSR is updated according to changes of theTIF_SSBDflag in the current and next running task.Currently, only the bit controlling speculative store bypass disable in
SPEC_CTRLMSR is updated and the related update functions all have "speculative_store" or "ssb" in their names.For enhanced mitigation control other bits in
SPEC_CTRLMSR need to be updated as well, which makes the SSB names inadequate.Rename the "speculative_store*" functions to a more generic name.
And add:
"No functional changes."
Signature · 4 lines
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.tip-bot for Thomas Gleixner
merge bot[tip:x86/pti] x86/speculation: Rename SSBD update functions
In reply to Thomas Gleixnerx86/speculation: Rename SSBD update functions
Commit-ID: 26c4d75b234040c11728a8acb796b3a85ba7507c Gitweb: https://git.kernel.org/tip/26c4d75b234040c11728a8acb796b3a85ba7507c Author: Thomas Gleixner <[email protected]> AuthorDate: Sun, 25 Nov 2018 19:33:34 +0100 Committer: Thomas Gleixner <[email protected]> CommitDate: Wed, 28 Nov 2018 11:57:06 +0100
x86/speculation: Rename SSBD update functions
During context switch, the SSBD bit in SPEC_CTRL MSR is updated according to changes of the TIF_SSBD flag in the current and next running task.
Currently, only the bit controlling speculative store bypass disable in SPEC_CTRL MSR is updated and the related update functions all have "speculative_store" or "ssb" in their names.
For enhanced mitigation control other bits in SPEC_CTRL MSR need to be updated as well, which makes the SSB names inadequate.
Rename the "speculative_store*" functions to a more generic name. No functional change.
--- arch/x86/include/asm/spec-ctrl.h | 6 +++--- arch/x86/kernel/cpu/bugs.c | 4 ++-- arch/x86/kernel/process.c | 12 ++++++------ 3 files changed, 11 insertions(+), 11 deletions(-)
arch/x86/include/asm/spec-ctrl.h +3 -3
@@ -70,11 +70,11 @@ extern void speculative_store_bypass_ht_init(void);static inline void speculative_store_bypass_ht_init (void ) { }#endif -extern void speculative_store_bypass_update (unsigned long tif); +extern void speculation_ctrl_update (unsigned long tif); -static inline void speculative_store_bypass_update_current (void ) +static inline void speculation_ctrl_update_current (void ) { -speculative_store_bypass_update (current_thread_info ()->flags); +speculation_ctrl_update (current_thread_info ()->flags); }#endif
arch/x86/kernel/cpu/bugs.c +2 -2
@@ -200,7 +200,7 @@ x86_virt_spec_ctrl(u64 guest_spec_ctrl, u64 guest_virt_spec_ctrl, bool setguest) tif = setguest ?ssbd_spec_ctrl_to_tif (guestval) :ssbd_spec_ctrl_to_tif (hostval); -speculative_store_bypass_update (tif); +speculation_ctrl_update (tif); } }EXPORT_SYMBOL_GPL (x86_virt_spec_ctrl);
@@ -632,7 +632,7 @@ static int ssb_prctl_set(struct task_struct *task, unsigned long ctrl)* mitigation until it is next scheduled. */ if (task == current && update) -speculative_store_bypass_update_current (); +speculation_ctrl_update_current ();return 0 ; }
arch/x86/kernel/process.c +6 -6
@@ -395,27 +395,27 @@ static __always_inline void amd_set_ssb_virt_state(unsigned long tifn)wrmsrl (MSR_AMD64_VIRT_SPEC_CTRL ,ssbd_tif_to_spec_ctrl (tifn)); } -static __always_inline void intel_set_ssb_state(unsigned long tifn) +static __always_inline void spec_ctrl_update_msr(unsigned long tifn) {u64 msr = x86_spec_ctrl_base |ssbd_tif_to_spec_ctrl (tifn);wrmsrl (MSR_IA32_SPEC_CTRL , msr); } -static __always_inline void __speculative_store_bypass_update(unsigned long tifn) +static __always_inline void __speculation_ctrl_update(unsigned long tifn) {if (static_cpu_has (X86_FEATURE_VIRT_SSBD ))amd_set_ssb_virt_state (tifn);else if (static_cpu_has (X86_FEATURE_LS_CFG_SSBD ))amd_set_core_ssb_state (tifn);else -intel_set_ssb_state (tifn); +spec_ctrl_update_msr (tifn); } -void speculative_store_bypass_update (unsigned long tif) +void speculation_ctrl_update (unsigned long tif) {preempt_disable (); -__speculative_store_bypass_update (tif); +__speculation_ctrl_update (tif);preempt_enable (); }
@@ -452,7 +452,7 @@ void __switch_to_xtra(struct task_struct *prev_p, struct task_struct *next_p,set_cpuid_faulting (!!(tifn & _TIF_NOCPUID));if ((tifp ^ tifn) & _TIF_SSBD) -__speculative_store_bypass_update (tifn); +__speculation_ctrl_update (tifn); }/*
Re: [patch V2 06/28] x86/speculation: Rename SSBD update functions
In reply to Thomas GleixnerQuoted 7 lines · Thomas Gleixner
During context switch, the SSBD bit in
SPEC_CTRLMSR is updated according to changes of theTIF_SSBDflag in the current and next running task.Currently, only the bit controlling speculative store bypass disable in
SPEC_CTRLMSR is updated and the related update functions all have "speculative_store" or "ssb" in their names.
Quoted 7 lines
For enhanced mitigation control other bits in
SPEC_CTRLMSR need to be updated as well, which makes the SSB names inadequate.Rename the "speculative_store*" functions to a more generic name.
Signed-off-by Tim Chen <[email protected]>Signed-off-by Thomas Gleixner <[email protected]>
Thank you!
PATCH v2 7/28 x86/speculation: Reorganize speculation control MSRs update
In reply to Thomas GleixnerThe logic to detect whether there's a change in the previous and next task's flag relevant to update speculation control MSRs are spread out across multiple functions.
Consolidate all checks needed for updating speculation control MSRs into the new __speculation_ctrl_update() helper function.
This makes it easy to pick the right speculation control MSR and the bits in the MSR that needs updating based on TIF flags changes.
--- arch/x86/kernel/process.c | 42 ++++++++++++++++++++++++++++++++---------- 1 file changed, 32 insertions(+), 10 deletions(-)
arch/x86/kernel/process.c +32 -10
@@ -397,25 +397,48 @@ static __always_inline void amd_set_ssb_ static __always_inline void spec_ctrl_update_msr(unsigned long tifn) { -u64 msr = x86_spec_ctrl_base |ssbd_tif_to_spec_ctrl (tifn); +u64 msr = x86_spec_ctrl_base; + +/* +* If X86_FEATURE_SSBD is not set, the SSBD bit is not to be +* touched. +*/ +if (static_cpu_has (X86_FEATURE_SSBD )) + msr |=ssbd_tif_to_spec_ctrl (tifn);wrmsrl (MSR_IA32_SPEC_CTRL , msr); } -static __always_inline void __speculation_ctrl_update(unsigned long tifn) +/* +* Update the MSRs managing speculation control, during context switch. +* +* tifp: Previous task's thread flags +* tifn: Next task's thread flags +*/ +static __always_inline void __speculation_ctrl_update(unsigned long tifp, +unsigned long tifn) { -if (static_cpu_has (X86_FEATURE_VIRT_SSBD )) -amd_set_ssb_virt_state (tifn); -else if (static_cpu_has (X86_FEATURE_LS_CFG_SSBD )) -amd_set_core_ssb_state (tifn); -else +bool updmsr = false; + +/* If TIF_SSBD is different, select the proper mitigation method */ +if ((tifp ^ tifn) & _TIF_SSBD) { +if (static_cpu_has (X86_FEATURE_VIRT_SSBD )) +amd_set_ssb_virt_state (tifn); +else if (static_cpu_has (X86_FEATURE_LS_CFG_SSBD )) +amd_set_core_ssb_state (tifn); +else if (static_cpu_has (X86_FEATURE_SSBD )) + updmsr = true; + } + +if (updmsr)spec_ctrl_update_msr (tifn); }void speculation_ctrl_update (unsigned long tif) { +/* Forced update. Make sure all relevant TIF flags are different */ preempt_disable (); -__speculation_ctrl_update (tif); +__speculation_ctrl_update (~tif, tif);preempt_enable (); }
@@ -451,8 +474,7 @@ void __switch_to_xtra(struct task_structif ((tifp ^ tifn) & _TIF_NOCPUID)set_cpuid_faulting (!!(tifn & _TIF_NOCPUID)); -if ((tifp ^ tifn) & _TIF_SSBD) -__speculation_ctrl_update (tifn); +__speculation_ctrl_update (tifp, tifn); }/*
Borislav Petkov
maintainerRe: [patch V2 07/28] x86/speculation: Reorganize speculation control MSRs update
Quoted 2 lines · Thomas Gleixner
The logic to detect whether there's a change in the previous and next task's flag relevant to update speculation control MSRs are spread out
s/are/is/
Quoted 7 lines
across multiple functions.
Consolidate all checks needed for updating speculation control MSRs into the new
__speculation_ctrl_update()helper function.This makes it easy to pick the right speculation control MSR and the bits in the MSR that needs updating based on TIF flags changes.
s/needs/need/
Signature · 4 lines
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.tip-bot for Tim Chen
merge bot[tip:x86/pti] x86/speculation: Reorganize speculation control MSRs update
In reply to Thomas Gleixnerx86/speculation: Reorganize speculation control MSRs update
Commit-ID: 01daf56875ee0cd50ed496a09b20eb369b45dfa5 Gitweb: https://git.kernel.org/tip/01daf56875ee0cd50ed496a09b20eb369b45dfa5 Author: Tim Chen <[email protected]> AuthorDate: Sun, 25 Nov 2018 19:33:35 +0100 Committer: Thomas Gleixner <[email protected]> CommitDate: Wed, 28 Nov 2018 11:57:06 +0100
x86/speculation: Reorganize speculation control MSRs update
The logic to detect whether there's a change in the previous and next task's flag relevant to update speculation control MSRs is spread out across multiple functions.
Consolidate all checks needed for updating speculation control MSRs into the new __speculation_ctrl_update() helper function.
This makes it easy to pick the right speculation control MSR and the bits in MSR_IA32_SPEC_CTRL that need updating based on TIF flags changes.
--- arch/x86/kernel/process.c | 46 +++++++++++++++++++++++++++++----------------- 1 file changed, 29 insertions(+), 17 deletions(-)
arch/x86/kernel/process.c +29 -17
@@ -395,27 +395,40 @@ static __always_inline void amd_set_ssb_virt_state(unsigned long tifn)wrmsrl (MSR_AMD64_VIRT_SPEC_CTRL ,ssbd_tif_to_spec_ctrl (tifn)); } -static __always_inline void spec_ctrl_update_msr(unsigned long tifn) -{ -u64 msr = x86_spec_ctrl_base |ssbd_tif_to_spec_ctrl (tifn); - -wrmsrl (MSR_IA32_SPEC_CTRL , msr); -} +/* +* Update the MSRs managing speculation control, during context switch. +* +* tifp: Previous task's thread flags +* tifn: Next task's thread flags +*/ +static __always_inline void __speculation_ctrl_update(unsigned long tifp, +unsigned long tifn) +{ +u64 msr = x86_spec_ctrl_base; +bool updmsr = false; + +/* If TIF_SSBD is different, select the proper mitigation method */ +if ((tifp ^ tifn) & _TIF_SSBD) { +if (static_cpu_has (X86_FEATURE_VIRT_SSBD )) { +amd_set_ssb_virt_state (tifn); + }else if (static_cpu_has (X86_FEATURE_LS_CFG_SSBD )) { +amd_set_core_ssb_state (tifn); + }else if (static_cpu_has (X86_FEATURE_SPEC_CTRL_SSBD ) || +static_cpu_has (X86_FEATURE_AMD_SSBD )) { + msr |=ssbd_tif_to_spec_ctrl (tifn); + updmsr = true; + } + } -static __always_inline void __speculation_ctrl_update(unsigned long tifn) -{ -if (static_cpu_has (X86_FEATURE_VIRT_SSBD )) -amd_set_ssb_virt_state (tifn); -else if (static_cpu_has (X86_FEATURE_LS_CFG_SSBD )) -amd_set_core_ssb_state (tifn); -else -spec_ctrl_update_msr (tifn); +if (updmsr) +wrmsrl (MSR_IA32_SPEC_CTRL , msr); }void speculation_ctrl_update (unsigned long tif) { +/* Forced update. Make sure all relevant TIF flags are different */ preempt_disable (); -__speculation_ctrl_update (tif); +__speculation_ctrl_update (~tif, tif);preempt_enable (); }
@@ -451,8 +464,7 @@ void __switch_to_xtra(struct task_struct *prev_p, struct task_struct *next_p,if ((tifp ^ tifn) & _TIF_NOCPUID)set_cpuid_faulting (!!(tifn & _TIF_NOCPUID)); -if ((tifp ^ tifn) & _TIF_SSBD) -__speculation_ctrl_update (tifn); +__speculation_ctrl_update (tifp, tifn); }/*
Re: [patch V2 07/28] x86/speculation: Reorganize speculation control MSRs update
In reply to Thomas GleixnerQuoted 7 lines · Thomas Gleixner
The logic to detect whether there's a change in the previous and next task's flag relevant to update speculation control MSRs are spread out across multiple functions.
Consolidate all checks needed for updating speculation control MSRs into the new
__speculation_ctrl_update()helper function.
Quoted 6 lines
This makes it easy to pick the right speculation control MSR and the bits in the MSR that needs updating based on TIF flags changes.
Originally-by Thomas Lendacky <[email protected]>Signed-off-by Tim Chen <[email protected]>Signed-off-by Thomas Gleixner <[email protected]>
.. and I also have two tiny comments below - feel free to incorporate or not them in.
Quoted 8 lines
--- arch/x86/kernel/process.c | 42 ++++++++++++++++++++++++++++++++---------- 1 file changed, 32 insertions(+), 10 deletions(-)
arch/x86/kernel/process.c static __always_inline void amd_set_ssb_ · line 405
Quoted 10 lines
arch/x86/kernel/process.c +6
-1@@ -397,25 +397,48 @@ static __always_inline void amd_set_ssb_ static __always_inline void spec_ctrl_update_msr(unsigned long tifn) { -u64 msr = x86_spec_ctrl_base |ssbd_tif_to_spec_ctrl (tifn); +u64 msr = x86_spec_ctrl_base; + +/* +* If X86_FEATURE_SSBD is not set, the SSBD bit is not to be +* touched. +*/
I had a bit of hard time parsing that. Could it perhaps be changed to:
"If X86_FEATURE_SSBD is off (not set), we MUST leave the SSBD bit alone"
Quoted 21 lines
arch/x86/kernel/process.c +10
-6+if (static_cpu_has (X86_FEATURE_SSBD )) + msr |=ssbd_tif_to_spec_ctrl (tifn);wrmsrl (MSR_IA32_SPEC_CTRL , msr); } -static __always_inline void __speculation_ctrl_update(unsigned long tifn) +/* +* Update the MSRs managing speculation control, during context switch. +* +* tifp: Previous task's thread flags +* tifn: Next task's thread flags +*/ +static __always_inline void __speculation_ctrl_update(unsigned long tifp, +unsigned long tifn) { -if (static_cpu_has (X86_FEATURE_VIRT_SSBD )) -amd_set_ssb_virt_state (tifn); -else if (static_cpu_has (X86_FEATURE_LS_CFG_SSBD )) -amd_set_core_ssb_state (tifn); -else
arch/x86/kernel/process.c static __always_inline void amd_set_ssb_ · line 430
Quoted 10 lines
arch/x86/kernel/process.c +10
-0+bool updmsr = false; + +/* If TIF_SSBD is different, select the proper mitigation method */ +if ((tifp ^ tifn) & _TIF_SSBD) { +if (static_cpu_has (X86_FEATURE_VIRT_SSBD )) +amd_set_ssb_virt_state (tifn); +else if (static_cpu_has (X86_FEATURE_LS_CFG_SSBD )) +amd_set_core_ssb_state (tifn); +else if (static_cpu_has (X86_FEATURE_SSBD )) + updmsr = true;
^
Nothing really big, but you have an extra space here.
Quoted 27 lines
arch/x86/kernel/process.c +6
-3+ } + +if (updmsr)spec_ctrl_update_msr (tifn); }void speculation_ctrl_update (unsigned long tif) { +/* Forced update. Make sure all relevant TIF flags are different */ preempt_disable (); -__speculation_ctrl_update (tif); +__speculation_ctrl_update (~tif, tif);preempt_enable (); }@@ -451,8 +474,7 @@ void __switch_to_xtra(struct task_structif ((tifp ^ tifn) & _TIF_NOCPUID)set_cpuid_faulting (!!(tifn & _TIF_NOCPUID)); -if ((tifp ^ tifn) & _TIF_SSBD) -__speculation_ctrl_update (tifn); +__speculation_ctrl_update (tifp, tifn); }/*
PATCH v2 8/28 sched/smt: Make sched_smt_present track topology
In reply to Thomas GleixnerCurrently the 'sched_smt_present' static key is enabled when at CPU bringup SMT topology is observed, but it is never disabled. However there is demand to also disable the key when the topology changes such that there is no SMT present anymore.
Implement this by making the key count the number of cores that have SMT enabled.
In particular, the SMT topology bits are set before interrrupts are enabled and similarly, are cleared after interrupts are disabled for the last time and the CPU dies.
--- kernel/sched/core.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-)
kernel/sched/core.c +11 -8
@@ -5738,15 +5738,10 @@ int sched_cpu_activate(unsigned int cpu)#ifdef CONFIG_SCHED_SMT /* -* The sched_smt_present static key needs to be evaluated on every -* hotplug event because at boot time SMT might be disabled when -* the number of booted CPUs is limited. -* -* If then later a sibling gets hotplugged, then the key would stay -* off and SMT scheduling would never be functional. +* When going up, increment the number of cores with SMT present. */ -if (cpumask_weight (cpu_smt_mask (cpu)) >1 ) -static_branch_enable_cpuslocked (&sched_smt_present); +if (cpumask_weight (cpu_smt_mask (cpu)) ==2 ) +static_branch_inc_cpuslocked (&sched_smt_present);#endif set_cpu_active (cpu, true);
@@ -5790,6 +5785,14 @@ int sched_cpu_deactivate(unsigned int cp*/ synchronize_rcu_mult (call_rcu, call_rcu_sched); +#ifdef CONFIG_SCHED_SMT +/* +* When going down, decrement the number of cores with SMT present. +*/ +if (cpumask_weight (cpu_smt_mask (cpu)) ==2 ) +static_branch_dec_cpuslocked (&sched_smt_present); +#endif +if (!sched_smp_initialized)return 0 ;
tip-bot for Peter Zijlstra (Intel)
merge bot[tip:x86/pti] sched/smt: Make sched_smt_present track topology
sched/smt: Make sched_smt_present track topology
Commit-ID: c5511d03ec090980732e929c318a7a6374b5550e Gitweb: https://git.kernel.org/tip/c5511d03ec090980732e929c318a7a6374b5550e Author: Peter Zijlstra (Intel) <[email protected]> AuthorDate: Sun, 25 Nov 2018 19:33:36 +0100 Committer: Thomas Gleixner <[email protected]> CommitDate: Wed, 28 Nov 2018 11:57:06 +0100
sched/smt: Make sched_smt_present track topology
Currently the 'sched_smt_present' static key is enabled when at CPU bringup SMT topology is observed, but it is never disabled. However there is demand to also disable the key when the topology changes such that there is no SMT present anymore.
Implement this by making the key count the number of cores that have SMT enabled.
In particular, the SMT topology bits are set before interrrupts are enabled and similarly, are cleared after interrupts are disabled for the last time and the CPU dies.
--- kernel/sched/core.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-)
kernel/sched/core.c +11 -8
@@ -5738,15 +5738,10 @@ int sched_cpu_activate(unsigned int cpu)#ifdef CONFIG_SCHED_SMT /* -* The sched_smt_present static key needs to be evaluated on every -* hotplug event because at boot time SMT might be disabled when -* the number of booted CPUs is limited. -* -* If then later a sibling gets hotplugged, then the key would stay -* off and SMT scheduling would never be functional. +* When going up, increment the number of cores with SMT present. */ -if (cpumask_weight (cpu_smt_mask (cpu)) >1 ) -static_branch_enable_cpuslocked (&sched_smt_present); +if (cpumask_weight (cpu_smt_mask (cpu)) ==2 ) +static_branch_inc_cpuslocked (&sched_smt_present);#endif set_cpu_active (cpu, true);
@@ -5790,6 +5785,14 @@ int sched_cpu_deactivate(unsigned int cpu)*/ synchronize_rcu_mult (call_rcu, call_rcu_sched); +#ifdef CONFIG_SCHED_SMT +/* +* When going down, decrement the number of cores with SMT present. +*/ +if (cpumask_weight (cpu_smt_mask (cpu)) ==2 ) +static_branch_dec_cpuslocked (&sched_smt_present); +#endif +if (!sched_smp_initialized)return 0 ;
Re: [patch V2 08/28] sched/smt: Make sched_smt_present track topology
In reply to Thomas GleixnerQuoted 11 lines · Thomas Gleixner
Currently the '
sched_smt_present' static key is enabled when at CPU bringup SMT topology is observed, but it is never disabled. However there is demand to also disable the key when the topology changes such that there is no SMT present anymore.Implement this by making the key count the number of cores that have SMT enabled.
In particular, the SMT topology bits are set before interrrupts are enabled and similarly, are cleared after interrupts are disabled for the last time and the CPU dies.
I see that the number you used is '2', but I thought that there are some CPUs out there (Knights Landing?) that could have four threads?
Would it be better to have a generic function that would provide the amount of threads the platform does expose - and use that instead of a constant value?
Quoted 46 lines
Signed-off-by Peter Zijlstra (Intel) <[email protected]>Signed-off-by Thomas Gleixner <[email protected]>--- kernel/sched/core.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-)kernel/sched/core.c +11
-8@@ -5738,15 +5738,10 @@ int sched_cpu_activate(unsigned int cpu)#ifdef CONFIG_SCHED_SMT /* -* The sched_smt_present static key needs to be evaluated on every -* hotplug event because at boot time SMT might be disabled when -* the number of booted CPUs is limited. -* -* If then later a sibling gets hotplugged, then the key would stay -* off and SMT scheduling would never be functional. +* When going up, increment the number of cores with SMT present. */ -if (cpumask_weight (cpu_smt_mask (cpu)) >1 ) -static_branch_enable_cpuslocked (&sched_smt_present); +if (cpumask_weight (cpu_smt_mask (cpu)) ==2 ) +static_branch_inc_cpuslocked (&sched_smt_present);#endif set_cpu_active (cpu, true);@@ -5790,6 +5785,14 @@ int sched_cpu_deactivate(unsigned int cp*/ synchronize_rcu_mult (call_rcu, call_rcu_sched); +#ifdef CONFIG_SCHED_SMT +/* +* When going down, decrement the number of cores with SMT present. +*/ +if (cpumask_weight (cpu_smt_mask (cpu)) ==2 ) +static_branch_dec_cpuslocked (&sched_smt_present); +#endif +if (!sched_smp_initialized)return 0 ;
Konrad Rzeszutek Wilk
Reviewed-byRe: [patch V2 08/28] sched/smt: Make sched_smt_present track topology
Quoted 13 lines · Konrad Rzeszutek Wilk
Quoted 11 lines · Thomas Gleixner
Currently the '
sched_smt_present' static key is enabled when at CPU bringup SMT topology is observed, but it is never disabled. However there is demand to also disable the key when the topology changes such that there is no SMT present anymore.Implement this by making the key count the number of cores that have SMT enabled.
In particular, the SMT topology bits are set before interrrupts are enabled and similarly, are cleared after interrupts are disabled for the last time and the CPU dies.
Quoted 6 lines
I see that the number you used is '2', but I thought that there are some CPUs out there (Knights Landing?) that could have four threads?
Would it be better to have a generic function that would provide the amount of threads the platform does expose - and use that instead of a constant value?
Nevermind - this would work even with 4 threads as we would hit the number '2' before '4' and the key would be turned on/off properly.
Sorry for the noise.
Thank you!
Quoted 47 lines
Quoted 47 lines
Signed-off-by Peter Zijlstra (Intel) <[email protected]>Signed-off-by Thomas Gleixner <[email protected]>--- kernel/sched/core.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-)kernel/sched/core.c +11
-8@@ -5738,15 +5738,10 @@ int sched_cpu_activate(unsigned int cpu)#ifdef CONFIG_SCHED_SMT /* -* The sched_smt_present static key needs to be evaluated on every -* hotplug event because at boot time SMT might be disabled when -* the number of booted CPUs is limited. -* -* If then later a sibling gets hotplugged, then the key would stay -* off and SMT scheduling would never be functional. +* When going up, increment the number of cores with SMT present. */ -if (cpumask_weight (cpu_smt_mask (cpu)) >1 ) -static_branch_enable_cpuslocked (&sched_smt_present); +if (cpumask_weight (cpu_smt_mask (cpu)) ==2 ) +static_branch_inc_cpuslocked (&sched_smt_present);#endif set_cpu_active (cpu, true);@@ -5790,6 +5785,14 @@ int sched_cpu_deactivate(unsigned int cp*/ synchronize_rcu_mult (call_rcu, call_rcu_sched); +#ifdef CONFIG_SCHED_SMT +/* +* When going down, decrement the number of cores with SMT present. +*/ +if (cpumask_weight (cpu_smt_mask (cpu)) ==2 ) +static_branch_dec_cpuslocked (&sched_smt_present); +#endif +if (!sched_smp_initialized)return 0 ;
Peter Zijlstra
maintainerRe: [patch V2 08/28] sched/smt: Make sched_smt_present track topology
Quoted 10 lines · Konrad Rzeszutek Wilk
Quoted 9 lines · Konrad Rzeszutek Wilk
Quoted 8 lines · Thomas Gleixner
Currently the '
sched_smt_present' static key is enabled when at CPU bringup SMT topology is observed, but it is never disabled. However there is demand to also disable the key when the topology changes such that there is no SMT present anymore.Implement this by making the key count the number of cores that have SMT enabled.
Quoted 6 lines
Quoted 6 lines
Quoted 3 lines
In particular, the SMT topology bits are set before interrrupts are enabled and similarly, are cleared after interrupts are disabled for the last time and the CPU dies.
I see that the number you used is '2', but I thought that there are some CPUs out there (Knights Landing?) that could have four threads?
This code is generic, Sparc, Power and I think MIPS all have parts with SMT8.
Quoted 6 lines
Quoted 3 lines
Would it be better to have a generic function that would provide the amount of threads the platform does expose - and use that instead of a constant value?
Nevermind - this would work even with 4 threads as we would hit the number '2' before '4' and the key would be turned on/off properly.
Indeed so, 2 is the point where we either have more than 1 sibling, or will go back to 1.
PATCH v2 9/28 x86/Kconfig: Select SCHED_SMT if SMP enabled
In reply to Thomas GleixnerCONFIG_SCHED_SMT is enabled by all distros, so there is not a real point to have it configurable. The runtime overhead in the core scheduler code is minimal because the actual SMT scheduling parts are conditional on a static key.
This allows to expose the scheduler's SMT state static key to the speculation control code. Alternatively the scheduler's static key could be made always available when CONFIG_SMP is enabled, but that's just adding an unused static key to every other architecture for nothing.
--- arch/x86/Kconfig | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-)
arch/x86/Kconfig +1 -7
@@ -1001,13 +1001,7 @@ config NR_CPUS to the kernel image. config SCHED_SMT - bool "SMT (Hyperthreading) scheduler support" - depends on SMP - ---help--- - SMT scheduler support improves the CPU scheduler's decision making - when dealing with Intel Pentium 4 chips with HyperThreading at a - cost of slightly increased overhead in some places. If unsure say - N here. + def_bool y if SMP config SCHED_MC def_bool y
tip-bot for Thomas Gleixner
merge bot[tip:x86/pti] x86/Kconfig: Select SCHED_SMT if SMP enabled
x86/Kconfig: Select SCHED_SMT if SMP enabled
Commit-ID: dbe733642e01dd108f71436aaea7b328cb28fd87 Gitweb: https://git.kernel.org/tip/dbe733642e01dd108f71436aaea7b328cb28fd87 Author: Thomas Gleixner <[email protected]> AuthorDate: Sun, 25 Nov 2018 19:33:37 +0100 Committer: Thomas Gleixner <[email protected]> CommitDate: Wed, 28 Nov 2018 11:57:07 +0100
x86/Kconfig: Select SCHED_SMT if SMP enabled
CONFIG_SCHED_SMT is enabled by all distros, so there is not a real point to have it configurable. The runtime overhead in the core scheduler code is minimal because the actual SMT scheduling parts are conditional on a static key.
This allows to expose the scheduler's SMT state static key to the speculation control code. Alternatively the scheduler's static key could be made always available when CONFIG_SMP is enabled, but that's just adding an unused static key to every other architecture for nothing.
--- arch/x86/Kconfig | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-)
arch/x86/Kconfig +1 -7
@@ -1000,13 +1000,7 @@ config NR_CPUS to the kernel image. config SCHED_SMT - bool "SMT (Hyperthreading) scheduler support" - depends on SMP - ---help--- - SMT scheduler support improves the CPU scheduler's decision making - when dealing with Intel Pentium 4 chips with HyperThreading at a - cost of slightly increased overhead in some places. If unsure say - N here. + def_bool y if SMP config SCHED_MC def_bool y
Re: [patch V2 09/28] x86/Kconfig: Select SCHED_SMT if SMP enabled
In reply to Thomas GleixnerQuoted 11 lines · Thomas Gleixner
CONFIG_SCHED_SMTis enabled by all distros, so there is not a real point to have it configurable. The runtime overhead in the core scheduler code is minimal because the actual SMT scheduling parts are conditional on a static key.This allows to expose the scheduler's SMT state static key to the speculation control code. Alternatively the scheduler's static key could be made always available when
CONFIG_SMPis enabled, but that's just adding an unused static key to every other architecture for nothing.Signed-off-by Thomas Gleixner <[email protected]>
Quoted 24 lines
--- arch/x86/Kconfig | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-)arch/x86/Kconfig +1
-7@@ -1001,13 +1001,7 @@ config NR_CPUS to the kernel image. config SCHED_SMT - bool "SMT (Hyperthreading) scheduler support" - depends on SMP - ---help--- - SMT scheduler support improves the CPU scheduler's decision making - when dealing with Intel Pentium 4 chips with HyperThreading at a - cost of slightly increased overhead in some places. If unsure say - N here. + def_bool y if SMP config SCHED_MC def_bool y
PATCH v2 10/28 sched/smt: Expose sched_smt_present static key
In reply to Thomas GleixnerMake the scheduler's 'sched_smt_present' static key globaly available, so it can be used in the x86 speculation control code.
Provide a query function and a stub for the CONFIG_SMP=n case.
---
v1 -> v2: Move SMT stuff to separate header. Unbreaks ia64 build
--- include/linux/sched/smt.h | 18 ++++++++++++++++++ kernel/sched/sched.h | 4 +--- 2 files changed, 19 insertions(+), 3 deletions(-)
include/linux/sched/smt.h new file +18 -0
@@ -0,0 +1,18 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _LINUX_SCHED_SMT_H +#define _LINUX_SCHED_SMT_H + +#include <linux/static_key.h> + +#ifdef CONFIG_SCHED_SMT +extern struct static_key_false sched_smt_present; + +static __always_inline bool sched_smt_active(void) +{ +return static_branch_likely (&sched_smt_present); +} +#else +static inline bool sched_smt_active (void ) {return false; } +#endif + +#endif
kernel/sched/sched.h +1 -3
@@ -23,6 +23,7 @@#include <linux/sched/prio.h> #include <linux/sched/rt.h> #include <linux/sched/signal.h> +#include <linux/sched/smt.h> #include <linux/sched/stat.h> #include <linux/sched/sysctl.h> #include <linux/sched/task.h>
@@ -936,9 +937,6 @@ static inline int cpu_of(struct rq *rq)#ifdef CONFIG_SCHED_SMT - -extern struct static_key_false sched_smt_present; -extern void __update_idle_core (struct rq *rq);static inline void update_idle_core (struct rq *rq)
tip-bot for Thomas Gleixner
merge bot[tip:x86/pti] sched/smt: Expose sched_smt_present static key
sched/smt: Expose sched_smt_present static key
Commit-ID: 321a874a7ef85655e93b3206d0f36b4a6097f948 Gitweb: https://git.kernel.org/tip/321a874a7ef85655e93b3206d0f36b4a6097f948 Author: Thomas Gleixner <[email protected]> AuthorDate: Sun, 25 Nov 2018 19:33:38 +0100 Committer: Thomas Gleixner <[email protected]> CommitDate: Wed, 28 Nov 2018 11:57:07 +0100
sched/smt: Expose sched_smt_present static key
Make the scheduler's 'sched_smt_present' static key globaly available, so it can be used in the x86 speculation control code.
Provide a query function and a stub for the CONFIG_SMP=n case.
--- include/linux/sched/smt.h | 18 ++++++++++++++++++ kernel/sched/sched.h | 4 +--- 2 files changed, 19 insertions(+), 3 deletions(-)
include/linux/sched/smt.h new file +18 -0
@@ -0,0 +1,18 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _LINUX_SCHED_SMT_H +#define _LINUX_SCHED_SMT_H + +#include <linux/static_key.h> + +#ifdef CONFIG_SCHED_SMT +extern struct static_key_false sched_smt_present; + +static __always_inline bool sched_smt_active(void) +{ +return static_branch_likely (&sched_smt_present); +} +#else +static inline bool sched_smt_active (void ) {return false; } +#endif + +#endif
kernel/sched/sched.h +1 -3
@@ -23,6 +23,7 @@#include <linux/sched/prio.h> #include <linux/sched/rt.h> #include <linux/sched/signal.h> +#include <linux/sched/smt.h> #include <linux/sched/stat.h> #include <linux/sched/sysctl.h> #include <linux/sched/task.h>
@@ -936,9 +937,6 @@ static inline int cpu_of(struct rq *rq)#ifdef CONFIG_SCHED_SMT - -extern struct static_key_false sched_smt_present; -extern void __update_idle_core (struct rq *rq);static inline void update_idle_core (struct rq *rq)
Re: [patch V2 10/28] sched/smt: Expose sched_smt_present static key
In reply to Thomas GleixnerQuoted 6 lines · Thomas Gleixner
Make the scheduler's '
sched_smt_present' static key globaly available, so it can be used in the x86 speculation control code.Provide a query function and a stub for the
CONFIG_SMP=ncase.Signed-off-by Thomas Gleixner <[email protected]>
Quoted 52 lines
---
v1 -> v2: Move SMT stuff to separate header. Unbreaks ia64 build
--- include/linux/sched/smt.h | 18 ++++++++++++++++++ kernel/sched/sched.h | 4 +--- 2 files changed, 19 insertions(+), 3 deletions(-)include/linux/sched/smt.h new file +18
-0@@ -0,0 +1,18 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _LINUX_SCHED_SMT_H +#define _LINUX_SCHED_SMT_H + +#include <linux/static_key.h> + +#ifdef CONFIG_SCHED_SMT +extern struct static_key_false sched_smt_present; + +static __always_inline bool sched_smt_active(void) +{ +return static_branch_likely (&sched_smt_present); +} +#else +static inline bool sched_smt_active (void ) {return false; } +#endif + +#endif kernel/sched/sched.h +1
-3@@ -23,6 +23,7 @@#include <linux/sched/prio.h> #include <linux/sched/rt.h> #include <linux/sched/signal.h> +#include <linux/sched/smt.h> #include <linux/sched/stat.h> #include <linux/sched/sysctl.h> #include <linux/sched/task.h> @@ -936,9 +937,6 @@ static inline int cpu_of(struct rq *rq)#ifdef CONFIG_SCHED_SMT - -extern struct static_key_false sched_smt_present; -extern void __update_idle_core (struct rq *rq);static inline void update_idle_core (struct rq *rq)
PATCH v2 11/28 x86/speculation: Rework SMT state change
In reply to Thomas Gleixnerarch_smt_update() is only called when the sysfs SMT control knob is changed. This means that when SMT is enabled in the sysfs control knob the system is considered to have SMT active even if all siblings are offline.
To allow finegrained control of the speculation mitigations, the actual SMT state is more interesting than the fact that siblings could be enabled.
Rework the code, so arch_smt_update() is invoked from each individual CPU hotplug function, and simplify the update function while at it.
---
v1 -> v2: Adapt to new header sched/smt.h
---
arch/x86/kernel/cpu/bugs.c | 11 +++++------ include/linux/sched/smt.h | 2 ++ kernel/cpu.c | 15 +++++++++------ 3 files changed, 16 insertions(+), 12 deletions(-)
arch/x86/kernel/cpu/bugs.c +5 -6
@@ -14,6 +14,7 @@#include <linux/module.h> #include <linux/nospec.h> #include <linux/prctl.h> +#include <linux/sched/smt.h> #include <asm/spec-ctrl.h> #include <asm/cmdline.h>
@@ -344,16 +345,14 @@ void arch_smt_update(void)return ;mutex_lock (&spec_ctrl_mutex); - mask = x86_spec_ctrl_base; -if (cpu_smt_control ==CPU_SMT_ENABLED ) + + mask = x86_spec_ctrl_base & ~SPEC_CTRL_STIBP ; +if (sched_smt_active ()) mask |=SPEC_CTRL_STIBP ; -else - mask &= ~SPEC_CTRL_STIBP ;if (mask != x86_spec_ctrl_base) {pr_info ("Spectre v2 cross-process SMT mitigation: %s STIBP\n" , - cpu_smt_control ==CPU_SMT_ENABLED ? -"Enabling" :"Disabling" ); + mask &SPEC_CTRL_STIBP ?"Enabling" :"Disabling" ); x86_spec_ctrl_base = mask;on_each_cpu (update_stibp_msr,NULL ,1 ); }
include/linux/sched/smt.h +2 -0
@@ -15,4 +15,6 @@ static __always_inline bool sched_smt_acstatic inline bool sched_smt_active (void ) {return false; }#endif +void arch_smt_update (void ); +#endif
kernel/cpu.c +9 -6
@@ -10,6 +10,7 @@#include <linux/sched/signal.h> #include <linux/sched/hotplug.h> #include <linux/sched/task.h> +#include <linux/sched/smt.h> #include <linux/unistd.h> #include <linux/cpu.h> #include <linux/oom.h>
@@ -367,6 +368,12 @@ static void lockdep_release_cpus_lock(vo#endif /* CONFIG_HOTPLUG_CPU */ +/* +* Architectures that need SMT-specific errata handling during SMT hotplug +* should override this. +*/ +void __weak arch_smt_update (void ) { } +#ifdef CONFIG_HOTPLUG_SMT enum cpuhp_smt_control cpu_smt_control__read_mostly =CPU_SMT_ENABLED ;EXPORT_SYMBOL_GPL (cpu_smt_control);
@@ -1011,6 +1018,7 @@ static int __ref _cpu_down(unsigned int* concurrent CPU hotplug via cpu_add_remove_lock. */ lockup_detector_cleanup (); +arch_smt_update ();return ret; }
@@ -1139,6 +1147,7 @@ static int _cpu_up(unsigned int cpu, int ret =cpuhp_up_callbacks (cpu, st, target); out:cpus_write_unlock (); +arch_smt_update ();return ret; }
@@ -2055,12 +2064,6 @@ static void cpuhp_online_cpu_device(unsikobject_uevent (&dev->kobj,KOBJ_ONLINE ); } -/* -* Architectures that need SMT-specific errata handling during SMT hotplug -* should override this. -*/ -void __weak arch_smt_update (void ) { }; -static int cpuhp_smt_disable (enum cpuhp_smt_control ctrlval) {int cpu, ret =0 ;
tip-bot for Thomas Gleixner
merge bot[tip:x86/pti] x86/speculation: Rework SMT state change
x86/speculation: Rework SMT state change
Commit-ID: a74cfffb03b73d41e08f84c2e5c87dec0ce3db9f Gitweb: https://git.kernel.org/tip/a74cfffb03b73d41e08f84c2e5c87dec0ce3db9f Author: Thomas Gleixner <[email protected]> AuthorDate: Sun, 25 Nov 2018 19:33:39 +0100 Committer: Thomas Gleixner <[email protected]> CommitDate: Wed, 28 Nov 2018 11:57:07 +0100
x86/speculation: Rework SMT state change
arch_smt_update() is only called when the sysfs SMT control knob is changed. This means that when SMT is enabled in the sysfs control knob the system is considered to have SMT active even if all siblings are offline.
To allow finegrained control of the speculation mitigations, the actual SMT state is more interesting than the fact that siblings could be enabled.
Rework the code, so arch_smt_update() is invoked from each individual CPU hotplug function, and simplify the update function while at it.
--- arch/x86/kernel/cpu/bugs.c | 11 +++++------ include/linux/sched/smt.h | 2 ++ kernel/cpu.c | 15 +++++++++------ 3 files changed, 16 insertions(+), 12 deletions(-)
arch/x86/kernel/cpu/bugs.c +5 -6
@@ -14,6 +14,7 @@#include <linux/module.h> #include <linux/nospec.h> #include <linux/prctl.h> +#include <linux/sched/smt.h> #include <asm/spec-ctrl.h> #include <asm/cmdline.h>
@@ -344,16 +345,14 @@ void arch_smt_update(void)return ;mutex_lock (&spec_ctrl_mutex); - mask = x86_spec_ctrl_base; -if (cpu_smt_control ==CPU_SMT_ENABLED ) + + mask = x86_spec_ctrl_base & ~SPEC_CTRL_STIBP ; +if (sched_smt_active ()) mask |=SPEC_CTRL_STIBP ; -else - mask &= ~SPEC_CTRL_STIBP ;if (mask != x86_spec_ctrl_base) {pr_info ("Spectre v2 cross-process SMT mitigation: %s STIBP\n" , - cpu_smt_control ==CPU_SMT_ENABLED ? -"Enabling" :"Disabling" ); + mask &SPEC_CTRL_STIBP ?"Enabling" :"Disabling" ); x86_spec_ctrl_base = mask;on_each_cpu (update_stibp_msr,NULL ,1 ); }
include/linux/sched/smt.h +2 -0
@@ -15,4 +15,6 @@ static __always_inline bool sched_smt_active(void)static inline bool sched_smt_active (void ) {return false; }#endif +void arch_smt_update (void ); +#endif
kernel/cpu.c +9 -6
@@ -10,6 +10,7 @@#include <linux/sched/signal.h> #include <linux/sched/hotplug.h> #include <linux/sched/task.h> +#include <linux/sched/smt.h> #include <linux/unistd.h> #include <linux/cpu.h> #include <linux/oom.h>
@@ -367,6 +368,12 @@ static void lockdep_release_cpus_lock(void)#endif /* CONFIG_HOTPLUG_CPU */ +/* +* Architectures that need SMT-specific errata handling during SMT hotplug +* should override this. +*/ +void __weak arch_smt_update (void ) { } +#ifdef CONFIG_HOTPLUG_SMT enum cpuhp_smt_control cpu_smt_control__read_mostly =CPU_SMT_ENABLED ;EXPORT_SYMBOL_GPL (cpu_smt_control);
@@ -1011,6 +1018,7 @@ out:* concurrent CPU hotplug via cpu_add_remove_lock. */ lockup_detector_cleanup (); +arch_smt_update ();return ret; }
@@ -1139,6 +1147,7 @@ static int _cpu_up(unsigned int cpu, int tasks_frozen, enum cpuhp_state target) ret =cpuhp_up_callbacks (cpu, st, target); out:cpus_write_unlock (); +arch_smt_update ();return ret; }
@@ -2055,12 +2064,6 @@ static void cpuhp_online_cpu_device(unsigned int cpu)kobject_uevent (&dev->kobj,KOBJ_ONLINE ); } -/* -* Architectures that need SMT-specific errata handling during SMT hotplug -* should override this. -*/ -void __weak arch_smt_update (void ) { }; -static int cpuhp_smt_disable (enum cpuhp_smt_control ctrlval) {int cpu, ret =0 ;
PATCH v2 12/28 x86/l1tf: Show actual SMT state
In reply to Thomas GleixnerUse the now exposed real SMT state, not the SMT sysfs control knob state. This reflects the state of the system when the mitigation status is queried.
This does not change the warning in the VMX launch code. There the dependency on the control knob makes sense because siblings could be brought online anytime after launching the VM.
--- arch/x86/kernel/cpu/bugs.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
arch/x86/kernel/cpu/bugs.c +3 -2
@@ -832,13 +832,14 @@ static ssize_t l1tf_show_state(char *bufif (l1tf_vmx_mitigation ==VMENTER_L1D_FLUSH_EPT_DISABLED || (l1tf_vmx_mitigation ==VMENTER_L1D_FLUSH_NEVER && - cpu_smt_control ==CPU_SMT_ENABLED )) +sched_smt_active ())) {return sprintf (buf,"%s; VMX: %s\n" ,L1TF_DEFAULT_MSG , l1tf_vmx_states[l1tf_vmx_mitigation]); + }return sprintf (buf,"%s; VMX: %s, SMT %s\n" ,L1TF_DEFAULT_MSG , l1tf_vmx_states[l1tf_vmx_mitigation], - cpu_smt_control == CPU_SMT_ENABLED ? "vulnerable" : "disabled"); + sched_smt_active() ? "vulnerable" : "disabled"); }#else static ssize_t l1tf_show_state (char *buf)
tip-bot for Thomas Gleixner
merge bot[tip:x86/pti] x86/l1tf: Show actual SMT state
x86/l1tf: Show actual SMT state
Commit-ID: 130d6f946f6f2a972ee3ec8540b7243ab99abe97 Gitweb: https://git.kernel.org/tip/130d6f946f6f2a972ee3ec8540b7243ab99abe97 Author: Thomas Gleixner <[email protected]> AuthorDate: Sun, 25 Nov 2018 19:33:40 +0100 Committer: Thomas Gleixner <[email protected]> CommitDate: Wed, 28 Nov 2018 11:57:08 +0100
x86/l1tf: Show actual SMT state
Use the now exposed real SMT state, not the SMT sysfs control knob state. This reflects the state of the system when the mitigation status is queried.
This does not change the warning in the VMX launch code. There the dependency on the control knob makes sense because siblings could be brought online anytime after launching the VM.
--- arch/x86/kernel/cpu/bugs.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
arch/x86/kernel/cpu/bugs.c +3 -2
@@ -832,13 +832,14 @@ static ssize_t l1tf_show_state(char *buf)if (l1tf_vmx_mitigation ==VMENTER_L1D_FLUSH_EPT_DISABLED || (l1tf_vmx_mitigation ==VMENTER_L1D_FLUSH_NEVER && - cpu_smt_control ==CPU_SMT_ENABLED )) +sched_smt_active ())) {return sprintf (buf,"%s; VMX: %s\n" ,L1TF_DEFAULT_MSG , l1tf_vmx_states[l1tf_vmx_mitigation]); + }return sprintf (buf,"%s; VMX: %s, SMT %s\n" ,L1TF_DEFAULT_MSG , l1tf_vmx_states[l1tf_vmx_mitigation], - cpu_smt_control == CPU_SMT_ENABLED ? "vulnerable" : "disabled"); + sched_smt_active() ? "vulnerable" : "disabled"); }#else static ssize_t l1tf_show_state (char *buf)
PATCH v2 13/28 x86/speculation: Reorder the spec_v2 code
In reply to Thomas GleixnerReorder the code so it is better grouped.
--- arch/x86/kernel/cpu/bugs.c | 168 ++++++++++++++++++++++----------------------- 1 file changed, 84 insertions(+), 84 deletions(-)
arch/x86/kernel/cpu/bugs.c +84 -84
@@ -124,29 +124,6 @@ void __init check_bugs(void)#endif } -/* The kernel command line selection */ -enum spectre_v2_mitigation_cmd { -SPECTRE_V2_CMD_NONE , -SPECTRE_V2_CMD_AUTO , -SPECTRE_V2_CMD_FORCE , -SPECTRE_V2_CMD_RETPOLINE , -SPECTRE_V2_CMD_RETPOLINE_GENERIC , -SPECTRE_V2_CMD_RETPOLINE_AMD , -}; - -static const char *spectre_v2_strings[] = { - [SPECTRE_V2_NONE ] ="Vulnerable" , - [SPECTRE_V2_RETPOLINE_GENERIC ] ="Mitigation: Full generic retpoline" , - [SPECTRE_V2_RETPOLINE_AMD ] ="Mitigation: Full AMD retpoline" , - [SPECTRE_V2_IBRS_ENHANCED ] ="Mitigation: Enhanced IBRS" , -}; - -#undef pr_fmt -#define pr_fmt (fmt) "Spectre V2 : " fmt - -static enum spectre_v2_mitigation spectre_v2_enabled__ro_after_init = -SPECTRE_V2_NONE ; -void x86_virt_spec_ctrl (u64 guest_spec_ctrl,u64 guest_virt_spec_ctrl,bool setguest) {
@@ -216,6 +193,12 @@ static void x86_amd_ssb_disable(void)wrmsrl (MSR_AMD64_LS_CFG , msrval); } +#undef pr_fmt +#define pr_fmt (fmt) "Spectre V2 : " fmt + +static enum spectre_v2_mitigation spectre_v2_enabled__ro_after_init = +SPECTRE_V2_NONE ; +#ifdef RETPOLINE static bool spectre_v2_bad_module;
@@ -237,18 +220,6 @@ static inline const char *spectre_v2_modstatic inline const char *spectre_v2_module_string (void ) {return "" ; }#endif -static void __init spec2_print_if_insecure (const char *reason) -{ -if (boot_cpu_has_bug (X86_BUG_SPECTRE_V2 )) -pr_info ("%s selected on command line.\n" , reason); -} - -static void __init spec2_print_if_secure (const char *reason) -{ -if (!boot_cpu_has_bug (X86_BUG_SPECTRE_V2 )) -pr_info ("%s selected on command line.\n" , reason); -} -static inline bool match_option (const char *arg,int arglen,const char *opt) {int len =strlen (opt);
@@ -256,24 +227,53 @@ static inline bool match_option(const chreturn len == arglen && !strncmp (arg, opt, len); } +/* The kernel command line selection for spectre v2 */ +enum spectre_v2_mitigation_cmd { +SPECTRE_V2_CMD_NONE , +SPECTRE_V2_CMD_AUTO , +SPECTRE_V2_CMD_FORCE , +SPECTRE_V2_CMD_RETPOLINE , +SPECTRE_V2_CMD_RETPOLINE_GENERIC , +SPECTRE_V2_CMD_RETPOLINE_AMD , +}; + +static const char *spectre_v2_strings[] = { + [SPECTRE_V2_NONE ] ="Vulnerable" , + [SPECTRE_V2_RETPOLINE_GENERIC ] ="Mitigation: Full generic retpoline" , + [SPECTRE_V2_RETPOLINE_AMD ] ="Mitigation: Full AMD retpoline" , + [SPECTRE_V2_IBRS_ENHANCED ] ="Mitigation: Enhanced IBRS" , +}; +static const struct {const char *option;enum spectre_v2_mitigation_cmd cmd;bool secure; } mitigation_options[] = { - {"off" ,SPECTRE_V2_CMD_NONE , false }, - {"on" ,SPECTRE_V2_CMD_FORCE , true }, - {"retpoline" ,SPECTRE_V2_CMD_RETPOLINE , false }, - {"retpoline,amd" ,SPECTRE_V2_CMD_RETPOLINE_AMD , false }, - {"retpoline,generic" ,SPECTRE_V2_CMD_RETPOLINE_GENERIC , false }, - {"auto" ,SPECTRE_V2_CMD_AUTO , false }, + {"off" ,SPECTRE_V2_CMD_NONE , false }, + {"on" ,SPECTRE_V2_CMD_FORCE , true }, + {"retpoline" ,SPECTRE_V2_CMD_RETPOLINE , false }, + {"retpoline,amd" ,SPECTRE_V2_CMD_RETPOLINE_AMD , false }, + {"retpoline,generic" ,SPECTRE_V2_CMD_RETPOLINE_GENERIC , false }, + {"auto" ,SPECTRE_V2_CMD_AUTO , false }, }; +static void __init spec2_print_if_insecure (const char *reason) +{ +if (boot_cpu_has_bug (X86_BUG_SPECTRE_V2 )) +pr_info ("%s selected on command line.\n" , reason); +} + +static void __init spec2_print_if_secure (const char *reason) +{ +if (!boot_cpu_has_bug (X86_BUG_SPECTRE_V2 )) +pr_info ("%s selected on command line.\n" , reason); +} +static enum spectre_v2_mitigation_cmd __init spectre_v2_parse_cmdline (void ) { +enum spectre_v2_mitigation_cmd cmd =SPECTRE_V2_CMD_AUTO ;char arg[20 ];int ret, i; -enum spectre_v2_mitigation_cmd cmd =SPECTRE_V2_CMD_AUTO ;if (cmdline_find_option_bool (boot_command_line,"nospectre_v2" ))return SPECTRE_V2_CMD_NONE ;
@@ -317,48 +317,6 @@ static enum spectre_v2_mitigation_cmd __return cmd; } -static bool stibp_needed (void ) -{ -if (spectre_v2_enabled ==SPECTRE_V2_NONE ) -return false; - -/* Enhanced IBRS makes using STIBP unnecessary. */ -if (spectre_v2_enabled ==SPECTRE_V2_IBRS_ENHANCED ) -return false; - -if (!boot_cpu_has (X86_FEATURE_STIBP )) -return false; - -return true; -} - -static void update_stibp_msr (void *info) -{ -wrmsrl (MSR_IA32_SPEC_CTRL , x86_spec_ctrl_base); -} - -void arch_smt_update (void ) -{ -u64 mask; - -if (!stibp_needed ()) -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 ); - } -mutex_unlock (&spec_ctrl_mutex); -} -static void __init spectre_v2_select_mitigation (void ) {enum spectre_v2_mitigation_cmd cmd =spectre_v2_parse_cmdline ();
@@ -462,6 +420,48 @@ static void __init spectre_v2_select_mitarch_smt_update (); } +static bool stibp_needed (void ) +{ +if (spectre_v2_enabled ==SPECTRE_V2_NONE ) +return false; + +/* Enhanced IBRS makes using STIBP unnecessary. */ +if (spectre_v2_enabled ==SPECTRE_V2_IBRS_ENHANCED ) +return false; + +if (!boot_cpu_has (X86_FEATURE_STIBP )) +return false; + +return true; +} + +static void update_stibp_msr (void *info) +{ +wrmsrl (MSR_IA32_SPEC_CTRL , x86_spec_ctrl_base); +} + +void arch_smt_update (void ) +{ +u64 mask; + +if (!stibp_needed ()) +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 ); + } +mutex_unlock (&spec_ctrl_mutex); +} +#undef pr_fmt#define pr_fmt (fmt) "Speculative Store Bypass: " fmt
Borislav Petkov
maintainerRe: [patch V2 13/28] x86/speculation: Reorder the spec_v2 code
Quoted 7 lines · Thomas Gleixner
Reorder the code so it is better grouped.
Signed-off-by Thomas Gleixner <[email protected]>--- arch/x86/kernel/cpu/bugs.c | 168 ++++++++++++++++++++++----------------------- 1 file changed, 84 insertions(+), 84 deletions(-)
...
arch/x86/kernel/cpu/bugs.c static inline bool match_option(const ch · line 251
Quoted 5 lines
static const struct {
arch/x86/kernel/cpu/bugs.c +0
-0const char *option;enum spectre_v2_mitigation_cmd cmd;bool secure; } mitigation_options[] = {
Yeah, now that we have ssb_mitigation_options and v2_user_options also, those should probably be called "spectre_v2_options" or something more specific to have the code more clear which options the code is using...
Quoted 13 lines
arch/x86/kernel/cpu/bugs.c +6
-6- { "off", SPECTRE_V2_CMD_NONE, false }, - { "on", SPECTRE_V2_CMD_FORCE, true }, - { "retpoline", SPECTRE_V2_CMD_RETPOLINE, false }, - { "retpoline,amd", SPECTRE_V2_CMD_RETPOLINE_AMD, false }, - { "retpoline,generic", SPECTRE_V2_CMD_RETPOLINE_GENERIC, false }, - { "auto", SPECTRE_V2_CMD_AUTO, false }, + { "off", SPECTRE_V2_CMD_NONE, false }, + { "on", SPECTRE_V2_CMD_FORCE, true }, + { "retpoline", SPECTRE_V2_CMD_RETPOLINE, false }, + { "retpoline,amd", SPECTRE_V2_CMD_RETPOLINE_AMD, false }, + { "retpoline,generic", SPECTRE_V2_CMD_RETPOLINE_GENERIC, false }, + { "auto", SPECTRE_V2_CMD_AUTO, false }, };
Signature · 4 lines
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.tip-bot for Thomas Gleixner
merge bot[tip:x86/pti] x86/speculation: Reorder the spec_v2 code
In reply to Thomas Gleixnerx86/speculation: Reorder the spec_v2 code
Commit-ID: 15d6b7aab0793b2de8a05d8a828777dd24db424e Gitweb: https://git.kernel.org/tip/15d6b7aab0793b2de8a05d8a828777dd24db424e Author: Thomas Gleixner <[email protected]> AuthorDate: Sun, 25 Nov 2018 19:33:41 +0100 Committer: Thomas Gleixner <[email protected]> CommitDate: Wed, 28 Nov 2018 11:57:08 +0100
x86/speculation: Reorder the spec_v2 code
Reorder the code so it is better grouped. No functional change.
--- arch/x86/kernel/cpu/bugs.c | 168 ++++++++++++++++++++++----------------------- 1 file changed, 84 insertions(+), 84 deletions(-)
arch/x86/kernel/cpu/bugs.c +84 -84
@@ -124,29 +124,6 @@ void __init check_bugs(void)#endif } -/* The kernel command line selection */ -enum spectre_v2_mitigation_cmd { -SPECTRE_V2_CMD_NONE , -SPECTRE_V2_CMD_AUTO , -SPECTRE_V2_CMD_FORCE , -SPECTRE_V2_CMD_RETPOLINE , -SPECTRE_V2_CMD_RETPOLINE_GENERIC , -SPECTRE_V2_CMD_RETPOLINE_AMD , -}; - -static const char *spectre_v2_strings[] = { - [SPECTRE_V2_NONE ] ="Vulnerable" , - [SPECTRE_V2_RETPOLINE_GENERIC ] ="Mitigation: Full generic retpoline" , - [SPECTRE_V2_RETPOLINE_AMD ] ="Mitigation: Full AMD retpoline" , - [SPECTRE_V2_IBRS_ENHANCED ] ="Mitigation: Enhanced IBRS" , -}; - -#undef pr_fmt -#define pr_fmt (fmt) "Spectre V2 : " fmt - -static enum spectre_v2_mitigation spectre_v2_enabled__ro_after_init = -SPECTRE_V2_NONE ; -void x86_virt_spec_ctrl (u64 guest_spec_ctrl,u64 guest_virt_spec_ctrl,bool setguest) {
@@ -216,6 +193,12 @@ static void x86_amd_ssb_disable(void)wrmsrl (MSR_AMD64_LS_CFG , msrval); } +#undef pr_fmt +#define pr_fmt (fmt) "Spectre V2 : " fmt + +static enum spectre_v2_mitigation spectre_v2_enabled__ro_after_init = +SPECTRE_V2_NONE ; +#ifdef RETPOLINE static bool spectre_v2_bad_module;
@@ -237,18 +220,6 @@ static inline const char *spectre_v2_module_string(void)static inline const char *spectre_v2_module_string (void ) {return "" ; }#endif -static void __init spec2_print_if_insecure (const char *reason) -{ -if (boot_cpu_has_bug (X86_BUG_SPECTRE_V2 )) -pr_info ("%s selected on command line.\n" , reason); -} - -static void __init spec2_print_if_secure (const char *reason) -{ -if (!boot_cpu_has_bug (X86_BUG_SPECTRE_V2 )) -pr_info ("%s selected on command line.\n" , reason); -} -static inline bool match_option (const char *arg,int arglen,const char *opt) {int len =strlen (opt);
@@ -256,24 +227,53 @@ static inline bool match_option(const char *arg, int arglen, const char *opt)return len == arglen && !strncmp (arg, opt, len); } +/* The kernel command line selection for spectre v2 */ +enum spectre_v2_mitigation_cmd { +SPECTRE_V2_CMD_NONE , +SPECTRE_V2_CMD_AUTO , +SPECTRE_V2_CMD_FORCE , +SPECTRE_V2_CMD_RETPOLINE , +SPECTRE_V2_CMD_RETPOLINE_GENERIC , +SPECTRE_V2_CMD_RETPOLINE_AMD , +}; + +static const char *spectre_v2_strings[] = { + [SPECTRE_V2_NONE ] ="Vulnerable" , + [SPECTRE_V2_RETPOLINE_GENERIC ] ="Mitigation: Full generic retpoline" , + [SPECTRE_V2_RETPOLINE_AMD ] ="Mitigation: Full AMD retpoline" , + [SPECTRE_V2_IBRS_ENHANCED ] ="Mitigation: Enhanced IBRS" , +}; +static const struct {const char *option;enum spectre_v2_mitigation_cmd cmd;bool secure; } mitigation_options[] = { - {"off" ,SPECTRE_V2_CMD_NONE , false }, - {"on" ,SPECTRE_V2_CMD_FORCE , true }, - {"retpoline" ,SPECTRE_V2_CMD_RETPOLINE , false }, - {"retpoline,amd" ,SPECTRE_V2_CMD_RETPOLINE_AMD , false }, - {"retpoline,generic" ,SPECTRE_V2_CMD_RETPOLINE_GENERIC , false }, - {"auto" ,SPECTRE_V2_CMD_AUTO , false }, + {"off" ,SPECTRE_V2_CMD_NONE , false }, + {"on" ,SPECTRE_V2_CMD_FORCE , true }, + {"retpoline" ,SPECTRE_V2_CMD_RETPOLINE , false }, + {"retpoline,amd" ,SPECTRE_V2_CMD_RETPOLINE_AMD , false }, + {"retpoline,generic" ,SPECTRE_V2_CMD_RETPOLINE_GENERIC , false }, + {"auto" ,SPECTRE_V2_CMD_AUTO , false }, }; +static void __init spec2_print_if_insecure (const char *reason) +{ +if (boot_cpu_has_bug (X86_BUG_SPECTRE_V2 )) +pr_info ("%s selected on command line.\n" , reason); +} + +static void __init spec2_print_if_secure (const char *reason) +{ +if (!boot_cpu_has_bug (X86_BUG_SPECTRE_V2 )) +pr_info ("%s selected on command line.\n" , reason); +} +static enum spectre_v2_mitigation_cmd __init spectre_v2_parse_cmdline (void ) { +enum spectre_v2_mitigation_cmd cmd =SPECTRE_V2_CMD_AUTO ;char arg[20 ];int ret, i; -enum spectre_v2_mitigation_cmd cmd =SPECTRE_V2_CMD_AUTO ;if (cmdline_find_option_bool (boot_command_line,"nospectre_v2" ))return SPECTRE_V2_CMD_NONE ;
@@ -317,48 +317,6 @@ static enum spectre_v2_mitigation_cmd __init spectre_v2_parse_cmdline(void)return cmd; } -static bool stibp_needed (void ) -{ -if (spectre_v2_enabled ==SPECTRE_V2_NONE ) -return false; - -/* Enhanced IBRS makes using STIBP unnecessary. */ -if (spectre_v2_enabled ==SPECTRE_V2_IBRS_ENHANCED ) -return false; - -if (!boot_cpu_has (X86_FEATURE_STIBP )) -return false; - -return true; -} - -static void update_stibp_msr (void *info) -{ -wrmsrl (MSR_IA32_SPEC_CTRL , x86_spec_ctrl_base); -} - -void arch_smt_update (void ) -{ -u64 mask; - -if (!stibp_needed ()) -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 ); - } -mutex_unlock (&spec_ctrl_mutex); -} -static void __init spectre_v2_select_mitigation (void ) {enum spectre_v2_mitigation_cmd cmd =spectre_v2_parse_cmdline ();
@@ -462,6 +420,48 @@ specv2_set_mode:arch_smt_update (); } +static bool stibp_needed (void ) +{ +if (spectre_v2_enabled ==SPECTRE_V2_NONE ) +return false; + +/* Enhanced IBRS makes using STIBP unnecessary. */ +if (spectre_v2_enabled ==SPECTRE_V2_IBRS_ENHANCED ) +return false; + +if (!boot_cpu_has (X86_FEATURE_STIBP )) +return false; + +return true; +} + +static void update_stibp_msr (void *info) +{ +wrmsrl (MSR_IA32_SPEC_CTRL , x86_spec_ctrl_base); +} + +void arch_smt_update (void ) +{ +u64 mask; + +if (!stibp_needed ()) +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 ); + } +mutex_unlock (&spec_ctrl_mutex); +} +#undef pr_fmt#define pr_fmt (fmt) "Speculative Store Bypass: " fmt
PATCH v2 14/28 x86/speculation: Mark string arrays const correctly
In reply to Thomas Gleixnercheckpatch.pl muttered when reshuffling the code:
WARNING: static const char * array should probably be static const char * const
Fix up all the string arrays.
--- arch/x86/kernel/cpu/bugs.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
arch/x86/kernel/cpu/bugs.c +3 -3
@@ -237,7 +237,7 @@ enum spectre_v2_mitigation_cmd { SPECTRE_V2_CMD_RETPOLINE_AMD, }; -static const char *spectre_v2_strings[] = { +static const char *const spectre_v2_strings[] = { [SPECTRE_V2_NONE ] ="Vulnerable" , [SPECTRE_V2_RETPOLINE_GENERIC ] ="Mitigation: Full generic retpoline" , [SPECTRE_V2_RETPOLINE_AMD ] ="Mitigation: Full AMD retpoline" ,
@@ -476,7 +476,7 @@ enum ssb_mitigation_cmd { SPEC_STORE_BYPASS_CMD_SECCOMP, }; -static const char *ssb_strings[] = { +static const char *const ssb_strings[] = { [SPEC_STORE_BYPASS_NONE ] ="Vulnerable" , [SPEC_STORE_BYPASS_DISABLE ] ="Mitigation: Speculative Store Bypass disabled" , [SPEC_STORE_BYPASS_PRCTL ] ="Mitigation: Speculative Store Bypass disabled via prctl" ,
@@ -816,7 +816,7 @@ early_param("l1tf", l1tf_cmdline);#define L1TF_DEFAULT_MSG "Mitigation: PTE Inversion"#if IS_ENABLED (CONFIG_KVM_INTEL ) -static const char *l1tf_vmx_states[] = { +static const char *const l1tf_vmx_states[] = { [VMENTER_L1D_FLUSH_AUTO ] ="auto" , [VMENTER_L1D_FLUSH_NEVER ] ="vulnerable" , [VMENTER_L1D_FLUSH_COND] = "conditional cache flushes",
tip-bot for Thomas Gleixner
merge bot[tip:x86/pti] x86/speculation: Mark string arrays const correctly
x86/speculation: Mark string arrays const correctly
Commit-ID: 8770709f411763884535662744a3786a1806afd3 Gitweb: https://git.kernel.org/tip/8770709f411763884535662744a3786a1806afd3 Author: Thomas Gleixner <[email protected]> AuthorDate: Sun, 25 Nov 2018 19:33:42 +0100 Committer: Thomas Gleixner <[email protected]> CommitDate: Wed, 28 Nov 2018 11:57:09 +0100
x86/speculation: Mark string arrays const correctly
checkpatch.pl muttered when reshuffling the code:
WARNING: static const char * array should probably be static const char * const
Fix up all the string arrays.
--- arch/x86/kernel/cpu/bugs.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
arch/x86/kernel/cpu/bugs.c +3 -3
@@ -237,7 +237,7 @@ enum spectre_v2_mitigation_cmd { SPECTRE_V2_CMD_RETPOLINE_AMD, }; -static const char *spectre_v2_strings[] = { +static const char *const spectre_v2_strings[] = { [SPECTRE_V2_NONE ] ="Vulnerable" , [SPECTRE_V2_RETPOLINE_GENERIC ] ="Mitigation: Full generic retpoline" , [SPECTRE_V2_RETPOLINE_AMD ] ="Mitigation: Full AMD retpoline" ,
@@ -476,7 +476,7 @@ enum ssb_mitigation_cmd { SPEC_STORE_BYPASS_CMD_SECCOMP, }; -static const char *ssb_strings[] = { +static const char *const ssb_strings[] = { [SPEC_STORE_BYPASS_NONE ] ="Vulnerable" , [SPEC_STORE_BYPASS_DISABLE ] ="Mitigation: Speculative Store Bypass disabled" , [SPEC_STORE_BYPASS_PRCTL ] ="Mitigation: Speculative Store Bypass disabled via prctl" ,
@@ -816,7 +816,7 @@ early_param("l1tf", l1tf_cmdline);#define L1TF_DEFAULT_MSG "Mitigation: PTE Inversion"#if IS_ENABLED (CONFIG_KVM_INTEL ) -static const char *l1tf_vmx_states[] = { +static const char *const l1tf_vmx_states[] = { [VMENTER_L1D_FLUSH_AUTO ] ="auto" , [VMENTER_L1D_FLUSH_NEVER ] ="vulnerable" , [VMENTER_L1D_FLUSH_COND] = "conditional cache flushes",
PATCH v2 15/28 x86/speculataion: Mark command line parser data __initdata
In reply to Thomas GleixnerNo point to keep that around.
--- arch/x86/kernel/cpu/bugs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
arch/x86/kernel/cpu/bugs.c +2 -2
@@ -248,7 +248,7 @@ static const struct {const char *option;enum spectre_v2_mitigation_cmd cmd;bool secure; -} mitigation_options[] = { +} mitigation_options[] __initdata = { { "off", SPECTRE_V2_CMD_NONE, false }, { "on", SPECTRE_V2_CMD_FORCE, true }, { "retpoline", SPECTRE_V2_CMD_RETPOLINE, false },
@@ -486,7 +486,7 @@ static const char * const ssb_strings[]static const struct {const char *option;enum ssb_mitigation_cmd cmd; -} ssb_mitigation_options[] = { +} ssb_mitigation_options[]__initdata = { {"auto" ,SPEC_STORE_BYPASS_CMD_AUTO },/* Platform decides */ {"on" ,SPEC_STORE_BYPASS_CMD_ON },/* Disable Speculative Store Bypass */ {"off" ,SPEC_STORE_BYPASS_CMD_NONE },/* Don't touch Speculative Store Bypass */
tip-bot for Thomas Gleixner
merge bot[tip:x86/pti] x86/speculataion: Mark command line parser data __initdata
x86/speculataion: Mark command line parser data __initdata
Commit-ID: 30ba72a990f5096ae08f284de17986461efcc408 Gitweb: https://git.kernel.org/tip/30ba72a990f5096ae08f284de17986461efcc408 Author: Thomas Gleixner <[email protected]> AuthorDate: Sun, 25 Nov 2018 19:33:43 +0100 Committer: Thomas Gleixner <[email protected]> CommitDate: Wed, 28 Nov 2018 11:57:09 +0100
x86/speculataion: Mark command line parser data __initdata
No point to keep that around.
--- arch/x86/kernel/cpu/bugs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
arch/x86/kernel/cpu/bugs.c +2 -2
@@ -248,7 +248,7 @@ static const struct {const char *option;enum spectre_v2_mitigation_cmd cmd;bool secure; -} mitigation_options[] = { +} mitigation_options[] __initdata = { { "off", SPECTRE_V2_CMD_NONE, false }, { "on", SPECTRE_V2_CMD_FORCE, true }, { "retpoline", SPECTRE_V2_CMD_RETPOLINE, false },
@@ -486,7 +486,7 @@ static const char * const ssb_strings[] = {static const struct {const char *option;enum ssb_mitigation_cmd cmd; -} ssb_mitigation_options[] = { +} ssb_mitigation_options[]__initdata = { {"auto" ,SPEC_STORE_BYPASS_CMD_AUTO },/* Platform decides */ {"on" ,SPEC_STORE_BYPASS_CMD_ON },/* Disable Speculative Store Bypass */ {"off" ,SPEC_STORE_BYPASS_CMD_NONE },/* Don't touch Speculative Store Bypass */
PATCH v2 16/28 x86/speculation: Unify conditional spectre v2 print functions
In reply to Thomas GleixnerThere is no point in having two functions and a conditional at the call site.
--- arch/x86/kernel/cpu/bugs.c | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-)
arch/x86/kernel/cpu/bugs.c +4 -13
@@ -257,15 +257,9 @@ static const struct { { "auto", SPECTRE_V2_CMD_AUTO, false }, }; -static void __init spec2_print_if_insecure (const char *reason) +static void __init spec_v2_print_cond (const char *reason,bool secure) { -if (boot_cpu_has_bug (X86_BUG_SPECTRE_V2 )) -pr_info ("%s selected on command line.\n" , reason); -} - -static void __init spec2_print_if_secure (const char *reason) -{ -if (!boot_cpu_has_bug (X86_BUG_SPECTRE_V2 )) +if (boot_cpu_has_bug (X86_BUG_SPECTRE_V2 ) != secure)pr_info ("%s selected on command line.\n" , reason); }
@@ -309,11 +303,8 @@ static enum spectre_v2_mitigation_cmd __return SPECTRE_V2_CMD_AUTO ; } -if (mitigation_options[i].secure) -spec2_print_if_secure (mitigation_options[i].option); -else -spec2_print_if_insecure (mitigation_options[i].option); - +spec_v2_print_cond (mitigation_options[i].option, + mitigation_options[i].secure);return cmd; }
tip-bot for Thomas Gleixner
merge bot[tip:x86/pti] x86/speculation: Unify conditional spectre v2 print functions
x86/speculation: Unify conditional spectre v2 print functions
Commit-ID: 495d470e9828500e0155027f230449ac5e29c025 Gitweb: https://git.kernel.org/tip/495d470e9828500e0155027f230449ac5e29c025 Author: Thomas Gleixner <[email protected]> AuthorDate: Sun, 25 Nov 2018 19:33:44 +0100 Committer: Thomas Gleixner <[email protected]> CommitDate: Wed, 28 Nov 2018 11:57:09 +0100
x86/speculation: Unify conditional spectre v2 print functions
There is no point in having two functions and a conditional at the call site.
--- arch/x86/kernel/cpu/bugs.c | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-)
arch/x86/kernel/cpu/bugs.c +4 -13
@@ -257,15 +257,9 @@ static const struct { { "auto", SPECTRE_V2_CMD_AUTO, false }, }; -static void __init spec2_print_if_insecure (const char *reason) +static void __init spec_v2_print_cond (const char *reason,bool secure) { -if (boot_cpu_has_bug (X86_BUG_SPECTRE_V2 )) -pr_info ("%s selected on command line.\n" , reason); -} - -static void __init spec2_print_if_secure (const char *reason) -{ -if (!boot_cpu_has_bug (X86_BUG_SPECTRE_V2 )) +if (boot_cpu_has_bug (X86_BUG_SPECTRE_V2 ) != secure)pr_info ("%s selected on command line.\n" , reason); }
@@ -309,11 +303,8 @@ static enum spectre_v2_mitigation_cmd __init spectre_v2_parse_cmdline(void)return SPECTRE_V2_CMD_AUTO ; } -if (mitigation_options[i].secure) -spec2_print_if_secure (mitigation_options[i].option); -else -spec2_print_if_insecure (mitigation_options[i].option); - +spec_v2_print_cond (mitigation_options[i].option, + mitigation_options[i].secure);return cmd; }
PATCH v2 17/28 x86/speculation: Add command line control for indirect branch speculation
In reply to Thomas GleixnerAdd command line control for user space indirect branch speculation mitigations. The new option is: spectre_v2_user=
The initial options are:
- on: Unconditionally enabled
- off: Unconditionally disabled
-auto: Kernel selects mitigation (default off for now)When the spectre_v2= command line argument is either 'on' or 'off' this implies that the application to application control follows that state even if a contradicting spectre_v2_user= argument is supplied.
---
V1 -> V2: Change the option to spectre_v2_user=
--- Documentation/admin-guide/kernel-parameters.txt | 32 +++++ arch/x86/include/asm/nospec-branch.h | 10 + arch/x86/kernel/cpu/bugs.c | 133 ++++++++++++++++++++---- 3 files changed, 156 insertions(+), 19 deletions(-)
Documentation/admin-guide/kernel-parameters.txt +30 -2
@@ -4194,9 +4194,13 @@ spectre_v2= [X86] Control mitigation of Spectre variant 2 (indirect branch speculation) vulnerability. + The default operation protects the kernel from + user space attacks. - on - unconditionally enable - off - unconditionally disable + on - unconditionally enable, implies + spectre_v2_user=on + off - unconditionally disable, implies + spectre_v2_user=off auto - kernel detects whether your CPU model is vulnerable
@@ -4206,6 +4210,12 @@ CONFIG_RETPOLINE configuration option, and the compiler with which the kernel was built. + Selecting 'on' will also enable the mitigation + against user space to user space task attacks. + + Selecting 'off' will disable both the kernel and + the user space protections. + Specific mitigations can also be selected manually: retpoline - replace indirect branches
@@ -4215,6 +4225,24 @@ Not specifying this option is equivalent to spectre_v2=auto. + spectre_v2_user= + [X86] Control mitigation of Spectre variant 2 + (indirect branch speculation) vulnerability between + user space tasks + + on - Unconditionally enable mitigations. Is + enforced by spectre_v2=on + + off - Unconditionally disable mitigations. Is + enforced by spectre_v2=off + + auto - Kernel selects the mitigation depending on + the available CPU features and vulnerability. + Default is off. + + Not specifying this option is equivalent to + spectre_v2_user=auto. + spec_store_bypass_disable= [HW] Control Speculative Store Bypass (SSB) Disable mitigation (Speculative Store Bypass vulnerability)
arch/x86/include/asm/nospec-branch.h +10 -0
@@ -3,6 +3,8 @@#ifndef _ASM_X86_NOSPEC_BRANCH_H_#define _ASM_X86_NOSPEC_BRANCH_H_ +#include <linux/static_key.h> +#include <asm/alternative.h> #include <asm/alternative-asm.h> #include <asm/cpufeatures.h>
@@ -226,6 +228,12 @@ enum spectre_v2_mitigation { SPECTRE_V2_IBRS_ENHANCED, }; +/* The indirect branch speculation control variants */ +enum spectre_v2_user_mitigation { +SPECTRE_V2_USER_NONE , +SPECTRE_V2_USER_STRICT , +}; +/* The Speculative Store Bypass disable variants */ enum ssb_mitigation {SPEC_STORE_BYPASS_NONE ,
@@ -303,6 +311,8 @@ do { \ preempt_enable(); \ }while (0 ) +DECLARE_STATIC_KEY_FALSE (switch_to_cond_stibp); +#endif /* __ASSEMBLY__ */ /*
arch/x86/kernel/cpu/bugs.c +116 -17
179 lines not on this page · raw message
tip-bot for Thomas Gleixner
merge bot[tip:x86/pti] x86/speculation: Add command line control for indirect branch speculation
x86/speculation: Add command line control for indirect branch speculation
Commit-ID: fa1202ef224391b6f5b26cdd44cc50495e8fab54 Gitweb: https://git.kernel.org/tip/fa1202ef224391b6f5b26cdd44cc50495e8fab54 Author: Thomas Gleixner <[email protected]> AuthorDate: Sun, 25 Nov 2018 19:33:45 +0100 Committer: Thomas Gleixner <[email protected]> CommitDate: Wed, 28 Nov 2018 11:57:10 +0100
x86/speculation: Add command line control for indirect branch speculation
Add command line control for user space indirect branch speculation mitigations. The new option is: spectre_v2_user=
The initial options are:
- on: Unconditionally enabled
- off: Unconditionally disabled
-auto: Kernel selects mitigation (default off for now)When the spectre_v2= command line argument is either 'on' or 'off' this implies that the application to application control follows that state even if a contradicting spectre_v2_user= argument is supplied.
--- Documentation/admin-guide/kernel-parameters.txt | 32 +++++- arch/x86/include/asm/nospec-branch.h | 10 ++ arch/x86/kernel/cpu/bugs.c | 133 +++++++++++++++++++++--- 3 files changed, 156 insertions(+), 19 deletions(-)
Documentation/admin-guide/kernel-parameters.txt +30 -2
@@ -4194,9 +4194,13 @@ spectre_v2= [X86] Control mitigation of Spectre variant 2 (indirect branch speculation) vulnerability. + The default operation protects the kernel from + user space attacks. - on - unconditionally enable - off - unconditionally disable + on - unconditionally enable, implies + spectre_v2_user=on + off - unconditionally disable, implies + spectre_v2_user=off auto - kernel detects whether your CPU model is vulnerable
@@ -4206,6 +4210,12 @@ CONFIG_RETPOLINE configuration option, and the compiler with which the kernel was built. + Selecting 'on' will also enable the mitigation + against user space to user space task attacks. + + Selecting 'off' will disable both the kernel and + the user space protections. + Specific mitigations can also be selected manually: retpoline - replace indirect branches
@@ -4215,6 +4225,24 @@ Not specifying this option is equivalent to spectre_v2=auto. + spectre_v2_user= + [X86] Control mitigation of Spectre variant 2 + (indirect branch speculation) vulnerability between + user space tasks + + on - Unconditionally enable mitigations. Is + enforced by spectre_v2=on + + off - Unconditionally disable mitigations. Is + enforced by spectre_v2=off + + auto - Kernel selects the mitigation depending on + the available CPU features and vulnerability. + Default is off. + + Not specifying this option is equivalent to + spectre_v2_user=auto. + spec_store_bypass_disable= [HW] Control Speculative Store Bypass (SSB) Disable mitigation (Speculative Store Bypass vulnerability)
arch/x86/include/asm/nospec-branch.h +10 -0
@@ -3,6 +3,8 @@#ifndef _ASM_X86_NOSPEC_BRANCH_H_#define _ASM_X86_NOSPEC_BRANCH_H_ +#include <linux/static_key.h> +#include <asm/alternative.h> #include <asm/alternative-asm.h> #include <asm/cpufeatures.h>
@@ -226,6 +228,12 @@ enum spectre_v2_mitigation { SPECTRE_V2_IBRS_ENHANCED, }; +/* The indirect branch speculation control variants */ +enum spectre_v2_user_mitigation { +SPECTRE_V2_USER_NONE , +SPECTRE_V2_USER_STRICT , +}; +/* The Speculative Store Bypass disable variants */ enum ssb_mitigation {SPEC_STORE_BYPASS_NONE ,
@@ -303,6 +311,8 @@ do { \ preempt_enable(); \ }while (0 ) +DECLARE_STATIC_KEY_FALSE (switch_to_cond_stibp); +#endif /* __ASSEMBLY__ */ /*
arch/x86/kernel/cpu/bugs.c +116 -17
179 lines not on this page · raw message
PATCH v2 18/28 x86/speculation: Prepare for per task indirect branch speculation control
In reply to Thomas GleixnerTo avoid the overhead of STIBP always on, it's necessary to allow per task control of STIBP.
Add a new task flag TIF_SPEC_IB and evaluate it during context switch if SMT is active and flag evaluation is enabled by the speculation control code. Add the conditional evaluation to x86_virt_spec_ctrl() as well so the guest/host switch works properly.
This has no effect because TIF_SPEC_IB cannot be set yet and the static key which controls evaluation is off. Preparatory patch for adding the control code.
[ tglx: Simplify the context switch logic and make the TIF evaluation
depend on SMP=y and on the static key controlling the conditional update. Rename it to TIF_SPEC_IB because it controls both STIBP and IBPB ]
---
v1 -> v2: Remove pointless include. Use consistent comments.
--- arch/x86/include/asm/msr-index.h | 5 +++-- arch/x86/include/asm/spec-ctrl.h | 12 ++++++++++++ arch/x86/include/asm/thread_info.h | 5 ++++- arch/x86/kernel/cpu/bugs.c | 4 ++++ arch/x86/kernel/process.c | 23 +++++++++++++++++++++-- 5 files changed, 44 insertions(+), 5 deletions(-)
arch/x86/include/asm/msr-index.h +3 -2
@@ -41,9 +41,10 @@#define MSR_IA32_SPEC_CTRL 0x00000048/* Speculation Control */ #define SPEC_CTRL_IBRS (1 << 0)/* Indirect Branch Restricted Speculation */ -#define SPEC_CTRL_STIBP (1 << 1)/* Single Thread Indirect Branch Predictors */ +#define SPEC_CTRL_STIBP_SHIFT 1/* Single Thread Indirect Branch Predictor (STIBP) bit */ +#define SPEC_CTRL_STIBP (1 << SPEC_CTRL_STIBP_SHIFT)/* STIBP mask */ #define SPEC_CTRL_SSBD_SHIFT 2/* Speculative Store Bypass Disable bit */ -#define SPEC_CTRL_SSBD (1 << SPEC_CTRL_SSBD_SHIFT)/* Speculative Store Bypass Disable */ +#define SPEC_CTRL_SSBD (1 << SPEC_CTRL_SSBD_SHIFT)/* Speculative Store Bypass Disable */ #define MSR_IA32_PRED_CMD 0x00000049/* Prediction Command */ #define PRED_CMD_IBPB (1 << 0)/* Indirect Branch Prediction Barrier */
arch/x86/include/asm/spec-ctrl.h +12 -0
@@ -53,12 +53,24 @@ static inline u64 ssbd_tif_to_spec_ctrl(return (tifn & _TIF_SSBD) >> (TIF_SSBD -SPEC_CTRL_SSBD_SHIFT ); } +static inline u64 stibp_tif_to_spec_ctrl (u64 tifn) +{ +BUILD_BUG_ON (TIF_SPEC_IB <SPEC_CTRL_STIBP_SHIFT ); +return (tifn & _TIF_SPEC_IB) >> (TIF_SPEC_IB -SPEC_CTRL_STIBP_SHIFT ); +} +static inline unsigned long ssbd_spec_ctrl_to_tif (u64 spec_ctrl) {BUILD_BUG_ON (TIF_SSBD <SPEC_CTRL_SSBD_SHIFT );return (spec_ctrl &SPEC_CTRL_SSBD ) << (TIF_SSBD -SPEC_CTRL_SSBD_SHIFT ); } +static inline unsigned long stibp_spec_ctrl_to_tif (u64 spec_ctrl) +{ +BUILD_BUG_ON (TIF_SPEC_IB <SPEC_CTRL_STIBP_SHIFT ); +return (spec_ctrl &SPEC_CTRL_STIBP ) << (TIF_SPEC_IB -SPEC_CTRL_STIBP_SHIFT ); +} +static inline u64 ssbd_tif_to_amd_ls_cfg (u64 tifn) {return (tifn & _TIF_SSBD) ? x86_amd_ls_cfg_ssbd_mask :0ULL ;
arch/x86/include/asm/thread_info.h +4 -1
@@ -83,6 +83,7 @@ struct thread_info {#define TIF_SYSCALL_EMU 6/* syscall emulation active */ #define TIF_SYSCALL_AUDIT 7/* syscall auditing active */ #define TIF_SECCOMP 8/* secure computing */ +#define TIF_SPEC_IB 9/* Indirect branch speculation mitigation */ #define TIF_USER_RETURN_NOTIFY 11/* notify kernel of userspace return */ #define TIF_UPROBE 12/* breakpointed or singlestepping */ #define TIF_PATCH_PENDING 13/* pending live patching update */
@@ -110,6 +111,7 @@ struct thread_info {#define _TIF_SYSCALL_EMU (1 << TIF_SYSCALL_EMU)#define _TIF_SYSCALL_AUDIT (1 << TIF_SYSCALL_AUDIT)#define _TIF_SECCOMP (1 << TIF_SECCOMP) +#define _TIF_SPEC_IB (1 << TIF_SPEC_IB)#define _TIF_USER_RETURN_NOTIFY (1 << TIF_USER_RETURN_NOTIFY)#define _TIF_UPROBE (1 << TIF_UPROBE)#define _TIF_PATCH_PENDING (1 << TIF_PATCH_PENDING)
@@ -146,7 +148,8 @@ struct thread_info {/* flags to check in __switch_to() */ #define _TIF_WORK_CTXSW \ - (_TIF_IO_BITMAP|_TIF_NOCPUID|_TIF_NOTSC|_TIF_BLOCKSTEP|_TIF_SSBD) + (_TIF_IO_BITMAP|_TIF_NOCPUID|_TIF_NOTSC|_TIF_BLOCKSTEP| \ + _TIF_SSBD|_TIF_SPEC_IB)#define _TIF_WORK_CTXSW_PREV (_TIF_WORK_CTXSW|_TIF_USER_RETURN_NOTIFY)#define _TIF_WORK_CTXSW_NEXT (_TIF_WORK_CTXSW)
arch/x86/kernel/cpu/bugs.c +4 -0
@@ -148,6 +148,10 @@ x86_virt_spec_ctrl(u64 guest_spec_ctrl, static_cpu_has(X86_FEATURE_AMD_SSBD)) hostval |=ssbd_tif_to_spec_ctrl (ti->flags); +/* Conditional STIBP enabled? */ +if (static_branch_unlikely (&switch_to_cond_stibp)) + hostval |=stibp_tif_to_spec_ctrl (ti->flags); +if (hostval != guestval) { msrval = setguest ? guestval : hostval;wrmsrl (MSR_IA32_SPEC_CTRL , msrval);
arch/x86/kernel/process.c +21 -2
@@ -406,6 +406,11 @@ static __always_inline void spec_ctrl_upif (static_cpu_has (X86_FEATURE_SSBD )) msr |=ssbd_tif_to_spec_ctrl (tifn); +/* Only evaluate if conditional STIBP is enabled */ +if (IS_ENABLED (CONFIG_SMP ) && +static_branch_unlikely (&switch_to_cond_stibp)) + msr |=stibp_tif_to_spec_ctrl (tifn); +wrmsrl (MSR_IA32_SPEC_CTRL , msr); }
@@ -418,10 +423,16 @@ static __always_inline void spec_ctrl_up static __always_inline void __speculation_ctrl_update(unsigned long tifp,unsigned long tifn) { +unsigned long tif_diff = tifp ^ tifn;bool updmsr = false; -/* If TIF_SSBD is different, select the proper mitigation method */ -if ((tifp ^ tifn) & _TIF_SSBD) { +/* +* If TIF_SSBD is different, select the proper mitigation +* method. Note that if SSBD mitigation is disabled or permanentely +* enabled this branch can't be taken because nothing can set +* TIF_SSBD. +*/ +if (tif_diff & _TIF_SSBD) {if (static_cpu_has (X86_FEATURE_VIRT_SSBD ))amd_set_ssb_virt_state (tifn);else if (static_cpu_has (X86_FEATURE_LS_CFG_SSBD ))
@@ -430,6 +441,14 @@ static __always_inline void __speculatio updmsr = true; } +/* +* Only evaluate TIF_SPEC_IB if conditional STIBP is enabled, +* otherwise avoid the MSR write. +*/ +if (IS_ENABLED (CONFIG_SMP ) && +static_branch_unlikely (&switch_to_cond_stibp)) + updmsr |= !!(tif_diff & _TIF_SPEC_IB); +if (updmsr)spec_ctrl_update_msr (tifn); }
Re: [patch V2 18/28] x86/speculation: Prepare for per task indirect branch speculation control
Quoted 118 lines · Thomas Gleixner
To avoid the overhead of STIBP always on, it's necessary to allow per task control of STIBP.
Add a new task flag
TIF_SPEC_IBand evaluate it during context switch if SMT is active and flag evaluation is enabled by the speculation control code. Add the conditional evaluation tox86_virt_spec_ctrl()as well so the guest/host switch works properly.This has no effect because
TIF_SPEC_IBcannot be set yet and the static key which controls evaluation is off. Preparatory patch for adding the control code.[ tglx: Simplify the context switch logic and make the TIF evaluation
depend on SMP=y and on the static key controlling the conditional update. Rename it toTIF_SPEC_IBbecause it controls both STIBP and IBPB ]Signed-off-by Tim Chen <[email protected]>Signed-off-by Thomas Gleixner <[email protected]>---
v1 -> v2: Remove pointless include. Use consistent comments.
--- arch/x86/include/asm/msr-index.h | 5 +++-- arch/x86/include/asm/spec-ctrl.h | 12 ++++++++++++ arch/x86/include/asm/thread_info.h | 5 ++++- arch/x86/kernel/cpu/bugs.c | 4 ++++ arch/x86/kernel/process.c | 23 +++++++++++++++++++++-- 5 files changed, 44 insertions(+), 5 deletions(-)arch/x86/include/asm/msr-index.h +3
-2@@ -41,9 +41,10 @@#define MSR_IA32_SPEC_CTRL 0x00000048/* Speculation Control */ #define SPEC_CTRL_IBRS (1 << 0)/* Indirect Branch Restricted Speculation */ -#define SPEC_CTRL_STIBP (1 << 1)/* Single Thread Indirect Branch Predictors */ +#define SPEC_CTRL_STIBP_SHIFT 1/* Single Thread Indirect Branch Predictor (STIBP) bit */ +#define SPEC_CTRL_STIBP (1 << SPEC_CTRL_STIBP_SHIFT)/* STIBP mask */ #define SPEC_CTRL_SSBD_SHIFT 2/* Speculative Store Bypass Disable bit */ -#define SPEC_CTRL_SSBD (1 << SPEC_CTRL_SSBD_SHIFT)/* Speculative Store Bypass Disable */ +#define SPEC_CTRL_SSBD (1 << SPEC_CTRL_SSBD_SHIFT)/* Speculative Store Bypass Disable */ #define MSR_IA32_PRED_CMD 0x00000049/* Prediction Command */ #define PRED_CMD_IBPB (1 << 0)/* Indirect Branch Prediction Barrier */ arch/x86/include/asm/spec-ctrl.h +12
-0@@ -53,12 +53,24 @@ static inline u64 ssbd_tif_to_spec_ctrl(return (tifn & _TIF_SSBD) >> (TIF_SSBD -SPEC_CTRL_SSBD_SHIFT ); } +static inline u64 stibp_tif_to_spec_ctrl (u64 tifn) +{ +BUILD_BUG_ON (TIF_SPEC_IB <SPEC_CTRL_STIBP_SHIFT ); +return (tifn & _TIF_SPEC_IB) >> (TIF_SPEC_IB -SPEC_CTRL_STIBP_SHIFT ); +} +static inline unsigned long ssbd_spec_ctrl_to_tif (u64 spec_ctrl) {BUILD_BUG_ON (TIF_SSBD <SPEC_CTRL_SSBD_SHIFT );return (spec_ctrl &SPEC_CTRL_SSBD ) << (TIF_SSBD -SPEC_CTRL_SSBD_SHIFT ); } +static inline unsigned long stibp_spec_ctrl_to_tif (u64 spec_ctrl) +{ +BUILD_BUG_ON (TIF_SPEC_IB <SPEC_CTRL_STIBP_SHIFT ); +return (spec_ctrl &SPEC_CTRL_STIBP ) << (TIF_SPEC_IB -SPEC_CTRL_STIBP_SHIFT ); +} +static inline u64 ssbd_tif_to_amd_ls_cfg (u64 tifn) {return (tifn & _TIF_SSBD) ? x86_amd_ls_cfg_ssbd_mask :0ULL ;arch/x86/include/asm/thread_info.h +4
-1@@ -83,6 +83,7 @@ struct thread_info {#define TIF_SYSCALL_EMU 6/* syscall emulation active */ #define TIF_SYSCALL_AUDIT 7/* syscall auditing active */ #define TIF_SECCOMP 8/* secure computing */ +#define TIF_SPEC_IB 9/* Indirect branch speculation mitigation */ #define TIF_USER_RETURN_NOTIFY 11/* notify kernel of userspace return */ #define TIF_UPROBE 12/* breakpointed or singlestepping */ #define TIF_PATCH_PENDING 13/* pending live patching update */ @@ -110,6 +111,7 @@ struct thread_info {#define _TIF_SYSCALL_EMU (1 << TIF_SYSCALL_EMU)#define _TIF_SYSCALL_AUDIT (1 << TIF_SYSCALL_AUDIT)#define _TIF_SECCOMP (1 << TIF_SECCOMP) +#define _TIF_SPEC_IB (1 << TIF_SPEC_IB)#define _TIF_USER_RETURN_NOTIFY (1 << TIF_USER_RETURN_NOTIFY)#define _TIF_UPROBE (1 << TIF_UPROBE)#define _TIF_PATCH_PENDING (1 << TIF_PATCH_PENDING)@@ -146,7 +148,8 @@ struct thread_info {/* flags to check in __switch_to() */ #define _TIF_WORK_CTXSW \ - (_TIF_IO_BITMAP|_TIF_NOCPUID|_TIF_NOTSC|_TIF_BLOCKSTEP|_TIF_SSBD) + (_TIF_IO_BITMAP|_TIF_NOCPUID|_TIF_NOTSC|_TIF_BLOCKSTEP| \ + _TIF_SSBD|_TIF_SPEC_IB)#define _TIF_WORK_CTXSW_PREV (_TIF_WORK_CTXSW|_TIF_USER_RETURN_NOTIFY)#define _TIF_WORK_CTXSW_NEXT (_TIF_WORK_CTXSW)arch/x86/kernel/cpu/bugs.c +4
-0@@ -148,6 +148,10 @@ x86_virt_spec_ctrl(u64 guest_spec_ctrl, static_cpu_has(X86_FEATURE_AMD_SSBD)) hostval |=ssbd_tif_to_spec_ctrl (ti->flags); +/* Conditional STIBP enabled? */ +if (static_branch_unlikely (&switch_to_cond_stibp)) + hostval |=stibp_tif_to_spec_ctrl (ti->flags); +if (hostval != guestval) { msrval = setguest ? guestval : hostval;wrmsrl (MSR_IA32_SPEC_CTRL , msrval);
arch/x86/kernel/process.c static __always_inline void spec_ctrl_up · line 407
Quoted 2 lines
arch/x86/kernel/process.c +0
-0@@ -406,6 +406,11 @@ static __always_inline void spec_ctrl_upif (static_cpu_has (X86_FEATURE_SSBD )) msr |=ssbd_tif_to_spec_ctrl (tifn);
I did some quick testing and found my original logic was flawed. Since spec_ctrl_update_msr() can now be called for STIBP, an additional check is needed to set the SSBD MSR bit.
Both X86_FEATURE_VIRT_SSBD and X86_FEATURE_LS_CFG_SSBD cause X86_FEATURE_SSBD to be set. Before this patch, spec_ctrl_update_msr() was only called if X86_FEATURE_SSBD was set and one of the other SSBD features wasn't set. But now, STIBP can cause spec_ctrl_update_msr() to get called and cause the SSBD MSR bit to be set when it shouldn't (could result in a GP fault).
Thanks,
Tom
Quoted 45 lines
arch/x86/kernel/process.c +21
-2+/* Only evaluate if conditional STIBP is enabled */ +if (IS_ENABLED (CONFIG_SMP ) && +static_branch_unlikely (&switch_to_cond_stibp)) + msr |=stibp_tif_to_spec_ctrl (tifn); +wrmsrl (MSR_IA32_SPEC_CTRL , msr); }@@ -418,10 +423,16 @@ static __always_inline void spec_ctrl_up static __always_inline void __speculation_ctrl_update(unsigned long tifp,unsigned long tifn) { +unsigned long tif_diff = tifp ^ tifn;bool updmsr = false; -/* If TIF_SSBD is different, select the proper mitigation method */ -if ((tifp ^ tifn) & _TIF_SSBD) { +/* +* If TIF_SSBD is different, select the proper mitigation +* method. Note that if SSBD mitigation is disabled or permanentely +* enabled this branch can't be taken because nothing can set +* TIF_SSBD. +*/ +if (tif_diff & _TIF_SSBD) {if (static_cpu_has (X86_FEATURE_VIRT_SSBD ))amd_set_ssb_virt_state (tifn);else if (static_cpu_has (X86_FEATURE_LS_CFG_SSBD ))@@ -430,6 +441,14 @@ static __always_inline void __speculatio updmsr = true; } +/* +* Only evaluate TIF_SPEC_IB if conditional STIBP is enabled, +* otherwise avoid the MSR write. +*/ +if (IS_ENABLED (CONFIG_SMP ) && +static_branch_unlikely (&switch_to_cond_stibp)) + updmsr |= !!(tif_diff & _TIF_SPEC_IB); +if (updmsr)spec_ctrl_update_msr (tifn); }
Re: [patch V2 18/28] x86/speculation: Prepare for per task indirect branch speculation control
Quoted 23 lines · Thomas
Quoted 16 lines
arch/x86/kernel/cpu/bugs.c +4
-0@@ -148,6 +148,10 @@ x86_virt_spec_ctrl(u64 guest_spec_ctrl, static_cpu_has(X86_FEATURE_AMD_SSBD)) hostval |=ssbd_tif_to_spec_ctrl (ti->flags); +/* Conditional STIBP enabled? */ +if (static_branch_unlikely (&switch_to_cond_stibp)) + hostval |=stibp_tif_to_spec_ctrl (ti->flags); +if (hostval != guestval) { msrval = setguest ? guestval : hostval;wrmsrl (MSR_IA32_SPEC_CTRL , msrval);arch/x86/kernel/process.c static __always_inline void spec_ctrl_up · line 407
Quoted 2 lines
arch/x86/kernel/process.c +0
-0@@ -406,6 +406,11 @@ static __always_inline void spec_ctrl_upif (static_cpu_has (X86_FEATURE_SSBD )) msr |=ssbd_tif_to_spec_ctrl (tifn);I did some quick testing and found my original logic was flawed. Since
spec_ctrl_update_msr()can now be called for STIBP, an additional check is needed to set the SSBD MSR bit.
Quoted 7 lines
Both
X86_FEATURE_VIRT_SSBDandX86_FEATURE_LS_CFG_SSBDcauseX86_FEATURE_SSBDto be set. Before this patch,spec_ctrl_update_msr()was only called ifX86_FEATURE_SSBDwas set and one of the other SSBD features wasn't set. But now, STIBP can causespec_ctrl_update_msr()to get called and cause the SSBD MSR bit to be set when it shouldn't (could result in a GP fault).
I think it will be cleaner just to fold the msr update into __speculation_ctrl_update to fix this issue.
Something like this perhaps.
Thanks.
Tim
---
arch/x86/kernel/process.c +8 -22
@@ -398,25 +398,6 @@ static __always_inline void amd_set_ssb_virt_state(unsigned long tifn)wrmsrl (MSR_AMD64_VIRT_SPEC_CTRL ,ssbd_tif_to_spec_ctrl (tifn)); } -static __always_inline void spec_ctrl_update_msr(unsigned long tifn) -{ -u64 msr = x86_spec_ctrl_base; - -/* -* If X86_FEATURE_SSBD is not set, the SSBD bit is not to be -* touched. -*/ -if (static_cpu_has (X86_FEATURE_SSBD )) - msr |=ssbd_tif_to_spec_ctrl (tifn); - -/* Only evaluate if conditional STIBP is enabled */ -if (IS_ENABLED (CONFIG_SMP ) && -static_branch_unlikely (&switch_to_cond_stibp)) - msr |=stibp_tif_to_spec_ctrl (tifn); - -wrmsrl (MSR_IA32_SPEC_CTRL , msr); -} -/* * Update the MSRs managing speculation control, during context switch. *
@@ -428,6 +409,7 @@ static __always_inline void __speculation_ctrl_update(unsigned long tifp, {unsigned long tif_diff = tifp ^ tifn;bool updmsr = false; +u64 msr = x86_spec_ctrl_base;/* * If TIF_SSBD is different, select the proper mitigation
@@ -440,8 +422,10 @@ static __always_inline void __speculation_ctrl_update(unsigned long tifp,amd_set_ssb_virt_state (tifn); else if (static_cpu_has(X86_FEATURE_LS_CFG_SSBD)) amd_set_core_ssb_state(tifn); - else if (static_cpu_has(X86_FEATURE_SSBD)) + else if (static_cpu_has(X86_FEATURE_SSBD)) { updmsr = true; + msr |=ssbd_tif_to_spec_ctrl (tifn); + } }/*
@@ -449,11 +433,13 @@ static __always_inline void __speculation_ctrl_update(unsigned long tifp,* otherwise avoid the MSR write. */ if (IS_ENABLED (CONFIG_SMP ) && -static_branch_unlikely (&switch_to_cond_stibp)) +static_branch_unlikely (&switch_to_cond_stibp)) { updmsr |= !!(tif_diff & _TIF_SPEC_IB); + msr |=stibp_tif_to_spec_ctrl (tifn); + }if (updmsr) -spec_ctrl_update_msr (tifn); +wrmsrl (MSR_IA32_SPEC_CTRL , msr); }void speculation_ctrl_update (unsigned long tif)
Re: [patch V2 18/28] x86/speculation: Prepare for per task indirect branch speculation control
Quoted 2 lines · Tim Chen
I think it will be cleaner just to fold the msr update into
__speculation_ctrl_updateto fix this issue.
Yes, that looks nicer and avoids a couple of extra static_cpu_has() evaluations. I'll fold it into the proper places.
Thanks,
tglx
Re: [patch V2 18/28] x86/speculation: Prepare for per task indirect branch speculation control
In reply to Lendacky, ThomasQuoted 11 lines · Thomas
arch/x86/kernel/process.c static __always_inline void spec_ctrl_up · line 407
Quoted 5 lines · Thomas Gleixner
arch/x86/kernel/process.c +0
-0@@ -406,6 +406,11 @@ static __always_inline void spec_ctrl_upif (static_cpu_has (X86_FEATURE_SSBD )) msr |=ssbd_tif_to_spec_ctrl (tifn);I did some quick testing and found my original logic was flawed. Since
spec_ctrl_update_msr()can now be called for STIBP, an additional check is needed to set the SSBD MSR bit.
Quoted 6 lines
Both
X86_FEATURE_VIRT_SSBDandX86_FEATURE_LS_CFG_SSBDcauseX86_FEATURE_SSBDto be set. Before this patch,spec_ctrl_update_msr()was only called ifX86_FEATURE_SSBDwas set and one of the other SSBD features wasn't set. But now, STIBP can causespec_ctrl_update_msr()to get called and cause the SSBD MSR bit to be set when it shouldn't (could result in a GP fault).
The below should fix that. We have the same logic in x86_virt_spec_ctrl()
Thanks,
tglx
8<---------------
arch/x86/kernel/process.c +4 -3
@@ -403,10 +403,11 @@ static __always_inline void spec_ctrl_upu64 msr = x86_spec_ctrl_base;/* -* If X86_FEATURE_SSBD is not set, the SSBD bit is not to be -* touched. +* If SSBD is not controlled in MSR_SPEC_CTRL, the SSBD bit has not +* to be touched. */ -if (static_cpu_has (X86_FEATURE_SSBD )) +if (static_cpu_has (X86_FEATURE_SPEC_CTRL_SSBD ) || +static_cpu_has (X86_FEATURE_AMD_SSBD )) msr |=ssbd_tif_to_spec_ctrl (tifn);/* Only evaluate if conditional STIBP is enabled */
Re: [patch V2 18/28] x86/speculation: Prepare for per task indirect branch speculation control
Quoted 12 lines · Thomas Gleixner
Quoted 11 lines · Thomas
arch/x86/kernel/process.c static __always_inline void spec_ctrl_up · line 407
Quoted 5 lines · Thomas Gleixner
arch/x86/kernel/process.c +0
-0@@ -406,6 +406,11 @@ static __always_inline void spec_ctrl_upif (static_cpu_has (X86_FEATURE_SSBD )) msr |=ssbd_tif_to_spec_ctrl (tifn);I did some quick testing and found my original logic was flawed. Since
spec_ctrl_update_msr()can now be called for STIBP, an additional check is needed to set the SSBD MSR bit.
Quoted 8 lines
Quoted 6 lines
Both
X86_FEATURE_VIRT_SSBDandX86_FEATURE_LS_CFG_SSBDcauseX86_FEATURE_SSBDto be set. Before this patch,spec_ctrl_update_msr()was only called ifX86_FEATURE_SSBDwas set and one of the other SSBD features wasn't set. But now, STIBP can causespec_ctrl_update_msr()to get called and cause the SSBD MSR bit to be set when it shouldn't (could result in a GP fault).The below should fix that. We have the same logic in
x86_virt_spec_ctrl()
Actually it's incomplete. Full version below.
Thanks,
tglx
8<-----------------
arch/x86/kernel/process.c +6 -4
@@ -403,10 +403,11 @@ static __always_inline void spec_ctrl_upu64 msr = x86_spec_ctrl_base;/* -* If X86_FEATURE_SSBD is not set, the SSBD bit is not to be -* touched. +* If SSBD is not controlled in MSR_SPEC_CTRL, the SSBD bit has not +* to be touched. */ -if (static_cpu_has (X86_FEATURE_SSBD )) +if (static_cpu_has (X86_FEATURE_SPEC_CTRL_SSBD ) || +static_cpu_has (X86_FEATURE_AMD_SSBD )) msr |=ssbd_tif_to_spec_ctrl (tifn);/* Only evaluate if conditional STIBP is enabled */
@@ -440,7 +441,8 @@ static __always_inline void __speculatioamd_set_ssb_virt_state (tifn); else if (static_cpu_has(X86_FEATURE_LS_CFG_SSBD)) amd_set_core_ssb_state(tifn); - else if (static_cpu_has(X86_FEATURE_SSBD)) + else if (static_cpu_has(X86_FEATURE_SPEC_CTRL_SSBD) || +static_cpu_has (X86_FEATURE_AMD_SSBD )) updmsr = true; }
Re: [patch V2 18/28] x86/speculation: Prepare for per task indirect branch speculation control
Quoted 20 lines · Thomas Gleixner
Quoted 19 lines · Thomas Gleixner
Quoted 17 lines · Thomas
arch/x86/kernel/process.c static __always_inline void spec_ctrl_up · line 407
Quoted 5 lines · Thomas Gleixner
arch/x86/kernel/process.c +0
-0@@ -406,6 +406,11 @@ static __always_inline void spec_ctrl_upif (static_cpu_has (X86_FEATURE_SSBD )) msr |=ssbd_tif_to_spec_ctrl (tifn);I did some quick testing and found my original logic was flawed. Since
spec_ctrl_update_msr()can now be called for STIBP, an additional check is needed to set the SSBD MSR bit.Both
X86_FEATURE_VIRT_SSBDandX86_FEATURE_LS_CFG_SSBDcauseX86_FEATURE_SSBDto be set. Before this patch,spec_ctrl_update_msr()was only called ifX86_FEATURE_SSBDwas set and one of the other SSBD features wasn't set. But now, STIBP can causespec_ctrl_update_msr()to get called and cause the SSBD MSR bit to be set when it shouldn't (could result in a GP fault).
Quoted 3 lines
Quoted 1 line
The below should fix that. We have the same logic in
x86_virt_spec_ctrl()Actually it's incomplete. Full version below.
Just one little nit on the comment below, otherwise works nicely.
Thanks,
Tom
Quoted 9 lines
Thanks,
tglx
8<-----------------
arch/x86/kernel/process.c static __always_inline void spec_ctrl_up · line 406
Quoted 6 lines
arch/x86/kernel/process.c +1
-2@@ -403,10 +403,11 @@ static __always_inline void spec_ctrl_upu64 msr = x86_spec_ctrl_base;/* -* If X86_FEATURE_SSBD is not set, the SSBD bit is not to be -* touched. +* If SSBD is not controlled in MSR_SPEC_CTRL, the SSBD bit has not
s/has not/is not/
Quoted 19 lines
arch/x86/kernel/process.c +5
-2+* to be touched. */ -if (static_cpu_has (X86_FEATURE_SSBD )) +if (static_cpu_has (X86_FEATURE_SPEC_CTRL_SSBD ) || +static_cpu_has (X86_FEATURE_AMD_SSBD )) msr |=ssbd_tif_to_spec_ctrl (tifn);/* Only evaluate if conditional STIBP is enabled */ @@ -440,7 +441,8 @@ static __always_inline void __speculatioamd_set_ssb_virt_state (tifn); else if (static_cpu_has(X86_FEATURE_LS_CFG_SSBD)) amd_set_core_ssb_state(tifn); - else if (static_cpu_has(X86_FEATURE_SSBD)) + else if (static_cpu_has(X86_FEATURE_SPEC_CTRL_SSBD) || +static_cpu_has (X86_FEATURE_AMD_SSBD )) updmsr = true; }
tip-bot for Tim Chen
merge bot[tip:x86/pti] x86/speculation: Prepare for per task indirect branch speculation control
In reply to Thomas Gleixnerx86/speculation: Prepare for per task indirect branch speculation control
Commit-ID: 5bfbe3ad5840d941b89bcac54b821ba14f50a0ba Gitweb: https://git.kernel.org/tip/5bfbe3ad5840d941b89bcac54b821ba14f50a0ba Author: Tim Chen <[email protected]> AuthorDate: Sun, 25 Nov 2018 19:33:46 +0100 Committer: Thomas Gleixner <[email protected]> CommitDate: Wed, 28 Nov 2018 11:57:10 +0100
x86/speculation: Prepare for per task indirect branch speculation control
To avoid the overhead of STIBP always on, it's necessary to allow per task control of STIBP.
Add a new task flag TIF_SPEC_IB and evaluate it during context switch if SMT is active and flag evaluation is enabled by the speculation control code. Add the conditional evaluation to x86_virt_spec_ctrl() as well so the guest/host switch works properly.
This has no effect because TIF_SPEC_IB cannot be set yet and the static key which controls evaluation is off. Preparatory patch for adding the control code.
[ tglx: Simplify the context switch logic and make the TIF evaluation
depend on SMP=y and on the static key controlling the conditional update. Rename it to TIF_SPEC_IB because it controls both STIBP and IBPB ]
--- arch/x86/include/asm/msr-index.h | 5 +++-- arch/x86/include/asm/spec-ctrl.h | 12 ++++++++++++ arch/x86/include/asm/thread_info.h | 5 ++++- arch/x86/kernel/cpu/bugs.c | 4 ++++ arch/x86/kernel/process.c | 20 ++++++++++++++++++-- 5 files changed, 41 insertions(+), 5 deletions(-)
arch/x86/include/asm/msr-index.h +3 -2
@@ -41,9 +41,10 @@#define MSR_IA32_SPEC_CTRL 0x00000048/* Speculation Control */ #define SPEC_CTRL_IBRS (1 << 0)/* Indirect Branch Restricted Speculation */ -#define SPEC_CTRL_STIBP (1 << 1)/* Single Thread Indirect Branch Predictors */ +#define SPEC_CTRL_STIBP_SHIFT 1/* Single Thread Indirect Branch Predictor (STIBP) bit */ +#define SPEC_CTRL_STIBP (1 << SPEC_CTRL_STIBP_SHIFT)/* STIBP mask */ #define SPEC_CTRL_SSBD_SHIFT 2/* Speculative Store Bypass Disable bit */ -#define SPEC_CTRL_SSBD (1 << SPEC_CTRL_SSBD_SHIFT)/* Speculative Store Bypass Disable */ +#define SPEC_CTRL_SSBD (1 << SPEC_CTRL_SSBD_SHIFT)/* Speculative Store Bypass Disable */ #define MSR_IA32_PRED_CMD 0x00000049/* Prediction Command */ #define PRED_CMD_IBPB (1 << 0)/* Indirect Branch Prediction Barrier */
arch/x86/include/asm/spec-ctrl.h +12 -0
@@ -53,12 +53,24 @@ static inline u64 ssbd_tif_to_spec_ctrl(u64 tifn)return (tifn & _TIF_SSBD) >> (TIF_SSBD -SPEC_CTRL_SSBD_SHIFT ); } +static inline u64 stibp_tif_to_spec_ctrl (u64 tifn) +{ +BUILD_BUG_ON (TIF_SPEC_IB <SPEC_CTRL_STIBP_SHIFT ); +return (tifn & _TIF_SPEC_IB) >> (TIF_SPEC_IB -SPEC_CTRL_STIBP_SHIFT ); +} +static inline unsigned long ssbd_spec_ctrl_to_tif (u64 spec_ctrl) {BUILD_BUG_ON (TIF_SSBD <SPEC_CTRL_SSBD_SHIFT );return (spec_ctrl &SPEC_CTRL_SSBD ) << (TIF_SSBD -SPEC_CTRL_SSBD_SHIFT ); } +static inline unsigned long stibp_spec_ctrl_to_tif (u64 spec_ctrl) +{ +BUILD_BUG_ON (TIF_SPEC_IB <SPEC_CTRL_STIBP_SHIFT ); +return (spec_ctrl &SPEC_CTRL_STIBP ) << (TIF_SPEC_IB -SPEC_CTRL_STIBP_SHIFT ); +} +static inline u64 ssbd_tif_to_amd_ls_cfg (u64 tifn) {return (tifn & _TIF_SSBD) ? x86_amd_ls_cfg_ssbd_mask :0ULL ;
arch/x86/include/asm/thread_info.h +4 -1
@@ -83,6 +83,7 @@ struct thread_info {#define TIF_SYSCALL_EMU 6/* syscall emulation active */ #define TIF_SYSCALL_AUDIT 7/* syscall auditing active */ #define TIF_SECCOMP 8/* secure computing */ +#define TIF_SPEC_IB 9/* Indirect branch speculation mitigation */ #define TIF_USER_RETURN_NOTIFY 11/* notify kernel of userspace return */ #define TIF_UPROBE 12/* breakpointed or singlestepping */ #define TIF_PATCH_PENDING 13/* pending live patching update */
@@ -110,6 +111,7 @@ struct thread_info {#define _TIF_SYSCALL_EMU (1 << TIF_SYSCALL_EMU)#define _TIF_SYSCALL_AUDIT (1 << TIF_SYSCALL_AUDIT)#define _TIF_SECCOMP (1 << TIF_SECCOMP) +#define _TIF_SPEC_IB (1 << TIF_SPEC_IB)#define _TIF_USER_RETURN_NOTIFY (1 << TIF_USER_RETURN_NOTIFY)#define _TIF_UPROBE (1 << TIF_UPROBE)#define _TIF_PATCH_PENDING (1 << TIF_PATCH_PENDING)
@@ -146,7 +148,8 @@ struct thread_info {/* flags to check in __switch_to() */ #define _TIF_WORK_CTXSW \ - (_TIF_IO_BITMAP|_TIF_NOCPUID|_TIF_NOTSC|_TIF_BLOCKSTEP|_TIF_SSBD) + (_TIF_IO_BITMAP|_TIF_NOCPUID|_TIF_NOTSC|_TIF_BLOCKSTEP| \ + _TIF_SSBD|_TIF_SPEC_IB)#define _TIF_WORK_CTXSW_PREV (_TIF_WORK_CTXSW|_TIF_USER_RETURN_NOTIFY)#define _TIF_WORK_CTXSW_NEXT (_TIF_WORK_CTXSW)
arch/x86/kernel/cpu/bugs.c +4 -0
@@ -148,6 +148,10 @@ x86_virt_spec_ctrl(u64 guest_spec_ctrl, u64 guest_virt_spec_ctrl, bool setguest) static_cpu_has(X86_FEATURE_AMD_SSBD)) hostval |=ssbd_tif_to_spec_ctrl (ti->flags); +/* Conditional STIBP enabled? */ +if (static_branch_unlikely (&switch_to_cond_stibp)) + hostval |=stibp_tif_to_spec_ctrl (ti->flags); +if (hostval != guestval) { msrval = setguest ? guestval : hostval;wrmsrl (MSR_IA32_SPEC_CTRL , msrval);
arch/x86/kernel/process.c +18 -2
@@ -404,11 +404,17 @@ static __always_inline void amd_set_ssb_virt_state(unsigned long tifn) static __always_inline void __speculation_ctrl_update(unsigned long tifp,unsigned long tifn) { +unsigned long tif_diff = tifp ^ tifn;u64 msr = x86_spec_ctrl_base;bool updmsr = false; -/* If TIF_SSBD is different, select the proper mitigation method */ -if ((tifp ^ tifn) & _TIF_SSBD) { +/* +* If TIF_SSBD is different, select the proper mitigation +* method. Note that if SSBD mitigation is disabled or permanentely +* enabled this branch can't be taken because nothing can set +* TIF_SSBD. +*/ +if (tif_diff & _TIF_SSBD) {if (static_cpu_has (X86_FEATURE_VIRT_SSBD )) {amd_set_ssb_virt_state (tifn); }else if (static_cpu_has (X86_FEATURE_LS_CFG_SSBD )) {
@@ -420,6 +426,16 @@ static __always_inline void __speculation_ctrl_update(unsigned long tifp, } } +/* +* Only evaluate TIF_SPEC_IB if conditional STIBP is enabled, +* otherwise avoid the MSR write. +*/ +if (IS_ENABLED (CONFIG_SMP ) && +static_branch_unlikely (&switch_to_cond_stibp)) { + updmsr |= !!(tif_diff & _TIF_SPEC_IB); + msr |=stibp_tif_to_spec_ctrl (tifn); + } +if (updmsr)wrmsrl (MSR_IA32_SPEC_CTRL , msr); }
PATCH v2 19/28 x86/process: Consolidate and simplify switch_to_xtra() code
In reply to Thomas GleixnerMove the conditional invocation of __switch_to_xtra() into an inline function so the logic can be shared between 32 and 64 bit.
Remove the handthrough of the TSS pointer and retrieve the pointer directly in the bitmap handling function. Use this_cpu_ptr() instead of the per_cpu() indirection.
This is a preparatory change so integration of conditional indirect branch speculation optimization happens only in one place.
--- arch/x86/include/asm/switch_to.h | 3 --- arch/x86/kernel/process.c | 12 +++++++----- arch/x86/kernel/process.h | 24 ++++++++++++++++++++++++ arch/x86/kernel/process_32.c | 10 +++------- arch/x86/kernel/process_64.c | 10 +++------- 5 files changed, 37 insertions(+), 22 deletions(-)
arch/x86/include/asm/switch_to.h +0 -3
@@ -11,9 +11,6 @@ struct task_struct *__switch_to_asm(stru __visible struct task_struct *__switch_to(struct task_struct *prev,struct task_struct *next); -struct tss_struct ; -void __switch_to_xtra (struct task_struct *prev_p,struct task_struct *next_p, -struct tss_struct *tss);/* This runs runs on the previous thread's stack. */ static inline void prepare_switch_to (struct task_struct *next)
arch/x86/kernel/process.c +7 -5
@@ -40,6 +40,8 @@#include <asm/prctl.h> #include <asm/spec-ctrl.h> +#include "process.h" +/* * per-CPU TSS segments. Threads are completely 'soft' on Linux, * no more per-task TSS's. The TSS size is kept cacheline-aligned
@@ -252,11 +254,12 @@ void arch_setup_new_exec(void)enable_cpuid (); } -static inline void switch_to_bitmap (struct tss_struct *tss, -struct thread_struct *prev, +static inline void switch_to_bitmap (struct thread_struct *prev,struct thread_struct *next,unsigned long tifp,unsigned long tifn) { +struct tss_struct *tss =this_cpu_ptr (&cpu_tss_rw); +if (tifn & _TIF_IO_BITMAP) {/* * Copy the relevant range of the IO bitmap.
@@ -461,8 +464,7 @@ void speculation_ctrl_update(unsigned lopreempt_enable (); } -void __switch_to_xtra (struct task_struct *prev_p,struct task_struct *next_p, -struct tss_struct *tss) +void __switch_to_xtra (struct task_struct *prev_p,struct task_struct *next_p) {struct thread_struct *prev, *next;unsigned long tifp, tifn;
@@ -472,7 +474,7 @@ void __switch_to_xtra(struct task_struct tifn =READ_ONCE (task_thread_info (next_p)->flags); tifp =READ_ONCE (task_thread_info (prev_p)->flags); -switch_to_bitmap (tss, prev, next, tifp, tifn); +switch_to_bitmap (prev, next, tifp, tifn);propagate_user_return_notify (prev_p, next_p);
arch/x86/kernel/process.h new file +24 -0
@@ -0,0 +1,24 @@ +// SPDX-License-Identifier: GPL-2.0 +// +// Code shared between 32 and 64 bit + +void __switch_to_xtra (struct task_struct *prev_p,struct task_struct *next_p); + +/* +* This needs to be inline to optimize for the common case where no extra +* work needs to be done. +*/ +static inline void switch_to_extra (struct task_struct *prev, +struct task_struct *next) +{ +unsigned long next_tif =task_thread_info (next)->flags; +unsigned long prev_tif =task_thread_info (prev)->flags; + +/* +* __switch_to_xtra() handles debug registers, i/o bitmaps, +* speculation mitigations etc. +*/ +if (unlikely (next_tif & _TIF_WORK_CTXSW_NEXT || + prev_tif & _TIF_WORK_CTXSW_PREV)) +__switch_to_xtra (prev, next); +}
arch/x86/kernel/process_32.c +3 -7
@@ -59,6 +59,8 @@#include <asm/intel_rdt_sched.h> #include <asm/proto.h> +#include "process.h" +void __show_regs (struct pt_regs *regs,enum show_regs_mode mode) {unsigned long cr0 =0L , cr2 =0L , cr3 =0L , cr4 =0L ;
@@ -232,7 +234,6 @@ EXPORT_SYMBOL_GPL(start_thread);struct fpu *prev_fpu = &prev->fpu;struct fpu *next_fpu = &next->fpu;int cpu =smp_processor_id (); -struct tss_struct *tss = &per_cpu (cpu_tss_rw, cpu);/* never put a printk in __switch_to... printk() calls wake_up*() indirectly */
@@ -264,12 +265,7 @@ EXPORT_SYMBOL_GPL(start_thread);if (get_kernel_rpl () &&unlikely (prev->iopl != next->iopl))set_iopl_mask (next->iopl); -/* -* Now maybe handle debug registers and/or IO bitmaps -*/ -if (unlikely (task_thread_info (prev_p)->flags & _TIF_WORK_CTXSW_PREV || -task_thread_info (next_p)->flags & _TIF_WORK_CTXSW_NEXT)) -__switch_to_xtra (prev_p, next_p, tss); +switch_to_extra (prev_p, next_p);/* * Leave lazy mode, flushing any hypercalls made here.
arch/x86/kernel/process_64.c +3 -7
@@ -60,6 +60,8 @@#include <asm/unistd_32_ia32.h> #endif +#include "process.h" +/* Prints also some state that isn't saved in the pt_regs */ void __show_regs (struct pt_regs *regs,enum show_regs_mode mode) {
@@ -553,7 +555,6 @@ void compat_start_thread(struct pt_regsstruct fpu *prev_fpu = &prev->fpu;struct fpu *next_fpu = &next->fpu;int cpu =smp_processor_id (); -struct tss_struct *tss = &per_cpu (cpu_tss_rw, cpu);WARN_ON_ONCE (IS_ENABLED (CONFIG_DEBUG_ENTRY ) &&this_cpu_read (irq_count) !=-1 );
@@ -617,12 +618,7 @@ void compat_start_thread(struct pt_regs/* Reload sp0. */ update_task_stack (next_p); -/* -* Now maybe reload the debug registers and handle I/O bitmaps -*/ -if (unlikely (task_thread_info (next_p)->flags & _TIF_WORK_CTXSW_NEXT || -task_thread_info (prev_p)->flags & _TIF_WORK_CTXSW_PREV)) -__switch_to_xtra (prev_p, next_p, tss); +switch_to_extra (prev_p, next_p);#ifdef CONFIG_XEN_PV /*
Borislav Petkov
maintainerRe: [patch V2 19/28] x86/process: Consolidate and simplify switch_to_xtra() code
Quoted 4 lines · Thomas Gleixner
Move the conditional invocation of
__switch_to_xtra()into an inline function so the logic can be shared between 32 and 64 bit.Remove the handthrough of the TSS pointer and retrieve the pointer directly
s/handthrough/passing/
I guess.
Quoted 14 lines
in the bitmap handling function. Use
this_cpu_ptr()instead of theper_cpu()indirection.This is a preparatory change so integration of conditional indirect branch speculation optimization happens only in one place.
Signed-off-by Thomas Gleixner <[email protected]>--- arch/x86/include/asm/switch_to.h | 3 --- arch/x86/kernel/process.c | 12 +++++++----- arch/x86/kernel/process.h | 24 ++++++++++++++++++++++++ arch/x86/kernel/process_32.c | 10 +++------- arch/x86/kernel/process_64.c | 10 +++------- 5 files changed, 37 insertions(+), 22 deletions(-)
...
arch/x86/kernel/process.h line 3
Quoted 6 lines
arch/x86/kernel/process.h new file +3
-0@@ -0,0 +1,24 @@ +// SPDX-License-Identifier: GPL-2.0 +// +// Code shared between 32 and 64 bit
checkpatch mutters here too:
WARNING: Missing or malformed SPDX-License-Identifier tag in line 1 #105: FILE: arch/x86/kernel/process.h:1:
+// SPDX-License-Identifier: GPL-2.0
I guess you want /* comments.
Signature · 4 lines
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.tip-bot for Thomas Gleixner
merge bot[tip:x86/pti] x86/process: Consolidate and simplify switch_to_xtra() code
In reply to Thomas Gleixnerx86/process: Consolidate and simplify switch_to_xtra() code
Commit-ID: ff16701a29cba3aafa0bd1656d766813b2d0a811 Gitweb: https://git.kernel.org/tip/ff16701a29cba3aafa0bd1656d766813b2d0a811 Author: Thomas Gleixner <[email protected]> AuthorDate: Sun, 25 Nov 2018 19:33:47 +0100 Committer: Thomas Gleixner <[email protected]> CommitDate: Wed, 28 Nov 2018 11:57:11 +0100
x86/process: Consolidate and simplify switch_to_xtra() code
Move the conditional invocation of __switch_to_xtra() into an inline function so the logic can be shared between 32 and 64 bit.
Remove the handthrough of the TSS pointer and retrieve the pointer directly in the bitmap handling function. Use this_cpu_ptr() instead of the per_cpu() indirection.
This is a preparatory change so integration of conditional indirect branch speculation optimization happens only in one place.
--- arch/x86/include/asm/switch_to.h | 3 --- arch/x86/kernel/process.c | 12 +++++++----- arch/x86/kernel/process.h | 24 ++++++++++++++++++++++++ arch/x86/kernel/process_32.c | 10 +++------- arch/x86/kernel/process_64.c | 10 +++------- 5 files changed, 37 insertions(+), 22 deletions(-)
arch/x86/include/asm/switch_to.h +0 -3
@@ -11,9 +11,6 @@ struct task_struct *__switch_to_asm(struct task_struct *prev, __visible struct task_struct *__switch_to(struct task_struct *prev,struct task_struct *next); -struct tss_struct ; -void __switch_to_xtra (struct task_struct *prev_p,struct task_struct *next_p, -struct tss_struct *tss);/* This runs runs on the previous thread's stack. */ static inline void prepare_switch_to (struct task_struct *next)
arch/x86/kernel/process.c +7 -5
@@ -40,6 +40,8 @@#include <asm/prctl.h> #include <asm/spec-ctrl.h> +#include "process.h" +/* * per-CPU TSS segments. Threads are completely 'soft' on Linux, * no more per-task TSS's. The TSS size is kept cacheline-aligned
@@ -252,11 +254,12 @@ void arch_setup_new_exec(void)enable_cpuid (); } -static inline void switch_to_bitmap (struct tss_struct *tss, -struct thread_struct *prev, +static inline void switch_to_bitmap (struct thread_struct *prev,struct thread_struct *next,unsigned long tifp,unsigned long tifn) { +struct tss_struct *tss =this_cpu_ptr (&cpu_tss_rw); +if (tifn & _TIF_IO_BITMAP) {/* * Copy the relevant range of the IO bitmap.
@@ -448,8 +451,7 @@ void speculation_ctrl_update(unsigned long tif)preempt_enable (); } -void __switch_to_xtra (struct task_struct *prev_p,struct task_struct *next_p, -struct tss_struct *tss) +void __switch_to_xtra (struct task_struct *prev_p,struct task_struct *next_p) {struct thread_struct *prev, *next;unsigned long tifp, tifn;
@@ -459,7 +461,7 @@ void __switch_to_xtra(struct task_struct *prev_p, struct task_struct *next_p, tifn =READ_ONCE (task_thread_info (next_p)->flags); tifp =READ_ONCE (task_thread_info (prev_p)->flags); -switch_to_bitmap (tss, prev, next, tifp, tifn); +switch_to_bitmap (prev, next, tifp, tifn);propagate_user_return_notify (prev_p, next_p);
arch/x86/kernel/process.h new file +24 -0
@@ -0,0 +1,24 @@ +// SPDX-License-Identifier: GPL-2.0 +// +// Code shared between 32 and 64 bit + +void __switch_to_xtra (struct task_struct *prev_p,struct task_struct *next_p); + +/* +* This needs to be inline to optimize for the common case where no extra +* work needs to be done. +*/ +static inline void switch_to_extra (struct task_struct *prev, +struct task_struct *next) +{ +unsigned long next_tif =task_thread_info (next)->flags; +unsigned long prev_tif =task_thread_info (prev)->flags; + +/* +* __switch_to_xtra() handles debug registers, i/o bitmaps, +* speculation mitigations etc. +*/ +if (unlikely (next_tif & _TIF_WORK_CTXSW_NEXT || + prev_tif & _TIF_WORK_CTXSW_PREV)) +__switch_to_xtra (prev, next); +}
arch/x86/kernel/process_32.c +3 -7
@@ -59,6 +59,8 @@#include <asm/intel_rdt_sched.h> #include <asm/proto.h> +#include "process.h" +void __show_regs (struct pt_regs *regs,enum show_regs_mode mode) {unsigned long cr0 =0L , cr2 =0L , cr3 =0L , cr4 =0L ;
@@ -232,7 +234,6 @@ __switch_to(struct task_struct *prev_p, struct task_struct *next_p)struct fpu *prev_fpu = &prev->fpu;struct fpu *next_fpu = &next->fpu;int cpu =smp_processor_id (); -struct tss_struct *tss = &per_cpu (cpu_tss_rw, cpu);/* never put a printk in __switch_to... printk() calls wake_up*() indirectly */
@@ -264,12 +265,7 @@ __switch_to(struct task_struct *prev_p, struct task_struct *next_p)if (get_kernel_rpl () &&unlikely (prev->iopl != next->iopl))set_iopl_mask (next->iopl); -/* -* Now maybe handle debug registers and/or IO bitmaps -*/ -if (unlikely (task_thread_info (prev_p)->flags & _TIF_WORK_CTXSW_PREV || -task_thread_info (next_p)->flags & _TIF_WORK_CTXSW_NEXT)) -__switch_to_xtra (prev_p, next_p, tss); +switch_to_extra (prev_p, next_p);/* * Leave lazy mode, flushing any hypercalls made here.
arch/x86/kernel/process_64.c +3 -7
@@ -60,6 +60,8 @@#include <asm/unistd_32_ia32.h> #endif +#include "process.h" +/* Prints also some state that isn't saved in the pt_regs */ void __show_regs (struct pt_regs *regs,enum show_regs_mode mode) {
@@ -553,7 +555,6 @@ __switch_to(struct task_struct *prev_p, struct task_struct *next_p)struct fpu *prev_fpu = &prev->fpu;struct fpu *next_fpu = &next->fpu;int cpu =smp_processor_id (); -struct tss_struct *tss = &per_cpu (cpu_tss_rw, cpu);WARN_ON_ONCE (IS_ENABLED (CONFIG_DEBUG_ENTRY ) &&this_cpu_read (irq_count) !=-1 );
@@ -617,12 +618,7 @@ __switch_to(struct task_struct *prev_p, struct task_struct *next_p)/* Reload sp0. */ update_task_stack (next_p); -/* -* Now maybe reload the debug registers and handle I/O bitmaps -*/ -if (unlikely (task_thread_info (next_p)->flags & _TIF_WORK_CTXSW_NEXT || -task_thread_info (prev_p)->flags & _TIF_WORK_CTXSW_PREV)) -__switch_to_xtra (prev_p, next_p, tss); +switch_to_extra (prev_p, next_p);#ifdef CONFIG_XEN_PV /*
PATCH v2 20/28 x86/speculation: Avoid __switch_to_xtra() calls
In reply to Thomas GleixnerThe TIF_SPEC_IB bit does not need to be evaluated in the decision to invoke __switch_to_xtra() when:
- CONFIG_SMP is disabled
- The conditional STIPB mode is disabled
The TIF_SPEC_IB bit still controls IBPB in both cases so the TIF work mask checks might invoke __switch_to_xtra() for nothing if TIF_SPEC_IB is the only set bit in the work masks.
Optimize it out by masking the bit at compile time for CONFIG_SMP=n and at run time when the static key controlling the conditional STIBP mode is disabled.
--- arch/x86/include/asm/thread_info.h | 13 +++++++++++-- arch/x86/kernel/process.h | 15 +++++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-)
arch/x86/include/asm/thread_info.h +11 -2
@@ -147,9 +147,18 @@ struct thread_info { _TIF_FSCHECK)/* flags to check in __switch_to() */ -#define _TIF_WORK_CTXSW \ +#define _TIF_WORK_CTXSW_BASE \ (_TIF_IO_BITMAP|_TIF_NOCPUID|_TIF_NOTSC|_TIF_BLOCKSTEP| \ - _TIF_SSBD|_TIF_SPEC_IB) + _TIF_SSBD) + +/* +* Avoid calls to __switch_to_xtra() on UP as STIBP is not evaluated. +*/ +#ifdef CONFIG_SMP +# define _TIF_WORK_CTXSW (_TIF_WORK_CTXSW_BASE | _TIF_SPEC_IB) +#else +# define _TIF_WORK_CTXSW (_TIF_WORK_CTXSW_BASE) +#endif #define _TIF_WORK_CTXSW_PREV (_TIF_WORK_CTXSW|_TIF_USER_RETURN_NOTIFY)#define _TIF_WORK_CTXSW_NEXT (_TIF_WORK_CTXSW)
arch/x86/kernel/process.h +15 -0
@@ -2,6 +2,8 @@// // Code shared between 32 and 64 bit +#include <asm/spec-ctrl.h> +void __switch_to_xtra (struct task_struct *prev_p,struct task_struct *next_p);/*
@@ -14,6 +16,19 @@ static inline void switch_to_extra(strucunsigned long next_tif =task_thread_info (next)->flags;unsigned long prev_tif =task_thread_info (prev)->flags; +if (IS_ENABLED (CONFIG_SMP )) { +/* +* Avoid __switch_to_xtra() invocation when conditional +* STIPB is disabled and the only different bit is +* TIF_SPEC_IB. For CONFIG_SMP=n TIF_SPEC_IB is not +* in the TIF_WORK_CTXSW masks. +*/ +if (!static_branch_likely (&switch_to_cond_stibp)) { + prev_tif &= ~_TIF_SPEC_IB; + next_tif &= ~_TIF_SPEC_IB; + } + } +/* * __switch_to_xtra() handles debug registers, i/o bitmaps, * speculation mitigations etc.
tip-bot for Thomas Gleixner
merge bot[tip:x86/pti] x86/speculation: Avoid __switch_to_xtra() calls
x86/speculation: Avoid __switch_to_xtra() calls
Commit-ID: 5635d99953f04b550738f6f4c1c532667c3fd872 Gitweb: https://git.kernel.org/tip/5635d99953f04b550738f6f4c1c532667c3fd872 Author: Thomas Gleixner <[email protected]> AuthorDate: Sun, 25 Nov 2018 19:33:48 +0100 Committer: Thomas Gleixner <[email protected]> CommitDate: Wed, 28 Nov 2018 11:57:11 +0100
x86/speculation: Avoid __switch_to_xtra() calls
The TIF_SPEC_IB bit does not need to be evaluated in the decision to invoke __switch_to_xtra() when:
- CONFIG_SMP is disabled
- The conditional STIPB mode is disabled
The TIF_SPEC_IB bit still controls IBPB in both cases so the TIF work mask checks might invoke __switch_to_xtra() for nothing if TIF_SPEC_IB is the only set bit in the work masks.
Optimize it out by masking the bit at compile time for CONFIG_SMP=n and at run time when the static key controlling the conditional STIBP mode is disabled.
--- arch/x86/include/asm/thread_info.h | 13 +++++++++++-- arch/x86/kernel/process.h | 15 +++++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-)
arch/x86/include/asm/thread_info.h +11 -2
@@ -147,9 +147,18 @@ struct thread_info { _TIF_FSCHECK)/* flags to check in __switch_to() */ -#define _TIF_WORK_CTXSW \ +#define _TIF_WORK_CTXSW_BASE \ (_TIF_IO_BITMAP|_TIF_NOCPUID|_TIF_NOTSC|_TIF_BLOCKSTEP| \ - _TIF_SSBD|_TIF_SPEC_IB) + _TIF_SSBD) + +/* +* Avoid calls to __switch_to_xtra() on UP as STIBP is not evaluated. +*/ +#ifdef CONFIG_SMP +# define _TIF_WORK_CTXSW (_TIF_WORK_CTXSW_BASE | _TIF_SPEC_IB) +#else +# define _TIF_WORK_CTXSW (_TIF_WORK_CTXSW_BASE) +#endif #define _TIF_WORK_CTXSW_PREV (_TIF_WORK_CTXSW|_TIF_USER_RETURN_NOTIFY)#define _TIF_WORK_CTXSW_NEXT (_TIF_WORK_CTXSW)
arch/x86/kernel/process.h +15 -0
@@ -2,6 +2,8 @@// // Code shared between 32 and 64 bit +#include <asm/spec-ctrl.h> +void __switch_to_xtra (struct task_struct *prev_p,struct task_struct *next_p);/*
@@ -14,6 +16,19 @@ static inline void switch_to_extra(struct task_struct *prev,unsigned long next_tif =task_thread_info (next)->flags;unsigned long prev_tif =task_thread_info (prev)->flags; +if (IS_ENABLED (CONFIG_SMP )) { +/* +* Avoid __switch_to_xtra() invocation when conditional +* STIPB is disabled and the only different bit is +* TIF_SPEC_IB. For CONFIG_SMP=n TIF_SPEC_IB is not +* in the TIF_WORK_CTXSW masks. +*/ +if (!static_branch_likely (&switch_to_cond_stibp)) { + prev_tif &= ~_TIF_SPEC_IB; + next_tif &= ~_TIF_SPEC_IB; + } + } +/* * __switch_to_xtra() handles debug registers, i/o bitmaps, * speculation mitigations etc.
PATCH v2 21/28 x86/speculation: Prepare for conditional IBPB in switch_mm()
In reply to Thomas GleixnerThe IBPB speculation barrier is issued from switch_mm() when the kernel switches to a user space task with a different mm than the user space task which ran last on the same CPU.
An additional optimization is to avoid IBPB when the incoming task can be ptraced by the outgoing task. This optimization only works when switching directly between two user space tasks. When switching from a kernel task to a user space task the optimization fails because the previous task cannot be accessed anymore. So for quite some scenarios the optimization is just adding overhead.
The upcoming conditional IBPB support will issue IBPB only for user space tasks which have the TIF_SPEC_IB bit set. This requires to handle the following cases:
1) Switch from a user space task (potential attacker) which has TIF_SPEC_IB set to a user space task (potential victim) which has TIF_SPEC_IB not set.
2) Switch from a user space task (potential attacker) which has TIF_SPEC_IB not set to a user space task (potential victim) which has TIF_SPEC_IB set.
This needs to be optimized for the case where the IBPB can be avoided when only kernel threads ran in between user space tasks which belong to the same process.
The current check whether two tasks belong to the same context is using the tasks context id. While correct, it's simpler to use the mm pointer because it allows to mangle the TIF_SPEC_IB bit into it. The context id based mechanism requires extra storage, which creates worse code.
When a task is scheduled out its TIF_SPEC_IB bit is mangled as bit 0 into the per CPU storage which is used to track the last user space mm which was running on a CPU. This bit can be used together with the TIF_SPEC_IB bit of the incoming task to make the decision whether IBPB needs to be issued or not to cover the two cases above.
As conditional IBPB is going to be the default, remove the dubious ptrace check for the IBPB always case and simply issue IBPB always when the process changes.
Move the storage to a different place in the struct as the original one created a hole.
--- arch/x86/include/asm/nospec-branch.h | 2 arch/x86/include/asm/tlbflush.h | 8 +- arch/x86/kernel/cpu/bugs.c | 29 +++++++-- arch/x86/mm/tlb.c | 109 +++++++++++++++++++++++++---------- 4 files changed, 110 insertions(+), 38 deletions(-)
arch/x86/include/asm/nospec-branch.h +2 -0
@@ -312,6 +312,8 @@ do { \ } while (0)DECLARE_STATIC_KEY_FALSE (switch_to_cond_stibp); +DECLARE_STATIC_KEY_FALSE (switch_mm_cond_ibpb); +DECLARE_STATIC_KEY_FALSE (switch_mm_always_ibpb);#endif /* __ASSEMBLY__ */
arch/x86/include/asm/tlbflush.h +6 -2
@@ -169,10 +169,14 @@ struct tlb_state {#define LOADED_MM_SWITCHING ((struct mm_struct *)1) +/* Last user mm for optimizing IBPB */ +union { +struct mm_struct *last_user_mm; +unsigned long last_user_mm_ibpb; + }; +u16 loaded_mm_asid;u16 next_asid; -/* last user mm's ctx id */ -u64 last_ctx_id;/* * We can be in one of several states:
arch/x86/kernel/cpu/bugs.c +24 -5
@@ -56,6 +56,10 @@ u64 __ro_after_init x86_amd_ls_cfg_ssbd_/* Control conditional STIPB in switch_to() */ DEFINE_STATIC_KEY_FALSE (switch_to_cond_stibp); +/* Control conditional IBPB in switch_mm() */ +DEFINE_STATIC_KEY_FALSE (switch_mm_cond_ibpb); +/* Control unconditional IBPB in switch_mm() */ +DEFINE_STATIC_KEY_FALSE (switch_mm_always_ibpb);void __init check_bugs (void ) {
@@ -331,7 +335,17 @@ spectre_v2_user_select_mitigation(enum s/* Initialize Indirect Branch Prediction Barrier */ if (boot_cpu_has (X86_FEATURE_IBPB )) {setup_force_cpu_cap (X86_FEATURE_USE_IBPB ); -pr_info ("Spectre v2 mitigation: Enabling Indirect Branch Prediction Barrier\n" ); + +switch (mode) { +case SPECTRE_V2_USER_STRICT : +static_branch_enable (&switch_mm_always_ibpb); +break ; +default : +break ; + } + +pr_info ("mitigation: Enabling %s Indirect Branch Prediction Barrier\n" , + mode ==SPECTRE_V2_USER_STRICT ?"always-on" :"conditional" ); }/* If enhanced IBRS is enabled no STIPB required */
@@ -955,10 +969,15 @@ static char *stibp_state(void)static char *ibpb_state (void ) { -if (boot_cpu_has (X86_FEATURE_USE_IBPB )) -return ", IBPB" ; -else -return "" ; +if (boot_cpu_has (X86_FEATURE_IBPB )) { +switch (spectre_v2_user) { +case SPECTRE_V2_USER_NONE : +return ", IBPB: disabled" ; +case SPECTRE_V2_USER_STRICT : +return ", IBPB: always-on" ; + } + } +return "" ; } static ssize_t cpu_show_common(struct device *dev, struct device_attribute *attr,
arch/x86/mm/tlb.c +78 -31
151 lines not on this page · raw message
Re: [patch V2 21/28] x86/speculation: Prepare for conditional IBPB in switch_mm()
arch/x86/mm/tlb.c
Quoted 9 lines · Thomas Gleixner
/*
+ * Use bit 0 to mangle theTIF_SPEC_IBstate into the mm pointer which is
+ * stored in cpu_tlb_state.last_user_mm_ibpb.
+ */arch/x86/mm/tlb.c +5
-0+#define LAST_USER_MM_IBPB 0x1UL + +/* +unsigned long next_tif = task_thread_info(next)->flags; +unsigned long ibpb = (next_tif >> TIF_SPEC_IB) & LAST_USR_MM_IBPB;
That wants to be LAST_USER_ of course. That's what you get for last minute changes...
Re: [patch V2 21/28] x86/speculation: Prepare for conditional IBPB in switch_mm()
In reply to Thomas GleixnerQuoted 4 lines
The current check whether two tasks belong to the same context is using the tasks context id. While correct, it's simpler to use the mm pointer because it allows to mangle the
TIF_SPEC_IBbit into it. The context id based mechanism requires extra storage, which creates worse code.
[We tried similar in some really early versions, but it was replaced with the context id later.]
One issue with using the pointer is that the pointer can be reused when the original mm_struct is freed, and then gets reallocated immediately to an attacker. Then the attacker may avoid the IBPB.
Given it's probably hard to generate any reasonable leak bandwidth with such a complex scenario, but it still seemed better to close the hole.
Because of concerns with that the counter ID was used instead.
The ID can wrap too, but since it's 64bit, it will take very long.
-Andi
Re: [patch V2 21/28] x86/speculation: Prepare for conditional IBPB in switch_mm()
Quoted 8 lines · Andi Kleen
Quoted 4 lines
The current check whether two tasks belong to the same context is using the tasks context id. While correct, it's simpler to use the mm pointer because it allows to mangle the
TIF_SPEC_IBbit into it. The context id based mechanism requires extra storage, which creates worse code.[We tried similar in some really early versions, but it was replaced with the context id later.]
Quoted 6 lines
One issue with using the pointer is that the pointer can be reused when the original
mm_structis freed, and then gets reallocated immediately to an attacker. Then the attacker may avoid the IBPB.Given it's probably hard to generate any reasonable leak bandwidth with such a complex scenario, but it still seemed better to close the hole.
Sorry, but that's really a purely academic exercise.
Thanks,
tglx
Re: [patch V2 21/28] x86/speculation: Prepare for conditional IBPB in switch_mm()
Quoted 14 lines · Thomas Gleixner
Quoted 8 lines · Andi Kleen
Quoted 4 lines
The current check whether two tasks belong to the same context is using the tasks context id. While correct, it's simpler to use the mm pointer because it allows to mangle the
TIF_SPEC_IBbit into it. The context id based mechanism requires extra storage, which creates worse code.[We tried similar in some really early versions, but it was replaced with the context id later.]
Quoted 10 lines
Quoted 6 lines
One issue with using the pointer is that the pointer can be reused when the original
mm_structis freed, and then gets reallocated immediately to an attacker. Then the attacker may avoid the IBPB.Given it's probably hard to generate any reasonable leak bandwidth with such a complex scenario, but it still seemed better to close the hole.
Sorry, but that's really a purely academic exercise.
I would guess that it’s actually very easy to force mm_struct* reuse. Don’t the various allocators try to allocate hot memory? There’s nothing hotter than a just-freed allocation of the same size.
Can someone explain the actual problem with ctx_id? If you just need an extra bit, how about:
2*ctx_id vs 2*ctx_id+1
Or any of the many variants of approximately the same thing?
—Andy
Re: [patch V2 21/28] x86/speculation: Prepare for conditional IBPB in switch_mm()
Quoted 15 lines · Andy Lutomirski
Quoted 15 lines · Thomas Gleixner
Quoted 12 lines · Andi Kleen
Quoted 4 lines
The current check whether two tasks belong to the same context is using the tasks context id. While correct, it's simpler to use the mm pointer because it allows to mangle the
TIF_SPEC_IBbit into it. The context id based mechanism requires extra storage, which creates worse code.[We tried similar in some really early versions, but it was replaced with the context id later.]
One issue with using the pointer is that the pointer can be reused when the original
mm_structis freed, and then gets reallocated immediately to an attacker. Then the attacker may avoid the IBPB.
Quoted 8 lines
Quoted 4 lines
Quoted 2 lines
Given it's probably hard to generate any reasonable leak bandwidth with such a complex scenario, but it still seemed better to close the hole.
Sorry, but that's really a purely academic exercise.
I would guess that it’s actually very easy to force mm_struct* reuse. Don’t the various allocators try to allocate hot memory? There’s nothing hotter than a just-freed allocation of the same size.
Sure, but this is about a indirect branch predictor attack against something which reuses the mm.
So you'd need to pull off:
P1 poisons branch predictor
P1 exit
P2 starts and resuses mm(P1) and uses the poisoned branch predictor
the only thing between P1 and P2 is either idle or some other kernel thread, but no other user task. If that happens then the code would not issue IBPB as it assumes to switch back to the same process.
Even if you can pull that off the speculation would hit the startup code of P2, which is truly a source of secret information. Creating a valuable attack based on mm reuse is really less proabable than a lottery jackpot.
So using mm is really good enough and results in better assembly code which is surely more valuable than addressing some hypothetical hole.
Thanks,
tglx
Re: [patch V2 21/28] x86/speculation: Prepare for conditional IBPB in switch_mm()
Quoted 39 lines · Thomas Gleixner
Quoted 15 lines · Andy Lutomirski
Quoted 15 lines · Thomas Gleixner
Quoted 12 lines · Andi Kleen
Quoted 4 lines
The current check whether two tasks belong to the same context is using the tasks context id. While correct, it's simpler to use the mm pointer because it allows to mangle the
TIF_SPEC_IBbit into it. The context id based mechanism requires extra storage, which creates worse code.[We tried similar in some really early versions, but it was replaced with the context id later.]
One issue with using the pointer is that the pointer can be reused when the original
mm_structis freed, and then gets reallocated immediately to an attacker. Then the attacker may avoid the IBPB.Quoted 8 lines
Quoted 4 lines
Quoted 2 lines
Given it's probably hard to generate any reasonable leak bandwidth with such a complex scenario, but it still seemed better to close the hole.
Sorry, but that's really a purely academic exercise.
I would guess that it’s actually very easy to force mm_struct* reuse. Don’t the various allocators try to allocate hot memory? There’s nothing hotter than a just-freed allocation of the same size.
Sure, but this is about a indirect branch predictor attack against something which reuses the mm.
So you'd need to pull off:
P1 poisons branch predictor
P1 exitP2 starts and resuses mm(P1) and uses the poisoned branch predictor
the only thing between P1 and P2 is either idle or some other kernel thread, but no other user task. If that happens then the code would not issue IBPB as it assumes to switch back to the same process.
Quoted 6 lines
Even if you can pull that off the speculation would hit the startup code of P2, which is truly a source of secret information. Creating a valuable attack based on mm reuse is really less proabable than a lottery jackpot.
So using mm is really good enough and results in better assembly code which is surely more valuable than addressing some hypothetical hole.
OTOH we could probably close even this with very little cost if we added an IBPB to non-threaded fork() and vfork()+exec() paths? Those are really slow paths compared to all the context switch paths we are trying to optimize here.
Alternatively we could IBPB on the post-exit() final task struct freeing, which too is a relative slow path compared to the context switch paths.
But no strong opinion.
Thanks,
Ingo
Re: [patch V2 21/28] x86/speculation: Prepare for conditional IBPB in switch_mm()
In reply to Thomas GleixnerQuoted 10 lines · Thomas Gleixner
Quoted 8 lines · Andi Kleen
Quoted 4 lines
The current check whether two tasks belong to the same context is using the tasks context id. While correct, it's simpler to use the mm pointer because it allows to mangle the
TIF_SPEC_IBbit into it. The context id based mechanism requires extra storage, which creates worse code.[We tried similar in some really early versions, but it was replaced with the context id later.]
Quoted 8 lines
Quoted 6 lines
One issue with using the pointer is that the pointer can be reused when the original
mm_structis freed, and then gets reallocated immediately to an attacker. Then the attacker may avoid the IBPB.Given it's probably hard to generate any reasonable leak bandwidth with such a complex scenario, but it still seemed better to close the hole.
Sorry, but that's really a purely academic exercise.
Ok fair enough. I guess it's acceptable if you add a comment explaining it.
-Andi
Re: [patch V2 21/28] x86/speculation: Prepare for conditional IBPB in switch_mm()
Quoted 15 lines · Andi Kleen
Quoted 14 lines · Thomas Gleixner
Quoted 12 lines · Andi Kleen
Quoted 4 lines
The current check whether two tasks belong to the same context is using the tasks context id. While correct, it's simpler to use the mm pointer because it allows to mangle the
TIF_SPEC_IBbit into it. The context id based mechanism requires extra storage, which creates worse code.[We tried similar in some really early versions, but it was replaced with the context id later.]
One issue with using the pointer is that the pointer can be reused when the original
mm_structis freed, and then gets reallocated immediately to an attacker. Then the attacker may avoid the IBPB.
Quoted 6 lines
Quoted 4 lines
Quoted 2 lines
Given it's probably hard to generate any reasonable leak bandwidth with such a complex scenario, but it still seemed better to close the hole.
Sorry, but that's really a purely academic exercise.
Ok fair enough. I guess it's acceptable if you add a comment explaining it.
Will do.
Thanks,
tglx
tip-bot for Thomas Gleixner
merge bot[tip:x86/pti] x86/speculation: Prepare for conditional IBPB in switch_mm()
In reply to Thomas Gleixnerx86/speculation: Prepare for conditional IBPB in switch_mm()
Commit-ID: 4c71a2b6fd7e42814aa68a6dec88abf3b42ea573 Gitweb: https://git.kernel.org/tip/4c71a2b6fd7e42814aa68a6dec88abf3b42ea573 Author: Thomas Gleixner <[email protected]> AuthorDate: Sun, 25 Nov 2018 19:33:49 +0100 Committer: Thomas Gleixner <[email protected]> CommitDate: Wed, 28 Nov 2018 11:57:11 +0100
x86/speculation: Prepare for conditional IBPB in switch_mm()
The IBPB speculation barrier is issued from switch_mm() when the kernel switches to a user space task with a different mm than the user space task which ran last on the same CPU.
An additional optimization is to avoid IBPB when the incoming task can be ptraced by the outgoing task. This optimization only works when switching directly between two user space tasks. When switching from a kernel task to a user space task the optimization fails because the previous task cannot be accessed anymore. So for quite some scenarios the optimization is just adding overhead.
The upcoming conditional IBPB support will issue IBPB only for user space tasks which have the TIF_SPEC_IB bit set. This requires to handle the following cases:
1) Switch from a user space task (potential attacker) which has TIF_SPEC_IB set to a user space task (potential victim) which has TIF_SPEC_IB not set.
2) Switch from a user space task (potential attacker) which has TIF_SPEC_IB not set to a user space task (potential victim) which has TIF_SPEC_IB set.
This needs to be optimized for the case where the IBPB can be avoided when only kernel threads ran in between user space tasks which belong to the same process.
The current check whether two tasks belong to the same context is using the tasks context id. While correct, it's simpler to use the mm pointer because it allows to mangle the TIF_SPEC_IB bit into it. The context id based mechanism requires extra storage, which creates worse code.
When a task is scheduled out its TIF_SPEC_IB bit is mangled as bit 0 into the per CPU storage which is used to track the last user space mm which was running on a CPU. This bit can be used together with the TIF_SPEC_IB bit of the incoming task to make the decision whether IBPB needs to be issued or not to cover the two cases above.
As conditional IBPB is going to be the default, remove the dubious ptrace check for the IBPB always case and simply issue IBPB always when the process changes.
Move the storage to a different place in the struct as the original one created a hole.
--- arch/x86/include/asm/nospec-branch.h | 2 + arch/x86/include/asm/tlbflush.h | 8 ++- arch/x86/kernel/cpu/bugs.c | 29 +++++++-- arch/x86/mm/tlb.c | 115 ++++++++++++++++++++++++++--------- 4 files changed, 118 insertions(+), 36 deletions(-)
arch/x86/include/asm/nospec-branch.h +2 -0
@@ -312,6 +312,8 @@ do { \ } while (0)DECLARE_STATIC_KEY_FALSE (switch_to_cond_stibp); +DECLARE_STATIC_KEY_FALSE (switch_mm_cond_ibpb); +DECLARE_STATIC_KEY_FALSE (switch_mm_always_ibpb);#endif /* __ASSEMBLY__ */
arch/x86/include/asm/tlbflush.h +6 -2
@@ -169,10 +169,14 @@ struct tlb_state {#define LOADED_MM_SWITCHING ((struct mm_struct *)1) +/* Last user mm for optimizing IBPB */ +union { +struct mm_struct *last_user_mm; +unsigned long last_user_mm_ibpb; + }; +u16 loaded_mm_asid;u16 next_asid; -/* last user mm's ctx id */ -u64 last_ctx_id;/* * We can be in one of several states:
arch/x86/kernel/cpu/bugs.c +24 -5
@@ -56,6 +56,10 @@ u64 __ro_after_init x86_amd_ls_cfg_ssbd_mask;/* Control conditional STIPB in switch_to() */ DEFINE_STATIC_KEY_FALSE (switch_to_cond_stibp); +/* Control conditional IBPB in switch_mm() */ +DEFINE_STATIC_KEY_FALSE (switch_mm_cond_ibpb); +/* Control unconditional IBPB in switch_mm() */ +DEFINE_STATIC_KEY_FALSE (switch_mm_always_ibpb);void __init check_bugs (void ) {
@@ -331,7 +335,17 @@ spectre_v2_user_select_mitigation(enum spectre_v2_mitigation_cmd v2_cmd)/* Initialize Indirect Branch Prediction Barrier */ if (boot_cpu_has (X86_FEATURE_IBPB )) {setup_force_cpu_cap (X86_FEATURE_USE_IBPB ); -pr_info ("Spectre v2 mitigation: Enabling Indirect Branch Prediction Barrier\n" ); + +switch (mode) { +case SPECTRE_V2_USER_STRICT : +static_branch_enable (&switch_mm_always_ibpb); +break ; +default : +break ; + } + +pr_info ("mitigation: Enabling %s Indirect Branch Prediction Barrier\n" , + mode ==SPECTRE_V2_USER_STRICT ?"always-on" :"conditional" ); }/* If enhanced IBRS is enabled no STIPB required */
@@ -955,10 +969,15 @@ static char *stibp_state(void)static char *ibpb_state (void ) { -if (boot_cpu_has (X86_FEATURE_USE_IBPB )) -return ", IBPB" ; -else -return "" ; +if (boot_cpu_has (X86_FEATURE_IBPB )) { +switch (spectre_v2_user) { +case SPECTRE_V2_USER_NONE : +return ", IBPB: disabled" ; +case SPECTRE_V2_USER_STRICT : +return ", IBPB: always-on" ; + } + } +return "" ; } static ssize_t cpu_show_common(struct device *dev, struct device_attribute *attr,
arch/x86/mm/tlb.c +86 -29
159 lines not on this page · raw message
PATCH v2 22/28 ptrace: Remove unused ptrace_may_access_sched() and MODE_IBRS
In reply to Thomas GleixnerThe IBPB control code in x86 removed the usage. Remove the functionality which was introduced for this.
--- include/linux/ptrace.h | 17 ----------------- kernel/ptrace.c | 10 ---------- 2 files changed, 27 deletions(-)
include/linux/ptrace.h +0 -17
@@ -64,15 +64,12 @@ extern void exit_ptrace(struct task_stru#define PTRACE_MODE_NOAUDIT 0x04#define PTRACE_MODE_FSCREDS 0x08#define PTRACE_MODE_REALCREDS 0x10 -#define PTRACE_MODE_SCHED 0x20 -#define PTRACE_MODE_IBPB 0x40/* shorthands for READ/ATTACH and FSCREDS/REALCREDS combinations */ #define PTRACE_MODE_READ_FSCREDS (PTRACE_MODE_READ | PTRACE_MODE_FSCREDS)#define PTRACE_MODE_READ_REALCREDS (PTRACE_MODE_READ | PTRACE_MODE_REALCREDS)#define PTRACE_MODE_ATTACH_FSCREDS (PTRACE_MODE_ATTACH | PTRACE_MODE_FSCREDS)#define PTRACE_MODE_ATTACH_REALCREDS (PTRACE_MODE_ATTACH | PTRACE_MODE_REALCREDS) -#define PTRACE_MODE_SPEC_IBPB (PTRACE_MODE_ATTACH_REALCREDS | PTRACE_MODE_IBPB)/** * ptrace_may_access - check whether the caller is permitted to access
@@ -90,20 +87,6 @@ extern void exit_ptrace(struct task_stru*/ extern bool ptrace_may_access (struct task_struct *task,unsigned int mode); -/** -* ptrace_may_access - check whether the caller is permitted to access -* a target task. -* @task: target task -* @mode: selects type of access and caller credentials -* -* Returns true on success, false on denial. -* -* Similar to ptrace_may_access(). Only to be called from context switch -* code. Does not call into audit and the regular LSM hooks due to locking -* constraints. -*/ -extern bool ptrace_may_access_sched (struct task_struct *task,unsigned int mode); -static inline int ptrace_reparented (struct task_struct *child) {return !same_thread_group (child->real_parent, child->parent);
kernel/ptrace.c +0 -10
@@ -261,9 +261,6 @@ static int ptrace_check_attach(struct tastatic int ptrace_has_cap (struct user_namespace *ns,unsigned int mode) { -if (mode &PTRACE_MODE_SCHED ) -return false; -if (mode &PTRACE_MODE_NOAUDIT )return has_ns_capability_noaudit (current, ns,CAP_SYS_PTRACE );else
@@ -331,16 +328,9 @@ static int __ptrace_may_access(struct ta !ptrace_has_cap(mm->user_ns, mode)))return -EPERM ; -if (mode &PTRACE_MODE_SCHED ) -return 0 ;return security_ptrace_access_check (task, mode); } -bool ptrace_may_access_sched (struct task_struct *task,unsigned int mode) -{ -return __ptrace_may_access (task, mode |PTRACE_MODE_SCHED ); -} -bool ptrace_may_access (struct task_struct *task,unsigned int mode) {int err;
tip-bot for Thomas Gleixner
merge bot[tip:x86/pti] ptrace: Remove unused ptrace_may_access_sched() and MODE_IBRS
ptrace: Remove unused ptrace_may_access_sched() and MODE_IBRS
Commit-ID: 46f7ecb1e7359f183f5bbd1e08b90e10e52164f9 Gitweb: https://git.kernel.org/tip/46f7ecb1e7359f183f5bbd1e08b90e10e52164f9 Author: Thomas Gleixner <[email protected]> AuthorDate: Sun, 25 Nov 2018 19:33:50 +0100 Committer: Thomas Gleixner <[email protected]> CommitDate: Wed, 28 Nov 2018 11:57:11 +0100
ptrace: Remove unused ptrace_may_access_sched() and MODE_IBRS
The IBPB control code in x86 removed the usage. Remove the functionality which was introduced for this.
--- include/linux/ptrace.h | 17 ----------------- kernel/ptrace.c | 10 ---------- 2 files changed, 27 deletions(-)
include/linux/ptrace.h +0 -17
@@ -64,15 +64,12 @@ extern void exit_ptrace(struct task_struct *tracer, struct list_head *dead);#define PTRACE_MODE_NOAUDIT 0x04#define PTRACE_MODE_FSCREDS 0x08#define PTRACE_MODE_REALCREDS 0x10 -#define PTRACE_MODE_SCHED 0x20 -#define PTRACE_MODE_IBPB 0x40/* shorthands for READ/ATTACH and FSCREDS/REALCREDS combinations */ #define PTRACE_MODE_READ_FSCREDS (PTRACE_MODE_READ | PTRACE_MODE_FSCREDS)#define PTRACE_MODE_READ_REALCREDS (PTRACE_MODE_READ | PTRACE_MODE_REALCREDS)#define PTRACE_MODE_ATTACH_FSCREDS (PTRACE_MODE_ATTACH | PTRACE_MODE_FSCREDS)#define PTRACE_MODE_ATTACH_REALCREDS (PTRACE_MODE_ATTACH | PTRACE_MODE_REALCREDS) -#define PTRACE_MODE_SPEC_IBPB (PTRACE_MODE_ATTACH_REALCREDS | PTRACE_MODE_IBPB)/** * ptrace_may_access - check whether the caller is permitted to access
@@ -90,20 +87,6 @@ extern void exit_ptrace(struct task_struct *tracer, struct list_head *dead);*/ extern bool ptrace_may_access (struct task_struct *task,unsigned int mode); -/** -* ptrace_may_access - check whether the caller is permitted to access -* a target task. -* @task: target task -* @mode: selects type of access and caller credentials -* -* Returns true on success, false on denial. -* -* Similar to ptrace_may_access(). Only to be called from context switch -* code. Does not call into audit and the regular LSM hooks due to locking -* constraints. -*/ -extern bool ptrace_may_access_sched (struct task_struct *task,unsigned int mode); -static inline int ptrace_reparented (struct task_struct *child) {return !same_thread_group (child->real_parent, child->parent);
kernel/ptrace.c +0 -10
@@ -261,9 +261,6 @@ static int ptrace_check_attach(struct task_struct *child, bool ignore_state)static int ptrace_has_cap (struct user_namespace *ns,unsigned int mode) { -if (mode &PTRACE_MODE_SCHED ) -return false; -if (mode &PTRACE_MODE_NOAUDIT )return has_ns_capability_noaudit (current, ns,CAP_SYS_PTRACE );else
@@ -331,16 +328,9 @@ ok: !ptrace_has_cap(mm->user_ns, mode)))return -EPERM ; -if (mode &PTRACE_MODE_SCHED ) -return 0 ;return security_ptrace_access_check (task, mode); } -bool ptrace_may_access_sched (struct task_struct *task,unsigned int mode) -{ -return __ptrace_may_access (task, mode |PTRACE_MODE_SCHED ); -} -bool ptrace_may_access (struct task_struct *task,unsigned int mode) {int err;
PATCH v2 23/28 x86/speculation: Split out TIF update
In reply to Thomas GleixnerThe update of the TIF_SSBD flag and the conditional speculation control MSR update is done in the ssb_prctl_set() function directly. The upcoming prctl support for controlling indirect branch speculation via STIBP needs the same mechanism.
Split the code out and make it reusable. Reword the comment about updates for other tasks.
---
V1 -> V2: Update comment.
--- arch/x86/kernel/cpu/bugs.c | 35 +++++++++++++++++++++++------------ 1 file changed, 23 insertions(+), 12 deletions(-)
arch/x86/kernel/cpu/bugs.c +23 -12
@@ -702,10 +702,29 @@ static void ssb_select_mitigation(void)#undef pr_fmt#define pr_fmt (fmt) "Speculation prctl: " fmt -static int ssb_prctl_set (struct task_struct *task,unsigned long ctrl) +static void task_update_spec_tif (struct task_struct *tsk,int tifbit,bool on) {bool update; +if (on) + update = !test_and_set_tsk_thread_flag (tsk, tifbit); +else + update =test_and_clear_tsk_thread_flag (tsk, tifbit); + +/* +* Immediately update the speculation control MSRs for the current +* task, but for a non-current task delay setting the CPU +* mitigation until it is scheduled next. +* +* This can only happen for SECCOMP mitigation. For PRCTL it's +* always the current task. +*/ +if (tsk == current && update) +speculation_ctrl_update_current (); +} + +static int ssb_prctl_set (struct task_struct *task,unsigned long ctrl) +{if (ssb_mode !=SPEC_STORE_BYPASS_PRCTL && ssb_mode !=SPEC_STORE_BYPASS_SECCOMP )return -ENXIO ;
@@ -716,28 +735,20 @@ static int ssb_prctl_set(struct task_strif (task_spec_ssb_force_disable (task))return -EPERM ;task_clear_spec_ssb_disable (task); - update =test_and_clear_tsk_thread_flag (task,TIF_SSBD ); +task_update_spec_tif (task,TIF_SSBD , false);break ;case PR_SPEC_DISABLE :task_set_spec_ssb_disable (task); - update = !test_and_set_tsk_thread_flag (task,TIF_SSBD ); +task_update_spec_tif (task,TIF_SSBD , true);break ;case PR_SPEC_FORCE_DISABLE :task_set_spec_ssb_disable (task);task_set_spec_ssb_force_disable (task); - update = !test_and_set_tsk_thread_flag (task,TIF_SSBD ); +task_update_spec_tif (task,TIF_SSBD , true);break ;default :return -ERANGE ; } - -/* -* If being set on non-current task, delay setting the CPU -* mitigation until it is next scheduled. -*/ -if (task == current && update) -speculation_ctrl_update_current (); -return 0 ; }
tip-bot for Thomas Gleixner
merge bot[tip:x86/pti] x86/speculation: Split out TIF update
x86/speculation: Split out TIF update
Commit-ID: e6da8bb6f9abb2628381904b24163c770e630bac Gitweb: https://git.kernel.org/tip/e6da8bb6f9abb2628381904b24163c770e630bac Author: Thomas Gleixner <[email protected]> AuthorDate: Sun, 25 Nov 2018 19:33:51 +0100 Committer: Thomas Gleixner <[email protected]> CommitDate: Wed, 28 Nov 2018 11:57:12 +0100
x86/speculation: Split out TIF update
The update of the TIF_SSBD flag and the conditional speculation control MSR update is done in the ssb_prctl_set() function directly. The upcoming prctl support for controlling indirect branch speculation via STIBP needs the same mechanism.
Split the code out and make it reusable. Reword the comment about updates for other tasks.
--- arch/x86/kernel/cpu/bugs.c | 35 +++++++++++++++++++++++------------ 1 file changed, 23 insertions(+), 12 deletions(-)
arch/x86/kernel/cpu/bugs.c +23 -12
@@ -702,10 +702,29 @@ static void ssb_select_mitigation(void)#undef pr_fmt#define pr_fmt (fmt) "Speculation prctl: " fmt -static int ssb_prctl_set (struct task_struct *task,unsigned long ctrl) +static void task_update_spec_tif (struct task_struct *tsk,int tifbit,bool on) {bool update; +if (on) + update = !test_and_set_tsk_thread_flag (tsk, tifbit); +else + update =test_and_clear_tsk_thread_flag (tsk, tifbit); + +/* +* Immediately update the speculation control MSRs for the current +* task, but for a non-current task delay setting the CPU +* mitigation until it is scheduled next. +* +* This can only happen for SECCOMP mitigation. For PRCTL it's +* always the current task. +*/ +if (tsk == current && update) +speculation_ctrl_update_current (); +} + +static int ssb_prctl_set (struct task_struct *task,unsigned long ctrl) +{if (ssb_mode !=SPEC_STORE_BYPASS_PRCTL && ssb_mode !=SPEC_STORE_BYPASS_SECCOMP )return -ENXIO ;
@@ -716,28 +735,20 @@ static int ssb_prctl_set(struct task_struct *task, unsigned long ctrl)if (task_spec_ssb_force_disable (task))return -EPERM ;task_clear_spec_ssb_disable (task); - update =test_and_clear_tsk_thread_flag (task,TIF_SSBD ); +task_update_spec_tif (task,TIF_SSBD , false);break ;case PR_SPEC_DISABLE :task_set_spec_ssb_disable (task); - update = !test_and_set_tsk_thread_flag (task,TIF_SSBD ); +task_update_spec_tif (task,TIF_SSBD , true);break ;case PR_SPEC_FORCE_DISABLE :task_set_spec_ssb_disable (task);task_set_spec_ssb_force_disable (task); - update = !test_and_set_tsk_thread_flag (task,TIF_SSBD ); +task_update_spec_tif (task,TIF_SSBD , true);break ;default :return -ERANGE ; } - -/* -* If being set on non-current task, delay setting the CPU -* mitigation until it is next scheduled. -*/ -if (task == current && update) -speculation_ctrl_update_current (); -return 0 ; }
PATCH v2 24/28 x86/speculation: Prepare arch_smt_update() for PRCTL mode
In reply to Thomas GleixnerThe 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. Mark the SMP function call argument __unused while at it.
---
v1 -> v2: s/app2app/user/. Mark smp function argument __unused
--- arch/x86/kernel/cpu/bugs.c | 46 ++++++++++++++++++++++++--------------------- 1 file changed, 25 insertions(+), 21 deletions(-)
arch/x86/kernel/cpu/bugs.c +25 -21
@@ -530,40 +530,44 @@ static void __init spectre_v2_select_mitarch_smt_update (); } -static bool stibp_needed (void ) +static void update_stibp_msr (void * __unused) { -/* Enhanced IBRS makes using STIBP unnecessary. */ -if (spectre_v2_enabled ==SPECTRE_V2_IBRS_ENHANCED ) -return false; - -/* Check for strict user mitigation mode */ -return spectre_v2_user ==SPECTRE_V2_USER_STRICT ; +wrmsrl (MSR_IA32_SPEC_CTRL , x86_spec_ctrl_base); } -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 user space SMT mitigation: STIBP %s\n" , + mask &SPEC_CTRL_STIBP ?"always-on" :"off" ); + 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 implies STIBP. 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_user) { +case SPECTRE_V2_USER_NONE : +break ; +case SPECTRE_V2_USER_STRICT : +update_stibp_strict (); +break ; } +mutex_unlock (&spec_ctrl_mutex); }
Re: [patch V2 24/28] x86/speculation: Prepare arch_smt_update() for PRCTL mode
Quoted 41 lines · Thomas Gleixner
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. Mark the SMP function call argument
__unusedwhile at it.Signed-off-by Thomas Gleixner <[email protected]>---
v1 -> v2: s/app2app/user/. Mark smp function argument
__unused--- arch/x86/kernel/cpu/bugs.c | 46 ++++++++++++++++++++++++--------------------- 1 file changed, 25 insertions(+), 21 deletions(-)arch/x86/kernel/cpu/bugs.c +6
-9@@ -530,40 +530,44 @@ static void __init spectre_v2_select_mitarch_smt_update (); } -static bool stibp_needed (void ) +static void update_stibp_msr (void * __unused) { -/* Enhanced IBRS makes using STIBP unnecessary. */ -if (spectre_v2_enabled ==SPECTRE_V2_IBRS_ENHANCED ) -return false; - -/* Check for strict user mitigation mode */ -return spectre_v2_user ==SPECTRE_V2_USER_STRICT ; +wrmsrl (MSR_IA32_SPEC_CTRL , x86_spec_ctrl_base); } -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 ; +
arch/x86/kernel/cpu/bugs.c static void __init spectre_v2_select_mit · line 552
Quoted 10 lines
arch/x86/kernel/cpu/bugs.c +10
-0+if (sched_smt_active ()) + mask |=SPEC_CTRL_STIBP ; + +if (mask == x86_spec_ctrl_base) +return ; + +pr_info ("Spectre v2 user space SMT mitigation: STIBP %s\n" , + mask &SPEC_CTRL_STIBP ?"always-on" :"off" ); + x86_spec_ctrl_base = mask; +on_each_cpu (update_stibp_msr,NULL ,1 );
Some more testing using spectre_v2_user=on and I've found that during boot up, once the first SMT thread is encountered no more updates to MSRs for STIBP are done for any CPUs brought up after that. The first SMT thread will cause mask != x86_spec_ctrl_base, but then x86_spec_ctrl_base is set to mask and the check always causes a return for subsequent CPUs that are brought up.
Talking to our HW folks, they recommend that it be set on all threads, so I'm not sure what the right approach would be for this.
Also, I've seen some BIOSes set up the cores/threads where the core and its thread are enumerated before the next core and its thread, etc. If that were the case, I think this would result in only the first core and its thread having STIBP set, right?
Thanks,
Tom
Quoted 35 lines
arch/x86/kernel/cpu/bugs.c +9
-12}void arch_smt_update (void ) { -u64 mask; - -if (!stibp_needed ()) +/* Enhanced IBRS implies STIBP. 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_user) { +case SPECTRE_V2_USER_NONE : +break ; +case SPECTRE_V2_USER_STRICT : +update_stibp_strict (); +break ; } +mutex_unlock (&spec_ctrl_mutex); }
Re: [patch V2 24/28] x86/speculation: Prepare arch_smt_update() for PRCTL mode
Quoted 16 lines · Thomas
Quoted 15 lines · Thomas Gleixner
arch/x86/kernel/cpu/bugs.c +13
-1+/* 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 user space SMT mitigation: STIBP %s\n" , + mask &SPEC_CTRL_STIBP ?"always-on" :"off" ); + x86_spec_ctrl_base = mask;
Quoted 8 lines
arch/x86/kernel/cpu/bugs.c static void __init spectre_v2_select_mit · line 552
Quoted 1 line
arch/x86/kernel/cpu/bugs.c +1
-0+on_each_cpu (update_stibp_msr,NULL ,1 );Some more testing using spectre_v2_user=on and I've found that during boot up, once the first SMT thread is encountered no more updates to MSRs for STIBP are done for any CPUs brought up after that. The first SMT thread will cause mask !=
x86_spec_ctrl_base, but thenx86_spec_ctrl_baseis set to mask and the check always causes a return for subsequent CPUs that are brought up.
The above code merily handles the switch between SMT and non-SMT mode, because there all other online CPUs need to be updated, but after that each upcoming CPU calls x86_spec_ctrl_setup_ap() which will write the MSR. So it's all good.
Thanks,
tglx
Re: [patch V2 24/28] x86/speculation: Prepare arch_smt_update() for PRCTL mode
Quoted 26 lines · Thomas Gleixner
Quoted 16 lines · Thomas
Quoted 15 lines · Thomas Gleixner
arch/x86/kernel/cpu/bugs.c +13
-1+/* 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 user space SMT mitigation: STIBP %s\n" , + mask &SPEC_CTRL_STIBP ?"always-on" :"off" ); + x86_spec_ctrl_base = mask;Quoted 8 lines
arch/x86/kernel/cpu/bugs.c static void __init spectre_v2_select_mit · line 552
Quoted 1 line
arch/x86/kernel/cpu/bugs.c +1
-0+on_each_cpu (update_stibp_msr,NULL ,1 );Some more testing using spectre_v2_user=on and I've found that during boot up, once the first SMT thread is encountered no more updates to MSRs for STIBP are done for any CPUs brought up after that. The first SMT thread will cause mask !=
x86_spec_ctrl_base, but thenx86_spec_ctrl_baseis set to mask and the check always causes a return for subsequent CPUs that are brought up.
Quoted 4 lines
The above code merily handles the switch between SMT and non-SMT mode, because there all other online CPUs need to be updated, but after that each upcoming CPU calls
x86_spec_ctrl_setup_ap()which will write the MSR. So it's all good.
Yup, sorry for the noise. Trying to test different scenarios using some code hacks and put them in the wrong place, so wasn't triggering the WRMSR in x86_spec_ctrl_setup_ap().
Thanks,
Tom
Quoted 5 lines
Thanks,
tglx
tip-bot for Thomas Gleixner
merge bot[tip:x86/pti] x86/speculation: Prepare arch_smt_update() for PRCTL mode
In reply to Thomas Gleixnerx86/speculation: Prepare arch_smt_update() for PRCTL mode
Commit-ID: 6893a959d7fdebbab5f5aa112c277d5a44435ba1 Gitweb: https://git.kernel.org/tip/6893a959d7fdebbab5f5aa112c277d5a44435ba1 Author: Thomas Gleixner <[email protected]> AuthorDate: Sun, 25 Nov 2018 19:33:52 +0100 Committer: Thomas Gleixner <[email protected]> CommitDate: Wed, 28 Nov 2018 11:57:13 +0100
x86/speculation: Prepare arch_smt_update() for PRCTL mode
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. Mark the SMP function call argument __unused while at it.
--- arch/x86/kernel/cpu/bugs.c | 46 +++++++++++++++++++++++++--------------------- 1 file changed, 25 insertions(+), 21 deletions(-)
arch/x86/kernel/cpu/bugs.c +25 -21
@@ -530,40 +530,44 @@ specv2_set_mode:arch_smt_update (); } -static bool stibp_needed (void ) +static void update_stibp_msr (void * __unused) { -/* Enhanced IBRS makes using STIBP unnecessary. */ -if (spectre_v2_enabled ==SPECTRE_V2_IBRS_ENHANCED ) -return false; - -/* Check for strict user mitigation mode */ -return spectre_v2_user ==SPECTRE_V2_USER_STRICT ; +wrmsrl (MSR_IA32_SPEC_CTRL , x86_spec_ctrl_base); } -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 ("Update user space SMT mitigation: STIBP %s\n" , + mask &SPEC_CTRL_STIBP ?"always-on" :"off" ); + 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 implies STIBP. 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_user) { +case SPECTRE_V2_USER_NONE : +break ; +case SPECTRE_V2_USER_STRICT : +update_stibp_strict (); +break ; } +mutex_unlock (&spec_ctrl_mutex); }
PATCH v2 25/28 x86/speculation: Add prctl() control for indirect branch speculation
In reply to Thomas GleixnerAdd the PR_SPEC_INDIRECT_BRANCH option for the PR_GET_SPECULATION_CTRL and PR_SET_SPECULATION_CTRL prctls to allow fine grained per task control of indirect branch speculation via STIBP and IBPB.
Invocations:
Check indirect branch speculation status with
- prctl(PR_GET_SPECULATION_CTRL, PR_SPEC_INDIRECT_BRANCH, 0, 0, 0);
Enable indirect branch speculation with
- prctl(PR_SET_SPECULATION_CTRL, PR_SPEC_INDIRECT_BRANCH, PR_SPEC_ENABLE, 0, 0);
Disable indirect branch speculation with
- prctl(PR_SET_SPECULATION_CTRL, PR_SPEC_INDIRECT_BRANCH, PR_SPEC_DISABLE, 0, 0);
Force disable indirect branch speculation with
- prctl(PR_SET_SPECULATION_CTRL, PR_SPEC_INDIRECT_BRANCH, PR_SPEC_FORCE_DISABLE, 0, 0);
See Documentation/userspace-api/spec_ctrl.rst.
---
V1 -> V2: s/INDIR_/INDIRECT_/ in ABI
s/INDIR_BRANCH/IB/ for internal functions and defines s/app2app/user/
Merge the DISABLE cases
--- Documentation/userspace-api/spec_ctrl.rst | 9 ++++ arch/x86/include/asm/nospec-branch.h | 1 arch/x86/kernel/cpu/bugs.c | 67 ++++++++++++++++++++++++++++++ include/linux/sched.h | 9 ++++ include/uapi/linux/prctl.h | 1 tools/include/uapi/linux/prctl.h | 1 6 files changed, 88 insertions(+)
Documentation/userspace-api/spec_ctrl.rst +9 -0
@@ -92,3 +92,12 @@ Speculation misfeature controls * prctl(PR_SET_SPECULATION_CTRL, PR_SPEC_STORE_BYPASS, PR_SPEC_ENABLE, 0, 0); * prctl(PR_SET_SPECULATION_CTRL, PR_SPEC_STORE_BYPASS, PR_SPEC_DISABLE, 0, 0); * prctl(PR_SET_SPECULATION_CTRL, PR_SPEC_STORE_BYPASS, PR_SPEC_FORCE_DISABLE, 0, 0); + +- PR_SPEC_INDIR_BRANCH: Indirect Branch Speculation in User Processes + (Mitigate Spectre V2 style attacks against user processes) + + Invocations: + * prctl(PR_GET_SPECULATION_CTRL, PR_SPEC_INDIRECT_BRANCH, 0, 0, 0); + * prctl(PR_SET_SPECULATION_CTRL, PR_SPEC_INDIRECT_BRANCH, PR_SPEC_ENABLE, 0, 0); + * prctl(PR_SET_SPECULATION_CTRL, PR_SPEC_INDIRECT_BRANCH, PR_SPEC_DISABLE, 0, 0); + * prctl(PR_SET_SPECULATION_CTRL, PR_SPEC_INDIRECT_BRANCH, PR_SPEC_FORCE_DISABLE, 0, 0);
arch/x86/include/asm/nospec-branch.h +1 -0
@@ -232,6 +232,7 @@ enum spectre_v2_mitigation {enum spectre_v2_user_mitigation {SPECTRE_V2_USER_NONE ,SPECTRE_V2_USER_STRICT , +SPECTRE_V2_USER_PRCTL , };/* The Speculative Store Bypass disable variants */
arch/x86/kernel/cpu/bugs.c +67 -0
108 lines not on this page · raw message
include/linux/sched.h +9 -0
@@ -1453,6 +1453,8 @@ static inline bool is_percpu_thread(void#define PFA_SPREAD_SLAB 2/* Spread some slab caches over cpuset */ #define PFA_SPEC_SSB_DISABLE 3/* Speculative Store Bypass disabled */ #define PFA_SPEC_SSB_FORCE_DISABLE 4/* Speculative Store Bypass force disabled*/ +#define PFA_SPEC_IB_DISABLE 5/* Indirect branch speculation restricted */ +#define PFA_SPEC_IB_FORCE_DISABLE 6/* Indirect branch speculation permanently restricted */ #define TASK_PFA_TEST (name, func) \ static inline bool task_##func(struct task_struct *p) \
@@ -1484,6 +1486,13 @@ TASK_PFA_CLEAR(SPEC_SSB_DISABLE, spec_ss TASK_PFA_TEST(SPEC_SSB_FORCE_DISABLE, spec_ssb_force_disable) TASK_PFA_SET(SPEC_SSB_FORCE_DISABLE, spec_ssb_force_disable) +TASK_PFA_TEST(SPEC_IB_DISABLE, spec_ib_disable) +TASK_PFA_SET(SPEC_IB_DISABLE, spec_ib_disable) +TASK_PFA_CLEAR(SPEC_IB_DISABLE, spec_ib_disable) + +TASK_PFA_TEST(SPEC_IB_FORCE_DISABLE, spec_ib_force_disable) +TASK_PFA_SET(SPEC_IB_FORCE_DISABLE, spec_ib_force_disable) +static inline void current_restore_flags (unsigned long orig_flags,unsigned long flags) {
include/uapi/linux/prctl.h +1 -0
@@ -212,6 +212,7 @@ struct prctl_mm_map {#define PR_SET_SPECULATION_CTRL 53/* Speculation control variants */ # define PR_SPEC_STORE_BYPASS 0 +# define PR_SPEC_INDIRECT_BRANCH 1/* Return and control values for PR_SET/GET_SPECULATION_CTRL */ # define PR_SPEC_NOT_AFFECTED 0# define PR_SPEC_PRCTL (1UL << 0)
tools/include/uapi/linux/prctl.h +1 -0
@@ -212,6 +212,7 @@ struct prctl_mm_map {#define PR_SET_SPECULATION_CTRL 53/* Speculation control variants */ # define PR_SPEC_STORE_BYPASS 0 +# define PR_SPEC_INDIRECT_BRANCH 1/* Return and control values for PR_SET/GET_SPECULATION_CTRL */ # define PR_SPEC_NOT_AFFECTED 0# define PR_SPEC_PRCTL (1UL << 0)
tip-bot for Thomas Gleixner
merge bot[tip:x86/pti] x86/speculation: Add prctl() control for indirect branch speculation
x86/speculation: Add prctl() control for indirect branch speculation
Commit-ID: 9137bb27e60e554dab694eafa4cca241fa3a694f Gitweb: https://git.kernel.org/tip/9137bb27e60e554dab694eafa4cca241fa3a694f Author: Thomas Gleixner <[email protected]> AuthorDate: Sun, 25 Nov 2018 19:33:53 +0100 Committer: Thomas Gleixner <[email protected]> CommitDate: Wed, 28 Nov 2018 11:57:13 +0100
x86/speculation: Add prctl() control for indirect branch speculation
Add the PR_SPEC_INDIRECT_BRANCH option for the PR_GET_SPECULATION_CTRL and PR_SET_SPECULATION_CTRL prctls to allow fine grained per task control of indirect branch speculation via STIBP and IBPB.
Invocations:
Check indirect branch speculation status with
- prctl(PR_GET_SPECULATION_CTRL, PR_SPEC_INDIRECT_BRANCH, 0, 0, 0);
Enable indirect branch speculation with
- prctl(PR_SET_SPECULATION_CTRL, PR_SPEC_INDIRECT_BRANCH, PR_SPEC_ENABLE, 0, 0);
Disable indirect branch speculation with
- prctl(PR_SET_SPECULATION_CTRL, PR_SPEC_INDIRECT_BRANCH, PR_SPEC_DISABLE, 0, 0);
Force disable indirect branch speculation with
- prctl(PR_SET_SPECULATION_CTRL, PR_SPEC_INDIRECT_BRANCH, PR_SPEC_FORCE_DISABLE, 0, 0);
See Documentation/userspace-api/spec_ctrl.rst.
--- Documentation/userspace-api/spec_ctrl.rst | 9 +++++ arch/x86/include/asm/nospec-branch.h | 1 + arch/x86/kernel/cpu/bugs.c | 67 +++++++++++++++++++++++++++++++ arch/x86/kernel/process.c | 5 +++ include/linux/sched.h | 9 +++++ include/uapi/linux/prctl.h | 1 + tools/include/uapi/linux/prctl.h | 1 + 7 files changed, 93 insertions(+)
Documentation/userspace-api/spec_ctrl.rst +9 -0
@@ -92,3 +92,12 @@ Speculation misfeature controls * prctl(PR_SET_SPECULATION_CTRL, PR_SPEC_STORE_BYPASS, PR_SPEC_ENABLE, 0, 0); * prctl(PR_SET_SPECULATION_CTRL, PR_SPEC_STORE_BYPASS, PR_SPEC_DISABLE, 0, 0); * prctl(PR_SET_SPECULATION_CTRL, PR_SPEC_STORE_BYPASS, PR_SPEC_FORCE_DISABLE, 0, 0); + +- PR_SPEC_INDIR_BRANCH: Indirect Branch Speculation in User Processes + (Mitigate Spectre V2 style attacks against user processes) + + Invocations: + * prctl(PR_GET_SPECULATION_CTRL, PR_SPEC_INDIRECT_BRANCH, 0, 0, 0); + * prctl(PR_SET_SPECULATION_CTRL, PR_SPEC_INDIRECT_BRANCH, PR_SPEC_ENABLE, 0, 0); + * prctl(PR_SET_SPECULATION_CTRL, PR_SPEC_INDIRECT_BRANCH, PR_SPEC_DISABLE, 0, 0); + * prctl(PR_SET_SPECULATION_CTRL, PR_SPEC_INDIRECT_BRANCH, PR_SPEC_FORCE_DISABLE, 0, 0);
arch/x86/include/asm/nospec-branch.h +1 -0
@@ -232,6 +232,7 @@ enum spectre_v2_mitigation {enum spectre_v2_user_mitigation {SPECTRE_V2_USER_NONE ,SPECTRE_V2_USER_STRICT , +SPECTRE_V2_USER_PRCTL , };/* The Speculative Store Bypass disable variants */
arch/x86/kernel/cpu/bugs.c +67 -0
108 lines not on this page · raw message
arch/x86/kernel/process.c +5 -0
@@ -450,6 +450,11 @@ static unsigned long speculation_ctrl_update_tif(struct task_struct *tsk)set_tsk_thread_flag (tsk,TIF_SSBD ); elseclear_tsk_thread_flag (tsk ,TIF_SSBD ); + +if (task_spec_ib_disable (tsk)) +set_tsk_thread_flag (tsk,TIF_SPEC_IB ); +else +clear_tsk_thread_flag (tsk,TIF_SPEC_IB ); }/* Return the updated threadinfo flags*/ return task_thread_info (tsk)->flags;
include/linux/sched.h +9 -0
@@ -1453,6 +1453,8 @@ static inline bool is_percpu_thread(void)#define PFA_SPREAD_SLAB 2/* Spread some slab caches over cpuset */ #define PFA_SPEC_SSB_DISABLE 3/* Speculative Store Bypass disabled */ #define PFA_SPEC_SSB_FORCE_DISABLE 4/* Speculative Store Bypass force disabled*/ +#define PFA_SPEC_IB_DISABLE 5/* Indirect branch speculation restricted */ +#define PFA_SPEC_IB_FORCE_DISABLE 6/* Indirect branch speculation permanently restricted */ #define TASK_PFA_TEST (name, func) \ static inline bool task_##func(struct task_struct *p) \
@@ -1484,6 +1486,13 @@ TASK_PFA_CLEAR(SPEC_SSB_DISABLE, spec_ssb_disable) TASK_PFA_TEST(SPEC_SSB_FORCE_DISABLE, spec_ssb_force_disable) TASK_PFA_SET(SPEC_SSB_FORCE_DISABLE, spec_ssb_force_disable) +TASK_PFA_TEST(SPEC_IB_DISABLE, spec_ib_disable) +TASK_PFA_SET(SPEC_IB_DISABLE, spec_ib_disable) +TASK_PFA_CLEAR(SPEC_IB_DISABLE, spec_ib_disable) + +TASK_PFA_TEST(SPEC_IB_FORCE_DISABLE, spec_ib_force_disable) +TASK_PFA_SET(SPEC_IB_FORCE_DISABLE, spec_ib_force_disable) +static inline void current_restore_flags (unsigned long orig_flags,unsigned long flags) {
include/uapi/linux/prctl.h +1 -0
@@ -212,6 +212,7 @@ struct prctl_mm_map {#define PR_SET_SPECULATION_CTRL 53/* Speculation control variants */ # define PR_SPEC_STORE_BYPASS 0 +# define PR_SPEC_INDIRECT_BRANCH 1/* Return and control values for PR_SET/GET_SPECULATION_CTRL */ # define PR_SPEC_NOT_AFFECTED 0# define PR_SPEC_PRCTL (1UL << 0)
tools/include/uapi/linux/prctl.h +1 -0
@@ -212,6 +212,7 @@ struct prctl_mm_map {#define PR_SET_SPECULATION_CTRL 53/* Speculation control variants */ # define PR_SPEC_STORE_BYPASS 0 +# define PR_SPEC_INDIRECT_BRANCH 1/* Return and control values for PR_SET/GET_SPECULATION_CTRL */ # define PR_SPEC_NOT_AFFECTED 0# define PR_SPEC_PRCTL (1UL << 0)
PATCH v2 26/28 x86/speculation: Enable prctl mode for spectre_v2_user
In reply to Thomas GleixnerNow 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.
---
V1 -> V2: Fix comments
s/app2app/user/
Make the IBPB printout depend on the static keys as it can be independent of the mode, e.g. when SMT is not supported. Remove the CONFIG_SMP conditional
--- Documentation/admin-guide/kernel-parameters.txt | 7 +++- arch/x86/kernel/cpu/bugs.c | 41 ++++++++++++++++++------ 2 files changed, 38 insertions(+), 10 deletions(-)
Documentation/admin-guide/kernel-parameters.txt +6 -1
@@ -4236,9 +4236,14 @@ off - Unconditionally disable mitigations. Is enforced by spectre_v2=off + prctl - Indirect branch speculation is enabled, + but mitigation can be enabled via prctl + per thread. The mitigation control state + is inherited on fork. + auto - Kernel selects the mitigation depending on the available CPU features and vulnerability. - Default is off. + Default is prctl. Not specifying this option is equivalent to spectre_v2_user=auto.
arch/x86/kernel/cpu/bugs.c +32 -9
106 lines not on this page · raw message
Re: [patch V2 26/28] x86/speculation: Enable prctl mode for spectre_v2_user
Documentation/admin-guide/kernel-parameters.txt line 4240
Quoted 2 lines · Thomas Gleixner
Documentation/admin-guide/kernel-parameters.txt +2
-0+ prctl - Indirect branch speculation is enabled, + but mitigation can be enabled via prctl
s/can be/is only/ or "must be".
Thanks,
Dominik
tip-bot for Thomas Gleixner
merge bot[tip:x86/pti] x86/speculation: Enable prctl mode for spectre_v2_user
In reply to Thomas Gleixnerx86/speculation: Enable prctl mode for spectre_v2_user
Commit-ID: 7cc765a67d8e04ef7d772425ca5a2a1e2b894c15 Gitweb: https://git.kernel.org/tip/7cc765a67d8e04ef7d772425ca5a2a1e2b894c15 Author: Thomas Gleixner <[email protected]> AuthorDate: Sun, 25 Nov 2018 19:33:54 +0100 Committer: Thomas Gleixner <[email protected]> CommitDate: Wed, 28 Nov 2018 11:57:13 +0100
x86/speculation: Enable prctl mode for spectre_v2_user
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.
--- Documentation/admin-guide/kernel-parameters.txt | 7 ++++- arch/x86/kernel/cpu/bugs.c | 41 +++++++++++++++++++------ 2 files changed, 38 insertions(+), 10 deletions(-)
Documentation/admin-guide/kernel-parameters.txt +6 -1
@@ -4236,9 +4236,14 @@ off - Unconditionally disable mitigations. Is enforced by spectre_v2=off + prctl - Indirect branch speculation is enabled, + but mitigation can be enabled via prctl + per thread. The mitigation control state + is inherited on fork. + auto - Kernel selects the mitigation depending on the available CPU features and vulnerability. - Default is off. + Default is prctl. Not specifying this option is equivalent to spectre_v2_user=auto.
arch/x86/kernel/cpu/bugs.c +32 -9
106 lines not on this page · raw message
PATCH v2 27/28 x86/speculation: Add seccomp Spectre v2 user space protection mode
In reply to Thomas GleixnerIf 'prctl' mode of user space protection from spectre v2 is selected on the kernel command-line, STIBP and IBPB are applied on tasks which restrict their indirect branch speculation via prctl.
SECCOMP enables the SSBD mitigation for sandboxed tasks already, so it makes sense to prevent spectre v2 user space to user space attacks as well.
The mitigation guide documents how STIPB works:
Setting bit 1 (STIBP) of the IA32_SPEC_CTRL MSR on a logical processor prevents the predicted targets of indirect branches on any logical processor of that core from being controlled by software that executes (or executed previously) on another logical processor of the same core.
Ergo setting STIBP protects the task itself from being attacked from a task running on a different hyper-thread and protects the tasks running on different hyper-threads from being attacked.
IBPB is issued when the task switches out, so malicious sandbox code cannot mistrain the branch predictor for the next user space task on the same logical processor.
--- Documentation/admin-guide/kernel-parameters.txt | 9 ++++++++- arch/x86/include/asm/nospec-branch.h | 1 + arch/x86/kernel/cpu/bugs.c | 17 ++++++++++++++++- 3 files changed, 25 insertions(+), 2 deletions(-)
Documentation/admin-guide/kernel-parameters.txt +8 -1
@@ -4241,9 +4241,16 @@ per thread. The mitigation control state is inherited on fork. + seccomp + - Same as "prctl" above, but all seccomp + threads will enable the mitigation unless + they explicitly opt out. + auto - Kernel selects the mitigation depending on the available CPU features and vulnerability. - Default is prctl. + + Default mitigation: + If CONFIG_SECCOMP=y "seccomp", otherwise "prctl" Not specifying this option is equivalent to spectre_v2_user=auto.
arch/x86/include/asm/nospec-branch.h +1 -0
@@ -233,6 +233,7 @@ enum spectre_v2_user_mitigation {SPECTRE_V2_USER_NONE ,SPECTRE_V2_USER_STRICT ,SPECTRE_V2_USER_PRCTL , + SPECTRE_V2_USER_SECCOMP, };/* The Speculative Store Bypass disable variants */
arch/x86/kernel/cpu/bugs.c +16 -1
@@ -256,12 +256,14 @@ enum spectre_v2_user_cmd {SPECTRE_V2_USER_CMD_AUTO ,SPECTRE_V2_USER_CMD_FORCE ,SPECTRE_V2_USER_CMD_PRCTL , + SPECTRE_V2_USER_CMD_SECCOMP, };static const char *const spectre_v2_user_strings[] = { [SPECTRE_V2_USER_NONE ] ="User space: Vulnerable" , [SPECTRE_V2_USER_STRICT ] ="User space: Mitigation: STIBP protection" , [SPECTRE_V2_USER_PRCTL ] ="User space: Mitigation: STIBP via prctl" , + [SPECTRE_V2_USER_SECCOMP ] ="User space: Mitigation: STIBP via seccomp and prctl" , }; static const struct {
@@ -273,6 +275,7 @@ static const struct { { "off", SPECTRE_V2_USER_CMD_NONE, false }, { "on", SPECTRE_V2_USER_CMD_FORCE, true }, { "prctl", SPECTRE_V2_USER_CMD_PRCTL, false }, + { "seccomp", SPECTRE_V2_USER_CMD_SECCOMP, false }, };static void __init spec_v2_user_print_cond (const char *reason,bool secure)
@@ -332,10 +335,16 @@ spectre_v2_user_select_mitigation(enum scase SPECTRE_V2_USER_CMD_FORCE : mode =SPECTRE_V2_USER_STRICT ;break ; -case SPECTRE_V2_USER_CMD_AUTO :case SPECTRE_V2_USER_CMD_PRCTL : mode =SPECTRE_V2_USER_PRCTL ;break ; +case SPECTRE_V2_USER_CMD_AUTO : +case SPECTRE_V2_USER_CMD_SECCOMP : +if (IS_ENABLED (CONFIG_SECCOMP )) + mode =SPECTRE_V2_USER_SECCOMP ; +else + mode =SPECTRE_V2_USER_PRCTL ; +break ; }/* Initialize Indirect Branch Prediction Barrier */
@@ -347,6 +356,7 @@ spectre_v2_user_select_mitigation(enum sstatic_branch_enable (&switch_mm_always_ibpb);break ;case SPECTRE_V2_USER_PRCTL : +case SPECTRE_V2_USER_SECCOMP :static_branch_enable (&switch_mm_cond_ibpb);break ;default :
@@ -591,6 +601,7 @@ void arch_smt_update(void)update_stibp_strict ();break ;case SPECTRE_V2_USER_PRCTL : +case SPECTRE_V2_USER_SECCOMP :update_indir_branch_cond ();break ; }
@@ -837,6 +848,8 @@ void arch_seccomp_spec_mitigate(struct t {if (ssb_mode ==SPEC_STORE_BYPASS_SECCOMP )ssb_prctl_set (task,PR_SPEC_FORCE_DISABLE ); +if (spectre_v2_user ==SPECTRE_V2_USER_SECCOMP ) +ib_prctl_set (task,PR_SPEC_FORCE_DISABLE ); }#endif
@@ -868,6 +881,7 @@ static int ib_prctl_get(struct task_strucase SPECTRE_V2_USER_NONE :return PR_SPEC_ENABLE ;case SPECTRE_V2_USER_PRCTL : +case SPECTRE_V2_USER_SECCOMP :if (task_spec_ib_force_disable (task))return PR_SPEC_PRCTL |PR_SPEC_FORCE_DISABLE ;if (test_tsk_thread_flag (task,TIF_SPEC_IB ))
@@ -1067,6 +1081,7 @@ static char *stibp_state(void)case SPECTRE_V2_USER_STRICT :return ", STIBP: forced" ;case SPECTRE_V2_USER_PRCTL : +case SPECTRE_V2_USER_SECCOMP :if (static_key_enabled (&switch_to_cond_stibp))return ", STIBP: conditional" ; }
Randy Dunlap
reviewerRe: [patch V2 27/28] x86/speculation: Add seccomp Spectre v2 user space protection mode
Hi,
Can you alter this without publishing a v3?
(see below)
Quoted 7 lines · Thomas Gleixner
Documentation/admin-guide/kernel-parameters.txt +1
-0@@ -4241,9 +4241,16 @@ per thread. The mitigation control state is inherited on fork. + seccomp
Documentation/admin-guide/kernel-parameters.txt line 4253
Quoted 10 lines
Documentation/admin-guide/kernel-parameters.txt +7
-1+ - Same as "prctl" above, but all seccomp + threads will enable the mitigation unless + they explicitly opt out. + auto - Kernel selects the mitigation depending on the available CPU features and vulnerability. - Default is prctl. + + Default mitigation: + If CONFIG_SECCOMP=y "seccomp", otherwise "prctl"
If CONFIG_SECCOMP=y then "seccomp", otherwise "prctl".
Documentation/admin-guide/kernel-parameters.txt line 4256
Quoted 3 lines
Documentation/admin-guide/kernel-parameters.txt +0
-0Not specifying this option is equivalent to spectre_v2_user=auto.
g'day.
Signature · 1 line
~Randy
Re: [patch V2 27/28] x86/speculation: Add seccomp Spectre v2 user space protection mode
In reply to Thomas Gleixner[ You forgot to fix your quilt setup.. ]
Quoted 7 lines · Thomas Gleixner
The mitigation guide documents how STIPB works:
Setting bit 1 (STIBP) of the
IA32_SPEC_CTRLMSR on a logical processor prevents the predicted targets of indirect branches on any logical processor of that core from being controlled by software that executes (or executed previously) on another logical processor of the same core.
Can we please just fix this stupid lie?
Yes, Intel calls it "STIBP" and tries to make it out to be about the indirect branch predictor being per-SMT thread.
But the reason it is unacceptable is apparently because in reality it just disables indirect branch prediction entirely. So yes, *technically* it's true that that limits indirect branch prediction to just a single SMT core, but in reality it is just a "go really slow" mode.
If STIBP had actually just keyed off the logical SMT thread, we wouldn't need to have worried about it in the first place.
So let's document reality rather than Intel's Pollyanna world-view.
Reality matters. It's why we had to go all this. Lying about things and making it appear like it's not a big deal was why the original patch made it through without people noticing.
Linus
Re: [patch V2 27/28] x86/speculation: Add seccomp Spectre v2 user space protection mode
Quoted 17 lines · Linus Torvalds
Quoted 6 lines
The mitigation guide documents how STIPB works:
Setting bit 1 (STIBP) of the
IA32_SPEC_CTRLMSR on a logical processor prevents the predicted targets of indirect branches on any logical processor of that core from being controlled by software that executes (or executed previously) on another logical processor of the same core.Can we please just fix this stupid lie?
Yes, Intel calls it "STIBP" and tries to make it out to be about the indirect branch predictor being per-SMT thread.
But the reason it is unacceptable is apparently because in reality it just disables indirect branch prediction entirely. So yes, *technically* it's true that that limits indirect branch prediction to just a single SMT core, but in reality it is just a "go really slow" mode.
Quoted 8 lines
If STIBP had actually just keyed off the logical SMT thread, we wouldn't need to have worried about it in the first place.
So let's document reality rather than Intel's Pollyanna world-view.
Reality matters. It's why we had to go all this. Lying about things and making it appear like it's not a big deal was why the original patch made it through without people noticing.
Yeah, exactly; the documentation doesn't discourage STIBP use (well, the AMD one now actually does).
I am all in favor of documenting the truth rather than the documented behavior, but I guess without having a word from CPU folks, explaining how exactly this is implemented in reality, we can just guess based on observed symptoms (which is what we'll do anyway I guess if we don't get any better / more accurate wording).
Arjan, Tim, would you have a wording handy that would be guaranteed to describe the reality for the sake of changelog?
Thanks,
Signature · 2 lines
Jiri Kosina SUSE Labs
Re: [patch V2 27/28] x86/speculation: Add seccomp Spectre v2 user space protection mode
In reply to Linus TorvaldsQuoted 1 line · Linus Torvalds
[ You forgot to fix your quilt setup.. ]
Duh. Should have pinned that package.
Quoted 10 lines · Thomas Gleixner
Quoted 7 lines
The mitigation guide documents how STIPB works:
Setting bit 1 (STIBP) of the
IA32_SPEC_CTRLMSR on a logical processor prevents the predicted targets of indirect branches on any logical processor of that core from being controlled by software that executes (or executed previously) on another logical processor of the same core.Can we please just fix this stupid lie?
Well, it's not a lie. The above is correct, it just does not tell WHY this works.
Quoted 7 lines
Yes, Intel calls it "STIBP" and tries to make it out to be about the indirect branch predictor being per-SMT thread.
But the reason it is unacceptable is apparently because in reality it just disables indirect branch prediction entirely. So yes, *technically* it's true that that limits indirect branch prediction to just a single SMT core, but in reality it is just a "go really slow" mode.
Indeed. Just checked the documentation again, it's also not clear whether IBPB is required if STIPB is in use.
Thanks,
tglx
Re: [patch V2 27/28] x86/speculation: Add seccomp Spectre v2 user space protection mode
Quoted 15 lines · Thomas Gleixner
Quoted 1 line · Linus Torvalds
[ You forgot to fix your quilt setup.. ]
Duh. Should have pinned that package.
Quoted 9 lines · Thomas Gleixner
Quoted 7 lines
The mitigation guide documents how STIPB works:
Setting bit 1 (STIBP) of the
IA32_SPEC_CTRLMSR on a logical processor prevents the predicted targets of indirect branches on any logical processor of that core from being controlled by software that executes (or executed previously) on another logical processor of the same core.
Quoted 4 lines
Quoted 1 line
Can we please just fix this stupid lie?
Well, it's not a lie. The above is correct, it just does not tell WHY this works.
Well, it's a "technically correct but misleading" phrase, which has much more of the effects of an actual "lie" than that of a true description of it.
I.e. in terms of what effects it's likely going to have on readers not aware of the underlying mechanics it's much more correct to call it a "lie" than to call it "truth" - which I think is at the core of Linus's argument.
Quoted 11 lines
Quoted 8 lines
Yes, Intel calls it "STIBP" and tries to make it out to be about the indirect branch predictor being per-SMT thread.
But the reason it is unacceptable is apparently because in reality it just disables indirect branch prediction entirely. So yes, *technically* it's true that that limits indirect branch prediction to just a single SMT core, but in reality it is just a "go really slow" mode.
Indeed. Just checked the documentation again, it's also not clear whether IBPB is required if STIPB is in use.
So I think we should clarify all this.
Thanks,
Ingo
Re: [patch V2 27/28] x86/speculation: Add seccomp Spectre v2 user space protection mode
In reply to Thomas GleixnerHello,
Quoted 2 lines · Thomas Gleixner
Indeed. Just checked the documentation again, it's also not clear whether IBPB is required if STIPB is in use.
I tried to ask this question too earlier:
https://lkml.kernel.org/r/[email protected]
If the BTB mistraining in SECCOMP context with STIBP set in SPEC_CTRL, can still influence the hyperthreading sibling after STIBP is cleared, IBPB is needed before clearing STIBP. Otherwise it's not. Unless told otherwise, it'd be safe to assume IBPB is needed in such case.
The SPEC_CTRL MSR specs seems a catch-all lowest common denominator and so intuition or measurement of the exact behavior in one CPU model, don't necessarily give a result that can be applied to all microcodes out there.
Thanks,
Andrea
Re: [patch V2 27/28] x86/speculation: Add seccomp Spectre v2 user space protection mode
Quoted 6 lines · Andrea Arcangeli
Hello,
Quoted 2 lines · Thomas Gleixner
Indeed. Just checked the documentation again, it's also not clear whether IBPB is required if STIPB is in use.
Quoted 8 lines
I tried to ask this question too earlier:
https://lkml.kernel.org/r/[email protected]
If the BTB mistraining in SECCOMP context with STIBP set in
SPEC_CTRL, can still influence the hyperthreading sibling after STIBP is cleared, IBPB is needed before clearing STIBP. Otherwise it's not. Unless told otherwise, it'd be safe to assume IBPB is needed in such case.
IBPB is still issued. I won't change that before we have clarification.
But I doubt it's necessary. STIBP seems to be a rather big hammer.
Thanks,
tglx
Re: [patch V2 27/28] x86/speculation: Add seccomp Spectre v2 user space protection mode
Quoted 10 lines · Thomas Gleixner
Quoted 8 lines · Andrea Arcangeli
Hello,
Quoted 2 lines · Thomas Gleixner
Indeed. Just checked the documentation again, it's also not clear whether IBPB is required if STIPB is in use.
I tried to ask this question too earlier:
Quoted 8 lines
Quoted 6 lines
https://lkml.kernel.org/r/[email protected]
If the BTB mistraining in SECCOMP context with STIBP set in
SPEC_CTRL, can still influence the hyperthreading sibling after STIBP is cleared, IBPB is needed before clearing STIBP. Otherwise it's not. Unless told otherwise, it'd be safe to assume IBPB is needed in such case.IBPB is still issued. I won't change that before we have clarification.
From an AMD standpoint, we recommend still issuing the IBPB.
Quoted 2 lines
But I doubt it's necessary. STIBP seems to be a rather big hammer.
For AMD parts that support STIBP, you will see likely differing levels of performance impact. AMD also has a CPUID bit (0x8000_0008_EBX[17]) that indicates STIBP always on mode is preferred [1] to toggling the MSR.
I was planning to do a follow on patch set for that support after this series is accepted rather than ask that it be added to this series at this time (unless folks would prefer that it be done now?).
Thanks,
Tom
Quoted 5 lines
Thanks,
tglx
Re: [patch V2 27/28] x86/speculation: Add seccomp Spectre v2 user space protection mode
Quoted 13 lines · Thomas
Quoted 12 lines · Thomas Gleixner
Quoted 10 lines · Andrea Arcangeli
Hello,
Quoted 2 lines · Thomas Gleixner
Indeed. Just checked the documentation again, it's also not clear whether IBPB is required if STIPB is in use.
I tried to ask this question too earlier:
Quoted 9 lines
Quoted 6 lines
Quoted 4 lines
If the BTB mistraining in SECCOMP context with STIBP set in
SPEC_CTRL, can still influence the hyperthreading sibling after STIBP is cleared, IBPB is needed before clearing STIBP. Otherwise it's not. Unless told otherwise, it'd be safe to assume IBPB is needed in such case.IBPB is still issued. I won't change that before we have clarification.
From an AMD standpoint, we recommend still issuing the IBPB.
Yes, our Intel HW architect also recommends still issuing the IBPB. We're now getting approval for some additional explanations of STIBP. Those additional explanations should help clarify things.
Thanks.
Tim
Re: [patch V2 27/28] x86/speculation: Add seccomp Spectre v2 user space protection mode
In reply to Linus TorvaldsQuoted 21 lines · Linus Torvalds
[ You forgot to fix your quilt setup.. ]
Quoted 7 lines · Thomas Gleixner
The mitigation guide documents how STIPB works:
Setting bit 1 (STIBP) of the
IA32_SPEC_CTRLMSR on a logical processor prevents the predicted targets of indirect branches on any logical processor of that core from being controlled by software that executes (or executed previously) on another logical processor of the same core.Can we please just fix this stupid lie?
Yes, Intel calls it "STIBP" and tries to make it out to be about the indirect branch predictor being per-SMT thread.
But the reason it is unacceptable is apparently because in reality it just disables indirect branch prediction entirely. So yes, *technically* it's true that that limits indirect branch prediction to just a single SMT core, but in reality it is just a "go really slow" mode.
Quoted 9 lines
If STIBP had actually just keyed off the logical SMT thread, we wouldn't need to have worried about it in the first place.
So let's document reality rather than Intel's Pollyanna world-view.
Reality matters. It's why we had to go all this. Lying about things and making it appear like it's not a big deal was why the original patch made it through without people noticing.
To make the usage of STIBP and its working principles clear, here are some additional explanations of STIBP from our Intel HW architects. This should also help answer some of the questions from Thomas and others on STIBP's usages with IBPB and IBRS.
Thanks.
Tim
---
STIBP
^^^^^
Implementations of STIBP on existing Core-family processors (where STIBP functionality was added through a microcode update) work by disabling branch predictors that both:
1. Contain indirect branch predictions for both hardware threads, and
2. Do not contain a dedicated thread ID bit
Unlike IBRS and IBPB, STIBP does not affect all branch predictors that contain indirect branch predictions. STIBP only affects those branch predictors where software on one hardware thread can create a prediction that can then be used by the other hardware thread. This is part of what makes STIBP have lower performance overhead than IBRS on current implementations.
IBRS is a superset of STIBP functionality; thus, setting both STIBP and IBRS is redundant. On processors without enhanced IBRS, we recommend using retpoline or setting IBRS only during ring 0 and VMM modes. IBPB should be used when switching to a different process/guest that does not trust the last process/guest that ran on a particular hardware thread. For performance reasons, IBRS should not be left set during application execution.
Processes that are particularly security-sensitive may set STIBP when they execute to prevent their indirect branch predictions from being controlled by another hardware thread on the same physical core. On existing Core-family processors, this comes at significant performance cost to both hardware threads due to disabling some indirect branch predictors (as described earlier). Because of this, we do not recommend setting STIBP during all application execution.
STIBP is architecturally defined to apply to all hardware threads on the physical core on which it is set. Because of this, STIBP can be set when running an untrusted process to ensure that the untrusted process does not control the indirect branch predictions of software running on other hardware threads (for example, threads that do not have STIBP or IBRS set) while STIBP is still set. Before running with both STIBP and IBRS cleared, an IBPB can be executed to ensure that any indirect branch predictions that were installed by the untrusted process while STIBP was set are not used by the other hardware thread once STIBP and IBRS are cleared. Regardless of the usage model, STIBP should be used judiciously due to its impact on performance.
Enhanced IBRS is a feature that also provides a superset of STIBP functionality; therefore it is redundant to set both STIBP and enhanced IBRS. Processors with enhanced IBRS add a thread ID bit to the needed indirect branch predictors and use that bit to ensure that indirect branch predictions are only used by the thread that created them.
On processors with enhanced IBRS support, we recommend setting IBRS to 1 and left set. The traditional IBRS model of setting IBRS only during ring 0 execution is just as secure on parts with enhanced IBRS support as it is on parts with vanilla IBRS, but the WRMSRs on ring transitions and/or VM exit/entry will cost performance compared to just leaving IBRS set. Again, there is no need to use STIBP when IBRS is set. However, IBPB should still be used when switching to a different application/guest that does not trust the last application/guest that ran on a particular hardware thread. Guests in a VM migration pool that includes hardware without enhanced IBRS may not have IA32_ARCH_CAPABILITIES.IBRS_ALL (enhanced IBRS) enumerated to them and thus may use the traditional IBRS usage model of setting IBRS only in ring 0. For performance reasons, once a guest has been shown to frequently write IA32_SPEC_CTRL, we do not recommend that the VMM cause a VM exit on such WRMSRs. The VMM running on processors that support enhanced IBRS should allow the IA32_SPEC_CTRL-writing guest to control guest IA32_SPEC_CTRL. The VMM should thus set IBRS after VM exits from such guests to protect itself (or use alternative techniques like retpoline, secret removal, or indirect branch removal).
Re: [patch V2 27/28] x86/speculation: Add seccomp Spectre v2 user space protection mode
Quoted 21 lines · Tim Chen
Quoted 10 lines
Can we please just fix this stupid lie?
Yes, Intel calls it "STIBP" and tries to make it out to be about the indirect branch predictor being per-SMT thread.
But the reason it is unacceptable is apparently because in reality it just disables indirect branch prediction entirely. So yes, *technically* it's true that that limits indirect branch prediction to just a single SMT core, but in reality it is just a "go really slow" mode.
Quoted 9 lines
If STIBP had actually just keyed off the logical SMT thread, we wouldn't need to have worried about it in the first place.
So let's document reality rather than Intel's Pollyanna world-view.
Reality matters. It's why we had to go all this. Lying about things and making it appear like it's not a big deal was why the original patch made it through without people noticing.
Quoted 4 lines
To make the usage of STIBP and its working principles clear, here are some additional explanations of STIBP from our Intel HW architects. This should also help answer some of the questions from Thomas and others on STIBP's usages with IBPB and IBRS.
Thanks a lot, this indeed does shed some light.
I have one question though:
[ ... snip ... ]
Quoted 2 lines
On processors with enhanced IBRS support, we recommend setting IBRS to 1 and left set.
Then why doesn't CPU with EIBRS support acutally *default* to '1', with opt-out possibility for OS?
Thanks,
Signature · 2 lines
Jiri Kosina SUSE Labs
Re: [patch V2 27/28] x86/speculation: Add seccomp Spectre v2 user space protection mode
Quoted 5 lines
Quoted 2 lines
On processors with enhanced IBRS support, we recommend setting IBRS to 1 and left set.
Then why doesn't CPU with EIBRS support acutally *default* to '1', with opt-out possibility for OS?
the BIOSes could indeed get this set up this way.
do you want to trust the bios to get it right?
Re: [patch V2 27/28] x86/speculation: Add seccomp Spectre v2 user space protection mode
In reply to Jiri KosinaQuoted 5 lines
Quoted 2 lines
On processors with enhanced IBRS support, we recommend setting IBRS to 1 and left set.
Then why doesn't CPU with EIBRS support acutally *default* to '1', with opt-out possibility for OS?
(slightly longer answer)
you can pretty much assume that on these CPUs, IBRS doesn't actually do anything (e.g. just a scratch bit)
we could debate (and did :-)) for some time what the default value should be at boot, but it kind of is one of those minor issues that should not hold up getting things out.
it could well be that the cpus that do this will ship with 1 as default, but it's hard to guarantee across many products and different CPU vendors when time was tight.
Re: [patch V2 27/28] x86/speculation: Add seccomp Spectre v2 user space protection mode
In reply to Tim ChenQuoted 12 lines · Tim Chen
To make the usage of STIBP and its working principles clear, here are some additional explanations of STIBP from our Intel HW architects. This should also help answer some of the questions from Thomas and others on STIBP's usages with IBPB and IBRS.
Thanks.
Tim
---
Quoted 8 lines
STIBP
^^^^^
Implementations of STIBP on existing Core-family processors (where STIBP functionality was added through a microcode update) work by disabling branch predictors that both:1. Contain indirect branch predictions for both hardware threads, and
2. Do not contain a dedicated thread ID bit
Honestly, it still feels entirely misguided to me.
The above is not STIBP. It's just "disable IB". There's nothing "ST" about it.
So on processors where there is no thread ID bit (or per-thread predictors), Intel simply SHOULD NOT EXPOSE this at all.
As it is, I refuse to call this shit "STIBP", because on current CPU's that's simply a lie.
Being "technically correct" is not an excuse. It's just lying. I would really hope that we restrict the lying to politicians, and not do it in technical documentation.
Linus
Re: [patch V2 27/28] x86/speculation: Add seccomp Spectre v2 user space protection mode
Quoted 16 lines · Linus Torvalds
Quoted 8 lines
STIBP
^^^^^
Implementations of STIBP on existing Core-family processors (where STIBP functionality was added through a microcode update) work by disabling branch predictors that both:1. Contain indirect branch predictions for both hardware threads, and
2. Do not contain a dedicated thread ID bitHonestly, it still feels entirely misguided to me.
The above is not STIBP. It's just "disable IB". There's nothing "ST" about it.
So on processors where there is no thread ID bit (or per-thread predictors), Intel simply SHOULD NOT EXPOSE this at all.
Quoted 7 lines
As it is, I refuse to call this shit "STIBP", because on current CPU's that's simply a lie.
Being "technically correct" is not an excuse. It's just lying. I would really hope that we restrict the lying to politicians, and not do it in technical documentation.
Linus,
I consulted our HW architects to get their thinking behind the STIBP name and why it is exposed on CPUs without thread ID bit.
1) Why expose STIBP even when it is just a scratch bit?
VM migration pools prefer that bits which guests have direct access to (as we recommend for IA32_SPEC_CTRL) do not cause #GP when they are migrated to different processors in order to prevent guests from crashing or require restricting VM migration targets. That is why we decided to allow the STIBP bit to be set and to return the last value written even on parts where it has no other effect (e.g. Atom parts without multithreading). There was also discomfort with allowing a bit to be set, to return the last value written, and to meet the architecturally documented behavior, but to not enumerate that it is supported. Not enumerating STIBP would make it more difficult for software to understand things like how the CPU does reserved bit checks. That is why we enumerate and support STIBP even when it affects no branch predictors.
2) Why did we not call STIBP "disable IB":
* It does not disable all indirect branch predictors (on current Core), just a subset
* It does not disable any indirect branch predictors (on future Core)
* It does not disable any indirect branch predictors (on Atom parts with no SMT)
* It does more than disable some indirect branch predictors (on some non-Core)
3) Philosophy for naming architectural bits.
The microcode updates and future hardware changes do a variety of different micro-architectural behaviors in order to achieve the goals behind each MSR bit. This means that the names aren't the most descriptive for each individual project. Had we instead exposed the different functionality/behavior to software for each CPU then it would have been a model-specific feature, likely needing different software behavior for different CPUID family/model/stepping. This would have been painful for the OS, and even more painful for VMMs and VM migration pools. This is why we made these bits architectural and used a common name and definition across the projects.
We don't object to the Linux community using an alternative name for STIBP (but not Disable IB), so long as it is accurate across our products. Changing our MSR name in the SDM seems like would it
cause unneeded confusion and work.
Thanks.
Tim
tip-bot for Thomas Gleixner
merge bot[tip:x86/pti] x86/speculation: Add seccomp Spectre v2 user space protection mode
In reply to Thomas Gleixnerx86/speculation: Add seccomp Spectre v2 user space protection mode
Commit-ID: 6b3e64c237c072797a9ec918654a60e3a46488e2 Gitweb: https://git.kernel.org/tip/6b3e64c237c072797a9ec918654a60e3a46488e2 Author: Thomas Gleixner <[email protected]> AuthorDate: Sun, 25 Nov 2018 19:33:55 +0100 Committer: Thomas Gleixner <[email protected]> CommitDate: Wed, 28 Nov 2018 11:57:14 +0100
x86/speculation: Add seccomp Spectre v2 user space protection mode
If 'prctl' mode of user space protection from spectre v2 is selected on the kernel command-line, STIBP and IBPB are applied on tasks which restrict their indirect branch speculation via prctl.
SECCOMP enables the SSBD mitigation for sandboxed tasks already, so it makes sense to prevent spectre v2 user space to user space attacks as well.
The Intel mitigation guide documents how STIPB works:
Setting bit 1 (STIBP) of the IA32_SPEC_CTRL MSR on a logical processor prevents the predicted targets of indirect branches on any logical processor of that core from being controlled by software that executes (or executed previously) on another logical processor of the same core.
Ergo setting STIBP protects the task itself from being attacked from a task running on a different hyper-thread and protects the tasks running on different hyper-threads from being attacked.
While the document suggests that the branch predictors are shielded between the logical processors, the observed performance regressions suggest that STIBP simply disables the branch predictor more or less completely. Of course the document wording is vague, but the fact that there is also no requirement for issuing IBPB when STIBP is used points clearly in that direction. The kernel still issues IBPB even when STIBP is used until Intel clarifies the whole mechanism.
IBPB is issued when the task switches out, so malicious sandbox code cannot mistrain the branch predictor for the next user space task on the same logical processor.
--- Documentation/admin-guide/kernel-parameters.txt | 9 ++++++++- arch/x86/include/asm/nospec-branch.h | 1 + arch/x86/kernel/cpu/bugs.c | 17 ++++++++++++++++- 3 files changed, 25 insertions(+), 2 deletions(-)
Documentation/admin-guide/kernel-parameters.txt +8 -1
@@ -4241,9 +4241,16 @@ per thread. The mitigation control state is inherited on fork. + seccomp + - Same as "prctl" above, but all seccomp + threads will enable the mitigation unless + they explicitly opt out. + auto - Kernel selects the mitigation depending on the available CPU features and vulnerability. - Default is prctl. + + Default mitigation: + If CONFIG_SECCOMP=y then "seccomp", otherwise "prctl" Not specifying this option is equivalent to spectre_v2_user=auto.
arch/x86/include/asm/nospec-branch.h +1 -0
@@ -233,6 +233,7 @@ enum spectre_v2_user_mitigation {SPECTRE_V2_USER_NONE ,SPECTRE_V2_USER_STRICT ,SPECTRE_V2_USER_PRCTL , + SPECTRE_V2_USER_SECCOMP, };/* The Speculative Store Bypass disable variants */
arch/x86/kernel/cpu/bugs.c +16 -1
@@ -256,12 +256,14 @@ enum spectre_v2_user_cmd {SPECTRE_V2_USER_CMD_AUTO ,SPECTRE_V2_USER_CMD_FORCE ,SPECTRE_V2_USER_CMD_PRCTL , + SPECTRE_V2_USER_CMD_SECCOMP, };static const char *const spectre_v2_user_strings[] = { [SPECTRE_V2_USER_NONE ] ="User space: Vulnerable" , [SPECTRE_V2_USER_STRICT ] ="User space: Mitigation: STIBP protection" , [SPECTRE_V2_USER_PRCTL ] ="User space: Mitigation: STIBP via prctl" , + [SPECTRE_V2_USER_SECCOMP ] ="User space: Mitigation: STIBP via seccomp and prctl" , }; static const struct {
@@ -273,6 +275,7 @@ static const struct { { "off", SPECTRE_V2_USER_CMD_NONE, false }, { "on", SPECTRE_V2_USER_CMD_FORCE, true }, { "prctl", SPECTRE_V2_USER_CMD_PRCTL, false }, + { "seccomp", SPECTRE_V2_USER_CMD_SECCOMP, false }, };static void __init spec_v2_user_print_cond (const char *reason,bool secure)
@@ -332,10 +335,16 @@ spectre_v2_user_select_mitigation(enum spectre_v2_mitigation_cmd v2_cmd)case SPECTRE_V2_USER_CMD_FORCE : mode =SPECTRE_V2_USER_STRICT ;break ; -case SPECTRE_V2_USER_CMD_AUTO :case SPECTRE_V2_USER_CMD_PRCTL : mode =SPECTRE_V2_USER_PRCTL ;break ; +case SPECTRE_V2_USER_CMD_AUTO : +case SPECTRE_V2_USER_CMD_SECCOMP : +if (IS_ENABLED (CONFIG_SECCOMP )) + mode =SPECTRE_V2_USER_SECCOMP ; +else + mode =SPECTRE_V2_USER_PRCTL ; +break ; }/* Initialize Indirect Branch Prediction Barrier */
@@ -347,6 +356,7 @@ spectre_v2_user_select_mitigation(enum spectre_v2_mitigation_cmd v2_cmd)static_branch_enable (&switch_mm_always_ibpb);break ;case SPECTRE_V2_USER_PRCTL : +case SPECTRE_V2_USER_SECCOMP :static_branch_enable (&switch_mm_cond_ibpb);break ;default :
@@ -591,6 +601,7 @@ void arch_smt_update(void)update_stibp_strict ();break ;case SPECTRE_V2_USER_PRCTL : +case SPECTRE_V2_USER_SECCOMP :update_indir_branch_cond ();break ; }
@@ -833,6 +844,8 @@ void arch_seccomp_spec_mitigate(struct task_struct *task) {if (ssb_mode ==SPEC_STORE_BYPASS_SECCOMP )ssb_prctl_set (task,PR_SPEC_FORCE_DISABLE ); +if (spectre_v2_user ==SPECTRE_V2_USER_SECCOMP ) +ib_prctl_set (task,PR_SPEC_FORCE_DISABLE ); }#endif
@@ -864,6 +877,7 @@ static int ib_prctl_get(struct task_struct *task)case SPECTRE_V2_USER_NONE :return PR_SPEC_ENABLE ;case SPECTRE_V2_USER_PRCTL : +case SPECTRE_V2_USER_SECCOMP :if (task_spec_ib_force_disable (task))return PR_SPEC_PRCTL |PR_SPEC_FORCE_DISABLE ;if (task_spec_ib_disable (task))
@@ -1063,6 +1077,7 @@ static char *stibp_state(void)case SPECTRE_V2_USER_STRICT :return ", STIBP: forced" ;case SPECTRE_V2_USER_PRCTL : +case SPECTRE_V2_USER_SECCOMP :if (static_key_enabled (&switch_to_cond_stibp))return ", STIBP: conditional" ; }
Re: [patch V2 27/28] x86/speculation: Add seccomp Spectre v2 user space protection mode
In reply to Thomas Gleixnerarch/x86/kernel/cpu/bugs.c enum spectre_v2_user_cmd { · line 267
Quoted 6 lines
static const char * const spectre_v2_user_strings[] = {
arch/x86/kernel/cpu/bugs.c +1
-0[SPECTRE_V2_USER_NONE] = "User space: Vulnerable", [SPECTRE_V2_USER_STRICT] = "User space: Mitigation: STIBP protection", [SPECTRE_V2_USER_PRCTL] = "User space: Mitigation: STIBP via prctl", + [SPECTRE_V2_USER_SECCOMP] = "User space: Mitigation: STIBP via seccomp and prctl", };
Since there's some heartburn about the STIBP naming, should we make this more generic? Maybe something like "SMT hardening", so it says:
"User space: Mitigation: SMT hardening via prctl"
or,
"User space: Mitigation: maybe go slow on indirect branches via prctl"
if we're trying be more precise on the effects. :)
PATCH v2 28/28 x86/speculation: Provide IBPB always command line options
In reply to Thomas GleixnerProvide the possibility to enable IBPB always in combination with 'prctl' and 'seccomp'.
Add the extra command line options and rework the IBPB selection to evaluate the command instead of the mode selected by the STIPB switch case.
--- Documentation/admin-guide/kernel-parameters.txt | 12 ++++++++ arch/x86/kernel/cpu/bugs.c | 34 ++++++++++++++++-------- 2 files changed, 35 insertions(+), 11 deletions(-)
Documentation/admin-guide/kernel-parameters.txt +12 -0
@@ -4241,11 +4241,23 @@ per thread. The mitigation control state is inherited on fork. + prctl,ibpb + - Like "prctl" above, but only STIBP is + controlled per thread. IBPB is issued + always when switching between different user + space processes. + seccomp - Same as "prctl" above, but all seccomp threads will enable the mitigation unless they explicitly opt out. + seccomp,ibpb + - Like "seccomp" above, but only STIBP is + controlled per thread. IBPB is issued + always when switching between different + user space processes. + auto - Kernel selects the mitigation depending on the available CPU features and vulnerability.
arch/x86/kernel/cpu/bugs.c +23 -11
@@ -256,7 +256,9 @@ enum spectre_v2_user_cmd {SPECTRE_V2_USER_CMD_AUTO ,SPECTRE_V2_USER_CMD_FORCE ,SPECTRE_V2_USER_CMD_PRCTL , +SPECTRE_V2_USER_CMD_PRCTL_IBPB ,SPECTRE_V2_USER_CMD_SECCOMP , + SPECTRE_V2_USER_CMD_SECCOMP_IBPB, }; static const char * const spectre_v2_user_strings[] = {
@@ -271,11 +273,13 @@ static const struct {enum spectre_v2_user_cmd cmd;bool secure; } v2_user_options[] __initdata = { - { "auto", SPECTRE_V2_USER_CMD_AUTO, false }, - { "off", SPECTRE_V2_USER_CMD_NONE, false }, - { "on", SPECTRE_V2_USER_CMD_FORCE, true }, - { "prctl", SPECTRE_V2_USER_CMD_PRCTL, false }, - { "seccomp", SPECTRE_V2_USER_CMD_SECCOMP, false }, + { "auto", SPECTRE_V2_USER_CMD_AUTO, false }, + { "off", SPECTRE_V2_USER_CMD_NONE, false }, + { "on", SPECTRE_V2_USER_CMD_FORCE, true }, + { "prctl", SPECTRE_V2_USER_CMD_PRCTL, false }, + { "prctl,ibpb", SPECTRE_V2_USER_CMD_PRCTL_IBPB, false }, + { "seccomp", SPECTRE_V2_USER_CMD_SECCOMP, false }, + { "seccomp,ibpb", SPECTRE_V2_USER_CMD_SECCOMP_IBPB, false }, }; static void __init spec_v2_user_print_cond(const char *reason, bool secure)
@@ -321,6 +325,7 @@ spectre_v2_user_select_mitigation(enum s {enum spectre_v2_user_mitigation mode =SPECTRE_V2_USER_NONE ;bool smt_possible =IS_ENABLED (CONFIG_SMP ); +enum spectre_v2_user_cmd cmd;if (!boot_cpu_has (X86_FEATURE_IBPB ) && !boot_cpu_has (X86_FEATURE_STIBP ))return ;
@@ -329,17 +334,20 @@ spectre_v2_user_select_mitigation(enum s cpu_smt_control == CPU_SMT_NOT_SUPPORTED) smt_possible = false; -switch (spectre_v2_parse_user_cmdline (v2_cmd)) { + cmd =spectre_v2_parse_user_cmdline (v2_cmd); +switch (cmd) {case SPECTRE_V2_USER_CMD_NONE : goto set_mode;case SPECTRE_V2_USER_CMD_FORCE : mode =SPECTRE_V2_USER_STRICT ;break ;case SPECTRE_V2_USER_CMD_PRCTL : +case SPECTRE_V2_USER_CMD_PRCTL_IBPB : mode =SPECTRE_V2_USER_PRCTL ;break ;case SPECTRE_V2_USER_CMD_AUTO :case SPECTRE_V2_USER_CMD_SECCOMP : +case SPECTRE_V2_USER_CMD_SECCOMP_IBPB :if (IS_ENABLED (CONFIG_SECCOMP )) mode =SPECTRE_V2_USER_SECCOMP ;else
@@ -351,12 +359,15 @@ spectre_v2_user_select_mitigation(enum sif (boot_cpu_has (X86_FEATURE_IBPB )) {setup_force_cpu_cap (X86_FEATURE_USE_IBPB ); -switch (mode) { -case SPECTRE_V2_USER_STRICT : +switch (cmd) { +case SPECTRE_V2_USER_CMD_FORCE : +case SPECTRE_V2_USER_CMD_PRCTL_IBPB : +case SPECTRE_V2_USER_CMD_SECCOMP_IBPB :static_branch_enable (&switch_mm_always_ibpb);break ; -case SPECTRE_V2_USER_PRCTL : -case SPECTRE_V2_USER_SECCOMP : +case SPECTRE_V2_USER_CMD_PRCTL : +case SPECTRE_V2_USER_CMD_AUTO : +case SPECTRE_V2_USER_CMD_SECCOMP :static_branch_enable (&switch_mm_cond_ibpb);break ;default :
@@ -364,7 +375,8 @@ spectre_v2_user_select_mitigation(enum s }pr_info ("mitigation: Enabling %s Indirect Branch Prediction Barrier\n" , - mode ==SPECTRE_V2_USER_STRICT ?"always-on" :"conditional" ); +static_key_enabled (&switch_mm_always_ibpb) ? +"always-on" :"conditional" ); }/* If enhanced IBRS is enabled no STIPB required */
tip-bot for Thomas Gleixner
merge bot[tip:x86/pti] x86/speculation: Provide IBPB always command line options
x86/speculation: Provide IBPB always command line options
Commit-ID: 55a974021ec952ee460dc31ca08722158639de72 Gitweb: https://git.kernel.org/tip/55a974021ec952ee460dc31ca08722158639de72 Author: Thomas Gleixner <[email protected]> AuthorDate: Sun, 25 Nov 2018 19:33:56 +0100 Committer: Thomas Gleixner <[email protected]> CommitDate: Wed, 28 Nov 2018 11:57:14 +0100
x86/speculation: Provide IBPB always command line options
Provide the possibility to enable IBPB always in combination with 'prctl' and 'seccomp'.
Add the extra command line options and rework the IBPB selection to evaluate the command instead of the mode selected by the STIPB switch case.
--- Documentation/admin-guide/kernel-parameters.txt | 12 +++++++++ arch/x86/kernel/cpu/bugs.c | 34 +++++++++++++++++-------- 2 files changed, 35 insertions(+), 11 deletions(-)
Documentation/admin-guide/kernel-parameters.txt +12 -0
@@ -4241,11 +4241,23 @@ per thread. The mitigation control state is inherited on fork. + prctl,ibpb + - Like "prctl" above, but only STIBP is + controlled per thread. IBPB is issued + always when switching between different user + space processes. + seccomp - Same as "prctl" above, but all seccomp threads will enable the mitigation unless they explicitly opt out. + seccomp,ibpb + - Like "seccomp" above, but only STIBP is + controlled per thread. IBPB is issued + always when switching between different + user space processes. + auto - Kernel selects the mitigation depending on the available CPU features and vulnerability.
arch/x86/kernel/cpu/bugs.c +23 -11
@@ -256,7 +256,9 @@ enum spectre_v2_user_cmd {SPECTRE_V2_USER_CMD_AUTO ,SPECTRE_V2_USER_CMD_FORCE ,SPECTRE_V2_USER_CMD_PRCTL , +SPECTRE_V2_USER_CMD_PRCTL_IBPB ,SPECTRE_V2_USER_CMD_SECCOMP , + SPECTRE_V2_USER_CMD_SECCOMP_IBPB, }; static const char * const spectre_v2_user_strings[] = {
@@ -271,11 +273,13 @@ static const struct {enum spectre_v2_user_cmd cmd;bool secure; } v2_user_options[] __initdata = { - { "auto", SPECTRE_V2_USER_CMD_AUTO, false }, - { "off", SPECTRE_V2_USER_CMD_NONE, false }, - { "on", SPECTRE_V2_USER_CMD_FORCE, true }, - { "prctl", SPECTRE_V2_USER_CMD_PRCTL, false }, - { "seccomp", SPECTRE_V2_USER_CMD_SECCOMP, false }, + { "auto", SPECTRE_V2_USER_CMD_AUTO, false }, + { "off", SPECTRE_V2_USER_CMD_NONE, false }, + { "on", SPECTRE_V2_USER_CMD_FORCE, true }, + { "prctl", SPECTRE_V2_USER_CMD_PRCTL, false }, + { "prctl,ibpb", SPECTRE_V2_USER_CMD_PRCTL_IBPB, false }, + { "seccomp", SPECTRE_V2_USER_CMD_SECCOMP, false }, + { "seccomp,ibpb", SPECTRE_V2_USER_CMD_SECCOMP_IBPB, false }, }; static void __init spec_v2_user_print_cond(const char *reason, bool secure)
@@ -321,6 +325,7 @@ spectre_v2_user_select_mitigation(enum spectre_v2_mitigation_cmd v2_cmd) {enum spectre_v2_user_mitigation mode =SPECTRE_V2_USER_NONE ;bool smt_possible =IS_ENABLED (CONFIG_SMP ); +enum spectre_v2_user_cmd cmd;if (!boot_cpu_has (X86_FEATURE_IBPB ) && !boot_cpu_has (X86_FEATURE_STIBP ))return ;
@@ -329,17 +334,20 @@ spectre_v2_user_select_mitigation(enum spectre_v2_mitigation_cmd v2_cmd) cpu_smt_control == CPU_SMT_NOT_SUPPORTED) smt_possible = false; -switch (spectre_v2_parse_user_cmdline (v2_cmd)) { + cmd =spectre_v2_parse_user_cmdline (v2_cmd); +switch (cmd) {case SPECTRE_V2_USER_CMD_NONE : goto set_mode;case SPECTRE_V2_USER_CMD_FORCE : mode =SPECTRE_V2_USER_STRICT ;break ;case SPECTRE_V2_USER_CMD_PRCTL : +case SPECTRE_V2_USER_CMD_PRCTL_IBPB : mode =SPECTRE_V2_USER_PRCTL ;break ;case SPECTRE_V2_USER_CMD_AUTO :case SPECTRE_V2_USER_CMD_SECCOMP : +case SPECTRE_V2_USER_CMD_SECCOMP_IBPB :if (IS_ENABLED (CONFIG_SECCOMP )) mode =SPECTRE_V2_USER_SECCOMP ;else
@@ -351,12 +359,15 @@ spectre_v2_user_select_mitigation(enum spectre_v2_mitigation_cmd v2_cmd)if (boot_cpu_has (X86_FEATURE_IBPB )) {setup_force_cpu_cap (X86_FEATURE_USE_IBPB ); -switch (mode) { -case SPECTRE_V2_USER_STRICT : +switch (cmd) { +case SPECTRE_V2_USER_CMD_FORCE : +case SPECTRE_V2_USER_CMD_PRCTL_IBPB : +case SPECTRE_V2_USER_CMD_SECCOMP_IBPB :static_branch_enable (&switch_mm_always_ibpb);break ; -case SPECTRE_V2_USER_PRCTL : -case SPECTRE_V2_USER_SECCOMP : +case SPECTRE_V2_USER_CMD_PRCTL : +case SPECTRE_V2_USER_CMD_AUTO : +case SPECTRE_V2_USER_CMD_SECCOMP :static_branch_enable (&switch_mm_cond_ibpb);break ;default :
@@ -364,7 +375,8 @@ spectre_v2_user_select_mitigation(enum spectre_v2_mitigation_cmd v2_cmd) }pr_info ("mitigation: Enabling %s Indirect Branch Prediction Barrier\n" , - mode ==SPECTRE_V2_USER_STRICT ?"always-on" :"conditional" ); +static_key_enabled (&switch_mm_always_ibpb) ? +"always-on" :"conditional" ); }/* If enhanced IBRS is enabled no STIPB required */
Quoted 8 lines · Thomas Gleixner
Thats hopefully the final version of this. Changes since V1:
- Renamed the command line option and related code to spectre_v2_user= as suggested by Josh.
- Thought more about the back to back optimization and finally left the IBPB code in
switch_mm().
Quoted 7 lines
It still removes the ptrace check for the always IBPB case. That's substantial overhead for dubious value now that the default is conditional (prctl/seccomp) IBPB.
- Added two options which allow conditional STIBP and IBPB always mode.
- Addressed the review comments
With the typo review feedback outlined in the discussions:
Thanks,
Ingo
Quoted 14 lines · Thomas Gleixner
Thats hopefully the final version of this. Changes since V1:
- Renamed the command line option and related code to spectre_v2_user= as suggested by Josh.
- Thought more about the back to back optimization and finally left the IBPB code in
switch_mm().It still removes the ptrace check for the always IBPB case. That's substantial overhead for dubious value now that the default is conditional (prctl/seccomp) IBPB.
- Added two options which allow conditional STIBP and IBPB always mode.
Quoted 7 lines
- Addressed the review comments
Documentation is still work in progress. Thanks Andi for providing the first draft for it.
Still based on tip.git x86/pti as it has been discussed to remove the minimal RETPOLINE bandaid from stable kernels as well.
I've integrated the latest review feedback and the change which plugs the TIF async update issue and pushed all of it out to:
git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86/pti
For the stable 4.14.y and 4.19.y trees, I've collected the missing bits and pieces and uploaded tarballs which contain everything ready for consumption:
https://tglx.de/~tglx/patches-spec-4.14.y.tar.xz
sha256 of patches-spec-4.14.y.tar:
3d2976ef06ab5556c1c6cba975b0c9390eb57f43c506fb7f8834bb484feb9b17https://tglx.de/~tglx/patches-spec-4.19.y.tar.xz
sha256 of patches-spec-4.19.y.tar:
b7666cf378ad63810a17e98a471aae81a49738c552dbe912aea49de83f8145ccThanks everyone for review, discussion, testing ... !
Thanks,
tglx
Quoted 19 lines · Thomas Gleixner
I've integrated the latest review feedback and the change which plugs the TIF async update issue and pushed all of it out to:
git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.gitx86/ptiFor the stable 4.14.y and 4.19.y trees, I've collected the missing bits and pieces and uploaded tarballs which contain everything ready for consumption:
https://tglx.de/~tglx/patches-spec-4.14.y.tar.xz sha256 of patches-spec-4.14.y.tar: 3d2976ef06ab5556c1c6cba975b0c9390eb57f43c506fb7f8834bb484feb9b17 https://tglx.de/~tglx/patches-spec-4.19.y.tar.xz sha256 of patches-spec-4.19.y.tar: b7666cf378ad63810a17e98a471aae81a49738c552dbe912aea49de83f8145cc
Quoted 1 line
Thanks everyone for review, discussion, testing ... !
Big thanks to Thomas for getting all these changes merged.
Tim
Hi!
Quoted 2 lines
Documentation/admin-guide/kernel-parameters.txt | 56 ++ Documentation/userspace-api/spec_ctrl.rst | 9
Could we name this speculation.rst instead? _ is inconsistent, and spec could be shorthand for other stuff, too...
Pavel
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html