[MODERATED] Re: [patch 07/11] [PATCH v2 07/10] Linux Patch #7

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Fri, Apr 20, 2018 at 07:42:42PM +0200, speck for Borislav Petkov wrote:
> On Thu, Apr 19, 2018 at 10:25:47PM -0400, speck for konrad.wilk_at_oracle.com wrote:
> > 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.
> 
> Konrad, this is still unnecessary and adding superfluous complexity to
> an already crazy early boot path. Let me clarify the flow:
> 
> First you are on the BSP:
> 
> setup_arch()
> |-> early_cpu_init
> |-> early_identify_cpu
> |-> cpu_set_bug_bits
> 
> now you have all the X86_BUG bits set so that you can test them in the
> functions later.

I am not worrying about the X86_BUG, I am thinking about the
X86_FEATURE_MDD. See below.
> 
> Then, you're still on the BSP and can do check_bugs() with all cmdline
> options picking apart etc etc.
> 
> setup_arch()
> |-> check_bugs()
> |-> ssb_select_mitigation()
> |-> identify_boot_cpu()			<--- HERE you call ->c_init() on the BSP
> |-> spectre_v2_select_mitigation
> 
> In that order!
> 
> So, all you wanna do works without adding this new function pointer.

With that I will need to add in early_init_amd the code to
set_cpu_cap(c, X86_FEATURE_MDD) based on reading the MSR_AMD64_LS_CFG.

That is needed as the 'ssb_select_mitigation' ends calling
ssb_parse_cmdline which immediately exits if:

381 static enum ssb_mitigation_cmd __init ssb_parse_cmdline(void)
..
391         if (!boot_cpu_has(X86_FEATURE_MDD))	<===
392                 return SSB_CMD_NONE;


Adding this:
diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index e207eb2e8011..6a800a71b6c0 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -676,6 +676,15 @@ static void early_init_amd(struct cpuinfo_x86 *c)
                set_cpu_bug(c, X86_BUG_AMD_E400);
 
        early_detect_mem_encrypt(c);
+
+       switch (c->x86) {
+       case 0x15:
+       case 0x16:
+       case 0x17:
+               if (!rdmsrl_safe(MSR_AMD64_LS_CFG, &msr_MSR_AMD64_LS_CFG_val))
+                       set_cpu_cap(c, X86_FEATURE_MDD);
+       default:
+       }
 }
 
 static void init_amd_k8(struct cpuinfo_x86 *c)

in conjunction with your flow will make it work just fine.