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 <gleb@xxxxxxxxxx>
> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
