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.
--
Gleb.
--
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
