* [MODERATED] Re: ***UNCHECKED*** [patch 3/8] [PATCH v1.3.1 3/7] Linux Patch 3 [not found] <20180418141551.07CBB6111A@crypto-ml.lab.linutronix.de> @ 2018-04-18 15:37 ` Borislav Petkov 2018-04-18 16:00 ` [MODERATED] Re: ***UNCHECKED*** " Borislav Petkov 2018-04-19 20:51 ` [MODERATED] Re: ***UNCHECKED*** " Konrad Rzeszutek Wilk 0 siblings, 2 replies; 7+ messages in thread From: Borislav Petkov @ 2018-04-18 15:37 UTC (permalink / raw) To: speck On Thu, Apr 12, 2018 at 10:26:52PM -0400, speck for konrad.wilk_at_oracle.com wrote: > diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c > index 561cb228605a..73f76d0f5181 100644 > --- a/arch/x86/kernel/cpu/bugs.c > +++ b/arch/x86/kernel/cpu/bugs.c > @@ -72,6 +72,9 @@ void __init check_bugs(void) > */ > if (!direct_gbpages) > set_memory_4k((unsigned long)__va(0), 1); > + > + if (mdd_at_boot()) > + wrmsrl(MSR_IA32_SPEC_CTRL, SPEC_CTRL_MDD); You can't do this on the common path as that would explode on !Intel. Instead, move that check to init_intel(). And add to init_amd_zn() code reading MSR 0xc0011020, setting bit 10 to 1b and then writing it back in the mdd_at_boot() case. We have a define for that MSR already - MSR_AMD64_LS_CFG. > > @@ -317,7 +320,14 @@ static void __init spectre_v2_select_mitigation(void) > #undef pr_fmt > #define pr_fmt(fmt) "MDD: " fmt > > -static enum md_mitigation md_mode = MD_NONE; > +enum md_mitigation md_mode = MD_NONE; > +/* When switching from lower privilege level (cpl3) to higher (cpl0). */ > +u64 spec_ctrl_priv; > +EXPORT_SYMBOL_GPL(spec_ctrl_priv); > + > +/* When switching from higher to lower privilege level. */ > +u64 spec_ctrl_unpriv; > +EXPORT_SYMBOL_GPL(spec_ctrl_unpriv); I don't like exporting some arbitrary variables to the rest of the kernel. Do accessors instead pls. -- Regards/Gruss, Boris. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) -- ^ permalink raw reply [flat|nested] 7+ messages in thread
* [MODERATED] Re: ***UNCHECKED*** Re: [patch 3/8] [PATCH v1.3.1 3/7] Linux Patch 3 2018-04-18 15:37 ` [MODERATED] Re: ***UNCHECKED*** [patch 3/8] [PATCH v1.3.1 3/7] Linux Patch 3 Borislav Petkov @ 2018-04-18 16:00 ` Borislav Petkov 2018-04-18 18:07 ` [MODERATED] " Borislav Petkov 2018-04-19 20:51 ` [MODERATED] Re: ***UNCHECKED*** " Konrad Rzeszutek Wilk 1 sibling, 1 reply; 7+ messages in thread From: Borislav Petkov @ 2018-04-18 16:00 UTC (permalink / raw) To: speck On Wed, Apr 18, 2018 at 05:37:13PM +0200, speck for Borislav Petkov wrote: > And add to init_amd_zn() code reading MSR 0xc0011020, setting bit 10 to > 1b and then writing it back in the mdd_at_boot() case. We have a define > for that MSR already - MSR_AMD64_LS_CFG. ... and while you're at it, do the respective thing in: init_amd_bd() and there set MSR C001_0120, bit 54. and for F16h, add case 0x16: init_amd_jg(c); break; in the switch statement in init_amd() and in that new function set MSR C001_1020, bit 33. All for the mdd_at_boot() case. Thx. -- Regards/Gruss, Boris. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) -- ^ permalink raw reply [flat|nested] 7+ messages in thread
* [MODERATED] Re: [patch 3/8] [PATCH v1.3.1 3/7] Linux Patch 3 2018-04-18 16:00 ` [MODERATED] Re: ***UNCHECKED*** " Borislav Petkov @ 2018-04-18 18:07 ` Borislav Petkov 0 siblings, 0 replies; 7+ messages in thread From: Borislav Petkov @ 2018-04-18 18:07 UTC (permalink / raw) To: speck On Wed, Apr 18, 2018 at 06:00:29PM +0200, Borislav Petkov wrote: > On Wed, Apr 18, 2018 at 05:37:13PM +0200, speck for Borislav Petkov wrote: > > And add to init_amd_zn() code reading MSR 0xc0011020, setting bit 10 to > > 1b and then writing it back in the mdd_at_boot() case. We have a define > > for that MSR already - MSR_AMD64_LS_CFG. > > ... and while you're at it, do the respective thing in: > > init_amd_bd() and there set MSR C001_0120, bit 54. Correction: F15h should use the same MSR C001_1020, bit 54. Basically it is the same MSR but different bits for the different families. Thx. -- Regards/Gruss, Boris. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) -- ^ permalink raw reply [flat|nested] 7+ messages in thread
* [MODERATED] Re: ***UNCHECKED*** [patch 3/8] [PATCH v1.3.1 3/7] Linux Patch 3 2018-04-18 15:37 ` [MODERATED] Re: ***UNCHECKED*** [patch 3/8] [PATCH v1.3.1 3/7] Linux Patch 3 Borislav Petkov 2018-04-18 16:00 ` [MODERATED] Re: ***UNCHECKED*** " Borislav Petkov @ 2018-04-19 20:51 ` Konrad Rzeszutek Wilk 2018-04-19 21:10 ` [MODERATED] " Borislav Petkov 1 sibling, 1 reply; 7+ messages in thread From: Konrad Rzeszutek Wilk @ 2018-04-19 20:51 UTC (permalink / raw) To: speck On Wed, Apr 18, 2018 at 05:37:13PM +0200, speck for Borislav Petkov wrote: > On Thu, Apr 12, 2018 at 10:26:52PM -0400, speck for konrad.wilk_at_oracle.com wrote: > > diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c > > index 561cb228605a..73f76d0f5181 100644 > > --- a/arch/x86/kernel/cpu/bugs.c > > +++ b/arch/x86/kernel/cpu/bugs.c > > @@ -72,6 +72,9 @@ void __init check_bugs(void) > > */ > > if (!direct_gbpages) > > set_memory_4k((unsigned long)__va(0), 1); > > + > > + if (mdd_at_boot()) > > + wrmsrl(MSR_IA32_SPEC_CTRL, SPEC_CTRL_MDD); > > You can't do this on the common path as that would explode on !Intel. > > Instead, move that check to init_intel(). That won't work. The reason is that check_bugs first calls identify_boot_cpu() which calls identify_cpu() which calls this_cpu->init. Once identify_boot_cpu() is done _then_ it walks through the spectre_v2_select_mitigation() and ssb_select_mitigation(). > > And add to init_amd_zn() code reading MSR 0xc0011020, setting bit 10 to > 1b and then writing it back in the mdd_at_boot() case. We have a define > for that MSR already - MSR_AMD64_LS_CFG. I am thinking that perhaps we can add a new function: From 258854941d792de5de4ffdefcce8f52e8984e2e5 Mon Sep 17 00:00:00 2001 From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Date: Thu, 19 Apr 2018 16:47:38 -0400 Subject: [PATCH] x86/cpu: Add fix_this_cpu to be called _after_ check_bugs and at BSP. We do a lot of things in the check_bugs() - one of the first things we do is identify_boot_cpu() which calls identify_cpu() which calls this_cpu->init. Once identify_boot_cpu() is done _then_ it walks through the spectre_v2_select_mitigation() and alternative_assembler(). If there are some CPU fix ups _after_ spectre_v2 is done we can't activate those on the BSP as we have already called 'this_cpu->init'. Hence add a new function to fixup CPUs. Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> --- arch/x86/include/asm/processor.h | 1 + arch/x86/kernel/cpu/bugs.c | 1 + arch/x86/kernel/cpu/common.c | 8 ++++++++ arch/x86/kernel/cpu/cpu.h | 1 + 4 files changed, 11 insertions(+) diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h index b0ccd4847a58..b22bc9be2385 100644 --- a/arch/x86/include/asm/processor.h +++ b/arch/x86/include/asm/processor.h @@ -195,6 +195,7 @@ extern void init_amd_cacheinfo(struct cpuinfo_x86 *c); extern void detect_extended_topology(struct cpuinfo_x86 *c); extern void detect_ht(struct cpuinfo_x86 *c); +extern void apply_cpu_fixes(void); #ifdef CONFIG_X86_32 extern int have_cpuid_p(void); diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c index 4632c26e13e0..9c2987b4f7fa 100644 --- a/arch/x86/kernel/cpu/bugs.c +++ b/arch/x86/kernel/cpu/bugs.c @@ -77,6 +77,7 @@ void __init check_bugs(void) if (!direct_gbpages) set_memory_4k((unsigned long)__va(0), 1); #endif + apply_cpu_fixes(); } /* The kernel command line selection */ diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c index f7d80658cced..a59f7902a7fa 100644 --- a/arch/x86/kernel/cpu/common.c +++ b/arch/x86/kernel/cpu/common.c @@ -955,6 +955,12 @@ static void __init cpu_set_bug_bits(struct cpuinfo_x86 *c) setup_force_cpu_bug(X86_BUG_CPU_SSB); }; +void apply_cpu_fixes(void) +{ + if (this_cpu->c_bug_fix) + this_cpu->c_bug_fix(&boot_cpu_data); +} + /* * Do minimum CPU detection early. * Fields really needed: vendor, cpuid_level, family, model, mask, @@ -1311,6 +1317,8 @@ static void identify_cpu(struct cpuinfo_x86 *c) #ifdef CONFIG_NUMA numa_add_cpu(smp_processor_id()); #endif + if (this_cpu->c_bug_fix) + this_cpu->c_bug_fix(c); } /* diff --git a/arch/x86/kernel/cpu/cpu.h b/arch/x86/kernel/cpu/cpu.h index e806b11a99af..7081634caac5 100644 --- a/arch/x86/kernel/cpu/cpu.h +++ b/arch/x86/kernel/cpu/cpu.h @@ -15,6 +15,7 @@ struct cpu_dev { void (*c_identify)(struct cpuinfo_x86 *); void (*c_detect_tlb)(struct cpuinfo_x86 *); void (*c_bsp_resume)(struct cpuinfo_x86 *); + void (*c_bug_fix)(struct cpuinfo_x86 *); int c_x86_vendor; #ifdef CONFIG_X86_32 /* Optional vendor specific routine to obtain the cache size. */ -- 2.14.3 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [MODERATED] Re: [patch 3/8] [PATCH v1.3.1 3/7] Linux Patch 3 2018-04-19 20:51 ` [MODERATED] Re: ***UNCHECKED*** " Konrad Rzeszutek Wilk @ 2018-04-19 21:10 ` Borislav Petkov 2018-04-20 0:29 ` Konrad Rzeszutek Wilk 0 siblings, 1 reply; 7+ messages in thread From: Borislav Petkov @ 2018-04-19 21:10 UTC (permalink / raw) To: speck On Thu, Apr 19, 2018 at 04:51:37PM -0400, speck for Konrad Rzeszutek Wilk wrote: > That won't work. The reason is that check_bugs first calls > identify_boot_cpu() which calls identify_cpu() which calls > this_cpu->init. Once identify_boot_cpu() is done _then_ it walks > through the spectre_v2_select_mitigation() and ssb_select_mitigation(). So call md_select_mitigation() before identify_boot_cpu() in check_bugs(). -- Regards/Gruss, Boris. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) -- ^ permalink raw reply [flat|nested] 7+ messages in thread
* [MODERATED] Re: [patch 3/8] [PATCH v1.3.1 3/7] Linux Patch 3 2018-04-19 21:10 ` [MODERATED] " Borislav Petkov @ 2018-04-20 0:29 ` Konrad Rzeszutek Wilk 2018-04-20 2:39 ` Konrad Rzeszutek Wilk 0 siblings, 1 reply; 7+ messages in thread From: Konrad Rzeszutek Wilk @ 2018-04-20 0:29 UTC (permalink / raw) To: speck On Thu, Apr 19, 2018 at 11:10:03PM +0200, speck for Borislav Petkov wrote: > On Thu, Apr 19, 2018 at 04:51:37PM -0400, speck for Konrad Rzeszutek Wilk wrote: > > That won't work. The reason is that check_bugs first calls > > identify_boot_cpu() which calls identify_cpu() which calls > > this_cpu->init. Once identify_boot_cpu() is done _then_ it walks > > through the spectre_v2_select_mitigation() and ssb_select_mitigation(). > > So call md_select_mitigation() before identify_boot_cpu() in > check_bugs(). That will mean that the init_speculation_control() won't happen until _after_ md_select_mitigation(). And that throws a wrench in that as we need to rdmsrl(SPEC_CTRL) (which is right now done from spectre_v2_select_mitigation): 347 if (boot_cpu_has(X86_FEATURE_IBPB) || boot_cpu_has(X86_FEATURE_IBRS)) 348 rdmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base); 349 } And x86_spec_ctrl_base is then ORed in md_select_mitigation(). But spectre_v2_select_mitigation depends on identify_boot_cpu. md_select_mitigation depends on spectre_v2_select_mitigation. If I move the identify_boot_cpu before md_select_mitigation things get wonky. Maybe it will be easier to explain when I send out the patches? ^ permalink raw reply [flat|nested] 7+ messages in thread
* [MODERATED] Re: [patch 3/8] [PATCH v1.3.1 3/7] Linux Patch 3 2018-04-20 0:29 ` Konrad Rzeszutek Wilk @ 2018-04-20 2:39 ` Konrad Rzeszutek Wilk 0 siblings, 0 replies; 7+ messages in thread From: Konrad Rzeszutek Wilk @ 2018-04-20 2:39 UTC (permalink / raw) To: speck On Thu, Apr 19, 2018 at 08:29:21PM -0400, speck for Konrad Rzeszutek Wilk wrote: > On Thu, Apr 19, 2018 at 11:10:03PM +0200, speck for Borislav Petkov wrote: > > On Thu, Apr 19, 2018 at 04:51:37PM -0400, speck for Konrad Rzeszutek Wilk wrote: > > > That won't work. The reason is that check_bugs first calls > > > identify_boot_cpu() which calls identify_cpu() which calls > > > this_cpu->init. Once identify_boot_cpu() is done _then_ it walks > > > through the spectre_v2_select_mitigation() and ssb_select_mitigation(). > > > > So call md_select_mitigation() before identify_boot_cpu() in > > check_bugs(). > > That will mean that the init_speculation_control() won't happen > until _after_ md_select_mitigation(). > > And that throws a wrench in that as we need to rdmsrl(SPEC_CTRL) > (which is right now done from spectre_v2_select_mitigation): > > 347 if (boot_cpu_has(X86_FEATURE_IBPB) || boot_cpu_has(X86_FEATURE_IBRS)) > 348 rdmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base); > 349 } > > And x86_spec_ctrl_base is then ORed in md_select_mitigation(). > > But > spectre_v2_select_mitigation depends on identify_boot_cpu. > md_select_mitigation depends on spectre_v2_select_mitigation. > > If I move the identify_boot_cpu before md_select_mitigation things > get wonky. > > Maybe it will be easier to explain when I send out the patches? I fixed the circular dependency I mentioned here in the patches. (See the v2 posting, Patch #3). But then I realized we still have one more issue - mainly the X86_FEATURE_MDD. That is you asked in one of the patches review to guard the md_parse_cmdline with: if (!boot_cpu_has(X86_FEATURE_MDD)) return MD_CMD_NONE; And on AMD the X86_FEATURE_MDD (with the v2 patch set just sent) it gets set in c_init as it detects which family can safely touch the MSR_AMD64_LS_CFG MSR: if (!rdmsrl_safe(MSR_AMD64_LS_CFG, &msr_MSR_AMD64_LS_CFG_val)) set_cpu_cap(c, X86_FEATURE_MDD); (as the CPUID.7.EDX[31] is not going to be set on AMD I assume). See the v2 - Patch #9. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-04-20 2:40 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <20180418141551.07CBB6111A@crypto-ml.lab.linutronix.de> 2018-04-18 15:37 ` [MODERATED] Re: ***UNCHECKED*** [patch 3/8] [PATCH v1.3.1 3/7] Linux Patch 3 Borislav Petkov 2018-04-18 16:00 ` [MODERATED] Re: ***UNCHECKED*** " Borislav Petkov 2018-04-18 18:07 ` [MODERATED] " Borislav Petkov 2018-04-19 20:51 ` [MODERATED] Re: ***UNCHECKED*** " Konrad Rzeszutek Wilk 2018-04-19 21:10 ` [MODERATED] " Borislav Petkov 2018-04-20 0:29 ` Konrad Rzeszutek Wilk 2018-04-20 2:39 ` Konrad Rzeszutek Wilk
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.