Kevin Pedretti wrote:
> 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.
>
Patch looks good, but I'm missing a signed-off-by: line.
Eddie, can you also take a look?
> --- 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;
--
error compiling committee.c: too many arguments to function
-------------------------------------------------------------------------
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
[email protected]
https://lists.sourceforge.net/lists/listinfo/kvm-devel
