From mboxrd@z Thu Jan  1 00:00:00 1970
From: Sheng Yang <sheng@linux.intel.com>
Subject: Re: [PATCH 2/2] Interrupt unhalts vcpu when it shouldn't
Date: Tue, 24 Mar 2009 13:24:34 +0800
Message-ID: <200903241324.35410.sheng@linux.intel.com>
References: <20090323101205.25798.69237.stgit@dhcp-1-237.tlv.redhat.com> <49C79D40.4000101@redhat.com> <49C7A816.2090605@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain;
  charset="utf-8"
Content-Transfer-Encoding: 7bit
Cc: Gleb Natapov <gleb@redhat.com>, kvm@vger.kernel.org,
	Marcelo Tosatti <mtosatti@redhat.com>
To: Avi Kivity <avi@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mga07.intel.com ([143.182.124.22]:31137 "EHLO
	azsmga101.ch.intel.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org
	with ESMTP id S1750797AbZCXFYl (ORCPT <rfc822;kvm@vger.kernel.org>);
	Tue, 24 Mar 2009 01:24:41 -0400
In-Reply-To: <49C7A816.2090605@redhat.com>
Content-Disposition: inline
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Monday 23 March 2009 23:17:42 Avi Kivity wrote:
> Avi Kivity wrote:
> > Gleb Natapov wrote:
> >> kvm_vcpu_block() unhalts vpu on an interrupt/timer without checking
> >> if interrupt window is actually opened.
> >>
> >>
> >> +static int svm_interrupt_allowed(struct kvm_vcpu *vcpu)
> >> +{
> >> +    struct vcpu_svm *svm = to_svm(vcpu);
> >> +    struct vmcb *vmcb = svm->vmcb;
> >> +    return (vmcb->save.rflags & X86_EFLAGS_IF) && +
> >> !(vmcb->control.int_state & SVM_INTERRUPT_SHADOW_MASK) &&
> >> +        (svm->vcpu.arch.hflags & HF_GIF_MASK);
> >> +}
> >> +
> >>
> >> +static int vmx_interrupt_allowed(struct kvm_vcpu *vcpu)
> >> +{
> >> +    vmx_update_window_states(vcpu);
> >> +    return vcpu->arch.interrupt_window_open;
> >> +}
> >> +
> >>  static void kvm_do_inject_irq(struct kvm_vcpu *vcpu)
> >>  }
> >> +
> >> +int kvm_arch_interrupt_allowed(struct kvm_vcpu *vcpu)
> >> +{
> >> +    return kvm_x86_ops->interrupt_allowed(vcpu);
> >> +}
> >
> > If the guest enables interrupts but sets tpr/cr8 to block interrupts,
> > we'll spin (like we do now).
> >
> > So I think this should be called kvm_arch_can_accept_interrupt() and
> > take tpr into account.
>
> kvm_cpu_has_interrupt() takes the tpr into account, so we're okay here.
>
> Marcelo, Sheng?

Yes, looks good to me.

-- 
regards
Yang, Sheng