From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mx2.suse.de ([195.135.220.15]) by Galois.linutronix.de with esmtps (TLS1.0:DHE_RSA_CAMELLIA_256_CBC_SHA1:256) (Exim 4.80) (envelope-from ) id 1fIWFC-0000Xl-5v for speck@linutronix.de; Tue, 15 May 2018 11:27:43 +0200 Received: from relay2.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 6D459AC21 for ; Tue, 15 May 2018 09:27:36 +0000 (UTC) Date: Tue, 15 May 2018 11:27:24 +0200 From: Borislav Petkov Subject: [MODERATED] Re: [patch 13/15] Hidden 13 Message-ID: <20180515092724.GA11988@pd.tnic> 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: quoted-printable 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 >=20 > 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 s/Though/However,/ > and the bitmask is inverted for no real reason. Aside of that it is missing > 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. >=20 > Add the STIBP bit if supported and use the mask in x86_spec_ctrl_set_guest() > to sanitize the value which is supplied by the guest. >=20 > Signed-off-by: Thomas Gleixner > --- > arch/x86/kernel/cpu/bugs.c | 22 +++++++++++++++++----- > 1 file changed, 17 insertions(+), 5 deletions(-) >=20 > --- 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); > =20 > + /* Allow STIBP in MSR_SPEC_CTRL if supported */ > + if (boot_cpu_has(X86_FEATURE_STIBP)) > + x86_spec_ctrl_mask |=3D SPEC_CTRL_STIBP; > + > /* Select the proper spectre mitigation before patching alternatives */ > spectre_v2_select_mitigation(); > =20 > @@ -134,19 +138,27 @@ static enum spectre_v2_mitigation spectr > SPECTRE_V2_NONE; > =20 > void > -x86_virt_spec_ctrl(u64 guest_spec_ctrl, u64 guest_virt_spec_ctrl, bool gue= st) > +x86_virt_spec_ctrl(u64 guest_spec_ctrl, u64 guest_virt_spec_ctrl, bool set= guest) > { > u64 hostssbd =3D ssbd_tif_to_spec_ctrl(current_thread_info()->flags); > - u64 msr, host =3D x86_spec_ctrl_base; > + u64 msr, guest, host =3D x86_spec_ctrl_base; > =20 > /* 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 =3D host & ~x86_spec_ctrl_mask; > + guest |=3D guest_spec_ctrl & x86_spec_ctrl_mask; > + > /* SSBD controlled in MSR_SPEC_CTRL */ > if (static_cpu_has(X86_FEATURE_SPEC_CTRL_SSBD)) > host |=3D hostssbd; > =20 > - if (host !=3D guest_spec_ctrl) { > - msr =3D guest ? guest_spec_ctrl : host; > + if (host !=3D guest) { > + msr =3D setguest ? guest : host; Just a nitpick: msrval =3D setguest ? guest : host; wrmsrl(MSR_IA32_SPEC_CTRL, msrval); calling it "msrval" is a bit clearer as it shows that's you're selecting the MSR *value* and not the MSR itself. (And yes, we do select which MSRs to access in other places). With that Reviewed-by: Borislav Petkov --=20 Regards/Gruss, Boris. SUSE Linux GmbH, GF: Felix Imend=C3=B6rffer, Jane Smithard, Graham Norton, HR= B 21284 (AG N=C3=BCrnberg) --=20