The RETPOLINE_AMD feature is set by default for AMD hardware. This
feature expects LFENCE to be serializing in order to perform the proper
speculation control. If LFENCE cannot be determined to be serializing
(for example, when running under a hypervisor that does not allow writing
to the MSR that makes LFENCE serializing) the feature needs to be
disabled. The kernel will then fall back to using the generic retpoline
support.
The following patches are included in this series:
- Add a function to clear the RETPOLINE_AMD feature and update the
variable used for sysfs output.
- If LFENCE can not be determined to be serializing call the new
function to clear the RETPOLINE_AMD feature.
This patch series is based on tip:x86/pti.
---
Tom Lendacky (2):
x86/retpoline: Add a function to clear the RETPOLINE_AMD feature
x86/cpu/AMD: Clear RETPOLINE_AMD if LFENCE is not serializing
arch/x86/include/asm/nospec-branch.h | 1 +
arch/x86/kernel/cpu/amd.c | 4 ++++
arch/x86/kernel/cpu/bugs.c | 10 ++++++++++
3 files changed, 15 insertions(+)
--
Tom Lendacky
For AMD hardware, the RETPOLINE_AMD feature is dependent on LFENCE being
a serializing instruction. Create a function to allow RETPOLINE_AMD to
be cleared if it cannot be determined that LFENCE is serializing. In
addition, update the spectre_v2_enabled variable so that sysfs output is
correct.
Signed-off-by: Tom Lendacky <[email protected]>
---
arch/x86/include/asm/nospec-branch.h | 1 +
arch/x86/kernel/cpu/bugs.c | 10 ++++++++++
2 files changed, 11 insertions(+)
diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
index 8ddf851..5785684 100644
--- a/arch/x86/include/asm/nospec-branch.h
+++ b/arch/x86/include/asm/nospec-branch.h
@@ -153,6 +153,7 @@
#endif
void spectre_v2_check_boottime_disable(void);
+void retpoline_amd_disable(void);
#endif /* __ASSEMBLY__ */
#endif /* __NOSPEC_BRANCH_H__ */
diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index b957f77..a4c594c 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -170,6 +170,16 @@ void __init spectre_v2_check_boottime_disable(void)
return;
}
+void retpoline_amd_disable(void)
+{
+ if (!boot_cpu_has(X86_FEATURE_RETPOLINE_AMD))
+ return;
+
+ setup_clear_cpu_cap(X86_FEATURE_RETPOLINE_AMD);
+ spectre_v2_enabled = retp_compiler() ?
+ SPECTRE_V2_RETPOLINE_GENERIC : SPECTRE_V2_RETPOLINE_MINIMAL;
+}
+
#ifdef CONFIG_SYSFS
ssize_t cpu_show_meltdown(struct device *dev,
struct device_attribute *attr, char *buf)
During early boot, spectre_v2_check_boottime_disable() sets the
RETPOLINE_AMD feature for AMD hardware (before init_amd() is called).
Since the RETPOLINE_AMD feature uses LFENCE to perform speculation
control, it can only be used if LFENCE is serializing. Clear the
RETPOLINE_AMD feature if the MSR write to make LFENCE serializing
is not successful.
Signed-off-by: Tom Lendacky <[email protected]>
---
arch/x86/kernel/cpu/amd.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index ea831c8..8a0076b 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -13,6 +13,7 @@
#include <asm/smp.h>
#include <asm/pci-direct.h>
#include <asm/delay.h>
+#include <asm/nospec-branch.h>
#ifdef CONFIG_X86_64
# include <asm/mmconfig.h>
@@ -854,6 +855,9 @@ static void init_amd(struct cpuinfo_x86 *c)
} else {
/* MFENCE stops RDTSC speculation */
set_cpu_cap(c, X86_FEATURE_MFENCE_RDTSC);
+
+ /* Don't use the LFENCE-based retpoline support */
+ retpoline_amd_disable();
}
}
On Tue, 9 Jan 2018, Tom Lendacky wrote:
> For AMD hardware, the RETPOLINE_AMD feature is dependent on LFENCE being
> a serializing instruction. Create a function to allow RETPOLINE_AMD to
> be cleared if it cannot be determined that LFENCE is serializing. In
> addition, update the spectre_v2_enabled variable so that sysfs output is
> correct.
>
> Signed-off-by: Tom Lendacky <[email protected]>
> ---
> arch/x86/include/asm/nospec-branch.h | 1 +
> arch/x86/kernel/cpu/bugs.c | 10 ++++++++++
> 2 files changed, 11 insertions(+)
>
> diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
> index 8ddf851..5785684 100644
> --- a/arch/x86/include/asm/nospec-branch.h
> +++ b/arch/x86/include/asm/nospec-branch.h
> @@ -153,6 +153,7 @@
> #endif
>
> void spectre_v2_check_boottime_disable(void);
> +void retpoline_amd_disable(void);
>
> #endif /* __ASSEMBLY__ */
> #endif /* __NOSPEC_BRANCH_H__ */
> diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
> index b957f77..a4c594c 100644
> --- a/arch/x86/kernel/cpu/bugs.c
> +++ b/arch/x86/kernel/cpu/bugs.c
> @@ -170,6 +170,16 @@ void __init spectre_v2_check_boottime_disable(void)
> return;
> }
>
> +void retpoline_amd_disable(void)
> +{
> + if (!boot_cpu_has(X86_FEATURE_RETPOLINE_AMD))
> + return;
> +
> + setup_clear_cpu_cap(X86_FEATURE_RETPOLINE_AMD);
> + spectre_v2_enabled = retp_compiler() ?
> + SPECTRE_V2_RETPOLINE_GENERIC : SPECTRE_V2_RETPOLINE_MINIMAL;
> +}
Urgh. That's an awful hack. why not do the obvious?
Thanks,
tglx
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -148,14 +148,21 @@ void __init spectre_v2_check_boottime_di
retpoline:
if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD) {
retpoline_amd:
+ if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD ||
+ !boot_cpu_has(X86_FEATURE_LFENCE_RDTSC)) {
+ pr_info("AMD retpoline not supported, fall back to generic\n");
+ goto retpoline_generic;
+ }
+
spectre_v2_enabled = retp_compiler() ?
SPECTRE_V2_RETPOLINE_AMD : SPECTRE_V2_RETPOLINE_MINIMAL_AMD;
setup_force_cpu_cap(X86_FEATURE_RETPOLINE_AMD);
- } else {
- retpoline_generic:
- spectre_v2_enabled = retp_compiler() ?
- SPECTRE_V2_RETPOLINE_GENERIC : SPECTRE_V2_RETPOLINE_MINIMAL;
+ setup_force_cpu_cap(X86_FEATURE_RETPOLINE);
+ return;
}
+retpoline_generic:
+ spectre_v2_enabled = retp_compiler() ?
+ SPECTRE_V2_RETPOLINE_GENERIC : SPECTRE_V2_RETPOLINE_MINIMAL;
setup_force_cpu_cap(X86_FEATURE_RETPOLINE);
return;
#else
On 1/9/2018 4:46 PM, Thomas Gleixner wrote:
> On Tue, 9 Jan 2018, Tom Lendacky wrote:
>> For AMD hardware, the RETPOLINE_AMD feature is dependent on LFENCE being
>> a serializing instruction. Create a function to allow RETPOLINE_AMD to
>> be cleared if it cannot be determined that LFENCE is serializing. In
>> addition, update the spectre_v2_enabled variable so that sysfs output is
>> correct.
>>
>> Signed-off-by: Tom Lendacky <[email protected]>
>> ---
>> arch/x86/include/asm/nospec-branch.h | 1 +
>> arch/x86/kernel/cpu/bugs.c | 10 ++++++++++
>> 2 files changed, 11 insertions(+)
>>
>> diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
>> index 8ddf851..5785684 100644
>> --- a/arch/x86/include/asm/nospec-branch.h
>> +++ b/arch/x86/include/asm/nospec-branch.h
>> @@ -153,6 +153,7 @@
>> #endif
>>
>> void spectre_v2_check_boottime_disable(void);
>> +void retpoline_amd_disable(void);
>>
>> #endif /* __ASSEMBLY__ */
>> #endif /* __NOSPEC_BRANCH_H__ */
>> diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
>> index b957f77..a4c594c 100644
>> --- a/arch/x86/kernel/cpu/bugs.c
>> +++ b/arch/x86/kernel/cpu/bugs.c
>> @@ -170,6 +170,16 @@ void __init spectre_v2_check_boottime_disable(void)
>> return;
>> }
>>
>> +void retpoline_amd_disable(void)
>> +{
>> + if (!boot_cpu_has(X86_FEATURE_RETPOLINE_AMD))
>> + return;
>> +
>> + setup_clear_cpu_cap(X86_FEATURE_RETPOLINE_AMD);
>> + spectre_v2_enabled = retp_compiler() ?
>> + SPECTRE_V2_RETPOLINE_GENERIC : SPECTRE_V2_RETPOLINE_MINIMAL;
>> +}
>
> Urgh. That's an awful hack. why not do the obvious?
My first attempt was very similar to your change below, but testing
showed that spectre_v2_check_boottime_disable() is called before the
X86_FEATURE_LFENCE_RDTSC can be set. I can look at moving where the
X86_FEATURE_LFENCE_RDTSC is set, maybe into early_init_amd() or such
if you think that would be best.
Thanks,
Tom
>
> Thanks,
>
> tglx
>
> --- a/arch/x86/kernel/cpu/bugs.c
> +++ b/arch/x86/kernel/cpu/bugs.c
> @@ -148,14 +148,21 @@ void __init spectre_v2_check_boottime_di
> retpoline:
> if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD) {
> retpoline_amd:
> + if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD ||
> + !boot_cpu_has(X86_FEATURE_LFENCE_RDTSC)) {
> + pr_info("AMD retpoline not supported, fall back to generic\n");
> + goto retpoline_generic;
> + }
> +
> spectre_v2_enabled = retp_compiler() ?
> SPECTRE_V2_RETPOLINE_AMD : SPECTRE_V2_RETPOLINE_MINIMAL_AMD;
> setup_force_cpu_cap(X86_FEATURE_RETPOLINE_AMD);
> - } else {
> - retpoline_generic:
> - spectre_v2_enabled = retp_compiler() ?
> - SPECTRE_V2_RETPOLINE_GENERIC : SPECTRE_V2_RETPOLINE_MINIMAL;
> + setup_force_cpu_cap(X86_FEATURE_RETPOLINE);
> + return;
> }
> +retpoline_generic:
> + spectre_v2_enabled = retp_compiler() ?
> + SPECTRE_V2_RETPOLINE_GENERIC : SPECTRE_V2_RETPOLINE_MINIMAL;
> setup_force_cpu_cap(X86_FEATURE_RETPOLINE);
> return;
> #else
>
>
On Tue, 9 Jan 2018, Tom Lendacky wrote:
> On 1/9/2018 4:46 PM, Thomas Gleixner wrote:
> > Urgh. That's an awful hack. why not do the obvious?
>
> My first attempt was very similar to your change below, but testing
> showed that spectre_v2_check_boottime_disable() is called before the
> X86_FEATURE_LFENCE_RDTSC can be set. I can look at moving where the
> X86_FEATURE_LFENCE_RDTSC is set, maybe into early_init_amd() or such
> if you think that would be best.
Wait, we can move the selection _AFTER_ identify_boot_cpu().
Thanks,
tglx
8<----------------
--- a/arch/x86/include/asm/nospec-branch.h
+++ b/arch/x86/include/asm/nospec-branch.h
@@ -152,7 +152,5 @@
# define THUNK_TARGET(addr) [thunk_target] "rm" (addr)
#endif
-void spectre_v2_check_boottime_disable(void);
-
#endif /* __ASSEMBLY__ */
#endif /* __NOSPEC_BRANCH_H__ */
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -24,6 +24,8 @@
#include <asm/pgtable.h>
#include <asm/set_memory.h>
+static void __init spectre_v2_check_boottime_disable(void);
+
void __init check_bugs(void)
{
identify_boot_cpu();
@@ -33,6 +35,9 @@ void __init check_bugs(void)
print_cpu_info(&boot_cpu_data);
}
+ /* Select the proper spectre mitigation before patching alternatives */
+ spectre_v2_check_boottime_disable();
+
#ifdef CONFIG_X86_32
/*
* Check whether we are able to run this kernel safely on SMP.
@@ -106,7 +111,7 @@ static inline bool match_option(const ch
return len == arglen && !strncmp(arg, opt, len);
}
-void __init spectre_v2_check_boottime_disable(void)
+static void __init spectre_v2_check_boottime_disable(void)
{
char arg[20];
int ret;
@@ -148,14 +153,21 @@ void __init spectre_v2_check_boottime_di
retpoline:
if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD) {
retpoline_amd:
+ if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD ||
+ !boot_cpu_has(X86_FEATURE_LFENCE_RDTSC)) {
+ pr_info("AMD retpoline not supported, fall back to generic\n");
+ goto retpoline_generic;
+ }
+
spectre_v2_enabled = retp_compiler() ?
SPECTRE_V2_RETPOLINE_AMD : SPECTRE_V2_RETPOLINE_MINIMAL_AMD;
setup_force_cpu_cap(X86_FEATURE_RETPOLINE_AMD);
- } else {
- retpoline_generic:
- spectre_v2_enabled = retp_compiler() ?
- SPECTRE_V2_RETPOLINE_GENERIC : SPECTRE_V2_RETPOLINE_MINIMAL;
+ setup_force_cpu_cap(X86_FEATURE_RETPOLINE);
+ return;
}
+retpoline_generic:
+ spectre_v2_enabled = retp_compiler() ?
+ SPECTRE_V2_RETPOLINE_GENERIC : SPECTRE_V2_RETPOLINE_MINIMAL;
setup_force_cpu_cap(X86_FEATURE_RETPOLINE);
return;
#else
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -1322,8 +1322,6 @@ void __init setup_arch(char **cmdline_p)
register_refined_jiffies(CLOCK_TICK_RATE);
- spectre_v2_check_boottime_disable();
-
#ifdef CONFIG_EFI
if (efi_enabled(EFI_BOOT))
efi_apply_memmap_quirks();
On 1/9/2018 5:09 PM, Thomas Gleixner wrote:
> On Tue, 9 Jan 2018, Tom Lendacky wrote:
>> On 1/9/2018 4:46 PM, Thomas Gleixner wrote:
>>> Urgh. That's an awful hack. why not do the obvious?
>>
>> My first attempt was very similar to your change below, but testing
>> showed that spectre_v2_check_boottime_disable() is called before the
>> X86_FEATURE_LFENCE_RDTSC can be set. I can look at moving where the
>> X86_FEATURE_LFENCE_RDTSC is set, maybe into early_init_amd() or such
>> if you think that would be best.
>
> Wait, we can move the selection _AFTER_ identify_boot_cpu().
Much cleaner. Since it's just a single patch now, do you want me to
re-submit this after I test it or will you just pick this up as is?
I did notice that the patch does change the behavior associated with
the command line options, though. Not sure if that was intentional.
Thanks,
Tom
>
> Thanks,
>
> tglx
>
> 8<----------------
>
> --- a/arch/x86/include/asm/nospec-branch.h
> +++ b/arch/x86/include/asm/nospec-branch.h
> @@ -152,7 +152,5 @@
> # define THUNK_TARGET(addr) [thunk_target] "rm" (addr)
> #endif
>
> -void spectre_v2_check_boottime_disable(void);
> -
> #endif /* __ASSEMBLY__ */
> #endif /* __NOSPEC_BRANCH_H__ */
> --- a/arch/x86/kernel/cpu/bugs.c
> +++ b/arch/x86/kernel/cpu/bugs.c
> @@ -24,6 +24,8 @@
> #include <asm/pgtable.h>
> #include <asm/set_memory.h>
>
> +static void __init spectre_v2_check_boottime_disable(void);
> +
> void __init check_bugs(void)
> {
> identify_boot_cpu();
> @@ -33,6 +35,9 @@ void __init check_bugs(void)
> print_cpu_info(&boot_cpu_data);
> }
>
> + /* Select the proper spectre mitigation before patching alternatives */
> + spectre_v2_check_boottime_disable();
> +
> #ifdef CONFIG_X86_32
> /*
> * Check whether we are able to run this kernel safely on SMP.
> @@ -106,7 +111,7 @@ static inline bool match_option(const ch
> return len == arglen && !strncmp(arg, opt, len);
> }
>
> -void __init spectre_v2_check_boottime_disable(void)
> +static void __init spectre_v2_check_boottime_disable(void)
> {
> char arg[20];
> int ret;
> @@ -148,14 +153,21 @@ void __init spectre_v2_check_boottime_di
> retpoline:
> if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD) {
> retpoline_amd:
> + if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD ||
> + !boot_cpu_has(X86_FEATURE_LFENCE_RDTSC)) {
> + pr_info("AMD retpoline not supported, fall back to generic\n");
> + goto retpoline_generic;
> + }
> +
> spectre_v2_enabled = retp_compiler() ?
> SPECTRE_V2_RETPOLINE_AMD : SPECTRE_V2_RETPOLINE_MINIMAL_AMD;
> setup_force_cpu_cap(X86_FEATURE_RETPOLINE_AMD);
> - } else {
> - retpoline_generic:
> - spectre_v2_enabled = retp_compiler() ?
> - SPECTRE_V2_RETPOLINE_GENERIC : SPECTRE_V2_RETPOLINE_MINIMAL;
> + setup_force_cpu_cap(X86_FEATURE_RETPOLINE);
> + return;
> }
> +retpoline_generic:
> + spectre_v2_enabled = retp_compiler() ?
> + SPECTRE_V2_RETPOLINE_GENERIC : SPECTRE_V2_RETPOLINE_MINIMAL;
> setup_force_cpu_cap(X86_FEATURE_RETPOLINE);
> return;
> #else
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -1322,8 +1322,6 @@ void __init setup_arch(char **cmdline_p)
>
> register_refined_jiffies(CLOCK_TICK_RATE);
>
> - spectre_v2_check_boottime_disable();
> -
> #ifdef CONFIG_EFI
> if (efi_enabled(EFI_BOOT))
> efi_apply_memmap_quirks();
>
On Tue, 9 Jan 2018, Tom Lendacky wrote:
> On 1/9/2018 5:09 PM, Thomas Gleixner wrote:
> > On Tue, 9 Jan 2018, Tom Lendacky wrote:
> >> On 1/9/2018 4:46 PM, Thomas Gleixner wrote:
> >>> Urgh. That's an awful hack. why not do the obvious?
> >>
> >> My first attempt was very similar to your change below, but testing
> >> showed that spectre_v2_check_boottime_disable() is called before the
> >> X86_FEATURE_LFENCE_RDTSC can be set. I can look at moving where the
> >> X86_FEATURE_LFENCE_RDTSC is set, maybe into early_init_amd() or such
> >> if you think that would be best.
> >
> > Wait, we can move the selection _AFTER_ identify_boot_cpu().
>
> Much cleaner. Since it's just a single patch now, do you want me to
> re-submit this after I test it or will you just pick this up as is?
>
> I did notice that the patch does change the behavior associated with
> the command line options, though. Not sure if that was intentional.
Yes. I noticed while looking at your issue that when AMD is selected on the
command line then we have no support at all on intel. So I prefer to err
out and enable the generic version.
If you can just polish it up with a changelog and resubmit after testing
that would be appreciated as I'm steam blasting the IBRS stuff at the
moment.
Thanks,
tglx
On 1/9/2018 5:43 PM, Thomas Gleixner wrote:
> On Tue, 9 Jan 2018, Tom Lendacky wrote:
>> On 1/9/2018 5:09 PM, Thomas Gleixner wrote:
>>> On Tue, 9 Jan 2018, Tom Lendacky wrote:
>>>> On 1/9/2018 4:46 PM, Thomas Gleixner wrote:
>>>>> Urgh. That's an awful hack. why not do the obvious?
>>>>
>>>> My first attempt was very similar to your change below, but testing
>>>> showed that spectre_v2_check_boottime_disable() is called before the
>>>> X86_FEATURE_LFENCE_RDTSC can be set. I can look at moving where the
>>>> X86_FEATURE_LFENCE_RDTSC is set, maybe into early_init_amd() or such
>>>> if you think that would be best.
>>>
>>> Wait, we can move the selection _AFTER_ identify_boot_cpu().
>>
>> Much cleaner. Since it's just a single patch now, do you want me to
>> re-submit this after I test it or will you just pick this up as is?
>>
>> I did notice that the patch does change the behavior associated with
>> the command line options, though. Not sure if that was intentional.
>
> Yes. I noticed while looking at your issue that when AMD is selected on the
> command line then we have no support at all on intel. So I prefer to err
> out and enable the generic version.
>
> If you can just polish it up with a changelog and resubmit after testing
> that would be appreciated as I'm steam blasting the IBRS stuff at the
> moment.
Will do. I'll add your Signed-off-by when I submit it.
Thanks,
Tom
>
> Thanks,
>
> tglx
>