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:51:47AM +0000, Zhang, Yang Z wrote:
> 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@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?
> >> 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.
> 
Anyone can use RTC is Linux guest. Don't know about others.

--
			Gleb.