Re: [PATCH] Fix almost infinite loop in APIC

[Marcelo Tosatti <mtosatti@redhat.com>]

On Wed, Jan 14, 2009 at 05:17:22PM +0800, Sheng Yang wrote:
> > So calculating the offset using last interrupt injection is not very
> > reasonable in this case.
> 
> Um... I think it's not easy to find a complete reasonable result here, I 
> haven't look into how OS us TMCCT, maybe it can help us to find a more 
> reasonable result here.

Linux, FreeBSD use it for calibration only. And Windows too, apparently
(at least a couple of versions tested).

What about this?

Index: kvm/arch/x86/kvm/lapic.c
===================================================================
--- kvm.orig/arch/x86/kvm/lapic.c
+++ kvm/arch/x86/kvm/lapic.c
@@ -511,52 +511,31 @@ static void apic_send_ipi(struct kvm_lap
 
 static u32 apic_get_tmcct(struct kvm_lapic *apic)
 {
-	u64 counter_passed;
-	ktime_t passed, now;
-	u32 tmcct;
+	ktime_t remaining;
+	u32 tmcct = 0;
 
 	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;
-	}
+	/*
+	 * Since reinjection is not rate-limited, use the delay
+ 	 * to inject the last interrupt as an estimate.
+ 	 */
+	if (unlikely(atomic_read(&apic->timer.pending) > 0)) {
+		remaining = apic->timer.injection_delay;
+		if (ktime_to_ns(remaining) > apic->timer.period)
+			remaining = ns_to_ktime(apic->timer.period);
+        } else
+		remaining = hrtimer_expires_remaining(&apic->timer.dev);
+
+	if (remaining.tv64 > 0)
+		tmcct = div64_u64(ktime_to_ns(remaining),
+			(APIC_BUS_CYCLE_NS * apic->timer.divide_count));
+
+	WARN_ON(tmcct > apic_get_reg(apic, APIC_TMICT));
 
 	return tmcct;
 }
@@ -653,8 +632,6 @@ static void start_apic_timer(struct kvm_
 {
 	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);
@@ -972,6 +949,7 @@ static int __apic_timer_fn(struct kvm_la
 		set_bit(KVM_REQ_PENDING_TIMER, &apic->vcpu->requests);
 	if (waitqueue_active(q))
 		wake_up_interruptible(q);
+	apic->timer.int_trigger = ktime_get();
 
 	if (apic_lvtt_period(apic)) {
 		result = 1;
@@ -1112,12 +1090,14 @@ void kvm_inject_apic_timer_irqs(struct k
 
 void kvm_apic_timer_intr_post(struct kvm_vcpu *vcpu, int vec)
 {
+	ktime_t delta;
 	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);
+	if (apic && apic_lvt_vector(apic, APIC_LVTT) == vec &&
+	    atomic_read(&apic->timer.pending) == 0) {
+		delta = ktime_sub(ktime_get(), apic->timer.int_trigger);
+		apic->timer.injection_delay = delta;
+	}
 }
 
 int kvm_get_apic_interrupt(struct kvm_vcpu *vcpu)
Index: kvm/arch/x86/kvm/lapic.h
===================================================================
--- kvm.orig/arch/x86/kvm/lapic.h
+++ kvm/arch/x86/kvm/lapic.h
@@ -12,7 +12,8 @@ struct kvm_lapic {
 		atomic_t pending;
 		s64 period;	/* unit: ns */
 		u32 divide_count;
-		ktime_t last_update;
+		ktime_t injection_delay;
+		ktime_t int_trigger;
 		struct hrtimer dev;
 	} timer;
 	struct kvm_vcpu *vcpu;