* KVM: migrate PIT timer
@ 2008-05-25 5:29 Marcelo Tosatti
2008-05-27 13:17 ` Avi Kivity
0 siblings, 1 reply; 4+ messages in thread
From: Marcelo Tosatti @ 2008-05-25 5:29 UTC (permalink / raw)
To: Avi Kivity; +Cc: kvm-devel
Avi,
There was a discussion regarding PIT timer migration sometime ago, you
said:
"I'm not sure hrtimer migration would work 100% reliably (suppose it
fired just after a vcpu migration) so I think a queue_work is better."
I fail to see any problem with a timer firing either before or after the
request bit is set in the vcpu's request mask during vcpu_load().
Whatever the case, nothing will stop __vcpu_run() from migrating the
timer to vcpu0 (in PIT's case), and any instances that fired on a
different CPU between vcpu_load() and hrtimer_cancel/start will be
injected in kvm_inject_pending_timer_irqs(). Is there anything
you can think of?
-------
Migrate the PIT timer to the physical CPU which vcpu0 is scheduled on,
similarly to what is done for the LAPIC timers, otherwise PIT interrupts
will be delayed until an unrelated event causes an exit.
Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
Index: kvm.realtip/arch/x86/kvm/i8254.c
===================================================================
--- kvm.realtip.orig/arch/x86/kvm/i8254.c
+++ kvm.realtip/arch/x86/kvm/i8254.c
@@ -200,7 +200,6 @@ static int __pit_timer_fn(struct kvm_kpi
atomic_inc(&pt->pending);
smp_mb__after_atomic_inc();
- /* FIXME: handle case where the guest is in guest mode */
if (vcpu0 && waitqueue_active(&vcpu0->wq)) {
vcpu0->arch.mp_state = KVM_MP_STATE_RUNNABLE;
wake_up_interruptible(&vcpu0->wq);
@@ -237,6 +236,20 @@ static enum hrtimer_restart pit_timer_fn
return HRTIMER_NORESTART;
}
+void __kvm_migrate_pit_timer(struct kvm_vcpu *vcpu)
+{
+ struct kvm_pit *pit = vcpu->kvm->arch.vpit;
+ struct hrtimer *timer;
+
+ if (!pit)
+ return;
+
+ timer = &pit->pit_state.pit_timer.timer;
+ if (hrtimer_cancel(timer))
+ hrtimer_start(timer, timer->expires, HRTIMER_MODE_ABS);
+}
+
+
static void destroy_pit_timer(struct kvm_kpit_timer *pt)
{
pr_debug("pit: execute del timer!\n");
Index: kvm.realtip/arch/x86/kvm/irq.h
===================================================================
-- kvm.realtip.orig/arch/x86/kvm/irq.h
+++ kvm.realtip/arch/x86/kvm/irq.h
@@ -84,6 +84,7 @@ void kvm_timer_intr_post(struct kvm_vcpu
void kvm_inject_pending_timer_irqs(struct kvm_vcpu *vcpu);
void kvm_inject_apic_timer_irqs(struct kvm_vcpu *vcpu);
void __kvm_migrate_apic_timer(struct kvm_vcpu *vcpu);
+void __kvm_migrate_pit_timer(struct kvm_vcpu *vcpu);
int pit_has_pending_timer(struct kvm_vcpu *vcpu);
int apic_has_pending_timer(struct kvm_vcpu *vcpu);
Index: kvm.realtip/arch/x86/kvm/svm.c
===================================================================
--- kvm.realtip.orig/arch/x86/kvm/svm.c
+++ kvm.realtip/arch/x86/kvm/svm.c
@@ -690,7 +690,7 @@ static void svm_vcpu_load(struct kvm_vcp
delta = vcpu->arch.host_tsc - tsc_this;
svm->vmcb->control.tsc_offset += delta;
vcpu->cpu = cpu;
- kvm_migrate_apic_timer(vcpu);
+ kvm_migrate_timers(vcpu);
}
for (i = 0; i < NR_HOST_SAVE_USER_MSRS; i++)
Index: kvm.realtip/arch/x86/kvm/vmx.c
===================================================================
--- kvm.realtip.orig/arch/x86/kvm/vmx.c
+++ kvm.realtip/arch/x86/kvm/vmx.c
@@ -616,7 +616,7 @@ static void vmx_vcpu_load(struct kvm_vcp
if (vcpu->cpu != cpu) {
vcpu_clear(vmx);
- kvm_migrate_apic_timer(vcpu);
+ kvm_migrate_timers(vcpu);
vpid_sync_vcpu_all(vmx);
local_irq_disable();
list_add(&vmx->local_vcpus_link,
Index: kvm.realtip/arch/x86/kvm/x86.c
===================================================================
--- kvm.realtip.orig/arch/x86/kvm/x86.c
+++ kvm.realtip/arch/x86/kvm/x86.c
@@ -2744,8 +2744,10 @@ again:
goto out;
if (vcpu->requests) {
- if (test_and_clear_bit(KVM_REQ_MIGRATE_TIMER, &vcpu->requests))
+ if (test_and_clear_bit(KVM_REQ_MIGRATE_APIC, &vcpu->requests))
__kvm_migrate_apic_timer(vcpu);
+ if (test_and_clear_bit(KVM_REQ_MIGRATE_PIT, &vcpu->requests))
+ __kvm_migrate_pit_timer(vcpu);
if (test_and_clear_bit(KVM_REQ_REPORT_TPR_ACCESS,
&vcpu->requests)) {
kvm_run->exit_reason = KVM_EXIT_TPR_ACCESS;
Index: kvm.realtip/include/linux/kvm_host.h
===================================================================
--- kvm.realtip.orig/include/linux/kvm_host.h
+++ kvm.realtip/include/linux/kvm_host.h
@@ -29,10 +29,11 @@
* vcpu->requests bit members
*/
#define KVM_REQ_TLB_FLUSH 0
-#define KVM_REQ_MIGRATE_TIMER 1
+#define KVM_REQ_MIGRATE_APIC 1
#define KVM_REQ_REPORT_TPR_ACCESS 2
#define KVM_REQ_MMU_RELOAD 3
#define KVM_REQ_TRIPLE_FAULT 4
+#define KVM_REQ_MIGRATE_PIT 5
struct kvm_vcpu;
extern struct kmem_cache *kvm_vcpu_cache;
@@ -294,9 +295,11 @@ static inline gpa_t gfn_to_gpa(gfn_t gfn
return (gpa_t)gfn << PAGE_SHIFT;
}
-static inline void kvm_migrate_apic_timer(struct kvm_vcpu *vcpu)
+static inline void kvm_migrate_timers(struct kvm_vcpu *vcpu)
{
- set_bit(KVM_REQ_MIGRATE_TIMER, &vcpu->requests);
+ set_bit(KVM_REQ_MIGRATE_APIC, &vcpu->requests);
+ if (vcpu->vcpu_id == 0)
+ set_bit(KVM_REQ_MIGRATE_PIT, &vcpu->requests);
}
enum kvm_stat_kind {
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: KVM: migrate PIT timer
2008-05-25 5:29 KVM: migrate PIT timer Marcelo Tosatti
@ 2008-05-27 13:17 ` Avi Kivity
2008-05-27 15:10 ` Marcelo Tosatti
0 siblings, 1 reply; 4+ messages in thread
From: Avi Kivity @ 2008-05-27 13:17 UTC (permalink / raw)
To: Marcelo Tosatti; +Cc: kvm-devel
Marcelo Tosatti wrote:
> Avi,
>
> There was a discussion regarding PIT timer migration sometime ago, you
> said:
>
> "I'm not sure hrtimer migration would work 100% reliably (suppose it
> fired just after a vcpu migration) so I think a queue_work is better."
>
> I fail to see any problem with a timer firing either before or after the
> request bit is set in the vcpu's request mask during vcpu_load().
>
> Whatever the case, nothing will stop __vcpu_run() from migrating the
> timer to vcpu0 (in PIT's case), and any instances that fired on a
> different CPU between vcpu_load() and hrtimer_cancel/start will be
> injected in kvm_inject_pending_timer_irqs(). Is there anything
> you can think of?
>
>
No, it seems fine.
> -------
>
> Migrate the PIT timer to the physical CPU which vcpu0 is scheduled on,
> similarly to what is done for the LAPIC timers, otherwise PIT interrupts
> will be delayed until an unrelated event causes an exit.
>
>
> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
>
>
> Index: kvm.realtip/arch/x86/kvm/i8254.c
> ===================================================================
> --- kvm.realtip.orig/arch/x86/kvm/i8254.c
> +++ kvm.realtip/arch/x86/kvm/i8254.c
> @@ -200,7 +200,6 @@ static int __pit_timer_fn(struct kvm_kpi
>
> atomic_inc(&pt->pending);
> smp_mb__after_atomic_inc();
> - /* FIXME: handle case where the guest is in guest mode */
> if (vcpu0 && waitqueue_active(&vcpu0->wq)) {
> vcpu0->arch.mp_state = KVM_MP_STATE_RUNNABLE;
> wake_up_interruptible(&vcpu0->wq);
> @@ -237,6 +236,20 @@ static enum hrtimer_restart pit_timer_fn
> return HRTIMER_NORESTART;
> }
>
> +void __kvm_migrate_pit_timer(struct kvm_vcpu *vcpu)
> +{
> + struct kvm_pit *pit = vcpu->kvm->arch.vpit;
> + struct hrtimer *timer;
> +
> + if (!pit)
> + return;
> +
> + timer = &pit->pit_state.pit_timer.timer;
> + if (hrtimer_cancel(timer))
> + hrtimer_start(timer, timer->expires, HRTIMER_MODE_ABS);
> +}
> +
> +
>
Extra newline.
> static void destroy_pit_timer(struct kvm_kpit_timer *pt)
> {
> pr_debug("pit: execute del timer!\n");
> Index: kvm.realtip/arch/x86/kvm/irq.h
> ===================================================================
> -- kvm.realtip.orig/arch/x86/kvm/irq.h
> +++ kvm.realtip/arch/x86/kvm/irq.h
> @@ -84,6 +84,7 @@ void kvm_timer_intr_post(struct kvm_vcpu
> void kvm_inject_pending_timer_irqs(struct kvm_vcpu *vcpu);
> void kvm_inject_apic_timer_irqs(struct kvm_vcpu *vcpu);
> void __kvm_migrate_apic_timer(struct kvm_vcpu *vcpu);
> +void __kvm_migrate_pit_timer(struct kvm_vcpu *vcpu);
>
> int pit_has_pending_timer(struct kvm_vcpu *vcpu);
> int apic_has_pending_timer(struct kvm_vcpu *vcpu);
> Index: kvm.realtip/arch/x86/kvm/svm.c
> ===================================================================
> --- kvm.realtip.orig/arch/x86/kvm/svm.c
> +++ kvm.realtip/arch/x86/kvm/svm.c
> @@ -690,7 +690,7 @@ static void svm_vcpu_load(struct kvm_vcp
> delta = vcpu->arch.host_tsc - tsc_this;
> svm->vmcb->control.tsc_offset += delta;
> vcpu->cpu = cpu;
> - kvm_migrate_apic_timer(vcpu);
> + kvm_migrate_timers(vcpu);
> }
>
> for (i = 0; i < NR_HOST_SAVE_USER_MSRS; i++)
> Index: kvm.realtip/arch/x86/kvm/vmx.c
> ===================================================================
> --- kvm.realtip.orig/arch/x86/kvm/vmx.c
> +++ kvm.realtip/arch/x86/kvm/vmx.c
> @@ -616,7 +616,7 @@ static void vmx_vcpu_load(struct kvm_vcp
>
> if (vcpu->cpu != cpu) {
> vcpu_clear(vmx);
> - kvm_migrate_apic_timer(vcpu);
> + kvm_migrate_timers(vcpu);
> vpid_sync_vcpu_all(vmx);
> local_irq_disable();
> list_add(&vmx->local_vcpus_link,
> Index: kvm.realtip/arch/x86/kvm/x86.c
> ===================================================================
> --- kvm.realtip.orig/arch/x86/kvm/x86.c
> +++ kvm.realtip/arch/x86/kvm/x86.c
> @@ -2744,8 +2744,10 @@ again:
> goto out;
>
> if (vcpu->requests) {
> - if (test_and_clear_bit(KVM_REQ_MIGRATE_TIMER, &vcpu->requests))
> + if (test_and_clear_bit(KVM_REQ_MIGRATE_APIC, &vcpu->requests))
> __kvm_migrate_apic_timer(vcpu);
> + if (test_and_clear_bit(KVM_REQ_MIGRATE_PIT, &vcpu->requests))
> + __kvm_migrate_pit_timer(vcpu);
> if (test_and_clear_bit(KVM_REQ_REPORT_TPR_ACCESS,
> &vcpu->requests)) {
> kvm_run->exit_reason = KVM_EXIT_TPR_ACCESS;
> Index: kvm.realtip/include/linux/kvm_host.h
> ===================================================================
> --- kvm.realtip.orig/include/linux/kvm_host.h
> +++ kvm.realtip/include/linux/kvm_host.h
> @@ -29,10 +29,11 @@
> * vcpu->requests bit members
> */
> #define KVM_REQ_TLB_FLUSH 0
> -#define KVM_REQ_MIGRATE_TIMER 1
> +#define KVM_REQ_MIGRATE_APIC 1
> #define KVM_REQ_REPORT_TPR_ACCESS 2
> #define KVM_REQ_MMU_RELOAD 3
> #define KVM_REQ_TRIPLE_FAULT 4
> +#define KVM_REQ_MIGRATE_PIT 5
>
> struct kvm_vcpu;
> extern struct kmem_cache *kvm_vcpu_cache;
> @@ -294,9 +295,11 @@ static inline gpa_t gfn_to_gpa(gfn_t gfn
> return (gpa_t)gfn << PAGE_SHIFT;
> }
>
> -static inline void kvm_migrate_apic_timer(struct kvm_vcpu *vcpu)
> +static inline void kvm_migrate_timers(struct kvm_vcpu *vcpu)
> {
> - set_bit(KVM_REQ_MIGRATE_TIMER, &vcpu->requests);
> + set_bit(KVM_REQ_MIGRATE_APIC, &vcpu->requests);
> + if (vcpu->vcpu_id == 0)
> + set_bit(KVM_REQ_MIGRATE_PIT, &vcpu->requests);
> }
>
Instead of having two timer bits, how about keeping just on
MIGRATE_TIMER bit which migrates both, and having
__kvm_migrate_pit_timer() return if not on vcpu 0?
This can result in shorter code, and in less proliferation of the "pit
is bound to vcpu 0" logic.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: KVM: migrate PIT timer
2008-05-27 13:17 ` Avi Kivity
@ 2008-05-27 15:10 ` Marcelo Tosatti
2008-05-28 6:34 ` Avi Kivity
0 siblings, 1 reply; 4+ messages in thread
From: Marcelo Tosatti @ 2008-05-27 15:10 UTC (permalink / raw)
To: Avi Kivity; +Cc: kvm-devel
On Tue, May 27, 2008 at 04:17:30PM +0300, Avi Kivity wrote:
> Instead of having two timer bits, how about keeping just on
> MIGRATE_TIMER bit which migrates both, and having
> __kvm_migrate_pit_timer() return if not on vcpu 0?
>
> This can result in shorter code, and in less proliferation of the "pit
> is bound to vcpu 0" logic.
Sure.
Migrate the PIT timer to the physical CPU which vcpu0 is scheduled on,
similarly to what is done for the LAPIC timers, otherwise PIT interrupts
will be delayed until an unrelated event causes an exit.
Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c
index 6d6dc6c..9b33b27 100644
--- a/arch/x86/kvm/i8254.c
+++ b/arch/x86/kvm/i8254.c
@@ -200,7 +200,6 @@ static int __pit_timer_fn(struct kvm_kpit_state *ps)
atomic_inc(&pt->pending);
smp_mb__after_atomic_inc();
- /* FIXME: handle case where the guest is in guest mode */
if (vcpu0 && waitqueue_active(&vcpu0->wq)) {
vcpu0->arch.mp_state = KVM_MP_STATE_RUNNABLE;
wake_up_interruptible(&vcpu0->wq);
@@ -237,6 +236,19 @@ static enum hrtimer_restart pit_timer_fn(struct hrtimer *data)
return HRTIMER_NORESTART;
}
+void __kvm_migrate_pit_timer(struct kvm_vcpu *vcpu)
+{
+ struct kvm_pit *pit = vcpu->kvm->arch.vpit;
+ struct hrtimer *timer;
+
+ if (vcpu->vcpu_id != 0 || !pit)
+ return;
+
+ timer = &pit->pit_state.pit_timer.timer;
+ if (hrtimer_cancel(timer))
+ hrtimer_start(timer, timer->expires, HRTIMER_MODE_ABS);
+}
+
static void destroy_pit_timer(struct kvm_kpit_timer *pt)
{
pr_debug("pit: execute del timer!\n");
diff --git a/arch/x86/kvm/irq.c b/arch/x86/kvm/irq.c
index ce1f583..76d736b 100644
--- a/arch/x86/kvm/irq.c
+++ b/arch/x86/kvm/irq.c
@@ -94,3 +94,9 @@ void kvm_timer_intr_post(struct kvm_vcpu *vcpu, int vec)
/* TODO: PIT, RTC etc. */
}
EXPORT_SYMBOL_GPL(kvm_timer_intr_post);
+
+void __kvm_migrate_timers(struct kvm_vcpu *vcpu)
+{
+ __kvm_migrate_apic_timer(vcpu);
+ __kvm_migrate_pit_timer(vcpu);
+}
diff --git a/arch/x86/kvm/irq.h b/arch/x86/kvm/irq.h
index 1802134..2a15be2 100644
--- a/arch/x86/kvm/irq.h
+++ b/arch/x86/kvm/irq.h
@@ -84,6 +84,8 @@ void kvm_timer_intr_post(struct kvm_vcpu *vcpu, int vec);
void kvm_inject_pending_timer_irqs(struct kvm_vcpu *vcpu);
void kvm_inject_apic_timer_irqs(struct kvm_vcpu *vcpu);
void __kvm_migrate_apic_timer(struct kvm_vcpu *vcpu);
+void __kvm_migrate_pit_timer(struct kvm_vcpu *vcpu);
+void __kvm_migrate_timers(struct kvm_vcpu *vcpu);
int pit_has_pending_timer(struct kvm_vcpu *vcpu);
int apic_has_pending_timer(struct kvm_vcpu *vcpu);
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index baf9607..238e8f3 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -690,7 +690,7 @@ static void svm_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
delta = vcpu->arch.host_tsc - tsc_this;
svm->vmcb->control.tsc_offset += delta;
vcpu->cpu = cpu;
- kvm_migrate_apic_timer(vcpu);
+ kvm_migrate_timers(vcpu);
}
for (i = 0; i < NR_HOST_SAVE_USER_MSRS; i++)
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index aaa99ed..22f6c19 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -616,7 +616,7 @@ static void vmx_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
if (vcpu->cpu != cpu) {
vcpu_clear(vmx);
- kvm_migrate_apic_timer(vcpu);
+ kvm_migrate_timers(vcpu);
vpid_sync_vcpu_all(vmx);
local_irq_disable();
list_add(&vmx->local_vcpus_link,
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 314f470..07bf9a7 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2744,7 +2744,7 @@ again:
if (vcpu->requests) {
if (test_and_clear_bit(KVM_REQ_MIGRATE_TIMER, &vcpu->requests))
- __kvm_migrate_apic_timer(vcpu);
+ __kvm_migrate_timers(vcpu);
if (test_and_clear_bit(KVM_REQ_REPORT_TPR_ACCESS,
&vcpu->requests)) {
kvm_run->exit_reason = KVM_EXIT_TPR_ACCESS;
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index d622309..4741063 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -294,7 +294,7 @@ static inline gpa_t gfn_to_gpa(gfn_t gfn)
return (gpa_t)gfn << PAGE_SHIFT;
}
-static inline void kvm_migrate_apic_timer(struct kvm_vcpu *vcpu)
+static inline void kvm_migrate_timers(struct kvm_vcpu *vcpu)
{
set_bit(KVM_REQ_MIGRATE_TIMER, &vcpu->requests);
}
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: KVM: migrate PIT timer
2008-05-27 15:10 ` Marcelo Tosatti
@ 2008-05-28 6:34 ` Avi Kivity
0 siblings, 0 replies; 4+ messages in thread
From: Avi Kivity @ 2008-05-28 6:34 UTC (permalink / raw)
To: Marcelo Tosatti; +Cc: kvm-devel
Marcelo Tosatti wrote:
> On Tue, May 27, 2008 at 04:17:30PM +0300, Avi Kivity wrote:
>
>> Instead of having two timer bits, how about keeping just on
>> MIGRATE_TIMER bit which migrates both, and having
>> __kvm_migrate_pit_timer() return if not on vcpu 0?
>>
>> This can result in shorter code, and in less proliferation of the "pit
>> is bound to vcpu 0" logic.
>>
>
> Sure.
>
> Migrate the PIT timer to the physical CPU which vcpu0 is scheduled on,
> similarly to what is done for the LAPIC timers, otherwise PIT interrupts
> will be delayed until an unrelated event causes an exit.
>
>
Thanks, applied.
--
Do not meddle in the internals of kernels, for they are subtle and quick to panic.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2008-05-28 6:34 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-05-25 5:29 KVM: migrate PIT timer Marcelo Tosatti
2008-05-27 13:17 ` Avi Kivity
2008-05-27 15:10 ` Marcelo Tosatti
2008-05-28 6:34 ` Avi Kivity
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox