Hi,

While booting a non-Linux OS under kvm-46, I noticed that reading
APIC_TMCCT before initializing APIC_TDCR to something other than its
boot time value would lead to a host kernel divide by zero exception.
It's due to apic->timer.divide_count being set to 0 at boot... it should
be set to 2 since APIC_TDCR=0 means 'divide count by 2'.  The last hunk
of the attached patch results in apic->timer.divide_count being set to 2
and eliminates the oops.

The other changes to apic_get_tmcct() are intended to clean it up a bit,
although completely untested other than to verify 0 is returned for a
read of APIC_TMCCT at boot.  'apic' should not be used before the
ASSERT() and using u32 for counter_passed makes it fairly easy to
overflow.

Kevin
--- kvm-46.orig/kernel/lapic.c  2007-10-10 02:06:36.000000000 -0600
+++ kvm-46.fix/kernel/lapic.c   2007-10-12 22:50:01.000000000 -0600
@@ -487,12 +487,19 @@
 
 static u32 apic_get_tmcct(struct kvm_lapic *apic)
 {
-       u32 counter_passed;
-       ktime_t passed, now = apic->timer.dev.base->get_time();
-       u32 tmcct = apic_get_reg(apic, APIC_TMICT);
+       u64 counter_passed;
+       ktime_t passed, now;
+       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)
+               return 0;
+
        if (unlikely(ktime_to_ns(now) <=
                ktime_to_ns(apic->timer.last_update))) {
                /* Wrap around */
@@ -507,15 +514,24 @@
 
        counter_passed = div64_64(ktime_to_ns(passed),
                                  (APIC_BUS_CYCLE_NS * 
apic->timer.divide_count));
-       tmcct -= counter_passed;
 
-       if (tmcct <= 0) {
-               if (unlikely(!apic_lvtt_period(apic)))
+       if (counter_passed > tmcct) {
+               if (unlikely(!apic_lvtt_period(apic))) {
+                       /* one-shot timers stick at 0 until reset */
                        tmcct = 0;
-               else
-                       do {
-                               tmcct += apic_get_reg(apic, APIC_TMICT);
-                       } while (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;
        }
 
        return tmcct;
@@ -844,7 +860,7 @@
                apic_set_reg(apic, APIC_ISR + 0x10 * i, 0);
                apic_set_reg(apic, APIC_TMR + 0x10 * i, 0);
        }
-       apic->timer.divide_count = 0;
+       update_divide_count(apic);
        atomic_set(&apic->timer.pending, 0);
        if (vcpu->vcpu_id == 0)
                vcpu->apic_base |= MSR_IA32_APICBASE_BSP;
-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel

Reply via email to