From mboxrd@z Thu Jan  1 00:00:00 1970
From: Gleb Natapov <gleb@redhat.com>
Subject: Re: [PATCH v6 6/6] KVM: Use eoi to track RTC interrupt delivery
 status
Date: Fri, 22 Mar 2013 10:30:45 +0200
Message-ID: <20130322083045.GQ9382@redhat.com>
References: <1363929845-29005-1-git-send-email-yang.z.zhang@intel.com>
 <1363929845-29005-7-git-send-email-yang.z.zhang@intel.com>
 <20130322075047.GN9382@redhat.com>
 <A9667DDFB95DB7438FA9D7D576C3D87E099EB9F8@SHSMSX101.ccr.corp.intel.com>
 <20130322081441.GP9382@redhat.com>
 <A9667DDFB95DB7438FA9D7D576C3D87E099EBABB@SHSMSX101.ccr.corp.intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"mtosatti@redhat.com" <mtosatti@redhat.com>,
	"Zhang, Xiantao" <xiantao.zhang@intel.com>
To: "Zhang, Yang Z" <yang.z.zhang@intel.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:23616 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1754091Ab3CVIas (ORCPT <rfc822;kvm@vger.kernel.org>);
	Fri, 22 Mar 2013 04:30:48 -0400
Content-Disposition: inline
In-Reply-To: <A9667DDFB95DB7438FA9D7D576C3D87E099EBABB@SHSMSX101.ccr.corp.intel.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Fri, Mar 22, 2013 at 08:25:21AM +0000, Zhang, Yang Z wrote:
> Gleb Natapov wrote on 2013-03-22:
> > On Fri, Mar 22, 2013 at 08:05:27AM +0000, Zhang, Yang Z wrote:
> >> Gleb Natapov wrote on 2013-03-22:
> >>> On Fri, Mar 22, 2013 at 01:24:05PM +0800, Yang Zhang wrote:
> >>>> From: Yang Zhang <yang.z.zhang@Intel.com>
> >>>> 
> >>>> Current interrupt coalescing logci which only used by RTC has conflict
> >>>> with Posted Interrupt.
> >>>> This patch introduces a new mechinism to use eoi to track interrupt:
> >>>> When delivering an interrupt to vcpu, the pending_eoi set to number of
> >>>> vcpu that received the interrupt. And decrease it when each vcpu writing
> >>>> eoi. No subsequent RTC interrupt can deliver to vcpu until all vcpus
> >>>> write eoi.
> >>>> 
> >>>> Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
> >>>> ---
> >>>>  virt/kvm/ioapic.c |   40 +++++++++++++++++++++++++++++++++++++++- 1
> >>>>  files changed, 39 insertions(+), 1 deletions(-)
> >>>> diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
> >>>> index c991e58..df16daf 100644
> >>>> --- a/virt/kvm/ioapic.c
> >>>> +++ b/virt/kvm/ioapic.c
> >>>> @@ -114,6 +114,29 @@ static void rtc_irq_restore(struct kvm_ioapic
> > *ioapic)
> >>>>  	ioapic->rtc_status.pending_eoi = pending_eoi;
> >>>>  }
> >>>> +static void rtc_irq_ack_eoi(struct kvm_vcpu *vcpu,
> >>>> +			struct rtc_status *rtc_status, int irq)
> >>>> +{
> >>>> +	if (irq != RTC_GSI)
> >>>> +		return;
> >>>> +
> >>>> +	if (test_and_clear_bit(vcpu->vcpu_id, rtc_status->dest_map))
> >>>> +		--rtc_status->pending_eoi;
> >>>> +
> >>>> +	WARN_ON(rtc_status->pending_eoi < 0);
> >>>> +}
> >>>> +
> >>>> +static bool rtc_irq_check(struct kvm_ioapic *ioapic, int irq)
> >>>> +{
> >>>> +	if (irq != RTC_GSI)
> >>>> +		return false;
> >>>> +
> >>>> +	if (ioapic->rtc_status.pending_eoi > 0)
> >>>> +		return true; /* coalesced */
> >>>> +
> >>>> +	return false;
> >>>> +}
> >>>> +
> >>>>  static int ioapic_service(struct kvm_ioapic *ioapic, unsigned int idx)
> >>>>  {
> >>>>  	union kvm_ioapic_redirect_entry *pent;
> >>>> @@ -229,6 +252,7 @@ static int ioapic_deliver(struct kvm_ioapic *ioapic, int
> >>> irq)
> >>>>  {
> >>>>  	union kvm_ioapic_redirect_entry *entry = &ioapic->redirtbl[irq];
> >>>>  	struct kvm_lapic_irq irqe;
> >>>> +	int ret;
> >>>> 
> >>>>  	ioapic_debug("dest=%x dest_mode=%x delivery_mode=%x "
> >>>>  		     "vector=%x trig_mode=%x\n",
> >>>> @@ -244,7 +268,14 @@ static int ioapic_deliver(struct kvm_ioapic *ioapic,
> > int
> >>> irq)
> >>>>  	irqe.level = 1;
> >>>>  	irqe.shorthand = 0;
> >>>> -	return kvm_irq_delivery_to_apic(ioapic->kvm, NULL, &irqe, NULL);
> >>>> +	if (irq == RTC_GSI) {
> >>>> +		ret = kvm_irq_delivery_to_apic(ioapic->kvm, NULL, &irqe,
> >>>> +				ioapic->rtc_status.dest_map);
> >>>> +		ioapic->rtc_status.pending_eoi = ret;
> >>> We should track status only if IRQ_STATUS ioctl was used to inject an
> >>> interrupt.
> >> We already know RTC will use IRQ_STATUS ioctl. Why check it again?
> >> 
> > QEMU does. QEMU is not the only userspace.
> And this will break other userspace.
> 
How?

--
			Gleb.