From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753434AbcKTQWs (ORCPT ); Sun, 20 Nov 2016 11:22:48 -0500 Received: from mail-ua0-f172.google.com ([209.85.217.172]:32972 "EHLO mail-ua0-f172.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752857AbcKTQWq (ORCPT ); Sun, 20 Nov 2016 11:22:46 -0500 MIME-Version: 1.0 In-Reply-To: <20161120111917.pw3alolx4fksfwbv@pd.tnic> References: <70eac6639f23df8be5fe03fa1984aedd5d40077a.1479598603.git.luto@kernel.org> <20161120111917.pw3alolx4fksfwbv@pd.tnic> From: Andy Lutomirski Date: Sun, 20 Nov 2016 08:22:25 -0800 Message-ID: Subject: Re: [PATCH] x86/boot: Fail the boot if !M486 and CPUID is missing To: Borislav Petkov Cc: Matthew Whitehead , Brian Gerst , "linux-kernel@vger.kernel.org" , X86 ML Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Nov 20, 2016 3:19 AM, "Borislav Petkov" 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