On Nov 20, 2016 3:19 AM, "Borislav Petkov" <[email protected]> wrote: > > On Sat, Nov 19, 2016 at 03:37:30PM -0800, Andy Lutomirski wrote: > > Linux will have all kinds of sporadic problems on systems that don't > > have the CPUID instruction unless CONFIG_M486=y. In particular, > > sync_core() will explode. > > Btw, I think we should do something like this, in addition: > > --- > diff --git a/arch/x86/include/asm/cpufeatures.h > b/arch/x86/include/asm/cpufeatures.h > index 50425dd7e113..ee9de769941f 100644 > --- a/arch/x86/include/asm/cpufeatures.h > +++ b/arch/x86/include/asm/cpufeatures.h > @@ -105,6 +105,7 @@ > #define X86_FEATURE_AMD_DCM ( 3*32+27) /* multi-node processor */ > #define X86_FEATURE_APERFMPERF ( 3*32+28) /* APERFMPERF */ > #define X86_FEATURE_NONSTOP_TSC_S3 ( 3*32+30) /* TSC doesn't stop in S3 > state */ > +#define X86_FEATURE_CPUID ( 3*32+31) /* CPU has CPUID instruction > itself */ > > /* Intel-defined CPU features, CPUID level 0x00000001 (ecx), word 4 */ > #define X86_FEATURE_XMM3 ( 4*32+ 0) /* "pni" SSE-3 */ > diff --git a/arch/x86/include/asm/processor.h > b/arch/x86/include/asm/processor.h > index 1f6a92903b09..63aa4842c0ae 100644 > --- a/arch/x86/include/asm/processor.h > +++ b/arch/x86/include/asm/processor.h > @@ -602,34 +602,21 @@ static __always_inline void cpu_relax(void) > rep_nop(); > } > > -/* Stop speculative execution and prefetching of modified code. */ > +/* > + * Stop speculative execution and prefetching of modified code. CPUID is a > + * barrier to speculative execution. Prefetched instructions are > automatically > + * invalidated when modified. > + */ > static inline void sync_core(void) > { > int tmp; > > -#ifdef CONFIG_M486 > - /* > - * Do a CPUID if available, otherwise do a jump. The jump > - * can conveniently enough be the jump around CPUID. > - */ > - asm volatile("cmpl %2,%1\n\t" > - "jl 1f\n\t" > - "cpuid\n" > - "1:" > - : "=a" (tmp) > - : "rm" (boot_cpu_data.cpuid_level), "ri" (0), "0" (1) > - : "ebx", "ecx", "edx", "memory"); > -#else > - /* > - * CPUID is a barrier to speculative execution. > - * Prefetched instructions are automatically > - * invalidated when modified. > - */ > - asm volatile("cpuid" > - : "=a" (tmp) > - : "0" (1) > - : "ebx", "ecx", "edx", "memory"); > -#endif > + /* Do a CPUID if available, otherwise do a forward jump. */ > + alternative_io("jmp 1f\n\t1:", "cpuid", > + X86_FEATURE_CPUID, > + "=a" (tmp), > + "0" (1) > + : "ebx", "ecx", "edx", "memory"); > }
This makes me nervous: don't some CPUs actually need the cpuid instruction when patching alternatives? And with this approach, we won't have the cpuid instruction there until after patching. Why not change this function entirely: write_cr2(0); CR2 should be available on all 32-bit CPUs. It clobbers fewer registers. More usefully, CPUID causes an exit when running under most hypervisors, and that's quite slow. The only case I can think of where CPUID should be faster than MOV to CR2 is on Xen PV before Ivy Bridge, and I'm not sure I care about performance there. (On Xen PV, it will do a hypercall instead, but the hypercall should be good enough to serialize, too.) Or we could do it dynamically: bt $X86_FEATURE_CPUID, CPU_FLAGS(boot_cpu_data) # or whatever -- I think we need to add an asm offset jnc 1f # here's our jump cpuid 1: It's not like CPUID is fast enough that avoiding a predictable branch will help measurably. --Andy
