Gleb Natapov wrote on 2013-03-22:
> 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@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.
>>
> Anyone can use RTC is Linux guest. Don't know about others.
I see.
Since pass IRQ_STATUS to ioapic need to change many functions, how about add a variable in rtc_status:
struct rtc_status {
bool IRQ_STATUS
};
And set it in kvm_vm_ioctl():
case KVM_IRQ_LINE_STATUS:
if(irq == RTC_GSI && ioapic)
ioapic->rtc_status.IRQ_STATUS = true;
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
