Index: kvm/arch/x86/kvm/i8254.c
===================================================================
@@ -207,7 +207,7 @@ static int __pit_timer_fn(struct kvm_kpi
 	hrtimer_add_expires_ns(&pt->timer, pt->period);
 	pt->scheduled = hrtimer_get_expires_ns(&pt->timer);
 	if (pt->period)
-		ps->channels[0].count_load_time = hrtimer_get_expires(&pt->timer);
+		ps->channels[0].count_load_time = ktime_get();
 
 	return (pt->period == 0 ? 0 : 1);
 }

Marcelo Tosatti

author

2008-12-29 17:46 UTC #2 lore raw

PATCH 2/2 KVM: PIT: provide an option to disable interrupt reinjection

In reply to Marcelo Tosatti

Certain clocks (such as TSC) in older 2.6 guests overaccount for lost ticks, causing severe time drift. Interrupt reinjection magnifies the problem.

Provide an option to disable it.

Signed-off-by Marcelo Tosatti <[email protected]>

arch/x86/kvm/i8254.c +3 -0

Index: kvm/arch/x86/kvm/i8254.c
===================================================================
@@ -201,6 +201,9 @@ static int __pit_timer_fn(struct kvm_kpi
 	if (!atomic_inc_and_test(&pt->pending))
 		set_bit(KVM_REQ_PENDING_TIMER, &vcpu0->requests);
 
+	if (pt->no_reinject)
+		atomic_set(&pt->pending, 1);
+
 	if (vcpu0 && waitqueue_active(&vcpu0->wq))
 		wake_up_interruptible(&vcpu0->wq);

arch/x86/kvm/i8254.h +1 -0

Index: kvm/arch/x86/kvm/i8254.h
===================================================================
@@ -9,6 +9,7 @@ struct kvm_kpit_timer {
 	s64 period; /* unit: ns */
 	s64 scheduled;
 	atomic_t pending;
+	bool no_reinject;
 };
 
 struct kvm_kpit_channel_state {

arch/x86/kvm/x86.c +17 -0

Index: kvm/arch/x86/kvm/x86.c
===================================================================
@@ -991,6 +991,7 @@ int kvm_dev_ioctl_check_extension(long e
 	case KVM_CAP_NOP_IO_DELAY:
 	case KVM_CAP_MP_STATE:
 	case KVM_CAP_SYNC_MMU:
+	case KVM_CAP_PIT_NO_REINJECT:
 		r = 1;
 		break;
 	case KVM_CAP_COALESCED_MMIO:

@@ -1723,6 +1724,12 @@ static int kvm_vm_ioctl_set_pit(struct k
 	return r;
 }
 
+static int kvm_vm_ioctl_no_reinject(struct kvm *kvm)
+{
+	kvm->arch.vpit->pit_state.pit_timer.no_reinject = true;
+	return 0;
+}
+
 /*
  * Get (and clear) the dirty memory log for a memory slot.
  */

@@ -1920,6 +1927,16 @@ long kvm_arch_vm_ioctl(struct file *filp
 		r = 0;
 		break;
 	}
+	case KVM_PIT_NO_REINJECT: {
+		r = -ENXIO;
+		if (!kvm->arch.vpit)
+			goto out;
+		r = kvm_vm_ioctl_no_reinject(kvm);
+		if (r)
+			goto out;
+		r = 0;
+		break;
+	}
 	default:
 		;
 	}

include/linux/kvm.h +4 -0

Index: kvm/include/linux/kvm.h
===================================================================
@@ -396,6 +396,9 @@ struct kvm_trace_rec {
 #if defined(CONFIG_X86)
 #define KVM_CAP_SET_GUEST_DEBUG 23
 #endif
+#if defined(CONFIG_X86)
+#define KVM_CAP_PIT_NO_REINJECT 24
+#endif
 
 /*
  * ioctls for VM fds

@@ -429,6 +432,7 @@ struct kvm_trace_rec {
 				   struct kvm_assigned_pci_dev)
 #define KVM_ASSIGN_IRQ _IOR(KVMIO, 0x70, \
 			    struct kvm_assigned_irq)
+#define KVM_PIT_NO_REINJECT	_IO(KVMIO, 0x71)
 
 /*
  * ioctls for vcpu fds

Avi Kivity

2008-12-30 10:08 UTC #3 lore raw

Re: [patch 2/2] KVM: PIT: provide an option to disable interrupt reinjection

Quoted 13 lines · Marcelo Tosatti

Certain clocks (such as TSC) in older 2.6 guests overaccount for lost ticks, causing severe time drift. Interrupt reinjection magnifies the problem.
Provide an option to disable it.
Signed-off-by Marcelo Tosatti <[email protected]>
Index: kvm/arch/x86/kvm/i8254.c
===================================================================

arch/x86/kvm/i8254.c static int __pit_timer_fn(struct kvm_kpi · line 207

Quoted 7 lines

arch/x86/kvm/i8254.c +3 -0

@@ -201,6 +201,9 @@ static int __pit_timer_fn(struct kvm_kpi
 	if (!atomic_inc_and_test(&pt->pending))
 		set_bit(KVM_REQ_PENDING_TIMER, &vcpu0->requests);
 
+	if (pt->no_reinject)
+		atomic_set(&pt->pending, 1);
+

What about moving this to the place where we atomic_inc()? Any reason not to?

arch/x86/kvm/i8254.h struct kvm_kpit_timer { · line 14

Quoted 9 lines

arch/x86/kvm/i8254.h +1 -0

@@ -9,6 +9,7 @@ struct kvm_kpit_timer {
 	s64 period; /* unit: ns */
 	s64 scheduled;
 	atomic_t pending;
+	bool no_reinject;
 };

Negative logic = !good. Call it reinject and init to true.

arch/x86/kvm/i8254.h int kvm_dev_ioctl_check_extension(long e · line 995

Quoted 6 lines

arch/x86/kvm/i8254.h +1 -0

@@ -991,6 +991,7 @@ int kvm_dev_ioctl_check_extension(long e
 	case KVM_CAP_NOP_IO_DELAY:
 	case KVM_CAP_MP_STATE:
 	case KVM_CAP_SYNC_MMU:
+	case KVM_CAP_PIT_NO_REINJECT:

Negative logic. PIT_REINJECT_CONTROL?

Signature · 1 line

error compiling committee.c: too many arguments to function

Marcelo Tosatti

author

2008-12-30 16:13 UTC #4 lore raw

Re: [patch 2/2] KVM: PIT: provide an option to disable interrupt reinjection

Quoted 9 lines · Avi Kivity

Quoted 8 lines · Marcelo Tosatti
Certain clocks (such as TSC) in older 2.6 guests overaccount for lost ticks, causing severe time drift. Interrupt reinjection magnifies the problem.
Provide an option to disable it.
Signed-off-by Marcelo Tosatti <[email protected]>

Quoted 14 lines

arch/x86/kvm/i8254.c static int __pit_timer_fn(struct kvm_kpi · line 206
Quoted 11 lines
Index: kvm/arch/x86/kvm/i8254.c
===================================================================
arch/x86/kvm/i8254.c +2 -0
@@ -201,6 +201,9 @@ static int __pit_timer_fn(struct kvm_kpi
 	if (!atomic_inc_and_test(&pt->pending))
 		set_bit(KVM_REQ_PENDING_TIMER, &vcpu0->requests);
 +	if (pt->no_reinject)
+		atomic_set(&pt->pending, 1);
+
 
What about moving this to the place where we atomic_inc()? Any reason not to?

The atomic_inc_and_test is right above, and we want to set KVM_REQ_PENDING_TIMER on 0->1 transitions, so better keep it separate. BTW the KVM_REQ_PENDING_TIMER optimization is broken (atomic_inc_and_test returns true if the _new_ value is zero) but thats another issue.

Can't see the improvement you're suggesting.

Quoted 11 lines

arch/x86/kvm/i8254.h struct kvm_kpit_timer { · line 14
Quoted 9 lines
arch/x86/kvm/i8254.h +1 -0
@@ -9,6 +9,7 @@ struct kvm_kpit_timer {
 	s64 period; /* unit: ns */
 	s64 scheduled;
 	atomic_t pending;
+	bool no_reinject;
 };
 
Negative logic = !good. Call it reinject and init to true.

Sure.

Avi Kivity

2008-12-30 16:35 UTC #5 lore raw

Re: [patch 2/2] KVM: PIT: provide an option to disable interrupt reinjection

Quoted 27 lines · Marcelo Tosatti

Quoted 10 lines · Avi Kivity
Quoted 8 lines · Marcelo Tosatti
Certain clocks (such as TSC) in older 2.6 guests overaccount for lost ticks, causing severe time drift. Interrupt reinjection magnifies the problem.
Provide an option to disable it.
Signed-off-by Marcelo Tosatti <[email protected]>
Quoted 14 lines
arch/x86/kvm/i8254.c static int __pit_timer_fn(struct kvm_kpi · line 207
Quoted 12 lines
Index: kvm/arch/x86/kvm/i8254.c
===================================================================
arch/x86/kvm/i8254.c +2 -0
@@ -201,6 +201,9 @@ static int __pit_timer_fn(struct kvm_kpi
 	if (!atomic_inc_and_test(&pt->pending))
 		set_bit(KVM_REQ_PENDING_TIMER, &vcpu0->requests);
 +	if (pt->no_reinject)
+		atomic_set(&pt->pending, 1);
+
 
 
What about moving this to the place where we atomic_inc()? Any reason not to?

Quoted 8 lines

The atomic_inc_and_test is right above, and we want to set KVM_REQ_PENDING_TIMER on 0->1 transitions, so better keep it separate. BTW the KVM_REQ_PENDING_TIMER optimization is broken (atomic_inc_and_test returns true if the _new_ value is zero) but thats another issue.
Can't see the improvement you're suggesting.

Right, scratch that. Didn't read closely enough.

Signature · 1 line

error compiling committee.c: too many arguments to function