Re: [PATCH 05/15] Coalesce userspace/kernel irqchip interrupt injection logic.

From: Gleb Natapov <gleb@redhat.com>
To: Jan Kiszka <jan.kiszka@web.de>
Cc: avi@redhat.com, kvm@vger.kernel.org, joerg.roedel@amd.com,
	sheng@linux.intel.com, Dmitry Baryshkov <dbaryshkov@gmail.com>
Subject: Re: [PATCH 05/15] Coalesce userspace/kernel irqchip interrupt injection logic.
Date: Sat, 18 Apr 2009 19:20:40 +0300	[thread overview]
Message-ID: <20090418162040.GH27675@redhat.com> (raw)
In-Reply-To: <49E997C6.4090503@web.de>

On Sat, Apr 18, 2009 at 11:05:10AM +0200, Jan Kiszka wrote:
> Gleb Natapov wrote:
> > Start to use interrupt/exception queues like VMX does.
> > This also fix the bug that if exit was caused by a guest
> > internal exception access to IDT the exception was not
> > reinjected.
> > 
> > Signed-off-by: Gleb Natapov <gleb@redhat.com>
> > ---
> >  arch/x86/kvm/svm.c |  176 ++++++++++++++++++++++------------------------------
> >  1 files changed, 75 insertions(+), 101 deletions(-)
> > 
> > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> > index 52c41aa..053370d 100644
> > --- a/arch/x86/kvm/svm.c
> > +++ b/arch/x86/kvm/svm.c
> > @@ -70,7 +70,6 @@ module_param(npt, int, S_IRUGO);
> >  static int nested = 0;
> >  module_param(nested, int, S_IRUGO);
> >  
> > -static void kvm_reput_irq(struct vcpu_svm *svm);
> >  static void svm_flush_tlb(struct kvm_vcpu *vcpu);
> >  
> >  static int nested_svm_exit_handled(struct vcpu_svm *svm, bool kvm_override);
> > @@ -199,9 +198,7 @@ static void svm_queue_exception(struct kvm_vcpu *vcpu, unsigned nr,
> >  
> >  static bool svm_exception_injected(struct kvm_vcpu *vcpu)
> >  {
> > -	struct vcpu_svm *svm = to_svm(vcpu);
> > -
> > -	return !(svm->vmcb->control.exit_int_info & SVM_EXITINTINFO_VALID);
> > +	return false;
> >  }
> >  
> >  static int is_external_interrupt(u32 info)
> > @@ -976,12 +973,9 @@ static int svm_guest_debug(struct kvm_vcpu *vcpu, struct kvm_guest_debug *dbg)
> >  
> >  static int svm_get_irq(struct kvm_vcpu *vcpu)
> >  {
> > -	struct vcpu_svm *svm = to_svm(vcpu);
> > -	u32 exit_int_info = svm->vmcb->control.exit_int_info;
> > -
> > -	if (is_external_interrupt(exit_int_info))
> > -		return exit_int_info & SVM_EVTINJ_VEC_MASK;
> > -	return -1;
> > +	if (!vcpu->arch.interrupt.pending)
> > +		return -1;
> > +	return vcpu->arch.interrupt.nr;
> >  }
> >  
> >  static void load_host_msrs(struct kvm_vcpu *vcpu)
> > @@ -1088,17 +1082,8 @@ static void svm_set_dr(struct kvm_vcpu *vcpu, int dr, unsigned long value,
> >  
> >  static int pf_interception(struct vcpu_svm *svm, struct kvm_run *kvm_run)
> >  {
> > -	u32 exit_int_info = svm->vmcb->control.exit_int_info;
> > -	struct kvm *kvm = svm->vcpu.kvm;
> >  	u64 fault_address;
> >  	u32 error_code;
> > -	bool event_injection = false;
> > -
> > -	if (!irqchip_in_kernel(kvm) &&
> > -	    is_external_interrupt(exit_int_info)) {
> > -		event_injection = true;
> > -		kvm_push_irq(&svm->vcpu, exit_int_info & SVM_EVTINJ_VEC_MASK);
> > -	}
> >  
> >  	fault_address  = svm->vmcb->control.exit_info_2;
> >  	error_code = svm->vmcb->control.exit_info_1;
> > @@ -1118,9 +1103,11 @@ static int pf_interception(struct vcpu_svm *svm, struct kvm_run *kvm_run)
> >  	 */
> >  	if (npt_enabled)
> >  		svm_flush_tlb(&svm->vcpu);
> > -
> > -	if (!npt_enabled && event_injection)
> > -		kvm_mmu_unprotect_page_virt(&svm->vcpu, fault_address);
> > +	else {
> > +		if (svm->vcpu.arch.interrupt.pending ||
> > +				svm->vcpu.arch.exception.pending)
> > +			kvm_mmu_unprotect_page_virt(&svm->vcpu, fault_address);
> > +	}
> 
> Without understanding yet why kvm_mmu_unprotect_page_virt is required
> here, this looks like it is lacking '|| svm->vcpu.arch.nmi_injected'.
> Interrupts and exceptions are re-queued on fault-during-injection,
> therefore they are now pending again, right?
> 
Yes right, and we discussed this with Avi already. I'll send another
patch after this series will be applied (exactly the same issue exists
for VMX btw).

--
			Gleb.