On Wed, Jan 20, 2016 at 02:41:22AM -0800, H. Peter Anvin wrote:
> Ah. What would be even more of a win would be to rebias
> static_cpu_has_bug() so that the fallthrough case is the functional
> one. Easily done by reversing the labels.
By reversing you mean this:
---
diff --git a/arch/x86/include/asm/cpufeature.h
b/arch/x86/include/asm/cpufeature.h
index 77c51f4c15b7..49fa56f2b083 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -174,10 +174,10 @@ static __always_inline __pure bool _static_cpu_has(u16
bit)
[bitnum] "i" (1 << (bit & 7)),
[cap_word] "m" (((const char
*)boot_cpu_data.x86_capability)[bit >> 3])
: : t_yes, t_no);
- t_yes:
- return true;
t_no:
return false;
+ t_yes:
+ return true;
#else
return boot_cpu_has(bit);
#endif /* CC_HAVE_ASM_GOTO */
---
?
In any case, here's what happens with the current patchset:
vmlinux:
ffffffff8100472a: e9 50 0e de 00 jmpq ffffffff81de557f
<__alt_instructions_end+0x7aa>
ffffffff8100472f: 66 8c d0 mov %ss,%ax
ffffffff81004732: 66 83 f8 18 cmp $0x18,%ax
ffffffff81004736: 74 07 je ffffffff8100473f
<__switch_to+0x2ef>
ffffffff81004738: b8 18 00 00 00 mov $0x18,%eax
ffffffff8100473d: 8e d0 mov %eax,%ss
ffffffff8100473f: 48 83 c4 18 add $0x18,%rsp
ffffffff81004743: 4c 89 e0 mov %r12,%rax
ffffffff81004746: 5b pop %rbx
ffffffff81004747: 41 5c pop %r12
ffffffff81004749: 41 5d pop %r13
ffffffff8100474b: 41 5e pop %r14
ffffffff8100474d: 41 5f pop %r15
ffffffff8100474f: 5d pop %rbp
ffffffff81004750: c3 retq
That first JMP above sends us to the dynamic section which is in asm now:
ffffffff81de557f: f6 05 8f de d1 ff 01 testb $0x1,-0x2e2171(%rip)
# ffffffff81b03415 <boot_cpu_data+0x55>
ffffffff81de5586: 0f 85 a3 f1 21 ff jne ffffffff8100472f
<__switch_to+0x2df>
ffffffff81de558c: e9 ae f1 21 ff jmpq ffffffff8100473f
<__switch_to+0x2ef>
After X86_FEATURE_ALWAYS patching, that first JMP has become a 2-byte JMP:
[ 0.306333] apply_alternatives: feat: 3*32+21, old: (ffffffff8100472a, len:
5), repl: (ffffffff81de4e12, len: 5), pad: 0
[ 0.308005] ffffffff8100472a: old_insn: e9 50 0e de 00
[ 0.312012] ffffffff81de4e12: rpl_insn: e9 28 f9 21 ff
[ 0.318201] recompute_jump: target RIP: ffffffff8100473f, new_displ: 0x15
[ 0.320007] recompute_jump: final displ: 0x00000013, JMP 0xffffffff8100473f
[ 0.324005] ffffffff8100472a: final_insn: eb 13 0f 1f 00
so basically we jump over the %ss fixup:
ffffffff8100472a: eb 13 0f 1f 00 jmp ffffffff8100473f
ffffffff8100472f: 66 8c d0 mov %ss,%ax
ffffffff81004732: 66 83 f8 18 cmp $0x18,%ax
ffffffff81004736: 74 07 je ffffffff8100473f
<__switch_to+0x2ef>
ffffffff81004738: b8 18 00 00 00 mov $0x18,%eax
ffffffff8100473d: 8e d0 mov %eax,%ss
ffffffff8100473f: 48 83 c4 18 add $0x18,%rsp
<----
ffffffff81004743: 4c 89 e0 mov %r12,%rax
ffffffff81004746: 5b pop %rbx
ffffffff81004747: 41 5c pop %r12
ffffffff81004749: 41 5d pop %r13
ffffffff8100474b: 41 5e pop %r14
ffffffff8100474d: 41 5f pop %r15
ffffffff8100474f: 5d pop %rbp
ffffffff81004750: c3 retq
After X86_BUG_SYSRET_SS_ATTRS patching:
[ 0.330367] apply_alternatives: feat: 16*32+8, old: (ffffffff8100472a, len:
5), repl: (ffffffff81de3996, len: 0), pad: 0
[ 0.332005] ffffffff8100472a: old_insn: eb 13 0f 1f 00
[ 0.338332] ffffffff8100472a: final_insn: 0f 1f 44 00 00
ffffffff8100472a: 0f 1f 44 00 00 nop
ffffffff8100472f: 66 8c d0 mov %ss,%ax
ffffffff81004732: 66 83 f8 18 cmp $0x18,%ax
ffffffff81004736: 74 07 je ffffffff8100473f
<__switch_to+0x2ef>
ffffffff81004738: b8 18 00 00 00 mov $0x18,%eax
ffffffff8100473d: 8e d0 mov %eax,%ss
ffffffff8100473f: 48 83 c4 18 add $0x18,%rsp
ffffffff81004743: 4c 89 e0 mov %r12,%rax
ffffffff81004746: 5b pop %rbx
ffffffff81004747: 41 5c pop %r12
ffffffff81004749: 41 5d pop %r13
ffffffff8100474b: 41 5e pop %r14
ffffffff8100474d: 41 5f pop %r15
ffffffff8100474f: 5d pop %rbp
ffffffff81004750: c3 retq
So the penalty for the !X86_BUG_SYSRET_SS_ATTRS CPUs is a 2-byte JMP. Do
we care?
In the case we do, we could do this:
JMP ss_fixup
ret:
RET return prev_p;
ss_fixup:
<fixup SS>
jmp ret
and the !X86_BUG_SYSRET_SS_ATTRS CPUs would overwrite that
"JMP ss_fixup" with a NOP and they're fine. However, the
X86_BUG_SYSRET_SS_ATTRS CPUs will have to do two jumps, one to the fixup
code and one back to RET.
Now, how about I convert
unsigned short ss_sel;
savesegment(ss, ss_sel);
if (ss_sel != __KERNEL_DS)
loadsegment(ss, __KERNEL_DS);
into asm and into an alternative()?
Then, the !X86_BUG_SYSRET_SS_ATTRS CPUs will trade off that JMP with a
bunch of NOPs which will pollute I$.
Hmmm.
--
Regards/Gruss,
Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284
(AG Nürnberg)
--
