* [MODERATED] [PATCH v2 0/2] v2: more sampling fun 0
@ 2020-02-24 21:45 mark gross
2020-01-16 22:16 ` [MODERATED] [PATCH v2 2/2] v2: more sampling fun 2 mark gross
` (5 more replies)
0 siblings, 6 replies; 30+ messages in thread
From: mark gross @ 2020-02-24 21:45 UTC (permalink / raw)
To: speck
From: mark gross <mgross@linux.intel.com>
Subject: [PATCH v2 0/2] more sampling fun
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 bechmark of calling RDRAND many times shows a 10x slowdown.
This patch set enables kernel command line control of this mitigation and
exports vulnerability and mitigation status.
This patch set includes 2 patches: The first patch updates cpu_vuln_whitelist
with support for a 16 bit field for enumerating based on stepping as well as
vendor, family, model.
The second patch enables the command line control of the mitigation as well as
the sysfs export of vulnerability status.
The documentation patch is pending on the official white paper to be complete
such that I can make sure the in tree documentation is consistent with the
white paper.
The microcode defaults to enabling the mitigation.
changes since last version:
use GENMASK is most places recomended by Ben.
Fixed sysfs reporting issue associated with TSX=on case.
The next version is pending white paper finalization. The disclosure of this
issues is coming in May.
mark gross (2):
Add capability to specify a range of steppings in the vulnerability
white list structure.
WIP SRBDS mitigation enabling.
arch/x86/include/asm/cpu_device_id.h | 12 ++++
arch/x86/include/asm/cpufeatures.h | 3 +
arch/x86/include/asm/msr-index.h | 4 ++
arch/x86/kernel/cpu/bugs.c | 84 ++++++++++++++++++++++++++++
arch/x86/kernel/cpu/common.c | 52 ++++++++++++-----
arch/x86/kernel/cpu/cpu.h | 10 ++++
arch/x86/kernel/cpu/intel.c | 2 +
arch/x86/kernel/cpu/match.c | 26 +++++++++
drivers/base/cpu.c | 8 +++
9 files changed, 187 insertions(+), 14 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 30+ messages in thread* [MODERATED] [PATCH v2 2/2] v2: more sampling fun 2 2020-02-24 21:45 [MODERATED] [PATCH v2 0/2] v2: more sampling fun 0 mark gross @ 2020-01-16 22:16 ` mark gross 2020-02-06 22:11 ` [MODERATED] [PATCH v2 1/2] v2: more sampling fun 1 mark gross ` (4 subsequent siblings) 5 siblings, 0 replies; 30+ messages in thread From: mark gross @ 2020-01-16 22:16 UTC (permalink / raw) To: speck From: mark gross <mgross@linux.intel.com> Subject: [PATCH v2 2/2] WIP SRBDS mitigation enabling. From: mark gross <mgross@linux.intel.com> Subject: [PATCH v2 2/2] WIP SRBDS mitigation enabling. 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 ensures that the shared buffer is overwritten before it is released for reuse. We subdivide processors that are vulnerable to SRBDS into two classes: X86_BUG_SRBDS: models that are vulnerable X86_BUG_SRBDS_TSX: models only vulnerable when TSX is enabled. The latter are not vulnerable to SRBDS if TSX is disabled on all cores. The mitigation is activated by default on affected processors and it slows down /dev/urandom. The latency of RDRAND and RDSEED instructions is increased by 10x. We don't expect this to be noticeable in most cases. This patch: * enables administrator to configure the mitigation off when desired using either mitigations=off or srbds_mitgation=off. * exports vulnerability status via sysfs Signed-off-by: mark gross <mgross@linux.intel.com> Reviewed-by: tony luck <tony.luck@intel.com> --- arch/x86/include/asm/cpufeatures.h | 3 ++ arch/x86/include/asm/msr-index.h | 4 ++ arch/x86/kernel/cpu/bugs.c | 84 ++++++++++++++++++++++++++++++ arch/x86/kernel/cpu/common.c | 24 +++++++++ arch/x86/kernel/cpu/cpu.h | 10 ++++ arch/x86/kernel/cpu/intel.c | 2 + drivers/base/cpu.c | 8 +++ 7 files changed, 135 insertions(+) diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h index f3327cb56edf..e7d032542d63 100644 --- a/arch/x86/include/asm/cpufeatures.h +++ b/arch/x86/include/asm/cpufeatures.h @@ -360,6 +360,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 */ @@ -404,5 +405,7 @@ #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 */ +#define X86_BUG_SRBDS_TSX X86_BUG(25) /* CPU may leak RNG bits if not mitigated when TSX is enabled */ #endif /* _ASM_X86_CPUFEATURES_H */ diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h index d5e517d1c3dd..806f4dcb64ad 100644 --- a/arch/x86/include/asm/msr-index.h +++ b/arch/x86/include/asm/msr-index.h @@ -119,6 +119,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 SRBDS_MITG_DIS BIT(0) + #define MSR_IA32_SYSENTER_CS 0x00000174 #define MSR_IA32_SYSENTER_ESP 0x00000175 #define MSR_IA32_SYSENTER_EIP 0x00000176 diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c index ed54b3b21c39..472cc0f214ff 100644 --- a/arch/x86/kernel/cpu/bugs.c +++ b/arch/x86/kernel/cpu/bugs.c @@ -41,6 +41,7 @@ static void __init l1tf_select_mitigation(void); static void __init mds_select_mitigation(void); static void __init mds_print_mitigation(void); static void __init taa_select_mitigation(void); +static void __init srbds_select_mitigation(void); /* The base value of the SPEC_CTRL MSR that always has to be preserved. */ u64 x86_spec_ctrl_base; @@ -108,6 +109,7 @@ void __init check_bugs(void) l1tf_select_mitigation(); mds_select_mitigation(); taa_select_mitigation(); + srbds_select_mitigation(); /* * As MDS and TAA mitigations are inter-related, print MDS @@ -397,6 +399,69 @@ static int __init tsx_async_abort_parse_cmdline(char *str) } early_param("tsx_async_abort", tsx_async_abort_parse_cmdline); +#undef pr_fmt +#define pr_fmt(fmt) "SRBDS: " fmt + +enum srbds_mitigations srbds_mitigation __ro_after_init = SRBDS_MITIGATION_FULL; +static const char * const srbds_strings[] = { + [SRBDS_MITIGATION_OFF] = "Vulnerable", + [SRBDS_MITIGATION_UCODE_NEEDED] = "Vulnerable: no microcode", + [SRBDS_MITIGATION_FULL] = "Mitigation: bus lock when using RDRAND or RDSEED", +}; + +void srbds_configure_mitigation(void) +{ + u64 mcu_ctrl; + + if (!boot_cpu_has_bug(X86_BUG_SRBDS) && !boot_cpu_has_bug(X86_BUG_SRBDS_TSX)) + return; + + if (!boot_cpu_has(X86_FEATURE_SRBDS_CTRL)) + return; + + rdmsrl(MSR_IA32_MCU_OPT_CTRL, mcu_ctrl); + if (srbds_mitigation == SRBDS_MITIGATION_FULL) + mcu_ctrl &= ~SRBDS_MITG_DIS; + else if (srbds_mitigation == SRBDS_MITIGATION_OFF) + mcu_ctrl |= SRBDS_MITG_DIS; + + if (boot_cpu_has_bug(X86_BUG_SRBDS_TSX) && !boot_cpu_has(X86_FEATURE_RTM)) + mcu_ctrl |= SRBDS_MITG_DIS; + + wrmsrl(MSR_IA32_MCU_OPT_CTRL, mcu_ctrl); +} + +static void __init srbds_select_mitigation(void) +{ + if (!boot_cpu_has_bug(X86_BUG_SRBDS) && + !boot_cpu_has_bug(X86_BUG_SRBDS_TSX)) + return; + + if (cpu_mitigations_off()) { + srbds_mitigation = SRBDS_MITIGATION_OFF; + goto out; + } + + if (!boot_cpu_has(X86_FEATURE_SRBDS_CTRL)) + srbds_mitigation = SRBDS_MITIGATION_UCODE_NEEDED; + +out: + srbds_configure_mitigation(); +} + +static int __init srbds_parse_cmdline(char *str) +{ + if (!str) + return -EINVAL; + + if (!strcmp(str, "off")) + srbds_mitigation = SRBDS_MITIGATION_OFF; + + return 0; +} + +early_param("srbds_mitigation", srbds_parse_cmdline); + #undef pr_fmt #define pr_fmt(fmt) "Spectre V1 : " fmt @@ -1528,6 +1593,11 @@ static char *ibpb_state(void) return ""; } +static ssize_t srbds_show_state(char *buf) +{ + return sprintf(buf, "%s\n", srbds_strings[srbds_mitigation]); +} + static ssize_t cpu_show_common(struct device *dev, struct device_attribute *attr, char *buf, unsigned int bug) { @@ -1572,6 +1642,10 @@ static ssize_t cpu_show_common(struct device *dev, struct device_attribute *attr case X86_BUG_ITLB_MULTIHIT: return itlb_multihit_show_state(buf); + case X86_BUG_SRBDS: + case X86_BUG_SRBDS_TSX: + return srbds_show_state(buf); + default: break; } @@ -1618,4 +1692,14 @@ ssize_t cpu_show_itlb_multihit(struct device *dev, struct device_attribute *attr { return cpu_show_common(dev, attr, buf, X86_BUG_ITLB_MULTIHIT); } + +ssize_t cpu_show_special_register_data_sampling(struct device *dev, struct device_attribute *attr, char *buf) +{ + if (boot_cpu_has_bug(X86_BUG_SRBDS)) + return cpu_show_common(dev, attr, buf, X86_BUG_SRBDS); + else if (boot_cpu_has_bug(X86_BUG_SRBDS_TSX) && boot_cpu_has(X86_FEATURE_RTM)) + return cpu_show_common(dev, attr, buf, X86_BUG_SRBDS_TSX); + else + return sprintf(buf, "Not affected\n"); +} #endif diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c index c0519be5f563..c9bee58f1e25 100644 --- a/arch/x86/kernel/cpu/common.c +++ b/arch/x86/kernel/cpu/common.c @@ -1007,6 +1007,8 @@ static void identify_cpu_without_cpuid(struct cpuinfo_x86 *c) #define NO_SWAPGS BIT(6) #define NO_ITLB_MULTIHIT BIT(7) #define NO_SPECTRE_V2 BIT(8) +#define SRBDS BIT(9) +#define SRBDS_TSX BIT(10) #define VULNWL(_vendor, _family, _model, __stepping, _whitelist) \ {{ X86_VENDOR_##_vendor, _family, _model, X86_FEATURE_ANY, _whitelist }, __stepping} @@ -1014,6 +1016,9 @@ static void identify_cpu_without_cpuid(struct cpuinfo_x86 *c) #define VULNWL_INTEL(model, whitelist) \ VULNWL(INTEL, 6, INTEL_FAM6_##model, X86_STEPPING_ANY, whitelist) +#define VULNWL_INTEL_STEPPING(model, stepping, whitelist) \ + VULNWL(INTEL, 6, INTEL_FAM6_##model, stepping, whitelist) + #define VULNWL_AMD(family, whitelist) \ VULNWL(AMD, family, X86_MODEL_ANY, X86_STEPPING_ANY, whitelist) @@ -1042,6 +1047,19 @@ static const struct x86_cpu_id_ext cpu_vuln_whitelist[] __initconst = { VULNWL_INTEL(CORE_YONAH, NO_SSB), + VULNWL_INTEL(IVYBRIDGE, SRBDS), /*06_3A*/ + VULNWL_INTEL(HASWELL, SRBDS), /*06_3C*/ + VULNWL_INTEL(HASWELL_L, SRBDS), /*06_45*/ + VULNWL_INTEL(HASWELL_G, SRBDS), /*06_46*/ + VULNWL_INTEL(BROADWELL_G, SRBDS), /*06_47*/ + VULNWL_INTEL(BROADWELL, SRBDS), /*06_3D*/ + VULNWL_INTEL(SKYLAKE_L, SRBDS), /*06_4E*/ + VULNWL_INTEL(SKYLAKE, SRBDS), /*06_5E*/ + VULNWL_INTEL_STEPPING(KABYLAKE_L, GENMASK(0xA,0), SRBDS), /*06_8E steppings <=A*/ + VULNWL_INTEL_STEPPING(KABYLAKE_L, GENMASK(0xC,0xB), SRBDS_TSX), /*06_8E stepping = 0xB|0xC if TSX enabled*/ + VULNWL_INTEL_STEPPING(KABYLAKE, GENMASK(0xB,0), SRBDS), /*06_9E steppings <=B*/ + VULNWL_INTEL_STEPPING(KABYLAKE, GENMASK(0xD,0xC), SRBDS_TSX), /*06_9E stepping = 0xC|0xD if TSX enabled*/ + VULNWL_INTEL(ATOM_AIRMONT_MID, NO_L1TF | MSBDS_ONLY | NO_SWAPGS | NO_ITLB_MULTIHIT), VULNWL_INTEL(ATOM_AIRMONT_NP, NO_L1TF | NO_SWAPGS | NO_ITLB_MULTIHIT), @@ -1124,6 +1142,12 @@ static void __init cpu_set_bug_bits(struct cpuinfo_x86 *c) if (!cpu_matches(NO_SWAPGS)) setup_force_cpu_bug(X86_BUG_SWAPGS); + if (cpu_matches(SRBDS)) + setup_force_cpu_bug(X86_BUG_SRBDS); + + if (cpu_matches(SRBDS_TSX)) + setup_force_cpu_bug(X86_BUG_SRBDS_TSX); + /* * When the CPU is not mitigated for TAA (TAA_NO=0) set TAA bug when: * - TSX is supported or diff --git a/arch/x86/kernel/cpu/cpu.h b/arch/x86/kernel/cpu/cpu.h index 37fdefd14f28..f2b3fd4d4274 100644 --- a/arch/x86/kernel/cpu/cpu.h +++ b/arch/x86/kernel/cpu/cpu.h @@ -44,7 +44,17 @@ struct _tlb_table { extern const struct cpu_dev *const __x86_cpu_dev_start[], *const __x86_cpu_dev_end[]; +enum srbds_mitigations { + SRBDS_MITIGATION_OFF, + SRBDS_MITIGATION_UCODE_NEEDED, + SRBDS_MITIGATION_FULL, +}; + +extern __ro_after_init enum srbds_mitigations srbds_mitigation; +void srbds_configure_mitigation(void); + #ifdef CONFIG_CPU_SUP_INTEL + enum tsx_ctrl_states { TSX_CTRL_ENABLE, TSX_CTRL_DISABLE, diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c index be82cd5841c3..1b083a2a415b 100644 --- a/arch/x86/kernel/cpu/intel.c +++ b/arch/x86/kernel/cpu/intel.c @@ -684,6 +684,8 @@ static void init_intel(struct cpuinfo_x86 *c) tsx_enable(); if (tsx_ctrl_state == TSX_CTRL_DISABLE) tsx_disable(); + + srbds_configure_mitigation(); } #ifdef CONFIG_X86_32 diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c index 6265871a4af2..d69e094e790c 100644 --- a/drivers/base/cpu.c +++ b/drivers/base/cpu.c @@ -567,6 +567,12 @@ ssize_t __weak cpu_show_itlb_multihit(struct device *dev, return sprintf(buf, "Not affected\n"); } +ssize_t __weak cpu_show_special_register_data_sampling(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); @@ -575,6 +581,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(special_register_data_sampling, 0444, cpu_show_special_register_data_sampling, NULL); static struct attribute *cpu_root_vulnerabilities_attrs[] = { &dev_attr_meltdown.attr, @@ -585,6 +592,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_special_register_data_sampling.attr, NULL }; -- 2.17.1 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* [MODERATED] [PATCH v2 1/2] v2: more sampling fun 1 2020-02-24 21:45 [MODERATED] [PATCH v2 0/2] v2: more sampling fun 0 mark gross 2020-01-16 22:16 ` [MODERATED] [PATCH v2 2/2] v2: more sampling fun 2 mark gross @ 2020-02-06 22:11 ` mark gross 2020-02-25 16:55 ` [MODERATED] Re: [PATCH v2 0/2] v2: more sampling fun 0 Josh Poimboeuf ` (3 subsequent siblings) 5 siblings, 0 replies; 30+ messages in thread From: mark gross @ 2020-02-06 22:11 UTC (permalink / raw) To: speck From: mark gross <mgross@linux.intel.com> Subject: [PATCH v2 1/2] Add capability to specify a range of steppings in the vulnerability white list structure. From: mark gross <mgross@linux.intel.com> Subject: [PATCH v2 1/2] Add capability to specify a range of steppings in the Intel has produced processors with the same CPUID family+model. Code may need to check the stepping when programming model specific behavior. Add an API to allow easy specification of stepping or range of steppings with a 16 bit bitmask. Update cpu_vuln_whitelist using this new API. I implemented this in the way I did to avoid modifying x86_cpu_id as that structure is an exported ABI and any change would impact user mode code using the structure. Signed-off-by: mark gross <mgross@linux.intel.com> Reviewed-by: tony luck <tony.luck@intel.com> --- arch/x86/include/asm/cpu_device_id.h | 12 ++++++++++++ arch/x86/kernel/cpu/common.c | 28 ++++++++++++++-------------- arch/x86/kernel/cpu/match.c | 26 ++++++++++++++++++++++++++ 3 files changed, 52 insertions(+), 14 deletions(-) diff --git a/arch/x86/include/asm/cpu_device_id.h b/arch/x86/include/asm/cpu_device_id.h index 31c379c1da41..e9de5f6d784a 100644 --- a/arch/x86/include/asm/cpu_device_id.h +++ b/arch/x86/include/asm/cpu_device_id.h @@ -35,7 +35,19 @@ struct x86_cpu_desc { .x86_microcode_rev = (revision), \ } +/* + * Match a range of steppings + */ + +struct x86_cpu_id_ext { + struct x86_cpu_id id; + __u16 steppings; /* bit map of steppings to match against */ +}; + +#define X86_STEPPING_ANY GENMASK(15,0) + extern const struct x86_cpu_id *x86_match_cpu(const struct x86_cpu_id *match); +const struct x86_cpu_id_ext *x86_match_cpu_ext(const struct x86_cpu_id_ext *match); extern bool x86_cpu_has_min_microcode_rev(const struct x86_cpu_desc *table); #endif /* _ASM_X86_CPU_DEVICE_ID */ diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c index 52c9bfbbdb2a..c0519be5f563 100644 --- a/arch/x86/kernel/cpu/common.c +++ b/arch/x86/kernel/cpu/common.c @@ -1008,23 +1008,23 @@ static void identify_cpu_without_cpuid(struct cpuinfo_x86 *c) #define NO_ITLB_MULTIHIT BIT(7) #define NO_SPECTRE_V2 BIT(8) -#define VULNWL(_vendor, _family, _model, _whitelist) \ - { X86_VENDOR_##_vendor, _family, _model, X86_FEATURE_ANY, _whitelist } +#define VULNWL(_vendor, _family, _model, __stepping, _whitelist) \ + {{ X86_VENDOR_##_vendor, _family, _model, X86_FEATURE_ANY, _whitelist }, __stepping} #define VULNWL_INTEL(model, whitelist) \ - VULNWL(INTEL, 6, INTEL_FAM6_##model, whitelist) + VULNWL(INTEL, 6, INTEL_FAM6_##model, X86_STEPPING_ANY, whitelist) #define VULNWL_AMD(family, whitelist) \ - VULNWL(AMD, family, X86_MODEL_ANY, whitelist) + VULNWL(AMD, family, X86_MODEL_ANY, X86_STEPPING_ANY, whitelist) #define VULNWL_HYGON(family, whitelist) \ - VULNWL(HYGON, family, X86_MODEL_ANY, whitelist) + VULNWL(HYGON, family, X86_MODEL_ANY, X86_STEPPING_ANY, whitelist) -static const __initconst struct x86_cpu_id cpu_vuln_whitelist[] = { - VULNWL(ANY, 4, X86_MODEL_ANY, NO_SPECULATION), - VULNWL(CENTAUR, 5, X86_MODEL_ANY, NO_SPECULATION), - VULNWL(INTEL, 5, X86_MODEL_ANY, NO_SPECULATION), - VULNWL(NSC, 5, X86_MODEL_ANY, NO_SPECULATION), +static const struct x86_cpu_id_ext cpu_vuln_whitelist[] __initconst = { + VULNWL(ANY, 4, X86_MODEL_ANY, X86_STEPPING_ANY, NO_SPECULATION), + VULNWL(CENTAUR, 5, X86_MODEL_ANY, X86_STEPPING_ANY, NO_SPECULATION), + VULNWL(INTEL, 5, X86_MODEL_ANY, X86_STEPPING_ANY, NO_SPECULATION), + VULNWL(NSC, 5, X86_MODEL_ANY, X86_STEPPING_ANY, NO_SPECULATION), /* Intel Family 6 */ VULNWL_INTEL(ATOM_SALTWELL, NO_SPECULATION | NO_ITLB_MULTIHIT), @@ -1070,16 +1070,16 @@ static const __initconst struct x86_cpu_id cpu_vuln_whitelist[] = { VULNWL_HYGON(X86_FAMILY_ANY, NO_MELTDOWN | NO_L1TF | NO_MDS | NO_SWAPGS | NO_ITLB_MULTIHIT), /* Zhaoxin Family 7 */ - VULNWL(CENTAUR, 7, X86_MODEL_ANY, NO_SPECTRE_V2 | NO_SWAPGS), - VULNWL(ZHAOXIN, 7, X86_MODEL_ANY, NO_SPECTRE_V2 | NO_SWAPGS), + VULNWL(CENTAUR, 7, X86_MODEL_ANY, X86_STEPPING_ANY, NO_SPECTRE_V2 | NO_SWAPGS), + VULNWL(ZHAOXIN, 7, X86_MODEL_ANY, X86_STEPPING_ANY, NO_SPECTRE_V2 | NO_SWAPGS), {} }; static bool __init cpu_matches(unsigned long which) { - const struct x86_cpu_id *m = x86_match_cpu(cpu_vuln_whitelist); + const struct x86_cpu_id_ext *m = x86_match_cpu_ext(cpu_vuln_whitelist); - return m && !!(m->driver_data & which); + return m && !!(m->id.driver_data & which); } u64 x86_read_arch_cap_msr(void) diff --git a/arch/x86/kernel/cpu/match.c b/arch/x86/kernel/cpu/match.c index 6dd78d8235e4..118c503b1c36 100644 --- a/arch/x86/kernel/cpu/match.c +++ b/arch/x86/kernel/cpu/match.c @@ -49,6 +49,32 @@ const struct x86_cpu_id *x86_match_cpu(const struct x86_cpu_id *match) } EXPORT_SYMBOL(x86_match_cpu); +/* + * Extend x86_match_cpu to support matching a range of steppings. + */ +const struct x86_cpu_id_ext *x86_match_cpu_ext(const struct x86_cpu_id_ext *match) +{ + const struct x86_cpu_id_ext *m; + struct cpuinfo_x86 *c = &boot_cpu_data; + + for (m = match; m->id.vendor | m->id.family | m->id.model | m->id.feature; m++) { + if (m->id.vendor != X86_VENDOR_ANY && c->x86_vendor != m->id.vendor) + continue; + if (m->id.family != X86_FAMILY_ANY && c->x86 != m->id.family) + continue; + if (m->id.model != X86_MODEL_ANY && c->x86_model != m->id.model) + continue; + if (m->steppings != X86_STEPPING_ANY && + !(BIT(c->x86_stepping) & m->steppings)) + continue; + if (m->id.feature != X86_FEATURE_ANY && !cpu_has(c, m->id.feature)) + continue; + return m; + } + return NULL; +} +EXPORT_SYMBOL(x86_match_cpu_ext); + static const struct x86_cpu_desc * x86_match_cpu_with_stepping(const struct x86_cpu_desc *match) { -- 2.17.1 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* [MODERATED] Re: [PATCH v2 0/2] v2: more sampling fun 0 2020-02-24 21:45 [MODERATED] [PATCH v2 0/2] v2: more sampling fun 0 mark gross 2020-01-16 22:16 ` [MODERATED] [PATCH v2 2/2] v2: more sampling fun 2 mark gross 2020-02-06 22:11 ` [MODERATED] [PATCH v2 1/2] v2: more sampling fun 1 mark gross @ 2020-02-25 16:55 ` Josh Poimboeuf 2020-02-25 17:43 ` mark gross [not found] ` <5e5595e6.1c69fb81.69e80.2880SMTPIN_ADDED_BROKEN@mx.google.com> ` (2 subsequent siblings) 5 siblings, 1 reply; 30+ messages in thread From: Josh Poimboeuf @ 2020-02-25 16:55 UTC (permalink / raw) To: speck On Mon, Feb 24, 2020 at 01:45:10PM -0800, speck for mark gross wrote: > From: mark gross <mgross@linux.intel.com> > Subject: [PATCH v2 0/2] more sampling fun It looks like the actual patches didn't come through? -- Josh ^ permalink raw reply [flat|nested] 30+ messages in thread
* [MODERATED] Re: [PATCH v2 0/2] v2: more sampling fun 0 2020-02-25 16:55 ` [MODERATED] Re: [PATCH v2 0/2] v2: more sampling fun 0 Josh Poimboeuf @ 2020-02-25 17:43 ` mark gross 2020-02-25 20:47 ` Thomas Gleixner 0 siblings, 1 reply; 30+ messages in thread From: mark gross @ 2020-02-25 17:43 UTC (permalink / raw) To: speck On Tue, Feb 25, 2020 at 10:55:30AM -0600, speck for Josh Poimboeuf wrote: > On Mon, Feb 24, 2020 at 01:45:10PM -0800, speck for mark gross wrote: > > From: mark gross <mgross@linux.intel.com> > > Subject: [PATCH v2 0/2] more sampling fun > > It looks like the actual patches didn't come through? Heavy sigh.... I have nothing but badd luck with this. I updated my .gitconfig on the new workstation to use mgross@linux.intel.com and I git commit --ammend ed the commit I thought was blocked because it had an author of mark.gross@intel.com instead of the other one. I assume they are getting held up by something on the list server end but, I have no idea what I'm doing to piss it off. :( If anyone could help clue me in I'll try to correct whatever I'm doing wrong. The other two patches where sent using the same process: * git format-patch -o mail -n --to speck@linutronix.de --thread --cover-letter v5.6-rc3 * speckify-gitmail -s "more sampling fun" mail/ spec/ * for b in `ls`; do sendmail -t -i -f smtp.intel.com <$b; done --mark ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 0/2] v2: more sampling fun 0 2020-02-25 17:43 ` mark gross @ 2020-02-25 20:47 ` Thomas Gleixner 2020-02-25 21:51 ` [MODERATED] " mark gross 0 siblings, 1 reply; 30+ messages in thread From: Thomas Gleixner @ 2020-02-25 20:47 UTC (permalink / raw) To: speck speck for mark gross <speck@linutronix.de> writes: > On Tue, Feb 25, 2020 at 10:55:30AM -0600, speck for Josh Poimboeuf wrote: >> On Mon, Feb 24, 2020 at 01:45:10PM -0800, speck for mark gross wrote: >> > From: mark gross <mgross@linux.intel.com> >> > Subject: [PATCH v2 0/2] more sampling fun >> >> It looks like the actual patches didn't come through? > > Heavy sigh.... > > I have nothing but badd luck with this. I updated my .gitconfig on the new > workstation to use mgross@linux.intel.com and I git commit --ammend ed the > commit I thought was blocked because it had an author of mark.gross@intel.com > instead of the other one. I put an alias in, so both addresses work on the list. > I assume they are getting held up by something on the list server end but, I > have no idea what I'm doing to piss it off. :( > > If anyone could help clue me in I'll try to correct whatever I'm doing wrong. > The other two patches where sent using the same process: > > * git format-patch -o mail -n --to speck@linutronix.de --thread --cover-letter v5.6-rc3 > * speckify-gitmail -s "more sampling fun" mail/ spec/ > * for b in `ls`; do sendmail -t -i -f smtp.intel.com <$b; done Which looks about correct, but then the receiving end says: 2020-02-24 22:57:31 1j6LjH-00076x-B2 <= smtp.intel.com@mtg-dev.jf.intel.com H=mga11.intel.com [192.55.52.93] P=esmtps X=TLS1.2:DHE_RSA_AES_256_CBC_SHA256:256 S=3729 id=cover.1582580710.git.mgross@linux.intel.com 2020-02-24 22:57:32 1j6LjH-00076x-B2 => crypto-ml <speck@linutronix.de> R=local_user T=mail_spool 2020-02-24 22:57:32 1j6LjH-00076x-B2 Completed That's the cover letter. And then the other two seem to be broken: 2020-02-24 22:57:32 H=mga07.intel.com [134.134.136.100] sender verify fail for <smtp.intel.com@mtg-dev.jf.intel.com>: Unrouteable address 2020-02-24 22:57:32 H=mga07.intel.com [134.134.136.100] F=<smtp.intel.com@mtg-dev.jf.intel.com> rejected RCPT <speck@linutronix.de>: Sender verify failed 2020-02-24 22:57:32 H=mga06.intel.com [134.134.136.31] sender verify fail for <smtp.intel.com@mtg-dev.jf.intel.com>: Unrouteable address 2020-02-24 22:57:32 H=mga06.intel.com [134.134.136.31] F=<smtp.intel.com@mtg-dev.jf.intel.com> rejected RCPT <speck@linutronix.de>: Sender verify failed Now, looking at your sendmail command line: -t -i -f smtp.intel.com -f is the envelope sender address. smtp.intel.com is definitely not a valid envelope-from address, it's a hostname and it looks like sendmail magically makes that into: envelope-from <smtp.intel.com@mtg-dev.jf.intel.com> and that mtg-dev.jf.intel.com is probably your host name: mtg-dev . Jones Farm . intel.com which is bonkers to begin with because I'm pretty sure that nothing can send mail to that address. Now in the cover letter the From: field is correct: From: mark gross <mgross@linux.intel.com> but I'm pretty sure, that your other two mails are malformatted and sendmail throws your bonkers -f "address" at it which causes the sender verify to fail. And of course because your envelope-from is broken, the bounce (reject) info does not make it to you. May I recommend to check the mails in mail/ and spec/ before sending them out. You can also check them for a valid From: address after the fact of course :) BOFH excuse #208: Your mail is being routed through Germany ... and they're censoring us. ^ permalink raw reply [flat|nested] 30+ messages in thread
* [MODERATED] Re: [PATCH v2 0/2] v2: more sampling fun 0 2020-02-25 20:47 ` Thomas Gleixner @ 2020-02-25 21:51 ` mark gross 0 siblings, 0 replies; 30+ messages in thread From: mark gross @ 2020-02-25 21:51 UTC (permalink / raw) To: speck On Tue, Feb 25, 2020 at 09:47:10PM +0100, speck for Thomas Gleixner wrote: > speck for mark gross <speck@linutronix.de> writes: > > > On Tue, Feb 25, 2020 at 10:55:30AM -0600, speck for Josh Poimboeuf wrote: > >> On Mon, Feb 24, 2020 at 01:45:10PM -0800, speck for mark gross wrote: > >> > From: mark gross <mgross@linux.intel.com> > >> > Subject: [PATCH v2 0/2] more sampling fun > >> > >> It looks like the actual patches didn't come through? > > > > Heavy sigh.... > > > > I have nothing but badd luck with this. I updated my .gitconfig on the new > > workstation to use mgross@linux.intel.com and I git commit --ammend ed the > > commit I thought was blocked because it had an author of mark.gross@intel.com > > instead of the other one. > > I put an alias in, so both addresses work on the list. > > > I assume they are getting held up by something on the list server end but, I > > have no idea what I'm doing to piss it off. :( > > > > If anyone could help clue me in I'll try to correct whatever I'm doing wrong. > > The other two patches where sent using the same process: > > > > * git format-patch -o mail -n --to speck@linutronix.de --thread --cover-letter v5.6-rc3 > > * speckify-gitmail -s "more sampling fun" mail/ spec/ > > * for b in `ls`; do sendmail -t -i -f smtp.intel.com <$b; done > > Which looks about correct, but then the receiving end says: > > 2020-02-24 22:57:31 1j6LjH-00076x-B2 <= smtp.intel.com@mtg-dev.jf.intel.com H=mga11.intel.com [192.55.52.93] P=esmtps X=TLS1.2:DHE_RSA_AES_256_CBC_SHA256:256 S=3729 id=cover.1582580710.git.mgross@linux.intel.com > 2020-02-24 22:57:32 1j6LjH-00076x-B2 => crypto-ml <speck@linutronix.de> R=local_user T=mail_spool > 2020-02-24 22:57:32 1j6LjH-00076x-B2 Completed > > That's the cover letter. And then the other two seem to be broken: > > 2020-02-24 22:57:32 H=mga07.intel.com [134.134.136.100] sender verify fail for <smtp.intel.com@mtg-dev.jf.intel.com>: Unrouteable address > 2020-02-24 22:57:32 H=mga07.intel.com [134.134.136.100] F=<smtp.intel.com@mtg-dev.jf.intel.com> rejected RCPT <speck@linutronix.de>: Sender verify failed > > 2020-02-24 22:57:32 H=mga06.intel.com [134.134.136.31] sender verify fail for <smtp.intel.com@mtg-dev.jf.intel.com>: Unrouteable address > 2020-02-24 22:57:32 H=mga06.intel.com [134.134.136.31] F=<smtp.intel.com@mtg-dev.jf.intel.com> rejected RCPT <speck@linutronix.de>: Sender verify failed > > Now, looking at your sendmail command line: > > -t -i -f smtp.intel.com > > -f is the envelope sender address. smtp.intel.com is definitely not a > valid envelope-from address, it's a hostname and it looks like sendmail > magically makes that into: doh, thats likely my problem! I changed it to mgross@linux.intel.com and am retrying. > > envelope-from <smtp.intel.com@mtg-dev.jf.intel.com> > > and that mtg-dev.jf.intel.com is probably your host name: > > mtg-dev . Jones Farm . intel.com All true. > > which is bonkers to begin with because I'm pretty sure that nothing can > send mail to that address. > > Now in the cover letter the From: field is correct: > > From: mark gross <mgross@linux.intel.com> > > but I'm pretty sure, that your other two mails are malformatted and > sendmail throws your bonkers -f "address" at it which causes the sender > verify to fail. > > And of course because your envelope-from is broken, the bounce (reject) > info does not make it to you. > > May I recommend to check the mails in mail/ and spec/ before sending > them out. You can also check them for a valid From: address after the > fact of course :) > > BOFH excuse #208: > > Your mail is being routed through Germany ... and they're censoring us. Thanks! --mark ^ permalink raw reply [flat|nested] 30+ messages in thread
[parent not found: <5e5595e6.1c69fb81.69e80.2880SMTPIN_ADDED_BROKEN@mx.google.com>]
* [MODERATED] Re: [PATCH v2 2/2] v2: more sampling fun 2 [not found] ` <5e5595e6.1c69fb81.69e80.2880SMTPIN_ADDED_BROKEN@mx.google.com> @ 2020-02-26 7:27 ` Greg KH 2020-02-26 18:02 ` mark gross 0 siblings, 1 reply; 30+ messages in thread From: Greg KH @ 2020-02-26 7:27 UTC (permalink / raw) To: speck On Thu, Jan 16, 2020 at 02:16:07PM -0800, speck for mark gross wrote: > * exports vulnerability status via sysfs You forgot to document the new sysfs file :( ^ permalink raw reply [flat|nested] 30+ messages in thread
* [MODERATED] Re: [PATCH v2 2/2] v2: more sampling fun 2 2020-02-26 7:27 ` [MODERATED] Re: [PATCH v2 2/2] v2: more sampling fun 2 Greg KH @ 2020-02-26 18:02 ` mark gross 0 siblings, 0 replies; 30+ messages in thread From: mark gross @ 2020-02-26 18:02 UTC (permalink / raw) To: speck On Wed, Feb 26, 2020 at 08:27:52AM +0100, speck for Greg KH wrote: > On Thu, Jan 16, 2020 at 02:16:07PM -0800, speck for mark gross wrote: > > * exports vulnerability status via sysfs > > You forgot to document the new sysfs file :( I'll add this for the next update. Thanks! --mark ^ permalink raw reply [flat|nested] 30+ messages in thread
* [MODERATED] Re: [PATCH v2 1/2] v2: more sampling fun 1 2020-02-24 21:45 [MODERATED] [PATCH v2 0/2] v2: more sampling fun 0 mark gross ` (3 preceding siblings ...) [not found] ` <5e5595e6.1c69fb81.69e80.2880SMTPIN_ADDED_BROKEN@mx.google.com> @ 2020-02-26 11:07 ` Borislav Petkov 2020-02-26 17:11 ` mark gross 2020-02-26 11:46 ` [MODERATED] Re: [PATCH v2 2/2] v2: more sampling fun 2 Borislav Petkov 5 siblings, 1 reply; 30+ messages in thread From: Borislav Petkov @ 2020-02-26 11:07 UTC (permalink / raw) To: speck On Thu, Feb 06, 2020 at 02:11:02PM -0800, speck for mark gross wrote: > From: mark gross <mgross@linux.intel.com> > Subject: [PATCH v2 1/2] Add capability to specify a range of steppings in the > vulnerability white list structure. > > From: mark gross <mgross@linux.intel.com> > Subject: [PATCH v2 1/2] Add capability to specify a range of steppings in the That second subject is incomplete. Do just one pls. Also, you need a subject prefix: x86/CPU: Add ... git log arch/x86/ is your friend if you're wondering what to call it. > Intel has produced processors with the same CPUID family+model. Code > may need to check the stepping when programming model specific behavior. > > Add an API to allow easy specification of stepping or range of steppings > with a 16 bit bitmask. > > Update cpu_vuln_whitelist using this new API. > > I implemented this in the way I did to avoid modifying x86_cpu_id as > that structure is an exported ABI and any change would impact user mode > code using the structure. Exported ABI? Which usermode code uses this? The module loading tools? Even if, we do add new struct members at the end of exported structs just fine. So what is the problem here? > Signed-off-by: mark gross <mgross@linux.intel.com> > Reviewed-by: tony luck <tony.luck@intel.com> Please write names capitalized. -- Regards/Gruss, Boris. SUSE Software Solutions Germany GmbH, GF: Felix Imendörffer, HRB 36809, AG Nürnberg -- ^ permalink raw reply [flat|nested] 30+ messages in thread
* [MODERATED] Re: [PATCH v2 1/2] v2: more sampling fun 1 2020-02-26 11:07 ` [MODERATED] Re: [PATCH v2 1/2] v2: more sampling fun 1 Borislav Petkov @ 2020-02-26 17:11 ` mark gross 2020-02-26 17:59 ` Borislav Petkov ` (2 more replies) 0 siblings, 3 replies; 30+ messages in thread From: mark gross @ 2020-02-26 17:11 UTC (permalink / raw) To: speck On Wed, Feb 26, 2020 at 12:07:37PM +0100, speck for Borislav Petkov wrote: > On Thu, Feb 06, 2020 at 02:11:02PM -0800, speck for mark gross wrote: > > From: mark gross <mgross@linux.intel.com> > > Subject: [PATCH v2 1/2] Add capability to specify a range of steppings in the > > vulnerability white list structure. > > > > From: mark gross <mgross@linux.intel.com> > > Subject: [PATCH v2 1/2] Add capability to specify a range of steppings in the > > That second subject is incomplete. Do just one pls. Ok FWIW the instructions for using the speckify-gitmail said something about copying the subject and from lines into the body. > Also, you need a subject prefix: > > x86/CPU: Add ... ok > > git log arch/x86/ > > is your friend if you're wondering what to call it. Thanks! > > Intel has produced processors with the same CPUID family+model. Code > > may need to check the stepping when programming model specific behavior. > > > > Add an API to allow easy specification of stepping or range of steppings > > with a 16 bit bitmask. > > > > Update cpu_vuln_whitelist using this new API. > > > > I implemented this in the way I did to avoid modifying x86_cpu_id as > > that structure is an exported ABI and any change would impact user mode > > code using the structure. > > Exported ABI? Which usermode code uses this? The module loading tools? Yeah, Andi pointed it out to me on an internal review. I don't know what tool is using it. > > Even if, we do add new struct members at the end of exported structs > just fine. So what is the problem here? FWIW doing it this way made a cleaner patch without touching a dozen other files using that structure. I'd rather stay with the way it is but, if you feel strongly I can do a version of what I had before only adding the new members to the end. Please let me know. > > Signed-off-by: mark gross <mgross@linux.intel.com> > > Reviewed-by: tony luck <tony.luck@intel.com> > > Please write names capitalized. Ok. Thanks! --mark > -- > Regards/Gruss, > Boris. > > SUSE Software Solutions Germany GmbH, GF: Felix Imendörffer, HRB 36809, AG Nürnberg > -- ^ permalink raw reply [flat|nested] 30+ messages in thread
* [MODERATED] Re: [PATCH v2 1/2] v2: more sampling fun 1 2020-02-26 17:11 ` mark gross @ 2020-02-26 17:59 ` Borislav Petkov 2020-02-26 18:16 ` Thomas Gleixner 2020-02-26 22:11 ` mark gross 2020-02-26 18:55 ` Thomas Gleixner 2020-02-26 21:13 ` Andi Kleen 2 siblings, 2 replies; 30+ messages in thread From: Borislav Petkov @ 2020-02-26 17:59 UTC (permalink / raw) To: speck On Wed, Feb 26, 2020 at 09:11:03AM -0800, speck for mark gross wrote: > Yeah, Andi pointed it out to me on an internal review. I don't know what tool > is using it. Then how do you write a patch and state in the commit message that something is an ABI without knowing what the situation actually is?! > FWIW doing it this way made a cleaner patch without touching a dozen other > files using that structure. I'd rather stay with the way it is but, if you > feel strongly I can do a version of what I had before only adding the new > members to the end. Please let me know. Looking at that table again - cpu_vuln_whitelist - that is a *whitelist*. See how all the bits start with "NO_"? Except maybe MSBDS_ONLY. What you're doing is, you're misusing it to match models and steppings to set SRBDS* bug flags. What you should actually be doing is setting those bug flags in early_init_intel() where you can go wild with the steppings checking and then you won't need to touch x86_cpu_id at all. IMO. -- Regards/Gruss, Boris. SUSE Software Solutions Germany GmbH, GF: Felix Imendörffer, HRB 36809, AG Nürnberg -- ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 1/2] v2: more sampling fun 1 2020-02-26 17:59 ` Borislav Petkov @ 2020-02-26 18:16 ` Thomas Gleixner 2020-02-26 22:13 ` [MODERATED] " mark gross 2020-02-26 22:11 ` mark gross 1 sibling, 1 reply; 30+ messages in thread From: Thomas Gleixner @ 2020-02-26 18:16 UTC (permalink / raw) To: speck speck for Borislav Petkov <speck@linutronix.de> writes: > On Wed, Feb 26, 2020 at 09:11:03AM -0800, speck for mark gross wrote: >> Yeah, Andi pointed it out to me on an internal review. I don't know what tool >> is using it. > > Then how do you write a patch and state in the commit message that > something is an ABI without knowing what the situation actually is?! > >> FWIW doing it this way made a cleaner patch without touching a dozen other >> files using that structure. I'd rather stay with the way it is but, if you >> feel strongly I can do a version of what I had before only adding the new >> members to the end. Please let me know. > > Looking at that table again - cpu_vuln_whitelist - that is a > *whitelist*. See how all the bits start with "NO_"? Except maybe > MSBDS_ONLY. Well, that was a decision to not have NO_MSDALL and NO_MDSSOMETHING as it made some of logic in the code simpler. > What you're doing is, you're misusing it to match models and steppings > to set SRBDS* bug flags. > > What you should actually be doing is setting those bug flags in > early_init_intel() where you can go wild with the steppings checking and > then you won't need to touch x86_cpu_id at all. Either that or add a new cpu_vuln_shitlist beside the whitelist and stick the new stuff into that. It kinda makes sense to keep all this vulnerability nonsense in one place. Thanks, tglx ^ permalink raw reply [flat|nested] 30+ messages in thread
* [MODERATED] Re: [PATCH v2 1/2] v2: more sampling fun 1 2020-02-26 18:16 ` Thomas Gleixner @ 2020-02-26 22:13 ` mark gross 2020-02-26 23:53 ` Thomas Gleixner 0 siblings, 1 reply; 30+ messages in thread From: mark gross @ 2020-02-26 22:13 UTC (permalink / raw) To: speck On Wed, Feb 26, 2020 at 07:16:33PM +0100, speck for Thomas Gleixner wrote: > speck for Borislav Petkov <speck@linutronix.de> writes: > > On Wed, Feb 26, 2020 at 09:11:03AM -0800, speck for mark gross wrote: > >> Yeah, Andi pointed it out to me on an internal review. I don't know what tool > >> is using it. > > > > Then how do you write a patch and state in the commit message that > > something is an ABI without knowing what the situation actually is?! > > > >> FWIW doing it this way made a cleaner patch without touching a dozen other > >> files using that structure. I'd rather stay with the way it is but, if you > >> feel strongly I can do a version of what I had before only adding the new > >> members to the end. Please let me know. > > > > Looking at that table again - cpu_vuln_whitelist - that is a > > *whitelist*. See how all the bits start with "NO_"? Except maybe > > MSBDS_ONLY. > > Well, that was a decision to not have NO_MSDALL and NO_MDSSOMETHING as > it made some of logic in the code simpler. > > > What you're doing is, you're misusing it to match models and steppings > > to set SRBDS* bug flags. > > > > What you should actually be doing is setting those bug flags in > > early_init_intel() where you can go wild with the steppings checking and > > then you won't need to touch x86_cpu_id at all. > > Either that or add a new cpu_vuln_shitlist beside the whitelist and > stick the new stuff into that. It kinda makes sense to keep all this > vulnerability nonsense in one place. I'm ok with either way. is there a consensus for making annother cpu_vuln_list? --mark ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 1/2] v2: more sampling fun 1 2020-02-26 22:13 ` [MODERATED] " mark gross @ 2020-02-26 23:53 ` Thomas Gleixner 2020-02-27 16:43 ` [MODERATED] " mark gross 0 siblings, 1 reply; 30+ messages in thread From: Thomas Gleixner @ 2020-02-26 23:53 UTC (permalink / raw) To: speck speck for mark gross <speck@linutronix.de> writes: > On Wed, Feb 26, 2020 at 07:16:33PM +0100, speck for Thomas Gleixner wrote: >> speck for Borislav Petkov <speck@linutronix.de> writes: >> > What you should actually be doing is setting those bug flags in >> > early_init_intel() where you can go wild with the steppings checking and >> > then you won't need to touch x86_cpu_id at all. >> >> Either that or add a new cpu_vuln_shitlist beside the whitelist and >> stick the new stuff into that. It kinda makes sense to keep all this >> vulnerability nonsense in one place. > > I'm ok with either way. is there a consensus for making annother > cpu_vuln_list? Go for the extra list at the same place so all that stuff is in one place. ^ permalink raw reply [flat|nested] 30+ messages in thread
* [MODERATED] Re: [PATCH v2 1/2] v2: more sampling fun 1 2020-02-26 23:53 ` Thomas Gleixner @ 2020-02-27 16:43 ` mark gross 0 siblings, 0 replies; 30+ messages in thread From: mark gross @ 2020-02-27 16:43 UTC (permalink / raw) To: speck On Thu, Feb 27, 2020 at 12:53:54AM +0100, speck for Thomas Gleixner wrote: > speck for mark gross <speck@linutronix.de> writes: > > On Wed, Feb 26, 2020 at 07:16:33PM +0100, speck for Thomas Gleixner wrote: > >> speck for Borislav Petkov <speck@linutronix.de> writes: > >> > What you should actually be doing is setting those bug flags in > >> > early_init_intel() where you can go wild with the steppings checking and > >> > then you won't need to touch x86_cpu_id at all. > >> > >> Either that or add a new cpu_vuln_shitlist beside the whitelist and > >> stick the new stuff into that. It kinda makes sense to keep all this > >> vulnerability nonsense in one place. > > > > I'm ok with either way. is there a consensus for making annother > > cpu_vuln_list? > > Go for the extra list at the same place so all that stuff is in one > place. I will do that. --mark ^ permalink raw reply [flat|nested] 30+ messages in thread
* [MODERATED] Re: [PATCH v2 1/2] v2: more sampling fun 1 2020-02-26 17:59 ` Borislav Petkov 2020-02-26 18:16 ` Thomas Gleixner @ 2020-02-26 22:11 ` mark gross 2020-02-26 22:43 ` Borislav Petkov 1 sibling, 1 reply; 30+ messages in thread From: mark gross @ 2020-02-26 22:11 UTC (permalink / raw) To: speck On Wed, Feb 26, 2020 at 06:59:50PM +0100, speck for Borislav Petkov wrote: > On Wed, Feb 26, 2020 at 09:11:03AM -0800, speck for mark gross wrote: > > Yeah, Andi pointed it out to me on an internal review. I don't know what tool > > is using it. > > Then how do you write a patch and state in the commit message that > something is an ABI without knowing what the situation actually is?! Easily, I trusted Andi's feedback. I'm good with it as I think he initially created that data structure. I'll get specific user mode users of the structure and call it out in the commit comment for the next version. > > > FWIW doing it this way made a cleaner patch without touching a dozen other > > files using that structure. I'd rather stay with the way it is but, if you > > feel strongly I can do a version of what I had before only adding the new > > members to the end. Please let me know. > > Looking at that table again - cpu_vuln_whitelist - that is a > *whitelist*. See how all the bits start with "NO_"? Except maybe > MSBDS_ONLY. > > What you're doing is, you're misusing it to match models and steppings > to set SRBDS* bug flags. > > What you should actually be doing is setting those bug flags in > early_init_intel() where you can go wild with the steppings checking and > then you won't need to touch x86_cpu_id at all. I can certainly make a new table elsewhere if you want all the special case / hard-coded vulnerabilities spread around the kernel source tree as opposed to one centralized place. --mark ^ permalink raw reply [flat|nested] 30+ messages in thread
* [MODERATED] Re: [PATCH v2 1/2] v2: more sampling fun 1 2020-02-26 22:11 ` mark gross @ 2020-02-26 22:43 ` Borislav Petkov 2020-02-26 23:34 ` mark gross 0 siblings, 1 reply; 30+ messages in thread From: Borislav Petkov @ 2020-02-26 22:43 UTC (permalink / raw) To: speck On Wed, Feb 26, 2020 at 02:11:02PM -0800, speck for mark gross wrote: > Easily, I trusted Andi's feedback. I'm good with it as I think he initially > created that data structure. I'll get specific user mode users of the > structure and call it out in the commit comment for the next version. No need for any of that - see tglx's reply. Forget the ABI boohoo. > I can certainly make a new table elsewhere if you want all the special case / > hard-coded vulnerabilities spread around the kernel source tree as opposed to > one centralized place. A second shitlist table under the whitelist one makes sense because it'll keep all the vulns together. Thx. -- Regards/Gruss, Boris. SUSE Software Solutions Germany GmbH, GF: Felix Imendörffer, HRB 36809, AG Nürnberg -- ^ permalink raw reply [flat|nested] 30+ messages in thread
* [MODERATED] Re: [PATCH v2 1/2] v2: more sampling fun 1 2020-02-26 22:43 ` Borislav Petkov @ 2020-02-26 23:34 ` mark gross 0 siblings, 0 replies; 30+ messages in thread From: mark gross @ 2020-02-26 23:34 UTC (permalink / raw) To: speck On Wed, Feb 26, 2020 at 11:43:54PM +0100, speck for Borislav Petkov wrote: > On Wed, Feb 26, 2020 at 02:11:02PM -0800, speck for mark gross wrote: > > Easily, I trusted Andi's feedback. I'm good with it as I think he initially > > created that data structure. I'll get specific user mode users of the > > structure and call it out in the commit comment for the next version. > > No need for any of that - see tglx's reply. Forget the ABI boohoo. thanks > > I can certainly make a new table elsewhere if you want all the special case / > > hard-coded vulnerabilities spread around the kernel source tree as opposed to > > one centralized place. > > A second shitlist table under the whitelist one makes sense because > it'll keep all the vulns together. I'll create an affected_cpu table near the existing one. --mark > Thx. > > -- > Regards/Gruss, > Boris. > > SUSE Software Solutions Germany GmbH, GF: Felix Imendörffer, HRB 36809, AG Nürnberg > -- ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 1/2] v2: more sampling fun 1 2020-02-26 17:11 ` mark gross 2020-02-26 17:59 ` Borislav Petkov @ 2020-02-26 18:55 ` Thomas Gleixner 2020-02-26 22:23 ` [MODERATED] " mark gross 2020-02-26 21:13 ` Andi Kleen 2 siblings, 1 reply; 30+ messages in thread From: Thomas Gleixner @ 2020-02-26 18:55 UTC (permalink / raw) To: speck speck for mark gross <speck@linutronix.de> writes: > On Wed, Feb 26, 2020 at 12:07:37PM +0100, speck for Borislav Petkov wrote: >> On Thu, Feb 06, 2020 at 02:11:02PM -0800, speck for mark gross wrote: >> > From: mark gross <mgross@linux.intel.com> >> > Subject: [PATCH v2 1/2] Add capability to specify a range of steppings in the >> > vulnerability white list structure. >> > >> > From: mark gross <mgross@linux.intel.com> >> > Subject: [PATCH v2 1/2] Add capability to specify a range of steppings in the >> >> That second subject is incomplete. Do just one pls. > Ok FWIW the instructions for using the speckify-gitmail said something about > copying the subject and from lines into the body. speckify-gitmail -h .... The script does the following steps on all mails in the plain text input directory: - Verify that 'To' is the intended recipient - Remove all Cc's - Copy 'From' and 'Subject' into the mail body - Replace the 'Subject' with the innocent subject which is by default 'Hidden' or something you supplied with the -s option - Encrypt the mail with the recipients GPG key and store it in the output directory The important part of the sentence before the list of actions is: "The script does" That's not an instruction. That's a description of the functionality. But then this was written by an evil German and as you've seen with the mails, these Germans are out there to get you. Thanks, tglx ^ permalink raw reply [flat|nested] 30+ messages in thread
* [MODERATED] Re: [PATCH v2 1/2] v2: more sampling fun 1 2020-02-26 18:55 ` Thomas Gleixner @ 2020-02-26 22:23 ` mark gross 0 siblings, 0 replies; 30+ messages in thread From: mark gross @ 2020-02-26 22:23 UTC (permalink / raw) To: speck On Wed, Feb 26, 2020 at 07:55:43PM +0100, speck for Thomas Gleixner wrote: > speck for mark gross <speck@linutronix.de> writes: > > > On Wed, Feb 26, 2020 at 12:07:37PM +0100, speck for Borislav Petkov wrote: > >> On Thu, Feb 06, 2020 at 02:11:02PM -0800, speck for mark gross wrote: > >> > From: mark gross <mgross@linux.intel.com> > >> > Subject: [PATCH v2 1/2] Add capability to specify a range of steppings in the > >> > vulnerability white list structure. > >> > > >> > From: mark gross <mgross@linux.intel.com> > >> > Subject: [PATCH v2 1/2] Add capability to specify a range of steppings in the > >> > >> That second subject is incomplete. Do just one pls. > > Ok FWIW the instructions for using the speckify-gitmail said something about > > copying the subject and from lines into the body. > > speckify-gitmail -h > .... > > The script does the following steps on all mails in the plain text > input directory: > > - Verify that 'To' is the intended recipient > > - Remove all Cc's > > - Copy 'From' and 'Subject' into the mail body > > - Replace the 'Subject' with the innocent subject which is by > default 'Hidden' or something you supplied with the -s option > > - Encrypt the mail with the recipients GPG key and store it in > the output directory > > The important part of the sentence before the list of actions is: > > "The script does" > > That's not an instruction. That's a description of the functionality. > > But then this was written by an evil German and as you've seen with the > mails, these Germans are out there to get you. Those tricky Germans, always forcing me to read more carefully than comes naturally to me. Thanks, --mark ^ permalink raw reply [flat|nested] 30+ messages in thread
* [MODERATED] Re: [PATCH v2 1/2] v2: more sampling fun 1 2020-02-26 17:11 ` mark gross 2020-02-26 17:59 ` Borislav Petkov 2020-02-26 18:55 ` Thomas Gleixner @ 2020-02-26 21:13 ` Andi Kleen 2020-02-26 22:01 ` Thomas Gleixner 2 siblings, 1 reply; 30+ messages in thread From: Andi Kleen @ 2020-02-26 21:13 UTC (permalink / raw) To: speck > > > I implemented this in the way I did to avoid modifying x86_cpu_id as > > > that structure is an exported ABI and any change would impact user mode > > > code using the structure. > > > > Exported ABI? Which usermode code uses this? The module loading tools? > > Yeah, Andi pointed it out to me on an internal review. I don't know what tool > is using it. Yes it's modprobe to find which module to load. All of mod_devicetable.h is ABI -Andi ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v2 1/2] v2: more sampling fun 1 2020-02-26 21:13 ` Andi Kleen @ 2020-02-26 22:01 ` Thomas Gleixner 2020-02-27 7:08 ` [MODERATED] " Greg KH 0 siblings, 1 reply; 30+ messages in thread From: Thomas Gleixner @ 2020-02-26 22:01 UTC (permalink / raw) To: speck speck for Andi Kleen <speck@linutronix.de> writes: >> > > I implemented this in the way I did to avoid modifying x86_cpu_id as >> > > that structure is an exported ABI and any change would impact user mode >> > > code using the structure. >> > >> > Exported ABI? Which usermode code uses this? The module loading tools? >> >> Yeah, Andi pointed it out to me on an internal review. I don't know what tool >> is using it. > > Yes it's modprobe to find which module to load. > > All of mod_devicetable.h is ABI That's simply not true. mod_devicetable.h is a kernel internal header which gets analyzed by the kernel internal tool modpost which uses this header to generate the ALIAS entries in the .modinfo section of the .ko elf file. The ALIAS entries are user space ABI but not the header file. If the header file changes then file2alias.c has to be updated as clearly stated in the comment at the top of mod_devicetable.h And if you look at the driver_data member of x86_cpu_id then you'll notice that file2alias does not care about it at all, neither would it care about a stepping entry or whatever. Thanks, tglx ^ permalink raw reply [flat|nested] 30+ messages in thread
* [MODERATED] Re: [PATCH v2 1/2] v2: more sampling fun 1 2020-02-26 22:01 ` Thomas Gleixner @ 2020-02-27 7:08 ` Greg KH 0 siblings, 0 replies; 30+ messages in thread From: Greg KH @ 2020-02-27 7:08 UTC (permalink / raw) To: speck On Wed, Feb 26, 2020 at 11:01:36PM +0100, speck for Thomas Gleixner wrote: > speck for Andi Kleen <speck@linutronix.de> writes: > > >> > > I implemented this in the way I did to avoid modifying x86_cpu_id as > >> > > that structure is an exported ABI and any change would impact user mode > >> > > code using the structure. > >> > > >> > Exported ABI? Which usermode code uses this? The module loading tools? > >> > >> Yeah, Andi pointed it out to me on an internal review. I don't know what tool > >> is using it. > > > > Yes it's modprobe to find which module to load. > > > > All of mod_devicetable.h is ABI > > That's simply not true. > > mod_devicetable.h is a kernel internal header which gets analyzed by the > kernel internal tool modpost which uses this header to generate the > ALIAS entries in the .modinfo section of the .ko elf file. > > The ALIAS entries are user space ABI but not the header file. To be pedantic, it's a string sent to userspace, _but_ not really an ABI as it depeneds entirely on the kernel that is running at the time. It's only used to pass back into a tool like modprobe to try to figure out what module to load for that specific kernel. So that string can change structure, with no side affects at all as it is explicitly tied to that kernel version anyway. thanks, greg k-h ^ permalink raw reply [flat|nested] 30+ messages in thread
* [MODERATED] Re: [PATCH v2 2/2] v2: more sampling fun 2 2020-02-24 21:45 [MODERATED] [PATCH v2 0/2] v2: more sampling fun 0 mark gross ` (4 preceding siblings ...) 2020-02-26 11:07 ` [MODERATED] Re: [PATCH v2 1/2] v2: more sampling fun 1 Borislav Petkov @ 2020-02-26 11:46 ` Borislav Petkov 2020-02-26 17:35 ` mark gross 5 siblings, 1 reply; 30+ messages in thread From: Borislav Petkov @ 2020-02-26 11:46 UTC (permalink / raw) To: speck On Thu, Jan 16, 2020 at 02:16:07PM -0800, speck for mark gross wrote: > From: mark gross <mgross@linux.intel.com> > Subject: [PATCH v2 2/2] WIP SRBDS mitigation enabling. > > From: mark gross <mgross@linux.intel.com> > Subject: [PATCH v2 2/2] WIP SRBDS mitigation enabling. In addition to the notes in the previous mail, your subject needs a verb: "Add ... mitigation" or so. See git log -p arch/x86/kernel/cpu/bugs.c output for inspiration. > 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 ensures that the shared > buffer is overwritten before it is released for reuse. > > We subdivide processors that are vulnerable to SRBDS into two classes: "We" is? > X86_BUG_SRBDS: models that are vulnerable > X86_BUG_SRBDS_TSX: models only vulnerable when TSX is enabled. > > The latter are not vulnerable to SRBDS if TSX is disabled on all cores. > > The mitigation is activated by default on affected processors and it slows > down /dev/urandom. The latency of RDRAND and RDSEED instructions is > increased by 10x. We don't expect this to be noticeable in most cases. This "We" sounds like you mean Intel...? > This patch: Avoid having "This patch" or "This commit" in the commit message. It is tautologically useless. Also, do $ git grep 'This patch' Documentation/process for more details. > @@ -397,6 +399,69 @@ static int __init tsx_async_abort_parse_cmdline(char *str) > } > early_param("tsx_async_abort", tsx_async_abort_parse_cmdline); > > +#undef pr_fmt > +#define pr_fmt(fmt) "SRBDS: " fmt > + > +enum srbds_mitigations srbds_mitigation __ro_after_init = SRBDS_MITIGATION_FULL; > +static const char * const srbds_strings[] = { > + [SRBDS_MITIGATION_OFF] = "Vulnerable", > + [SRBDS_MITIGATION_UCODE_NEEDED] = "Vulnerable: no microcode", > + [SRBDS_MITIGATION_FULL] = "Mitigation: bus lock when using RDRAND or RDSEED", > +}; > + > +void srbds_configure_mitigation(void) > +{ > + u64 mcu_ctrl; > + > + if (!boot_cpu_has_bug(X86_BUG_SRBDS) && !boot_cpu_has_bug(X86_BUG_SRBDS_TSX)) > + return; > + > + if (!boot_cpu_has(X86_FEATURE_SRBDS_CTRL)) > + return; > + > + rdmsrl(MSR_IA32_MCU_OPT_CTRL, mcu_ctrl); What's up with virtualization? Is that MSR going to be exported or what? If so, you need the _safe() accessors here. > + if (srbds_mitigation == SRBDS_MITIGATION_FULL) > + mcu_ctrl &= ~SRBDS_MITG_DIS; > + else if (srbds_mitigation == SRBDS_MITIGATION_OFF) > + mcu_ctrl |= SRBDS_MITG_DIS; > + > + if (boot_cpu_has_bug(X86_BUG_SRBDS_TSX) && !boot_cpu_has(X86_FEATURE_RTM)) > + mcu_ctrl |= SRBDS_MITG_DIS; > + > + wrmsrl(MSR_IA32_MCU_OPT_CTRL, mcu_ctrl); > +} > + > +static void __init srbds_select_mitigation(void) > +{ > + if (!boot_cpu_has_bug(X86_BUG_SRBDS) && > + !boot_cpu_has_bug(X86_BUG_SRBDS_TSX)) > + return; Why is that check here again if you do it in srbds_configure_mitigation() above? I'm guessing you can remove the one above... > + > + if (cpu_mitigations_off()) { > + srbds_mitigation = SRBDS_MITIGATION_OFF; > + goto out; > + } > + > + if (!boot_cpu_has(X86_FEATURE_SRBDS_CTRL)) > + srbds_mitigation = SRBDS_MITIGATION_UCODE_NEEDED; > + > +out: > + srbds_configure_mitigation(); > +} > + > +static int __init srbds_parse_cmdline(char *str) > +{ > + if (!str) > + return -EINVAL; > + > + if (!strcmp(str, "off")) > + srbds_mitigation = SRBDS_MITIGATION_OFF; > + > + return 0; > +} > + > +early_param("srbds_mitigation", srbds_parse_cmdline); "srb_sampling=" or something more readable pls. -- Regards/Gruss, Boris. SUSE Software Solutions Germany GmbH, GF: Felix Imendörffer, HRB 36809, AG Nürnberg -- ^ permalink raw reply [flat|nested] 30+ messages in thread
* [MODERATED] Re: [PATCH v2 2/2] v2: more sampling fun 2 2020-02-26 11:46 ` [MODERATED] Re: [PATCH v2 2/2] v2: more sampling fun 2 Borislav Petkov @ 2020-02-26 17:35 ` mark gross 2020-02-26 18:13 ` Borislav Petkov 0 siblings, 1 reply; 30+ messages in thread From: mark gross @ 2020-02-26 17:35 UTC (permalink / raw) To: speck On Wed, Feb 26, 2020 at 12:46:41PM +0100, speck for Borislav Petkov wrote: > On Thu, Jan 16, 2020 at 02:16:07PM -0800, speck for mark gross wrote: > > From: mark gross <mgross@linux.intel.com> > > Subject: [PATCH v2 2/2] WIP SRBDS mitigation enabling. > > > > From: mark gross <mgross@linux.intel.com> > > Subject: [PATCH v2 2/2] WIP SRBDS mitigation enabling. > > In addition to the notes in the previous mail, your subject needs a > verb: ok > "Add ... mitigation" or so. > > See > > git log -p arch/x86/kernel/cpu/bugs.c > > output for inspiration. ok I'll take any insperation I can get. thanks > > 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 ensures that the shared > > buffer is overwritten before it is released for reuse. > > > > We subdivide processors that are vulnerable to SRBDS into two classes: > > "We" is? its just me for that instance of the word. > > X86_BUG_SRBDS: models that are vulnerable > > X86_BUG_SRBDS_TSX: models only vulnerable when TSX is enabled. > > > > The latter are not vulnerable to SRBDS if TSX is disabled on all cores. > > > > The mitigation is activated by default on affected processors and it slows > > down /dev/urandom. The latency of RDRAND and RDSEED instructions is > > increased by 10x. We don't expect this to be noticeable in most cases. > > This "We" sounds like you mean Intel...? yes, this instance of "we" is an intel statement from that white paper I'm waiting on for the documentation patch. > > This patch: > > Avoid having "This patch" or "This commit" in the commit message. It is > tautologically useless. good point. > > Also, do > > $ git grep 'This patch' Documentation/process > > for more details. "imperative mood" ok. > > > @@ -397,6 +399,69 @@ static int __init tsx_async_abort_parse_cmdline(char *str) > > } > > early_param("tsx_async_abort", tsx_async_abort_parse_cmdline); > > > > +#undef pr_fmt > > +#define pr_fmt(fmt) "SRBDS: " fmt > > + > > +enum srbds_mitigations srbds_mitigation __ro_after_init = SRBDS_MITIGATION_FULL; > > +static const char * const srbds_strings[] = { > > + [SRBDS_MITIGATION_OFF] = "Vulnerable", > > + [SRBDS_MITIGATION_UCODE_NEEDED] = "Vulnerable: no microcode", > > + [SRBDS_MITIGATION_FULL] = "Mitigation: bus lock when using RDRAND or RDSEED", > > +}; > > + > > +void srbds_configure_mitigation(void) > > +{ > > + u64 mcu_ctrl; > > + > > + if (!boot_cpu_has_bug(X86_BUG_SRBDS) && !boot_cpu_has_bug(X86_BUG_SRBDS_TSX)) > > + return; > > + > > + if (!boot_cpu_has(X86_FEATURE_SRBDS_CTRL)) > > + return; > > + > > + rdmsrl(MSR_IA32_MCU_OPT_CTRL, mcu_ctrl); > > What's up with virtualization? Is that MSR going to be exported or what? Myself and a few Intel folks I talked to about virtualization aspects of this don't expect it will be but, I think that is up to the VMM vendors. > > If so, you need the _safe() accessors here. > > > + if (srbds_mitigation == SRBDS_MITIGATION_FULL) > > + mcu_ctrl &= ~SRBDS_MITG_DIS; > > + else if (srbds_mitigation == SRBDS_MITIGATION_OFF) > > + mcu_ctrl |= SRBDS_MITG_DIS; > > + > > + if (boot_cpu_has_bug(X86_BUG_SRBDS_TSX) && !boot_cpu_has(X86_FEATURE_RTM)) > > + mcu_ctrl |= SRBDS_MITG_DIS; > > + > > + wrmsrl(MSR_IA32_MCU_OPT_CTRL, mcu_ctrl); > > +} > > + > > +static void __init srbds_select_mitigation(void) > > +{ > > + if (!boot_cpu_has_bug(X86_BUG_SRBDS) && > > + !boot_cpu_has_bug(X86_BUG_SRBDS_TSX)) > > + return; > > Why is that check here again if you do it in > srbds_configure_mitigation() above? I'm guessing you can remove the one > above... I wrote it this way initially because at the time I expected there to be more dynamic support for managing this mitigation WRT post boot changes to TSX availability. The expectation I was later given was to make it a boot commandline control only. I kept it this way mostly out of laziness and simplification of the number of mitigation states to track. > > > + > > + if (cpu_mitigations_off()) { > > + srbds_mitigation = SRBDS_MITIGATION_OFF; > > + goto out; > > + } > > + > > + if (!boot_cpu_has(X86_FEATURE_SRBDS_CTRL)) > > + srbds_mitigation = SRBDS_MITIGATION_UCODE_NEEDED; > > + > > +out: > > + srbds_configure_mitigation(); > > +} > > + > > +static int __init srbds_parse_cmdline(char *str) > > +{ > > + if (!str) > > + return -EINVAL; > > + > > + if (!strcmp(str, "off")) > > + srbds_mitigation = SRBDS_MITIGATION_OFF; > > + > > + return 0; > > +} > > + > > +early_param("srbds_mitigation", srbds_parse_cmdline); > > "srb_sampling=" or something more readable pls. srb_sampling is no more readable to me and doesn't convay what the command line is for. But, more readable is always better. Perhaps changing into a single value, with any '=' like: disable_srbs_mitigation or srbds_mitigation_off would be better? Thanks for the feedback, -mark ^ permalink raw reply [flat|nested] 30+ messages in thread
* [MODERATED] Re: [PATCH v2 2/2] v2: more sampling fun 2 2020-02-26 17:35 ` mark gross @ 2020-02-26 18:13 ` Borislav Petkov 2020-02-26 22:37 ` mark gross 0 siblings, 1 reply; 30+ messages in thread From: Borislav Petkov @ 2020-02-26 18:13 UTC (permalink / raw) To: speck On Wed, Feb 26, 2020 at 09:35:08AM -0800, speck for mark gross wrote: > > This "We" sounds like you mean Intel...? > yes, this instance of "we" is an intel statement from that white paper I'm > waiting on for the documentation patch. See how "we" means different things and how you should avoid it in commit messages? IOW, pls use passive voice. > I wrote it this way initially because at the time I expected there to be more > dynamic support for managing this mitigation WRT post boot changes to TSX > availability. The expectation I was later given was to make it a boot > commandline control only. I kept it this way mostly out of laziness and > simplification of the number of mitigation states to track. Drop it then. > srb_sampling is no more readable to me and doesn't convay what the > command line is for. But, more readable is always better. Ok, let's see what better suggestions/preferences the others might have. > Perhaps changing into a single value, with any '=' like: > disable_srbs_mitigation or srbds_mitigation_off would be better? You don't need to have "disable" or "off" in the actual command line switch's name if you give it the on/off after the "=". And as I said already, you don't need to have a tautology by putting "mitigation" in the mitigation option - it is a mitigation. -- Regards/Gruss, Boris. SUSE Software Solutions Germany GmbH, GF: Felix Imendörffer, HRB 36809, AG Nürnberg -- ^ permalink raw reply [flat|nested] 30+ messages in thread
* [MODERATED] Re: [PATCH v2 2/2] v2: more sampling fun 2 2020-02-26 18:13 ` Borislav Petkov @ 2020-02-26 22:37 ` mark gross 2020-02-26 22:50 ` Borislav Petkov 0 siblings, 1 reply; 30+ messages in thread From: mark gross @ 2020-02-26 22:37 UTC (permalink / raw) To: speck On Wed, Feb 26, 2020 at 07:13:25PM +0100, speck for Borislav Petkov wrote: > On Wed, Feb 26, 2020 at 09:35:08AM -0800, speck for mark gross wrote: > > > This "We" sounds like you mean Intel...? > > yes, this instance of "we" is an intel statement from that white paper I'm > > waiting on for the documentation patch. > > See how "we" means different things and how you should avoid it in > commit messages? yeah, the first instance of 'we' was an error. This second one was a copy and paste from the whitepaper. Next time I'll tweak what I copy and replace "we" with "intel" or just reword it. > IOW, pls use passive voice. ok. > > > I wrote it this way initially because at the time I expected there to be more > > dynamic support for managing this mitigation WRT post boot changes to TSX > > availability. The expectation I was later given was to make it a boot > > commandline control only. I kept it this way mostly out of laziness and > > simplification of the number of mitigation states to track. > > Drop it then. Drop it: as in disregard your comment on this point or Drop it: as in rework it to avoid the extra check for TSX availability? > > > srb_sampling is no more readable to me and doesn't convay what the > > command line is for. But, more readable is always better. > > Ok, let's see what better suggestions/preferences the others might have. > > > Perhaps changing into a single value, with any '=' like: > > disable_srbs_mitigation or srbds_mitigation_off would be better? > > You don't need to have "disable" or "off" in the actual command line > switch's name if you give it the on/off after the "=". ok > > And as I said already, you don't need to have a tautology by putting > "mitigation" in the mitigation option - it is a mitigation. My initial version just had "srbds=off" as the commanline. But, I noticed other mitigations included the "_mitigtion" and thought to mimic that. I like dropping the "_mitigation" part of the option. I will change it to just "srbds=off" for the next version. Thank you for the help! --mark ^ permalink raw reply [flat|nested] 30+ messages in thread
* [MODERATED] Re: [PATCH v2 2/2] v2: more sampling fun 2 2020-02-26 22:37 ` mark gross @ 2020-02-26 22:50 ` Borislav Petkov 2020-02-26 23:42 ` mark gross 0 siblings, 1 reply; 30+ messages in thread From: Borislav Petkov @ 2020-02-26 22:50 UTC (permalink / raw) To: speck On Wed, Feb 26, 2020 at 02:37:00PM -0800, speck for mark gross wrote: > Drop it: as in disregard your comment on this point > or > Drop it: as in rework it to avoid the extra check for TSX availability? ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ That one. > My initial version just had "srbds=off" as the commanline. But, I noticed > other mitigations included the "_mitigtion" and thought to mimic that. Which mitigation command line option has "_mitigation"? I don't see one. -- Regards/Gruss, Boris. SUSE Software Solutions Germany GmbH, GF: Felix Imendörffer, HRB 36809, AG Nürnberg -- ^ permalink raw reply [flat|nested] 30+ messages in thread
* [MODERATED] Re: [PATCH v2 2/2] v2: more sampling fun 2 2020-02-26 22:50 ` Borislav Petkov @ 2020-02-26 23:42 ` mark gross 0 siblings, 0 replies; 30+ messages in thread From: mark gross @ 2020-02-26 23:42 UTC (permalink / raw) To: speck On Wed, Feb 26, 2020 at 11:50:28PM +0100, speck for Borislav Petkov wrote: > On Wed, Feb 26, 2020 at 02:37:00PM -0800, speck for mark gross wrote: > > Drop it: as in disregard your comment on this point > > or > > Drop it: as in rework it to avoid the extra check for TSX availability? > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > That one. > > > My initial version just had "srbds=off" as the commanline. But, I noticed > > other mitigations included the "_mitigtion" and thought to mimic that. > > Which mitigation command line option has "_mitigation"? I don't see one. Now that I did a "git grep _mitigation | grep param" I don't find any. I thought I saw it for mds and taa. Sorry for my confusion. --mark ^ permalink raw reply [flat|nested] 30+ messages in thread
end of thread, other threads:[~2020-02-27 16:43 UTC | newest]
Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-24 21:45 [MODERATED] [PATCH v2 0/2] v2: more sampling fun 0 mark gross
2020-01-16 22:16 ` [MODERATED] [PATCH v2 2/2] v2: more sampling fun 2 mark gross
2020-02-06 22:11 ` [MODERATED] [PATCH v2 1/2] v2: more sampling fun 1 mark gross
2020-02-25 16:55 ` [MODERATED] Re: [PATCH v2 0/2] v2: more sampling fun 0 Josh Poimboeuf
2020-02-25 17:43 ` mark gross
2020-02-25 20:47 ` Thomas Gleixner
2020-02-25 21:51 ` [MODERATED] " mark gross
[not found] ` <5e5595e6.1c69fb81.69e80.2880SMTPIN_ADDED_BROKEN@mx.google.com>
2020-02-26 7:27 ` [MODERATED] Re: [PATCH v2 2/2] v2: more sampling fun 2 Greg KH
2020-02-26 18:02 ` mark gross
2020-02-26 11:07 ` [MODERATED] Re: [PATCH v2 1/2] v2: more sampling fun 1 Borislav Petkov
2020-02-26 17:11 ` mark gross
2020-02-26 17:59 ` Borislav Petkov
2020-02-26 18:16 ` Thomas Gleixner
2020-02-26 22:13 ` [MODERATED] " mark gross
2020-02-26 23:53 ` Thomas Gleixner
2020-02-27 16:43 ` [MODERATED] " mark gross
2020-02-26 22:11 ` mark gross
2020-02-26 22:43 ` Borislav Petkov
2020-02-26 23:34 ` mark gross
2020-02-26 18:55 ` Thomas Gleixner
2020-02-26 22:23 ` [MODERATED] " mark gross
2020-02-26 21:13 ` Andi Kleen
2020-02-26 22:01 ` Thomas Gleixner
2020-02-27 7:08 ` [MODERATED] " Greg KH
2020-02-26 11:46 ` [MODERATED] Re: [PATCH v2 2/2] v2: more sampling fun 2 Borislav Petkov
2020-02-26 17:35 ` mark gross
2020-02-26 18:13 ` Borislav Petkov
2020-02-26 22:37 ` mark gross
2020-02-26 22:50 ` Borislav Petkov
2020-02-26 23:42 ` mark gross
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).