From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from userp2120.oracle.com ([156.151.31.85]) by Galois.linutronix.de with esmtps (TLS1.2:RSA_AES_256_CBC_SHA256:256) (Exim 4.80) (envelope-from ) id 1fInLF-0003iL-Ei for speck@linutronix.de; Wed, 16 May 2018 05:43:06 +0200 Received: from pps.filterd (userp2120.oracle.com [127.0.0.1]) by userp2120.oracle.com (8.16.0.22/8.16.0.22) with SMTP id w4G3friM175192 for ; Wed, 16 May 2018 03:42:58 GMT Received: from userv0022.oracle.com (userv0022.oracle.com [156.151.31.74]) by userp2120.oracle.com with ESMTP id 2hx29w2vqd-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for ; Wed, 16 May 2018 03:42:58 +0000 Received: from userv0122.oracle.com (userv0122.oracle.com [156.151.31.75]) by userv0022.oracle.com (8.14.4/8.14.4) with ESMTP id w4G3gwFd027974 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for ; Wed, 16 May 2018 03:42:58 GMT Received: from abhmp0014.oracle.com (abhmp0014.oracle.com [141.146.116.20]) by userv0122.oracle.com (8.14.4/8.14.4) with ESMTP id w4G3gvcF026675 for ; Wed, 16 May 2018 03:42:58 GMT Date: Tue, 15 May 2018 23:42:56 -0400 From: Konrad Rzeszutek Wilk Subject: [MODERATED] Re: [patch 13/15] Hidden 13 Message-ID: <20180516034256.GD26939@char.us.oracle.com> References: <20180513140048.543641807@linutronix.de> <20180513140539.304778544@linutronix.de> MIME-Version: 1.0 In-Reply-To: <20180513140539.304778544@linutronix.de> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit To: speck@linutronix.de List-ID: On Sun, May 13, 2018 at 04:01:01PM +0200, speck for Thomas Gleixner wrote: > Subject: [patch 13/15] x86/bugs: Rework spec_ctrl base and mask logic > From: Thomas Gleixner > > x86_spec_ctrL_mask is intended to mask out bits from a MSR_SPEC_CTRL value > which are not to be modified. Though the implementation is not really used > and the bitmask is inverted for no real reason. Aside of that it is missing This is due to: Subject: [patch 11/15] x86/bugs: Expose x86_spec_ctrl_base directly Would it make sense to remove the inversion from that patch? Which naturally would change the logic here.. > the STIBP bit if it is supported by the platform, so if the mask would be > used in x86_virt_spec_ctrl() then it would prevent a guest from setting > STIBP. Ouch. > > Add the STIBP bit if supported and use the mask in x86_spec_ctrl_set_guest() What OS actually uses that bit? > to sanitize the value which is supplied by the guest. > > Signed-off-by: Thomas Gleixner > --- > arch/x86/kernel/cpu/bugs.c | 22 +++++++++++++++++----- > 1 file changed, 17 insertions(+), 5 deletions(-) > > --- a/arch/x86/kernel/cpu/bugs.c > +++ b/arch/x86/kernel/cpu/bugs.c > @@ -68,6 +68,10 @@ void __init check_bugs(void) > if (boot_cpu_has(X86_FEATURE_MSR_SPEC_CTRL)) > rdmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base); > > + /* Allow STIBP in MSR_SPEC_CTRL if supported */ > + if (boot_cpu_has(X86_FEATURE_STIBP)) > + x86_spec_ctrl_mask |= SPEC_CTRL_STIBP; > + > /* Select the proper spectre mitigation before patching alternatives */ > spectre_v2_select_mitigation(); > > @@ -134,19 +138,27 @@ static enum spectre_v2_mitigation spectr > SPECTRE_V2_NONE; > > void > -x86_virt_spec_ctrl(u64 guest_spec_ctrl, u64 guest_virt_spec_ctrl, bool guest) > +x86_virt_spec_ctrl(u64 guest_spec_ctrl, u64 guest_virt_spec_ctrl, bool setguest) > { > u64 hostssbd = ssbd_tif_to_spec_ctrl(current_thread_info()->flags); > - u64 msr, host = x86_spec_ctrl_base; > + u64 msr, guest, host = x86_spec_ctrl_base; > > /* Is MSR_SPEC_CTRL implemented ? */ > if (static_cpu_has(X86_FEATURE_MSR_SPEC_CTRL)) { > + /* > + * Restrict guest_spec_ctrl to supported values. Clear the > + * modifiable bits in the host base value and or the > + * modifiable bits from the guest value. > + */ > + guest = host & ~x86_spec_ctrl_mask; > + guest |= guest_spec_ctrl & x86_spec_ctrl_mask; > + > /* SSBD controlled in MSR_SPEC_CTRL */ > if (static_cpu_has(X86_FEATURE_SPEC_CTRL_SSBD)) > host |= hostssbd; > > - if (host != guest_spec_ctrl) { > - msr = guest ? guest_spec_ctrl : host; > + if (host != guest) { > + msr = setguest ? guest : host; > wrmsrl(MSR_IA32_SPEC_CTRL, msr); > } > } > @@ -493,7 +505,7 @@ static enum ssb_mitigation __init __ssb_ > switch (boot_cpu_data.x86_vendor) { > case X86_VENDOR_INTEL: > x86_spec_ctrl_base |= SPEC_CTRL_SSBD; > - x86_spec_ctrl_mask &= ~SPEC_CTRL_SSBD; > + x86_spec_ctrl_mask |= SPEC_CTRL_SSBD; > wrmsrl(MSR_IA32_SPEC_CTRL, x86_spec_ctrl_base); > break; > case X86_VENDOR_AMD: >