[patch 0/2] PIT: optionally disable interrupt reinjection

[patch 0/2] PIT: optionally disable interrupt reinjection

[Marcelo Tosatti]

-- 

[patch 1/2] KVM: PIT: fix i8254 pending count read

[Marcelo Tosatti]

[-- Attachment #1: i8254-fix-count --]
[-- Type: text/plain, Size: 669 bytes --]

count_load_time assignment is bogus: its supposed to contain what it
means, not the expiration time.

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>

Index: kvm/arch/x86/kvm/i8254.c
===================================================================
--- kvm.orig/arch/x86/kvm/i8254.c
+++ 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);
 }

-- 

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

[Marcelo Tosatti]

[-- Attachment #1: i8254-disable-reinject --]
[-- Type: text/plain, Size: 2576 bytes --]

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 <mtosatti@redhat.com>

Index: kvm/arch/x86/kvm/i8254.c
===================================================================
--- kvm.orig/arch/x86/kvm/i8254.c
+++ 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);
 
Index: kvm/arch/x86/kvm/i8254.h
===================================================================
--- kvm.orig/arch/x86/kvm/i8254.h
+++ 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 {
Index: kvm/arch/x86/kvm/x86.c
===================================================================
--- kvm.orig/arch/x86/kvm/x86.c
+++ 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:
 		;
 	}
Index: kvm/include/linux/kvm.h
===================================================================
--- kvm.orig/include/linux/kvm.h
+++ 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

-- 

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

[Avi Kivity]

Marcelo Tosatti wrote:
> 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 <mtosatti@redhat.com>
>
> Index: kvm/arch/x86/kvm/i8254.c
> ===================================================================
> --- kvm.orig/arch/x86/kvm/i8254.c
> +++ 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);
> +
>   

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

> --- kvm.orig/arch/x86/kvm/i8254.h
> +++ 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;
>  };
>   

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

> @@ -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?

-- 
error compiling committee.c: too many arguments to function

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

[Marcelo Tosatti]

On Tue, Dec 30, 2008 at 12:08:01PM +0200, Avi Kivity wrote:
> Marcelo Tosatti wrote:
>> 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 <mtosatti@redhat.com>
>>
>> Index: kvm/arch/x86/kvm/i8254.c
>> ===================================================================
>> --- kvm.orig/arch/x86/kvm/i8254.c
>> +++ 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);
>> +
>>   
>
> 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.

>> --- kvm.orig/arch/x86/kvm/i8254.h
>> +++ 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;
>>  };
>>   
>
> Negative logic = !good.  Call it reinject and init to true.

Sure.

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

[Avi Kivity]

Marcelo Tosatti wrote:
> On Tue, Dec 30, 2008 at 12:08:01PM +0200, Avi Kivity wrote:
>   
>> Marcelo Tosatti wrote:
>>     
>>> 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 <mtosatti@redhat.com>
>>>
>>> Index: kvm/arch/x86/kvm/i8254.c
>>> ===================================================================
>>> --- kvm.orig/arch/x86/kvm/i8254.c
>>> +++ 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);
>>> +
>>>   
>>>       
>> 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.
>   

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

-- 
error compiling committee.c: too many arguments to function

[patch 0/2] PIT: optionally disable interrupt reinjection (v2)

[Marcelo Tosatti]

Addressing comments.


-- 

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

[Marcelo Tosatti]

[-- Attachment #1: i8254-disable-reinject --]
[-- Type: text/plain, Size: 3516 bytes --]

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 <mtosatti@redhat.com>

Index: kvm/arch/x86/kvm/i8254.c
===================================================================
--- kvm.orig/arch/x86/kvm/i8254.c
+++ 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->reinject)
+		atomic_set(&pt->pending, 1);
+
 	if (vcpu0 && waitqueue_active(&vcpu0->wq))
 		wake_up_interruptible(&vcpu0->wq);
 
@@ -580,6 +583,7 @@ struct kvm_pit *kvm_create_pit(struct kv
 	pit_state->irq_ack_notifier.gsi = 0;
 	pit_state->irq_ack_notifier.irq_acked = kvm_pit_ack_irq;
 	kvm_register_irq_ack_notifier(kvm, &pit_state->irq_ack_notifier);
+	pit_state->pit_timer.reinject = true;
 	mutex_unlock(&pit->pit_state.lock);
 
 	kvm_pit_reset(pit);
Index: kvm/arch/x86/kvm/i8254.h
===================================================================
--- kvm.orig/arch/x86/kvm/i8254.h
+++ kvm/arch/x86/kvm/i8254.h
@@ -9,6 +9,7 @@ struct kvm_kpit_timer {
 	s64 period; /* unit: ns */
 	s64 scheduled;
 	atomic_t pending;
+	bool reinject;
 };
 
 struct kvm_kpit_channel_state {
Index: kvm/arch/x86/kvm/x86.c
===================================================================
--- kvm.orig/arch/x86/kvm/x86.c
+++ 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_REINJECT_CONTROL:
 		r = 1;
 		break;
 	case KVM_CAP_COALESCED_MMIO:
@@ -1723,6 +1724,13 @@ static int kvm_vm_ioctl_set_pit(struct k
 	return r;
 }
 
+static int kvm_vm_ioctl_pit_reinject(struct kvm *kvm,
+				     struct kvm_pit_reinject_control *control)
+{
+	kvm->arch.vpit->pit_state.pit_timer.reinject = control->reinject;
+	return 0;
+}
+
 /*
  * Get (and clear) the dirty memory log for a memory slot.
  */
@@ -1920,6 +1928,20 @@ long kvm_arch_vm_ioctl(struct file *filp
 		r = 0;
 		break;
 	}
+	case KVM_PIT_REINJECT_CONTROL: {
+		struct kvm_pit_reinject_control control;
+		r =  -EFAULT;
+		if (copy_from_user(&control, argp, sizeof(control)))
+			goto out;
+		r = -ENXIO;
+		if (!kvm->arch.vpit)
+			goto out;
+		r = kvm_vm_ioctl_pit_reinject(kvm, &control);
+		if (r)
+			goto out;
+		r = 0;
+		break;
+	}
 	default:
 		;
 	}
Index: kvm/include/linux/kvm.h
===================================================================
--- kvm.orig/include/linux/kvm.h
+++ 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_REINJECT_CONTROL 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_REINJECT_CONTROL  _IO(KVMIO, 0x71)
 
 /*
  * ioctls for vcpu fds
Index: kvm/arch/x86/include/asm/kvm.h
===================================================================
--- kvm.orig/arch/x86/include/asm/kvm.h
+++ kvm/arch/x86/include/asm/kvm.h
@@ -226,4 +226,8 @@ struct kvm_guest_debug_arch {
 struct kvm_pit_state {
 	struct kvm_pit_channel_state channels[3];
 };
+
+struct kvm_pit_reinject_control {
+	__u8 reinject;
+};
 #endif /* _ASM_X86_KVM_H */

--