From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marcelo Tosatti <mtosatti@redhat.com>
Subject: Re: [PATCH v7 2/3] x86, apicv: add virtual interrupt delivery support
Date: Wed, 19 Dec 2012 23:01:23 -0200
Message-ID: <20121220010123.GB21033@amt.cnet>
References: <1355722250-7122-1-git-send-email-yang.z.zhang@intel.com>
 <1355722250-7122-3-git-send-email-yang.z.zhang@intel.com>
 <20121220005936.GA21033@amt.cnet>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: kvm@vger.kernel.org, gleb@redhat.com, haitao.shan@intel.com,
	Kevin Tian <kevin.tian@intel.com>
To: Yang Zhang <yang.z.zhang@intel.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:2281 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1752217Ab2LTB2p (ORCPT <rfc822;kvm@vger.kernel.org>);
	Wed, 19 Dec 2012 20:28:45 -0500
Content-Disposition: inline
In-Reply-To: <20121220005936.GA21033@amt.cnet>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Wed, Dec 19, 2012 at 10:59:36PM -0200, Marcelo Tosatti wrote:
> On Mon, Dec 17, 2012 at 01:30:49PM +0800, Yang Zhang wrote:
> > From: Yang Zhang <yang.z.zhang@Intel.com>
> > 
> > Virtual interrupt delivery avoids KVM to inject vAPIC interrupts
> > manually, which is fully taken care of by the hardware. This needs
> > some special awareness into existing interrupr injection path:
> > 
> > - for pending interrupt, instead of direct injection, we may need
> >   update architecture specific indicators before resuming to guest.
> > 
> > - A pending interrupt, which is masked by ISR, should be also
> >   considered in above update action, since hardware will decide
> >   when to inject it at right time. Current has_interrupt and
> >   get_interrupt only returns a valid vector from injection p.o.v.
> > 
> > Signed-off-by: Kevin Tian <kevin.tian@intel.com>
> > Signed-off-by: Yang Zhang <yang.z.zhang@intel.com>
> 
> 
> Resuming previous discussion:
> 
> > > How about to recaculate irr_pending according the VIRR on each
> > > vmexit?
> > > 
> > No need really. Since HW can only clear VIRR the only situation that
> > may
> > happen is that irr_pending will be true but VIRR is empty and
> > apic_find_highest_irr() will return correct result in this case.
> 
> Self-IPI does cause VIRR to be set, see "29.1.5 Self-IPI 
> Virtualization".
> 
> Also, an example of problem with ISR caching optimization (which was written
> with ISR/IRR management entirely in software):
> 
> isr_count variable is never incremented (because HW sets ISR):
> kvm_cpu_get_interrupt does not call into kvm_get_apic_interrupt 
> if virtual interrupt delivery enabled.
> 
> Therefore apic_find_highest_isr can return -1 even though VISR
> is not zero.
> 
> In that case (VISR non-zero but apic_find_highest_isr() == -1), 
> apic_update_ppr does not correctly set
> 
> PPR = max(ISRV, TPR)
> 
> Which can result in kvm_arch_vcpu_runnable returning 1 when it should not.
> 
> Please disable usage of isr_count if virtual interrupt delivery enabled.
> Given that self-IPI writes to VIRR, also please disable usage of
> highest_isr_cache and irr_pending.

BTW, no need to update irr_pending or the other caching variables on
VM-exit. Just skip them if virtual interrupt delivery is enabled.