From mboxrd@z Thu Jan 1 00:00:00 1970 From: Radim =?utf-8?B?S3LEjW3DocWZ?= Subject: Re: [PATCH] KVM: SVM: fix cr8 intercept window Date: Thu, 13 Mar 2014 18:08:50 +0100 Message-ID: <20140313170846.GA23297@potion.brq.redhat.com> References: <1394561478-8815-1-git-send-email-rkrcmar@redhat.com> <20140312010513.GA8131@amt.cnet> <20140312104047.GA7488@potion.brq.redhat.com> <53209741.3060204@redhat.com> <20140313135206.GB30596@minantech.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: Paolo Bonzini , Marcelo Tosatti , linux-kernel@vger.kernel.org, kvm@vger.kernel.org, stable@vger.kernel.org To: Gleb Natapov Return-path: Content-Disposition: inline In-Reply-To: <20140313135206.GB30596@minantech.com> Sender: stable-owner@vger.kernel.org List-Id: kvm.vger.kernel.org 2014-03-13 15:52+0200, Gleb Natapov: > On Wed, Mar 12, 2014 at 06:20:01PM +0100, Paolo Bonzini wrote: > > Il 12/03/2014 11:40, Radim Kr=C4=8Dm=C3=A1=C5=99 ha scritto: > > >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-enab= le it > > >>>if handling KVM_REQ_EVENT, so there can be a window where we do = not > > >>>intercept cr8 writes, which allows an interrupt to disrupt a hig= her > > >>>priority task. > > >>> > > >>>Fix this by disabling intercepts in the same function that re-en= ables > > >>>them when needed. This fixes BSOD in Windows 2008. > > >>> > > >>>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(-) > > >>> > > >>>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 = vcpu_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; > > >>>- } > >=20 > > I think that the old code here makes little sense, and for two reas= ons: > >=20 > I agree that old code is wrong and the patch looks correct, but I onl= y > see how the bug may cause pending IRR to not be delivered in time, > not how interrupt can disrupt a higher priority task. True, the commit message is bad. We BSOD only because IRR is not injected right after lowering the TPR and code depends on values that had to be computed in it. Paolo, can you change the last sentence to ", which means we don't inject pending IRR immediately."? (or do we just forget it?) Thanks. --- My model of what is happening had misconceptions about Windows, KVM an= d SVM ... explained BSOD in IRQL 0xc more directly though, so it lasted till Paolo's review -- the list of things to (re)read is long.