On Mon, Aug 24, 2009 at 03:06:23PM +0300, Gleb Natapov wrote: > Use return value from kvm_set_irq() to track coalesced PIT interrupts > instead of ack/mask notifiers.
Gleb, What is the advantage of doing so? Ack notifiers are asynchronous notifications. Using the return value from kvm_set_irq implies that timer emulation is based on a "tick generating device" on the host side. What I mean is that the ack notifications are useful, since they are asynchronous. Supposing your goal is to get rid of ack notifiers, due to their burden in irqchip code? > Signed-off-by: Gleb Natapov <[email protected]> > diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c > index b857ca3..0b63991 100644 > --- a/arch/x86/kvm/i8254.c > +++ b/arch/x86/kvm/i8254.c > @@ -231,20 +231,7 @@ int pit_has_pending_timer(struct kvm_vcpu *vcpu) > { > struct kvm_pit *pit = vcpu->kvm->arch.vpit; > > - if (pit && kvm_vcpu_is_bsp(vcpu) && pit->pit_state.irq_ack) > - return atomic_read(&pit->pit_state.pit_timer.pending); > - return 0; > -} > - > -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); > - spin_lock(&ps->inject_lock); > - if (atomic_dec_return(&ps->pit_timer.pending) < 0) > - atomic_inc(&ps->pit_timer.pending); > - ps->irq_ack = 1; > - spin_unlock(&ps->inject_lock); > + return atomic_read(&pit->pit_state.pit_timer.pending); > } > > void __kvm_migrate_pit_timer(struct kvm_vcpu *vcpu) > @@ -297,7 +284,6 @@ static void create_pit_timer(struct kvm_kpit_state *ps, > u32 val, int is_period) > pt->vcpu = pt->kvm->bsp_vcpu; > > atomic_set(&pt->pending, 0); > - ps->irq_ack = 1; > > hrtimer_start(&pt->timer, ktime_add_ns(ktime_get(), interval), > HRTIMER_MODE_ABS); > @@ -577,17 +563,6 @@ void kvm_pit_reset(struct kvm_pit *pit) > mutex_unlock(&pit->pit_state.lock); > > atomic_set(&pit->pit_state.pit_timer.pending, 0); > - pit->pit_state.irq_ack = 1; > -} > - > -static void pit_mask_notifer(struct kvm_irq_mask_notifier *kimn, bool mask) > -{ > - struct kvm_pit *pit = container_of(kimn, struct kvm_pit, mask_notifier); > - > - if (!mask) { > - atomic_set(&pit->pit_state.pit_timer.pending, 0); > - pit->pit_state.irq_ack = 1; > - } > } > > static const struct kvm_io_device_ops pit_dev_ops = { > @@ -619,7 +594,6 @@ struct kvm_pit *kvm_create_pit(struct kvm *kvm, u32 flags) > > mutex_init(&pit->pit_state.lock); > mutex_lock(&pit->pit_state.lock); > - spin_lock_init(&pit->pit_state.inject_lock); > > kvm->arch.vpit = pit; > pit->kvm = kvm; > @@ -628,17 +602,11 @@ struct kvm_pit *kvm_create_pit(struct kvm *kvm, u32 > flags) > pit_state->pit = pit; > hrtimer_init(&pit_state->pit_timer.timer, > CLOCK_MONOTONIC, HRTIMER_MODE_ABS); > - pit_state->irq_ack_notifier.gsi = 0; > - pit_state->irq_ack_notifier.irq_acked = kvm_pit_ack_irq; > - kvm_register_irq_ack_notifier(kvm, &pit_state->irq_ack_notifier); > pit_state->pit_timer.reinject = true; > mutex_unlock(&pit->pit_state.lock); > > kvm_pit_reset(pit); > > - pit->mask_notifier.func = pit_mask_notifer; > - kvm_register_irq_mask_notifier(kvm, 0, &pit->mask_notifier); > - > kvm_iodevice_init(&pit->dev, &pit_dev_ops); > ret = __kvm_io_bus_register_dev(&kvm->pio_bus, &pit->dev); > if (ret < 0) > @@ -670,10 +638,6 @@ void kvm_free_pit(struct kvm *kvm) > struct hrtimer *timer; > > if (kvm->arch.vpit) { > - kvm_unregister_irq_mask_notifier(kvm, 0, > - &kvm->arch.vpit->mask_notifier); > - kvm_unregister_irq_ack_notifier(kvm, > - &kvm->arch.vpit->pit_state.irq_ack_notifier); > mutex_lock(&kvm->arch.vpit->pit_state.lock); > timer = &kvm->arch.vpit->pit_state.pit_timer.timer; > hrtimer_cancel(timer); > @@ -683,12 +647,12 @@ void kvm_free_pit(struct kvm *kvm) > } > } > > -static void __inject_pit_timer_intr(struct kvm *kvm) > +static int __inject_pit_timer_intr(struct kvm *kvm) > { > struct kvm_vcpu *vcpu; > - int i; > + int i, r; > > - kvm_set_irq(kvm, kvm->arch.vpit->irq_source_id, 0, 1); > + r = kvm_set_irq(kvm, kvm->arch.vpit->irq_source_id, 0, 1); > kvm_set_irq(kvm, kvm->arch.vpit->irq_source_id, 0, 0); > > /* > @@ -703,6 +667,8 @@ static void __inject_pit_timer_intr(struct kvm *kvm) > if (kvm->arch.vapics_in_nmi_mode > 0) > kvm_for_each_vcpu(i, vcpu, kvm) > kvm_apic_nmi_wd_deliver(vcpu); > + > + return r; > } > > void kvm_inject_pit_timer_irqs(struct kvm_vcpu *vcpu) > @@ -711,20 +677,14 @@ void kvm_inject_pit_timer_irqs(struct kvm_vcpu *vcpu) > struct kvm *kvm = vcpu->kvm; > struct kvm_kpit_state *ps; > > - if (pit) { > - int inject = 0; > - ps = &pit->pit_state; > - > - /* Try to inject pending interrupts when > - * last one has been acked. > - */ > - spin_lock(&ps->inject_lock); > - if (atomic_read(&ps->pit_timer.pending) && ps->irq_ack) { > - ps->irq_ack = 0; > - inject = 1; > - } > - spin_unlock(&ps->inject_lock); > - if (inject) > - __inject_pit_timer_intr(kvm); > - } > + if (!pit) > + return; > + > + ps = &pit->pit_state; > + > + if (!atomic_read(&ps->pit_timer.pending)) > + return; > + > + if (__inject_pit_timer_intr(kvm)) > + atomic_dec(&ps->pit_timer.pending); > } > diff --git a/arch/x86/kvm/i8254.h b/arch/x86/kvm/i8254.h > index d4c1c7f..19937da 100644 > --- a/arch/x86/kvm/i8254.h > +++ b/arch/x86/kvm/i8254.h > @@ -28,8 +28,6 @@ struct kvm_kpit_state { > struct mutex lock; > struct kvm_pit *pit; > spinlock_t inject_lock; > - unsigned long irq_ack; > - struct kvm_irq_ack_notifier irq_ack_notifier; > }; > > struct kvm_pit { > @@ -39,7 +37,6 @@ struct kvm_pit { > struct kvm *kvm; > struct kvm_kpit_state pit_state; > int irq_source_id; > - struct kvm_irq_mask_notifier mask_notifier; > }; > > #define KVM_PIT_BASE_ADDRESS 0x40 > -- > Gleb. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
