From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marcelo Tosatti Subject: Re: [PATCH] KVM: SVM: fix cr8 intercept window Date: Wed, 12 Mar 2014 11:54:41 -0300 Message-ID: <20140312145441.GA30707@amt.cnet> References: <1394561478-8815-1-git-send-email-rkrcmar@redhat.com> <20140312010513.GA8131@amt.cnet> <20140312104047.GA7488@potion.brq.redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org, pbonzini@redhat.com, gleb@kernel.org, stable@vger.kernel.org To: Radim =?utf-8?B?S3LEjW3DocWZ?= Return-path: Content-Disposition: inline In-Reply-To: <20140312104047.GA7488@potion.brq.redhat.com> Sender: stable-owner@vger.kernel.org List-Id: kvm.vger.kernel.org On Wed, Mar 12, 2014 at 11:40:48AM +0100, Radim Kr=C4=8Dm=C3=A1=C5=99 w= rote: > 2014-03-11 22:05-0300, Marcelo Tosatti: > > On Tue, Mar 11, 2014 at 07:11:18PM +0100, Radim Kr=C4=8Dm=C3=A1=C5=99= wrote: > > > We always disable cr8 intercept in its handler, but only re-enabl= e it > > > if handling KVM_REQ_EVENT, so there can be a window where we do n= ot > > > intercept cr8 writes, which allows an interrupt to disrupt a high= er > > > priority task. > > >=20 > > > Fix this by disabling intercepts in the same function that re-ena= bles > > > them when needed. This fixes BSOD in Windows 2008. > > >=20 > > > Cc: > > > Signed-off-by: Radim Kr=C4=8Dm=C3=A1=C5=99 > > > --- > > > arch/x86/kvm/svm.c | 6 +++--- > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > >=20 > > > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c > > > index 64d9bb9..f676c18 100644 > > > --- a/arch/x86/kvm/svm.c > > > +++ b/arch/x86/kvm/svm.c > > > @@ -3003,10 +3003,8 @@ static int cr8_write_interception(struct v= cpu_svm *svm) > > > u8 cr8_prev =3D kvm_get_cr8(&svm->vcpu); > > > /* instruction emulation calls kvm_set_cr8() */ > > > r =3D cr_interception(svm); > > > - if (irqchip_in_kernel(svm->vcpu.kvm)) { > > > - clr_cr_intercept(svm, INTERCEPT_CR8_WRITE); > > > + if (irqchip_in_kernel(svm->vcpu.kvm)) > > > return r; > > > - } > > > if (cr8_prev <=3D kvm_get_cr8(&svm->vcpu)) > > > return r; > > > kvm_run->exit_reason =3D KVM_EXIT_SET_TPR; > > > @@ -3568,6 +3566,8 @@ static void update_cr8_intercept(struct kvm= _vcpu *vcpu, int tpr, int irr) > > > if (is_guest_mode(vcpu) && (vcpu->arch.hflags & HF_VINTR_MASK)) > > > return; > > > =20 > > > + clr_cr_intercept(svm, INTERCEPT_CR8_WRITE); > > > + > > > if (irr =3D=3D -1) > > > return; > >=20 > > Shouldnt IRR be injected if TPR < IRR ? (via KVM_REQ_EVENT). > >=20 > > 1) IRR has interrupt 10. > > 2) TPR now 9 due to CR8 write. > > 3) 10 should be injected. >=20 > Definitely should, and we will set KVM_REQ_EVENT through kvm_set_cr8(= ) > if we lower the TPR. > (I checked that the bug isn't in apic_update_ppr().) Yep. > > Also not clearing the intercept can cause continuous CR8 writes to=20 > > exit until KVM_REQ_EVENT ?=20 >=20 > It is intended, I suppose this is because we run with V_INTR_MASKING,= so > writes to CR8 only affect V_TPR register; guest then raises it once m= ore > and APIC incorrectly gives us low priority interrupt. Reviewed-by: Marcelo Tosatti