From: Borislav Petkov <bp@suse.de>
To: "H. Peter Anvin" <hpa@zytor.com>
Cc: Andy Lutomirski <luto@amacapital.net>,
Brian Gerst <brgerst@gmail.com>,
the arch/x86 maintainers <x86@kernel.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Ingo Molnar <mingo@kernel.org>,
Denys Vlasenko <dvlasenk@redhat.com>,
Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: [PATCH] x86: static_cpu_has_safe: discard dynamic check after init
Date: Thu, 21 Jan 2016 23:14:42 +0100 [thread overview]
Message-ID: <20160121221442.GB300@pd.tnic> (raw)
In-Reply-To: <108BC768-CF19-4F71-BF6D-70FF2252ADB8@zytor.com>
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)
--
next prev parent reply other threads:[~2016-01-21 22:15 UTC|newest]
Thread overview: 66+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-01-16 19:22 [PATCH] x86: static_cpu_has_safe: discard dynamic check after init Brian Gerst
2016-01-16 19:36 ` Borislav Petkov
2016-01-16 19:58 ` Brian Gerst
2016-01-17 10:33 ` Borislav Petkov
2016-01-18 16:52 ` Brian Gerst
2016-01-18 17:49 ` Andy Lutomirski
2016-01-18 18:14 ` Borislav Petkov
2016-01-18 18:29 ` Andy Lutomirski
2016-01-18 18:39 ` Borislav Petkov
2016-01-18 19:45 ` H. Peter Anvin
2016-01-18 23:05 ` Borislav Petkov
2016-01-18 23:13 ` H. Peter Anvin
2016-01-18 23:25 ` Borislav Petkov
2016-01-19 13:57 ` Borislav Petkov
2016-01-19 16:23 ` Borislav Petkov
2016-01-19 23:10 ` Borislav Petkov
2016-01-19 23:26 ` Andy Lutomirski
2016-01-19 23:49 ` Boris Petkov
2016-01-20 4:03 ` H. Peter Anvin
2016-01-20 10:33 ` Borislav Petkov
2016-01-20 10:41 ` H. Peter Anvin
2016-01-21 22:14 ` Borislav Petkov [this message]
2016-01-21 22:22 ` H. Peter Anvin
2016-01-21 22:56 ` Borislav Petkov
2016-01-21 23:36 ` H. Peter Anvin
2016-01-21 23:37 ` H. Peter Anvin
2016-01-22 10:32 ` Borislav Petkov
2016-01-18 18:51 ` Borislav Petkov
2016-01-19 1:10 ` Borislav Petkov
2016-01-19 1:33 ` H. Peter Anvin
2016-01-19 9:22 ` Borislav Petkov
2016-01-20 4:02 ` H. Peter Anvin
2016-01-20 4:39 ` Brian Gerst
2016-01-20 4:42 ` H. Peter Anvin
2016-01-20 10:50 ` Borislav Petkov
2016-01-20 10:55 ` H. Peter Anvin
2016-01-20 11:05 ` Borislav Petkov
2016-01-20 14:48 ` H. Peter Anvin
2016-01-20 15:01 ` Borislav Petkov
2016-01-20 15:09 ` H. Peter Anvin
2016-01-20 16:04 ` Borislav Petkov
2016-01-20 16:16 ` H. Peter Anvin
-- strict thread matches above, loose matches on Subject: below --
2016-01-23 6:50 [PATCH] x86/head_64.S: do not use temporary register to check alignment Alexander Kuleshov
2016-01-26 9:31 ` Borislav Petkov
2016-01-26 21:12 [PATCH 00/10] tip-queue 2016-01-26, rest Borislav Petkov
2016-01-26 21:12 ` [PATCH 01/10] x86/asm: Add condition codes clobber to memory barrier macros Borislav Petkov
2016-01-26 21:12 ` [PATCH 02/10] x86/asm: Drop a comment left over from X86_OOSTORE Borislav Petkov
2016-01-26 21:12 ` [PATCH 03/10] x86/asm: Tweak the comment about wmb() use for IO Borislav Petkov
2016-01-26 21:12 ` [PATCH 04/10] x86/cpufeature: Carve out X86_FEATURE_* Borislav Petkov
2016-01-30 13:18 ` [tip:x86/asm] " tip-bot for Borislav Petkov
2016-01-26 21:12 ` [PATCH 05/10] x86/cpufeature: Replace the old static_cpu_has() with safe variant Borislav Petkov
2016-01-30 13:19 ` [tip:x86/asm] " tip-bot for Borislav Petkov
2016-01-26 21:12 ` [PATCH 06/10] x86/cpufeature: Get rid of the non-asm goto variant Borislav Petkov
2016-01-27 3:36 ` Brian Gerst
2016-01-27 8:41 ` Borislav Petkov
2016-01-27 8:43 ` [PATCH -v1.1 " Borislav Petkov
2016-01-30 13:19 ` [tip:x86/asm] " tip-bot for Borislav Petkov
2016-01-27 8:45 ` [PATCH -v1.1 8/10] x86/alternatives: Discard dynamic check after init Borislav Petkov
2016-01-30 13:20 ` [tip:x86/asm] " tip-bot for Brian Gerst
2016-01-26 21:12 ` [PATCH 07/10] x86/alternatives: Add an auxilary section Borislav Petkov
2016-01-30 13:19 ` [tip:x86/asm] " tip-bot for Borislav Petkov
2016-01-26 21:12 ` [PATCH 08/10] x86/alternatives: Discard dynamic check after init Borislav Petkov
2016-01-26 21:12 ` [PATCH 09/10] x86/vdso: Use static_cpu_has() Borislav Petkov
2016-01-30 13:20 ` [tip:x86/asm] " tip-bot for Borislav Petkov
2016-01-26 21:12 ` [PATCH 10/10] x86/head_64: Simplify kernel load address alignment check Borislav Petkov
2016-01-30 13:20 ` [tip:x86/boot] x86/boot: " tip-bot for Alexander Kuleshov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160121221442.GB300@pd.tnic \
--to=bp@suse.de \
--cc=brgerst@gmail.com \
--cc=dvlasenk@redhat.com \
--cc=hpa@zytor.com \
--cc=linux-kernel@vger.kernel.org \
--cc=luto@amacapital.net \
--cc=mingo@kernel.org \
--cc=torvalds@linux-foundation.org \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.