Re: [PATCH] x86: static_cpu_has_safe: discard dynamic check after init

[H. Peter Anvin]

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