v6 more sampling fun 0
Subsystems (4)
cover v6 more sampling fun 0
From: mark gross <[email protected]>
Subject: [PATCH 0/4] v6 more sampling fun
resend because I forgot the --thread option on the format-patch. sorry for the spam
This version of the Special Register Buffer Data Sampling has been updated to address feedback gotten. Including:
* is based on linus/master from this AM.
* cleanups and changes driven by review feedback from Josh and Thomas.
* addresses the case of intel fusing TSX off on lower end SKU's
* spits out some refactoring from the main patch to better isolate the SRBDS specific changes.
Thanks to all who have given me feedback on this.
---
Special Register Buffer Data Sampling is a sampling type of vulnerability that leaks data across cores sharing the HW-RNG for vulnerable processors.
This leak is fixed by a microcode update and is enabled by default.
This new microcode serializes processor access during execution of RDRAND or RDSEED. It ensures that the shared buffer is overwritten before it is released for reuse.
The mitigation impacts the throughput of the RDRAND and RDSEED instructions and latency of RT processing running on the socket while executing RDRAND or RDSEED. The micro benchmarks calling RDRAND many times show a slowdown.
This patch set enables kernel command line control of this mitigation and exports vulnerability and mitigation status.
This patch set includes 3 patches:
* The first patch adds steppings to x86_cpu_id structure and related macros
* The second patch enables the command line control of the mitigation as well as the sysfs export of vulnerability status.
* The third patch has the Documentation/admin-guide/hw-vuln documentation for this issue and the control over the mitigation.
mark gross (4):
x86/cpu: Add stepping field to x86_cpu_id structure x86/cpu: clean up cpu_matches
x86/speculation: Special Register Buffer Data Sampling (SRBDS)
mitigation control.
x86/speculation: SRBDS vulnerability and mitigation documentation
.../ABI/testing/sysfs-devices-system-cpu | 1 + Documentation/admin-guide/hw-vuln/index.rst | 1 + .../special-register-buffer-data-sampling.rst | 148 ++++++++++++++++++ .../admin-guide/kernel-parameters.txt | 20 +++ arch/x86/include/asm/cpu_device_id.h | 26 ++- arch/x86/include/asm/cpufeatures.h | 2 + arch/x86/include/asm/msr-index.h | 4 + arch/x86/kernel/cpu/bugs.c | 116 ++++++++++++++ arch/x86/kernel/cpu/common.c | 68 ++++++-- arch/x86/kernel/cpu/cpu.h | 2 + arch/x86/kernel/cpu/match.c | 7 +- drivers/base/cpu.c | 8 + include/linux/mod_devicetable.h | 2 + 13 files changed, 390 insertions(+), 15 deletions(-) create mode 100644 Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst
Signature · 1 line
2.17.1
PATCH 1/4 v6 more sampling fun 1
From: mark gross <[email protected]>
Subject: [PATCH 1/4] x86/cpu: Add stepping field to x86_cpu_id structure
Intel uses the same family/model for several CPUs. Sometimes the stepping must be checked to tell them apart.
On X86 there can be at most 16 steppings, add steppings bitmask to x86_cpu_id and X86_MATCH_VENDOR_FAMILY_MODEL_STEPPING_FEATURE macro and support for matching against family/model/stepping.
--- arch/x86/include/asm/cpu_device_id.h | 26 +++++++++++++++++++++++--- arch/x86/kernel/cpu/match.c | 7 ++++++- include/linux/mod_devicetable.h | 2 ++ 3 files changed, 31 insertions(+), 4 deletions(-)
arch/x86/include/asm/cpu_device_id.h +23 -3
@@ -21,11 +21,12 @@#define X86_CENTAUR_FAM6_NANO 0xf/** -* X86_MATCH_VENDOR_FAM_MODEL_FEATURE - Base macro for CPU matching +* X86_MATCH_VENDOR_FAM_MODEL_STEPPING_FEATURE - Base macro for CPU matching * @_vendor: The vendor name, e.g. INTEL, AMD, HYGON, ..., ANY * The name is expanded to X86_VENDOR_@_vendor * @_family: The family number or X86_FAMILY_ANY * @_model: The model number, model constant or X86_MODEL_ANY +* @_steppings: Bitmask for steppings, stepping constant or X86_STEPPING_ANY * @_feature: A X86_FEATURE bit or X86_FEATURE_ANY * @_data: Driver specific data or NULL. The internal storage * format is unsigned long. The supplied value, pointer
@@ -37,15 +38,34 @@* into another macro at the usage site for good reasons, then please * start this local macro with X86_MATCH to allow easy grepping. */ -#define X86_MATCH_VENDOR_FAM_MODEL_FEATURE (_vendor, _family, _model, \ - _feature, _data) { \ +#define X86_MATCH_VENDOR_FAM_MODEL_STEPPING_FEATURE (_vendor, _family, _model, \ + _steppings, _feature, _data) { \ .vendor = X86_VENDOR_##_vendor, \ .family = _family, \ .model = _model, \ + .steppings = _steppings, \ .feature = _feature, \ .driver_data = (unsigned long) _data \ } +/** +* X86_MATCH_VENDOR_FAM_MODEL_FEATURE - Base macro for CPU matching +* @_vendor: The vendor name, e.g. INTEL, AMD, HYGON, ..., ANY +* The name is expanded to X86_VENDOR_@_vendor +* @_family: The family number or X86_FAMILY_ANY +* @_model: The model number, model constant or X86_MODEL_ANY +* @_feature: A X86_FEATURE bit or X86_FEATURE_ANY +* @_data: Driver specific data or NULL. The internal storage +* format is unsigned long. The supplied value, pointer +* etc. is casted to unsigned long internally. +* +* The steppings arguments of X86_MATCH_VENDOR_FAM_MODEL_STEPPING_FEATURE() is +* set to wildcards. +*/ +#define X86_MATCH_VENDOR_FAM_MODEL_FEATURE (vendor, family, model, feature, data) \ + X86_MATCH_VENDOR_FAM_MODEL_STEPPING_FEATURE(vendor, family, model, \ + X86_STEPPING_ANY, feature, data) +/** * X86_MATCH_VENDOR_FAM_FEATURE - Macro for matching vendor, family and CPU feature * @vendor: The vendor name, e.g. INTEL, AMD, HYGON, ..., ANY
arch/x86/kernel/cpu/match.c +6 -1
@@ -39,13 +39,18 @@ const struct x86_cpu_id *x86_match_cpu(const struct x86_cpu_id *match)const struct x86_cpu_id *m;struct cpuinfo_x86 *c = &boot_cpu_data; -for (m = match; m->vendor | m->family | m->model | m->feature; m++) { +for (m = match; + m->vendor | m->family | m->model | m->steppings | m->feature; + m++) {if (m->vendor !=X86_VENDOR_ANY && c->x86_vendor != m->vendor)continue ;if (m->family !=X86_FAMILY_ANY && c->x86 != m->family)continue ;if (m->model !=X86_MODEL_ANY && c->x86_model != m->model)continue ; +if (m->steppings !=X86_STEPPING_ANY && + !(BIT (c->x86_stepping) & m->steppings)) +continue ;if (m->feature !=X86_FEATURE_ANY && !cpu_has (c, m->feature))continue ;return m;
include/linux/mod_devicetable.h +2 -0
@@ -663,6 +663,7 @@ struct x86_cpu_id {__u16 vendor;__u16 family;__u16 model; +__u16 steppings;__u16 feature;/* bit index */ kernel_ulong_t driver_data; };
@@ -671,6 +672,7 @@ struct x86_cpu_id {#define X86_VENDOR_ANY 0xffff#define X86_FAMILY_ANY 0#define X86_MODEL_ANY 0 +#define X86_STEPPING_ANY 0#define X86_FEATURE_ANY 0/* Same as FPU, you can't test for that */ /*
Signature · 1 line
2.17.1
PATCH 2/4 v6 more sampling fun 2
In reply to mark grossFrom: mark gross <[email protected]>
Subject: [PATCH 2/4] x86/cpu: clean up cpu_matches
To make cpu_matches reusable for alternitive matching tables, make cpu_matches take a x86_cpu_id table as a parameter.
--- arch/x86/kernel/cpu/common.c | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-)
arch/x86/kernel/cpu/common.c +14 -11
@@ -1075,9 +1075,9 @@ static const __initconst struct x86_cpu_id cpu_vuln_whitelist[] = { {} }; -static bool __init cpu_matches (unsigned long which) +static bool __init cpu_matches (unsigned long which,const struct x86_cpu_id *table) { -const struct x86_cpu_id *m =x86_match_cpu (cpu_vuln_whitelist); +const struct x86_cpu_id *m =x86_match_cpu (table);return m && !!(m->driver_data & which); }
@@ -1097,31 +1097,34 @@ static void __init cpu_set_bug_bits(struct cpuinfo_x86 *c)u64 ia32_cap =x86_read_arch_cap_msr ();/* Set ITLB_MULTIHIT bug if cpu is not in the whitelist and not mitigated */ -if (!cpu_matches (NO_ITLB_MULTIHIT ) && !(ia32_cap &ARCH_CAP_PSCHANGE_MC_NO )) +if (!cpu_matches (NO_ITLB_MULTIHIT , cpu_vuln_whitelist) && + !(ia32_cap &ARCH_CAP_PSCHANGE_MC_NO ))setup_force_cpu_bug (X86_BUG_ITLB_MULTIHIT ); -if (cpu_matches (NO_SPECULATION )) +if (cpu_matches (NO_SPECULATION , cpu_vuln_whitelist))return ;setup_force_cpu_bug (X86_BUG_SPECTRE_V1 ); -if (!cpu_matches (NO_SPECTRE_V2 )) +if (!cpu_matches (NO_SPECTRE_V2 , cpu_vuln_whitelist))setup_force_cpu_bug (X86_BUG_SPECTRE_V2 ); -if (!cpu_matches (NO_SSB ) && !(ia32_cap &ARCH_CAP_SSB_NO ) && +if (!cpu_matches (NO_SSB , cpu_vuln_whitelist) && + !(ia32_cap &ARCH_CAP_SSB_NO ) && !cpu_has (c,X86_FEATURE_AMD_SSB_NO ))setup_force_cpu_bug (X86_BUG_SPEC_STORE_BYPASS );if (ia32_cap &ARCH_CAP_IBRS_ALL )setup_force_cpu_cap (X86_FEATURE_IBRS_ENHANCED ); -if (!cpu_matches (NO_MDS ) && !(ia32_cap &ARCH_CAP_MDS_NO )) { +if (!cpu_matches (NO_MDS , cpu_vuln_whitelist) && + !(ia32_cap &ARCH_CAP_MDS_NO )) {setup_force_cpu_bug (X86_BUG_MDS ); -if (cpu_matches (MSBDS_ONLY )) +if (cpu_matches (MSBDS_ONLY , cpu_vuln_whitelist))setup_force_cpu_bug (X86_BUG_MSBDS_ONLY ); } -if (!cpu_matches (NO_SWAPGS )) +if (!cpu_matches (NO_SWAPGS , cpu_vuln_whitelist))setup_force_cpu_bug (X86_BUG_SWAPGS );/*
@@ -1139,7 +1142,7 @@ static void __init cpu_set_bug_bits(struct cpuinfo_x86 *c) (ia32_cap & ARCH_CAP_TSX_CTRL_MSR)))setup_force_cpu_bug (X86_BUG_TAA ); -if (cpu_matches (NO_MELTDOWN )) +if (cpu_matches (NO_MELTDOWN , cpu_vuln_whitelist))return ;/* Rogue Data Cache Load? No! */
@@ -1148,7 +1151,7 @@ static void __init cpu_set_bug_bits(struct cpuinfo_x86 *c)setup_force_cpu_bug (X86_BUG_CPU_MELTDOWN ); -if (cpu_matches (NO_L1TF )) +if (cpu_matches (NO_L1TF , cpu_vuln_whitelist))return ;setup_force_cpu_bug (X86_BUG_L1TF );
Signature · 1 line
2.17.1
PATCH 3/4 v6 more sampling fun 3
In reply to mark grossFrom: mark gross <[email protected]>
Subject: [PATCH 3/4] x86/speculation: Special Register Buffer Data Sampling
(SRBDS) mitigation control.
SRBDS is an MDS-like speculative side channel that can leak bits from the RNG across cores and threads. New microcode serializes the processor access during the execution of RDRAND and RDSEED. This ensures that the shared buffer is overwritten before it is released for reuse.
While it is present on all affected CPU models, the microcode mitigation is not needed on models that enumerate ARCH_CAPABILITIES[MDS_NO] in the cases where TSX is not supported or has been disabled with TSX_CTRL.
The mitigation is activated by default on affected processors and it increases latency for RDRAND and RDSEED instructions. Among other effects this will reduce throughput from /dev/urandom.
* enable administrator to configure the mitigation off when desired using either mitigations=off or srbds=off.
* export vulnerability status via sysfs
* rename file scoped macros to apply for non-whitelist table initializations.
--- .../ABI/testing/sysfs-devices-system-cpu | 1 + .../admin-guide/kernel-parameters.txt | 20 +++ arch/x86/include/asm/cpufeatures.h | 2 + arch/x86/include/asm/msr-index.h | 4 + arch/x86/kernel/cpu/bugs.c | 116 ++++++++++++++++++ arch/x86/kernel/cpu/common.c | 43 +++++++ arch/x86/kernel/cpu/cpu.h | 2 + drivers/base/cpu.c | 8 ++ 8 files changed, 196 insertions(+)
Documentation/ABI/testing/sysfs-devices-system-cpu +1 -0
@@ -492,6 +492,7 @@ What: /sys/devices/system/cpu/vulnerabilities /sys/devices/system/cpu/vulnerabilities/spec_store_bypass /sys/devices/system/cpu/vulnerabilities/l1tf /sys/devices/system/cpu/vulnerabilities/mds + /sys/devices/system/cpu/vulnerabilities/srbds /sys/devices/system/cpu/vulnerabilities/tsx_async_abort /sys/devices/system/cpu/vulnerabilities/itlb_multihit Date: January 2018
Documentation/admin-guide/kernel-parameters.txt +20 -0
@@ -4753,6 +4753,26 @@ the kernel will oops in either "warn" or "fatal" mode. + srbds= [X86,INTEL] + Control mitigation for the Special Register + Buffer Data Sampling (SRBDS) mitigation. + + Certain CPUs are vulnerable to an MDS-like + exploit which can leak bits from the random + number generator. + + By default, this issue is mitigated by + microcode. However, the microcode fix can cause + the RDRAND and RDSEED instructions to become + much slower. Among other effects, this will + result in reduced throughput from /dev/urandom. + + The microcode mitigation can be disabled with + the following option: + + off: Disable mitigation and remove + performance impact to RDRAND and RDSEED + srcutree.counter_wrap_check [KNL] Specifies how frequently to check for grace-period sequence counter wrap for the
arch/x86/include/asm/cpufeatures.h +2 -0
@@ -362,6 +362,7 @@#define X86_FEATURE_AVX512_4FMAPS (18*32+ 3)/* AVX-512 Multiply Accumulation Single precision */ #define X86_FEATURE_FSRM (18*32+ 4)/* Fast Short Rep Mov */ #define X86_FEATURE_AVX512_VP2INTERSECT (18*32+ 8)/* AVX-512 Intersect for D/Q */ +#define X86_FEATURE_SRBDS_CTRL (18*32+ 9)/* "" SRBDS mitigation MSR available */ #define X86_FEATURE_MD_CLEAR (18*32+10)/* VERW clears CPU buffers */ #define X86_FEATURE_TSX_FORCE_ABORT (18*32+13)/* "" TSX_FORCE_ABORT */ #define X86_FEATURE_PCONFIG (18*32+18)/* Intel PCONFIG */
@@ -407,5 +408,6 @@#define X86_BUG_SWAPGS X86_BUG(21)/* CPU is affected by speculation through SWAPGS */ #define X86_BUG_TAA X86_BUG(22)/* CPU is affected by TSX Async Abort(TAA) */ #define X86_BUG_ITLB_MULTIHIT X86_BUG(23)/* CPU may incur MCE during certain page attribute changes */ +#define X86_BUG_SRBDS X86_BUG(24)/* CPU may leak RNG bits if not mitigated */ #endif /* _ASM_X86_CPUFEATURES_H */
arch/x86/include/asm/msr-index.h +4 -0
@@ -128,6 +128,10 @@#define TSX_CTRL_RTM_DISABLE BIT(0)/* Disable RTM feature */ #define TSX_CTRL_CPUID_CLEAR BIT(1)/* Disable TSX enumeration */ +/* SRBDS support */ +#define MSR_IA32_MCU_OPT_CTRL 0x00000123 +#define RNGDS_MITG_DIS BIT(0) +#define MSR_IA32_SYSENTER_CS 0x00000174#define MSR_IA32_SYSENTER_ESP 0x00000175#define MSR_IA32_SYSENTER_EIP 0x00000176
arch/x86/kernel/cpu/bugs.c +116 -0
150 lines not on this page · raw message
arch/x86/kernel/cpu/common.c +43 -0
61 lines not on this page · raw message
arch/x86/kernel/cpu/cpu.h +2 -0
@@ -44,6 +44,8 @@ struct _tlb_table {extern const struct cpu_dev *const __x86_cpu_dev_start[], *const __x86_cpu_dev_end[]; +void update_srbds_msr (void ); +#ifdef CONFIG_CPU_SUP_INTEL enum tsx_ctrl_states { TSX_CTRL_ENABLE,
drivers/base/cpu.c +8 -0
@@ -562,6 +562,12 @@ ssize_t __weak cpu_show_itlb_multihit(struct device *dev,return sprintf (buf,"Not affected\n" ); } +ssize_t __weak cpu_show_srbds (struct device *dev, +struct device_attribute *attr,char *buf) +{ +return sprintf (buf,"Not affected\n" ); +} + static DEVICE_ATTR(meltdown, 0444, cpu_show_meltdown, NULL); static DEVICE_ATTR(spectre_v1, 0444, cpu_show_spectre_v1, NULL); static DEVICE_ATTR(spectre_v2, 0444, cpu_show_spectre_v2, NULL);
@@ -570,6 +576,7 @@ static DEVICE_ATTR(l1tf, 0444, cpu_show_l1tf, NULL); static DEVICE_ATTR(mds, 0444, cpu_show_mds, NULL); static DEVICE_ATTR(tsx_async_abort, 0444, cpu_show_tsx_async_abort, NULL); static DEVICE_ATTR(itlb_multihit, 0444, cpu_show_itlb_multihit, NULL); +static DEVICE_ATTR(srbds, 0444, cpu_show_srbds, NULL);static struct attribute *cpu_root_vulnerabilities_attrs[] = { &dev_attr_meltdown.attr,
@@ -580,6 +587,7 @@ static struct attribute *cpu_root_vulnerabilities_attrs[] = { &dev_attr_mds.attr, &dev_attr_tsx_async_abort.attr, &dev_attr_itlb_multihit.attr, + &dev_attr_srbds.attr, NULL };
Signature · 1 line
2.17.1
PATCH 4/4 v6 more sampling fun 4
In reply to mark grossFrom: mark gross <[email protected]>
Subject: [PATCH 4/4] x86/speculation: SRBDS vulnerability and mitigation
documentation
Add documentation for the SRBDS vulnerability and its mitigation.
--- Documentation/admin-guide/hw-vuln/index.rst | 1 + .../special-register-buffer-data-sampling.rst | 148 ++++++++++++++++++ 2 files changed, 149 insertions(+) create mode 100644 Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst
Documentation/admin-guide/hw-vuln/index.rst +1 -0
@@ -14,3 +14,4 @@ are configurable at compile, boot or run time. mds tsx_async_abort multihit.rst + special-register-buffer-data-sampling.rst
Documentation/admin-guide/hw-vuln/special-register-buffer-data-sampling.rst new file +148 -0
148 lines not on this page · raw message
Signature · 1 line
2.17.1
Re: [PATCH 3/4] v6 more sampling fun 3
In reply to mark grossMark,
Quoted 8 lines · speck for mark gross
(quoted diff) +8
-0+#define VULNBL_INTEL_STEPPING(model, steppings, issues) \ + X86_MATCH_VENDOR_FAM_MODEL_STEPPING_FEATURE(INTEL, 6, \ + INTEL_FAM6_##model, steppings, \ + X86_FEATURE_ANY, issues) + +#define SRBDS BIT(0) + +static const struct x86_cpu_id cpu_vuln_blacklist[] __initconst = {
Review
Quoted 10 lines
(quoted diff) +10
-0+ VULNBL_INTEL_STEPPING(IVYBRIDGE, X86_STEPPING_ANY, SRBDS), + VULNBL_INTEL_STEPPING(HASWELL, X86_STEPPING_ANY, SRBDS), + VULNBL_INTEL_STEPPING(HASWELL_L, X86_STEPPING_ANY, SRBDS), + VULNBL_INTEL_STEPPING(HASWELL_G, X86_STEPPING_ANY, SRBDS), + VULNBL_INTEL_STEPPING(BROADWELL_G, X86_STEPPING_ANY, SRBDS), + VULNBL_INTEL_STEPPING(BROADWELL, X86_STEPPING_ANY, SRBDS), + VULNBL_INTEL_STEPPING(SKYLAKE_L, X86_STEPPING_ANY, SRBDS), + VULNBL_INTEL_STEPPING(SKYLAKE, X86_STEPPING_ANY, SRBDS), + VULNBL_INTEL_STEPPING(KABYLAKE_L, GENMASK(0xC, 0), SRBDS), /*06_8E steppings <=C*/ + VULNBL_INTEL_STEPPING(KABYLAKE, GENMASK(0xD, 0), SRBDS), /*06_9E steppings <=D*/
Please don't use these horrible tail comments. Also the 06_NN part is of that comment is not helpful either. Please make stuff self explaining, i.e.
#define X86_STEPPINGS(mins, maxs) GENMASK(maxs, mins)
and use that in the blacklist:
VULNBL_INTEL_STEPPING(KABYLAKE_L, X86_STEPPINGS(0, 0xC), SRBDS), VULNBL_INTEL_STEPPING(KABYLAKE, X86_STEPPINGS(0, 0xD), SRBDS),
which is pretty obvious, right?
Quoted 13 lines
static bool
__initcpu_matches(unsigned long which, const struct x86_cpu_id *table){(quoted diff) +6
-0const struct x86_cpu_id *m = x86_match_cpu(table);@@ -1142,6 +1163,27 @@ static void __init cpu_set_bug_bits(struct cpuinfo_x86 *c) (ia32_cap & ARCH_CAP_TSX_CTRL_MSR))) setup_force_cpu_bug(X86_BUG_TAA); + if (cpu_matches(SRBDS, cpu_vuln_blacklist)) { + /* + * Some parts on the list don't have RDRAND or RDSEED. Make sure + * they show as "Not affected". + */ + if (!cpu_has(c, X86_FEATURE_RDRAND) &&
Review static void __init cpu_set_bug_bits(struct cpuinfo_x86 *c) · line 1181
Quoted 10 lines
(quoted diff) +10
-0+ !cpu_has(c, X86_FEATURE_RDSEED)) + goto srbds_not_affected; + /* + * Parts that have TSX fused off and are not affected by MDS + * are not affected by SRBDS. TSX_CTRL is only enumerated on + * parts where TSX fused on. + */ + if ((ia32_cap & ARCH_CAP_MDS_NO) && + !(ia32_cap & ARCH_CAP_TSX_CTRL_MSR)) + goto srbds_not_affected;
This is confusing vs. the documentation patch:
Review static void __init cpu_set_bug_bits(struct cpuinfo_x86 *c) · line 1187
Quoted 6 lines
(quoted diff) +6
-0+ Kabylake_L 06_8EH 0xB only if TSX is enabled + Kabylake_L 06_8EH 0xC only if TSX is enabled + + Kabylake 06_9EH <=B + Kabylake 06_9EH 0xC only if TSX is enabled + Kabylake 06_9EH 0xD only if TSX is enabled
IOW, this is only true for particular steppings of Kabylake[_L]
It might well be the case that exactly these steppings have MDS_NO and might have TSX fused off, but can we pretty please make this very explicit?
VULNBL_INTEL_STEPPING(KABYLAKE_L, X86_STEPPINGS(0x0, 0xA), SRBDS), VULNBL_INTEL_STEPPING(KABYLAKE_L, X86_STEPPINGS(0xB, 0xC), SRBDS_IF_TSX), VULNBL_INTEL_STEPPING(KABYLAKE, X86_STEPPINGS(0x0, 0xB), SRBDS), VULNBL_INTEL_STEPPING(KABYLAKE, X86_STEPPINGS(0xC, 0xD), SRBDS_IF_TSX),
and make the check for SRBDS_IF_TSX & TSX enabled in the code? You surely figure out how to define SRBDS_IF_TSX so the SRBDS match works for both :)
Thanks,
tglx
[MODERATED] Re: [PATCH 3/4] v6 more sampling fun 3
Quoted 17 lines · speck for Thomas Gleixner
Mark,
Quoted 14 lines · speck for mark gross
(quoted diff) +14
-0+#define VULNBL_INTEL_STEPPING(model, steppings, issues) \ + X86_MATCH_VENDOR_FAM_MODEL_STEPPING_FEATURE(INTEL, 6, \ + INTEL_FAM6_##model, steppings, \ + X86_FEATURE_ANY, issues) + +#define SRBDS BIT(0) + +static const struct x86_cpu_id cpu_vuln_blacklist[] __initconst = { + VULNBL_INTEL_STEPPING(IVYBRIDGE, X86_STEPPING_ANY, SRBDS), + VULNBL_INTEL_STEPPING(HASWELL, X86_STEPPING_ANY, SRBDS), + VULNBL_INTEL_STEPPING(HASWELL_L, X86_STEPPING_ANY, SRBDS), + VULNBL_INTEL_STEPPING(HASWELL_G, X86_STEPPING_ANY, SRBDS), + VULNBL_INTEL_STEPPING(BROADWELL_G, X86_STEPPING_ANY, SRBDS), + VULNBL_INTEL_STEPPING(BROADWELL, X86_STEPPING_ANY, SRBDS),
Quoted 8 lines
Review
Quoted 4 lines
(quoted diff) +4
-0+ VULNBL_INTEL_STEPPING(SKYLAKE_L, X86_STEPPING_ANY, SRBDS), + VULNBL_INTEL_STEPPING(SKYLAKE, X86_STEPPING_ANY, SRBDS), + VULNBL_INTEL_STEPPING(KABYLAKE_L, GENMASK(0xC, 0), SRBDS), /*06_8E steppings <=C*/ + VULNBL_INTEL_STEPPING(KABYLAKE, GENMASK(0xD, 0), SRBDS), /*06_9E steppings <=D*/Please don't use these horrible tail comments. Also the 06_NN part is of that comment is not helpful either. Please make stuff self explaining, i.e.
tail comments gone.
Quoted 1 line
#define
X86_STEPPINGS(mins, maxs)GENMASK(maxs, mins)
Adding this to the patch where I introduced steppings to x86_cpu_id.
Quoted 7 lines
and use that in the blacklist:
VULNBL_INTEL_STEPPING(KABYLAKE_L, X86_STEPPINGS(0, 0xC), SRBDS), VULNBL_INTEL_STEPPING(KABYLAKE, X86_STEPPINGS(0, 0xD), SRBDS),which is pretty obvious, right?
right!
Quoted 17 lines
Quoted 17 lines
static bool
__initcpu_matches(unsigned long which, const struct x86_cpu_id *table){(quoted diff) +10
-0const struct x86_cpu_id *m = x86_match_cpu(table);@@ -1142,6 +1163,27 @@ static void __init cpu_set_bug_bits(struct cpuinfo_x86 *c) (ia32_cap & ARCH_CAP_TSX_CTRL_MSR))) setup_force_cpu_bug(X86_BUG_TAA); + if (cpu_matches(SRBDS, cpu_vuln_blacklist)) { + /* + * Some parts on the list don't have RDRAND or RDSEED. Make sure + * they show as "Not affected". + */ + if (!cpu_has(c, X86_FEATURE_RDRAND) && + !cpu_has(c, X86_FEATURE_RDSEED)) + goto srbds_not_affected; + /* + * Parts that have TSX fused off and are not affected by MDS
Quoted 8 lines
Review static void __init cpu_set_bug_bits(struct cpuinfo_x86 *c) · line 1181
Quoted 6 lines
(quoted diff) +6
-0+ * are not affected by SRBDS. TSX_CTRL is only enumerated on + * parts where TSX fused on. + */ + if ((ia32_cap & ARCH_CAP_MDS_NO) && + !(ia32_cap & ARCH_CAP_TSX_CTRL_MSR)) + goto srbds_not_affected;This is confusing vs. the documentation patch:
I'll try to make it better.
attempt made. I will post it Monday.
Quoted 7 lines
Review static void __init cpu_set_bug_bits(struct cpuinfo_x86 *c) · line 1187
Quoted 6 lines
(quoted diff) +6
-0+ Kabylake_L 06_8EH 0xB only if TSX is enabled + Kabylake_L 06_8EH 0xC only if TSX is enabled + + Kabylake 06_9EH <=B + Kabylake 06_9EH 0xC only if TSX is enabled + Kabylake 06_9EH 0xD only if TSX is enabled
Quoted 5 lines
IOW, this is only true for particular steppings of Kabylake[_L]
It might well be the case that exactly these steppings have
MDS_NOand might have TSX fused off, but can we pretty please make this very explicit?
I thought that might be the case to but, I was told that the choice to fuse stuff off is made by marketing per bin splits. i.e. Kabylake i3's or Celerons will get the features fused off not all kabylakes or specific steppings.
Quoted 8 lines
VULNBL_INTEL_STEPPING(KABYLAKE_L, X86_STEPPINGS(0x0, 0xA), SRBDS), VULNBL_INTEL_STEPPING(KABYLAKE_L, X86_STEPPINGS(0xB, 0xC), SRBDS_IF_TSX), VULNBL_INTEL_STEPPING(KABYLAKE, X86_STEPPINGS(0x0, 0xB), SRBDS), VULNBL_INTEL_STEPPING(KABYLAKE, X86_STEPPINGS(0xC, 0xD), SRBDS_IF_TSX),and make the check for
SRBDS_IF_TSX& TSX enabled in the code? You surely figure out how to defineSRBDS_IF_TSXso the SRBDS match works for both :)
In fact that was somewhat how I hand the code initialy implemented treating the "onyl if tsx" case as a separate match bit.
Added SRBDS_IF_TSX to make the code more self documenting and more closely match documentation.
done.
Quoted 4 lines
Thanks,
tglx
Thank you!
--mark
A message that is not in the archive (sent off-list or not yet received): 585879ee414f627d2f5b8b5d63a13e35c9aa0688.158645=?utf-8?q?7409?=
[email protected]
[MODERATED] Re: [PATCH 4/4] v6 more sampling fun 4
On Thu, 2020-01-30 at 11:12 -0800, speck for mark gross wrote:
[...]
Quoted 6 lines
(quoted diff) +6
-0+The possible values contained in this file are: + + ============================== ============================================= + Not affected Processor not vulnerable + Vulnerable Processor vulnerable and mitigation disabled + Vulnerable: no microcode Processor vulnerable and microcode is missing
Review
Quoted 10 lines
(quoted diff) +10
-0+ mitigation + Mitigated Processor is vulnerable and mitigation is in + effect. + Not affected (TSX disabled) Processor is only vulnerable when TSX is + enabled while this system was booted with TSX + disabled. + Unknown Running on virtual guest processor that is + affected but with no way to know if host + processor is mitigated or vulnerable. + ============================== =============================================
[...]
The values in this table haven't been updated along with the code.
Ben.
Signature · 2 lines
Ben Hutchings Knowledge is power. France is bacon.
[MODERATED] Re: [PATCH 4/4] v6 more sampling fun 4
Quoted 20 lines · speck for Ben Hutchings
On Thu, 2020-01-30 at 11:12 -0800, speck for mark gross wrote:
[...]Quoted 16 lines
(quoted diff) +16
-0+The possible values contained in this file are: + + ============================== ============================================= + Not affected Processor not vulnerable + Vulnerable Processor vulnerable and mitigation disabled + Vulnerable: no microcode Processor vulnerable and microcode is missing + mitigation + Mitigated Processor is vulnerable and mitigation is in + effect. + Not affected (TSX disabled) Processor is only vulnerable when TSX is + enabled while this system was booted with TSX + disabled. + Unknown Running on virtual guest processor that is + affected but with no way to know if host + processor is mitigated or vulnerable. + ============================== =============================================[...]
Quoted 1 line
The values in this table haven't been updated along with the code.
updated Mitigated line to match exactly with code. --mark
Quoted 7 lines
Ben.
Signature · 2 lines
Ben Hutchings Knowledge is power. France is bacon.