Re: KVM: x86: vcpu state writeback should be aware of REQ_NMI

[Gleb Natapov <gleb@redhat.com>]

On Thu, Mar 24, 2011 at 03:33:35PM +0200, Avi Kivity wrote:
> On 03/24/2011 03:27 PM, Gleb Natapov wrote:
> >On Thu, Mar 24, 2011 at 09:47:00AM -0300, Marcelo Tosatti wrote:
> >>
> >>  Since "Fix race between nmi injection and enabling nmi window", pending NMI
> >>  can be represented in KVM_REQ_NMI vcpu->requests bit.
> >>
> >>  When setting vcpu state via SET_VCPU_EVENTS, for example during reset,
> >>  the REQ_NMI bit should be cleared otherwise pending NMI is transferred
> >>  to nmi_pending upon vcpu entry.
> >>
> >>  Also should consider requests bit on runnable conditional.
> >>
> >>  BZ: http://bugzilla.redhat.com/show_bug.cgi?id=684719
> >>
> >Looks like we need to clear request bit on cpu reset too. KVM_REQ_NMI
> >start to become more complicated that it was initially. May be replaced
> >it with something like this:
> >
> >
> >diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> >index 1b8b16a..6a66d19 100644
> >--- a/arch/x86/kvm/x86.c
> >+++ b/arch/x86/kvm/x86.c
> >@@ -5151,6 +5151,7 @@ static void kvm_put_guest_xcr0(struct kvm_vcpu *vcpu)
> >  static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
> >  {
> >  	int r;
> >+	int nmi_pending;
> >  	bool req_int_win = !irqchip_in_kernel(vcpu->kvm)&&
> >  		vcpu->run->request_interrupt_window;
> >
> >@@ -5188,19 +5189,19 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
> >  			r = 1;
> >  			goto out;
> >  		}
> >-		if (kvm_check_request(KVM_REQ_NMI, vcpu))
> >-			vcpu->arch.nmi_pending = true;
> >  	}
> >
> >  	r = kvm_mmu_reload(vcpu);
> >  	if (unlikely(r))
> >  		goto out;
> >
> >+	nmi_pending = vcpu->arch.nmi_pending;
> >+
> 
> ACCESS_ONCE() to prevent compiler cleverness
> 
> >  	if (kvm_check_request(KVM_REQ_EVENT, vcpu) || req_int_win) {
> >  		inject_pending_event(vcpu);
> 
> This can use a newer vcpu->arch.nmi_pending.
> 
We can do inject_pending_event(vcpu, nmi_pending); but is this a problem
that newer version of vcpu->arch.nmi_pending will be used? If NMI can be
injected it will - no problem. It it can't it will be delayed till the
next guest entry - no problem NMI is async anyway.

> >
> >  		/* enable NMI/IRQ window open exits if needed */
> >-		if (vcpu->arch.nmi_pending)
> >+		if (nmi_pending)
> >  			kvm_x86_ops->enable_nmi_window(vcpu);
> >  		else if (kvm_cpu_has_interrupt(vcpu) || req_int_win)
> >  			kvm_x86_ops->enable_irq_window(vcpu);
> 
> -- 
> error compiling committee.c: too many arguments to function

--
			Gleb.