Skip to contentSkip to conversation

APIC: Do not start zero-period timers

PATCH Superseded Jan Kiszka 3 participants 4 messages Jul 19, 2008 – Aug 18, 2008

lore
Share & tools
lore

Jan Kiszka

author
#0 lore raw

PATCH 1/1 APIC: Do not start zero-period timers

The APIC timer must not start when the initial count is (still) zero.

This caused occasional stalls when booting secondary CPUs of Linux SMP guests.

---
 hw/apic.c |    2 ++
 1 file changed, 2 insertions(+)
hw/apic.c +2 -0
Index: b/hw/apic.c
===================================================================
@@ -566,6 +566,8 @@ static void apic_timer_update(APICState
         d = (current_time - s->initial_count_load_time) >>
             s->count_shift;
         if (s->lvt[APIC_LVT_TIMER] & APIC_LVT_TIMER_PERIODIC) {
+            if (!s->initial_count)
+                goto no_timer;
             d = ((d / ((uint64_t)s->initial_count + 1)) + 1) * ((uint64_t)s->initial_count + 1);
         } else {
             if (d >= s->initial_count)
author
#1 lore raw

PATCH RESEND 1/1 APIC: Do not start zero-period timers

The APIC timer must not start when the initial count is (still) zero.

This caused occasional stalls when booting secondary CPUs of Linux SMP guests.

---
 hw/apic.c |    2 ++
 1 file changed, 2 insertions(+)
hw/apic.c +2 -0
Index: b/hw/apic.c
===================================================================
@@ -566,6 +566,8 @@ static void apic_timer_update(APICState
         d = (current_time - s->initial_count_load_time) >>
             s->count_shift;
         if (s->lvt[APIC_LVT_TIMER] & APIC_LVT_TIMER_PERIODIC) {
+            if (!s->initial_count)
+                goto no_timer;
             d = ((d / ((uint64_t)s->initial_count + 1)) + 1) * ((uint64_t)s->initial_count + 1);
         } else {
             if (d >= s->initial_count)
#2 lore raw
Quoted 5 lines · Jan Kiszka

The APIC timer must not start when the initial count is (still) zero.

This caused occasional stalls when booting secondary CPUs of Linux SMP guests.

Applied. Thanks.

Besides testing, I confirmed that this is what's done in the in-kernel APIC in KVM (which is also used by Xen). It seems really unfortunate to me that there are fixes in the in-kernel APIC that aren't in QEMU (and perhaps vice-versa). I wonder if there's any way to unify the two code bases?

Regards,

Anthony Liguori

#3 lore raw
Quoted 10 lines · Anthony Liguori
Quoted 5 lines · Jan Kiszka

The APIC timer must not start when the initial count is (still) zero.

This caused occasional stalls when booting secondary CPUs of Linux SMP guests.

Applied. Thanks.

Besides testing, I confirmed that this is what's done in the in-kernel APIC in KVM (which is also used by Xen). It seems really unfortunate to me that there are fixes in the in-kernel APIC that aren't in QEMU (and perhaps vice-versa). I wonder if there's any way to unify the two code bases?

Just a thought - if the code bases have similar textual content, one could start playing games like noticing when the kernel releases have APIC changes in KVM, and then mailing those as [PATCH] automatically to this list by detecting the commit log messages for them from kernel git.

And vice versa, even?

Even just a web page that shows the two implementations side by side, highlighting the diff, could probably be useful.

-- Asheesh.

Signature · 1 line
Experience is what you get when you were expecting something else.