From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marcelo Tosatti Subject: Re: [patch 4/8] KVM: x86: replace hrtimer based timer emulation Date: Wed, 8 Jul 2009 10:17:21 -0300 Message-ID: <20090708131721.GA3382@amt.cnet> References: <20090706015511.923596553@localhost.localdomain> <20090706015812.786509491@localhost.localdomain> <20090708125819.GM28046@redhat.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="nFreZHaLTZJo0R7j" Cc: kvm@vger.kernel.org To: Gleb Natapov Return-path: Received: from mx2.redhat.com ([66.187.237.31]:43185 "EHLO mx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750758AbZGHNRl (ORCPT ); Wed, 8 Jul 2009 09:17:41 -0400 Received: from int-mx2.corp.redhat.com (int-mx2.corp.redhat.com [172.16.27.26]) by mx2.redhat.com (8.13.8/8.13.8) with ESMTP id n68DHfqT029759 for ; Wed, 8 Jul 2009 09:17:41 -0400 Content-Disposition: inline In-Reply-To: <20090708125819.GM28046@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: --nFreZHaLTZJo0R7j Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Wed, Jul 08, 2009 at 03:58:19PM +0300, Gleb Natapov wrote: > Excellent patch series. > > On Sun, Jul 05, 2009 at 10:55:15PM -0300, Marcelo Tosatti wrote: > > int kvm_cpu_has_pending_timer(struct kvm_vcpu *vcpu) > > { > > - int ret; > > + ktime_t now, expires; > > > > - ret = pit_has_pending_timer(vcpu); > > - ret |= apic_has_pending_timer(vcpu); > > + expires = kvm_vcpu_next_timer_event(vcpu); > > + now = ktime_get(); > > + if (expires.tv64 <= now.tv64) { > > + if (kvm_arch_interrupt_allowed(vcpu)) > > + set_bit(KVM_REQ_UNHALT, &vcpu->requests); > You shouldn't unhalt vcpu here. Not every timer event will generate > interrupt (vector can be masked in pic/ioapic) Yeah. Note however that kvm_vcpu_next_timer_event only returns the expiration time for events that have been acked (for timers that have had events injected, but not acked it returns KTIME_MAX). So, the code above will set one spurious unhalt if: - inject timer irq - guest acks irq - guest mask irq - unhalt (once) I had a "kvm_timer_mask" callback before (along with the attached patch), but decided to keep it simpler using the ack trick above. I suppose one spurious unhalt is harmless, or is it a correctness issue? > or timer event can generate NMI instead of interrupt. In the NMI case it should not unhalt the processor? (but yes, bypassing the irq injection system its not a very beatiful shortcut, but its done in other places too eg i8254.c NMI injection via all cpus LINT0). > Leaving this code out probably means > that you can't remove kvm_inject_pending_timer_irqs() call from > __vcpu_run(). > > > + return 1; > > + } > > > > - return ret; > > + return 0; > > } > > EXPORT_SYMBOL(kvm_cpu_has_pending_timer); > > > > -- > Gleb. --nFreZHaLTZJo0R7j Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename=kvm-int-mask-helpers Index: kvm/arch/x86/kvm/i8259.c =================================================================== --- kvm.orig/arch/x86/kvm/i8259.c +++ kvm/arch/x86/kvm/i8259.c @@ -122,6 +122,18 @@ static inline int get_priority(struct kv return priority; } +int pic_int_is_masked(struct kvm *kvm, int irq) +{ + u8 imr; + struct kvm_pic *s = pic_irqchip(kvm); + + pic_lock(s); + imr = s->pics[SELECT_PIC(irq)].imr; + pic_unlock(s); + + return (imr & (1 << irq)); +} + /* * return the pic wanted interrupt. return -1 if none */ Index: kvm/arch/x86/kvm/irq.c =================================================================== --- kvm.orig/arch/x86/kvm/irq.c +++ kvm/arch/x86/kvm/irq.c @@ -42,6 +42,29 @@ int kvm_cpu_has_pending_timer(struct kvm EXPORT_SYMBOL(kvm_cpu_has_pending_timer); /* + * The PIT interrupt can be masked on different levels: PIC/IOAPIC/LAPIC. + * Hide the maze from the PIT mask notifier, all it cares about is whether + * the interrupt can reach the processor. + */ +void kvm_pit_mask_notifier(struct kvm_irq_mask_notifier *kimn, bool mask) +{ + bool masked = 0; + struct kvm *kvm; + struct kvm_pit *pit = container_of(kimn, struct kvm_pit, mask_notifier); + + kvm = pit->kvm; + + /* LAPIC hardware disabled or INTIN0 programmed as ExtINT */ + if (kvm_apic_accept_pic_intr(kvm->vcpus[0])) { + masked = pic_int_is_masked(kvm, 0); + /* IOAPIC -> LAPIC */ + } else + masked = ioapic_int_is_masked(kvm, 0); + + kvm_pit_internal_mask_notifier(kvm, mask); +} + +/* * check if there is pending interrupt without * intack. */ Index: kvm/virt/kvm/ioapic.c =================================================================== --- kvm.orig/virt/kvm/ioapic.c +++ kvm/virt/kvm/ioapic.c @@ -101,6 +101,16 @@ static int ioapic_service(struct kvm_ioa return injected; } +int ioapic_int_is_masked(struct kvm *kvm, int pin) +{ + struct kvm_ioapic *ioapic = kvm->arch.vioapic; + + if (!ioapic) + return 1; + + return ioapic->redirtbl[pin].fields.mask; +} + static void ioapic_write_indirect(struct kvm_ioapic *ioapic, u32 val) { unsigned index; Index: kvm/arch/x86/kvm/i8254.c =================================================================== --- kvm.orig/arch/x86/kvm/i8254.c +++ kvm/arch/x86/kvm/i8254.c @@ -558,9 +558,9 @@ void kvm_pit_reset(struct kvm_pit *pit) pit->pit_state.irq_ack = 1; } -static void pit_mask_notifer(struct kvm_irq_mask_notifier *kimn, bool mask) +void kvm_pit_internal_mask_notifier(struct kvm *kvm, bool mask) { - struct kvm_pit *pit = container_of(kimn, struct kvm_pit, mask_notifier); + struct kvm_pit *pit = kvm->arch.vpit; if (!mask) { atomic_set(&pit->pit_state.pit_timer.pending, 0); @@ -614,8 +614,8 @@ struct kvm_pit *kvm_create_pit(struct kv kvm_pit_reset(pit); - pit->mask_notifier.func = pit_mask_notifer; - kvm_register_irq_mask_notifier(kvm, 0, &pit->mask_notifier); + pit->mask_notifier.func = kvm_pit_mask_notifier; + kvm_register_irq_mask_notifier(kvm, 0, &kvm->arch.vpit->mask_notifier); kvm_iodevice_init(&pit->dev, &pit_dev_ops); kvm_io_bus_register_dev(&kvm->pio_bus, &pit->dev); Index: kvm/arch/x86/kvm/irq.h =================================================================== --- kvm.orig/arch/x86/kvm/irq.h +++ kvm/arch/x86/kvm/irq.h @@ -98,7 +98,13 @@ void __kvm_migrate_apic_timer(struct kvm void __kvm_migrate_pit_timer(struct kvm_vcpu *vcpu); void __kvm_migrate_timers(struct kvm_vcpu *vcpu); +void kvm_pit_internal_mask_notifier(struct kvm *kvm, bool mask); +void kvm_pit_mask_notifier(struct kvm_irq_mask_notifier *kimn, bool mask); + int pit_has_pending_timer(struct kvm_vcpu *vcpu); int apic_has_pending_timer(struct kvm_vcpu *vcpu); +int pic_int_is_masked(struct kvm *kvm, int irq); +int ioapic_int_is_masked(struct kvm *kvm, int irq); + #endif --nFreZHaLTZJo0R7j--