linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "H. Peter Anvin" <hpa@zytor.com>
To: Borislav Petkov <bp@suse.de>
Cc: 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>,
	Andy Lutomirski <luto@amacapital.net>,
	Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: [PATCH] x86: static_cpu_has_safe: discard dynamic check after init
Date: Tue, 19 Jan 2016 20:02:01 -0800	[thread overview]
Message-ID: <569F06B9.4060903@zytor.com> (raw)
In-Reply-To: <20160119092213.GA15071@pd.tnic>

On 01/19/16 01:22, Borislav Petkov wrote:
> On Mon, Jan 18, 2016 at 05:33:03PM -0800, H. Peter Anvin wrote:
>> Why the f do we call a subroutine for what amounts to a single bt or
>> test instruction?
> 
> No real reason. You can kick me when you see me next time:
> 
> 4a90a99c4f80 ("x86: Add a static_cpu_has_safe variant")
> 

So, here is my suggestion:

1. Just get rid of static_cpu_has_safe() and make static_cpu_has() safe.

2. Get rid of the non-asm goto variant and just fall back to dynamic if
asm goto is unavailable.  It doesn't make any sense, really, if it is
supposed to be safe, and by now the asm goto-capable gcc is in more wide
use.  (Originally the gcc 3.x fallback to pure dynamic didn't exist,
either.)

3. Put the dynamic test in the .init.text section and inline it:

	.section .init.text,"ax"
	testb %2,%3
	jnz %[t_yes]
	jmp %[t_no]
	.previous

	... "i" (1 << (bit & 7)),
	    "m" (((const char *)boot_cpu_data->x86_capability)[bit >> 3]) ...

(The code would be slightly simpler/cleaner with testl, but that would
unnecessarily create a long immediate, or with btl, but that would be
slower.  We could use CONST_MASK_ADDR() and CONST_MASK() from
asm/bitops.h, but I'm slightly uncomfortable with the idea of leveraging
an interface which is ultimately an internal implementation detail of
bitops.h that might change in the future without people realizing its
implications.)

The only thing we lose is the 2-byte optimization in case we have the
good luck for it to actually work.  At this point I'm thinking it isn't
worth it, and instead that safety trumps it.  It *would* be interesting
to see if there are any call sites where it would actually kick in.

	-hpa

  reply	other threads:[~2016-01-20  4:02 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
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 [this message]
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
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=569F06B9.4060903@zytor.com \
    --to=hpa@zytor.com \
    --cc=bp@suse.de \
    --cc=brgerst@gmail.com \
    --cc=dvlasenk@redhat.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).