Skip to content

Commit be261ff

Browse files
jpoimboeKAGA-KOKO
authored andcommitted
x86: Remove X86_FEATURE_MFENCE_RDTSC
AMD and Intel both have serializing lfence (X86_FEATURE_LFENCE_RDTSC). They've both had it for a long time, and AMD has had it enabled in Linux since Spectre v1 was announced. Back then, there was a proposal to remove the serializing mfence feature bit (X86_FEATURE_MFENCE_RDTSC), since both AMD and Intel have serializing lfence. At the time, it was (ahem) speculated that some hypervisors might not yet support its removal, so it remained for the time being. Now a year-and-a-half later, it should be safe to remove. I asked Andrew Cooper about whether it's still needed: So if you're virtualised, you've got no choice in the matter.  lfence is either dispatch-serialising or not on AMD, and you won't be able to change it. Furthermore, you can't accurately tell what state the bit is in, because the MSR might not be virtualised at all, or may not reflect the true state in hardware.  Worse still, attempting to set the bit may not be successful even if there isn't a fault for doing so. Xen sets the DE_CFG bit unconditionally, as does Linux by the looks of things (see MSR_F10H_DECFG_LFENCE_SERIALIZE_BIT).  ISTR other hypervisor vendors saying the same, but I don't have any information to hand. If you are running under a hypervisor which has been updated, then lfence will almost certainly be dispatch-serialising in practice, and you'll almost certainly see the bit already set in DE_CFG.  If you're running under a hypervisor which hasn't been patched since Spectre, you've already lost in many more ways. I'd argue that X86_FEATURE_MFENCE_RDTSC is not worth keeping. So remove it. This will reduce some code rot, and also make it easier to hook barrier_nospec() up to a cmdline disable for performance raisins, without having to need an alternative_3() macro. Signed-off-by: Josh Poimboeuf <[email protected]> Signed-off-by: Thomas Gleixner <[email protected]> Link: https://lkml.kernel.org/r/d990aa51e40063acb9888e8c1b688e41355a9588.1562255067.git.jpoimboe@redhat.com
1 parent 018ebca commit be261ff

File tree

6 files changed

+8
-42
lines changed

6 files changed

+8
-42
lines changed

arch/x86/include/asm/barrier.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,7 @@ static inline unsigned long array_index_mask_nospec(unsigned long index,
4949
#define array_index_mask_nospec array_index_mask_nospec
5050

5151
/* Prevent speculative execution past this barrier. */
52-
#define barrier_nospec() alternative_2("", "mfence", X86_FEATURE_MFENCE_RDTSC, \
53-
"lfence", X86_FEATURE_LFENCE_RDTSC)
52+
#define barrier_nospec() alternative("", "lfence", X86_FEATURE_LFENCE_RDTSC)
5453

5554
#define dma_rmb() barrier()
5655
#define dma_wmb() barrier()

arch/x86/include/asm/cpufeatures.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,6 @@
9696
#define X86_FEATURE_SYSCALL32 ( 3*32+14) /* "" syscall in IA32 userspace */
9797
#define X86_FEATURE_SYSENTER32 ( 3*32+15) /* "" sysenter in IA32 userspace */
9898
#define X86_FEATURE_REP_GOOD ( 3*32+16) /* REP microcode works well */
99-
#define X86_FEATURE_MFENCE_RDTSC ( 3*32+17) /* "" MFENCE synchronizes RDTSC */
10099
#define X86_FEATURE_LFENCE_RDTSC ( 3*32+18) /* "" LFENCE synchronizes RDTSC */
101100
#define X86_FEATURE_ACC_POWER ( 3*32+19) /* AMD Accumulated Power Mechanism */
102101
#define X86_FEATURE_NOPL ( 3*32+20) /* The NOPL (0F 1F) instructions */

arch/x86/include/asm/msr.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -233,8 +233,7 @@ static __always_inline unsigned long long rdtsc_ordered(void)
233233
* Thus, use the preferred barrier on the respective CPU, aiming for
234234
* RDTSCP as the default.
235235
*/
236-
asm volatile(ALTERNATIVE_3("rdtsc",
237-
"mfence; rdtsc", X86_FEATURE_MFENCE_RDTSC,
236+
asm volatile(ALTERNATIVE_2("rdtsc",
238237
"lfence; rdtsc", X86_FEATURE_LFENCE_RDTSC,
239238
"rdtscp", X86_FEATURE_RDTSCP)
240239
: EAX_EDX_RET(val, low, high)

arch/x86/kernel/cpu/amd.c

Lines changed: 3 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -879,32 +879,17 @@ static void init_amd(struct cpuinfo_x86 *c)
879879
init_amd_cacheinfo(c);
880880

881881
if (cpu_has(c, X86_FEATURE_XMM2)) {
882-
unsigned long long val;
883-
int ret;
884-
885882
/*
886-
* A serializing LFENCE has less overhead than MFENCE, so
887-
* use it for execution serialization. On families which
883+
* Use LFENCE for execution serialization. On families which
888884
* don't have that MSR, LFENCE is already serializing.
889885
* msr_set_bit() uses the safe accessors, too, even if the MSR
890886
* is not present.
891887
*/
892888
msr_set_bit(MSR_F10H_DECFG,
893889
MSR_F10H_DECFG_LFENCE_SERIALIZE_BIT);
894890

895-
/*
896-
* Verify that the MSR write was successful (could be running
897-
* under a hypervisor) and only then assume that LFENCE is
898-
* serializing.
899-
*/
900-
ret = rdmsrl_safe(MSR_F10H_DECFG, &val);
901-
if (!ret && (val & MSR_F10H_DECFG_LFENCE_SERIALIZE)) {
902-
/* A serializing LFENCE stops RDTSC speculation */
903-
set_cpu_cap(c, X86_FEATURE_LFENCE_RDTSC);
904-
} else {
905-
/* MFENCE stops RDTSC speculation */
906-
set_cpu_cap(c, X86_FEATURE_MFENCE_RDTSC);
907-
}
891+
/* A serializing LFENCE stops RDTSC speculation */
892+
set_cpu_cap(c, X86_FEATURE_LFENCE_RDTSC);
908893
}
909894

910895
/*

arch/x86/kernel/cpu/hygon.c

Lines changed: 3 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -330,32 +330,17 @@ static void init_hygon(struct cpuinfo_x86 *c)
330330
init_hygon_cacheinfo(c);
331331

332332
if (cpu_has(c, X86_FEATURE_XMM2)) {
333-
unsigned long long val;
334-
int ret;
335-
336333
/*
337-
* A serializing LFENCE has less overhead than MFENCE, so
338-
* use it for execution serialization. On families which
334+
* Use LFENCE for execution serialization. On families which
339335
* don't have that MSR, LFENCE is already serializing.
340336
* msr_set_bit() uses the safe accessors, too, even if the MSR
341337
* is not present.
342338
*/
343339
msr_set_bit(MSR_F10H_DECFG,
344340
MSR_F10H_DECFG_LFENCE_SERIALIZE_BIT);
345341

346-
/*
347-
* Verify that the MSR write was successful (could be running
348-
* under a hypervisor) and only then assume that LFENCE is
349-
* serializing.
350-
*/
351-
ret = rdmsrl_safe(MSR_F10H_DECFG, &val);
352-
if (!ret && (val & MSR_F10H_DECFG_LFENCE_SERIALIZE)) {
353-
/* A serializing LFENCE stops RDTSC speculation */
354-
set_cpu_cap(c, X86_FEATURE_LFENCE_RDTSC);
355-
} else {
356-
/* MFENCE stops RDTSC speculation */
357-
set_cpu_cap(c, X86_FEATURE_MFENCE_RDTSC);
358-
}
342+
/* A serializing LFENCE stops RDTSC speculation */
343+
set_cpu_cap(c, X86_FEATURE_LFENCE_RDTSC);
359344
}
360345

361346
/*

tools/arch/x86/include/asm/cpufeatures.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,6 @@
9696
#define X86_FEATURE_SYSCALL32 ( 3*32+14) /* "" syscall in IA32 userspace */
9797
#define X86_FEATURE_SYSENTER32 ( 3*32+15) /* "" sysenter in IA32 userspace */
9898
#define X86_FEATURE_REP_GOOD ( 3*32+16) /* REP microcode works well */
99-
#define X86_FEATURE_MFENCE_RDTSC ( 3*32+17) /* "" MFENCE synchronizes RDTSC */
10099
#define X86_FEATURE_LFENCE_RDTSC ( 3*32+18) /* "" LFENCE synchronizes RDTSC */
101100
#define X86_FEATURE_ACC_POWER ( 3*32+19) /* AMD Accumulated Power Mechanism */
102101
#define X86_FEATURE_NOPL ( 3*32+20) /* The NOPL (0F 1F) instructions */

0 commit comments

Comments
 (0)