2021-04-07 06:57:06

by Saripalli, RK

[permalink] [raw]
Subject: [PATCH 1/5] x86/cpufeatures: Define feature bits to support mitigation of PSF

From: Ramakrishna Saripalli <[email protected]>

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.

These features are being introduced to support mitigation from these 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_AMD_PSFD: CPUID_Fn80000008_EBX[28] ("PSF disable")
If this bit is 1, CPU implements PSF and PSF mitigation is
supported.

X86_FEATURE_PSFD: Generic Linux feature to indicate the CPU supports
controls to mitigate PSF.

X86_FEATURE_PSFD is set if and only if X86_FEATURE_AMD_PSFD
is set

X86_BUG_PSF: Generic Linux feature to indicate that the CPU is affected
by the PSF feature.

Signed-off-by: Ramakrishna Saripalli<[email protected]>
---
arch/x86/include/asm/cpufeatures.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index cc96e26d69f7..21e7f8d0d7d9 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -201,7 +201,7 @@
#define X86_FEATURE_INVPCID_SINGLE ( 7*32+ 7) /* Effectively INVPCID && CR4.PCIDE=1 */
#define X86_FEATURE_HW_PSTATE ( 7*32+ 8) /* AMD HW-PState */
#define X86_FEATURE_PROC_FEEDBACK ( 7*32+ 9) /* AMD ProcFeedbackInterface */
-/* FREE! ( 7*32+10) */
+#define X86_FEATURE_PSFD ( 7*32+10) /* Predictive Store Forward Disable */
#define X86_FEATURE_PTI ( 7*32+11) /* Kernel Page Table Isolation enabled */
#define X86_FEATURE_RETPOLINE ( 7*32+12) /* "" Generic Retpoline mitigation for Spectre variant 2 */
#define X86_FEATURE_RETPOLINE_AMD ( 7*32+13) /* "" AMD Retpoline mitigation for Spectre variant 2 */
@@ -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_AMD_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 */
@@ -428,5 +429,6 @@
#define X86_BUG_TAA X86_BUG(22) /* CPU is affected by TSX Async Abort(TAA) */
#define X86_BUG_ITLB_MULTIHIT X86_BUG(23) /* CPU may incur MCE during certain page attribute changes */
#define X86_BUG_SRBDS X86_BUG(24) /* CPU may leak RNG bits if not mitigated */
+#define X86_BUG_PSF X86_BUG(25) /* CPU is affected by Predictive Store Forwarding attack */

#endif /* _ASM_X86_CPUFEATURES_H */
--
2.25.1


2021-04-09 17:44:12

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH 1/5] x86/cpufeatures: Define feature bits to support mitigation of PSF

On Tue, Apr 06, 2021 at 10:50:00AM -0500, Ramakrishna Saripalli wrote:
> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
> index cc96e26d69f7..21e7f8d0d7d9 100644
> --- a/arch/x86/include/asm/cpufeatures.h
> +++ b/arch/x86/include/asm/cpufeatures.h
> @@ -201,7 +201,7 @@
> #define X86_FEATURE_INVPCID_SINGLE ( 7*32+ 7) /* Effectively INVPCID && CR4.PCIDE=1 */
> #define X86_FEATURE_HW_PSTATE ( 7*32+ 8) /* AMD HW-PState */
> #define X86_FEATURE_PROC_FEEDBACK ( 7*32+ 9) /* AMD ProcFeedbackInterface */
> -/* FREE! ( 7*32+10) */
> +#define X86_FEATURE_PSFD ( 7*32+10) /* Predictive Store Forward Disable */

You don't need this one...

> #define X86_FEATURE_PTI ( 7*32+11) /* Kernel Page Table Isolation enabled */
> #define X86_FEATURE_RETPOLINE ( 7*32+12) /* "" Generic Retpoline mitigation for Spectre variant 2 */
> #define X86_FEATURE_RETPOLINE_AMD ( 7*32+13) /* "" AMD Retpoline mitigation for Spectre variant 2 */
> @@ -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_AMD_PSFD (13*32+28) /* "" Predictive Store Forward Disable */

... when you have this one. And this one is AMD-specific so you can just
as well call it X86_FEATURE_PSFD and remove the "".

>
> /* Thermal and Power Management Leaf, CPUID level 0x00000006 (EAX), word 14 */
> #define X86_FEATURE_DTHERM (14*32+ 0) /* Digital Thermal Sensor */
> @@ -428,5 +429,6 @@
> #define X86_BUG_TAA X86_BUG(22) /* CPU is affected by TSX Async Abort(TAA) */
> #define X86_BUG_ITLB_MULTIHIT X86_BUG(23) /* CPU may incur MCE during certain page attribute changes */
> #define X86_BUG_SRBDS X86_BUG(24) /* CPU may leak RNG bits if not mitigated */
> +#define X86_BUG_PSF X86_BUG(25) /* CPU is affected by Predictive Store Forwarding attack */

And I think you don't need this one either if we do a "light" controls
thing but lemme look at the rest first.

Thx.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2021-04-09 18:25:30

by Saripalli, RK

[permalink] [raw]
Subject: Re: [PATCH 1/5] x86/cpufeatures: Define feature bits to support mitigation of PSF



On 4/9/2021 12:41 PM, Borislav Petkov wrote:
> On Tue, Apr 06, 2021 at 10:50:00AM -0500, Ramakrishna Saripalli wrote:
>> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
>> index cc96e26d69f7..21e7f8d0d7d9 100644
>> --- a/arch/x86/include/asm/cpufeatures.h
>> +++ b/arch/x86/include/asm/cpufeatures.h
>> @@ -201,7 +201,7 @@
>> #define X86_FEATURE_INVPCID_SINGLE ( 7*32+ 7) /* Effectively INVPCID && CR4.PCIDE=1 */
>> #define X86_FEATURE_HW_PSTATE ( 7*32+ 8) /* AMD HW-PState */
>> #define X86_FEATURE_PROC_FEEDBACK ( 7*32+ 9) /* AMD ProcFeedbackInterface */
>> -/* FREE! ( 7*32+10) */
>> +#define X86_FEATURE_PSFD ( 7*32+10) /* Predictive Store Forward Disable */
>
> You don't need this one...

Boris, I added this bit so we could detect later that PSF is supported.
But since PSF is AMD specific for now, I guess I will go along with your suggestions.
>
>> #define X86_FEATURE_PTI ( 7*32+11) /* Kernel Page Table Isolation enabled */
>> #define X86_FEATURE_RETPOLINE ( 7*32+12) /* "" Generic Retpoline mitigation for Spectre variant 2 */
>> #define X86_FEATURE_RETPOLINE_AMD ( 7*32+13) /* "" AMD Retpoline mitigation for Spectre variant 2 */
>> @@ -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_AMD_PSFD (13*32+28) /* "" Predictive Store Forward Disable */
>
> ... when you have this one. And this one is AMD-specific so you can just
> as well call it X86_FEATURE_PSFD and remove the "".
Ok
>
>>
>> /* Thermal and Power Management Leaf, CPUID level 0x00000006 (EAX), word 14 */
>> #define X86_FEATURE_DTHERM (14*32+ 0) /* Digital Thermal Sensor */
>> @@ -428,5 +429,6 @@
>> #define X86_BUG_TAA X86_BUG(22) /* CPU is affected by TSX Async Abort(TAA) */
>> #define X86_BUG_ITLB_MULTIHIT X86_BUG(23) /* CPU may incur MCE during certain page attribute changes */
>> #define X86_BUG_SRBDS X86_BUG(24) /* CPU may leak RNG bits if not mitigated */
>> +#define X86_BUG_PSF X86_BUG(25) /* CPU is affected by Predictive Store Forwarding attack */
>
> And I think you don't need this one either if we do a "light" controls
> thing but lemme look at the rest first.
>
Ok.
> Thx.
>

2021-04-09 19:41:18

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH 1/5] x86/cpufeatures: Define feature bits to support mitigation of PSF

On Fri, Apr 09, 2021 at 01:22:49PM -0500, Saripalli, RK wrote:
> > And I think you don't need this one either if we do a "light" controls
> > thing but lemme look at the rest first.

Ok, and what I mean with "lite" version is something like this below
which needs finishing and testing.

Initially, it could support the cmdline params:

predict_store_fwd={on,off,auto}

to give people the opportunity to experiment with the feature.

If it turns out that prctl and seccomp per-task toggling is needed then
sure, we can extend but I don't see the reason for a whole separate set
of options yet. Especially is ssbd already controls this.

AFAICT, of course and if I'm not missing some other aspect here.

Thx.

---
diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index 2d11384dc9ab..226b73700f88 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -1165,3 +1165,22 @@ void set_dr_addr_mask(unsigned long mask, int dr)
break;
}
}
+
+static int __init psf_cmdline(char *str)
+{
+ if (!boot_cpu_has(X86_FEATURE_PSFD))
+ return 0;
+
+ if (!str)
+ return -EINVAL;
+
+ if (!strcmp(str, "off")) {
+ x86_spec_ctrl_base |= SPEC_CTRL_PSFD;
+ setup_clear_cpu_cap(X86_FEATURE_PSFD);
+ }
+
+ return 0;
+}
+early_param("predict_store_fwd", psf_cmdline);
+
+

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2021-04-09 19:46:53

by Saripalli, RK

[permalink] [raw]
Subject: Re: [PATCH 1/5] x86/cpufeatures: Define feature bits to support mitigation of PSF

Boris, thank you.

On 4/9/2021 2:39 PM, Borislav Petkov wrote:
> On Fri, Apr 09, 2021 at 01:22:49PM -0500, Saripalli, RK wrote:
>>> And I think you don't need this one either if we do a "light" controls
>>> thing but lemme look at the rest first.
>
> Ok, and what I mean with "lite" version is something like this below
> which needs finishing and testing.
>
> Initially, it could support the cmdline params:
>
> predict_store_fwd={on,off,auto}
>
> to give people the opportunity to experiment with the feature.
>
> If it turns out that prctl and seccomp per-task toggling is needed then
> sure, we can extend but I don't see the reason for a whole separate set
> of options yet. Especially is ssbd already controls this.
>
> AFAICT, of course and if I'm not missing some other aspect here.
>
> Thx.

Yes, these options should be fine for now.
Like you said, if we get the need to add prctl and seccomp, I can always do that later.

What do you think auto should default to?.
In SSBD case, I believe auto defaults to prctl or seccomp.
Since we will not have that here, we should choose something for auto.


>
> ---
> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
> index 2d11384dc9ab..226b73700f88 100644
> --- a/arch/x86/kernel/cpu/amd.c
> +++ b/arch/x86/kernel/cpu/amd.c
> @@ -1165,3 +1165,22 @@ void set_dr_addr_mask(unsigned long mask, int dr)
> break;
> }
> }
> +
> +static int __init psf_cmdline(char *str)
> +{
> + if (!boot_cpu_has(X86_FEATURE_PSFD))
> + return 0;
> +
> + if (!str)
> + return -EINVAL;
> +
> + if (!strcmp(str, "off")) {
> + x86_spec_ctrl_base |= SPEC_CTRL_PSFD;
> + setup_clear_cpu_cap(X86_FEATURE_PSFD);
> + }
> +
> + return 0;
> +}
> +early_param("predict_store_fwd", psf_cmdline);
> +
> +
>

All the other mitigation x86 mitigation code goes into kernel/cpu/bugs.c.
I think psf_cmdline() or equivalent also belongs there and not in kernel/cpu/amd.c.

Looking forward to your feedback.

Thanks,
RK

2021-04-09 20:20:16

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH 1/5] x86/cpufeatures: Define feature bits to support mitigation of PSF

On Fri, Apr 09, 2021 at 02:45:23PM -0500, Saripalli, RK wrote:
> Yes, these options should be fine for now.
> Like you said, if we get the need to add prctl and seccomp, I can always do that later.
>
> What do you think auto should default to?.
> In SSBD case, I believe auto defaults to prctl or seccomp.
> Since we will not have that here, we should choose something for auto.

Or not add it yet. Just have "on" and "off" for now.

Which begs the question should this be controllable by the mitigations=
switch too?

I wanna say, let's have people evaluate and play with it first and
we can add it to that switch later. As long as we don't change the
user-visible controls - if anything we'll be extending them later,
potentially - we should be fine usage-wise and from user visibility POV.

> All the other mitigation x86 mitigation code goes into kernel/cpu/bugs.c.
> I think psf_cmdline() or equivalent also belongs there and not in kernel/cpu/amd.c.

It being AMD-specific, it can dwell in amd.c initially.

Thx.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

2021-04-09 20:30:24

by Saripalli, RK

[permalink] [raw]
Subject: Re: [PATCH 1/5] x86/cpufeatures: Define feature bits to support mitigation of PSF



On 4/9/2021 3:19 PM, Borislav Petkov wrote:
> On Fri, Apr 09, 2021 at 02:45:23PM -0500, Saripalli, RK wrote:
>> Yes, these options should be fine for now.
>> Like you said, if we get the need to add prctl and seccomp, I can always do that later.
>>
>> What do you think auto should default to?.
>> In SSBD case, I believe auto defaults to prctl or seccomp.
>> Since we will not have that here, we should choose something for auto.
>
> Or not add it yet. Just have "on" and "off" for now.

OK

>
> Which begs the question should this be controllable by the mitigations=
> switch too?

>
> I wanna say, let's have people evaluate and play with it first and
> we can add it to that switch later. As long as we don't change the
> user-visible controls - if anything we'll be extending them later,
> potentially - we should be fine usage-wise and from user visibility POV.

Agreed. We can add this later.
>
>> All the other mitigation x86 mitigation code goes into kernel/cpu/bugs.c.
>> I think psf_cmdline() or equivalent also belongs there and not in kernel/cpu/amd.c.
>
> It being AMD-specific, it can dwell in amd.c initially.

OK
>
> Thx.
>