From mboxrd@z Thu Jan 1 00:00:00 1970 From: Zachary Amsden Subject: Re: [PATCH 1/3] Introduce a workqueue to deliver PIT timer interrupts. Date: Fri, 11 Jun 2010 19:18:56 -1000 Message-ID: <4C1318C0.6010502@redhat.com> References: <1276202647-1598-1-git-send-email-clalance@redhat.com> <1276202647-1598-2-git-send-email-clalance@redhat.com> <20100611173523.GD6913@amt.cnet> <201006121215.41299.sheng.yang@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Marcelo Tosatti , Chris Lalancette , "kvm@vger.kernel.org" , Gleb Natapov To: "Yang, Sheng" Return-path: Received: from mx1.redhat.com ([209.132.183.28]:49421 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750893Ab0FLFTI (ORCPT ); Sat, 12 Jun 2010 01:19:08 -0400 In-Reply-To: <201006121215.41299.sheng.yang@intel.com> Sender: kvm-owner@vger.kernel.org List-ID: On 06/11/2010 06:15 PM, Yang, Sheng wrote: > On Saturday 12 June 2010 01:35:23 Marcelo Tosatti wrote: > >> On Thu, Jun 10, 2010 at 04:44:05PM -0400, Chris Lalancette wrote: >> >>> We really want to "kvm_set_irq" during the hrtimer callback, >>> but that is risky because that is during interrupt context. >>> Instead, offload the work to a workqueue, which is a bit safer >>> and should provide most of the same functionality. >>> >>> Signed-off-by: Chris Lalancette >>> --- >>> >>> arch/x86/kvm/i8254.c | 117 >>> ++++++++++++++++++++++++++++---------------------- arch/x86/kvm/i8254.h >>> | 4 +- >>> arch/x86/kvm/irq.c | 1 - >>> 3 files changed, 69 insertions(+), 53 deletions(-) >>> >>> diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c >>> index 188d827..99c7472 100644 >>> --- a/arch/x86/kvm/i8254.c >>> +++ b/arch/x86/kvm/i8254.c >>> @@ -34,6 +34,7 @@ >>> >>> #include >>> #include >>> >>> +#include >>> >>> #include "irq.h" >>> #include "i8254.h" >>> >>> @@ -244,11 +245,11 @@ static void kvm_pit_ack_irq(struct >>> kvm_irq_ack_notifier *kian) >>> >>> { >>> >>> struct kvm_kpit_state *ps = container_of(kian, struct kvm_kpit_state, >>> >>> irq_ack_notifier); >>> >>> - raw_spin_lock(&ps->inject_lock); >>> + spin_lock(&ps->inject_lock); >>> >>> if (atomic_dec_return(&ps->pit_timer.pending)< 0) >>> >>> atomic_inc(&ps->pit_timer.pending); >>> >>> ps->irq_ack = 1; >>> >>> - raw_spin_unlock(&ps->inject_lock); >>> + spin_unlock(&ps->inject_lock); >>> >>> } >>> >>> void __kvm_migrate_pit_timer(struct kvm_vcpu *vcpu) >>> >>> @@ -281,6 +282,58 @@ static struct kvm_timer_ops kpit_ops = { >>> >>> .is_periodic = kpit_is_periodic, >>> >>> }; >>> >>> +static void pit_do_work(struct work_struct *work) >>> +{ >>> + struct kvm_pit *pit = container_of(work, struct kvm_pit, >>> expired); + struct kvm *kvm = pit->kvm; >>> + struct kvm_vcpu *vcpu; >>> + int i; >>> + struct kvm_kpit_state *ps =&pit->pit_state; >>> + int inject = 0; >>> + >>> + /* Try to inject pending interrupts when >>> + * last one has been acked. >>> + */ >>> + spin_lock(&ps->inject_lock); >>> + if (ps->irq_ack) { >>> + ps->irq_ack = 0; >>> + inject = 1; >>> + } >>> + spin_unlock(&ps->inject_lock); >>> + if (inject) { >>> + kvm_set_irq(kvm, kvm->arch.vpit->irq_source_id, 0, 1); >>> + kvm_set_irq(kvm, kvm->arch.vpit->irq_source_id, 0, 0); >>> + >>> + /* >>> + * Provides NMI watchdog support via Virtual Wire mode. >>> + * The route is: PIT -> PIC -> LVT0 in NMI mode. >>> + * >>> + * Note: Our Virtual Wire implementation is simplified, >>> only + * propagating PIT interrupts to all VCPUs when >>> they have set + * LVT0 to NMI delivery. Other PIC >>> interrupts are just sent to + * VCPU0, and only if its >>> LVT0 is in EXTINT mode. + */ >>> + if (kvm->arch.vapics_in_nmi_mode> 0) >>> + kvm_for_each_vcpu(i, vcpu, kvm) >>> + kvm_apic_nmi_wd_deliver(vcpu); >>> + } >>> +} >>> + >>> +static enum hrtimer_restart pit_timer_fn(struct hrtimer *data) >>> +{ >>> + struct kvm_timer *ktimer = container_of(data, struct kvm_timer, timer); >>> + struct kvm_pit *pt = ktimer->kvm->arch.vpit; >>> + >>> + queue_work(pt->wq,&pt->expired); >>> >> So this disables interrupt reinjection. Older RHEL3 guests do not >> compensate for lost ticks, and as such are likely to drift without >> it (but RHEL3 is EOL, should one care?). >> >> Are there other guests which rely on PIT reinjection, or is it OK >> to remove it completly? >> > IIRC, the old kernel *does* compensate ticks, so we need disable reinjection. And > the latest kernel doesn't do this, so we have to do reinjection. > It looks like 2.4.21 has lost tick compensation for x86_64, but not for i386, so the reinjection is still needed.