Re: [PATCH] KVM: SVM: fix cr8 intercept window

["Radim Krčmář" <rkrcmar@redhat.com>]

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čmář ha scritto:
> > >2014-03-11 22:05-0300, Marcelo Tosatti:
> > >>On Tue, Mar 11, 2014 at 07:11:18PM +0100, Radim Krčmář wrote:
> > >>>We always disable cr8 intercept in its handler, but only re-enable 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 higher
> > >>>priority task.
> > >>>
> > >>>Fix this by disabling intercepts in the same function that re-enables
> > >>>them when needed. This fixes BSOD in Windows 2008.
> > >>>
> > >>>Cc: <stable@vger.kernel.org>
> > >>>Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
> > >>>---
> > >>> 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 = kvm_get_cr8(&svm->vcpu);
> > >>> 	/* instruction emulation calls kvm_set_cr8() */
> > >>> 	r = 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;
> > >>>-	}
> > 
> > I think that the old code here makes little sense, and for two reasons:
> > 
> I agree that old code is wrong and the patch looks correct, but I only
> 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 and
 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.