From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from userp2120.oracle.com ([156.151.31.85]) by Galois.linutronix.de with esmtps (TLS1.2:RSA_AES_256_CBC_SHA256:256) (Exim 4.80) (envelope-from ) id 1fImVb-00039K-54 for speck@linutronix.de; Wed, 16 May 2018 04:49:44 +0200 Received: from pps.filterd (userp2120.oracle.com [127.0.0.1]) by userp2120.oracle.com (8.16.0.22/8.16.0.22) with SMTP id w4G2kgCC141996 for ; Wed, 16 May 2018 02:49:36 GMT Received: from userv0022.oracle.com (userv0022.oracle.com [156.151.31.74]) by userp2120.oracle.com with ESMTP id 2hx29w2swj-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for ; Wed, 16 May 2018 02:49:36 +0000 Received: from aserv0122.oracle.com (aserv0122.oracle.com [141.146.126.236]) by userv0022.oracle.com (8.14.4/8.14.4) with ESMTP id w4G2nZHK030553 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for ; Wed, 16 May 2018 02:49:35 GMT Received: from abhmp0012.oracle.com (abhmp0012.oracle.com [141.146.116.18]) by aserv0122.oracle.com (8.14.4/8.14.4) with ESMTP id w4G2nZs5015725 for ; Wed, 16 May 2018 02:49:35 GMT Date: Tue, 15 May 2018 22:49:30 -0400 From: Konrad Rzeszutek Wilk Subject: [MODERATED] Re: [patch 03/15] Hidden 3 Message-ID: <20180516024930.GD18660@char.us.oracle.com> References: <20180513140048.543641807@linutronix.de> <20180513140538.470697861@linutronix.de> MIME-Version: 1.0 In-Reply-To: <20180513140538.470697861@linutronix.de> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit To: speck@linutronix.de List-ID: On Sun, May 13, 2018 at 04:00:51PM +0200, speck for Thomas Gleixner wrote: > Subject: [patch 03/15] x86/cpufeatures: Disentangle MSR_SPEC_CTRL enumeration from IBRS > From: Thomas Gleixner > > The availability of the SPEC_CTRL MSR is enumerated by a CPUID bit on > Intel and implied by IBRS or STIBP support on AMD. That's just confusing > and in case an AMD CPU has IBRS not supported because the underlying > problem has been fixed but has another bit valid in the SPEC_CTRL MSR, > the thing falls apart. > > Add a synthetic feature bit X86_FEATURE_MSR_SPEC_CTRL to denote the > availability on both Intel and AMD. > > Signed-off-by: Thomas Gleixner > --- > arch/x86/include/asm/cpufeatures.h | 1 + > arch/x86/kernel/cpu/bugs.c | 18 +++++++++++------- > arch/x86/kernel/cpu/common.c | 9 +++++++-- > arch/x86/kernel/cpu/intel.c | 1 + > 4 files changed, 20 insertions(+), 9 deletions(-) > > --- a/arch/x86/include/asm/cpufeatures.h > +++ b/arch/x86/include/asm/cpufeatures.h > @@ -206,6 +206,7 @@ > #define X86_FEATURE_RETPOLINE_AMD ( 7*32+13) /* "" AMD Retpoline mitigation for Spectre variant 2 */ > #define X86_FEATURE_INTEL_PPIN ( 7*32+14) /* Intel Processor Inventory Number */ > #define X86_FEATURE_CDP_L2 ( 7*32+15) /* Code and Data Prioritization L2 */ > +#define X86_FEATURE_MSR_SPEC_CTRL ( 7*32+16) /* "" MSR SPEC_CTRL is implemented */ > > #define X86_FEATURE_MBA ( 7*32+18) /* Memory Bandwidth Allocation */ > #define X86_FEATURE_RSB_CTXSW ( 7*32+19) /* "" Fill RSB on context switches */ > --- a/arch/x86/kernel/cpu/bugs.c > +++ b/arch/x86/kernel/cpu/bugs.c > @@ -64,7 +64,7 @@ void __init check_bugs(void) > * have unknown values. AMD64_LS_CFG MSR is cached in the early AMD > * init code as it is not enumerated and depends on the family. > */ > - if (boot_cpu_has(X86_FEATURE_IBRS)) > + if (boot_cpu_has(X86_FEATURE_MSR_SPEC_CTRL)) > rdmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base); > > /* Select the proper spectre mitigation before patching alternatives */ > @@ -145,7 +145,7 @@ u64 x86_spec_ctrl_get_default(void) > { > u64 msrval = x86_spec_ctrl_base; > > - if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL) > + if (static_cpu_has(X86_FEATURE_SPEC_CTRL)) You are using the static_cpu_has which ends up doing alternative patching - neat. But what if the machine at bootup has no SPEC_CTRL MSR support at all, and then we end up loading a new microcode with this and then suddenly SPEC_CTRL MSR is available? Wouldn't the static_cpu_has in this scenario end up always returning false? [And this may all be moot - perhaps we don't support this scenario?]