On Wednesday 21 January 2009 12:23:30 Marcelo Tosatti wrote:
> On Wed, Jan 21, 2009 at 10:40:49AM +0800, Sheng Yang wrote:
> > Mind continue to change...
> >
> > A: time_fire
> > B: intr_post
> > C: read TMCCT
> >
> > I think the sequence can be AABC or AACB(I just consider AABC last
> > night...). We have determined to return A->C as TMCCT, and the interval
> > between different A is period which can be ignored. So how about simply
> > return
> > hrtimer_expires_remaining() for all tmcct reading including pending ones?
> >
> > (And sorry for I still can't get why you return injection delay for
> > TMCCT...)
>
> You're right. How about this.
>
>
> diff --git a/arch/x86/kvm/irq.c b/arch/x86/kvm/irq.c
> index c019b8e..cf17ed5 100644
> --- a/arch/x86/kvm/irq.c
> +++ b/arch/x86/kvm/irq.c
> @@ -87,13 +87,6 @@ void kvm_inject_pending_timer_irqs(struct kvm_vcpu
> *vcpu) }
>  EXPORT_SYMBOL_GPL(kvm_inject_pending_timer_irqs);
>
> -void kvm_timer_intr_post(struct kvm_vcpu *vcpu, int vec)
> -{
> -     kvm_apic_timer_intr_post(vcpu, vec);
> -     /* TODO: PIT, RTC etc. */
> -}
> -EXPORT_SYMBOL_GPL(kvm_timer_intr_post);
> -
>  void __kvm_migrate_timers(struct kvm_vcpu *vcpu)
>  {
>       __kvm_migrate_apic_timer(vcpu);
> diff --git a/arch/x86/kvm/irq.h b/arch/x86/kvm/irq.h
> index 2bf32a0..82579ee 100644
> --- a/arch/x86/kvm/irq.h
> +++ b/arch/x86/kvm/irq.h
> @@ -89,7 +89,6 @@ static inline int irqchip_in_kernel(struct kvm *kvm)
>
>  void kvm_pic_reset(struct kvm_kpic_state *s);
>
> -void kvm_timer_intr_post(struct kvm_vcpu *vcpu, int vec);
>  void kvm_inject_pending_timer_irqs(struct kvm_vcpu *vcpu);
>  void kvm_inject_apic_timer_irqs(struct kvm_vcpu *vcpu);
>  void kvm_apic_nmi_wd_deliver(struct kvm_vcpu *vcpu);
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index afac68c..b66f71e 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -511,52 +511,22 @@ static void apic_send_ipi(struct kvm_lapic *apic)
>
>  static u32 apic_get_tmcct(struct kvm_lapic *apic)
>  {
> -     u64 counter_passed;
> -     ktime_t passed, now;
> +     ktime_t remaining;
> +     s64 ns;
>       u32 tmcct;
>
>       ASSERT(apic != NULL);
>
> -     now = apic->timer.dev.base->get_time();
> -     tmcct = apic_get_reg(apic, APIC_TMICT);
> -
>       /* if initial count is 0, current count should also be 0 */
> -     if (tmcct == 0)
> +     if (apic_get_reg(apic, APIC_TMICT) == 0)
>               return 0;
>
> -     if (unlikely(ktime_to_ns(now) <=
> -             ktime_to_ns(apic->timer.last_update))) {
> -             /* Wrap around */
> -             passed = ktime_add(( {
> -                                 (ktime_t) {
> -                                 .tv64 = KTIME_MAX -
> -                                 (apic->timer.last_update).tv64}; }
> -                                ), now);
> -             apic_debug("time elapsed\n");
> -     } else
> -             passed = ktime_sub(now, apic->timer.last_update);
> -
> -     counter_passed = div64_u64(ktime_to_ns(passed),
> -                                (APIC_BUS_CYCLE_NS * 
> apic->timer.divide_count));
> -
> -     if (counter_passed > tmcct) {
> -             if (unlikely(!apic_lvtt_period(apic))) {
> -                     /* one-shot timers stick at 0 until reset */
> -                     tmcct = 0;
> -             } else {
> -                     /*
> -                      * periodic timers reset to APIC_TMICT when they
> -                      * hit 0. The while loop simulates this happening N
> -                      * times. (counter_passed %= tmcct) would also work,
> -                      * but might be slower or not work on 32-bit??
> -                      */
> -                     while (counter_passed > tmcct)
> -                             counter_passed -= tmcct;
> -                     tmcct -= counter_passed;
> -             }
> -     } else {
> -             tmcct -= counter_passed;
> -     }
> +     remaining = hrtimer_expires_remaining(&apic->timer.dev);
> +     if (remaining.tv64 < 0)

Use ktime_to_ns() macro is better.

The remaining parts are fine with me. But please do more test. :)

Thanks for work!

-- 
regards
Yang, Sheng

> +             remaining = ktime_set(0, 0);
> +
> +     ns = ktime_to_ns(remaining) % apic->timer.period;
> +     tmcct = div64_u64(ns, (APIC_BUS_CYCLE_NS * apic->timer.divide_count));
>
>       return tmcct;
>  }
> @@ -653,8 +623,6 @@ static void start_apic_timer(struct kvm_lapic *apic)
>  {
>       ktime_t now = apic->timer.dev.base->get_time();
>
> -     apic->timer.last_update = now;
> -
>       apic->timer.period = apic_get_reg(apic, APIC_TMICT) *
>                   APIC_BUS_CYCLE_NS * apic->timer.divide_count;
>       atomic_set(&apic->timer.pending, 0);
> @@ -1110,16 +1078,6 @@ void kvm_inject_apic_timer_irqs(struct kvm_vcpu
> *vcpu) }
>  }
>
> -void kvm_apic_timer_intr_post(struct kvm_vcpu *vcpu, int vec)
> -{
> -     struct kvm_lapic *apic = vcpu->arch.apic;
> -
> -     if (apic && apic_lvt_vector(apic, APIC_LVTT) == vec)
> -             apic->timer.last_update = ktime_add_ns(
> -                             apic->timer.last_update,
> -                             apic->timer.period);
> -}
> -
>  int kvm_get_apic_interrupt(struct kvm_vcpu *vcpu)
>  {
>       int vector = kvm_apic_has_interrupt(vcpu);
> diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
> index 8185888..45ab6ee 100644
> --- a/arch/x86/kvm/lapic.h
> +++ b/arch/x86/kvm/lapic.h
> @@ -12,7 +12,6 @@ struct kvm_lapic {
>               atomic_t pending;
>               s64 period;     /* unit: ns */
>               u32 divide_count;
> -             ktime_t last_update;
>               struct hrtimer dev;
>       } timer;
>       struct kvm_vcpu *vcpu;
> @@ -42,7 +41,6 @@ void kvm_set_apic_base(struct kvm_vcpu *vcpu, u64 data);
>  void kvm_apic_post_state_restore(struct kvm_vcpu *vcpu);
>  int kvm_lapic_enabled(struct kvm_vcpu *vcpu);
>  int kvm_lapic_find_highest_irr(struct kvm_vcpu *vcpu);
> -void kvm_apic_timer_intr_post(struct kvm_vcpu *vcpu, int vec);
>
>  void kvm_lapic_set_vapic_addr(struct kvm_vcpu *vcpu, gpa_t vapic_addr);
>  void kvm_lapic_sync_from_vapic(struct kvm_vcpu *vcpu);
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index 14e517e..db5021b 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -2305,7 +2305,6 @@ static void svm_intr_assist(struct kvm_vcpu *vcpu)
>       /* Okay, we can deliver the interrupt: grab it and update PIC state. */
>       intr_vector = kvm_cpu_get_interrupt(vcpu);
>       svm_inject_irq(svm, intr_vector);
> -     kvm_timer_intr_post(vcpu, intr_vector);
>  out:
>       update_cr8_intercept(vcpu);
>  }
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 9b56d21..25aaf11 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -3377,7 +3377,6 @@ static void vmx_intr_assist(struct kvm_vcpu *vcpu)
>       }
>       if (vcpu->arch.interrupt.pending) {
>               vmx_inject_irq(vcpu, vcpu->arch.interrupt.nr);
> -             kvm_timer_intr_post(vcpu, vcpu->arch.interrupt.nr);
>               if (kvm_cpu_has_interrupt(vcpu))
>                       enable_irq_window(vcpu);
>       }

--
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

Reply via email to