From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from userp2130.oracle.com ([156.151.31.86]) by Galois.linutronix.de with esmtps (TLS1.2:RSA_AES_256_CBC_SHA256:256) (Exim 4.80) (envelope-from ) id 1f9Z4i-0000zM-SU for speck@linutronix.de; Fri, 20 Apr 2018 18:39:54 +0200 Received: from pps.filterd (userp2130.oracle.com [127.0.0.1]) by userp2130.oracle.com (8.16.0.22/8.16.0.22) with SMTP id w3KGVNW5074951 for ; Fri, 20 Apr 2018 16:39:46 GMT Received: from userv0021.oracle.com (userv0021.oracle.com [156.151.31.71]) by userp2130.oracle.com with ESMTP id 2hdrxp5ge2-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for ; Fri, 20 Apr 2018 16:39:46 +0000 Received: from userv0121.oracle.com (userv0121.oracle.com [156.151.31.72]) by userv0021.oracle.com (8.14.4/8.14.4) with ESMTP id w3KGdjWd010108 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for ; Fri, 20 Apr 2018 16:39:45 GMT Received: from abhmp0007.oracle.com (abhmp0007.oracle.com [141.146.116.13]) by userv0121.oracle.com (8.14.4/8.13.8) with ESMTP id w3KGdjY1023495 for ; Fri, 20 Apr 2018 16:39:45 GMT Date: Fri, 20 Apr 2018 12:39:40 -0400 From: Konrad Rzeszutek Wilk Subject: [MODERATED] Re: [patch 04/11] [PATCH v2 04/10] Linux Patch #4 Message-ID: <20180420163936.GA4615@localhost.localdomain> References: <20180420022613.057637144@localhost.localdomain> <20180420161533.GK13977@pd.tnic> MIME-Version: 1.0 In-Reply-To: <20180420161533.GK13977@pd.tnic> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable To: speck@linutronix.de List-ID: On Fri, Apr 20, 2018 at 06:15:33PM +0200, speck for Borislav Petkov wrote: > On Thu, Apr 19, 2018 at 10:25:44PM -0400, speck for konrad.wilk_at_oracle.c= om wrote: > > KVM/SVM/VMX/x86/spectre_v2: Support the combination of guest IBRS and our= s. > >=20 > > A guest may modify the SPEC_CTRL MSR from the value used by the > > kernel. Since we don't use IBRS, this means a value of zero > > is what we need in the host. > >=20 > > But the 336996-Speculative-Execution-Side-Channel-Mitigations.pdf > > refers to the other bits as reserved so we should respect the > > boot time SPEC_CTRL value and use that. > >=20 > > This allows us to deal with future extensions to the SPEC_CTRL > > interface if any at all. > >=20 > > Signed-off-by: Konrad Rzeszutek Wilk > > --- > > v2: New patch > > --- > > arch/x86/kvm/svm.c | 6 +++--- > > arch/x86/kvm/vmx.c | 6 +++--- > > 2 files changed, 6 insertions(+), 6 deletions(-) > >=20 > > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c > > index be9c839e2c89..f666b4c21559 100644 > > --- a/arch/x86/kvm/svm.c > > +++ b/arch/x86/kvm/svm.c > > @@ -5401,7 +5401,7 @@ static void svm_vcpu_run(struct kvm_vcpu *vcpu) > > * is no need to worry about the conditional branch over the wrmsr > > * being speculatively taken. > > */ > > - if (svm->spec_ctrl) > > + if (svm->spec_ctrl || need_spec_ctrl_acc()) > > native_wrmsrl(MSR_IA32_SPEC_CTRL, svm->spec_ctrl); >=20 > Instead of those silly set/clear helpers, why not do this: >=20 > if (svm->spec_ctrl) > x86_enable_ibrs(); Wouldn't we leak our MD state to the guest? That is the guest may have cleared everything (svm->spec_ctrl is zero when we VMEXIT), and now we would be running it with MD bit set? >=20 > and hide in x86_enable_ibrs() all that logic of checking > x86_spec_ctrl_base and writing the MSR and whatever else we will need in > the future? I think we need two helpers with this: x86_set_spec_ctrl_for_unpriv(svm->spec_ctrl); asm (.. VMENTER VMEXIT event ); x86_set_spec_ctrl_for_priv(svm->spec_ctrl); ? >=20 > This way you don't need to export helper functions any bit defines > around the place - just the enable/disable functions which hide the > whole logic? >=20 > > asm volatile ( > > @@ -5514,8 +5514,8 @@ static void svm_vcpu_run(struct kvm_vcpu *vcpu) > > if (unlikely(!msr_write_intercepted(vcpu, MSR_IA32_SPEC_CTRL))) > > svm->spec_ctrl =3D native_read_msr(MSR_IA32_SPEC_CTRL); > > =20 > > - if (svm->spec_ctrl) > > - native_wrmsrl(MSR_IA32_SPEC_CTRL, 0); > > + if (svm->spec_ctrl || need_spec_ctrl_acc()) > > + native_wrmsrl(MSR_IA32_SPEC_CTRL, clear_spec_ctrl(SPEC_CTRL_IBRS)); >=20 > Same here: >=20 > if (svm->spec_ctrl) > x86_disable_ibrs(); >=20 > And so on... >=20 > --=20 > Regards/Gruss, > Boris. >=20 > SUSE Linux GmbH, GF: Felix Imend=C3=B6rffer, Jane Smithard, Graham Norton, = HRB 21284 (AG N=C3=BCrnberg) > --=20