* [v6 0/1] Introduce support for PSF control. @ 2021-05-17 22:00 Ramakrishna Saripalli 2021-05-17 22:00 ` [v6 1/1] x86/bugs: Implement mitigation for Predictive Store Forwarding Ramakrishna Saripalli 2021-06-17 20:47 ` [v6 0/1] Introduce support for PSF control Saripalli, RK 0 siblings, 2 replies; 21+ messages in thread From: Ramakrishna Saripalli @ 2021-05-17 22:00 UTC (permalink / raw) To: linux-kernel, x86, tglx, mingo, bp, hpa; +Cc: bsd, rsaripal From: Ramakrishna Saripalli <rk.saripalli@amd.com> Predictive Store Forwarding: AMD Zen3 processors feature a new technology called Predictive Store Forwarding (PSF). https://www.amd.com/system/files/documents/security-analysis-predictive-store-forwarding.pdf PSF is a hardware-based micro-architectural optimization designed to improve the performance of code execution by predicting address dependencies between loads and stores. How PSF works: It is very common for a CPU to execute a load instruction to an address that was recently written by a store. Modern CPUs implement a technique known as Store-To-Load-Forwarding (STLF) to improve performance in such cases. With STLF, data from the store is forwarded directly to the load without having to wait for it to be written to memory. In a typical CPU, STLF occurs after the address of both the load and store are calculated and determined to match. PSF expands on this by speculating on the relationship between loads and stores without waiting for the address calculation to complete. With PSF, the CPU learns over time the relationship between loads and stores. If STLF typically occurs between a particular store and load, the CPU will remember this. In typical code, PSF provides a performance benefit by speculating on the load result and allowing later instructions to begin execution sooner than they otherwise would be able to. Causes of Incorrect PSF: Incorrect PSF predictions can occur due to two reasons. First, it is possible that the store/load pair had a dependency for a while but later stops having a dependency. This can occur if the address of either the store or load changes during the execution of the program. The second source of incorrect PSF predictions can occur if there is an alias in the PSF predictor structure. The PSF predictor tracks store-load pairs based on portions of their RIP. It is possible that a store-load pair which does have a dependency may alias in the predictor with another store-load pair which does not. This can result in incorrect speculation when the second store/load pair is executed. Security Analysis: Previous research has shown that when CPUs speculate on non-architectural paths it can lead to the potential of side channel attacks. In particular, programs that implement isolation, also known as ‘sandboxing’, entirely in software may need to be concerned with incorrect CPU speculation as they can occur due to bad PSF predictions. Because PSF speculation is limited to the current program context, the impact of bad PSF speculation is very similar to that of Speculative Store Bypass (Spectre v4) Predictive Store Forwarding controls: There are two hardware control bits which influence the PSF feature: - MSR 48h bit 2 – Speculative Store Bypass (SSBD) - MSR 48h bit 7 – Predictive Store Forwarding Disable (PSFD) The PSF feature is disabled if either of these bits are set. These bits are controllable on a per-thread basis in an SMT system. By default, both SSBD and PSFD are 0 meaning that the speculation features are enabled. While the SSBD bit disables PSF and speculative store bypass, PSFD only disables PSF. PSFD may be desirable for software which is concerned with the speculative behavior of PSF but desires a smaller performance impact than setting SSBD. Support for PSFD is indicated in CPUID Fn8000_0008 EBX[28]. All processors that support PSF will also support PSFD. ChangeLogs: V6->V5: Moved PSF control code to arch/x86/kernel/cpu/bugs.c PSF mitigation is similar to spec_control_bypass mitigation. PSF mitigation has only ON and OFF controls. Kernel parameter changed to predictive_store_fwd_disable. V5->V4: Replaced rdmsrl and wrmsrl for setting SPEC_CTRL_PSFD with a single call to msr_set_bit. Removed temporary variable to read and write the MSR V4->V3: Write to MSR_IA32_SPEC_CTRL properly Read MSR, modify PSFD bit based on kernel parameter and write back to MSR. Changes made in psf_cmdline() and check_bugs(). V3->V2: Set the X86_FEATURE_SPEC_CTRL_MSR cap in boot cpu caps. Fix kernel documentation for the kernel parameter. Rename PSF to a control instead of mitigation. V1->V2: - Smashed multiple commits into one commit. - Rename PSF to a control instead of mitigation. V1: - Initial patchset. - Kernel parameter controls enable and disable of PSF. Ramakrishna Saripalli (1): x86/bugs: Implement mitigation for Predictive Store Forwarding .../admin-guide/kernel-parameters.txt | 5 + arch/x86/include/asm/cpufeatures.h | 1 + arch/x86/include/asm/msr-index.h | 2 + arch/x86/include/asm/nospec-branch.h | 6 ++ arch/x86/kernel/cpu/bugs.c | 94 +++++++++++++++++++ 5 files changed, 108 insertions(+) base-commit: 0e16f466004d7f04296b9676a712a32a12367d1f -- 2.25.1 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [v6 1/1] x86/bugs: Implement mitigation for Predictive Store Forwarding 2021-05-17 22:00 [v6 0/1] Introduce support for PSF control Ramakrishna Saripalli @ 2021-05-17 22:00 ` Ramakrishna Saripalli 2021-05-18 2:55 ` Randy Dunlap ` (3 more replies) 2021-06-17 20:47 ` [v6 0/1] Introduce support for PSF control Saripalli, RK 1 sibling, 4 replies; 21+ messages in thread From: Ramakrishna Saripalli @ 2021-05-17 22:00 UTC (permalink / raw) To: linux-kernel, x86, tglx, mingo, bp, hpa, Jonathan Corbet; +Cc: bsd, rsaripal From: Ramakrishna Saripalli <rk.saripalli@amd.com> Certain AMD processors feature a new technology called Predictive Store Forwarding (PSF). PSF is a micro-architectural optimization designed to improve the performance of code execution by predicting dependencies between loads and stores. Incorrect PSF predictions can occur due to two reasons. - It is possible that the load/store pair may have had dependency for a while but the dependency has stopped because the address in the load/store pair has changed. - Second source of incorrect PSF prediction can occur because of an alias in the PSF predictor structure stored in the microarchitectural state. PSF predictor tracks load/store pair based on portions of instruction pointer. It is possible that a load/store pair which does have a dependency may be aliased by another load/store pair which does not have the same dependency. This can result in incorrect speculation. Software may be able to detect this aliasing and perform side-channel attacks. All CPUs that implement PSF provide one bit to disable this feature. If the bit to disable this feature is available, it means that the CPU implements PSF feature and is therefore vulnerable to PSF risks. The bits that are introduced X86_FEATURE_PSFD: CPUID_Fn80000008_EBX[28] ("PSF disable") If this bit is 1, CPU implements PSF and PSF control via SPEC_CTRL_MSR is supported in the CPU. All AMD processors that support PSF implement a bit in SPEC_CTRL MSR (0x48) to disable or enable Predictive Store Forwarding. PSF control introduces a new kernel parameter called predictive_store_fwd_disable. Kernel parameter predictive_store_fwd_disable has the following values - on. Disable PSF on all CPUs. - off. Enable PSF on all CPUs. This is also the default setting. Signed-off-by: Ramakrishna Saripalli<rk.saripalli@amd.com> --- .../admin-guide/kernel-parameters.txt | 5 + arch/x86/include/asm/cpufeatures.h | 1 + arch/x86/include/asm/msr-index.h | 2 + arch/x86/include/asm/nospec-branch.h | 6 ++ arch/x86/kernel/cpu/bugs.c | 94 +++++++++++++++++++ 5 files changed, 108 insertions(+) diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index 04545725f187..a5f694dccb24 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -3940,6 +3940,11 @@ Format: {"off"} Disable Hardware Transactional Memory + predictive_store_fwd_disable= [X86] This option controls PSF. + off - Turns on PSF. + on - Turns off PSF. + default : off. + preempt= [KNL] Select preemption mode if you have CONFIG_PREEMPT_DYNAMIC none - Limited to cond_resched() calls diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h index cc96e26d69f7..078f46022293 100644 --- a/arch/x86/include/asm/cpufeatures.h +++ b/arch/x86/include/asm/cpufeatures.h @@ -309,6 +309,7 @@ #define X86_FEATURE_AMD_SSBD (13*32+24) /* "" Speculative Store Bypass Disable */ #define X86_FEATURE_VIRT_SSBD (13*32+25) /* Virtualized Speculative Store Bypass Disable */ #define X86_FEATURE_AMD_SSB_NO (13*32+26) /* "" Speculative Store Bypass is fixed in hardware. */ +#define X86_FEATURE_PSFD (13*32+28) /* Predictive Store Forward Disable */ /* Thermal and Power Management Leaf, CPUID level 0x00000006 (EAX), word 14 */ #define X86_FEATURE_DTHERM (14*32+ 0) /* Digital Thermal Sensor */ diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h index 546d6ecf0a35..f569918c8754 100644 --- a/arch/x86/include/asm/msr-index.h +++ b/arch/x86/include/asm/msr-index.h @@ -51,6 +51,8 @@ #define SPEC_CTRL_STIBP BIT(SPEC_CTRL_STIBP_SHIFT) /* STIBP mask */ #define SPEC_CTRL_SSBD_SHIFT 2 /* Speculative Store Bypass Disable bit */ #define SPEC_CTRL_SSBD BIT(SPEC_CTRL_SSBD_SHIFT) /* Speculative Store Bypass Disable */ +#define SPEC_CTRL_PSFD_SHIFT 7 +#define SPEC_CTRL_PSFD BIT(SPEC_CTRL_PSFD_SHIFT) /* Predictive Store Forwarding Disable */ #define MSR_IA32_PRED_CMD 0x00000049 /* Prediction Command */ #define PRED_CMD_IBPB BIT(0) /* Indirect Branch Prediction Barrier */ diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h index cb9ad6b73973..1231099e5c07 100644 --- a/arch/x86/include/asm/nospec-branch.h +++ b/arch/x86/include/asm/nospec-branch.h @@ -198,6 +198,12 @@ enum ssb_mitigation { SPEC_STORE_BYPASS_SECCOMP, }; +/* The Predictive Store forward control variant */ +enum psf_mitigation { + PREDICTIVE_STORE_FORWARD_NONE, + PREDICTIVE_STORE_FORWARD_DISABLE, +}; + extern char __indirect_thunk_start[]; extern char __indirect_thunk_end[]; diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c index d41b70fe4918..c80ef4d86b72 100644 --- a/arch/x86/kernel/cpu/bugs.c +++ b/arch/x86/kernel/cpu/bugs.c @@ -38,6 +38,7 @@ static void __init spectre_v1_select_mitigation(void); static void __init spectre_v2_select_mitigation(void); static void __init ssb_select_mitigation(void); +static void __init psf_select_mitigation(void); static void __init l1tf_select_mitigation(void); static void __init mds_select_mitigation(void); static void __init mds_print_mitigation(void); @@ -107,6 +108,7 @@ void __init check_bugs(void) spectre_v1_select_mitigation(); spectre_v2_select_mitigation(); ssb_select_mitigation(); + psf_select_mitigation(); l1tf_select_mitigation(); mds_select_mitigation(); taa_select_mitigation(); @@ -1195,6 +1197,98 @@ static void ssb_select_mitigation(void) pr_info("%s\n", ssb_strings[ssb_mode]); } +#undef pr_fmt +#define pr_fmt(fmt) "Predictive Store Forward: " fmt + +static enum psf_mitigation psf_mode __ro_after_init = PREDICTIVE_STORE_FORWARD_NONE; + +/* The kernel command line selection */ +enum psf_mitigation_cmd { + PREDICTIVE_STORE_FORWARD_CMD_NONE, + PREDICTIVE_STORE_FORWARD_CMD_ON, +}; + +static const char * const psf_strings[] = { + [PREDICTIVE_STORE_FORWARD_NONE] = "Vulnerable", + [PREDICTIVE_STORE_FORWARD_DISABLE] = "Mitigation: Predictive Store Forward disabled", +}; + +static const struct { + const char *option; + enum psf_mitigation_cmd cmd; +} psf_mitigation_options[] __initconst = { + { "on", PREDICTIVE_STORE_FORWARD_CMD_ON }, /* Disable Speculative Store Bypass */ + { "off", PREDICTIVE_STORE_FORWARD_CMD_NONE }, /* Don't touch Speculative Store Bypass */ +}; + +static enum psf_mitigation_cmd __init psf_parse_cmdline(void) +{ + enum psf_mitigation_cmd cmd = PREDICTIVE_STORE_FORWARD_CMD_NONE; + char arg[20]; + int ret, i; + + ret = cmdline_find_option(boot_command_line, "predictive_store_fwd_disable", + arg, sizeof(arg)); + if (ret < 0) + return PREDICTIVE_STORE_FORWARD_CMD_NONE; + + for (i = 0; i < ARRAY_SIZE(psf_mitigation_options); i++) { + if (!match_option(arg, ret, psf_mitigation_options[i].option)) + continue; + + cmd = psf_mitigation_options[i].cmd; + break; + } + + if (i >= ARRAY_SIZE(psf_mitigation_options)) { + pr_err("unknown option (%s). Switching to AUTO select\n", arg); + return PREDICTIVE_STORE_FORWARD_CMD_NONE; + } + + return cmd; +} + +static enum psf_mitigation __init __psf_select_mitigation(void) +{ + enum psf_mitigation mode = PREDICTIVE_STORE_FORWARD_NONE; + enum psf_mitigation_cmd cmd; + + if (!boot_cpu_has(X86_FEATURE_PSFD)) + return mode; + + cmd = psf_parse_cmdline(); + + switch (cmd) { + case PREDICTIVE_STORE_FORWARD_CMD_ON: + mode = PREDICTIVE_STORE_FORWARD_DISABLE; + break; + default: + mode = PREDICTIVE_STORE_FORWARD_NONE; + break; + } + + x86_spec_ctrl_mask |= SPEC_CTRL_PSFD; + + if (ssb_mode == SPEC_STORE_BYPASS_DISABLE) + mode = PREDICTIVE_STORE_FORWARD_DISABLE; + + if (mode == PREDICTIVE_STORE_FORWARD_DISABLE) { + setup_force_cpu_cap(X86_FEATURE_PSFD); + x86_spec_ctrl_base |= SPEC_CTRL_PSFD; + wrmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base); + } + + return mode; +} + +static void psf_select_mitigation(void) +{ + psf_mode = __psf_select_mitigation(); + + if (boot_cpu_has(X86_FEATURE_PSFD)) + pr_info("%s\n", psf_strings[psf_mode]); +} + #undef pr_fmt #define pr_fmt(fmt) "Speculation prctl: " fmt -- 2.25.1 ^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [v6 1/1] x86/bugs: Implement mitigation for Predictive Store Forwarding 2021-05-17 22:00 ` [v6 1/1] x86/bugs: Implement mitigation for Predictive Store Forwarding Ramakrishna Saripalli @ 2021-05-18 2:55 ` Randy Dunlap 2021-05-18 12:27 ` Saripalli, RK 2021-05-19 5:38 ` Pawan Gupta ` (2 subsequent siblings) 3 siblings, 1 reply; 21+ messages in thread From: Randy Dunlap @ 2021-05-18 2:55 UTC (permalink / raw) To: Ramakrishna Saripalli, linux-kernel, x86, tglx, mingo, bp, hpa, Jonathan Corbet Cc: bsd Hi again, On 5/17/21 3:00 PM, Ramakrishna Saripalli wrote: > From: Ramakrishna Saripalli <rk.saripalli@amd.com> > > Certain AMD processors feature a new technology called Predictive Store > Forwarding (PSF). > > PSF is a micro-architectural optimization designed to improve the > performance of code execution by predicting dependencies between > loads and stores. > > Incorrect PSF predictions can occur due to two reasons. > ... > > Kernel parameter predictive_store_fwd_disable has the following values > > - on. Disable PSF on all CPUs. > > - off. Enable PSF on all CPUs. > This is also the default setting. > > Signed-off-by: Ramakrishna Saripalli<rk.saripalli@amd.com> > --- > .../admin-guide/kernel-parameters.txt | 5 + > arch/x86/include/asm/cpufeatures.h | 1 + > arch/x86/include/asm/msr-index.h | 2 + > arch/x86/include/asm/nospec-branch.h | 6 ++ > arch/x86/kernel/cpu/bugs.c | 94 +++++++++++++++++++ > 5 files changed, 108 insertions(+) > > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt > index 04545725f187..a5f694dccb24 100644 > --- a/Documentation/admin-guide/kernel-parameters.txt > +++ b/Documentation/admin-guide/kernel-parameters.txt > @@ -3940,6 +3940,11 @@ > Format: {"off"} > Disable Hardware Transactional Memory > > + predictive_store_fwd_disable= [X86] This option controls PSF. > + off - Turns on PSF. > + on - Turns off PSF. > + default : off. and as I did earlier, I still object to "off" meaning PSF is on and "on" meaning that PSF is off. It's not at all user friendly. If it's done this way because that's how the h/w bit is defined/used, that's not a good excuse IMHO. Hm, it sorta seems to be a common "theme" when dealing with mitigations. And too late to fix that. I look forward to h/w that doesn't need mitigations. ;) -- ~Randy ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [v6 1/1] x86/bugs: Implement mitigation for Predictive Store Forwarding 2021-05-18 2:55 ` Randy Dunlap @ 2021-05-18 12:27 ` Saripalli, RK 2021-05-18 20:35 ` Pawan Gupta 0 siblings, 1 reply; 21+ messages in thread From: Saripalli, RK @ 2021-05-18 12:27 UTC (permalink / raw) To: Randy Dunlap, linux-kernel, x86, tglx, mingo, bp, hpa, Jonathan Corbet Cc: bsd On 5/17/2021 9:55 PM, Randy Dunlap wrote: > Hi again, > > On 5/17/21 3:00 PM, Ramakrishna Saripalli wrote: >> From: Ramakrishna Saripalli <rk.saripalli@amd.com> >> >> Certain AMD processors feature a new technology called Predictive Store >> Forwarding (PSF). >> >> PSF is a micro-architectural optimization designed to improve the >> performance of code execution by predicting dependencies between >> loads and stores. >> >> Incorrect PSF predictions can occur due to two reasons. >> > ... > >> >> Kernel parameter predictive_store_fwd_disable has the following values >> >> - on. Disable PSF on all CPUs. >> >> - off. Enable PSF on all CPUs. >> This is also the default setting. >> >> Signed-off-by: Ramakrishna Saripalli<rk.saripalli@amd.com> >> --- >> .../admin-guide/kernel-parameters.txt | 5 + >> arch/x86/include/asm/cpufeatures.h | 1 + >> arch/x86/include/asm/msr-index.h | 2 + >> arch/x86/include/asm/nospec-branch.h | 6 ++ >> arch/x86/kernel/cpu/bugs.c | 94 +++++++++++++++++++ >> 5 files changed, 108 insertions(+) >> >> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt >> index 04545725f187..a5f694dccb24 100644 >> --- a/Documentation/admin-guide/kernel-parameters.txt >> +++ b/Documentation/admin-guide/kernel-parameters.txt >> @@ -3940,6 +3940,11 @@ >> Format: {"off"} >> Disable Hardware Transactional Memory >> >> + predictive_store_fwd_disable= [X86] This option controls PSF. >> + off - Turns on PSF. >> + on - Turns off PSF. >> + default : off. > > > and as I did earlier, I still object to "off" meaning PSF is on > and "on" meaning that PSF is off. > > It's not at all user friendly. > > If it's done this way because that's how the h/w bit is defined/used, > that's not a good excuse IMHO. > > Hm, it sorta seems to be a common "theme" when dealing with mitigations. > And too late to fix that. Based on previous feedback from Thomas Gleixner, I reworded this as a mitigation instead of as a "feature". In that vein, all the mitigation code moved into bugs.c like other mitigations, similar to spec_control_bypass_disable with an ON/OFF but no prctl/seccomp/auto. > > I look forward to h/w that doesn't need mitigations. ;) > ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [v6 1/1] x86/bugs: Implement mitigation for Predictive Store Forwarding 2021-05-18 12:27 ` Saripalli, RK @ 2021-05-18 20:35 ` Pawan Gupta 0 siblings, 0 replies; 21+ messages in thread From: Pawan Gupta @ 2021-05-18 20:35 UTC (permalink / raw) To: Saripalli, RK Cc: Randy Dunlap, linux-kernel, x86, tglx, mingo, bp, hpa, Jonathan Corbet, bsd On 18.05.2021 07:27, Saripalli, RK wrote: > > >On 5/17/2021 9:55 PM, Randy Dunlap wrote: >> Hi again, >> >> On 5/17/21 3:00 PM, Ramakrishna Saripalli wrote: >>> From: Ramakrishna Saripalli <rk.saripalli@amd.com> >>> >>> Certain AMD processors feature a new technology called Predictive Store >>> Forwarding (PSF). >>> >>> PSF is a micro-architectural optimization designed to improve the >>> performance of code execution by predicting dependencies between >>> loads and stores. >>> >>> Incorrect PSF predictions can occur due to two reasons. >>> >> ... >> >>> >>> Kernel parameter predictive_store_fwd_disable has the following values >>> >>> - on. Disable PSF on all CPUs. >>> >>> - off. Enable PSF on all CPUs. >>> This is also the default setting. >>> >>> Signed-off-by: Ramakrishna Saripalli<rk.saripalli@amd.com> >>> --- >>> .../admin-guide/kernel-parameters.txt | 5 + >>> arch/x86/include/asm/cpufeatures.h | 1 + >>> arch/x86/include/asm/msr-index.h | 2 + >>> arch/x86/include/asm/nospec-branch.h | 6 ++ >>> arch/x86/kernel/cpu/bugs.c | 94 +++++++++++++++++++ >>> 5 files changed, 108 insertions(+) >>> >>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt >>> index 04545725f187..a5f694dccb24 100644 >>> --- a/Documentation/admin-guide/kernel-parameters.txt >>> +++ b/Documentation/admin-guide/kernel-parameters.txt >>> @@ -3940,6 +3940,11 @@ >>> Format: {"off"} >>> Disable Hardware Transactional Memory >>> >>> + predictive_store_fwd_disable= [X86] This option controls PSF. >>> + off - Turns on PSF. >>> + on - Turns off PSF. >>> + default : off. >> >> >> and as I did earlier, I still object to "off" meaning PSF is on >> and "on" meaning that PSF is off. >> >> It's not at all user friendly. >> >> If it's done this way because that's how the h/w bit is defined/used, >> that's not a good excuse IMHO. >> >> Hm, it sorta seems to be a common "theme" when dealing with mitigations. >> And too late to fix that. > >Based on previous feedback from Thomas Gleixner, I reworded this as a mitigation instead of as a "feature". >In that vein, all the mitigation code moved into bugs.c like other mitigations, similar to >spec_control_bypass_disable with an ON/OFF but no prctl/seccomp/auto. Maybe change the help text to something like: on - Turns on PSF mitigation. off - Turns off PSF mitigation. Thanks, Pawan ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [v6 1/1] x86/bugs: Implement mitigation for Predictive Store Forwarding 2021-05-17 22:00 ` [v6 1/1] x86/bugs: Implement mitigation for Predictive Store Forwarding Ramakrishna Saripalli 2021-05-18 2:55 ` Randy Dunlap @ 2021-05-19 5:38 ` Pawan Gupta 2021-05-19 13:19 ` Saripalli, RK 2021-05-19 5:50 ` Pawan Gupta 2021-08-12 23:44 ` [v6 1/1] x86/bugs: Implement mitigation for Predictive Store Forwarding Josh Poimboeuf 3 siblings, 1 reply; 21+ messages in thread From: Pawan Gupta @ 2021-05-19 5:38 UTC (permalink / raw) To: Ramakrishna Saripalli Cc: linux-kernel, x86, tglx, mingo, bp, hpa, Jonathan Corbet, bsd On 17.05.2021 17:00, Ramakrishna Saripalli wrote: >From: Ramakrishna Saripalli <rk.saripalli@amd.com> > >Certain AMD processors feature a new technology called Predictive Store >Forwarding (PSF). > >PSF is a micro-architectural optimization designed to improve the >performance of code execution by predicting dependencies between >loads and stores. > >Incorrect PSF predictions can occur due to two reasons. > >- It is possible that the load/store pair may have had dependency for > a while but the dependency has stopped because the address in the > load/store pair has changed. > >- Second source of incorrect PSF prediction can occur because of an alias > in the PSF predictor structure stored in the microarchitectural state. > PSF predictor tracks load/store pair based on portions of instruction > pointer. It is possible that a load/store pair which does have a > dependency may be aliased by another load/store pair which does not have > the same dependency. This can result in incorrect speculation. > > Software may be able to detect this aliasing and perform side-channel > attacks. > >All CPUs that implement PSF provide one bit to disable this feature. >If the bit to disable this feature is available, it means that the CPU >implements PSF feature and is therefore vulnerable to PSF risks. > >The bits that are introduced > >X86_FEATURE_PSFD: CPUID_Fn80000008_EBX[28] ("PSF disable") > If this bit is 1, CPU implements PSF and PSF control > via SPEC_CTRL_MSR is supported in the CPU. > >All AMD processors that support PSF implement a bit in >SPEC_CTRL MSR (0x48) to disable or enable Predictive Store >Forwarding. > >PSF control introduces a new kernel parameter called > predictive_store_fwd_disable. > >Kernel parameter predictive_store_fwd_disable has the following values > >- on. Disable PSF on all CPUs. > >- off. Enable PSF on all CPUs. > This is also the default setting. > >Signed-off-by: Ramakrishna Saripalli<rk.saripalli@amd.com> >--- > .../admin-guide/kernel-parameters.txt | 5 + > arch/x86/include/asm/cpufeatures.h | 1 + > arch/x86/include/asm/msr-index.h | 2 + > arch/x86/include/asm/nospec-branch.h | 6 ++ > arch/x86/kernel/cpu/bugs.c | 94 +++++++++++++++++++ > 5 files changed, 108 insertions(+) > >diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt >index 04545725f187..a5f694dccb24 100644 >--- a/Documentation/admin-guide/kernel-parameters.txt >+++ b/Documentation/admin-guide/kernel-parameters.txt >@@ -3940,6 +3940,11 @@ > Format: {"off"} > Disable Hardware Transactional Memory > >+ predictive_store_fwd_disable= [X86] This option controls PSF. >+ off - Turns on PSF. >+ on - Turns off PSF. >+ default : off. >+ > preempt= [KNL] > Select preemption mode if you have CONFIG_PREEMPT_DYNAMIC > none - Limited to cond_resched() calls >diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h >index cc96e26d69f7..078f46022293 100644 >--- a/arch/x86/include/asm/cpufeatures.h >+++ b/arch/x86/include/asm/cpufeatures.h >@@ -309,6 +309,7 @@ > #define X86_FEATURE_AMD_SSBD (13*32+24) /* "" Speculative Store Bypass Disable */ > #define X86_FEATURE_VIRT_SSBD (13*32+25) /* Virtualized Speculative Store Bypass Disable */ > #define X86_FEATURE_AMD_SSB_NO (13*32+26) /* "" Speculative Store Bypass is fixed in hardware. */ >+#define X86_FEATURE_PSFD (13*32+28) /* Predictive Store Forward Disable */ > > /* Thermal and Power Management Leaf, CPUID level 0x00000006 (EAX), word 14 */ > #define X86_FEATURE_DTHERM (14*32+ 0) /* Digital Thermal Sensor */ >diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h >index 546d6ecf0a35..f569918c8754 100644 >--- a/arch/x86/include/asm/msr-index.h >+++ b/arch/x86/include/asm/msr-index.h >@@ -51,6 +51,8 @@ > #define SPEC_CTRL_STIBP BIT(SPEC_CTRL_STIBP_SHIFT) /* STIBP mask */ > #define SPEC_CTRL_SSBD_SHIFT 2 /* Speculative Store Bypass Disable bit */ > #define SPEC_CTRL_SSBD BIT(SPEC_CTRL_SSBD_SHIFT) /* Speculative Store Bypass Disable */ >+#define SPEC_CTRL_PSFD_SHIFT 7 >+#define SPEC_CTRL_PSFD BIT(SPEC_CTRL_PSFD_SHIFT) /* Predictive Store Forwarding Disable */ > > #define MSR_IA32_PRED_CMD 0x00000049 /* Prediction Command */ > #define PRED_CMD_IBPB BIT(0) /* Indirect Branch Prediction Barrier */ >diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h >index cb9ad6b73973..1231099e5c07 100644 >--- a/arch/x86/include/asm/nospec-branch.h >+++ b/arch/x86/include/asm/nospec-branch.h >@@ -198,6 +198,12 @@ enum ssb_mitigation { > SPEC_STORE_BYPASS_SECCOMP, > }; > >+/* The Predictive Store forward control variant */ >+enum psf_mitigation { >+ PREDICTIVE_STORE_FORWARD_NONE, >+ PREDICTIVE_STORE_FORWARD_DISABLE, >+}; >+ > extern char __indirect_thunk_start[]; > extern char __indirect_thunk_end[]; > >diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c >index d41b70fe4918..c80ef4d86b72 100644 >--- a/arch/x86/kernel/cpu/bugs.c >+++ b/arch/x86/kernel/cpu/bugs.c >@@ -38,6 +38,7 @@ > static void __init spectre_v1_select_mitigation(void); > static void __init spectre_v2_select_mitigation(void); > static void __init ssb_select_mitigation(void); >+static void __init psf_select_mitigation(void); > static void __init l1tf_select_mitigation(void); > static void __init mds_select_mitigation(void); > static void __init mds_print_mitigation(void); >@@ -107,6 +108,7 @@ void __init check_bugs(void) > spectre_v1_select_mitigation(); > spectre_v2_select_mitigation(); > ssb_select_mitigation(); >+ psf_select_mitigation(); > l1tf_select_mitigation(); > mds_select_mitigation(); > taa_select_mitigation(); >@@ -1195,6 +1197,98 @@ static void ssb_select_mitigation(void) > pr_info("%s\n", ssb_strings[ssb_mode]); > } > >+#undef pr_fmt >+#define pr_fmt(fmt) "Predictive Store Forward: " fmt >+ >+static enum psf_mitigation psf_mode __ro_after_init = PREDICTIVE_STORE_FORWARD_NONE; >+ >+/* The kernel command line selection */ >+enum psf_mitigation_cmd { >+ PREDICTIVE_STORE_FORWARD_CMD_NONE, >+ PREDICTIVE_STORE_FORWARD_CMD_ON, >+}; >+ >+static const char * const psf_strings[] = { >+ [PREDICTIVE_STORE_FORWARD_NONE] = "Vulnerable", >+ [PREDICTIVE_STORE_FORWARD_DISABLE] = "Mitigation: Predictive Store Forward disabled", >+}; >+ >+static const struct { >+ const char *option; >+ enum psf_mitigation_cmd cmd; >+} psf_mitigation_options[] __initconst = { >+ { "on", PREDICTIVE_STORE_FORWARD_CMD_ON }, /* Disable Speculative Store Bypass */ >+ { "off", PREDICTIVE_STORE_FORWARD_CMD_NONE }, /* Don't touch Speculative Store Bypass */ >+}; >+ >+static enum psf_mitigation_cmd __init psf_parse_cmdline(void) >+{ >+ enum psf_mitigation_cmd cmd = PREDICTIVE_STORE_FORWARD_CMD_NONE; >+ char arg[20]; >+ int ret, i; >+ >+ ret = cmdline_find_option(boot_command_line, "predictive_store_fwd_disable", >+ arg, sizeof(arg)); >+ if (ret < 0) >+ return PREDICTIVE_STORE_FORWARD_CMD_NONE; >+ >+ for (i = 0; i < ARRAY_SIZE(psf_mitigation_options); i++) { >+ if (!match_option(arg, ret, psf_mitigation_options[i].option)) >+ continue; >+ >+ cmd = psf_mitigation_options[i].cmd; >+ break; >+ } >+ >+ if (i >= ARRAY_SIZE(psf_mitigation_options)) { >+ pr_err("unknown option (%s). Switching to AUTO select\n", arg); >+ return PREDICTIVE_STORE_FORWARD_CMD_NONE; >+ } >+ >+ return cmd; >+} >+ >+static enum psf_mitigation __init __psf_select_mitigation(void) >+{ >+ enum psf_mitigation mode = PREDICTIVE_STORE_FORWARD_NONE; >+ enum psf_mitigation_cmd cmd; >+ >+ if (!boot_cpu_has(X86_FEATURE_PSFD)) >+ return mode; >+ >+ cmd = psf_parse_cmdline(); >+ >+ switch (cmd) { >+ case PREDICTIVE_STORE_FORWARD_CMD_ON: >+ mode = PREDICTIVE_STORE_FORWARD_DISABLE; >+ break; >+ default: >+ mode = PREDICTIVE_STORE_FORWARD_NONE; >+ break; >+ } >+ >+ x86_spec_ctrl_mask |= SPEC_CTRL_PSFD; >+ >+ if (ssb_mode == SPEC_STORE_BYPASS_DISABLE) >+ mode = PREDICTIVE_STORE_FORWARD_DISABLE; >+ >+ if (mode == PREDICTIVE_STORE_FORWARD_DISABLE) { >+ setup_force_cpu_cap(X86_FEATURE_PSFD); Why do we need to force set X86_FEATURE_PSFD here? Control will not reach here if this is not already set (because of boot_cpu_has() check at function entry). >+ x86_spec_ctrl_base |= SPEC_CTRL_PSFD; >+ wrmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base); >+ } >+ >+ return mode; >+} >+ >+static void psf_select_mitigation(void) >+{ >+ psf_mode = __psf_select_mitigation(); >+ >+ if (boot_cpu_has(X86_FEATURE_PSFD)) >+ pr_info("%s\n", psf_strings[psf_mode]); >+} For an on/off control this patch looks like an overkill. I think we can simplify it as below: static void psf_select_mitigation(void) { if (!boot_cpu_has(X86_FEATURE_PSFD)) return; x86_spec_ctrl_mask |= SPEC_CTRL_PSFD; if (ssb_mode == SPEC_STORE_BYPASS_DISABLE) psf_mode = PREDICTIVE_STORE_FORWARD_DISABLE; if (psf_mode == PREDICTIVE_STORE_FORWARD_DISABLE) { x86_spec_ctrl_base |= SPEC_CTRL_PSFD; wrmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base); } pr_info("%s\n", psf_strings[psf_mode]); } static int __init psfd_cmdline(char *str) { if (!boot_cpu_has(X86_FEATURE_PSFD)) return; if (!str) return -EINVAL; if (!strcmp(str, "on")) psf_mode = PREDICTIVE_STORE_FORWARD_DISABLE; return 0; } early_param("predictive_store_fwd_disable", psfd_cmdline); --- Thanks, Pawan ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [v6 1/1] x86/bugs: Implement mitigation for Predictive Store Forwarding 2021-05-19 5:38 ` Pawan Gupta @ 2021-05-19 13:19 ` Saripalli, RK 0 siblings, 0 replies; 21+ messages in thread From: Saripalli, RK @ 2021-05-19 13:19 UTC (permalink / raw) To: Pawan Gupta; +Cc: linux-kernel, x86, tglx, mingo, bp, hpa, Jonathan Corbet, bsd On 5/19/2021 12:38 AM, Pawan Gupta wrote: > On 17.05.2021 17:00, Ramakrishna Saripalli wrote: >> From: Ramakrishna Saripalli <rk.saripalli@amd.com> >> >> Certain AMD processors feature a new technology called Predictive Store >> Forwarding (PSF). >> >> PSF is a micro-architectural optimization designed to improve the >> performance of code execution by predicting dependencies between >> loads and stores. >> >> Incorrect PSF predictions can occur due to two reasons. >> >> - It is possible that the load/store pair may have had dependency for >> a while but the dependency has stopped because the address in the >> load/store pair has changed. >> >> - Second source of incorrect PSF prediction can occur because of an alias >> in the PSF predictor structure stored in the microarchitectural state. >> PSF predictor tracks load/store pair based on portions of instruction >> pointer. It is possible that a load/store pair which does have a >> dependency may be aliased by another load/store pair which does not have >> the same dependency. This can result in incorrect speculation. >> >> Software may be able to detect this aliasing and perform side-channel >> attacks. >> >> All CPUs that implement PSF provide one bit to disable this feature. >> If the bit to disable this feature is available, it means that the CPU >> implements PSF feature and is therefore vulnerable to PSF risks. >> >> The bits that are introduced >> >> X86_FEATURE_PSFD: CPUID_Fn80000008_EBX[28] ("PSF disable") >> If this bit is 1, CPU implements PSF and PSF control >> via SPEC_CTRL_MSR is supported in the CPU. >> >> All AMD processors that support PSF implement a bit in >> SPEC_CTRL MSR (0x48) to disable or enable Predictive Store >> Forwarding. >> >> PSF control introduces a new kernel parameter called >> predictive_store_fwd_disable. >> >> Kernel parameter predictive_store_fwd_disable has the following values >> >> - on. Disable PSF on all CPUs. >> >> - off. Enable PSF on all CPUs. >> This is also the default setting. >> >> Signed-off-by: Ramakrishna Saripalli<rk.saripalli@amd.com> >> --- >> .../admin-guide/kernel-parameters.txt | 5 + >> arch/x86/include/asm/cpufeatures.h | 1 + >> arch/x86/include/asm/msr-index.h | 2 + >> arch/x86/include/asm/nospec-branch.h | 6 ++ >> arch/x86/kernel/cpu/bugs.c | 94 +++++++++++++++++++ >> 5 files changed, 108 insertions(+) >> >> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt >> index 04545725f187..a5f694dccb24 100644 >> --- a/Documentation/admin-guide/kernel-parameters.txt >> +++ b/Documentation/admin-guide/kernel-parameters.txt >> @@ -3940,6 +3940,11 @@ >> Format: {"off"} >> Disable Hardware Transactional Memory >> >> + predictive_store_fwd_disable= [X86] This option controls PSF. >> + off - Turns on PSF. >> + on - Turns off PSF. >> + default : off. >> + >> preempt= [KNL] >> Select preemption mode if you have CONFIG_PREEMPT_DYNAMIC >> none - Limited to cond_resched() calls >> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h >> index cc96e26d69f7..078f46022293 100644 >> --- a/arch/x86/include/asm/cpufeatures.h >> +++ b/arch/x86/include/asm/cpufeatures.h >> @@ -309,6 +309,7 @@ >> #define X86_FEATURE_AMD_SSBD (13*32+24) /* "" Speculative Store Bypass Disable */ >> #define X86_FEATURE_VIRT_SSBD (13*32+25) /* Virtualized Speculative Store Bypass Disable */ >> #define X86_FEATURE_AMD_SSB_NO (13*32+26) /* "" Speculative Store Bypass is fixed in hardware. */ >> +#define X86_FEATURE_PSFD (13*32+28) /* Predictive Store Forward Disable */ >> >> /* Thermal and Power Management Leaf, CPUID level 0x00000006 (EAX), word 14 */ >> #define X86_FEATURE_DTHERM (14*32+ 0) /* Digital Thermal Sensor */ >> diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h >> index 546d6ecf0a35..f569918c8754 100644 >> --- a/arch/x86/include/asm/msr-index.h >> +++ b/arch/x86/include/asm/msr-index.h >> @@ -51,6 +51,8 @@ >> #define SPEC_CTRL_STIBP BIT(SPEC_CTRL_STIBP_SHIFT) /* STIBP mask */ >> #define SPEC_CTRL_SSBD_SHIFT 2 /* Speculative Store Bypass Disable bit */ >> #define SPEC_CTRL_SSBD BIT(SPEC_CTRL_SSBD_SHIFT) /* Speculative Store Bypass Disable */ >> +#define SPEC_CTRL_PSFD_SHIFT 7 >> +#define SPEC_CTRL_PSFD BIT(SPEC_CTRL_PSFD_SHIFT) /* Predictive Store Forwarding Disable */ >> >> #define MSR_IA32_PRED_CMD 0x00000049 /* Prediction Command */ >> #define PRED_CMD_IBPB BIT(0) /* Indirect Branch Prediction Barrier */ >> diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h >> index cb9ad6b73973..1231099e5c07 100644 >> --- a/arch/x86/include/asm/nospec-branch.h >> +++ b/arch/x86/include/asm/nospec-branch.h >> @@ -198,6 +198,12 @@ enum ssb_mitigation { >> SPEC_STORE_BYPASS_SECCOMP, >> }; >> >> +/* The Predictive Store forward control variant */ >> +enum psf_mitigation { >> + PREDICTIVE_STORE_FORWARD_NONE, >> + PREDICTIVE_STORE_FORWARD_DISABLE, >> +}; >> + >> extern char __indirect_thunk_start[]; >> extern char __indirect_thunk_end[]; >> >> diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c >> index d41b70fe4918..c80ef4d86b72 100644 >> --- a/arch/x86/kernel/cpu/bugs.c >> +++ b/arch/x86/kernel/cpu/bugs.c >> @@ -38,6 +38,7 @@ >> static void __init spectre_v1_select_mitigation(void); >> static void __init spectre_v2_select_mitigation(void); >> static void __init ssb_select_mitigation(void); >> +static void __init psf_select_mitigation(void); >> static void __init l1tf_select_mitigation(void); >> static void __init mds_select_mitigation(void); >> static void __init mds_print_mitigation(void); >> @@ -107,6 +108,7 @@ void __init check_bugs(void) >> spectre_v1_select_mitigation(); >> spectre_v2_select_mitigation(); >> ssb_select_mitigation(); >> + psf_select_mitigation(); >> l1tf_select_mitigation(); >> mds_select_mitigation(); >> taa_select_mitigation(); >> @@ -1195,6 +1197,98 @@ static void ssb_select_mitigation(void) >> pr_info("%s\n", ssb_strings[ssb_mode]); >> } >> >> +#undef pr_fmt >> +#define pr_fmt(fmt) "Predictive Store Forward: " fmt >> + >> +static enum psf_mitigation psf_mode __ro_after_init = PREDICTIVE_STORE_FORWARD_NONE; >> + >> +/* The kernel command line selection */ >> +enum psf_mitigation_cmd { >> + PREDICTIVE_STORE_FORWARD_CMD_NONE, >> + PREDICTIVE_STORE_FORWARD_CMD_ON, >> +}; >> + >> +static const char * const psf_strings[] = { >> + [PREDICTIVE_STORE_FORWARD_NONE] = "Vulnerable", >> + [PREDICTIVE_STORE_FORWARD_DISABLE] = "Mitigation: Predictive Store Forward disabled", >> +}; >> + >> +static const struct { >> + const char *option; >> + enum psf_mitigation_cmd cmd; >> +} psf_mitigation_options[] __initconst = { >> + { "on", PREDICTIVE_STORE_FORWARD_CMD_ON }, /* Disable Speculative Store Bypass */ >> + { "off", PREDICTIVE_STORE_FORWARD_CMD_NONE }, /* Don't touch Speculative Store Bypass */ >> +}; >> + >> +static enum psf_mitigation_cmd __init psf_parse_cmdline(void) >> +{ >> + enum psf_mitigation_cmd cmd = PREDICTIVE_STORE_FORWARD_CMD_NONE; >> + char arg[20]; >> + int ret, i; >> + >> + ret = cmdline_find_option(boot_command_line, "predictive_store_fwd_disable", >> + arg, sizeof(arg)); >> + if (ret < 0) >> + return PREDICTIVE_STORE_FORWARD_CMD_NONE; >> + >> + for (i = 0; i < ARRAY_SIZE(psf_mitigation_options); i++) { >> + if (!match_option(arg, ret, psf_mitigation_options[i].option)) >> + continue; >> + >> + cmd = psf_mitigation_options[i].cmd; >> + break; >> + } >> + >> + if (i >= ARRAY_SIZE(psf_mitigation_options)) { >> + pr_err("unknown option (%s). Switching to AUTO select\n", arg); >> + return PREDICTIVE_STORE_FORWARD_CMD_NONE; >> + } >> + >> + return cmd; >> +} >> + >> +static enum psf_mitigation __init __psf_select_mitigation(void) >> +{ >> + enum psf_mitigation mode = PREDICTIVE_STORE_FORWARD_NONE; >> + enum psf_mitigation_cmd cmd; >> + >> + if (!boot_cpu_has(X86_FEATURE_PSFD)) >> + return mode; >> + >> + cmd = psf_parse_cmdline(); >> + >> + switch (cmd) { >> + case PREDICTIVE_STORE_FORWARD_CMD_ON: >> + mode = PREDICTIVE_STORE_FORWARD_DISABLE; >> + break; >> + default: >> + mode = PREDICTIVE_STORE_FORWARD_NONE; >> + break; >> + } >> + >> + x86_spec_ctrl_mask |= SPEC_CTRL_PSFD; >> + >> + if (ssb_mode == SPEC_STORE_BYPASS_DISABLE) >> + mode = PREDICTIVE_STORE_FORWARD_DISABLE; >> + >> + if (mode == PREDICTIVE_STORE_FORWARD_DISABLE) { >> + setup_force_cpu_cap(X86_FEATURE_PSFD); > > Why do we need to force set X86_FEATURE_PSFD here? Control will not > reach here if this is not already set (because of boot_cpu_has() check > at function entry). > >> + x86_spec_ctrl_base |= SPEC_CTRL_PSFD; >> + wrmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base); >> + } >> + >> + return mode; >> +} >> + >> +static void psf_select_mitigation(void) >> +{ >> + psf_mode = __psf_select_mitigation(); >> + >> + if (boot_cpu_has(X86_FEATURE_PSFD)) >> + pr_info("%s\n", psf_strings[psf_mode]); >> +} > > For an on/off control this patch looks like an overkill. I think we can > simplify it as below: > > static void psf_select_mitigation(void) > { > if (!boot_cpu_has(X86_FEATURE_PSFD)) > return; > > x86_spec_ctrl_mask |= SPEC_CTRL_PSFD; > > if (ssb_mode == SPEC_STORE_BYPASS_DISABLE) > psf_mode = PREDICTIVE_STORE_FORWARD_DISABLE; > > if (psf_mode == PREDICTIVE_STORE_FORWARD_DISABLE) { > x86_spec_ctrl_base |= SPEC_CTRL_PSFD; > wrmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base); > } > > pr_info("%s\n", psf_strings[psf_mode]); > } > > static int __init psfd_cmdline(char *str) > { > if (!boot_cpu_has(X86_FEATURE_PSFD)) > return; > > if (!str) > return -EINVAL; > > if (!strcmp(str, "on")) > psf_mode = PREDICTIVE_STORE_FORWARD_DISABLE; > > return 0; > } > early_param("predictive_store_fwd_disable", psfd_cmdline); I agree that on/off can be simple like what you suggested. My patches version 2 to 5 were in fact structured this way but implemented in kernel/cpu/amd.c as it was deemed that that was the file logically speaking to put these changes in. Later reviews suggested that since this mitigation is similar to spec_control_bypass_disable (although it is a subset of the above), that it is better to have this in kernel/cpu/bugs.c and structured similar to spec_control_bypass_disable hence this patchset. > > --- > Thanks, > Pawan ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [v6 1/1] x86/bugs: Implement mitigation for Predictive Store Forwarding 2021-05-17 22:00 ` [v6 1/1] x86/bugs: Implement mitigation for Predictive Store Forwarding Ramakrishna Saripalli 2021-05-18 2:55 ` Randy Dunlap 2021-05-19 5:38 ` Pawan Gupta @ 2021-05-19 5:50 ` Pawan Gupta 2021-09-01 20:20 ` [v6 1/1] x86/bugs: Implement mitigation for Predictive Store Babu Moger 2021-09-01 20:30 ` Babu Moger 2021-08-12 23:44 ` [v6 1/1] x86/bugs: Implement mitigation for Predictive Store Forwarding Josh Poimboeuf 3 siblings, 2 replies; 21+ messages in thread From: Pawan Gupta @ 2021-05-19 5:50 UTC (permalink / raw) To: Ramakrishna Saripalli Cc: linux-kernel, x86, tglx, mingo, bp, hpa, Jonathan Corbet, bsd On 17.05.2021 17:00, Ramakrishna Saripalli wrote: >From: Ramakrishna Saripalli <rk.saripalli@amd.com> [...] >--- a/arch/x86/include/asm/nospec-branch.h >+++ b/arch/x86/include/asm/nospec-branch.h >@@ -198,6 +198,12 @@ enum ssb_mitigation { > SPEC_STORE_BYPASS_SECCOMP, > }; > >+/* The Predictive Store forward control variant */ >+enum psf_mitigation { >+ PREDICTIVE_STORE_FORWARD_NONE, >+ PREDICTIVE_STORE_FORWARD_DISABLE, >+}; >+ This can be moved to bugs.c which is the only user of psf_mitigation. Thanks, Pawan ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [v6 1/1] x86/bugs: Implement mitigation for Predictive Store 2021-05-19 5:50 ` Pawan Gupta @ 2021-09-01 20:20 ` Babu Moger 2021-09-01 20:30 ` Babu Moger 1 sibling, 0 replies; 21+ messages in thread From: Babu Moger @ 2021-09-01 20:20 UTC (permalink / raw) To: writetopawan Cc: bp, bsd, corbet, hpa, linux-kernel, mingo, rsaripal, tglx, x86 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [v6 1/1] x86/bugs: Implement mitigation for Predictive Store 2021-05-19 5:50 ` Pawan Gupta 2021-09-01 20:20 ` [v6 1/1] x86/bugs: Implement mitigation for Predictive Store Babu Moger @ 2021-09-01 20:30 ` Babu Moger 2021-09-01 20:35 ` Babu Moger 1 sibling, 1 reply; 21+ messages in thread From: Babu Moger @ 2021-09-01 20:30 UTC (permalink / raw) To: writetopawan Cc: bp, bsd, corbet, hpa, linux-kernel, mingo, babu.moger, tglx, x86 Pawan, That is kind of odd. The ssb_mitigation enums are defined in bug.c. To be consistent it is better to keep it in nospec-branch.h as they are related. Thanks Babu ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [v6 1/1] x86/bugs: Implement mitigation for Predictive Store 2021-09-01 20:30 ` Babu Moger @ 2021-09-01 20:35 ` Babu Moger 2021-09-02 17:35 ` Pawan Gupta 0 siblings, 1 reply; 21+ messages in thread From: Babu Moger @ 2021-09-01 20:35 UTC (permalink / raw) To: writetopawan; +Cc: bp, bsd, corbet, hpa, linux-kernel, mingo, tglx, x86 On 9/1/21 3:30 PM, Babu Moger wrote: > Pawan, That is kind of odd. The ssb_mitigation enums are defined in bug.c. To be consistent it is better to keep it in nospec-branch.h as they are related. Sorry, I meant all these mitigations related enums are in nospec-branch.h. So, it better to keep it there for consistency. > Thanks > Babu > ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [v6 1/1] x86/bugs: Implement mitigation for Predictive Store 2021-09-01 20:35 ` Babu Moger @ 2021-09-02 17:35 ` Pawan Gupta 0 siblings, 0 replies; 21+ messages in thread From: Pawan Gupta @ 2021-09-02 17:35 UTC (permalink / raw) To: Babu Moger; +Cc: bp, bsd, corbet, hpa, linux-kernel, mingo, tglx, x86 On 01.09.2021 15:35, Babu Moger wrote: > > >On 9/1/21 3:30 PM, Babu Moger wrote: >> Pawan, That is kind of odd. The ssb_mitigation enums are defined in bug.c. To be consistent it is better to keep it in nospec-branch.h as they are related. > >Sorry, I meant all these mitigations related enums are in nospec-branch.h. Thats not true: $ grep "^enum.*{$" arch/x86/kernel/cpu/bugs.c enum taa_mitigations { enum srbds_mitigations { enum spectre_v1_mitigation { enum spectre_v2_mitigation_cmd { enum spectre_v2_user_cmd { enum ssb_mitigation_cmd { >So, it better to keep it there for consistency. But, I am not too adamant about it. Thanks, Pawan ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [v6 1/1] x86/bugs: Implement mitigation for Predictive Store Forwarding 2021-05-17 22:00 ` [v6 1/1] x86/bugs: Implement mitigation for Predictive Store Forwarding Ramakrishna Saripalli ` (2 preceding siblings ...) 2021-05-19 5:50 ` Pawan Gupta @ 2021-08-12 23:44 ` Josh Poimboeuf 2021-09-02 18:16 ` [v6 1/1] x86/bugs: Implement mitigation for Predictive Store Babu Moger 3 siblings, 1 reply; 21+ messages in thread From: Josh Poimboeuf @ 2021-08-12 23:44 UTC (permalink / raw) To: Ramakrishna Saripalli Cc: linux-kernel, x86, tglx, mingo, bp, hpa, Jonathan Corbet, bsd On Mon, May 17, 2021 at 05:00:58PM -0500, Ramakrishna Saripalli wrote: > From: Ramakrishna Saripalli <rk.saripalli@amd.com> > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt > index 04545725f187..a5f694dccb24 100644 > --- a/Documentation/admin-guide/kernel-parameters.txt > +++ b/Documentation/admin-guide/kernel-parameters.txt > @@ -3940,6 +3940,11 @@ > Format: {"off"} > Disable Hardware Transactional Memory > > + predictive_store_fwd_disable= [X86] This option controls PSF. > + off - Turns on PSF. > + on - Turns off PSF. > + default : off. > + This needs a lot more text. > +static const char * const psf_strings[] = { > + [PREDICTIVE_STORE_FORWARD_NONE] = "Vulnerable", > + [PREDICTIVE_STORE_FORWARD_DISABLE] = "Mitigation: Predictive Store Forward disabled", This defaults to "Vulnerable", which is problematic for at least a few reasons. 1) I'm fairly sure this would be the first mitigation designed to default to "Vulnerable". Aside from whether that's a good idea, many users will be alarmed to see "Vulnerable" in sysfs. 2) If the system has the default per-process SSB mitigation (prctl/seccomp) then PSF will be automatically mitigated in the same way. In that case "Vulnerable" isn't an accurate description. (More on that below.) > +static const struct { > + const char *option; > + enum psf_mitigation_cmd cmd; > +} psf_mitigation_options[] __initconst = { > + { "on", PREDICTIVE_STORE_FORWARD_CMD_ON }, /* Disable Speculative Store Bypass */ > + { "off", PREDICTIVE_STORE_FORWARD_CMD_NONE }, /* Don't touch Speculative Store Bypass */ Copy/paste error in the comments: "Speculative Store Bypass" -> "Predictive Store Forwarding" I'd also recommend an "auto" option: { "auto", PREDICTIVE_STORE_FORWARD_CMD_AUTO }, /* Platform decides */ which would be the default. For now it would function the same as "off", but would give room for tweaking defaults later. > +static enum psf_mitigation __init __psf_select_mitigation(void) > +{ > + enum psf_mitigation mode = PREDICTIVE_STORE_FORWARD_NONE; > + enum psf_mitigation_cmd cmd; > + > + if (!boot_cpu_has(X86_FEATURE_PSFD)) > + return mode; > + > + cmd = psf_parse_cmdline(); > + > + switch (cmd) { > + case PREDICTIVE_STORE_FORWARD_CMD_ON: > + mode = PREDICTIVE_STORE_FORWARD_DISABLE; > + break; > + default: > + mode = PREDICTIVE_STORE_FORWARD_NONE; > + break; > + } > + > + x86_spec_ctrl_mask |= SPEC_CTRL_PSFD; A comment would help for this last line. I assume it's virt-related. > + > + if (ssb_mode == SPEC_STORE_BYPASS_DISABLE) > + mode = PREDICTIVE_STORE_FORWARD_DISABLE; > + > + if (mode == PREDICTIVE_STORE_FORWARD_DISABLE) { > + setup_force_cpu_cap(X86_FEATURE_PSFD); > + x86_spec_ctrl_base |= SPEC_CTRL_PSFD; > + wrmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base); > + } The PSF mitigation is (to some extent) dependent on the SSB mitigation, since turning off SSB implicitly turns off PSF. That should be reflected properly in sysfs for the prctl/seccomp cases. Here I'd propose adding something like: } else if (ssb_mode == SPEC_STORE_BYPASS_PRCTL) { mode = PREDICTIVE_STORE_FORWARD_SSB_PRCTL; } else if (ssb_mode == SPEC_STORE_BYPASS_SECCOMP) { mode = PREDICTIVE_STORE_FORWARD_SSB_SECCOMP; } And of course you'd need additional strings for those: [PREDICTIVE_STORE_FORWARD_SSB_PRCTL] = "Mitigation: Predictive Store Forward disabled via SSB prctl", [PREDICTIVE_STORE_FORWARD_SSB_SECCOMP] = "Mitigation: Predictive Store Forward disabled via SSB seccomp", -- Josh ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [v6 1/1] x86/bugs: Implement mitigation for Predictive Store 2021-08-12 23:44 ` [v6 1/1] x86/bugs: Implement mitigation for Predictive Store Forwarding Josh Poimboeuf @ 2021-09-02 18:16 ` Babu Moger 2021-09-03 0:07 ` Josh Poimboeuf 0 siblings, 1 reply; 21+ messages in thread From: Babu Moger @ 2021-09-02 18:16 UTC (permalink / raw) To: jpoimboe; +Cc: bp, bsd, corbet, hpa, linux-kernel, mingo, babu.moger, tglx, x86 I dont have this thread in my mailbox. Replying via git send-email. Josh, I took care of all your comments except this one below. >I'd also recommend an "auto" option: > > { "auto", PREDICTIVE_STORE_FORWARD_CMD_AUTO }, /* Platform decides */ > which would be the default. For now it would function the same as >"off", but would give room for tweaking defaults later. There is no plan for tweaking this option in the near future. I feel adding 'auto' option now is probably overkill and confusing. Thanks Babu ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [v6 1/1] x86/bugs: Implement mitigation for Predictive Store 2021-09-02 18:16 ` [v6 1/1] x86/bugs: Implement mitigation for Predictive Store Babu Moger @ 2021-09-03 0:07 ` Josh Poimboeuf [not found] ` <dca004cf-bacc-1a1f-56d6-c06e8bec167a@amd.com> 0 siblings, 1 reply; 21+ messages in thread From: Josh Poimboeuf @ 2021-09-03 0:07 UTC (permalink / raw) To: Babu Moger; +Cc: bp, bsd, corbet, hpa, linux-kernel, mingo, tglx, x86 On Thu, Sep 02, 2021 at 01:16:37PM -0500, Babu Moger wrote: > I dont have this thread in my mailbox. Replying via git send-email. > > Josh, > I took care of all your comments except this one below. > > >I'd also recommend an "auto" option: > > > > { "auto", PREDICTIVE_STORE_FORWARD_CMD_AUTO }, /* Platform decides */ > > > which would be the default. For now it would function the same as > >"off", but would give room for tweaking defaults later. > > There is no plan for tweaking this option in the near future. I feel > adding 'auto' option now is probably overkill and confusing. But if the PSF disable interface is modeled after SSB disable (which I believe it needs to be) then it's only logical to mirror SSB's default "auto" option. And, I actually think that calling it 'psf_disable=off' is *more* confusing than 'psf_disable=auto'. Because in this case, 'off' doesn't actually mean "off". It means "off, unless !ssb_disable=off, in which case implicitly mirror the SSB policy". So maybe there shouldn't even be an 'psf_disable=off' option, because it's not possible to ensure that PSF doesn't get disabled by SSB disable. BTW, is the list of PSF-affected CPUs the same as the list of SSB-affected CPUs? If there might be PSF CPUs which don't have SSB, then more logic will need to be added to ensure a sensible default. On a related note, is there a realistic, non-hypothetical need to have separate policies and cmdline options for both SSB and PSF? i.e. is there a real-world scenario where a user needs to disable PSF while leaving SSB enabled? Because trying to give them separate interfaces, when PSF disable is intertwined with SSB disable in hardware, is awkward and confusing. And the idea of adding another double-negative interface (disable=off!), just because a vulnerability is considered to be a CPU "feature", isn't very appetizing. So instead of adding a new double-negative interface, which only *half* works due to the ssb_disable dependency, and which is guaranteed to further confuse users, and which not even be used in the real world except possibly by confused users... I'm wondering if we can just start out with the simplest possible approach: don't change any code and instead just document the fact that "spec_store_bypass_disable=" also affects PSF. Then, later on, if a real-world need is demonstrated, actual code could be added to support disabling PSF independently (but of course it would never be fully independent since PSF disable is forced by SSB disable). -- Josh ^ permalink raw reply [flat|nested] 21+ messages in thread
[parent not found: <dca004cf-bacc-1a1f-56d6-c06e8bec167a@amd.com>]
* Re: [v6 1/1] x86/bugs: Implement mitigation for Predictive Store [not found] ` <dca004cf-bacc-1a1f-56d6-c06e8bec167a@amd.com> @ 2021-09-04 17:23 ` Josh Poimboeuf 2021-09-07 23:15 ` Babu Moger 2021-09-09 16:20 ` Bandan Das 0 siblings, 2 replies; 21+ messages in thread From: Josh Poimboeuf @ 2021-09-04 17:23 UTC (permalink / raw) To: Moger, Babu Cc: Babu Moger, bp, bsd, corbet, hpa, linux-kernel, mingo, tglx, x86 On Fri, Sep 03, 2021 at 07:52:43PM -0500, Moger, Babu wrote: > > BTW, is the list of PSF-affected CPUs the same as the list of > > SSB-affected CPUs? If there might be PSF CPUs which don't have SSB, > > then more logic will need to be added to ensure a sensible default. > I can't think of a scenario where it is not same on a system. To clarify, I'm asking about CPU capabilities. Are there any AMD CPUs with the PSF feature, which don't have SSB? > > On a related note, is there a realistic, non-hypothetical need to have > > separate policies and cmdline options for both SSB and PSF? i.e. is > > there a real-world scenario where a user needs to disable PSF while > > leaving SSB enabled? > > https://www.amd.com/system/files/documents/security-analysis-predictive-store-forwarding.pdf <https://www.amd.com/system/files/documents/security-analysis-predictive-store-forwarding.pdf> > There are some examples in the document. Probably it is too soon to tell if > those are real real-world scenarios as this feature is very new. I didn't see any actual examples. Are you referring to this sentence? "PSFD may be desirable for software which is concerned with the speculative behavior of PSF but desires a smaller performance impact than setting SSBD." > > Because trying to give them separate interfaces, when PSF disable is > > intertwined with SSB disable in hardware, is awkward and confusing. And > > the idea of adding another double-negative interface (disable=off!), > > just because a vulnerability is considered to be a CPU "feature", isn't > > very appetizing. > > > > So instead of adding a new double-negative interface, which only *half* > > works due to the ssb_disable dependency, and which is guaranteed to > > further confuse users, and which not even be used in the real world > > except possibly by confused users... > > > > I'm wondering if we can just start out with the simplest possible > > approach: don't change any code and instead just document the fact that > > "spec_store_bypass_disable=" also affects PSF. > > > > Then, later on, if a real-world need is demonstrated, actual code could > > be added to support disabling PSF independently (but of course it would > > never be fully independent since PSF disable is forced by SSB disable). > > Do you mean for now keep only 'on' and 'auto' and remove "off"? No, since PSF can already be mitigated with SSBD today, I'm suggesting that all code be removed from the patch and instead just update the documentation. -- Josh ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [v6 1/1] x86/bugs: Implement mitigation for Predictive Store 2021-09-04 17:23 ` Josh Poimboeuf @ 2021-09-07 23:15 ` Babu Moger 2021-09-08 18:20 ` Josh Poimboeuf 2021-09-09 16:20 ` Bandan Das 1 sibling, 1 reply; 21+ messages in thread From: Babu Moger @ 2021-09-07 23:15 UTC (permalink / raw) To: Josh Poimboeuf, Moger, Babu Cc: bp, bsd, corbet, hpa, linux-kernel, mingo, tglx, x86 On 9/4/21 12:23 PM, Josh Poimboeuf wrote: > On Fri, Sep 03, 2021 at 07:52:43PM -0500, Moger, Babu wrote: >>> BTW, is the list of PSF-affected CPUs the same as the list of >>> SSB-affected CPUs? If there might be PSF CPUs which don't have SSB, >>> then more logic will need to be added to ensure a sensible default. >> I can't think of a scenario where it is not same on a system. > > To clarify, I'm asking about CPU capabilities. Are there any AMD CPUs > with the PSF feature, which don't have SSB? No. That combination is not there. It is always SSB + PSF. > >>> On a related note, is there a realistic, non-hypothetical need to have >>> separate policies and cmdline options for both SSB and PSF? i.e. is >>> there a real-world scenario where a user needs to disable PSF while >>> leaving SSB enabled? >> >> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.amd.com%2Fsystem%2Ffiles%2Fdocuments%2Fsecurity-analysis-predictive-store-forwarding.pdf&data=04%7C01%7CBabu.Moger%40amd.com%7Cfcbc2781e8b54ed6ca6b08d96fc8bdf2%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637663730522361553%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=eI8DisfM4gpj%2B4AOfOUFGOFRZP6zqhSsTJIwINHK5GY%3D&reserved=0 <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.amd.com%2Fsystem%2Ffiles%2Fdocuments%2Fsecurity-analysis-predictive-store-forwarding.pdf&data=04%7C01%7CBabu.Moger%40amd.com%7Cfcbc2781e8b54ed6ca6b08d96fc8bdf2%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637663730522361553%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=eI8DisfM4gpj%2B4AOfOUFGOFRZP6zqhSsTJIwINHK5GY%3D&reserved=0> >> There are some examples in the document. Probably it is too soon to tell if >> those are real real-world scenarios as this feature is very new. > > I didn't see any actual examples. Are you referring to this sentence? Agree. It is not an actual example. There is also this text about where it can be useful. "AMD believes that for most applications, the security risk of PSF is likely low and where isolation is required, techniques such as address space isolation are preferred over software sandboxing." > > "PSFD may be desirable for software which is concerned with the > speculative behavior of PSF but desires a smaller performance impact > than setting SSBD." > >>> Because trying to give them separate interfaces, when PSF disable is >>> intertwined with SSB disable in hardware, is awkward and confusing. And >>> the idea of adding another double-negative interface (disable=off!), >>> just because a vulnerability is considered to be a CPU "feature", isn't >>> very appetizing. >>> >>> So instead of adding a new double-negative interface, which only *half* >>> works due to the ssb_disable dependency, and which is guaranteed to >>> further confuse users, and which not even be used in the real world >>> except possibly by confused users... >>> >>> I'm wondering if we can just start out with the simplest possible >>> approach: don't change any code and instead just document the fact that >>> "spec_store_bypass_disable=" also affects PSF. >>> >>> Then, later on, if a real-world need is demonstrated, actual code could >>> be added to support disabling PSF independently (but of course it would >>> never be fully independent since PSF disable is forced by SSB disable). >> >> Do you mean for now keep only 'on' and 'auto' and remove "off"? > > No, since PSF can already be mitigated with SSBD today, I'm suggesting > that all code be removed from the patch and instead just update the > documentation. > Hmm Interesting.. Just updating the documentation and without giving interface to enable or disable will not be a much of a value add. In the earlier discussion, the direction was to go with simple "on" and "off". https://lore.kernel.org/lkml/202105101508.BC6CC99FAD@keescook/ But, I am not sure right now. thanks Babu ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [v6 1/1] x86/bugs: Implement mitigation for Predictive Store 2021-09-07 23:15 ` Babu Moger @ 2021-09-08 18:20 ` Josh Poimboeuf 2021-09-10 16:08 ` Babu Moger 0 siblings, 1 reply; 21+ messages in thread From: Josh Poimboeuf @ 2021-09-08 18:20 UTC (permalink / raw) To: Babu Moger Cc: Moger, Babu, bp, bsd, corbet, hpa, linux-kernel, mingo, tglx, x86 On Tue, Sep 07, 2021 at 06:15:53PM -0500, Babu Moger wrote: > >>> Because trying to give them separate interfaces, when PSF disable is > >>> intertwined with SSB disable in hardware, is awkward and confusing. And > >>> the idea of adding another double-negative interface (disable=off!), > >>> just because a vulnerability is considered to be a CPU "feature", isn't > >>> very appetizing. > >>> > >>> So instead of adding a new double-negative interface, which only *half* > >>> works due to the ssb_disable dependency, and which is guaranteed to > >>> further confuse users, and which not even be used in the real world > >>> except possibly by confused users... > >>> > >>> I'm wondering if we can just start out with the simplest possible > >>> approach: don't change any code and instead just document the fact that > >>> "spec_store_bypass_disable=" also affects PSF. > >>> > >>> Then, later on, if a real-world need is demonstrated, actual code could > >>> be added to support disabling PSF independently (but of course it would > >>> never be fully independent since PSF disable is forced by SSB disable). > >> > >> Do you mean for now keep only 'on' and 'auto' and remove "off"? > > > > No, since PSF can already be mitigated with SSBD today, I'm suggesting > > that all code be removed from the patch and instead just update the > > documentation. > > > > Hmm Interesting.. > Just updating the documentation and without giving interface to enable or > disable will not be a much of a value add. It's also not a value add to create controls and added complexity for a feature which nobody needs. There's no harm in starting out with the simplest possible solution, which is no code at all. Code can always be added later if really needed... -- Josh ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [v6 1/1] x86/bugs: Implement mitigation for Predictive Store 2021-09-08 18:20 ` Josh Poimboeuf @ 2021-09-10 16:08 ` Babu Moger 0 siblings, 0 replies; 21+ messages in thread From: Babu Moger @ 2021-09-10 16:08 UTC (permalink / raw) To: Josh Poimboeuf Cc: Moger, Babu, bp, bsd, corbet, hpa, linux-kernel, mingo, tglx, x86 On 9/8/21 1:20 PM, Josh Poimboeuf wrote: > On Tue, Sep 07, 2021 at 06:15:53PM -0500, Babu Moger wrote: >>>>> Because trying to give them separate interfaces, when PSF disable is >>>>> intertwined with SSB disable in hardware, is awkward and confusing. And >>>>> the idea of adding another double-negative interface (disable=off!), >>>>> just because a vulnerability is considered to be a CPU "feature", isn't >>>>> very appetizing. >>>>> >>>>> So instead of adding a new double-negative interface, which only *half* >>>>> works due to the ssb_disable dependency, and which is guaranteed to >>>>> further confuse users, and which not even be used in the real world >>>>> except possibly by confused users... >>>>> >>>>> I'm wondering if we can just start out with the simplest possible >>>>> approach: don't change any code and instead just document the fact that >>>>> "spec_store_bypass_disable=" also affects PSF. >>>>> >>>>> Then, later on, if a real-world need is demonstrated, actual code could >>>>> be added to support disabling PSF independently (but of course it would >>>>> never be fully independent since PSF disable is forced by SSB disable). >>>> >>>> Do you mean for now keep only 'on' and 'auto' and remove "off"? >>> >>> No, since PSF can already be mitigated with SSBD today, I'm suggesting >>> that all code be removed from the patch and instead just update the >>> documentation. >>> >> >> Hmm Interesting.. >> Just updating the documentation and without giving interface to enable or >> disable will not be a much of a value add. > > It's also not a value add to create controls and added complexity for a > feature which nobody needs. There's no harm in starting out with the > simplest possible solution, which is no code at all. > > Code can always be added later if really needed... > Alright. Lets revisit this later when it seems reasonable to add this in the kernel. For now, I will focus on exposing this feature in KVM where guests can make use of it. It appears straight forward. Will send those patches soon. thanks Babu ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [v6 1/1] x86/bugs: Implement mitigation for Predictive Store 2021-09-04 17:23 ` Josh Poimboeuf 2021-09-07 23:15 ` Babu Moger @ 2021-09-09 16:20 ` Bandan Das 1 sibling, 0 replies; 21+ messages in thread From: Bandan Das @ 2021-09-09 16:20 UTC (permalink / raw) To: Josh Poimboeuf Cc: Moger, Babu, Babu Moger, bp, corbet, hpa, linux-kernel, mingo, tglx, x86 Josh Poimboeuf <jpoimboe@redhat.com> writes: > On Fri, Sep 03, 2021 at 07:52:43PM -0500, Moger, Babu wrote: >> > BTW, is the list of PSF-affected CPUs the same as the list of >> > SSB-affected CPUs? If there might be PSF CPUs which don't have SSB, >> > then more logic will need to be added to ensure a sensible default. >> I can't think of a scenario where it is not same on a system. > > To clarify, I'm asking about CPU capabilities. Are there any AMD CPUs > with the PSF feature, which don't have SSB? > >> > On a related note, is there a realistic, non-hypothetical need to have >> > separate policies and cmdline options for both SSB and PSF? i.e. is >> > there a real-world scenario where a user needs to disable PSF while >> > leaving SSB enabled? >> >> https://www.amd.com/system/files/documents/security-analysis-predictive-store-forwarding.pdf <https://www.amd.com/system/files/documents/security-analysis-predictive-store-forwarding.pdf> >> There are some examples in the document. Probably it is too soon to tell if >> those are real real-world scenarios as this feature is very new. > > I didn't see any actual examples. Are you referring to this sentence? > > "PSFD may be desirable for software which is concerned with the > speculative behavior of PSF but desires a smaller performance impact > than setting SSBD." > Sounds reasonable. It would have been good if the whitepaper mentioned any real examples which could benefit from selectively disabling psf. Generally speaking, as a user, I would either want to turn speculation entirely off or on which is what ssbd already does. >> > Because trying to give them separate interfaces, when PSF disable is >> > intertwined with SSB disable in hardware, is awkward and confusing. And >> > the idea of adding another double-negative interface (disable=off!), >> > just because a vulnerability is considered to be a CPU "feature", isn't >> > very appetizing. >> > >> > So instead of adding a new double-negative interface, which only *half* >> > works due to the ssb_disable dependency, and which is guaranteed to >> > further confuse users, and which not even be used in the real world >> > except possibly by confused users... >> > >> > I'm wondering if we can just start out with the simplest possible >> > approach: don't change any code and instead just document the fact that >> > "spec_store_bypass_disable=" also affects PSF. >> > >> > Then, later on, if a real-world need is demonstrated, actual code could >> > be added to support disabling PSF independently (but of course it would >> > never be fully independent since PSF disable is forced by SSB disable). >> >> Do you mean for now keep only 'on' and 'auto' and remove "off"? > > No, since PSF can already be mitigated with SSBD today, I'm suggesting > that all code be removed from the patch and instead just update the > documentation. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [v6 0/1] Introduce support for PSF control. 2021-05-17 22:00 [v6 0/1] Introduce support for PSF control Ramakrishna Saripalli 2021-05-17 22:00 ` [v6 1/1] x86/bugs: Implement mitigation for Predictive Store Forwarding Ramakrishna Saripalli @ 2021-06-17 20:47 ` Saripalli, RK 1 sibling, 0 replies; 21+ messages in thread From: Saripalli, RK @ 2021-06-17 20:47 UTC (permalink / raw) To: linux-kernel, x86, tglx, mingo, bp, hpa; +Cc: bsd On 5/17/2021 5:00 PM, Ramakrishna Saripalli wrote: > From: Ramakrishna Saripalli <rk.saripalli@amd.com> > > Predictive Store Forwarding: > AMD Zen3 processors feature a new technology called > Predictive Store Forwarding (PSF). > > https://www.amd.com/system/files/documents/security-analysis-predictive-store-forwarding.pdf > > PSF is a hardware-based micro-architectural optimization designed > to improve the performance of code execution by predicting address > dependencies between loads and stores. > > How PSF works: > > It is very common for a CPU to execute a load instruction to an address > that was recently written by a store. Modern CPUs implement a technique > known as Store-To-Load-Forwarding (STLF) to improve performance in such > cases. With STLF, data from the store is forwarded directly to the load > without having to wait for it to be written to memory. In a typical CPU, > STLF occurs after the address of both the load and store are calculated > and determined to match. > > PSF expands on this by speculating on the relationship between loads and > stores without waiting for the address calculation to complete. With PSF, > the CPU learns over time the relationship between loads and stores. > If STLF typically occurs between a particular store and load, the CPU will > remember this. > > In typical code, PSF provides a performance benefit by speculating on > the load result and allowing later instructions to begin execution > sooner than they otherwise would be able to. > > Causes of Incorrect PSF: > > Incorrect PSF predictions can occur due to two reasons. > > First, it is possible that the store/load pair had a dependency for a > while but later stops having a dependency. This can occur if the address > of either the store or load changes during the execution of the program. > > The second source of incorrect PSF predictions can occur if there is an > alias in the PSF predictor structure. The PSF predictor tracks > store-load pairs based on portions of their RIP. It is possible that a > store-load pair which does have a dependency may alias in the predictor > with another store-load pair which does not. > > This can result in incorrect speculation when the second store/load pair > is executed. > > Security Analysis: > > Previous research has shown that when CPUs speculate on non-architectural > paths it can lead to the potential of side channel attacks. > In particular, programs that implement isolation, also known as > ‘sandboxing’, entirely in software may need to be concerned with incorrect > CPU speculation as they can occur due to bad PSF predictions. > > Because PSF speculation is limited to the current program context, > the impact of bad PSF speculation is very similar to that of > Speculative Store Bypass (Spectre v4) > > Predictive Store Forwarding controls: > There are two hardware control bits which influence the PSF feature: > - MSR 48h bit 2 – Speculative Store Bypass (SSBD) > - MSR 48h bit 7 – Predictive Store Forwarding Disable (PSFD) > > The PSF feature is disabled if either of these bits are set. These bits > are controllable on a per-thread basis in an SMT system. By default, both > SSBD and PSFD are 0 meaning that the speculation features are enabled. > > While the SSBD bit disables PSF and speculative store bypass, PSFD only > disables PSF. > > PSFD may be desirable for software which is concerned with the > speculative behavior of PSF but desires a smaller performance impact than > setting SSBD. > > Support for PSFD is indicated in CPUID Fn8000_0008 EBX[28]. > All processors that support PSF will also support PSFD. > > ChangeLogs: > V6->V5: > Moved PSF control code to arch/x86/kernel/cpu/bugs.c > PSF mitigation is similar to spec_control_bypass mitigation. > PSF mitigation has only ON and OFF controls. > Kernel parameter changed to predictive_store_fwd_disable. > V5->V4: > Replaced rdmsrl and wrmsrl for setting SPEC_CTRL_PSFD with > a single call to msr_set_bit. > Removed temporary variable to read and write the MSR > V4->V3: > Write to MSR_IA32_SPEC_CTRL properly > Read MSR, modify PSFD bit based on kernel parameter and > write back to MSR. > > Changes made in psf_cmdline() and check_bugs(). > V3->V2: > Set the X86_FEATURE_SPEC_CTRL_MSR cap in boot cpu caps. > Fix kernel documentation for the kernel parameter. > Rename PSF to a control instead of mitigation. > > V1->V2: > - Smashed multiple commits into one commit. > - Rename PSF to a control instead of mitigation. > > V1: > - Initial patchset. > - Kernel parameter controls enable and disable of PSF. > > > Gentle ping. Any more concerns or feedback with this patch series?. Thanks, RK > > Ramakrishna Saripalli (1): > x86/bugs: Implement mitigation for Predictive Store Forwarding > > .../admin-guide/kernel-parameters.txt | 5 + > arch/x86/include/asm/cpufeatures.h | 1 + > arch/x86/include/asm/msr-index.h | 2 + > arch/x86/include/asm/nospec-branch.h | 6 ++ > arch/x86/kernel/cpu/bugs.c | 94 +++++++++++++++++++ > 5 files changed, 108 insertions(+) > > > base-commit: 0e16f466004d7f04296b9676a712a32a12367d1f > ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2021-09-10 16:10 UTC | newest] Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-05-17 22:00 [v6 0/1] Introduce support for PSF control Ramakrishna Saripalli 2021-05-17 22:00 ` [v6 1/1] x86/bugs: Implement mitigation for Predictive Store Forwarding Ramakrishna Saripalli 2021-05-18 2:55 ` Randy Dunlap 2021-05-18 12:27 ` Saripalli, RK 2021-05-18 20:35 ` Pawan Gupta 2021-05-19 5:38 ` Pawan Gupta 2021-05-19 13:19 ` Saripalli, RK 2021-05-19 5:50 ` Pawan Gupta 2021-09-01 20:20 ` [v6 1/1] x86/bugs: Implement mitigation for Predictive Store Babu Moger 2021-09-01 20:30 ` Babu Moger 2021-09-01 20:35 ` Babu Moger 2021-09-02 17:35 ` Pawan Gupta 2021-08-12 23:44 ` [v6 1/1] x86/bugs: Implement mitigation for Predictive Store Forwarding Josh Poimboeuf 2021-09-02 18:16 ` [v6 1/1] x86/bugs: Implement mitigation for Predictive Store Babu Moger 2021-09-03 0:07 ` Josh Poimboeuf [not found] ` <dca004cf-bacc-1a1f-56d6-c06e8bec167a@amd.com> 2021-09-04 17:23 ` Josh Poimboeuf 2021-09-07 23:15 ` Babu Moger 2021-09-08 18:20 ` Josh Poimboeuf 2021-09-10 16:08 ` Babu Moger 2021-09-09 16:20 ` Bandan Das 2021-06-17 20:47 ` [v6 0/1] Introduce support for PSF control Saripalli, RK
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.