Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 08F3E2176D
MIME-Version: 1.0
In-Reply-To: <a00d764b835898844df2b01ce506ab03e786a744.1515204614.git.tim.c.chen@linux.intel.com>
References: <cover.1515204614.git.tim.c.chen@linux.intel.com> <a00d764b835898844df2b01ce506ab03e786a744.1515204614.git.tim.c.chen@linux.intel.com>
From:   Andy Lutomirski <luto@kernel.org>
Date:   Thu, 18 Jan 2018 15:28:29 -0800
Message-ID: <CALCETrWS41HtEC7U1JOVCNgVOQy25Bi2QtrwQqEnMLfVgeKkxg@mail.gmail.com>
Subject: Re: [PATCH v2 1/8] x86/feature: Detect the x86 IBRS feature to
 control Speculation
To:     Tim Chen <tim.c.chen@linux.intel.com>
Cc:     Thomas Gleixner <tglx@linutronix.de>,
        Andy Lutomirski <luto@kernel.org>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Greg KH <gregkh@linuxfoundation.org>,
        Dave Hansen <dave.hansen@intel.com>,
        Andrea Arcangeli <aarcange@redhat.com>,
        Andi Kleen <ak@linux.intel.com>,
        Arjan Van De Ven <arjan.van.de.ven@intel.com>,
        David Woodhouse <dwmw@amazon.co.uk>,
        LKML <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8BIT
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, Jan 5, 2018 at 6:12 PM, Tim Chen <tim.c.chen@linux.intel.com> wrote:
> cpuid ax=0x7, return rdx bit 26 to indicate presence of this feature
> IA32_SPEC_CTRL (0x48)
> IA32_SPEC_CTRL, bit0 – Indirect Branch Restricted Speculation (IBRS)
>
> 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
> 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
>
> * 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
>
> * 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 <tim.c.chen@linux.intel.com>
> ---
>  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)
> +

I still can't find the PDF that documents this right now, but, from
memory, shouldn't the value that we write to the MSR on entry be 3,
not 1?  That is, isn't bit 1 STIBP and don't we want STIBP to be on?
And IIRC bit 1 was ignored on non-STIBP-capable machines.