From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from aserp2120.oracle.com ([141.146.126.78]) by Galois.linutronix.de with esmtps (TLS1.2:RSA_AES_256_CBC_SHA256:256) (Exim 4.80) (envelope-from ) id 1fCbq9-0003Lk-Hy for speck@linutronix.de; Sun, 29 Apr 2018 04:13:26 +0200 Received: from pps.filterd (aserp2120.oracle.com [127.0.0.1]) by aserp2120.oracle.com (8.16.0.22/8.16.0.22) with SMTP id w3T2DIRd084577 for ; Sun, 29 Apr 2018 02:13:18 GMT Received: from aserv0022.oracle.com (aserv0022.oracle.com [141.146.126.234]) by aserp2120.oracle.com with ESMTP id 2hmgxfh9t7-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for ; Sun, 29 Apr 2018 02:13:18 +0000 Received: from aserv0121.oracle.com (aserv0121.oracle.com [141.146.126.235]) by aserv0022.oracle.com (8.14.4/8.14.4) with ESMTP id w3T2DHRg021181 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for ; Sun, 29 Apr 2018 02:13:17 GMT Received: from abhmp0004.oracle.com (abhmp0004.oracle.com [141.146.116.10]) by aserv0121.oracle.com (8.14.4/8.13.8) with ESMTP id w3T2DHcU027970 for ; Sun, 29 Apr 2018 02:13:17 GMT Date: Sat, 28 Apr 2018 22:13:11 -0400 From: Konrad Rzeszutek Wilk Subject: [MODERATED] Re: [PATCH v6 07/11] [PATCH v6 07/10] Linux Patch #7 Message-ID: <20180429021257.GA6935@localhost.localdomain> References: <20180426234831.516018436@localhost.localdomain> MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit To: speck@linutronix.de List-ID: On Sat, Apr 28, 2018 at 09:38:29PM +0200, speck for Thomas Gleixner wrote: > On Thu, 26 Apr 2018, speck for konrad.wilk_at_oracle.com wrote: > > void __init check_bugs(void) > > { > > - identify_boot_cpu(); > > - > > I know that you did this for AMD, but it does not make it less wrong. The > right thing to do is to store the MSR_LS_CTR bit somewhere during the > identification and then use it here. That keeps the fiddling with these > MSRs in this code and not in some weird latch mode burried in the CPU init > code. Please keep in mind that the AP processors still need to engage the latch mode for the CPU - and they don't don't venture in the 'check_bugs' code. Which means "some weird latch mode burried in the CPU init code" still has to happen. It could be something as simple as this for BSP: 43 void __init check_bugs(void) 44 { 45 identify_boot_cpu(); 46 47 if (!IS_ENABLED(CONFIG_SMP)) { 48 pr_info("CPU: "); 49 print_cpu_info(&boot_cpu_data); 50 } 51 52 /* 53 * Read the SPEC_CTRL MSR to account for reserved bits which may have 54 * unknown values. 55 */ 56 if (boot_cpu_has(X86_FEATURE_IBRS)) 57 rdmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base); 58 59 /* 60 * Select proper mitigation for any exposure to the Speculative Store 61 * Bypass vulnerability. 62 */ 63 ssb_select_mitigation(); 64 65 if (cpu_has(c, X86_FEATURE_SPEC_STORE_BYPASS_DISABLE)) { 66 if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL) { 67 x86_set_spec_ctrl(SPEC_CTRL_RDS); 68 } else if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD) { 69 u64 val; 70 71 rdmsrl(MSR_AMD64_LS_CFG, val); 72 val |= (1 << x86_msr_ls_ctr_bit); 73 wrmsrl(MSR_AMD64_LS_CFG, val); 74 } 75 } [I actually would likely move this to some function called ssb_latch_mitigation_boot_cpu() or ssb_latch_mitigation()] 76 77 /* Select the proper spectre mitigation before patching alternatives */ 78 spectre_v2_select_mitigation(); For the AP processors: in init_amd: if (cpu_has(c, X86_FEATURE_SPEC_STORE_BYPASS_DISABLE)) msr_set_bit(MSR_AMD64_LS_CFG, x86_msr_ls_ctr_bit); for the AP CPUs this is needed in init_intel: 775 if (cpu_has(c, X86_FEATURE_SPEC_STORE_BYPASS_DISABLE)) 776 x86_set_spec_ctrl(SPEC_CTRL_RDS); And to figure out which bits to fiddle, for AMD we would do this in in early_init_amd: 678 if (c->x86 >= 0x15 && c->x86 <= 0x17) { 679 set_cpu_cap(c, X86_FEATURE_RDS); 680 switch (c->x86) { 681 case 0x15: x86_msr_ls_ctr_bit = 54; break; 682 case 0x16: x86_msr_ls_ctr_bit = 33; break; 683 case 0x17: x86_msr_ls_ctr_bit = 10; break; 684 } 685 } for Intel we already identify the X86_FEATURE_RDS thanks to reading during early init the CPUID.7.edx in get_cpu_cap. That ties all together.. Taking this one step further - the init_amd or init_intel could call the ssb_latch_mitigation_boot_cpu() function (which now should be called ssb_latch_mitigation) - and that would put all the bit/MSR fiddling code in one location (bugs.c). I think this is what you had in mind. I will do those changes tomorrow night and try them out, unless you had in mind a different way?.