* [PATCH v2 0/2] deadline TSC multi injection fix
@ 2014-10-10 17:15 Radim Krčmář
2014-10-10 17:15 ` [PATCH v2 1/2] KVM: x86: add apic_timer_expired() Radim Krčmář
2014-10-10 17:15 ` [PATCH v2 2/2] KVM: x86: fix deadline tsc interrupt injection Radim Krčmář
0 siblings, 2 replies; 3+ messages in thread
From: Radim Krčmář @ 2014-10-10 17:15 UTC (permalink / raw
To: linux-kernel; +Cc: kvm, Paolo Bonzini, Nadav Amit
First patch refactors code and introduces a minor should-not-happen
optimization. Second patch fixes the bug and improves handling of passed
deadline timers.
I still haven't looked at Amit's proposed optimization (not recomputing the
value of hrtimer on rewrites) long enough to be able to sign it off, sorry.
Radim Krčmář (2):
KVM: x86: add apic_timer_expired()
KVM: x86: fix deadline tsc interrupt injection
arch/x86/kvm/lapic.c | 47 +++++++++++++++++++++++++----------------------
1 file changed, 25 insertions(+), 22 deletions(-)
--
2.1.0
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH v2 1/2] KVM: x86: add apic_timer_expired()
2014-10-10 17:15 [PATCH v2 0/2] deadline TSC multi injection fix Radim Krčmář
@ 2014-10-10 17:15 ` Radim Krčmář
2014-10-10 17:15 ` [PATCH v2 2/2] KVM: x86: fix deadline tsc interrupt injection Radim Krčmář
1 sibling, 0 replies; 3+ messages in thread
From: Radim Krčmář @ 2014-10-10 17:15 UTC (permalink / raw
To: linux-kernel; +Cc: kvm, Paolo Bonzini, Nadav Amit
Make the code reusable.
If the timer was already pending, we shouldn't be waiting in a queue,
so wake_up can be skipped, simplifying the path.
There is no 'reinject' case => the comment is removed.
Current race behaves correctly.
Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
Not sure about the FIXME, and it might motivate someone to redo the
implicit checking as well :)
arch/x86/kvm/lapic.c | 37 +++++++++++++++++++++----------------
1 file changed, 21 insertions(+), 16 deletions(-)
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index b8345dd..a3cf68b 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -1034,6 +1034,26 @@ static void update_divide_count(struct kvm_lapic *apic)
apic->divide_count);
}
+static void apic_timer_expired(struct kvm_lapic *apic)
+{
+ struct kvm_vcpu *vcpu = apic->vcpu;
+ wait_queue_head_t *q = &vcpu->wq;
+
+ /*
+ * Note: KVM_REQ_PENDING_TIMER is implicitly checked in
+ * vcpu_enter_guest.
+ */
+ if (atomic_read(&apic->lapic_timer.pending))
+ return;
+
+ atomic_inc(&apic->lapic_timer.pending);
+ /* FIXME: this code should not know anything about vcpus */
+ kvm_make_request(KVM_REQ_PENDING_TIMER, vcpu);
+
+ if (waitqueue_active(q))
+ wake_up_interruptible(q);
+}
+
static void start_apic_timer(struct kvm_lapic *apic)
{
ktime_t now;
@@ -1538,23 +1558,8 @@ static enum hrtimer_restart apic_timer_fn(struct hrtimer *data)
{
struct kvm_timer *ktimer = container_of(data, struct kvm_timer, timer);
struct kvm_lapic *apic = container_of(ktimer, struct kvm_lapic, lapic_timer);
- struct kvm_vcpu *vcpu = apic->vcpu;
- wait_queue_head_t *q = &vcpu->wq;
- /*
- * There is a race window between reading and incrementing, but we do
- * not care about potentially losing timer events in the !reinject
- * case anyway. Note: KVM_REQ_PENDING_TIMER is implicitly checked
- * in vcpu_enter_guest.
- */
- if (!atomic_read(&ktimer->pending)) {
- atomic_inc(&ktimer->pending);
- /* FIXME: this code should not know anything about vcpus */
- kvm_make_request(KVM_REQ_PENDING_TIMER, vcpu);
- }
-
- if (waitqueue_active(q))
- wake_up_interruptible(q);
+ apic_timer_expired(apic);
if (lapic_is_periodic(apic)) {
hrtimer_add_expires_ns(&ktimer->timer, ktimer->period);
--
2.1.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH v2 2/2] KVM: x86: fix deadline tsc interrupt injection
2014-10-10 17:15 [PATCH v2 0/2] deadline TSC multi injection fix Radim Krčmář
2014-10-10 17:15 ` [PATCH v2 1/2] KVM: x86: add apic_timer_expired() Radim Krčmář
@ 2014-10-10 17:15 ` Radim Krčmář
1 sibling, 0 replies; 3+ messages in thread
From: Radim Krčmář @ 2014-10-10 17:15 UTC (permalink / raw
To: linux-kernel; +Cc: kvm, Paolo Bonzini, Nadav Amit
The check in kvm_set_lapic_tscdeadline_msr() was trying to prevent a
situation where we lose a pending deadline timer in a MSR write.
Losing it is fine, because it effectively occurs before the timer fired,
so we should be able to cancel or postpone it.
Another problem comes from interaction with QEMU, or other userspace
that can set deadline MSR without a good reason, when timer is already
pending: one guest's deadline request results in more than one
interrupt because one is injected immediately on MSR write from
userspace and one through hrtimer later.
The solution is to remove the injection when replacing a pending timer
and to improve the usual QEMU path, we inject without a hrtimer when the
deadline has already passed.
Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
Reported-by: Nadav Amit <namit@cs.technion.ac.il>
---
arch/x86/kvm/lapic.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index a3cf68b..8b30099 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -1116,9 +1116,10 @@ static void start_apic_timer(struct kvm_lapic *apic)
if (likely(tscdeadline > guest_tsc)) {
ns = (tscdeadline - guest_tsc) * 1000000ULL;
do_div(ns, this_tsc_khz);
- }
- hrtimer_start(&apic->lapic_timer.timer,
- ktime_add_ns(now, ns), HRTIMER_MODE_ABS);
+ hrtimer_start(&apic->lapic_timer.timer,
+ ktime_add_ns(now, ns), HRTIMER_MODE_ABS);
+ } else
+ apic_timer_expired(apic);
local_irq_restore(flags);
}
@@ -1375,9 +1376,6 @@ void kvm_set_lapic_tscdeadline_msr(struct kvm_vcpu *vcpu, u64 data)
return;
hrtimer_cancel(&apic->lapic_timer.timer);
- /* Inject here so clearing tscdeadline won't override new value */
- if (apic_has_pending_timer(vcpu))
- kvm_inject_apic_timer_irqs(vcpu);
apic->lapic_timer.tscdeadline = data;
start_apic_timer(apic);
}
--
2.1.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-10-10 17:15 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-10-10 17:15 [PATCH v2 0/2] deadline TSC multi injection fix Radim Krčmář
2014-10-10 17:15 ` [PATCH v2 1/2] KVM: x86: add apic_timer_expired() Radim Krčmář
2014-10-10 17:15 ` [PATCH v2 2/2] KVM: x86: fix deadline tsc interrupt injection Radim Krčmář
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.