Re: [PATCH] Fix almost infinite loop in APIC

Marcelo Tosatti
2009-01-14T17:04:21Z

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;