2018-01-23 16:56:02

by Woodhouse, David

[permalink] [raw]
Subject: [PATCH v2 0/5] Basic Speculation Control feature support

Baby steps... this is just the basic CPUID and MSR definitions for AMD
and Intel, followedby the complete no-brainer: Disable KPTI on Intel
CPUs which set the RDCL_NO bit to say that they don't need it.

David Woodhouse (5):
x86/cpufeatures: Add CPUID_7_EDX CPUID leaf
x86/cpufeatures: Add Intel feature bits for Speculation Control
x86/cpufeatures: Add AMD feature bits for Speculation Control
x86/msr: Add definitions for new speculation control MSRs
x86/pti: Do not enable PTI on fixed Intel processors

arch/x86/include/asm/cpufeature.h | 7 +++++--
arch/x86/include/asm/cpufeatures.h | 14 +++++++++++---
arch/x86/include/asm/disabled-features.h | 3 ++-
arch/x86/include/asm/msr-index.h | 12 ++++++++++++
arch/x86/include/asm/required-features.h | 3 ++-
arch/x86/kernel/cpu/common.c | 11 +++++++++--
arch/x86/kernel/cpu/scattered.c | 2 --
7 files changed, 41 insertions(+), 11 deletions(-)

--
2.7.4



2018-01-23 16:54:47

by Woodhouse, David

[permalink] [raw]
Subject: [PATCH v2 5/5] x86/pti: Do not enable PTI on fixed Intel processors

When they advertise the IA32_ARCH_CAPABILITIES MSR and it has the RDCL_NO
bit set, they don't need KPTI either.

Signed-off-by: David Woodhouse <[email protected]>
---
arch/x86/kernel/cpu/common.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index e5d66e9..c05d0fe 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -900,8 +900,14 @@ static void __init early_identify_cpu(struct cpuinfo_x86 *c)

setup_force_cpu_cap(X86_FEATURE_ALWAYS);

- if (c->x86_vendor != X86_VENDOR_AMD)
- setup_force_cpu_bug(X86_BUG_CPU_MELTDOWN);
+ if (c->x86_vendor != X86_VENDOR_AMD) {
+ u64 ia32_cap = 0;
+
+ if (cpu_has(c, X86_FEATURE_ARCH_CAPABILITIES))
+ rdmsrl(MSR_IA32_ARCH_CAPABILITIES, ia32_cap);
+ if (!(ia32_cap & ARCH_CAP_RDCL_NO))
+ setup_force_cpu_bug(X86_BUG_CPU_MELTDOWN);
+ }

setup_force_cpu_bug(X86_BUG_SPECTRE_V1);
setup_force_cpu_bug(X86_BUG_SPECTRE_V2);
--
2.7.4


2018-01-23 16:54:49

by Woodhouse, David

[permalink] [raw]
Subject: [PATCH v2 1/5] x86/cpufeatures: Add CPUID_7_EDX CPUID leaf

This is a pure feature bits leaf. We have two AVX512 feature bits in it
already which were handled as scattered bits, and I'm about to add three
more from this leaf for speculation control features.

Signed-off-by: David Woodhouse <[email protected]>
Reviewed-by: Borislav Petkov <[email protected]>
---
arch/x86/include/asm/cpufeature.h | 7 +++++--
arch/x86/include/asm/cpufeatures.h | 8 +++++---
arch/x86/include/asm/disabled-features.h | 3 ++-
arch/x86/include/asm/required-features.h | 3 ++-
arch/x86/kernel/cpu/common.c | 1 +
arch/x86/kernel/cpu/scattered.c | 2 --
6 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index ea9a7dd..70eddb3 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -29,6 +29,7 @@ enum cpuid_leafs
CPUID_8000_000A_EDX,
CPUID_7_ECX,
CPUID_8000_0007_EBX,
+ CPUID_7_EDX,
};

#ifdef CONFIG_X86_FEATURE_NAMES
@@ -79,8 +80,9 @@ extern const char * const x86_bug_flags[NBUGINTS*32];
CHECK_BIT_IN_MASK_WORD(REQUIRED_MASK, 15, feature_bit) || \
CHECK_BIT_IN_MASK_WORD(REQUIRED_MASK, 16, feature_bit) || \
CHECK_BIT_IN_MASK_WORD(REQUIRED_MASK, 17, feature_bit) || \
+ CHECK_BIT_IN_MASK_WORD(REQUIRED_MASK, 18, feature_bit) || \
REQUIRED_MASK_CHECK || \
- BUILD_BUG_ON_ZERO(NCAPINTS != 18))
+ BUILD_BUG_ON_ZERO(NCAPINTS != 19))

#define DISABLED_MASK_BIT_SET(feature_bit) \
( CHECK_BIT_IN_MASK_WORD(DISABLED_MASK, 0, feature_bit) || \
@@ -101,8 +103,9 @@ extern const char * const x86_bug_flags[NBUGINTS*32];
CHECK_BIT_IN_MASK_WORD(DISABLED_MASK, 15, feature_bit) || \
CHECK_BIT_IN_MASK_WORD(DISABLED_MASK, 16, feature_bit) || \
CHECK_BIT_IN_MASK_WORD(DISABLED_MASK, 17, feature_bit) || \
+ CHECK_BIT_IN_MASK_WORD(DISABLED_MASK, 18, feature_bit) || \
DISABLED_MASK_CHECK || \
- BUILD_BUG_ON_ZERO(NCAPINTS != 18))
+ BUILD_BUG_ON_ZERO(NCAPINTS != 19))

#define cpu_has(c, bit) \
(__builtin_constant_p(bit) && REQUIRED_MASK_BIT_SET(bit) ? 1 : \
diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 25b9375..7b25cf3 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -13,7 +13,7 @@
/*
* Defines x86 CPU feature bits
*/
-#define NCAPINTS 18 /* N 32-bit words worth of info */
+#define NCAPINTS 19 /* N 32-bit words worth of info */
#define NBUGINTS 1 /* N 32-bit bug flags */

/*
@@ -206,8 +206,6 @@
#define X86_FEATURE_RETPOLINE ( 7*32+12) /* Generic Retpoline mitigation for Spectre variant 2 */
#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_AVX512_4VNNIW ( 7*32+16) /* AVX-512 Neural Network Instructions */
-#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_RSB_CTXSW ( 7*32+19) /* Fill RSB on context switches */
@@ -319,6 +317,10 @@
#define X86_FEATURE_SUCCOR (17*32+ 1) /* Uncorrectable error containment and recovery */
#define X86_FEATURE_SMCA (17*32+ 3) /* Scalable MCA */

+/* Intel-defined CPU features, CPUID level 0x00000007:0 (EDX), word 18 */
+#define X86_FEATURE_AVX512_4VNNIW (18*32+ 2) /* AVX-512 Neural Network Instructions */
+#define X86_FEATURE_AVX512_4FMAPS (18*32+ 3) /* AVX-512 Multiply Accumulation Single precision */
+
/*
* BUG word(s)
*/
diff --git a/arch/x86/include/asm/disabled-features.h b/arch/x86/include/asm/disabled-features.h
index e428e16..c6a3af1 100644
--- a/arch/x86/include/asm/disabled-features.h
+++ b/arch/x86/include/asm/disabled-features.h
@@ -71,6 +71,7 @@
#define DISABLED_MASK15 0
#define DISABLED_MASK16 (DISABLE_PKU|DISABLE_OSPKE|DISABLE_LA57)
#define DISABLED_MASK17 0
-#define DISABLED_MASK_CHECK BUILD_BUG_ON_ZERO(NCAPINTS != 18)
+#define DISABLED_MASK18 0
+#define DISABLED_MASK_CHECK BUILD_BUG_ON_ZERO(NCAPINTS != 19)

#endif /* _ASM_X86_DISABLED_FEATURES_H */
diff --git a/arch/x86/include/asm/required-features.h b/arch/x86/include/asm/required-features.h
index d91ba04..fb3a6de 100644
--- a/arch/x86/include/asm/required-features.h
+++ b/arch/x86/include/asm/required-features.h
@@ -106,6 +106,7 @@
#define REQUIRED_MASK15 0
#define REQUIRED_MASK16 (NEED_LA57)
#define REQUIRED_MASK17 0
-#define REQUIRED_MASK_CHECK BUILD_BUG_ON_ZERO(NCAPINTS != 18)
+#define REQUIRED_MASK18 0
+#define REQUIRED_MASK_CHECK BUILD_BUG_ON_ZERO(NCAPINTS != 19)

#endif /* _ASM_X86_REQUIRED_FEATURES_H */
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 372ba3f..e5d66e9 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -745,6 +745,7 @@ void get_cpu_cap(struct cpuinfo_x86 *c)
cpuid_count(0x00000007, 0, &eax, &ebx, &ecx, &edx);
c->x86_capability[CPUID_7_0_EBX] = ebx;
c->x86_capability[CPUID_7_ECX] = ecx;
+ c->x86_capability[CPUID_7_EDX] = edx;
}

/* Extended state features: level 0x0000000d */
diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
index d0e6976..df11f5d 100644
--- a/arch/x86/kernel/cpu/scattered.c
+++ b/arch/x86/kernel/cpu/scattered.c
@@ -21,8 +21,6 @@ struct cpuid_bit {
static const struct cpuid_bit cpuid_bits[] = {
{ X86_FEATURE_APERFMPERF, CPUID_ECX, 0, 0x00000006, 0 },
{ X86_FEATURE_EPB, CPUID_ECX, 3, 0x00000006, 0 },
- { X86_FEATURE_AVX512_4VNNIW, CPUID_EDX, 2, 0x00000007, 0 },
- { X86_FEATURE_AVX512_4FMAPS, CPUID_EDX, 3, 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 },
--
2.7.4


2018-01-23 16:54:57

by Woodhouse, David

[permalink] [raw]
Subject: [PATCH v2 3/5] x86/cpufeatures: Add AMD feature bits for Speculation Control

AMD exposes the PRED_CMD/SPEC_CTRL MSRs slightly differently to Intel.
Documented at https://lkml.org/lkml/2018/1/21/112

Signed-off-by: David Woodhouse <[email protected]>
---
arch/x86/include/asm/cpufeatures.h | 3 +++
1 file changed, 3 insertions(+)

diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 0a51070..ae3212f 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -269,6 +269,9 @@
#define X86_FEATURE_CLZERO (13*32+ 0) /* CLZERO instruction */
#define X86_FEATURE_IRPERF (13*32+ 1) /* Instructions Retired Count */
#define X86_FEATURE_XSAVEERPTR (13*32+ 2) /* Always save/restore FP error pointers */
+#define X86_FEATURE_AMD_PRED_CMD (13*32+12) /* Prediction Command MSR (AMD) */
+#define X86_FEATURE_AMD_SPEC_CTRL (13*32+14) /* Speculation Control MSR only (AMD) */
+#define X86_FEATURE_AMD_STIBP (13*32+15) /* Single Thread Indirect Branch Predictors (AMD) */

/* Thermal and Power Management Leaf, CPUID level 0x00000006 (EAX), word 14 */
#define X86_FEATURE_DTHERM (14*32+ 0) /* Digital Thermal Sensor */
--
2.7.4


2018-01-23 16:55:52

by Woodhouse, David

[permalink] [raw]
Subject: [PATCH v2 2/5] x86/cpufeatures: Add Intel feature bits for Speculation Control

Add three feature bits exposed by new microcode on Intel CPUs for
speculation control.

Signed-off-by: David Woodhouse <[email protected]>
Reviewed-by: Borislav Petkov <[email protected]>
---
arch/x86/include/asm/cpufeatures.h | 3 +++
1 file changed, 3 insertions(+)

diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 7b25cf3..0a51070 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -320,6 +320,9 @@
/* Intel-defined CPU features, CPUID level 0x00000007:0 (EDX), word 18 */
#define X86_FEATURE_AVX512_4VNNIW (18*32+ 2) /* AVX-512 Neural Network Instructions */
#define X86_FEATURE_AVX512_4FMAPS (18*32+ 3) /* AVX-512 Multiply Accumulation Single precision */
+#define X86_FEATURE_SPEC_CTRL (18*32+26) /* Speculation Control (IBRS + IBPB) */
+#define X86_FEATURE_STIBP (18*32+27) /* Single Thread Indirect Branch Predictors */
+#define X86_FEATURE_ARCH_CAPABILITIES (18*32+29) /* IA32_ARCH_CAPABILITIES MSR (Intel) */

/*
* BUG word(s)
--
2.7.4


2018-01-23 16:55:52

by Woodhouse, David

[permalink] [raw]
Subject: [PATCH v2 4/5] x86/msr: Add definitions for new speculation control MSRs

Add MSR and bit definitions for SPEC_CTRL, PRED_CMD and ARCH_CAPABILITIES.

See Intel's 336996-Speculative-Execution-Side-Channel-Mitigations.pdf

Signed-off-by: David Woodhouse <[email protected]>
---
arch/x86/include/asm/msr-index.h | 12 ++++++++++++
1 file changed, 12 insertions(+)

diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index fa11fb1..eb83ff1 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -39,6 +39,13 @@

/* Intel MSRs. Some also available on other CPUs */

+#define MSR_IA32_SPEC_CTRL 0x00000048 /* Speculation Control */
+#define SPEC_CTRL_IBRS (1 << 0) /* Indirect Branch Restricted Speculation */
+#define SPEC_CTRL_STIBP (1 << 1) /* Single Thread Indirect Branch Predictors */
+
+#define MSR_IA32_PRED_CMD 0x00000049 /* Prediction Command */
+#define PRED_CMD_IBPB (1 << 0) /* Indirect Branch Prediction Barrier */
+
#define MSR_PPIN_CTL 0x0000004e
#define MSR_PPIN 0x0000004f

@@ -57,6 +64,11 @@
#define SNB_C3_AUTO_UNDEMOTE (1UL << 28)

#define MSR_MTRRcap 0x000000fe
+
+#define MSR_IA32_ARCH_CAPABILITIES 0x0000010a
+#define ARCH_CAP_RDCL_NO (1 << 0) /* Not susceptible to Meltdown */
+#define ARCH_CAP_IBRS_ALL (1 << 1) /* Enhanced IBRS support */
+
#define MSR_IA32_BBL_CR_CTL 0x00000119
#define MSR_IA32_BBL_CR_CTL3 0x0000011e

--
2.7.4


2018-01-23 18:12:46

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH v2 5/5] x86/pti: Do not enable PTI on fixed Intel processors

>
> - if (c->x86_vendor != X86_VENDOR_AMD)
> - setup_force_cpu_bug(X86_BUG_CPU_MELTDOWN);
> + if (c->x86_vendor != X86_VENDOR_AMD) {
> + u64 ia32_cap = 0;
> +
> + if (cpu_has(c, X86_FEATURE_ARCH_CAPABILITIES))
> + rdmsrl(MSR_IA32_ARCH_CAPABILITIES, ia32_cap);
> + if (!(ia32_cap & ARCH_CAP_RDCL_NO))
> + setup_force_cpu_bug(X86_BUG_CPU_MELTDOWN);

This means that in a hypervisor which passes through the CPUID,
but actually doesn't implement the MSR (so rdmsr #GPs and returns 0)
it would be cleared.

It would be better to usr rdmsrl_safe and check the return value.

-Andi

2018-01-23 18:29:02

by Dave Hansen

[permalink] [raw]
Subject: Re: [PATCH v2 4/5] x86/msr: Add definitions for new speculation control MSRs

On 01/23/2018 08:52 AM, David Woodhouse wrote:
> +#define MSR_IA32_ARCH_CAPABILITIES 0x0000010a
> +#define ARCH_CAP_RDCL_NO (1 << 0) /* Not susceptible to Meltdown */
> +#define ARCH_CAP_IBRS_ALL (1 << 1) /* Enhanced IBRS support */

Do we want to spell out the silly Intel acronym? I don't know how we
fit it on the right side, but I do think we need to do it _somewhere_.
We need the code to stand on its own to some degree and not subject the
masses to reading the spec to understand the code.

/* Not to susceptible Rogue Data Cache Load aka Meltdown: */
#define ARCH_CAP_RDCL_NO (1 << 0)

2018-01-23 18:32:24

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH v2 4/5] x86/msr: Add definitions for new speculation control MSRs

On Tue, Jan 23, 2018 at 10:27:24AM -0800, Dave Hansen wrote:
> On 01/23/2018 08:52 AM, David Woodhouse wrote:
> > +#define MSR_IA32_ARCH_CAPABILITIES 0x0000010a
> > +#define ARCH_CAP_RDCL_NO (1 << 0) /* Not susceptible to Meltdown */
> > +#define ARCH_CAP_IBRS_ALL (1 << 1) /* Enhanced IBRS support */
>
> Do we want to spell out the silly Intel acronym? I don't know how we
> fit it on the right side, but I do think we need to do it _somewhere_.
> We need the code to stand on its own to some degree and not subject the
> masses to reading the spec to understand the code.

Yes please, take pity on us :)

thanks,

greg k-h

2018-01-23 18:41:25

by Dave Hansen

[permalink] [raw]
Subject: Re: [PATCH v2 5/5] x86/pti: Do not enable PTI on fixed Intel processors

On 01/23/2018 08:52 AM, David Woodhouse wrote:
> When they advertise the IA32_ARCH_CAPABILITIES MSR and it has the RDCL_NO
> bit set, they don't need KPTI either.
>
> Signed-off-by: David Woodhouse <[email protected]>
> ---
> arch/x86/kernel/cpu/common.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> index e5d66e9..c05d0fe 100644
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c
> @@ -900,8 +900,14 @@ static void __init early_identify_cpu(struct cpuinfo_x86 *c)
>
> setup_force_cpu_cap(X86_FEATURE_ALWAYS);
>
> - if (c->x86_vendor != X86_VENDOR_AMD)
> - setup_force_cpu_bug(X86_BUG_CPU_MELTDOWN);
> + if (c->x86_vendor != X86_VENDOR_AMD) {
> + u64 ia32_cap = 0;
> +
> + if (cpu_has(c, X86_FEATURE_ARCH_CAPABILITIES))
> + rdmsrl(MSR_IA32_ARCH_CAPABILITIES, ia32_cap);
> + if (!(ia32_cap & ARCH_CAP_RDCL_NO))
> + setup_force_cpu_bug(X86_BUG_CPU_MELTDOWN);
> + }

I'd really rather we break this out into a nice, linear set of
true/false conditions.

bool early_cpu_vulnerable_meltdown(struct cpuinfo_x86 *c)
{
u64 ia32_cap = 0;

/* AMD processors are not subject to Meltdown exploit: */
if (c->x86_vendor == X86_VENDOR_AMD)
return false;

/* Assume all remaining CPUs not enumerating are vulnerable: */
if (!cpu_has(c, X86_FEATURE_ARCH_CAPABILITIES))
return true;

/*
* Does the CPU explicitly enumerate that it is not vulnerable
* to Rogue Data Cache Load (aka Meltdown)?
*/
rdmsrl(MSR_IA32_ARCH_CAPABILITIES, ia32_cap);
if (ia32_cap & ARCH_CAP_RDCL_NO)
return false;

/* Assume everything else is vulnerable */
return true;
}

Then we get a nice:

if (early_cpu_vulnerable_meltdown(c))
setup_force_cpu_bug(X86_BUG_CPU_MELTDOWN);
setup_force_cpu_bug(X86_BUG_SPECTRE_V1);
setup_force_cpu_bug(X86_BUG_SPECTRE_V2);

Which clearly shows that Meltdown is special.

2018-01-23 18:41:49

by Dave Hansen

[permalink] [raw]
Subject: Re: [PATCH v2 5/5] x86/pti: Do not enable PTI on fixed Intel processors

On 01/23/2018 08:52 AM, David Woodhouse wrote:
> When they advertise the IA32_ARCH_CAPABILITIES MSR and it has the RDCL_NO
> bit set, they don't need KPTI either.

Also, I hate to ask... Have you tested this patch?

2018-01-23 18:43:52

by Dave Hansen

[permalink] [raw]
Subject: Re: [PATCH v2 2/5] x86/cpufeatures: Add Intel feature bits for Speculation Control

On 01/23/2018 08:52 AM, David Woodhouse wrote:
>
> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
> index 7b25cf3..0a51070 100644
> --- a/arch/x86/include/asm/cpufeatures.h
> +++ b/arch/x86/include/asm/cpufeatures.h
> @@ -320,6 +320,9 @@
> /* Intel-defined CPU features, CPUID level 0x00000007:0 (EDX), word 18 */
> #define X86_FEATURE_AVX512_4VNNIW (18*32+ 2) /* AVX-512 Neural Network Instructions */
> #define X86_FEATURE_AVX512_4FMAPS (18*32+ 3) /* AVX-512 Multiply Accumulation Single precision */
> +#define X86_FEATURE_SPEC_CTRL (18*32+26) /* Speculation Control (IBRS + IBPB) */
> +#define X86_FEATURE_STIBP (18*32+27) /* Single Thread Indirect Branch Predictors */
> +#define X86_FEATURE_ARCH_CAPABILITIES (18*32+29) /* IA32_ARCH_CAPABILITIES MSR (Intel) */

Should we be adding flags (STIBP) for which we currently have no user in
the kernel?

2018-01-23 18:47:17

by Alan Cox

[permalink] [raw]
Subject: Re: [PATCH v2 5/5] x86/pti: Do not enable PTI on fixed Intel processors

On Tue, 23 Jan 2018 16:52:55 +0000
David Woodhouse <[email protected]> wrote:

> When they advertise the IA32_ARCH_CAPABILITIES MSR and it has the RDCL_NO
> bit set, they don't need KPTI either.

This is starting to get messy because we will eventually need to integrate

AMD processors - no meltdown but spectre
VIA processors - probably no vulnerabilities at
least on the old ones
Intel with ND set - No meltdown
Anybody with no speculation - No meltdown, no spectre, no id bit



and it expands a lot with all sorts of 32bit processors. Would it make
more sense to make it table driven or do we want a separate function so
we can do:

if (!in_order_cpu()) {
}

around the whole lot ? I'm guessing the latter makes sense then
somethhing like this patch I'm running on my old atom widgets in 64bit
mode

static __initdata struct x86_cpu_id cpu_in_order[] = {
{ X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_CEDARVIEW, X86_FEATURE_ANY },
{ X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_CLOVERVIEW, X86_FEATURE_ANY },
{ X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_LINCROFT, X86_FEATURE_ANY },
{ X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_PENWELL, X86_FEATURE_ANY },
{ X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_PINEVIEW, X86_FEATURE_ANY },
{}
};

static int in_order_cpu(void)
{
/* Processors with CPU id etc */
if (x86_match_cpu(cpu_in_order))
return 1;
/* Other rules here */
return 0;
}

Alan

2018-01-23 18:53:18

by Woodhouse, David

[permalink] [raw]
Subject: Re: [PATCH v2 4/5] x86/msr: Add definitions for new speculation control MSRs

On Tue, 2018-01-23 at 19:31 +0100, Greg KH wrote:
> On Tue, Jan 23, 2018 at 10:27:24AM -0800, Dave Hansen wrote:
> >
> > On 01/23/2018 08:52 AM, David Woodhouse wrote:
> > >
> > > +#define MSR_IA32_ARCH_CAPABILITIES 0x0000010a
> > > +#define ARCH_CAP_RDCL_NO (1 << 0)   /* Not susceptible to Meltdown */
> > > +#define ARCH_CAP_IBRS_ALL (1 << 1)   /* Enhanced IBRS support */
> > Do we want to spell out the silly Intel acronym?  I don't know how we
> > fit it on the right side, but I do think we need to do it _somewhere_.
> > We need the code to stand on its own to some degree and not subject the
> > masses to reading the spec to understand the code.
>
> Yes please, take pity on us :)

The comment says 'Not susceptible to Meltdown', and it's used in a
context where it disabled the X86_CPU_BUG_MELTDOWN bit. I think that's
a *bit* of a hint about what it does.

Expanding the silly Intel acronym doesn't give you any useful
information except perhaps which pointless bits of word soup they
happened to string together in the spec.


Attachments:
smime.p7s (5.09 kB)

2018-01-23 18:53:48

by Woodhouse, David

[permalink] [raw]
Subject: Re: [PATCH v2 5/5] x86/pti: Do not enable PTI on fixed Intel processors

On Tue, 2018-01-23 at 10:41 -0800, Dave Hansen wrote:
> On 01/23/2018 08:52 AM, David Woodhouse wrote:
> > When they advertise the IA32_ARCH_CAPABILITIES MSR and it has the RDCL_NO
> > bit set, they don't need KPTI either.
>
> Also, I hate to ask...  Have you tested this patch?

Nope. I deliberately haven't implemented IBRS_ALL because I have no
suitable hardware for that and there are some non-trivial parts, but
this one really *should* be trivial so seemed OK to do without
hardware.


Attachments:
smime.p7s (5.09 kB)

2018-01-23 19:05:14

by Andrew Cooper

[permalink] [raw]
Subject: Re: [PATCH v2 5/5] x86/pti: Do not enable PTI on fixed Intel processors

On 23/01/18 18:45, Alan Cox wrote:
> On Tue, 23 Jan 2018 16:52:55 +0000
> David Woodhouse <[email protected]> wrote:
>
>> When they advertise the IA32_ARCH_CAPABILITIES MSR and it has the RDCL_NO
>> bit set, they don't need KPTI either.
> This is starting to get messy because we will eventually need to integrate
>
> AMD processors - no meltdown but spectre
> VIA processors - probably no vulnerabilities at
> least on the old ones
> Intel with ND set - No meltdown
> Anybody with no speculation - No meltdown, no spectre, no id bit
>
>
>
> and it expands a lot with all sorts of 32bit processors. Would it make
> more sense to make it table driven or do we want a separate function so
> we can do:
>
> if (!in_order_cpu()) {
> }
>
> around the whole lot ? I'm guessing the latter makes sense then
> somethhing like this patch I'm running on my old atom widgets in 64bit
> mode
>
> static __initdata struct x86_cpu_id cpu_in_order[] = {
> { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_CEDARVIEW, X86_FEATURE_ANY },
> { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_CLOVERVIEW, X86_FEATURE_ANY },
> { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_LINCROFT, X86_FEATURE_ANY },
> { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_PENWELL, X86_FEATURE_ANY },
> { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_PINEVIEW, X86_FEATURE_ANY },
> {}
> };
>
> static int in_order_cpu(void)
> {
> /* Processors with CPU id etc */
> if (x86_match_cpu(cpu_in_order))
> return 1;
> /* Other rules here */
> return 0;
> }

Why does in-order vs out-of-order matter?

There are leaky SP3 gadgets which satisfy in-order requirements, so long
as the processor is capable of speculating 3 instructions past an
unresolved branch.

What would (at a guess) save an in-order speculative processor from
being vulnerable is if memory reads are issued and resolve in program
order, but in that case, it is not the in-order property of the
processor which makes it safe.

~Andrew

2018-01-23 20:40:48

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH v2 5/5] x86/pti: Do not enable PTI on fixed Intel processors

On Tue, Jan 23, 2018 at 10:45 AM, Alan Cox <[email protected]> wrote:
>
> static int in_order_cpu(void)
> {
> /* Processors with CPU id etc */
> if (x86_match_cpu(cpu_in_order))
> return 1;
> /* Other rules here */
> return 0;
> }

Note that "in-order" does *not* necessarily mean "no speculation". You
still do branch prediction, you still have a BTB for all but the
simplest in-order things, and you may still end up running
instructions at least part-way through a pipeline. Whether it gets to
a cache replacement or not is unclear.

So that naming is very misleading.

It is quite possible (even likely) that the old in-order Atoms do not
have this issue, because it's definitely true that in-order tends to
limit speculation a lot, and any mis-predict will _probably_ kill
instructions early enough that you'd never see this.

But meltdown, for example, is based entirely on uarch details in the
memory pipeline, not on in-order vs ooo. Again, an in-order CPU is
probably less aggressive in the memory pipeline too, so there is
probably strong correlation, but there is no causal relationship.

Linus

2018-01-23 20:46:39

by Alan Cox

[permalink] [raw]
Subject: Re: [PATCH v2 5/5] x86/pti: Do not enable PTI on fixed Intel processors

> > static int in_order_cpu(void)
> > {
> > /* Processors with CPU id etc */
> > if (x86_match_cpu(cpu_in_order))
> > return 1;
> > /* Other rules here */
> > return 0;
> > }
>
> Why does in-order vs out-of-order matter?
>
> There are leaky SP3 gadgets which satisfy in-order requirements, so long
> as the processor is capable of speculating 3 instructions past an
> unresolved branch.
>
> What would (at a guess) save an in-order speculative processor from
> being vulnerable is if memory reads are issued and resolve in program
> order, but in that case, it is not the in-order property of the
> processor which makes it safe.

Fair point - I should rename it cpu_speculates(). The atoms in that
list don't speculate.

Alan
[My Cyrix 6x86 had a different kind of meltdown problem....]

2018-01-23 20:52:39

by David Woodhouse

[permalink] [raw]
Subject: Re: [PATCH v2 5/5] x86/pti: Do not enable PTI on fixed Intel processors

> On Tue, 2018-01-23 at 18:45 +0000, Alan Cox wrote:
>
> static __initdata struct x86_cpu_id cpu_in_order[] = {
>         { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_CEDARVIEW, X86_FEATURE_ANY },
>         { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_CLOVERVIEW, X86_FEATURE_ANY },
>         { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_LINCROFT, X86_FEATURE_ANY },
>         { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_PENWELL, X86_FEATURE_ANY },
>         { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_PINEVIEW, X86_FEATURE_ANY },
>         {}
> };

Are those getting a microcode update to advertise RDCL_NO in the
IA32_ARCH_CAPABILITIES MSR?


Attachments:
smime.p7s (5.09 kB)

2018-01-24 01:22:48

by David Woodhouse

[permalink] [raw]
Subject: Re: [PATCH v2 5/5] x86/pti: Do not enable PTI on fixed Intel processors

On Tue, 2018-01-23 at 10:12 -0800, Andi Kleen wrote:
> > -     if (c->x86_vendor != X86_VENDOR_AMD)
> > -             setup_force_cpu_bug(X86_BUG_CPU_MELTDOWN);
> > +     if (c->x86_vendor != X86_VENDOR_AMD) {
> > +             u64 ia32_cap = 0;
> > +
> > +             if (cpu_has(c, X86_FEATURE_ARCH_CAPABILITIES))
> > +                     rdmsrl(MSR_IA32_ARCH_CAPABILITIES, ia32_cap);
> > +             if (!(ia32_cap & ARCH_CAP_RDCL_NO))
> > +                     setup_force_cpu_bug(X86_BUG_CPU_MELTDOWN);
>
> This means that in a hypervisor which passes through the CPUID,
> but actually doesn't implement the MSR (so rdmsr #GPs and returns 0)
> it would be cleared. 
>
> It would be better to usr rdmsrl_safe and check the return value.

This particular CPUID bit exists *purely* to advertise that the
IA32_ARCH_CAPABILITIES MSR exists. Nothing else. A hypervisor which
tells us the MSR exists and then doesn't expose it is fairly broken.
Not that it won't happen, of course.

But reading zero is fine. If the bit isn't set we do set the MELTDOWN
bug flag.


Attachments:
smime.p7s (5.09 kB)

2018-01-24 01:28:16

by David Woodhouse

[permalink] [raw]
Subject: Re: [PATCH v2 5/5] x86/pti: Do not enable PTI on fixed Intel processors

On Tue, 2018-01-23 at 10:40 -0800, Dave Hansen wrote:
>
> I'd really rather we break this out into a nice, linear set of
> true/false conditions.
>
> bool early_cpu_vulnerable_meltdown(struct cpuinfo_x86 *c)
> {
>         u64 ia32_cap = 0;
>
>         /* AMD processors are not subject to Meltdown exploit: */
>         if (c->x86_vendor == X86_VENDOR_AMD)
>                 return false;
>
>         /* Assume all remaining CPUs not enumerating are vulnerable: */
>         if (!cpu_has(c, X86_FEATURE_ARCH_CAPABILITIES))
>                 return true;
>
>         /*
>          * Does the CPU explicitly enumerate that it is not vulnerable
>          * to Rogue Data Cache Load (aka Meltdown)?
>          */
>         rdmsrl(MSR_IA32_ARCH_CAPABILITIES, ia32_cap);
>         if (ia32_cap & ARCH_CAP_RDCL_NO)
>                 return false;
>
>         /* Assume everything else is vulnerable */
>         return true;
> }

Makes sense. It also starts to address Alan's "starting to get messy"
comment, and gives a simple way to add other conditions.


Attachments:
smime.p7s (5.09 kB)

2018-01-24 01:29:15

by Dave Hansen

[permalink] [raw]
Subject: Re: [PATCH v2 2/5] x86/cpufeatures: Add Intel feature bits for Speculation Control

On 01/23/2018 05:23 PM, Woodhouse, David wrote:
> On Tue, 2018-01-23 at 10:43 -0800, Dave Hansen wrote:
...
>>>   /* Intel-defined CPU features, CPUID level 0x00000007:0 (EDX), word 18 */
>>>   #define X86_FEATURE_AVX512_4VNNIW    (18*32+ 2) /* AVX-512 Neural Network Instructions */
>>>   #define X86_FEATURE_AVX512_4FMAPS    (18*32+ 3) /* AVX-512 Multiply Accumulation Single precision */
>>> +#define X86_FEATURE_SPEC_CTRL                (18*32+26) /* Speculation Control (IBRS + IBPB) */
>>> +#define X86_FEATURE_STIBP            (18*32+27) /* Single Thread Indirect Branch Predictors */
>>> +#define X86_FEATURE_ARCH_CAPABILITIES        (18*32+29) /* IA32_ARCH_CAPABILITIES MSR (Intel) */
>> Should we be adding flags (STIBP) for which we currently have no user in
>> the kernel?
> They're in an existing word (now) so it costs us absolutely nothing to
> do so. And they'll be exposed to KVM guests in imminent patches if
> nothing else.

Doesn't just defining it here generate something in the tables that then
get exported in /proc/cpuinfo? That's far from our most strict ABI, but
a single #define here can be seen by users IIRC.

2018-01-24 08:32:16

by Woodhouse, David

[permalink] [raw]
Subject: Re: [PATCH v2 2/5] x86/cpufeatures: Add Intel feature bits for Speculation Control

On Tue, 2018-01-23 at 17:28 -0800, Dave Hansen wrote:
> On 01/23/2018 05:23 PM, Woodhouse, David wrote:
> >
> > On Tue, 2018-01-23 at 10:43 -0800, Dave Hansen wrote:
> ...
> >
> > >
> > > >
> > > >   /* Intel-defined CPU features, CPUID level 0x00000007:0 (EDX), word 18 */
> > > >   #define X86_FEATURE_AVX512_4VNNIW    (18*32+ 2) /* AVX-512 Neural Network Instructions */
> > > >   #define X86_FEATURE_AVX512_4FMAPS    (18*32+ 3) /* AVX-512 Multiply Accumulation Single precision */
> > > > +#define X86_FEATURE_SPEC_CTRL                (18*32+26) /* Speculation Control (IBRS + IBPB) */
> > > > +#define X86_FEATURE_STIBP            (18*32+27) /* Single Thread Indirect Branch Predictors */
> > > > +#define X86_FEATURE_ARCH_CAPABILITIES        (18*32+29) /* IA32_ARCH_CAPABILITIES MSR (Intel) */
> > > Should we be adding flags (STIBP) for which we currently have no user in
> > > the kernel?
> > They're in an existing word (now) so it costs us absolutely nothing to
> > do so. And they'll be exposed to KVM guests in imminent patches if
> > nothing else.
>
> Doesn't just defining it here generate something in the tables that then
> get exported in /proc/cpuinfo?  That's far from our most strict ABI, but
> a single #define here can be seen by users IIRC.

That's true, but still we're *working* on exposing and using these;
let's not go wild adding one feature at a time and having to tweak the
surrounding blacklist/enable/disable/expose logic at every step.


Attachments:
smime.p7s (5.09 kB)

2018-01-24 08:39:48

by Kirill A. Shutemov

[permalink] [raw]
Subject: Re: [PATCH v2 3/5] x86/cpufeatures: Add AMD feature bits for Speculation Control

On Tue, Jan 23, 2018 at 04:52:53PM +0000, David Woodhouse wrote:
> AMD exposes the PRED_CMD/SPEC_CTRL MSRs slightly differently to Intel.
> Documented at https://lkml.org/lkml/2018/1/21/112

lkml.org is not very stable. Maybe use messageid-based url instead?

http://lkml.kernel.org/r/[email protected]

--
Kirill A. Shutemov

2018-01-24 08:43:09

by David Woodhouse

[permalink] [raw]
Subject: Re: [PATCH v2 3/5] x86/cpufeatures: Add AMD feature bits for Speculation Control

On Wed, 2018-01-24 at 11:39 +0300, Kirill A. Shutemov wrote:
> On Tue, Jan 23, 2018 at 04:52:53PM +0000, David Woodhouse wrote:
> >
> > AMD exposes the PRED_CMD/SPEC_CTRL MSRs slightly differently to Intel.
> > Documented at https://lkml.org/lkml/2018/1/21/112
>
> lkml.org is not very stable. Maybe use messageid-based url instead?
>
> http://lkml.kernel.org/r/[email protected]

Well... I like the fact that your one has "amd.com" in it... but I did
want to see it a lot further to the left.

Tom?


Attachments:
smime.p7s (5.09 kB)

2018-01-24 16:26:56

by David Woodhouse

[permalink] [raw]
Subject: Re: [PATCH v2 5/5] x86/pti: Do not enable PTI on fixed Intel processors

On Tue, 2018-01-23 at 18:45 +0000, Alan Cox wrote:
> On Tue, 23 Jan 2018 16:52:55 +0000
> David Woodhouse <[email protected]> wrote:
>
> >
> > When they advertise the IA32_ARCH_CAPABILITIES MSR and it has the RDCL_NO
> > bit set, they don't need KPTI either.
> This is starting to get messy because we will eventually need to integrate
>
> AMD processors - no meltdown but spectre
> VIA processors - probably no vulnerabilities at
> least on the old ones
> Intel with ND set - No meltdown
> Anybody with no speculation - No meltdown, no spectre, no id bit
>
>
>
> and it expands a lot with all sorts of 32bit processors. Would it make
> more sense to make it table driven or do we want a separate function so
> we can do:
>
>                 if (!in_order_cpu()) {
>                 }
>
> around the whole lot ? I'm guessing the latter makes sense then
> somethhing like this patch I'm running on my old atom widgets in 64bit
> mode
>
> static __initdata struct x86_cpu_id cpu_in_order[] = {
>         { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_CEDARVIEW, X86_FEATURE_ANY },
>         { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_CLOVERVIEW, X86_FEATURE_ANY },
>         { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_LINCROFT, X86_FEATURE_ANY },
>         { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_PENWELL, X86_FEATURE_ANY },
>         { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_PINEVIEW, X86_FEATURE_ANY },
>         {}
> };
>
> static int in_order_cpu(void)
> {
> /* Processors with CPU id etc */
> if (x86_match_cpu(cpu_in_order))
> return 1;
> /* Other rules here */
> return 0;
> }

How's this? I'll send it out properly in a little while, but feel free
to heckle in advance...

From 26fd510f8100a869866fa416bf1bfb7ea22dcf9f Mon Sep 17 00:00:00 2001
From: David Woodhouse <[email protected]>
Date: Fri, 19 Jan 2018 14:43:08 +0100
Subject: [PATCH] x86/pti: Do not enable PTI on processors which are not
 vulnerable to Meltdown

Some old Atoms, anything in family 5 or 4, and newer CPUs when they advertise
the IA32_ARCH_CAPABILITIES MSR and it has the RDCL_NO bit set, are not vulnerable.

Roll the AMD exemption into the x86_match_cpu() table too.

Based on suggestions from Dave Hansen and Alan Cox.

Signed-off-by: David Woodhouse <[email protected]>
---
 arch/x86/kernel/cpu/common.c | 32 ++++++++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index e5d66e93ed81..23375561d819 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -47,6 +47,8 @@
 #include <asm/pat.h>
 #include <asm/microcode.h>
 #include <asm/microcode_intel.h>
+#include <asm/intel-family.h>
+#include <asm/cpu_device_id.h>
 
 #ifdef CONFIG_X86_LOCAL_APIC
 #include <asm/uv/uv.h>
@@ -853,6 +855,33 @@ static void identify_cpu_without_cpuid(struct cpuinfo_x86 *c)
 #endif
 }
 
+static const __initdata struct x86_cpu_id cpu_no_meltdown[] = {
+ { X86_VENDOR_AMD },
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_CEDARVIEW, X86_FEATURE_ANY },
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_CLOVERVIEW, X86_FEATURE_ANY },
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_LINCROFT, X86_FEATURE_ANY },
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_PENWELL, X86_FEATURE_ANY },
+ { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_PINEVIEW, X86_FEATURE_ANY },
+ { X86_VENDOR_ANY, 5 },
+ { X86_VENDOR_ANY, 4 },
+ {}
+};
+
+static bool __init early_cpu_vulnerable_meltdown(struct cpuinfo_x86 *c)
+{
+ u64 ia32_cap = 0;
+
+ if (x86_match_cpu(cpu_no_meltdown))
+                return false;
+
+ if (cpu_has(c, X86_FEATURE_ARCH_CAPABILITIES))
+ rdmsrl(MSR_IA32_ARCH_CAPABILITIES, ia32_cap);
+ if (ia32_cap & ARCH_CAP_RDCL_NO)
+ return false;
+
+ return true;
+}
+
 /*
  * Do minimum CPU detection early.
  * Fields really needed: vendor, cpuid_level, family, model, mask,
@@ -900,9 +929,8 @@ static void __init early_identify_cpu(struct cpuinfo_x86 *c)
 
  setup_force_cpu_cap(X86_FEATURE_ALWAYS);
 
- if (c->x86_vendor != X86_VENDOR_AMD)
+ if (early_cpu_vulnerable_meltdown(c))
  setup_force_cpu_bug(X86_BUG_CPU_MELTDOWN);
-
  setup_force_cpu_bug(X86_BUG_SPECTRE_V1);
  setup_force_cpu_bug(X86_BUG_SPECTRE_V2);
 
-- 
2.14.3


Attachments:
smime.p7s (5.09 kB)

2018-01-24 17:08:20

by Alan Cox

[permalink] [raw]
Subject: Re: [PATCH v2 5/5] x86/pti: Do not enable PTI on fixed Intel processors

>  
> +static const __initdata struct x86_cpu_id cpu_no_meltdown[] = {
> + { X86_VENDOR_AMD },
> + { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_CEDARVIEW, X86_FEATURE_ANY },
> + { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_CLOVERVIEW, X86_FEATURE_ANY },
> + { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_LINCROFT, X86_FEATURE_ANY },
> + { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_PENWELL, X86_FEATURE_ANY },
> + { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_PINEVIEW, X86_FEATURE_ANY },

As Linus said this should be no_specualtion[]

If we are going to capture 32bit here with your lines below I'll send you
an update at some point with all the 32bit families hunted down (some
like the CE4100 may take a bit of hunting)


> + { X86_VENDOR_ANY, 5 },

AND K5 speculates, Cyrix 6x86 speculates, IDT WinChip does not. I think
this should be

X86_VENDOR_ANY, 4
X86_VENDOR_INTEL, 5,
X86_VENDOR_CENTAUR, 5,


> +static bool __init early_cpu_vulnerable_meltdown(struct cpuinfo_x86 *c)
> +{
> + u64 ia32_cap = 0;
> +
> + if (x86_match_cpu(cpu_no_meltdown))
> +                return false;

These processors are also not vulnerable to spectre, so this patch
doesn't set the other flags correctly - that's why we need two levels of
logic here. "Bonnell" and "Saltwell" uarch Atom processors are not
vulnerable to Meltdown or Spectre, neithr is a 486, Pentium, Quark etc.



> +
> + if (cpu_has(c, X86_FEATURE_ARCH_CAPABILITIES))
> + rdmsrl(MSR_IA32_ARCH_CAPABILITIES, ia32_cap);
> + if (ia32_cap & ARCH_CAP_RDCL_NO)
> + return false;
> +
> + return true;
> +}
> +
>  /*
>   * Do minimum CPU detection early.
>   * Fields really needed: vendor, cpuid_level, family, model, mask,
> @@ -900,9 +929,8 @@ static void __init early_identify_cpu(struct cpuinfo_x86 *c)
>  
>   setup_force_cpu_cap(X86_FEATURE_ALWAYS);
>  
> - if (c->x86_vendor != X86_VENDOR_AMD)
> + if (early_cpu_vulnerable_meltdown(c))
>   setup_force_cpu_bug(X86_BUG_CPU_MELTDOWN);
> -
>   setup_force_cpu_bug(X86_BUG_SPECTRE_V1);
>   setup_force_cpu_bug(X86_BUG_SPECTRE_V2);
>  
> -- 
> 2.14.3

2018-01-24 17:42:58

by David Woodhouse

[permalink] [raw]
Subject: Re: [PATCH v2 5/5] x86/pti: Do not enable PTI on fixed Intel processors

On Wed, 2018-01-24 at 17:07 +0000, Alan Cox wrote:
> >
> >  
> > +static const __initdata struct x86_cpu_id cpu_no_meltdown[] = {
> > + { X86_VENDOR_AMD },
> > + { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_CEDARVIEW, X86_FEATURE_ANY },
> > + { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_CLOVERVIEW, X86_FEATURE_ANY },
> > + { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_LINCROFT, X86_FEATURE_ANY },
> > + { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_PENWELL, X86_FEATURE_ANY },
> > + { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_PINEVIEW, X86_FEATURE_ANY },
> As Linus said this should be no_specualtion[]
>
> If we are going to capture 32bit here with your lines below I'll send you
> an update at some point with all the 32bit families hunted down (some
> like the CE4100 may take a bit of hunting)
>
>
> >
> > + { X86_VENDOR_ANY, 5 },
> AND K5 speculates, Cyrix 6x86 speculates, IDT WinChip does not. I think
> this should be
>
> X86_VENDOR_ANY, 4
> X86_VENDOR_INTEL, 5,
> X86_VENDOR_CENTAUR, 5,

Hm, for the specific case of controlling X86_BUG_CPU_MELTDOWN it's not
just "speculates" which is the criterion. It's "optimises away the
permissions checks while speculating, on the assumption that it'll be
fixed up before retiring the instruction".

I think X86_BUG_CPU_SPECTRE_V2 might end being a lot closer to just "it
speculates".

> >
> > +static bool __init early_cpu_vulnerable_meltdown(struct cpuinfo_x86 *c)
> > +{
> > + u64 ia32_cap = 0;
> > +
> > + if (x86_match_cpu(cpu_no_meltdown))
> > +                return false;
> These processors are also not vulnerable to spectre, so this patch
> doesn't set the other flags correctly - that's why we need two levels of
> logic here. "Bonnell" and "Saltwell" uarch Atom processors are not
> vulnerable to Meltdown or Spectre, neithr is a 486, Pentium, Quark etc.

Yeah, I've deliberately not touched Spectre for this case.

By the time the dust settles we might end up with a bunch of different
match tables, *one* of which is "does not speculate at all". And the
conditions for the different bugs will each use different sets of match
tables. For example

 if (!x86_match_cpu(cpu_no_speculation_at_all) &&
     !x86_match_cpu(speculation_but_no_meltdown) &&
     !cpu_sets_rdcl_no())
setup_force_cpu_bug(X86_BUG_CPU_MELTDOWN);

 if (!x86_match_cpu(cpu_no_speculation_at_all) &&
     !x86_match_cpu(no_branch_target_buffer))
setup_force_cpu_bug(X86_BUG_SPECTRE_V2);

...


Let's gather the data and see how we want to break it down according to
which subsets are common. In the mean time Meltdown is the big one
which has performance implications and wants to be avoided if we can.


Attachments:
smime.p7s (5.09 kB)

2018-01-24 18:42:26

by Alan Cox

[permalink] [raw]
Subject: Re: [PATCH v2 5/5] x86/pti: Do not enable PTI on fixed Intel processors

> > AND K5 speculates, Cyrix 6x86 speculates, IDT WinChip does not. I think
> > this should be
> >
> > X86_VENDOR_ANY, 4
> > X86_VENDOR_INTEL, 5,
> > X86_VENDOR_CENTAUR, 5,
>
> Hm, for the specific case of controlling X86_BUG_CPU_MELTDOWN it's not
> just "speculates" which is the criterion. It's "optimises away the
> permissions checks while speculating, on the assumption that it'll be
> fixed up before retiring the instruction".

Nobody has published official statements on Cyrix or AMD 32bit processors
so we don't know if they are vulnerable to meltdown. One problem I
suspect is that as with things like Alpha 21264 - the people who knew are
probably long retired. We do know the Intel ones I listed are OK and the
Centaur.

If someone can figure out the Cyrix and AMD cases that would be great.

> By the time the dust settles we might end up with a bunch of different
> match tables, *one* of which is "does not speculate at all". And the
> conditions for the different bugs will each use different sets of match
> tables. For example
>
>  if (!x86_match_cpu(cpu_no_speculation_at_all) &&
>      !x86_match_cpu(speculation_but_no_meltdown) &&
>      !cpu_sets_rdcl_no())
> setup_force_cpu_bug(X86_BUG_CPU_MELTDOWN);
>
>  if (!x86_match_cpu(cpu_no_speculation_at_all) &&
>      !x86_match_cpu(no_branch_target_buffer))
> setup_force_cpu_bug(X86_BUG_SPECTRE_V2);

There are afaik no x86 processors that speculate and don't have a BTB.
It's a bit like building a racing car with no gearbox.

Alan

2018-01-24 19:00:12

by David Woodhouse

[permalink] [raw]
Subject: Re: [PATCH v2 5/5] x86/pti: Do not enable PTI on fixed Intel processors

On Wed, 2018-01-24 at 18:40 +0000, Alan Cox wrote:
> Nobody has published official statements on Cyrix or AMD 32bit processors
> so we don't know if they are vulnerable to meltdown. One problem I
> suspect is that as with things like Alpha 21264 - the people who knew are
> probably long retired. We do know the Intel ones I listed are OK and the
> Centaur.
>
> If someone can figure out the Cyrix and AMD cases that would be great.

Well Tom already submitted a patch to turn it off for *all* AMD, 32-bit
and 64-bit.

> >
> > By the time the dust settles we might end up with a bunch of different
> > match tables, *one* of which is "does not speculate at all". And the
> > conditions for the different bugs will each use different sets of match
> > tables. For example
> >
> >  if (!x86_match_cpu(cpu_no_speculation_at_all) &&
> >      !x86_match_cpu(speculation_but_no_meltdown) &&
> >      !cpu_sets_rdcl_no())
> > setup_force_cpu_bug(X86_BUG_CPU_MELTDOWN);
> >
> >  if (!x86_match_cpu(cpu_no_speculation_at_all) &&
> >      !x86_match_cpu(no_branch_target_buffer))
> > setup_force_cpu_bug(X86_BUG_SPECTRE_V2);
> There are afaik no x86 processors that speculate and don't have a BTB.
> It's a bit like building a racing car with no gearbox.

Right, "has a BTB and doesn't tag/flush it according to privilege level
and context". Which is the thing that, as lamented, Intel hasn't even
proposed a way to *tell* us that's the case, if/when they finally
manage to fix it up in the next generation *after* the IBRS_ALL hack.

So yeah, X86_BUG_SPECTRE_V2 probably *does* end up being "speculates".

I'll change !Meltdown from ANY,5 to INTEL,5 + CENTAUR,5 as you suggest,
and we can work on Spectre separately.

Thanks.


Attachments:
smime.p7s (5.09 kB)

2018-01-24 19:38:39

by David Woodhouse

[permalink] [raw]
Subject: Re: [PATCH v2 5/5] x86/pti: Do not enable PTI on fixed Intel processors

How about...

static const __initdata struct x86_cpu_id cpu_no_speculation[] = {
{ X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_CEDARVIEW, X86_FEATURE_ANY },
{ X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_CLOVERVIEW, X86_FEATURE_ANY },
{ X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_LINCROFT, X86_FEATURE_ANY },
{ X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_PENWELL, X86_FEATURE_ANY },
{ X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_PINEVIEW, X86_FEATURE_ANY },
{ X86_VENDOR_INTEL, 5 },
{ X86_VENDOR_CENTAUR, 5 },
{ X86_VENDOR_ANY, 4 },
{}
};

static const __initdata struct x86_cpu_id cpu_no_meltdown[] = {
{ X86_VENDOR_AMD },
{}
};

static bool __init early_cpu_vulnerable_meltdown(struct cpuinfo_x86 *c)
{
u64 ia32_cap = 0;

if (x86_match_cpu(cpu_no_meltdown))
                return false;

if (cpu_has(c, X86_FEATURE_ARCH_CAPABILITIES))
rdmsrl(MSR_IA32_ARCH_CAPABILITIES, ia32_cap);

/* Rogue Data Cache Load? No! */
if (ia32_cap & ARCH_CAP_RDCL_NO)
return false;

return true;
}

...



if (!x86_match_cpu(cpu_no_speculation)) {
if (early_cpu_vulnerable_meltdown(c))
setup_force_cpu_bug(X86_BUG_CPU_MELTDOWN);
setup_force_cpu_bug(X86_BUG_SPECTRE_V1);
setup_force_cpu_bug(X86_BUG_SPECTRE_V2);
}


Attachments:
smime.p7s (5.09 kB)