diff options
author | Marcelo Tosatti <mtosatti@redhat.com> | 2008-07-26 16:01:01 -0400 |
---|---|---|
committer | Avi Kivity <avi@qumranet.com> | 2008-10-15 04:15:17 -0400 |
commit | 3cf57fed216e2c1b6fdfeccb792650bab72a350a (patch) | |
tree | 5a557f5870f5fccfbdee3bd61cab5a15e6cdc012 /arch/x86/kvm | |
parent | f52447261bc8c21dfd4635196e32d2da1352f589 (diff) |
KVM: PIT: fix injection logic and count
The PIT injection logic is problematic under the following cases:
1) If there is a higher priority vector to be delivered by the time
kvm_pit_timer_intr_post is invoked ps->inject_pending won't be set.
This opens the possibility for missing many PIT event injections (say if
guest executes hlt at this point).
2) ps->inject_pending is racy with more than two vcpus. Since there's no locking
around read/dec of pt->pending, two vcpu's can inject two interrupts for a single
pt->pending count.
Fix 1 by using an irq ack notifier: only reinject when the previous irq
has been acked. Fix 2 with appropriate locking around manipulation of
pending count and irq_ack by the injection / ack paths.
Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
Signed-off-by: Avi Kivity <avi@qumranet.com>
Diffstat (limited to 'arch/x86/kvm')
-rw-r--r-- | arch/x86/kvm/i8254.c | 70 | ||||
-rw-r--r-- | arch/x86/kvm/i8254.h | 7 | ||||
-rw-r--r-- | arch/x86/kvm/irq.c | 1 |
3 files changed, 38 insertions, 40 deletions
diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c index c0f7872a9124..7d04dd3ef857 100644 --- a/arch/x86/kvm/i8254.c +++ b/arch/x86/kvm/i8254.c | |||
@@ -207,6 +207,8 @@ static int __pit_timer_fn(struct kvm_kpit_state *ps) | |||
207 | 207 | ||
208 | pt->timer.expires = ktime_add_ns(pt->timer.expires, pt->period); | 208 | pt->timer.expires = ktime_add_ns(pt->timer.expires, pt->period); |
209 | pt->scheduled = ktime_to_ns(pt->timer.expires); | 209 | pt->scheduled = ktime_to_ns(pt->timer.expires); |
210 | if (pt->period) | ||
211 | ps->channels[0].count_load_time = pt->timer.expires; | ||
210 | 212 | ||
211 | return (pt->period == 0 ? 0 : 1); | 213 | return (pt->period == 0 ? 0 : 1); |
212 | } | 214 | } |
@@ -215,12 +217,22 @@ int pit_has_pending_timer(struct kvm_vcpu *vcpu) | |||
215 | { | 217 | { |
216 | struct kvm_pit *pit = vcpu->kvm->arch.vpit; | 218 | struct kvm_pit *pit = vcpu->kvm->arch.vpit; |
217 | 219 | ||
218 | if (pit && vcpu->vcpu_id == 0 && pit->pit_state.inject_pending) | 220 | if (pit && vcpu->vcpu_id == 0 && pit->pit_state.irq_ack) |
219 | return atomic_read(&pit->pit_state.pit_timer.pending); | 221 | return atomic_read(&pit->pit_state.pit_timer.pending); |
220 | |||
221 | return 0; | 222 | return 0; |
222 | } | 223 | } |
223 | 224 | ||
225 | void kvm_pit_ack_irq(struct kvm_irq_ack_notifier *kian) | ||
226 | { | ||
227 | struct kvm_kpit_state *ps = container_of(kian, struct kvm_kpit_state, | ||
228 | irq_ack_notifier); | ||
229 | spin_lock(&ps->inject_lock); | ||
230 | if (atomic_dec_return(&ps->pit_timer.pending) < 0) | ||
231 | WARN_ON(1); | ||
232 | ps->irq_ack = 1; | ||
233 | spin_unlock(&ps->inject_lock); | ||
234 | } | ||
235 | |||
224 | static enum hrtimer_restart pit_timer_fn(struct hrtimer *data) | 236 | static enum hrtimer_restart pit_timer_fn(struct hrtimer *data) |
225 | { | 237 | { |
226 | struct kvm_kpit_state *ps; | 238 | struct kvm_kpit_state *ps; |
@@ -255,8 +267,9 @@ static void destroy_pit_timer(struct kvm_kpit_timer *pt) | |||
255 | hrtimer_cancel(&pt->timer); | 267 | hrtimer_cancel(&pt->timer); |
256 | } | 268 | } |
257 | 269 | ||
258 | static void create_pit_timer(struct kvm_kpit_timer *pt, u32 val, int is_period) | 270 | static void create_pit_timer(struct kvm_kpit_state *ps, u32 val, int is_period) |
259 | { | 271 | { |
272 | struct kvm_kpit_timer *pt = &ps->pit_timer; | ||
260 | s64 interval; | 273 | s64 interval; |
261 | 274 | ||
262 | interval = muldiv64(val, NSEC_PER_SEC, KVM_PIT_FREQ); | 275 | interval = muldiv64(val, NSEC_PER_SEC, KVM_PIT_FREQ); |
@@ -268,6 +281,7 @@ static void create_pit_timer(struct kvm_kpit_timer *pt, u32 val, int is_period) | |||
268 | pt->period = (is_period == 0) ? 0 : interval; | 281 | pt->period = (is_period == 0) ? 0 : interval; |
269 | pt->timer.function = pit_timer_fn; | 282 | pt->timer.function = pit_timer_fn; |
270 | atomic_set(&pt->pending, 0); | 283 | atomic_set(&pt->pending, 0); |
284 | ps->irq_ack = 1; | ||
271 | 285 | ||
272 | hrtimer_start(&pt->timer, ktime_add_ns(ktime_get(), interval), | 286 | hrtimer_start(&pt->timer, ktime_add_ns(ktime_get(), interval), |
273 | HRTIMER_MODE_ABS); | 287 | HRTIMER_MODE_ABS); |
@@ -302,11 +316,11 @@ static void pit_load_count(struct kvm *kvm, int channel, u32 val) | |||
302 | case 1: | 316 | case 1: |
303 | /* FIXME: enhance mode 4 precision */ | 317 | /* FIXME: enhance mode 4 precision */ |
304 | case 4: | 318 | case 4: |
305 | create_pit_timer(&ps->pit_timer, val, 0); | 319 | create_pit_timer(ps, val, 0); |
306 | break; | 320 | break; |
307 | case 2: | 321 | case 2: |
308 | case 3: | 322 | case 3: |
309 | create_pit_timer(&ps->pit_timer, val, 1); | 323 | create_pit_timer(ps, val, 1); |
310 | break; | 324 | break; |
311 | default: | 325 | default: |
312 | destroy_pit_timer(&ps->pit_timer); | 326 | destroy_pit_timer(&ps->pit_timer); |
@@ -520,7 +534,7 @@ void kvm_pit_reset(struct kvm_pit *pit) | |||
520 | mutex_unlock(&pit->pit_state.lock); | 534 | mutex_unlock(&pit->pit_state.lock); |
521 | 535 | ||
522 | atomic_set(&pit->pit_state.pit_timer.pending, 0); | 536 | atomic_set(&pit->pit_state.pit_timer.pending, 0); |
523 | pit->pit_state.inject_pending = 1; | 537 | pit->pit_state.irq_ack = 1; |
524 | } | 538 | } |
525 | 539 | ||
526 | struct kvm_pit *kvm_create_pit(struct kvm *kvm) | 540 | struct kvm_pit *kvm_create_pit(struct kvm *kvm) |
@@ -534,6 +548,7 @@ struct kvm_pit *kvm_create_pit(struct kvm *kvm) | |||
534 | 548 | ||
535 | mutex_init(&pit->pit_state.lock); | 549 | mutex_init(&pit->pit_state.lock); |
536 | mutex_lock(&pit->pit_state.lock); | 550 | mutex_lock(&pit->pit_state.lock); |
551 | spin_lock_init(&pit->pit_state.inject_lock); | ||
537 | 552 | ||
538 | /* Initialize PIO device */ | 553 | /* Initialize PIO device */ |
539 | pit->dev.read = pit_ioport_read; | 554 | pit->dev.read = pit_ioport_read; |
@@ -555,6 +570,9 @@ struct kvm_pit *kvm_create_pit(struct kvm *kvm) | |||
555 | pit_state->pit = pit; | 570 | pit_state->pit = pit; |
556 | hrtimer_init(&pit_state->pit_timer.timer, | 571 | hrtimer_init(&pit_state->pit_timer.timer, |
557 | CLOCK_MONOTONIC, HRTIMER_MODE_ABS); | 572 | CLOCK_MONOTONIC, HRTIMER_MODE_ABS); |
573 | pit_state->irq_ack_notifier.gsi = 0; | ||
574 | pit_state->irq_ack_notifier.irq_acked = kvm_pit_ack_irq; | ||
575 | kvm_register_irq_ack_notifier(kvm, &pit_state->irq_ack_notifier); | ||
558 | mutex_unlock(&pit->pit_state.lock); | 576 | mutex_unlock(&pit->pit_state.lock); |
559 | 577 | ||
560 | kvm_pit_reset(pit); | 578 | kvm_pit_reset(pit); |
@@ -592,37 +610,19 @@ void kvm_inject_pit_timer_irqs(struct kvm_vcpu *vcpu) | |||
592 | struct kvm_kpit_state *ps; | 610 | struct kvm_kpit_state *ps; |
593 | 611 | ||
594 | if (vcpu && pit) { | 612 | if (vcpu && pit) { |
613 | int inject = 0; | ||
595 | ps = &pit->pit_state; | 614 | ps = &pit->pit_state; |
596 | 615 | ||
597 | /* Try to inject pending interrupts when: | 616 | /* Try to inject pending interrupts when |
598 | * 1. Pending exists | 617 | * last one has been acked. |
599 | * 2. Last interrupt was accepted or waited for too long time*/ | 618 | */ |
600 | if (atomic_read(&ps->pit_timer.pending) && | 619 | spin_lock(&ps->inject_lock); |
601 | (ps->inject_pending || | 620 | if (atomic_read(&ps->pit_timer.pending) && ps->irq_ack) { |
602 | (jiffies - ps->last_injected_time | 621 | ps->irq_ack = 0; |
603 | >= KVM_MAX_PIT_INTR_INTERVAL))) { | 622 | inject = 1; |
604 | ps->inject_pending = 0; | ||
605 | __inject_pit_timer_intr(kvm); | ||
606 | ps->last_injected_time = jiffies; | ||
607 | } | ||
608 | } | ||
609 | } | ||
610 | |||
611 | void kvm_pit_timer_intr_post(struct kvm_vcpu *vcpu, int vec) | ||
612 | { | ||
613 | struct kvm_arch *arch = &vcpu->kvm->arch; | ||
614 | struct kvm_kpit_state *ps; | ||
615 | |||
616 | if (vcpu && arch->vpit) { | ||
617 | ps = &arch->vpit->pit_state; | ||
618 | if (atomic_read(&ps->pit_timer.pending) && | ||
619 | (((arch->vpic->pics[0].imr & 1) == 0 && | ||
620 | arch->vpic->pics[0].irq_base == vec) || | ||
621 | (arch->vioapic->redirtbl[0].fields.vector == vec && | ||
622 | arch->vioapic->redirtbl[0].fields.mask != 1))) { | ||
623 | ps->inject_pending = 1; | ||
624 | atomic_dec(&ps->pit_timer.pending); | ||
625 | ps->channels[0].count_load_time = ktime_get(); | ||
626 | } | 623 | } |
624 | spin_unlock(&ps->inject_lock); | ||
625 | if (inject) | ||
626 | __inject_pit_timer_intr(kvm); | ||
627 | } | 627 | } |
628 | } | 628 | } |
diff --git a/arch/x86/kvm/i8254.h b/arch/x86/kvm/i8254.h index db25c2a6c8c4..e436d4983aa1 100644 --- a/arch/x86/kvm/i8254.h +++ b/arch/x86/kvm/i8254.h | |||
@@ -8,7 +8,6 @@ struct kvm_kpit_timer { | |||
8 | int irq; | 8 | int irq; |
9 | s64 period; /* unit: ns */ | 9 | s64 period; /* unit: ns */ |
10 | s64 scheduled; | 10 | s64 scheduled; |
11 | ktime_t last_update; | ||
12 | atomic_t pending; | 11 | atomic_t pending; |
13 | }; | 12 | }; |
14 | 13 | ||
@@ -34,8 +33,9 @@ struct kvm_kpit_state { | |||
34 | u32 speaker_data_on; | 33 | u32 speaker_data_on; |
35 | struct mutex lock; | 34 | struct mutex lock; |
36 | struct kvm_pit *pit; | 35 | struct kvm_pit *pit; |
37 | bool inject_pending; /* if inject pending interrupts */ | 36 | spinlock_t inject_lock; |
38 | unsigned long last_injected_time; | 37 | unsigned long irq_ack; |
38 | struct kvm_irq_ack_notifier irq_ack_notifier; | ||
39 | }; | 39 | }; |
40 | 40 | ||
41 | struct kvm_pit { | 41 | struct kvm_pit { |
@@ -54,7 +54,6 @@ struct kvm_pit { | |||
54 | #define KVM_PIT_CHANNEL_MASK 0x3 | 54 | #define KVM_PIT_CHANNEL_MASK 0x3 |
55 | 55 | ||
56 | void kvm_inject_pit_timer_irqs(struct kvm_vcpu *vcpu); | 56 | void kvm_inject_pit_timer_irqs(struct kvm_vcpu *vcpu); |
57 | void kvm_pit_timer_intr_post(struct kvm_vcpu *vcpu, int vec); | ||
58 | void kvm_pit_load_count(struct kvm *kvm, int channel, u32 val); | 57 | void kvm_pit_load_count(struct kvm *kvm, int channel, u32 val); |
59 | struct kvm_pit *kvm_create_pit(struct kvm *kvm); | 58 | struct kvm_pit *kvm_create_pit(struct kvm *kvm); |
60 | void kvm_free_pit(struct kvm *kvm); | 59 | void kvm_free_pit(struct kvm *kvm); |
diff --git a/arch/x86/kvm/irq.c b/arch/x86/kvm/irq.c index 3c508afaa285..8c1b9c5def78 100644 --- a/arch/x86/kvm/irq.c +++ b/arch/x86/kvm/irq.c | |||
@@ -90,7 +90,6 @@ EXPORT_SYMBOL_GPL(kvm_inject_pending_timer_irqs); | |||
90 | void kvm_timer_intr_post(struct kvm_vcpu *vcpu, int vec) | 90 | void kvm_timer_intr_post(struct kvm_vcpu *vcpu, int vec) |
91 | { | 91 | { |
92 | kvm_apic_timer_intr_post(vcpu, vec); | 92 | kvm_apic_timer_intr_post(vcpu, vec); |
93 | kvm_pit_timer_intr_post(vcpu, vec); | ||
94 | /* TODO: PIT, RTC etc. */ | 93 | /* TODO: PIT, RTC etc. */ |
95 | } | 94 | } |
96 | EXPORT_SYMBOL_GPL(kvm_timer_intr_post); | 95 | EXPORT_SYMBOL_GPL(kvm_timer_intr_post); |