Patchwork [1/7] x86/feature: Detect the x86 feature to control Speculation

login
register
mail settings
Submitter tim
Date Jan. 4, 2018, 5:56 p.m.
Message ID <427aa76dea14532dea7e49f0bce4e7cf1dea7c6f.1515086770.git.tim.c.chen@linux.intel.com>
Download mbox | patch
Permalink /patch/10145335/
State New
Headers show

Comments

tim - Jan. 4, 2018, 5:56 p.m.
cpuid ax=0x7, return rdx bit 26 to indicate presence of this feature
IA32_SPEC_CTRL (0x48) and IA32_PRED_CMD (0x49)
IA32_SPEC_CTRL, bit0 – Indirect Branch Restricted Speculation (IBRS)
IA32_PRED_CMD,  bit0 – Indirect Branch Prediction Barrier (IBPB)

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.

Setting of IBPB ensures that earlier code's behavior does not control later
indirect branch predictions.  It is used when context switching to new
untrusted address space.  Unlike IBRS, it is a command MSR and does not retain
its state.

* 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

* IBRS is not guaranteed to differentiate two applications that use
the same CR3 due to recycling. Software can use an IBPB command when
recycling a page table base address.

* VMM software can similarly use an IBPB when recycling a controlling
VMCS pointer address

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   | 7 +++++++
 arch/x86/kernel/cpu/scattered.c    | 1 +
 3 files changed, 9 insertions(+)
gregkh@linuxfoundation.org - Jan. 4, 2018, 7:58 p.m.
On Thu, Jan 04, 2018 at 09:56:42AM -0800, Tim Chen wrote:
> cpuid ax=0x7, return rdx bit 26 to indicate presence of this feature
> IA32_SPEC_CTRL (0x48) and IA32_PRED_CMD (0x49)
> IA32_SPEC_CTRL, bit0 – Indirect Branch Restricted Speculation (IBRS)
> IA32_PRED_CMD,  bit0 – Indirect Branch Prediction Barrier (IBPB)
> 
> 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.
> 
> Setting of IBPB ensures that earlier code's behavior does not control later
> indirect branch predictions.  It is used when context switching to new
> untrusted address space.  Unlike IBRS, it is a command MSR and does not retain
> its state.
> 
> * 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
> 
> * IBRS is not guaranteed to differentiate two applications that use
> the same CR3 due to recycling. Software can use an IBPB command when
> recycling a page table base address.
> 
> * VMM software can similarly use an IBPB when recycling a controlling
> VMCS pointer address
> 
> 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   | 7 +++++++
>  arch/x86/kernel/cpu/scattered.c    | 1 +
>  3 files changed, 9 insertions(+)
> 
> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
> index 86c68cb..431f393 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 */

You should have gotten a build warning with just this patch, please also
update tools/arch/x86/include/asm/cpufeatures.h to fix that.

And why not use a free slot, (7*32+13) or (7*32+12) is free, right?

Or were you just trying to make backports "easier"?  :)

thanks,

greg k-h
tim - Jan. 4, 2018, 8:47 p.m.
On 01/04/2018 11:58 AM, Greg KH wrote:
> On Thu, Jan 04, 2018 at 09:56:42AM -0800, Tim Chen wrote:
>> cpuid ax=0x7, return rdx bit 26 to indicate presence of this feature
>> IA32_SPEC_CTRL (0x48) and IA32_PRED_CMD (0x49)
>> IA32_SPEC_CTRL, bit0 – Indirect Branch Restricted Speculation (IBRS)
>> IA32_PRED_CMD,  bit0 – Indirect Branch Prediction Barrier (IBPB)
>>
>> 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.
>>
>> Setting of IBPB ensures that earlier code's behavior does not control later
>> indirect branch predictions.  It is used when context switching to new
>> untrusted address space.  Unlike IBRS, it is a command MSR and does not retain
>> its state.
>>
>> * 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
>>
>> * IBRS is not guaranteed to differentiate two applications that use
>> the same CR3 due to recycling. Software can use an IBPB command when
>> recycling a page table base address.
>>
>> * VMM software can similarly use an IBPB when recycling a controlling
>> VMCS pointer address
>>
>> 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   | 7 +++++++
>>  arch/x86/kernel/cpu/scattered.c    | 1 +
>>  3 files changed, 9 insertions(+)
>>
>> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
>> index 86c68cb..431f393 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 */
> 
> You should have gotten a build warning with just this patch, please also
> update tools/arch/x86/include/asm/cpufeatures.h to fix that.

Sorry about that. Trying to get this patchset posted quickly
so I could have missed a few things.

> 
> And why not use a free slot, (7*32+13) or (7*32+12) is free, right?
> 
> Or were you just trying to make backports "easier"?  :)
> 
> 

There are future features related to speculation control.  So
putting it here so they can stay together.

Tim
David Woodhouse - Jan. 5, 2018, 11:14 a.m.
On Thu, 2018-01-04 at 09:56 -0800, Tim Chen wrote:
> cpuid ax=0x7, return rdx bit 26 to indicate presence of this feature
> IA32_SPEC_CTRL (0x48) and IA32_PRED_CMD (0x49)
> IA32_SPEC_CTRL, bit0 – Indirect Branch Restricted Speculation (IBRS)
> IA32_PRED_CMD,  bit0 – Indirect Branch Prediction Barrier (IBPB)

In a previous iteration of these patches, hadn't you already merged
support for the AMD variant? Where'd that go?
Thomas Gleixner - Jan. 5, 2018, 1:09 p.m.
On Thu, 4 Jan 2018, Tim Chen wrote:
> +#define MSR_IA32_SPEC_CTRL		0x00000048
> +#define SPEC_CTRL_FEATURE_DISABLE_IBRS	(0 << 0)
> +#define SPEC_CTRL_FEATURE_ENABLE_IBRS	(1 << 0)
> +
> +#define MSR_IA32_PRED_CMD		0x00000049
> +
>  #define MSR_IA32_PERFCTR0		0x000000c1
>  #define MSR_IA32_PERFCTR1		0x000000c2
>  #define MSR_FSB_FREQ			0x000000cd
> @@ -439,6 +445,7 @@
>  #define FEATURE_CONTROL_VMXON_ENABLED_INSIDE_SMX	(1<<1)
>  #define FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX	(1<<2)
>  #define FEATURE_CONTROL_LMCE				(1<<20)
> +#define FEATURE_SET_IBPB				(1<<0)

So how is that bit related to the control bits above? This file is
structured in obvious ways ....

Thanks,

	tglx
Andrea Arcangeli - Jan. 5, 2018, 1:44 p.m.
On Fri, Jan 05, 2018 at 02:09:43PM +0100, Thomas Gleixner wrote:
> On Thu, 4 Jan 2018, Tim Chen wrote:
> > +#define MSR_IA32_SPEC_CTRL		0x00000048
> > +#define SPEC_CTRL_FEATURE_DISABLE_IBRS	(0 << 0)
> > +#define SPEC_CTRL_FEATURE_ENABLE_IBRS	(1 << 0)
> > +
> > +#define MSR_IA32_PRED_CMD		0x00000049
> > +
> >  #define MSR_IA32_PERFCTR0		0x000000c1
> >  #define MSR_IA32_PERFCTR1		0x000000c2
> >  #define MSR_FSB_FREQ			0x000000cd
> > @@ -439,6 +445,7 @@
> >  #define FEATURE_CONTROL_VMXON_ENABLED_INSIDE_SMX	(1<<1)
> >  #define FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX	(1<<2)
> >  #define FEATURE_CONTROL_LMCE				(1<<20)
> > +#define FEATURE_SET_IBPB				(1<<0)
> 
> So how is that bit related to the control bits above? This file is
> structured in obvious ways ....

It's not related, FEATURE_SET_IBPB value is specific and only
meaningful to MSR_IA32_PRED_CMD.

The only use that you can ever make of that is:

static inline void __spec_ctrl_ibpb(void)
{
	native_wrmsrl(MSR_IA32_PRED_CMD, FEATURE_SET_IBPB);
}

static inline void spec_ctrl_ibpb(void)
{
	if (static_cpu_has(X86_FEATURE_IBPB_SUPPORT)) {
*obsolete line deleted to be replaced with static key*
			__spec_ctrl_ibpb();
	}
}

You need to set X86_FEATURE_IBPB_SUPPORT by hand if
X86_FEATURE_SPEC_CTRL is present. On some CPU X86_FEATURE_IBPB_SUPPORT
will show up in cpuid, in others only X86_FEATURE_SPEC_CTRL shows up
but that always implies X86_FEATURE_IBPB_SUPPORT.

void spec_ctrl_init(struct cpuinfo_x86 *c)
{
	if (c->x86_vendor != X86_VENDOR_INTEL &&
	    c->x86_vendor != X86_VENDOR_AMD)
		return;

	if (c != &boot_cpu_data) {
		spec_ctrl_cpu_init();
		return;
	}
[..]
	if (cpu_has_spec_ctrl()) {
		setup_force_cpu_cap(X86_FEATURE_IBPB_SUPPORT);
[..]
}

static void identify_cpu(struct cpuinfo_x86 *c)
[..]
	/* Set up SMEP/SMAP */
	setup_smep(c);
	setup_smap(c);

+	spec_ctrl_init(c);
[..]

static inline int cpu_has_spec_ctrl(void)
{
	return static_cpu_has(X86_FEATURE_SPEC_CTRL);
}

Note: if you don't drop all late microcode as discussed yesterday, the
above has to do a if (this/boot_cpu_has()) return 1; rmb() ; return
0;. rmb in the return 0 if there's more than one branch the CPU can
speculate through. Not from where it's called in spec_ctrl_init, but
for all other places that checks cpu_has_spec_ctrl().

Now about the late microcode my preference is not for static_cpu_has
and forcing the early microcode, but my long term preference is to
start with this/boot_cpu_has() and then turn static_cpu_has in a true
static key so that setup_force_cpu_cap shall also flip the static key
for all static_cpu_has(X86_FEATURE_IBPB_SUPPORT) also if run any time
after boot and not only if run before the static_cpu_has alternative
is patched in.
Thomas Gleixner - Jan. 5, 2018, 1:51 p.m.
On Fri, 5 Jan 2018, Andrea Arcangeli wrote:
> On Fri, Jan 05, 2018 at 02:09:43PM +0100, Thomas Gleixner wrote:
> Now about the late microcode my preference is not for static_cpu_has
> and forcing the early microcode, but my long term preference is to
> start with this/boot_cpu_has() and then turn static_cpu_has in a true
> static key so that setup_force_cpu_cap shall also flip the static key
> for all static_cpu_has(X86_FEATURE_IBPB_SUPPORT) also if run any time
> after boot and not only if run before the static_cpu_has alternative
> is patched in.

Fair enough. We can avoid the static_cpu_has() to begin with and fix it
later.

Thanks,

	tglx
Tom Lendacky - Jan. 5, 2018, 3:14 p.m.
On 1/5/2018 5:14 AM, David Woodhouse wrote:
> On Thu, 2018-01-04 at 09:56 -0800, Tim Chen wrote:
>> cpuid ax=0x7, return rdx bit 26 to indicate presence of this feature
>> IA32_SPEC_CTRL (0x48) and IA32_PRED_CMD (0x49)
>> IA32_SPEC_CTRL, bit0 – Indirect Branch Restricted Speculation (IBRS)
>> IA32_PRED_CMD,  bit0 – Indirect Branch Prediction Barrier (IBPB)
> 
> In a previous iteration of these patches, hadn't you already merged
> support for the AMD variant? Where'd that go?

It looks like this series is strictly IBRS related.  Even though some of
the IBPB definitions are in here, there isn't support later for indicating
IBPB is in use or any places where IBPB would be used. I was assuming that
there would be another series for IBPB support.  Is that the plan?

AMD is covered by the IBRS support as is, but we also have support for
the IBPB feature alone, identified by a different CPUID bit.  I was
waiting for the IBPB series to see if I needed to submit anything or
whether the patches were included.

Thanks,
Tom

>
tim - Jan. 5, 2018, 5:07 p.m.
On 01/05/2018 07:14 AM, Tom Lendacky wrote:
> On 1/5/2018 5:14 AM, David Woodhouse wrote:
>> On Thu, 2018-01-04 at 09:56 -0800, Tim Chen wrote:
>>> cpuid ax=0x7, return rdx bit 26 to indicate presence of this feature
>>> IA32_SPEC_CTRL (0x48) and IA32_PRED_CMD (0x49)
>>> IA32_SPEC_CTRL, bit0 – Indirect Branch Restricted Speculation (IBRS)
>>> IA32_PRED_CMD,  bit0 – Indirect Branch Prediction Barrier (IBPB)
>>
>> In a previous iteration of these patches, hadn't you already merged
>> support for the AMD variant? Where'd that go?
> 
> It looks like this series is strictly IBRS related.  Even though some of
> the IBPB definitions are in here, there isn't support later for indicating
> IBPB is in use or any places where IBPB would be used. I was assuming that
> there would be another series for IBPB support.  Is that the plan?

IBPB is meant to be a separate series.  We want to get retpoline
and IBRS out first.

> 
> AMD is covered by the IBRS support as is, but we also have support for
> the IBPB feature alone, identified by a different CPUID bit.  I was
> waiting for the IBPB series to see if I needed to submit anything or
> whether the patches were included.
> 

Tim

Patch

diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 86c68cb..431f393 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 1e7d710..f51e516 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -42,6 +42,12 @@ 
 #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)
+
+#define MSR_IA32_PRED_CMD		0x00000049
+
 #define MSR_IA32_PERFCTR0		0x000000c1
 #define MSR_IA32_PERFCTR1		0x000000c2
 #define MSR_FSB_FREQ			0x000000cd
@@ -439,6 +445,7 @@ 
 #define FEATURE_CONTROL_VMXON_ENABLED_INSIDE_SMX	(1<<1)
 #define FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX	(1<<2)
 #define FEATURE_CONTROL_LMCE				(1<<20)
+#define FEATURE_SET_IBPB				(1<<0)
 
 #define MSR_IA32_APICBASE		0x0000001b
 #define MSR_IA32_APICBASE_BSP		(1<<8)
diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
index 05459ad..bc50c40 100644
--- a/arch/x86/kernel/cpu/scattered.c
+++ b/arch/x86/kernel/cpu/scattered.c
@@ -24,6 +24,7 @@  static const struct cpuid_bit cpuid_bits[] = {
 	{ X86_FEATURE_INTEL_PT,		CPUID_EBX, 25, 0x00000007, 0 },
 	{ X86_FEATURE_AVX512_4VNNIW,    CPUID_EDX,  2, 0x00000007, 0 },
 	{ X86_FEATURE_AVX512_4FMAPS,    CPUID_EDX,  3, 0x00000007, 0 },
+	{ X86_FEATURE_SPEC_CTRL,	CPUID_EDX, 26, 0x00000007, 0 },
 	{ X86_FEATURE_CAT_L3,		CPUID_EBX,  1, 0x00000010, 0 },
 	{ X86_FEATURE_CAT_L2,		CPUID_EBX,  2, 0x00000010, 0 },
 	{ X86_FEATURE_CDP_L3,		CPUID_ECX,  2, 0x00000010, 1 },