Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754213AbeAGRO7 (ORCPT + 1 other); Sun, 7 Jan 2018 12:14:59 -0500 Received: from mga01.intel.com ([192.55.52.88]:49397 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754055AbeAGRO6 (ORCPT ); Sun, 7 Jan 2018 12:14:58 -0500 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.46,326,1511856000"; d="scan'208";a="18193344" Subject: Re: [PATCH v2 1/8] x86/feature: Detect the x86 IBRS feature to control Speculation To: Borislav Petkov Cc: Thomas Gleixner , Andy Lutomirski , Linus Torvalds , Greg KH , Dave Hansen , Andrea Arcangeli , Andi Kleen , Arjan Van De Ven , David Woodhouse , linux-kernel@vger.kernel.org References: <20180106125633.wu3j4xz7f6w67b3f@pd.tnic> From: Tim Chen Message-ID: <700ccc48-d33a-abc0-e5f3-e16f8fba554a@linux.intel.com> Date: Sun, 7 Jan 2018 09:14:57 -0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <20180106125633.wu3j4xz7f6w67b3f@pd.tnic> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Return-Path: On 01/06/2018 04:56 AM, Borislav Petkov wrote: > On Fri, Jan 05, 2018 at 06:12:16PM -0800, Tim Chen wrote: > > <--- This needs an introductory sentence here. > >> cpuid ax=0x7, return rdx bit 26 to indicate presence of this feature > > You can write that as CPUID(7).RDX[26]. > >> IA32_SPEC_CTRL (0x48) >> IA32_SPEC_CTRL, bit0 – Indirect Branch Restricted Speculation (IBRS) > > Ah, those are MSRs. Please say so. > >> If IBRS is set, near returns and near indirect jumps/calls will not allow >> their predicted target address to be controlled by code that executed in >> a less privileged prediction mode before the IBRS mode was last written >> with a value of 1 or on another logical processor so long as all RSB > > "RSB" is? RSB is return stack buffer, basically speculation addresses for return statement. > >> entries from the previous less privileged prediction mode are overwritten. >> >> * Thus a near indirect jump/call/return may be affected by code in a >> less privileged prediction mode that executed AFTER IBRS mode was last >> written with a value of 1 > > End sentences with a full stop. > >> * There is no need to clear IBRS before writing it with a value of >> 1. Unconditionally writing it with a value of 1 after the prediction >> mode change is sufficient > > This sounds strange. I know of funky MSRs like that but if it is not > the case here, no need to mention it then. > >> * Note: IBRS is not required in order to isolate branch predictions for >> SMM or SGX enclaves >> >> * Code executed by a sibling logical processor cannot control indirect >> jump/call/return predicted target when IBRS is set >> >> * SMEP will prevent supervisor mode using RSB entries filled by user code; >> this can reduce the need for software to overwrite RSB entries >> >> CPU performance could be reduced when running with IBRS set. >> >> Signed-off-by: Tim Chen >> --- >> arch/x86/include/asm/cpufeatures.h | 1 + >> arch/x86/include/asm/msr-index.h | 4 ++++ >> arch/x86/kernel/cpu/scattered.c | 1 + >> tools/arch/x86/include/asm/cpufeatures.h | 1 + >> 4 files changed, 7 insertions(+) >> >> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h >> index 07cdd17..5ee0737 100644 >> --- a/arch/x86/include/asm/cpufeatures.h >> +++ b/arch/x86/include/asm/cpufeatures.h >> @@ -209,6 +209,7 @@ >> #define X86_FEATURE_AVX512_4FMAPS ( 7*32+17) /* AVX-512 Multiply Accumulation Single precision */ >> >> #define X86_FEATURE_MBA ( 7*32+18) /* Memory Bandwidth Allocation */ >> +#define X86_FEATURE_SPEC_CTRL ( 7*32+19) /* Control Speculation Control */ >> >> /* Virtualization flags: Linux defined, word 8 */ >> #define X86_FEATURE_TPR_SHADOW ( 8*32+ 0) /* Intel TPR Shadow */ >> diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h >> index 34c4922..f881add 100644 >> --- a/arch/x86/include/asm/msr-index.h >> +++ b/arch/x86/include/asm/msr-index.h >> @@ -42,6 +42,10 @@ >> #define MSR_PPIN_CTL 0x0000004e >> #define MSR_PPIN 0x0000004f >> >> +#define MSR_IA32_SPEC_CTRL 0x00000048 >> +#define SPEC_CTRL_FEATURE_DISABLE_IBRS (0 << 0) >> +#define SPEC_CTRL_FEATURE_ENABLE_IBRS (1 << 0) > > s/_FEATURE// > > SPEC_CTRL_{ENABLE,DISABLE}_IBRS is good enough. > >> + >> #define MSR_IA32_PERFCTR0 0x000000c1 >> #define MSR_IA32_PERFCTR1 0x000000c2 >> #define MSR_FSB_FREQ 0x000000cd > ... > Thanks. will update the phrasing. Tim