Re: [PATCH v6 6/6] KVM: Use eoi to track RTC interrupt delivery status

[Gleb Natapov <gleb@redhat.com>]

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.

--
			Gleb.