* [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.