Gleb Natapov wrote on 2013-03-22: > On Fri, Mar 22, 2013 at 08:37:22AM +0000, Zhang, Yang Z wrote: >> Gleb Natapov wrote on 2013-03-22: >>> 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@xxxxxxxxx> >>>>>>>> >>>>>>>> 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@xxxxxxxxx> >>>>>>>> --- >>>>>>>> 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? >> If other userspace has the reinjection logic for RTC, but it not uses IRQ_STATUS, > then it cannot get the right coalescing info. If it also use IRQ_STATUS to get > coalescing info, then we don't need the IRQ_STATUS check. >> > If userspace does not care about irq status it does not use IRQ_STATUS > ioctl and we should not go extra mile to provide one. Not everyone cares > about running Windows as a guest. I see your point. But if no windows guest running, RTC is hardly used by other guests and the overheard can be ignore. Best regards, Yang -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html