add back pending timer irqs for kernel APIC timer

add back pending timer irqs for kernel APIC timer

[Dong, Eddie]

[-- Attachment #1: Type: text/plain, Size: 8214 bytes --]

    Add back pending irqs for apic timer to get precise guest
    APIC timer interrupt.
    
    Signed-off-by: Yaozu (Eddie) Dong <Eddie.Dong-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>


diff --git a/drivers/kvm/irq.h b/drivers/kvm/irq.h
index ed6d20a..8867c82 100644
--- a/drivers/kvm/irq.h
+++ b/drivers/kvm/irq.h
@@ -110,7 +110,7 @@ struct kvm_lapic {
 	unsigned long base_address;
 	struct kvm_io_device dev;
 	struct {
-		unsigned long pending;
+		atomic_t pending;
 		s64 period;	/* unit: ns */
 		u32 divide_count;
 		ktime_t last_update;
@@ -153,5 +153,7 @@ int kvm_apic_set_irq(struct kvm_lapic *apic, u8 vec,
u8 trig);
 void kvm_apic_post_state_restore(struct kvm_vcpu *vcpu);
 int kvm_ioapic_init(struct kvm *kvm);
 void kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int irq, int level);
+void kvm_pt_intr_post(struct kvm_vcpu *vcpu, int vec);
+void kvm_pt_update_irq(struct kvm_vcpu *vcpu);
 
 #endif
diff --git a/drivers/kvm/lapic.c b/drivers/kvm/lapic.c
index 352b8a7..cb4d3ca 100644
--- a/drivers/kvm/lapic.c
+++ b/drivers/kvm/lapic.c
@@ -30,6 +30,7 @@
 #include <asm/page.h>
 #include <asm/current.h>
 #include <asm/apicdef.h>
+#include <asm/atomic.h>
 #include "irq.h"
 
 #define PRId64 "d"
@@ -300,6 +301,7 @@ static int __apic_accept_irq(struct kvm_lapic *apic,
int delivery_mode,
 			     int vector, int level, int trig_mode)
 {
 	int result = 0;
+	int orig_irr;
 
 	switch (delivery_mode) {
 	case APIC_DM_FIXED:
@@ -308,7 +310,8 @@ static int __apic_accept_irq(struct kvm_lapic *apic,
int delivery_mode,
 		if (unlikely(!apic_enabled(apic)))
 			break;
 
-		if (apic_test_and_set_irr(vector, apic) && trig_mode) {
+		orig_irr = apic_test_and_set_irr(vector, apic);
+		if (orig_irr && trig_mode) {
 			apic_debug("level trig mode repeatedly for
vector %d",
 				   vector);
 			break;
@@ -322,7 +325,7 @@ static int __apic_accept_irq(struct kvm_lapic *apic,
int delivery_mode,
 
 		kvm_vcpu_kick(apic->vcpu);
 
-		result = 1;
+		result = (orig_irr == 0);
 		break;
 
 	case APIC_DM_REMRD:
@@ -352,18 +355,6 @@ static int __apic_accept_irq(struct kvm_lapic
*apic, int delivery_mode,
 	return result;
 }
 
-static inline int apic_accept_irq(struct kvm_lapic *apic, int
delivery_mode,
-				  int vector, int level, int trig_mode)
-{
-	int result = 0;
-
-	spin_lock_bh(&apic->lock);
-	result = __apic_accept_irq(apic, delivery_mode,
-				   vector, level, trig_mode);
-	spin_unlock_bh(&apic->lock);
-	return result;
-}
-
 struct kvm_lapic *kvm_apic_round_robin(struct kvm *kvm, u8 vector,
 				       unsigned long bitmap)
 {
@@ -595,6 +586,7 @@ static void apic_mmio_write(struct kvm_io_device
*this,
 				apic_set_reg(apic, APIC_LVTT + 0x10 * i,
 					     lvt_val | APIC_LVT_MASKED);
 			}
+			atomic_set(&apic->timer.pending, 0);
 
 		}
 		break;
@@ -632,12 +624,14 @@ static void apic_mmio_write(struct kvm_io_device
*this,
 			apic->timer.last_update = now;
 			apic->timer.period =
 			    APIC_BUS_CYCLE_NS * apic->timer.divide_count
* val;
+			apic_debug("LAPIC period = %lld\n",
apic->timer.period);
 
 			/* Make sure the lock ordering is coherent */
 			spin_unlock_bh(&apic->lock);
 			hrtimer_cancel(&apic->timer.dev);
+			atomic_set(&apic->timer.pending, 0);
 			hrtimer_start(&apic->timer.dev,
-				      ktime_add_ns(now, offset),
+				      ktime_add_ns(now,
apic->timer.period),
 				      HRTIMER_MODE_ABS);
 
 			apic_debug("%s: bus cycle is %" PRId64 "ns, now
0x%016"
@@ -816,7 +810,7 @@ static void lapic_reset(struct kvm_vcpu *vcpu)
 		apic_set_reg(apic, APIC_TMR + 0x10 * i, 0);
 	}
 	apic->timer.divide_count = 0;
-	apic->timer.pending = 0;
+	atomic_set(&apic->timer.pending, 0);
 	if (vcpu->vcpu_id == 0)
 		vcpu->apic_base |= MSR_IA32_APICBASE_BSP;
 	apic_update_ppr(apic);
@@ -857,38 +851,30 @@ EXPORT_SYMBOL_GPL(kvm_lapic_get_regs);
  * timer interface
 
*----------------------------------------------------------------------
  */
+
+/* TODO: make sure __apic_timer_fn runs in current pCPU */
 static int __apic_timer_fn(struct kvm_lapic *apic)
 {
-	u32 vector;
 	int result = 0;
 
-	if (unlikely(!apic_enabled(apic) ||
-		     !apic_lvt_enabled(apic, APIC_LVTT))) {
-		apic_debug("%s: time interrupt although apic is down\n",
-			   __FUNCTION__);
-		return 0;
-	}
-
-	vector = apic_lvt_vector(apic, APIC_LVTT);
-	apic->timer.last_update = apic->timer.dev.expires;
-	apic->timer.pending++;
-	__apic_accept_irq(apic, APIC_DM_FIXED, vector, 1, 0);
-
+	atomic_inc (&apic->timer.pending);
 	if (apic_lvtt_period(apic)) {
-		u32 offset;
-		u32 tmict = apic_get_reg(apic, APIC_TMICT);
-
-		offset = APIC_BUS_CYCLE_NS * apic->timer.divide_count *
tmict;
-
 		result = 1;
 		apic->timer.dev.expires = ktime_add_ns(
 					apic->timer.dev.expires,
 					apic->timer.period);
 	}
-
 	return result;
 }
 
+static int inject_apic_timer_irq(struct kvm_lapic *apic)
+{
+	int vector;
+
+	vector = apic_lvt_vector(apic, APIC_LVTT);
+	return __apic_accept_irq(apic, APIC_DM_FIXED, vector, 1, 0);
+}
+
 static enum hrtimer_restart apic_timer_fn(struct hrtimer *data)
 {
 	struct kvm_lapic *apic;
@@ -963,6 +949,28 @@ int kvm_apic_has_interrupt(struct kvm_vcpu *vcpu)
 	return highest_irr;
 }
 
+void kvm_pt_update_irq(struct kvm_vcpu *vcpu)
+{
+	struct kvm_lapic *apic = vcpu->apic;
+
+	if (apic && atomic_read(&apic->timer.pending) > 0) {
+		if (inject_apic_timer_irq(apic))
+			atomic_dec(&apic->timer.pending);
+	}
+}
+EXPORT_SYMBOL_GPL(kvm_pt_update_irq);
+
+void kvm_pt_intr_post(struct kvm_vcpu *vcpu, int vec)
+{
+	struct kvm_lapic *apic = vcpu->apic;
+
+	if (apic && apic_lvt_vector(apic, APIC_LVTT) == vec)
+		apic->timer.last_update = ktime_add_ns(
+				apic->timer.last_update,
+				apic->timer.period);
+}
+EXPORT_SYMBOL_GPL(kvm_pt_intr_post);
+
 int kvm_get_apic_interrupt(struct kvm_vcpu *vcpu)
 {
 	int vector = kvm_apic_has_interrupt(vcpu);
@@ -992,7 +1000,7 @@ void kvm_apic_post_state_restore(struct kvm_vcpu
*vcpu)
  	/* TODO: following code can be in a common API */
 	spin_lock_bh(&apic->lock);
  	hrtimer_cancel(&apic->timer.dev);
-	apic->timer.pending = 0;
+	atomic_set(&apic->timer.pending, 0);
 	val = apic_get_reg(apic, APIC_TDCR);
 	tmp = ((val & 0x3) | ((val & 0x8) >> 1)) + 1;
 	apic->timer.divide_count = 0x1 << (tmp & 0x7);
diff --git a/drivers/kvm/svm.c b/drivers/kvm/svm.c
index 96db35a..5e45e12 100644
--- a/drivers/kvm/svm.c
+++ b/drivers/kvm/svm.c
@@ -1462,6 +1462,7 @@ static void svm_intr_assist(struct vcpu_svm *svm)
 	struct vmcb *vmcb = svm->vmcb;
 	int intr_vector = -1;
 
+	kvm_pt_update_irq(vcpu);
 	if ((vmcb->control.exit_int_info & SVM_EVTINJ_VALID) &&
 	    ((vmcb->control.exit_int_info & SVM_EVTINJ_TYPE_MASK) == 0))
{
 		intr_vector = vmcb->control.exit_int_info &
@@ -1488,6 +1489,7 @@ static void svm_intr_assist(struct vcpu_svm *svm)
 	/* Okay, we can deliver the interrupt: grab it and update PIC
state. */
 	intr_vector = kvm_cpu_get_interrupt(&svm->vcpu);
 	svm_inject_irq(svm, intr_vector);
+	kvm_pt_intr_post(vcpu, vector);
 }
 
 static void kvm_reput_irq(struct vcpu_svm *svm)
diff --git a/drivers/kvm/vmx.c b/drivers/kvm/vmx.c
index 019197b..499bb45 100644
--- a/drivers/kvm/vmx.c
+++ b/drivers/kvm/vmx.c
@@ -2144,7 +2144,9 @@ static void vmx_intr_assist(struct kvm_vcpu *vcpu)
 {
 	u32 idtv_info_field, intr_info_field;
 	int has_ext_irq, interrupt_window_open;
+	int vector;
 
+	kvm_pt_update_irq(vcpu);
 	has_ext_irq = kvm_cpu_has_interrupt(vcpu);
 	intr_info_field = vmcs_read32(VM_ENTRY_INTR_INFO_FIELD);
 	idtv_info_field = vmcs_read32(IDT_VECTORING_INFO_FIELD);
@@ -2174,9 +2176,11 @@ static void vmx_intr_assist(struct kvm_vcpu
*vcpu)
 	interrupt_window_open =
 		((vmcs_readl(GUEST_RFLAGS) & X86_EFLAGS_IF) &&
 		 (vmcs_read32(GUEST_INTERRUPTIBILITY_INFO) & 3) == 0);
-	if (interrupt_window_open)
-		vmx_inject_irq(vcpu, kvm_cpu_get_interrupt(vcpu));
-	else
+	if (interrupt_window_open) {
+		vector = kvm_cpu_get_interrupt(vcpu);
+		vmx_inject_irq(vcpu, vector);
+		kvm_pt_intr_post(vcpu, vector);
+	} else
 		enable_irq_window(vcpu);
 }
 

[-- Attachment #2: apic-timer-pending.patch --]
[-- Type: application/octet-stream, Size: 8033 bytes --]

commit 94b1996ac56f55e4fb3773798fb30784dc03f451
Author: root <root@vt32-pae.(none)>
Date:   Fri Aug 10 23:33:31 2007 +0800

    Add back pending irqs for apic timer to get precise guest
    APIC timer interrupt.
    
    Signed-off-by: Yaozu (Eddie) Dong <Eddie.Dong@intel.com>

diff --git a/drivers/kvm/irq.h b/drivers/kvm/irq.h
index ed6d20a..8867c82 100644
--- a/drivers/kvm/irq.h
+++ b/drivers/kvm/irq.h
@@ -110,7 +110,7 @@ struct kvm_lapic {
 	unsigned long base_address;
 	struct kvm_io_device dev;
 	struct {
-		unsigned long pending;
+		atomic_t pending;
 		s64 period;	/* unit: ns */
 		u32 divide_count;
 		ktime_t last_update;
@@ -153,5 +153,7 @@ int kvm_apic_set_irq(struct kvm_lapic *apic, u8 vec, u8 trig);
 void kvm_apic_post_state_restore(struct kvm_vcpu *vcpu);
 int kvm_ioapic_init(struct kvm *kvm);
 void kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int irq, int level);
+void kvm_pt_intr_post(struct kvm_vcpu *vcpu, int vec);
+void kvm_pt_update_irq(struct kvm_vcpu *vcpu);
 
 #endif
diff --git a/drivers/kvm/lapic.c b/drivers/kvm/lapic.c
index 352b8a7..cb4d3ca 100644
--- a/drivers/kvm/lapic.c
+++ b/drivers/kvm/lapic.c
@@ -30,6 +30,7 @@
 #include <asm/page.h>
 #include <asm/current.h>
 #include <asm/apicdef.h>
+#include <asm/atomic.h>
 #include "irq.h"
 
 #define PRId64 "d"
@@ -300,6 +301,7 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
 			     int vector, int level, int trig_mode)
 {
 	int result = 0;
+	int orig_irr;
 
 	switch (delivery_mode) {
 	case APIC_DM_FIXED:
@@ -308,7 +310,8 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
 		if (unlikely(!apic_enabled(apic)))
 			break;
 
-		if (apic_test_and_set_irr(vector, apic) && trig_mode) {
+		orig_irr = apic_test_and_set_irr(vector, apic);
+		if (orig_irr && trig_mode) {
 			apic_debug("level trig mode repeatedly for vector %d",
 				   vector);
 			break;
@@ -322,7 +325,7 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
 
 		kvm_vcpu_kick(apic->vcpu);
 
-		result = 1;
+		result = (orig_irr == 0);
 		break;
 
 	case APIC_DM_REMRD:
@@ -352,18 +355,6 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
 	return result;
 }
 
-static inline int apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
-				  int vector, int level, int trig_mode)
-{
-	int result = 0;
-
-	spin_lock_bh(&apic->lock);
-	result = __apic_accept_irq(apic, delivery_mode,
-				   vector, level, trig_mode);
-	spin_unlock_bh(&apic->lock);
-	return result;
-}
-
 struct kvm_lapic *kvm_apic_round_robin(struct kvm *kvm, u8 vector,
 				       unsigned long bitmap)
 {
@@ -595,6 +586,7 @@ static void apic_mmio_write(struct kvm_io_device *this,
 				apic_set_reg(apic, APIC_LVTT + 0x10 * i,
 					     lvt_val | APIC_LVT_MASKED);
 			}
+			atomic_set(&apic->timer.pending, 0);
 
 		}
 		break;
@@ -632,12 +624,14 @@ static void apic_mmio_write(struct kvm_io_device *this,
 			apic->timer.last_update = now;
 			apic->timer.period =
 			    APIC_BUS_CYCLE_NS * apic->timer.divide_count * val;
+			apic_debug("LAPIC period = %lld\n", apic->timer.period);
 
 			/* Make sure the lock ordering is coherent */
 			spin_unlock_bh(&apic->lock);
 			hrtimer_cancel(&apic->timer.dev);
+			atomic_set(&apic->timer.pending, 0);
 			hrtimer_start(&apic->timer.dev,
-				      ktime_add_ns(now, offset),
+				      ktime_add_ns(now, apic->timer.period),
 				      HRTIMER_MODE_ABS);
 
 			apic_debug("%s: bus cycle is %" PRId64 "ns, now 0x%016"
@@ -816,7 +810,7 @@ static void lapic_reset(struct kvm_vcpu *vcpu)
 		apic_set_reg(apic, APIC_TMR + 0x10 * i, 0);
 	}
 	apic->timer.divide_count = 0;
-	apic->timer.pending = 0;
+	atomic_set(&apic->timer.pending, 0);
 	if (vcpu->vcpu_id == 0)
 		vcpu->apic_base |= MSR_IA32_APICBASE_BSP;
 	apic_update_ppr(apic);
@@ -857,38 +851,30 @@ EXPORT_SYMBOL_GPL(kvm_lapic_get_regs);
  * timer interface
  *----------------------------------------------------------------------
  */
+
+/* TODO: make sure __apic_timer_fn runs in current pCPU */
 static int __apic_timer_fn(struct kvm_lapic *apic)
 {
-	u32 vector;
 	int result = 0;
 
-	if (unlikely(!apic_enabled(apic) ||
-		     !apic_lvt_enabled(apic, APIC_LVTT))) {
-		apic_debug("%s: time interrupt although apic is down\n",
-			   __FUNCTION__);
-		return 0;
-	}
-
-	vector = apic_lvt_vector(apic, APIC_LVTT);
-	apic->timer.last_update = apic->timer.dev.expires;
-	apic->timer.pending++;
-	__apic_accept_irq(apic, APIC_DM_FIXED, vector, 1, 0);
-
+	atomic_inc (&apic->timer.pending);
 	if (apic_lvtt_period(apic)) {
-		u32 offset;
-		u32 tmict = apic_get_reg(apic, APIC_TMICT);
-
-		offset = APIC_BUS_CYCLE_NS * apic->timer.divide_count * tmict;
-
 		result = 1;
 		apic->timer.dev.expires = ktime_add_ns(
 					apic->timer.dev.expires,
 					apic->timer.period);
 	}
-
 	return result;
 }
 
+static int inject_apic_timer_irq(struct kvm_lapic *apic)
+{
+	int vector;
+
+	vector = apic_lvt_vector(apic, APIC_LVTT);
+	return __apic_accept_irq(apic, APIC_DM_FIXED, vector, 1, 0);
+}
+
 static enum hrtimer_restart apic_timer_fn(struct hrtimer *data)
 {
 	struct kvm_lapic *apic;
@@ -963,6 +949,28 @@ int kvm_apic_has_interrupt(struct kvm_vcpu *vcpu)
 	return highest_irr;
 }
 
+void kvm_pt_update_irq(struct kvm_vcpu *vcpu)
+{
+	struct kvm_lapic *apic = vcpu->apic;
+
+	if (apic && atomic_read(&apic->timer.pending) > 0) {
+		if (inject_apic_timer_irq(apic))
+			atomic_dec(&apic->timer.pending);
+	}
+}
+EXPORT_SYMBOL_GPL(kvm_pt_update_irq);
+
+void kvm_pt_intr_post(struct kvm_vcpu *vcpu, int vec)
+{
+	struct kvm_lapic *apic = vcpu->apic;
+
+	if (apic && apic_lvt_vector(apic, APIC_LVTT) == vec)
+		apic->timer.last_update = ktime_add_ns(
+				apic->timer.last_update,
+				apic->timer.period);
+}
+EXPORT_SYMBOL_GPL(kvm_pt_intr_post);
+
 int kvm_get_apic_interrupt(struct kvm_vcpu *vcpu)
 {
 	int vector = kvm_apic_has_interrupt(vcpu);
@@ -992,7 +1000,7 @@ void kvm_apic_post_state_restore(struct kvm_vcpu *vcpu)
  	/* TODO: following code can be in a common API */
 	spin_lock_bh(&apic->lock);
  	hrtimer_cancel(&apic->timer.dev);
-	apic->timer.pending = 0;
+	atomic_set(&apic->timer.pending, 0);
 	val = apic_get_reg(apic, APIC_TDCR);
 	tmp = ((val & 0x3) | ((val & 0x8) >> 1)) + 1;
 	apic->timer.divide_count = 0x1 << (tmp & 0x7);
diff --git a/drivers/kvm/svm.c b/drivers/kvm/svm.c
index 96db35a..5e45e12 100644
--- a/drivers/kvm/svm.c
+++ b/drivers/kvm/svm.c
@@ -1462,6 +1462,7 @@ static void svm_intr_assist(struct vcpu_svm *svm)
 	struct vmcb *vmcb = svm->vmcb;
 	int intr_vector = -1;
 
+	kvm_pt_update_irq(vcpu);
 	if ((vmcb->control.exit_int_info & SVM_EVTINJ_VALID) &&
 	    ((vmcb->control.exit_int_info & SVM_EVTINJ_TYPE_MASK) == 0)) {
 		intr_vector = vmcb->control.exit_int_info &
@@ -1488,6 +1489,7 @@ static void svm_intr_assist(struct vcpu_svm *svm)
 	/* Okay, we can deliver the interrupt: grab it and update PIC state. */
 	intr_vector = kvm_cpu_get_interrupt(&svm->vcpu);
 	svm_inject_irq(svm, intr_vector);
+	kvm_pt_intr_post(vcpu, vector);
 }
 
 static void kvm_reput_irq(struct vcpu_svm *svm)
diff --git a/drivers/kvm/vmx.c b/drivers/kvm/vmx.c
index 019197b..499bb45 100644
--- a/drivers/kvm/vmx.c
+++ b/drivers/kvm/vmx.c
@@ -2144,7 +2144,9 @@ static void vmx_intr_assist(struct kvm_vcpu *vcpu)
 {
 	u32 idtv_info_field, intr_info_field;
 	int has_ext_irq, interrupt_window_open;
+	int vector;
 
+	kvm_pt_update_irq(vcpu);
 	has_ext_irq = kvm_cpu_has_interrupt(vcpu);
 	intr_info_field = vmcs_read32(VM_ENTRY_INTR_INFO_FIELD);
 	idtv_info_field = vmcs_read32(IDT_VECTORING_INFO_FIELD);
@@ -2174,9 +2176,11 @@ static void vmx_intr_assist(struct kvm_vcpu *vcpu)
 	interrupt_window_open =
 		((vmcs_readl(GUEST_RFLAGS) & X86_EFLAGS_IF) &&
 		 (vmcs_read32(GUEST_INTERRUPTIBILITY_INFO) & 3) == 0);
-	if (interrupt_window_open)
-		vmx_inject_irq(vcpu, kvm_cpu_get_interrupt(vcpu));
-	else
+	if (interrupt_window_open) {
+		vector = kvm_cpu_get_interrupt(vcpu);
+		vmx_inject_irq(vcpu, vector);
+		kvm_pt_intr_post(vcpu, vector);
+	} else
 		enable_irq_window(vcpu);
 }
 

[-- Attachment #3: Type: text/plain, Size: 315 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

[-- Attachment #4: Type: text/plain, Size: 186 bytes --]

_______________________________________________
kvm-devel mailing list
kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/kvm-devel

Re: add back pending timer irqs for kernel APIC timer

[Dong, Eddie]

[-- Attachment #1: Type: text/plain, Size: 8881 bytes --]

kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org wrote:
>    Add back pending irqs for apic timer to get precise guest    APIC
> timer interrupt. 
> 
>    Signed-off-by: Yaozu (Eddie) Dong <Eddie.Dong-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> 
> 
a typo, please use this one.

With above patches, now guest timer is much accurate! sleep 60 
get exactly 60 seconds in host per my rough test.

BTW, AMD platform is not tested.

thx,eddie


diff --git a/drivers/kvm/irq.h b/drivers/kvm/irq.h
index ed6d20a..8867c82 100644
--- a/drivers/kvm/irq.h
+++ b/drivers/kvm/irq.h
@@ -110,7 +110,7 @@ struct kvm_lapic {
 	unsigned long base_address;
 	struct kvm_io_device dev;
 	struct {
-		unsigned long pending;
+		atomic_t pending;
 		s64 period;	/* unit: ns */
 		u32 divide_count;
 		ktime_t last_update;
@@ -153,5 +153,7 @@ int kvm_apic_set_irq(struct kvm_lapic *apic, u8 vec,
u8 trig);
 void kvm_apic_post_state_restore(struct kvm_vcpu *vcpu);
 int kvm_ioapic_init(struct kvm *kvm);
 void kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int irq, int level);
+void kvm_pt_intr_post(struct kvm_vcpu *vcpu, int vec);
+void kvm_pt_update_irq(struct kvm_vcpu *vcpu);
 
 #endif
diff --git a/drivers/kvm/lapic.c b/drivers/kvm/lapic.c
index 352b8a7..cb4d3ca 100644
--- a/drivers/kvm/lapic.c
+++ b/drivers/kvm/lapic.c
@@ -30,6 +30,7 @@
 #include <asm/page.h>
 #include <asm/current.h>
 #include <asm/apicdef.h>
+#include <asm/atomic.h>
 #include "irq.h"
 
 #define PRId64 "d"
@@ -300,6 +301,7 @@ static int __apic_accept_irq(struct kvm_lapic *apic,
int delivery_mode,
 			     int vector, int level, int trig_mode)
 {
 	int result = 0;
+	int orig_irr;
 
 	switch (delivery_mode) {
 	case APIC_DM_FIXED:
@@ -308,7 +310,8 @@ static int __apic_accept_irq(struct kvm_lapic *apic,
int delivery_mode,
 		if (unlikely(!apic_enabled(apic)))
 			break;
 
-		if (apic_test_and_set_irr(vector, apic) && trig_mode) {
+		orig_irr = apic_test_and_set_irr(vector, apic);
+		if (orig_irr && trig_mode) {
 			apic_debug("level trig mode repeatedly for
vector %d",
 				   vector);
 			break;
@@ -322,7 +325,7 @@ static int __apic_accept_irq(struct kvm_lapic *apic,
int delivery_mode,
 
 		kvm_vcpu_kick(apic->vcpu);
 
-		result = 1;
+		result = (orig_irr == 0);
 		break;
 
 	case APIC_DM_REMRD:
@@ -352,18 +355,6 @@ static int __apic_accept_irq(struct kvm_lapic
*apic, int delivery_mode,
 	return result;
 }
 
-static inline int apic_accept_irq(struct kvm_lapic *apic, int
delivery_mode,
-				  int vector, int level, int trig_mode)
-{
-	int result = 0;
-
-	spin_lock_bh(&apic->lock);
-	result = __apic_accept_irq(apic, delivery_mode,
-				   vector, level, trig_mode);
-	spin_unlock_bh(&apic->lock);
-	return result;
-}
-
 struct kvm_lapic *kvm_apic_round_robin(struct kvm *kvm, u8 vector,
 				       unsigned long bitmap)
 {
@@ -595,6 +586,7 @@ static void apic_mmio_write(struct kvm_io_device
*this,
 				apic_set_reg(apic, APIC_LVTT + 0x10 * i,
 					     lvt_val | APIC_LVT_MASKED);
 			}
+			atomic_set(&apic->timer.pending, 0);
 
 		}
 		break;
@@ -632,12 +624,14 @@ static void apic_mmio_write(struct kvm_io_device
*this,
 			apic->timer.last_update = now;
 			apic->timer.period =
 			    APIC_BUS_CYCLE_NS * apic->timer.divide_count
* val;
+			apic_debug("LAPIC period = %lld\n",
apic->timer.period);
 
 			/* Make sure the lock ordering is coherent */
 			spin_unlock_bh(&apic->lock);
 			hrtimer_cancel(&apic->timer.dev);
+			atomic_set(&apic->timer.pending, 0);
 			hrtimer_start(&apic->timer.dev,
-				      ktime_add_ns(now, offset),
+				      ktime_add_ns(now,
apic->timer.period),
 				      HRTIMER_MODE_ABS);
 
 			apic_debug("%s: bus cycle is %" PRId64 "ns, now
0x%016"
@@ -816,7 +810,7 @@ static void lapic_reset(struct kvm_vcpu *vcpu)
 		apic_set_reg(apic, APIC_TMR + 0x10 * i, 0);
 	}
 	apic->timer.divide_count = 0;
-	apic->timer.pending = 0;
+	atomic_set(&apic->timer.pending, 0);
 	if (vcpu->vcpu_id == 0)
 		vcpu->apic_base |= MSR_IA32_APICBASE_BSP;
 	apic_update_ppr(apic);
@@ -857,38 +851,30 @@ EXPORT_SYMBOL_GPL(kvm_lapic_get_regs);
  * timer interface
 
*----------------------------------------------------------------------
  */
+
+/* TODO: make sure __apic_timer_fn runs in current pCPU */
 static int __apic_timer_fn(struct kvm_lapic *apic)
 {
-	u32 vector;
 	int result = 0;
 
-	if (unlikely(!apic_enabled(apic) ||
-		     !apic_lvt_enabled(apic, APIC_LVTT))) {
-		apic_debug("%s: time interrupt although apic is down\n",
-			   __FUNCTION__);
-		return 0;
-	}
-
-	vector = apic_lvt_vector(apic, APIC_LVTT);
-	apic->timer.last_update = apic->timer.dev.expires;
-	apic->timer.pending++;
-	__apic_accept_irq(apic, APIC_DM_FIXED, vector, 1, 0);
-
+	atomic_inc (&apic->timer.pending);
 	if (apic_lvtt_period(apic)) {
-		u32 offset;
-		u32 tmict = apic_get_reg(apic, APIC_TMICT);
-
-		offset = APIC_BUS_CYCLE_NS * apic->timer.divide_count *
tmict;
-
 		result = 1;
 		apic->timer.dev.expires = ktime_add_ns(
 					apic->timer.dev.expires,
 					apic->timer.period);
 	}
-
 	return result;
 }
 
+static int inject_apic_timer_irq(struct kvm_lapic *apic)
+{
+	int vector;
+
+	vector = apic_lvt_vector(apic, APIC_LVTT);
+	return __apic_accept_irq(apic, APIC_DM_FIXED, vector, 1, 0);
+}
+
 static enum hrtimer_restart apic_timer_fn(struct hrtimer *data)
 {
 	struct kvm_lapic *apic;
@@ -963,6 +949,28 @@ int kvm_apic_has_interrupt(struct kvm_vcpu *vcpu)
 	return highest_irr;
 }
 
+void kvm_pt_update_irq(struct kvm_vcpu *vcpu)
+{
+	struct kvm_lapic *apic = vcpu->apic;
+
+	if (apic && atomic_read(&apic->timer.pending) > 0) {
+		if (inject_apic_timer_irq(apic))
+			atomic_dec(&apic->timer.pending);
+	}
+}
+EXPORT_SYMBOL_GPL(kvm_pt_update_irq);
+
+void kvm_pt_intr_post(struct kvm_vcpu *vcpu, int vec)
+{
+	struct kvm_lapic *apic = vcpu->apic;
+
+	if (apic && apic_lvt_vector(apic, APIC_LVTT) == vec)
+		apic->timer.last_update = ktime_add_ns(
+				apic->timer.last_update,
+				apic->timer.period);
+}
+EXPORT_SYMBOL_GPL(kvm_pt_intr_post);
+
 int kvm_get_apic_interrupt(struct kvm_vcpu *vcpu)
 {
 	int vector = kvm_apic_has_interrupt(vcpu);
@@ -992,7 +1000,7 @@ void kvm_apic_post_state_restore(struct kvm_vcpu
*vcpu)
  	/* TODO: following code can be in a common API */
 	spin_lock_bh(&apic->lock);
  	hrtimer_cancel(&apic->timer.dev);
-	apic->timer.pending = 0;
+	atomic_set(&apic->timer.pending, 0);
 	val = apic_get_reg(apic, APIC_TDCR);
 	tmp = ((val & 0x3) | ((val & 0x8) >> 1)) + 1;
 	apic->timer.divide_count = 0x1 << (tmp & 0x7);
diff --git a/drivers/kvm/svm.c b/drivers/kvm/svm.c
index 96db35a..ec172a1 100644
--- a/drivers/kvm/svm.c
+++ b/drivers/kvm/svm.c
@@ -1461,7 +1461,9 @@ static void svm_intr_assist(struct vcpu_svm *svm)
 {
 	struct vmcb *vmcb = svm->vmcb;
 	int intr_vector = -1;
+	struct kvm_vcpu *vcpu = &svm->vcpu;
 
+	kvm_pt_update_irq(vcpu);
 	if ((vmcb->control.exit_int_info & SVM_EVTINJ_VALID) &&
 	    ((vmcb->control.exit_int_info & SVM_EVTINJ_TYPE_MASK) == 0))
{
 		intr_vector = vmcb->control.exit_int_info &
@@ -1474,7 +1476,7 @@ static void svm_intr_assist(struct vcpu_svm *svm)
 	if (vmcb->control.int_ctl & V_IRQ_MASK)
 		return;
 
-	if (!kvm_cpu_has_interrupt(&svm->vcpu))
+	if (!kvm_cpu_has_interrupt(vcpu))
 		return;
 
 	if (!(vmcb->save.rflags & X86_EFLAGS_IF) ||
@@ -1486,8 +1488,9 @@ static void svm_intr_assist(struct vcpu_svm *svm)
 		return;
 	}
 	/* Okay, we can deliver the interrupt: grab it and update PIC
state. */
-	intr_vector = kvm_cpu_get_interrupt(&svm->vcpu);
+	intr_vector = kvm_cpu_get_interrupt(vcpu);
 	svm_inject_irq(svm, intr_vector);
+	kvm_pt_intr_post(vcpu, intr_vector);
 }
 
 static void kvm_reput_irq(struct vcpu_svm *svm)
diff --git a/drivers/kvm/vmx.c b/drivers/kvm/vmx.c
index 019197b..499bb45 100644
--- a/drivers/kvm/vmx.c
+++ b/drivers/kvm/vmx.c
@@ -2144,7 +2144,9 @@ static void vmx_intr_assist(struct kvm_vcpu *vcpu)
 {
 	u32 idtv_info_field, intr_info_field;
 	int has_ext_irq, interrupt_window_open;
+	int vector;
 
+	kvm_pt_update_irq(vcpu);
 	has_ext_irq = kvm_cpu_has_interrupt(vcpu);
 	intr_info_field = vmcs_read32(VM_ENTRY_INTR_INFO_FIELD);
 	idtv_info_field = vmcs_read32(IDT_VECTORING_INFO_FIELD);
@@ -2174,9 +2176,11 @@ static void vmx_intr_assist(struct kvm_vcpu
*vcpu)
 	interrupt_window_open =
 		((vmcs_readl(GUEST_RFLAGS) & X86_EFLAGS_IF) &&
 		 (vmcs_read32(GUEST_INTERRUPTIBILITY_INFO) & 3) == 0);
-	if (interrupt_window_open)
-		vmx_inject_irq(vcpu, kvm_cpu_get_interrupt(vcpu));
-	else
+	if (interrupt_window_open) {
+		vector = kvm_cpu_get_interrupt(vcpu);
+		vmx_inject_irq(vcpu, vector);
+		kvm_pt_intr_post(vcpu, vector);
+	} else
 		enable_irq_window(vcpu);
 }
 

[-- Attachment #2: apic-timer-pending2.patch --]
[-- Type: application/octet-stream, Size: 8287 bytes --]

    Add back pending irqs for apic timer to give guest
    precise APIC timer interrupt number.
    
    Signed-off-by: Yaozu (Eddie) Dong <Eddie.Dong@intel.com>

diff --git a/drivers/kvm/irq.h b/drivers/kvm/irq.h
index ed6d20a..8867c82 100644
--- a/drivers/kvm/irq.h
+++ b/drivers/kvm/irq.h
@@ -110,7 +110,7 @@ struct kvm_lapic {
 	unsigned long base_address;
 	struct kvm_io_device dev;
 	struct {
-		unsigned long pending;
+		atomic_t pending;
 		s64 period;	/* unit: ns */
 		u32 divide_count;
 		ktime_t last_update;
@@ -153,5 +153,7 @@ int kvm_apic_set_irq(struct kvm_lapic *apic, u8 vec, u8 trig);
 void kvm_apic_post_state_restore(struct kvm_vcpu *vcpu);
 int kvm_ioapic_init(struct kvm *kvm);
 void kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int irq, int level);
+void kvm_pt_intr_post(struct kvm_vcpu *vcpu, int vec);
+void kvm_pt_update_irq(struct kvm_vcpu *vcpu);
 
 #endif
diff --git a/drivers/kvm/lapic.c b/drivers/kvm/lapic.c
index 352b8a7..cb4d3ca 100644
--- a/drivers/kvm/lapic.c
+++ b/drivers/kvm/lapic.c
@@ -30,6 +30,7 @@
 #include <asm/page.h>
 #include <asm/current.h>
 #include <asm/apicdef.h>
+#include <asm/atomic.h>
 #include "irq.h"
 
 #define PRId64 "d"
@@ -300,6 +301,7 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
 			     int vector, int level, int trig_mode)
 {
 	int result = 0;
+	int orig_irr;
 
 	switch (delivery_mode) {
 	case APIC_DM_FIXED:
@@ -308,7 +310,8 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
 		if (unlikely(!apic_enabled(apic)))
 			break;
 
-		if (apic_test_and_set_irr(vector, apic) && trig_mode) {
+		orig_irr = apic_test_and_set_irr(vector, apic);
+		if (orig_irr && trig_mode) {
 			apic_debug("level trig mode repeatedly for vector %d",
 				   vector);
 			break;
@@ -322,7 +325,7 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
 
 		kvm_vcpu_kick(apic->vcpu);
 
-		result = 1;
+		result = (orig_irr == 0);
 		break;
 
 	case APIC_DM_REMRD:
@@ -352,18 +355,6 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
 	return result;
 }
 
-static inline int apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
-				  int vector, int level, int trig_mode)
-{
-	int result = 0;
-
-	spin_lock_bh(&apic->lock);
-	result = __apic_accept_irq(apic, delivery_mode,
-				   vector, level, trig_mode);
-	spin_unlock_bh(&apic->lock);
-	return result;
-}
-
 struct kvm_lapic *kvm_apic_round_robin(struct kvm *kvm, u8 vector,
 				       unsigned long bitmap)
 {
@@ -595,6 +586,7 @@ static void apic_mmio_write(struct kvm_io_device *this,
 				apic_set_reg(apic, APIC_LVTT + 0x10 * i,
 					     lvt_val | APIC_LVT_MASKED);
 			}
+			atomic_set(&apic->timer.pending, 0);
 
 		}
 		break;
@@ -632,12 +624,14 @@ static void apic_mmio_write(struct kvm_io_device *this,
 			apic->timer.last_update = now;
 			apic->timer.period =
 			    APIC_BUS_CYCLE_NS * apic->timer.divide_count * val;
+			apic_debug("LAPIC period = %lld\n", apic->timer.period);
 
 			/* Make sure the lock ordering is coherent */
 			spin_unlock_bh(&apic->lock);
 			hrtimer_cancel(&apic->timer.dev);
+			atomic_set(&apic->timer.pending, 0);
 			hrtimer_start(&apic->timer.dev,
-				      ktime_add_ns(now, offset),
+				      ktime_add_ns(now, apic->timer.period),
 				      HRTIMER_MODE_ABS);
 
 			apic_debug("%s: bus cycle is %" PRId64 "ns, now 0x%016"
@@ -816,7 +810,7 @@ static void lapic_reset(struct kvm_vcpu *vcpu)
 		apic_set_reg(apic, APIC_TMR + 0x10 * i, 0);
 	}
 	apic->timer.divide_count = 0;
-	apic->timer.pending = 0;
+	atomic_set(&apic->timer.pending, 0);
 	if (vcpu->vcpu_id == 0)
 		vcpu->apic_base |= MSR_IA32_APICBASE_BSP;
 	apic_update_ppr(apic);
@@ -857,38 +851,30 @@ EXPORT_SYMBOL_GPL(kvm_lapic_get_regs);
  * timer interface
  *----------------------------------------------------------------------
  */
+
+/* TODO: make sure __apic_timer_fn runs in current pCPU */
 static int __apic_timer_fn(struct kvm_lapic *apic)
 {
-	u32 vector;
 	int result = 0;
 
-	if (unlikely(!apic_enabled(apic) ||
-		     !apic_lvt_enabled(apic, APIC_LVTT))) {
-		apic_debug("%s: time interrupt although apic is down\n",
-			   __FUNCTION__);
-		return 0;
-	}
-
-	vector = apic_lvt_vector(apic, APIC_LVTT);
-	apic->timer.last_update = apic->timer.dev.expires;
-	apic->timer.pending++;
-	__apic_accept_irq(apic, APIC_DM_FIXED, vector, 1, 0);
-
+	atomic_inc (&apic->timer.pending);
 	if (apic_lvtt_period(apic)) {
-		u32 offset;
-		u32 tmict = apic_get_reg(apic, APIC_TMICT);
-
-		offset = APIC_BUS_CYCLE_NS * apic->timer.divide_count * tmict;
-
 		result = 1;
 		apic->timer.dev.expires = ktime_add_ns(
 					apic->timer.dev.expires,
 					apic->timer.period);
 	}
-
 	return result;
 }
 
+static int inject_apic_timer_irq(struct kvm_lapic *apic)
+{
+	int vector;
+
+	vector = apic_lvt_vector(apic, APIC_LVTT);
+	return __apic_accept_irq(apic, APIC_DM_FIXED, vector, 1, 0);
+}
+
 static enum hrtimer_restart apic_timer_fn(struct hrtimer *data)
 {
 	struct kvm_lapic *apic;
@@ -963,6 +949,28 @@ int kvm_apic_has_interrupt(struct kvm_vcpu *vcpu)
 	return highest_irr;
 }
 
+void kvm_pt_update_irq(struct kvm_vcpu *vcpu)
+{
+	struct kvm_lapic *apic = vcpu->apic;
+
+	if (apic && atomic_read(&apic->timer.pending) > 0) {
+		if (inject_apic_timer_irq(apic))
+			atomic_dec(&apic->timer.pending);
+	}
+}
+EXPORT_SYMBOL_GPL(kvm_pt_update_irq);
+
+void kvm_pt_intr_post(struct kvm_vcpu *vcpu, int vec)
+{
+	struct kvm_lapic *apic = vcpu->apic;
+
+	if (apic && apic_lvt_vector(apic, APIC_LVTT) == vec)
+		apic->timer.last_update = ktime_add_ns(
+				apic->timer.last_update,
+				apic->timer.period);
+}
+EXPORT_SYMBOL_GPL(kvm_pt_intr_post);
+
 int kvm_get_apic_interrupt(struct kvm_vcpu *vcpu)
 {
 	int vector = kvm_apic_has_interrupt(vcpu);
@@ -992,7 +1000,7 @@ void kvm_apic_post_state_restore(struct kvm_vcpu *vcpu)
  	/* TODO: following code can be in a common API */
 	spin_lock_bh(&apic->lock);
  	hrtimer_cancel(&apic->timer.dev);
-	apic->timer.pending = 0;
+	atomic_set(&apic->timer.pending, 0);
 	val = apic_get_reg(apic, APIC_TDCR);
 	tmp = ((val & 0x3) | ((val & 0x8) >> 1)) + 1;
 	apic->timer.divide_count = 0x1 << (tmp & 0x7);
diff --git a/drivers/kvm/svm.c b/drivers/kvm/svm.c
index 96db35a..ec172a1 100644
--- a/drivers/kvm/svm.c
+++ b/drivers/kvm/svm.c
@@ -1461,7 +1461,9 @@ static void svm_intr_assist(struct vcpu_svm *svm)
 {
 	struct vmcb *vmcb = svm->vmcb;
 	int intr_vector = -1;
+	struct kvm_vcpu *vcpu = &svm->vcpu;
 
+	kvm_pt_update_irq(vcpu);
 	if ((vmcb->control.exit_int_info & SVM_EVTINJ_VALID) &&
 	    ((vmcb->control.exit_int_info & SVM_EVTINJ_TYPE_MASK) == 0)) {
 		intr_vector = vmcb->control.exit_int_info &
@@ -1474,7 +1476,7 @@ static void svm_intr_assist(struct vcpu_svm *svm)
 	if (vmcb->control.int_ctl & V_IRQ_MASK)
 		return;
 
-	if (!kvm_cpu_has_interrupt(&svm->vcpu))
+	if (!kvm_cpu_has_interrupt(vcpu))
 		return;
 
 	if (!(vmcb->save.rflags & X86_EFLAGS_IF) ||
@@ -1486,8 +1488,9 @@ static void svm_intr_assist(struct vcpu_svm *svm)
 		return;
 	}
 	/* Okay, we can deliver the interrupt: grab it and update PIC state. */
-	intr_vector = kvm_cpu_get_interrupt(&svm->vcpu);
+	intr_vector = kvm_cpu_get_interrupt(vcpu);
 	svm_inject_irq(svm, intr_vector);
+	kvm_pt_intr_post(vcpu, intr_vector);
 }
 
 static void kvm_reput_irq(struct vcpu_svm *svm)
diff --git a/drivers/kvm/vmx.c b/drivers/kvm/vmx.c
index 019197b..499bb45 100644
--- a/drivers/kvm/vmx.c
+++ b/drivers/kvm/vmx.c
@@ -2144,7 +2144,9 @@ static void vmx_intr_assist(struct kvm_vcpu *vcpu)
 {
 	u32 idtv_info_field, intr_info_field;
 	int has_ext_irq, interrupt_window_open;
+	int vector;
 
+	kvm_pt_update_irq(vcpu);
 	has_ext_irq = kvm_cpu_has_interrupt(vcpu);
 	intr_info_field = vmcs_read32(VM_ENTRY_INTR_INFO_FIELD);
 	idtv_info_field = vmcs_read32(IDT_VECTORING_INFO_FIELD);
@@ -2174,9 +2176,11 @@ static void vmx_intr_assist(struct kvm_vcpu *vcpu)
 	interrupt_window_open =
 		((vmcs_readl(GUEST_RFLAGS) & X86_EFLAGS_IF) &&
 		 (vmcs_read32(GUEST_INTERRUPTIBILITY_INFO) & 3) == 0);
-	if (interrupt_window_open)
-		vmx_inject_irq(vcpu, kvm_cpu_get_interrupt(vcpu));
-	else
+	if (interrupt_window_open) {
+		vector = kvm_cpu_get_interrupt(vcpu);
+		vmx_inject_irq(vcpu, vector);
+		kvm_pt_intr_post(vcpu, vector);
+	} else
 		enable_irq_window(vcpu);
 }
 

[-- Attachment #3: Type: text/plain, Size: 315 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

[-- Attachment #4: Type: text/plain, Size: 186 bytes --]

_______________________________________________
kvm-devel mailing list
kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/kvm-devel

Re: add back pending timer irqs for kernel APIC timer

[Avi Kivity]

Dong, Eddie wrote:
> kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org wrote:
>   
>>    Add back pending irqs for apic timer to get precise guest    APIC
>> timer interrupt. 
>>
>>    Signed-off-by: Yaozu (Eddie) Dong <Eddie.Dong-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>>
>>
>>     
> a typo, please use this one.
>
> With above patches, now guest timer is much accurate! sleep 60 
> get exactly 60 seconds in host per my rough test.
>
> BTW, AMD platform is not tested.
>
> thx,eddie
>
>
> diff --git a/drivers/kvm/irq.h b/drivers/kvm/irq.h
> index ed6d20a..8867c82 100644
> --- a/drivers/kvm/irq.h
> +++ b/drivers/kvm/irq.h
> @@ -110,7 +110,7 @@ struct kvm_lapic {
>  	unsigned long base_address;
>  	struct kvm_io_device dev;
>  	struct {
> -		unsigned long pending;
> +		atomic_t pending;
>  		s64 period;	/* unit: ns */
>  		u32 divide_count;
>  		ktime_t last_update;
> @@ -153,5 +153,7 @@ int kvm_apic_set_irq(struct kvm_lapic *apic, u8 vec,
> u8 trig);
>  void kvm_apic_post_state_restore(struct kvm_vcpu *vcpu);
>  int kvm_ioapic_init(struct kvm *kvm);
>  void kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int irq, int level);
> +void kvm_pt_intr_post(struct kvm_vcpu *vcpu, int vec);
> +void kvm_pt_update_irq(struct kvm_vcpu *vcpu);
>   

What does "pt" mean in this context? "pending timer"?

Suggested descriptive names below.

> *----------------------------------------------------------------------
>   */
> +
> +/* TODO: make sure __apic_timer_fn runs in current pCPU */
>  static int __apic_timer_fn(struct kvm_lapic *apic)
>  {
> -	u32 vector;
>  	int result = 0;
>  
> -	if (unlikely(!apic_enabled(apic) ||
> -		     !apic_lvt_enabled(apic, APIC_LVTT))) {
> -		apic_debug("%s: time interrupt although apic is down\n",
> -			   __FUNCTION__);
> -		return 0;
> -	}
> -
> -	vector = apic_lvt_vector(apic, APIC_LVTT);
> -	apic->timer.last_update = apic->timer.dev.expires;
> -	apic->timer.pending++;
> -	__apic_accept_irq(apic, APIC_DM_FIXED, vector, 1, 0);
> -
> +	atomic_inc (&apic->timer.pending);
>   

Extra space.

>  	if (apic_lvtt_period(apic)) {
> -		u32 offset;
> -		u32 tmict = apic_get_reg(apic, APIC_TMICT);
> -
> -		offset = APIC_BUS_CYCLE_NS * apic->timer.divide_count *
> tmict;
> -
>  		result = 1;
>  		apic->timer.dev.expires = ktime_add_ns(
>  					apic->timer.dev.expires,
>  					apic->timer.period);
>  	}
> -
>  	return result;
>  }
>   

While this improves throughput, doesn't it decrease responsiveness?  
Suppose the guest is sleeping and there is no activity except for lapic 
interrupts.  Won't the wakeup get deferred indefinitely?

>  
> +void kvm_pt_update_irq(struct kvm_vcpu *vcpu)
> +{
> +	struct kvm_lapic *apic = vcpu->apic;
> +
> +	if (apic && atomic_read(&apic->timer.pending) > 0) {
>   

Extra braces.  Also this smells like a race.  What if timer.pending is 
updated just after this?

> +		if (inject_apic_timer_irq(apic))
> +			atomic_dec(&apic->timer.pending);
> +	}
> +}
> +EXPORT_SYMBOL_GPL(kvm_pt_update_irq);
>   

How about "kvm_inject_pending_timer_irqs"

> +
> +void kvm_pt_intr_post(struct kvm_vcpu *vcpu, int vec)
> +{
> +	struct kvm_lapic *apic = vcpu->apic;
> +
> +	if (apic && apic_lvt_vector(apic, APIC_LVTT) == vec)
> +		apic->timer.last_update = ktime_add_ns(
> +				apic->timer.last_update,
> +				apic->timer.period);
> +}
> +EXPORT_SYMBOL_GPL(kvm_pt_intr_post);
>   

kvm_update_pending_timer_irq?



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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: add back pending timer irqs for kernel APIC timer

[Dong, Eddie]

Avi Kivity wrote:
> What does "pt" mean in this context? "pending timer"?
> 
> Suggested descriptive names below.

Periodic timer. Comments added.


 
>>  	if (apic_lvtt_period(apic)) {
>> -		u32 offset;
>> -		u32 tmict = apic_get_reg(apic, APIC_TMICT);
>> -
>> -		offset = APIC_BUS_CYCLE_NS * apic->timer.divide_count *
tmict;
>> -
>>  		result = 1;
>>  		apic->timer.dev.expires = ktime_add_ns(
>>  					apic->timer.dev.expires,
>>  					apic->timer.period);
>>  	}
>> -
>>  	return result;
>>  }
>> 
> 
> While this improves throughput, doesn't it decrease responsiveness?
> Suppose the guest is sleeping and there is no activity except
> for lapic
> interrupts.  Won't the wakeup get deferred indefinitely?

didn't catch here. do u mean the guest is in HLT ? There is hole here
for HLT case.
Will fix.

> 
>> 
>> +void kvm_pt_update_irq(struct kvm_vcpu *vcpu)
>> +{
>> +	struct kvm_lapic *apic = vcpu->apic;
>> +
>> +	if (apic && atomic_read(&apic->timer.pending) > 0) {
>> 
> 
> Extra braces.  Also this smells like a race.  What if timer.pending
> is updated just after this? 

kvm_pt_update_irq is within intr_assist where host IRQ is disabled.
It can be updated only after we return to guest.

> 
>> +		if (inject_apic_timer_irq(apic))
>> +			atomic_dec(&apic->timer.pending);
>> +	}
>> +}
>> +EXPORT_SYMBOL_GPL(kvm_pt_update_irq);
>> 
> 
> How about "kvm_inject_pending_timer_irqs"

Sure. But we will have PIT too, so maybe kvm_inject_apic_timer_irqs?

> 
>> +
>> +void kvm_pt_intr_post(struct kvm_vcpu *vcpu, int vec) +{
>> +	struct kvm_lapic *apic = vcpu->apic;
>> +
>> +	if (apic && apic_lvt_vector(apic, APIC_LVTT) == vec)
>> +		apic->timer.last_update = ktime_add_ns(
>> +				apic->timer.last_update,
>> +				apic->timer.period);
>> +}
>> +EXPORT_SYMBOL_GPL(kvm_pt_intr_post);
>> 
> 
> kvm_update_pending_timer_irq?

only periodic timer has this issue, one shot time is ok.
update is used as kvm_pt_update_irq, here is the post action.



BTW, can u rebase the branch? I saw some error in today's branch.

Will send patch after you rebased.

thx,eddie

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: add back pending timer irqs for kernel APIC timer

[Avi Kivity]

Dong, Eddie wrote:
>>
>> While this improves throughput, doesn't it decrease responsiveness?
>> Suppose the guest is sleeping and there is no activity except
>> for lapic
>> interrupts.  Won't the wakeup get deferred indefinitely?
>>     
>
> didn't catch here. do u mean the guest is in HLT ? There is hole here
> for HLT case.
> Will fix.
>
>   

Yes, I meant HLT.

>>> +void kvm_pt_update_irq(struct kvm_vcpu *vcpu)
>>> +{
>>> +	struct kvm_lapic *apic = vcpu->apic;
>>> +
>>> +	if (apic && atomic_read(&apic->timer.pending) > 0) {
>>>
>>>       
>> Extra braces.  Also this smells like a race.  What if timer.pending
>> is updated just after this? 
>>     
>
> kvm_pt_update_irq is within intr_assist where host IRQ is disabled.
> It can be updated only after we return to guest.
>   

What if it runs on another cpu?

>>>       
>> How about "kvm_inject_pending_timer_irqs"
>>     
>
> Sure. But we will have PIT too, so maybe kvm_inject_apic_timer_irqs?
>
>   

Maybe kvm_inject_pending_timer_irqs() can call the apic and pit 
variants, so we have just on hook in vmx.c and svm.c?

>
> BTW, can u rebase the branch? I saw some error in today's branch.
>
> Will send patch after you rebased.
>   

Okay.  I am testing out kvm-34 now, will start rebase shortly after.  It 
will take some time as there have been quite a few changes.



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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: add back pending timer irqs for kernel APIC timer

[Dong, Eddie]

>> kvm_pt_update_irq is within intr_assist where host IRQ is disabled.
>> It can be updated only after we return to guest.
>> 
> 
> What if it runs on another cpu?

This case will be forbidden in next patch. We need to remove apic->lock
too.

> 
>>>> 
>>> How about "kvm_inject_pending_timer_irqs"
>>> 
>> 
>> Sure. But we will have PIT too, so maybe kvm_inject_apic_timer_irqs?
>> 
>> 
> 
> Maybe kvm_inject_pending_timer_irqs() can call the apic and pit
> variants, so we have just on hook in vmx.c and svm.c?

No problem. 

> 
>> 
>> BTW, can u rebase the branch? I saw some error in today's branch.
>> 
>> Will send patch after you rebased.
>> 
> 
> Okay.  I am testing out kvm-34 now, will start rebase shortly after. 
> It will take some time as there have been quite a few changes.
> 
> 
OK, so you pull in a lot today?

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: add back pending timer irqs for kernel APIC timer

[Dong, Eddie]

kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org wrote:
>>> kvm_pt_update_irq is within intr_assist where host IRQ is disabled.
>>> It can be updated only after we return to guest.
>>> 
>> 
>> What if it runs on another cpu?
> 
> This case will be forbidden in next patch. We need to remove
> apic->lock too. 
> 
To avoid confuse, I mean hrtimer will be migrated as guest migrated.
And stop hrtimer when it is not in guest mode.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: add back pending timer irqs for kernel APIC timer

[Avi Kivity]

Dong, Eddie wrote:
> kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org wrote:
>   
>>>> kvm_pt_update_irq is within intr_assist where host IRQ is disabled.
>>>> It can be updated only after we return to guest.
>>>>
>>>>         
>>> What if it runs on another cpu?
>>>       
>> This case will be forbidden in next patch. We need to remove
>> apic->lock too. 
>>
>>     
> To avoid confuse, I mean hrtimer will be migrated as guest migrated.
>   

Ah okay.  Makes sense, and we have the hooks for it already in vcpu_load.

> And stop hrtimer when it is not in guest mode.
>   

I'm not convinced this is important.

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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: add back pending timer irqs for kernel APIC timer

[Dong, Eddie]

[-- Attachment #1: Type: text/plain, Size: 16297 bytes --]

Here are rebased and fixed patches, please check.
thanks, eddie


commit 9b1a9ad955d9ac44ee6e60b4a565edf044d3dcc7
Author: root <root@vt32-pae.(none)>
Date:   Fri Aug 10 22:38:59 2007 +0800

    APIC_TMCCT is a dynamically running count which always
    need to be recalculated at the time it is read. Setting
    intermediate value is meaningless.
    
    Signed-off-by: Yaozu (Eddie) Dong <Eddie.Dong-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

diff --git a/drivers/kvm/lapic.c b/drivers/kvm/lapic.c
index d59b69f..fc53e88 100644
--- a/drivers/kvm/lapic.c
+++ b/drivers/kvm/lapic.c
@@ -445,7 +445,7 @@ static u32 apic_get_tmcct(struct kvm_lapic *apic)
 {
 	u32 counter_passed;
 	ktime_t passed, now = apic->timer.dev.base->get_time();
-	u32 tmcct = apic_get_reg(apic, APIC_TMCCT);
+	u32 tmcct = apic_get_reg(apic, APIC_TMICT);
 
 	ASSERT(apic != NULL);
 
@@ -474,9 +474,6 @@ static u32 apic_get_tmcct(struct kvm_lapic *apic)
 			} while (tmcct <= 0);
 	}
 
-	apic->timer.last_update = now;
-	apic_set_reg(apic, APIC_TMCCT, tmcct);
-
 	return tmcct;
 }
 
@@ -633,7 +630,6 @@ static void apic_mmio_write(struct kvm_io_device
*this,
 			u32 offset;
 
 			apic_set_reg(apic, APIC_TMICT, val);
-			apic_set_reg(apic, APIC_TMCCT, val);
 			apic->timer.last_update = now;
 			offset =
 			    APIC_BUS_CYCLE_NS * apic->timer.divide_count
* val;
@@ -815,7 +811,6 @@ static void lapic_reset(struct kvm_vcpu *vcpu)
 	apic_set_reg(apic, APIC_ICR2, 0);
 	apic_set_reg(apic, APIC_TDCR, 0);
 	apic_set_reg(apic, APIC_TMICT, 0);
-	apic_set_reg(apic, APIC_TMCCT, 0);
 	for (i = 0; i < 8; i++) {
 		apic_set_reg(apic, APIC_IRR + 0x10 * i, 0);
 		apic_set_reg(apic, APIC_ISR + 0x10 * i, 0);
@@ -886,13 +881,10 @@ static int __apic_timer_fn(struct kvm_lapic *apic)
 		u32 offset;
 		u32 tmict = apic_get_reg(apic, APIC_TMICT);
 
-		apic_set_reg(apic, APIC_TMCCT, tmict);
 		offset = APIC_BUS_CYCLE_NS * apic->timer.divide_count *
tmict;
 
 		result = 1;
 		apic->timer.dev.expires = ktime_add_ns(now, offset);
-	} else {
-		apic_set_reg(apic, APIC_TMCCT, 0);
 	}
 
 	return result;
@@ -996,7 +988,6 @@ void kvm_apic_post_state_restore(struct kvm_vcpu
*vcpu)
 			     MSR_IA32_APICBASE_BASE;
 	apic_set_reg(apic, APIC_LVR, APIC_VERSION);
 	val = apic_get_reg(apic, APIC_TMICT);
-	apic_set_reg(apic, APIC_TMCCT, val);
 	apic_update_ppr(apic);
 
  	/* TODO: following code can be in a common API */

        In order to calculate instant APIC counter register value,
        KVM refers host time stamp of previous APIC timer firing point
        (last_update) to get the time passed and thus calculate
        current APIC time counter register. Using NOW of
        the host hrtime firing time will have accumulated difference
        since the timer callback is executed with uncertainity delay,
        and it may introduce many drift as the time goes on.
    
        This patch fix this by adjust last_update by expected period.
    
        Signed-off-by: Yaozu (Eddie) Dong <Eddie.Dong-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
diff --git a/drivers/kvm/irq.c b/drivers/kvm/irq.c
diff --git a/drivers/kvm/irq.h b/drivers/kvm/irq.h
index 44e1fa4..ed6d20a 100644
--- a/drivers/kvm/irq.h
+++ b/drivers/kvm/irq.h
@@ -111,6 +111,7 @@ struct kvm_lapic {
 	struct kvm_io_device dev;
 	struct {
 		unsigned long pending;
+		s64 period;	/* unit: ns */
 		u32 divide_count;
 		ktime_t last_update;
 		struct hrtimer dev;
diff --git a/drivers/kvm/kvm_main.c b/drivers/kvm/kvm_main.c
diff --git a/drivers/kvm/lapic.c b/drivers/kvm/lapic.c
index 1430667..09aadcb 100644
--- a/drivers/kvm/lapic.c
+++ b/drivers/kvm/lapic.c
@@ -350,18 +350,6 @@ static int __apic_accept_irq(struct kvm_lapic
*apic, int delivery_mode,
 	return result;
 }
 
-static inline int apic_accept_irq(struct kvm_lapic *apic, int
delivery_mode,
-				  int vector, int level, int trig_mode)
-{
-	int result = 0;
-
-	spin_lock_bh(&apic->lock);
-	result = __apic_accept_irq(apic, delivery_mode,
-				   vector, level, trig_mode);
-	spin_unlock_bh(&apic->lock);
-	return result;
-}
-
 struct kvm_lapic *kvm_apic_round_robin(struct kvm *kvm, u8 vector,
 				       unsigned long bitmap)
 {
@@ -625,27 +613,28 @@ static void apic_mmio_write(struct kvm_io_device
*this,
 	case APIC_TMICT:
 		{
 			ktime_t now = apic->timer.dev.base->get_time();
-			u32 offset;
 
 			apic_set_reg(apic, APIC_TMICT, val);
 			apic->timer.last_update = now;
-			offset =
+			apic->timer.period =
 			    APIC_BUS_CYCLE_NS * apic->timer.divide_count
* val;
 
 			/* Make sure the lock ordering is coherent */
 			spin_unlock_bh(&apic->lock);
 			hrtimer_cancel(&apic->timer.dev);
 			hrtimer_start(&apic->timer.dev,
-				      ktime_add_ns(now, offset),
+				      ktime_add_ns(now,
apic->timer.period),
 				      HRTIMER_MODE_ABS);
 
 			apic_debug("%s: bus cycle is %" PRId64 "ns, now
0x%016"
 				   PRIx64 ", "
-				   "timer initial count 0x%x, offset
0x%x, "
+				   "timer initial count 0x%x, period
%lldns, "
 				   "expire @ 0x%016" PRIx64 ".\n",
__FUNCTION__,
 				   APIC_BUS_CYCLE_NS, ktime_to_ns(now),
-				   apic_get_reg(apic, APIC_TMICT),
offset,
-				   ktime_to_ns(ktime_add_ns(now,
offset)));
+				   apic_get_reg(apic, APIC_TMICT),
+				   apic->timer.period,
+				   ktime_to_ns(ktime_add_ns(now,
+						apic->timer.period)));
 		}
 		return;
 
@@ -760,7 +749,7 @@ void kvm_lapic_set_base(struct kvm_vcpu *vcpu, u64
value)
 
 	/* with FSB delivery interrupt, we can restart APIC
functionality */
 	apic_debug("apic base msr is 0x%016" PRIx64 ", and base address
is "
-		   "0x%lx.\n", apic->apic_base, apic->base_address);
+		   "0x%lx.\n", apic->vcpu->apic_base,
apic->base_address);
 
 	spin_unlock_bh(&apic->lock);
 }
@@ -859,7 +848,6 @@ EXPORT_SYMBOL_GPL(kvm_lapic_get_regs);
 static int __apic_timer_fn(struct kvm_lapic *apic)
 {
 	u32 vector;
-	ktime_t now;
 	int result = 0;
 
 	if (unlikely(!apic_enabled(apic) ||
@@ -870,8 +858,7 @@ static int __apic_timer_fn(struct kvm_lapic *apic)
 	}
 
 	vector = apic_lvt_vector(apic, APIC_LVTT);
-	now = apic->timer.dev.base->get_time();
-	apic->timer.last_update = now;
+	apic->timer.last_update = apic->timer.dev.expires;
 	apic->timer.pending++;
 	__apic_accept_irq(apic, APIC_DM_FIXED, vector, 1, 0);
 
@@ -882,7 +869,9 @@ static int __apic_timer_fn(struct kvm_lapic *apic)
 		offset = APIC_BUS_CYCLE_NS * apic->timer.divide_count *
tmict;
 
 		result = 1;
-		apic->timer.dev.expires = ktime_add_ns(now, offset);
+		apic->timer.dev.expires = ktime_add_ns(
+					apic->timer.dev.expires,
+					apic->timer.period);
 	}
 
 	return result;
diff --git a/drivers/kvm/svm.c b/drivers/kvm/svm.c
diff --git a/drivers/kvm/vmx.c b/drivers/kvm/vmx.c



commit f1c8d1076b48625469b69abd6767bf429f35120d
Author: root <root@vt32-pae.(none)>
Date:   Tue Aug 14 11:22:47 2007 +0800

        APIC timer IRQ is set every time when a certain period
        expires at host time, but the guest may be descheduled
        at that time and thus the irq be overwritten by later fire.
        This patch keep track of firing irq numbers and decrease
        only when the IRQ is injected to guest or buffered in
        APIC.
    
        Signed-off-by: Yaozu (Eddie) Dong <Eddie.Dong-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

diff --git a/drivers/kvm/irq.c b/drivers/kvm/irq.c
index e09cd65..eb72b56 100644
--- a/drivers/kvm/irq.c
+++ b/drivers/kvm/irq.c
@@ -78,3 +78,17 @@ void kvm_vcpu_kick(struct kvm_vcpu *vcpu)
 		smp_call_function_single(ipi_pcpu, vcpu_kick_intr, vcpu,
0, 0);
 }
 
+void kvm_inject_pending_timer_irqs(struct kvm_vcpu *vcpu)
+{
+	kvm_inject_apic_timer_irqs(vcpu);
+	/* TODO: PIT, RTC etc. */
+}
+EXPORT_SYMBOL_GPL(kvm_inject_pending_timer_irqs);
+
+void kvm_timer_intr_post(struct kvm_vcpu *vcpu, int vec)
+{
+	kvm_apic_timer_intr_post(vcpu, vec);
+	/* TODO: PIT, RTC etc. */
+}
+EXPORT_SYMBOL_GPL(kvm_timer_intr_post);
+
diff --git a/drivers/kvm/irq.h b/drivers/kvm/irq.h
index ed6d20a..419004e 100644
--- a/drivers/kvm/irq.h
+++ b/drivers/kvm/irq.h
@@ -110,7 +110,7 @@ struct kvm_lapic {
 	unsigned long base_address;
 	struct kvm_io_device dev;
 	struct {
-		unsigned long pending;
+		atomic_t pending;
 		s64 period;	/* unit: ns */
 		u32 divide_count;
 		ktime_t last_update;
@@ -153,5 +153,9 @@ int kvm_apic_set_irq(struct kvm_lapic *apic, u8 vec,
u8 trig);
 void kvm_apic_post_state_restore(struct kvm_vcpu *vcpu);
 int kvm_ioapic_init(struct kvm *kvm);
 void kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int irq, int level);
+void kvm_apic_timer_intr_post(struct kvm_vcpu *vcpu, int vec);
+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);
 
 #endif
diff --git a/drivers/kvm/kvm_main.c b/drivers/kvm/kvm_main.c
index c59e761..bb3fd24 100644
--- a/drivers/kvm/kvm_main.c
+++ b/drivers/kvm/kvm_main.c
@@ -283,6 +283,8 @@ EXPORT_SYMBOL_GPL(kvm_vcpu_init);
 void kvm_vcpu_uninit(struct kvm_vcpu *vcpu)
 {
 	kvm_mmu_destroy(vcpu);
+	if (vcpu->apic)
+		hrtimer_cancel(&vcpu->apic->timer.dev);
 	kvm_free_apic(vcpu->apic);
 	free_page((unsigned long)vcpu->pio_data);
 	free_page((unsigned long)vcpu->run);
diff --git a/drivers/kvm/lapic.c b/drivers/kvm/lapic.c
index 09aadcb..a6fec50 100644
--- a/drivers/kvm/lapic.c
+++ b/drivers/kvm/lapic.c
@@ -30,6 +30,7 @@
 #include <asm/page.h>
 #include <asm/current.h>
 #include <asm/apicdef.h>
+#include <asm/atomic.h>
 #include "irq.h"
 
 #define PRId64 "d"
@@ -298,6 +299,7 @@ static int __apic_accept_irq(struct kvm_lapic *apic,
int delivery_mode,
 			     int vector, int level, int trig_mode)
 {
 	int result = 0;
+	int orig_irr;
 
 	switch (delivery_mode) {
 	case APIC_DM_FIXED:
@@ -306,7 +308,8 @@ static int __apic_accept_irq(struct kvm_lapic *apic,
int delivery_mode,
 		if (unlikely(!apic_enabled(apic)))
 			break;
 
-		if (apic_test_and_set_irr(vector, apic) && trig_mode) {
+		orig_irr = apic_test_and_set_irr(vector, apic);
+		if (orig_irr && trig_mode) {
 			apic_debug("level trig mode repeatedly for
vector %d",
 				   vector);
 			break;
@@ -320,7 +323,7 @@ static int __apic_accept_irq(struct kvm_lapic *apic,
int delivery_mode,
 
 		kvm_vcpu_kick(apic->vcpu);
 
-		result = 1;
+		result = (orig_irr == 0);
 		break;
 
 	case APIC_DM_REMRD:
@@ -581,6 +584,7 @@ static void apic_mmio_write(struct kvm_io_device
*this,
 				apic_set_reg(apic, APIC_LVTT + 0x10 * i,
 					     lvt_val | APIC_LVT_MASKED);
 			}
+			atomic_set(&apic->timer.pending, 0);
 
 		}
 		break;
@@ -622,6 +626,7 @@ static void apic_mmio_write(struct kvm_io_device
*this,
 			/* Make sure the lock ordering is coherent */
 			spin_unlock_bh(&apic->lock);
 			hrtimer_cancel(&apic->timer.dev);
+			atomic_set(&apic->timer.pending, 0);
 			hrtimer_start(&apic->timer.dev,
 				      ktime_add_ns(now,
apic->timer.period),
 				      HRTIMER_MODE_ABS);
@@ -804,7 +809,7 @@ static void lapic_reset(struct kvm_vcpu *vcpu)
 		apic_set_reg(apic, APIC_TMR + 0x10 * i, 0);
 	}
 	apic->timer.divide_count = 0;
-	apic->timer.pending = 0;
+	atomic_set(&apic->timer.pending, 0);
 	if (vcpu->vcpu_id == 0)
 		vcpu->apic_base |= MSR_IA32_APICBASE_BSP;
 	apic_update_ppr(apic);
@@ -845,38 +850,33 @@ EXPORT_SYMBOL_GPL(kvm_lapic_get_regs);
  * timer interface
 
*----------------------------------------------------------------------
  */
+
+/* TODO: make sure __apic_timer_fn runs in current pCPU */
 static int __apic_timer_fn(struct kvm_lapic *apic)
 {
-	u32 vector;
 	int result = 0;
+	wait_queue_head_t *q = &apic->vcpu->wq;
 
-	if (unlikely(!apic_enabled(apic) ||
-		     !apic_lvt_enabled(apic, APIC_LVTT))) {
-		apic_debug("%s: time interrupt although apic is down\n",
-			   __FUNCTION__);
-		return 0;
-	}
-
-	vector = apic_lvt_vector(apic, APIC_LVTT);
-	apic->timer.last_update = apic->timer.dev.expires;
-	apic->timer.pending++;
-	__apic_accept_irq(apic, APIC_DM_FIXED, vector, 1, 0);
-
+	atomic_inc(&apic->timer.pending);
+	if (waitqueue_active(q))
+		wake_up_interruptible(q);
 	if (apic_lvtt_period(apic)) {
-		u32 offset;
-		u32 tmict = apic_get_reg(apic, APIC_TMICT);
-
-		offset = APIC_BUS_CYCLE_NS * apic->timer.divide_count *
tmict;
-
 		result = 1;
 		apic->timer.dev.expires = ktime_add_ns(
 					apic->timer.dev.expires,
 					apic->timer.period);
 	}
-
 	return result;
 }
 
+static int __inject_apic_timer_irq(struct kvm_lapic *apic)
+{
+	int vector;
+
+	vector = apic_lvt_vector(apic, APIC_LVTT);
+	return __apic_accept_irq(apic, APIC_DM_FIXED, vector, 1, 0);
+}
+
 static enum hrtimer_restart apic_timer_fn(struct hrtimer *data)
 {
 	struct kvm_lapic *apic;
@@ -951,6 +951,26 @@ int kvm_apic_has_interrupt(struct kvm_vcpu *vcpu)
 	return highest_irr;
 }
 
+void kvm_inject_apic_timer_irqs(struct kvm_vcpu *vcpu)
+{
+	struct kvm_lapic *apic = vcpu->apic;
+
+	if (apic && atomic_read(&apic->timer.pending) > 0) {
+		if (__inject_apic_timer_irq(apic))
+			atomic_dec(&apic->timer.pending);
+	}
+}
+
+void kvm_apic_timer_intr_post(struct kvm_vcpu *vcpu, int vec)
+{
+	struct kvm_lapic *apic = vcpu->apic;
+
+	if (apic && apic_lvt_vector(apic, APIC_LVTT) == vec)
+		apic->timer.last_update = ktime_add_ns(
+				apic->timer.last_update,
+				apic->timer.period);
+}
+
 int kvm_get_apic_interrupt(struct kvm_vcpu *vcpu)
 {
 	int vector = kvm_apic_has_interrupt(vcpu);
@@ -980,7 +1000,7 @@ void kvm_apic_post_state_restore(struct kvm_vcpu
*vcpu)
 	/* TODO: following code can be in a common API */
 	spin_lock_bh(&apic->lock);
 	hrtimer_cancel(&apic->timer.dev);
-	apic->timer.pending = 0;
+	atomic_set(&apic->timer.pending, 0);
 	val = apic_get_reg(apic, APIC_TDCR);
 	tmp = ((val & 0x3) | ((val & 0x8) >> 1)) + 1;
 	apic->timer.divide_count = 0x1 << (tmp & 0x7);
diff --git a/drivers/kvm/svm.c b/drivers/kvm/svm.c
index 139499b..b080feb 100644
--- a/drivers/kvm/svm.c
+++ b/drivers/kvm/svm.c
@@ -1331,7 +1331,9 @@ static void svm_intr_assist(struct vcpu_svm *svm)
 {
 	struct vmcb *vmcb = svm->vmcb;
 	int intr_vector = -1;
+	struct kvm_vcpu *vcpu = &svm->vcpu;
 
+	kvm_inject_pending_timer_irqs(vcpu);
 	if ((vmcb->control.exit_int_info & SVM_EVTINJ_VALID) &&
 	    ((vmcb->control.exit_int_info & SVM_EVTINJ_TYPE_MASK) == 0))
{
 		intr_vector = vmcb->control.exit_int_info &
@@ -1344,7 +1346,7 @@ static void svm_intr_assist(struct vcpu_svm *svm)
 	if (vmcb->control.int_ctl & V_IRQ_MASK)
 		return;
 
-	if (!kvm_cpu_has_interrupt(&svm->vcpu))
+	if (!kvm_cpu_has_interrupt(vcpu))
 		return;
 
 	if (!(vmcb->save.rflags & X86_EFLAGS_IF) ||
@@ -1356,8 +1358,9 @@ static void svm_intr_assist(struct vcpu_svm *svm)
 		return;
 	}
 	/* Okay, we can deliver the interrupt: grab it and update PIC
state. */
-	intr_vector = kvm_cpu_get_interrupt(&svm->vcpu);
+	intr_vector = kvm_cpu_get_interrupt(vcpu);
 	svm_inject_irq(svm, intr_vector);
+	kvm_timer_intr_post(vcpu, intr_vector);
 }
 
 static void kvm_reput_irq(struct vcpu_svm *svm)
diff --git a/drivers/kvm/vmx.c b/drivers/kvm/vmx.c
index aad02c5..9fa7356 100644
--- a/drivers/kvm/vmx.c
+++ b/drivers/kvm/vmx.c
@@ -2091,7 +2091,9 @@ static void vmx_intr_assist(struct kvm_vcpu *vcpu)
 {
 	u32 idtv_info_field, intr_info_field;
 	int has_ext_irq, interrupt_window_open;
+	int vector;
 
+	kvm_inject_pending_timer_irqs(vcpu);
 	has_ext_irq = kvm_cpu_has_interrupt(vcpu);
 	intr_info_field = vmcs_read32(VM_ENTRY_INTR_INFO_FIELD);
 	idtv_info_field = vmcs_read32(IDT_VECTORING_INFO_FIELD);
@@ -2121,9 +2123,11 @@ static void vmx_intr_assist(struct kvm_vcpu
*vcpu)
 	interrupt_window_open =
 		((vmcs_readl(GUEST_RFLAGS) & X86_EFLAGS_IF) &&
 		 (vmcs_read32(GUEST_INTERRUPTIBILITY_INFO) & 3) == 0);
-	if (interrupt_window_open)
-		vmx_inject_irq(vcpu, kvm_cpu_get_interrupt(vcpu));
-	else
+	if (interrupt_window_open) {
+		vector = kvm_cpu_get_interrupt(vcpu);
+		vmx_inject_irq(vcpu, vector);
+		kvm_timer_intr_post(vcpu, vector);
+	} else
 		enable_irq_window(vcpu);
 }
 



[-- Attachment #2: apic-timer-tmcct.patch --]
[-- Type: application/octet-stream, Size: 2281 bytes --]

commit 9b1a9ad955d9ac44ee6e60b4a565edf044d3dcc7
Author: root <root@vt32-pae.(none)>
Date:   Fri Aug 10 22:38:59 2007 +0800

    APIC_TMCCT is a dynamically running count which always
    need to be recalculated at the time it is read. Setting
    intermediate value is meaningless.
    
    Signed-off-by: Yaozu (Eddie) Dong <Eddie.Dong@intel.com>

diff --git a/drivers/kvm/lapic.c b/drivers/kvm/lapic.c
index d59b69f..fc53e88 100644
--- a/drivers/kvm/lapic.c
+++ b/drivers/kvm/lapic.c
@@ -445,7 +445,7 @@ static u32 apic_get_tmcct(struct kvm_lapic *apic)
 {
 	u32 counter_passed;
 	ktime_t passed, now = apic->timer.dev.base->get_time();
-	u32 tmcct = apic_get_reg(apic, APIC_TMCCT);
+	u32 tmcct = apic_get_reg(apic, APIC_TMICT);
 
 	ASSERT(apic != NULL);
 
@@ -474,9 +474,6 @@ static u32 apic_get_tmcct(struct kvm_lapic *apic)
 			} while (tmcct <= 0);
 	}
 
-	apic->timer.last_update = now;
-	apic_set_reg(apic, APIC_TMCCT, tmcct);
-
 	return tmcct;
 }
 
@@ -633,7 +630,6 @@ static void apic_mmio_write(struct kvm_io_device *this,
 			u32 offset;
 
 			apic_set_reg(apic, APIC_TMICT, val);
-			apic_set_reg(apic, APIC_TMCCT, val);
 			apic->timer.last_update = now;
 			offset =
 			    APIC_BUS_CYCLE_NS * apic->timer.divide_count * val;
@@ -815,7 +811,6 @@ static void lapic_reset(struct kvm_vcpu *vcpu)
 	apic_set_reg(apic, APIC_ICR2, 0);
 	apic_set_reg(apic, APIC_TDCR, 0);
 	apic_set_reg(apic, APIC_TMICT, 0);
-	apic_set_reg(apic, APIC_TMCCT, 0);
 	for (i = 0; i < 8; i++) {
 		apic_set_reg(apic, APIC_IRR + 0x10 * i, 0);
 		apic_set_reg(apic, APIC_ISR + 0x10 * i, 0);
@@ -886,13 +881,10 @@ static int __apic_timer_fn(struct kvm_lapic *apic)
 		u32 offset;
 		u32 tmict = apic_get_reg(apic, APIC_TMICT);
 
-		apic_set_reg(apic, APIC_TMCCT, tmict);
 		offset = APIC_BUS_CYCLE_NS * apic->timer.divide_count * tmict;
 
 		result = 1;
 		apic->timer.dev.expires = ktime_add_ns(now, offset);
-	} else {
-		apic_set_reg(apic, APIC_TMCCT, 0);
 	}
 
 	return result;
@@ -996,7 +988,6 @@ void kvm_apic_post_state_restore(struct kvm_vcpu *vcpu)
 			     MSR_IA32_APICBASE_BASE;
 	apic_set_reg(apic, APIC_LVR, APIC_VERSION);
 	val = apic_get_reg(apic, APIC_TMICT);
-	apic_set_reg(apic, APIC_TMCCT, val);
 	apic_update_ppr(apic);
 
  	/* TODO: following code can be in a common API */

[-- Attachment #3: apic-timer-last5.patch --]
[-- Type: application/octet-stream, Size: 4220 bytes --]

        In order to calculate instant APIC counter register value,
        KVM refers host time stamp of previous APIC timer firing point
        (last_update) to get the time passed and thus calculate
        current APIC time counter register. Using NOW of
        the host hrtime firing time will have accumulated difference
        since the timer callback is executed with uncertainity delay,
        and it may introduce many drift as the time goes on.
    
        This patch fix this by adjust last_update by expected period.
    
        Signed-off-by: Yaozu (Eddie) Dong <Eddie.Dong@intel.com>
diff --git a/drivers/kvm/irq.c b/drivers/kvm/irq.c
diff --git a/drivers/kvm/irq.h b/drivers/kvm/irq.h
index 44e1fa4..ed6d20a 100644
--- a/drivers/kvm/irq.h
+++ b/drivers/kvm/irq.h
@@ -111,6 +111,7 @@ struct kvm_lapic {
 	struct kvm_io_device dev;
 	struct {
 		unsigned long pending;
+		s64 period;	/* unit: ns */
 		u32 divide_count;
 		ktime_t last_update;
 		struct hrtimer dev;
diff --git a/drivers/kvm/kvm_main.c b/drivers/kvm/kvm_main.c
diff --git a/drivers/kvm/lapic.c b/drivers/kvm/lapic.c
index 1430667..09aadcb 100644
--- a/drivers/kvm/lapic.c
+++ b/drivers/kvm/lapic.c
@@ -350,18 +350,6 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
 	return result;
 }
 
-static inline int apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
-				  int vector, int level, int trig_mode)
-{
-	int result = 0;
-
-	spin_lock_bh(&apic->lock);
-	result = __apic_accept_irq(apic, delivery_mode,
-				   vector, level, trig_mode);
-	spin_unlock_bh(&apic->lock);
-	return result;
-}
-
 struct kvm_lapic *kvm_apic_round_robin(struct kvm *kvm, u8 vector,
 				       unsigned long bitmap)
 {
@@ -625,27 +613,28 @@ static void apic_mmio_write(struct kvm_io_device *this,
 	case APIC_TMICT:
 		{
 			ktime_t now = apic->timer.dev.base->get_time();
-			u32 offset;
 
 			apic_set_reg(apic, APIC_TMICT, val);
 			apic->timer.last_update = now;
-			offset =
+			apic->timer.period =
 			    APIC_BUS_CYCLE_NS * apic->timer.divide_count * val;
 
 			/* Make sure the lock ordering is coherent */
 			spin_unlock_bh(&apic->lock);
 			hrtimer_cancel(&apic->timer.dev);
 			hrtimer_start(&apic->timer.dev,
-				      ktime_add_ns(now, offset),
+				      ktime_add_ns(now, apic->timer.period),
 				      HRTIMER_MODE_ABS);
 
 			apic_debug("%s: bus cycle is %" PRId64 "ns, now 0x%016"
 				   PRIx64 ", "
-				   "timer initial count 0x%x, offset 0x%x, "
+				   "timer initial count 0x%x, period %lldns, "
 				   "expire @ 0x%016" PRIx64 ".\n", __FUNCTION__,
 				   APIC_BUS_CYCLE_NS, ktime_to_ns(now),
-				   apic_get_reg(apic, APIC_TMICT), offset,
-				   ktime_to_ns(ktime_add_ns(now, offset)));
+				   apic_get_reg(apic, APIC_TMICT),
+				   apic->timer.period,
+				   ktime_to_ns(ktime_add_ns(now,
+						apic->timer.period)));
 		}
 		return;
 
@@ -760,7 +749,7 @@ void kvm_lapic_set_base(struct kvm_vcpu *vcpu, u64 value)
 
 	/* with FSB delivery interrupt, we can restart APIC functionality */
 	apic_debug("apic base msr is 0x%016" PRIx64 ", and base address is "
-		   "0x%lx.\n", apic->apic_base, apic->base_address);
+		   "0x%lx.\n", apic->vcpu->apic_base, apic->base_address);
 
 	spin_unlock_bh(&apic->lock);
 }
@@ -859,7 +848,6 @@ EXPORT_SYMBOL_GPL(kvm_lapic_get_regs);
 static int __apic_timer_fn(struct kvm_lapic *apic)
 {
 	u32 vector;
-	ktime_t now;
 	int result = 0;
 
 	if (unlikely(!apic_enabled(apic) ||
@@ -870,8 +858,7 @@ static int __apic_timer_fn(struct kvm_lapic *apic)
 	}
 
 	vector = apic_lvt_vector(apic, APIC_LVTT);
-	now = apic->timer.dev.base->get_time();
-	apic->timer.last_update = now;
+	apic->timer.last_update = apic->timer.dev.expires;
 	apic->timer.pending++;
 	__apic_accept_irq(apic, APIC_DM_FIXED, vector, 1, 0);
 
@@ -882,7 +869,9 @@ static int __apic_timer_fn(struct kvm_lapic *apic)
 		offset = APIC_BUS_CYCLE_NS * apic->timer.divide_count * tmict;
 
 		result = 1;
-		apic->timer.dev.expires = ktime_add_ns(now, offset);
+		apic->timer.dev.expires = ktime_add_ns(
+					apic->timer.dev.expires,
+					apic->timer.period);
 	}
 
 	return result;
diff --git a/drivers/kvm/svm.c b/drivers/kvm/svm.c
diff --git a/drivers/kvm/vmx.c b/drivers/kvm/vmx.c

[-- Attachment #4: apic-timer-pending4.patch --]
[-- Type: application/octet-stream, Size: 9108 bytes --]

commit f1c8d1076b48625469b69abd6767bf429f35120d
Author: root <root@vt32-pae.(none)>
Date:   Tue Aug 14 11:22:47 2007 +0800

        APIC timer IRQ is set every time when a certain period
        expires at host time, but the guest may be descheduled
        at that time and thus the irq be overwritten by later fire.
        This patch keep track of firing irq numbers and decrease
        only when the IRQ is injected to guest or buffered in
        APIC.
    
        Signed-off-by: Yaozu (Eddie) Dong <Eddie.Dong@intel.com>

diff --git a/drivers/kvm/irq.c b/drivers/kvm/irq.c
index e09cd65..eb72b56 100644
--- a/drivers/kvm/irq.c
+++ b/drivers/kvm/irq.c
@@ -78,3 +78,17 @@ void kvm_vcpu_kick(struct kvm_vcpu *vcpu)
 		smp_call_function_single(ipi_pcpu, vcpu_kick_intr, vcpu, 0, 0);
 }
 
+void kvm_inject_pending_timer_irqs(struct kvm_vcpu *vcpu)
+{
+	kvm_inject_apic_timer_irqs(vcpu);
+	/* TODO: PIT, RTC etc. */
+}
+EXPORT_SYMBOL_GPL(kvm_inject_pending_timer_irqs);
+
+void kvm_timer_intr_post(struct kvm_vcpu *vcpu, int vec)
+{
+	kvm_apic_timer_intr_post(vcpu, vec);
+	/* TODO: PIT, RTC etc. */
+}
+EXPORT_SYMBOL_GPL(kvm_timer_intr_post);
+
diff --git a/drivers/kvm/irq.h b/drivers/kvm/irq.h
index ed6d20a..419004e 100644
--- a/drivers/kvm/irq.h
+++ b/drivers/kvm/irq.h
@@ -110,7 +110,7 @@ struct kvm_lapic {
 	unsigned long base_address;
 	struct kvm_io_device dev;
 	struct {
-		unsigned long pending;
+		atomic_t pending;
 		s64 period;	/* unit: ns */
 		u32 divide_count;
 		ktime_t last_update;
@@ -153,5 +153,9 @@ int kvm_apic_set_irq(struct kvm_lapic *apic, u8 vec, u8 trig);
 void kvm_apic_post_state_restore(struct kvm_vcpu *vcpu);
 int kvm_ioapic_init(struct kvm *kvm);
 void kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int irq, int level);
+void kvm_apic_timer_intr_post(struct kvm_vcpu *vcpu, int vec);
+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);
 
 #endif
diff --git a/drivers/kvm/kvm_main.c b/drivers/kvm/kvm_main.c
index c59e761..bb3fd24 100644
--- a/drivers/kvm/kvm_main.c
+++ b/drivers/kvm/kvm_main.c
@@ -283,6 +283,8 @@ EXPORT_SYMBOL_GPL(kvm_vcpu_init);
 void kvm_vcpu_uninit(struct kvm_vcpu *vcpu)
 {
 	kvm_mmu_destroy(vcpu);
+	if (vcpu->apic)
+		hrtimer_cancel(&vcpu->apic->timer.dev);
 	kvm_free_apic(vcpu->apic);
 	free_page((unsigned long)vcpu->pio_data);
 	free_page((unsigned long)vcpu->run);
diff --git a/drivers/kvm/lapic.c b/drivers/kvm/lapic.c
index 09aadcb..a6fec50 100644
--- a/drivers/kvm/lapic.c
+++ b/drivers/kvm/lapic.c
@@ -30,6 +30,7 @@
 #include <asm/page.h>
 #include <asm/current.h>
 #include <asm/apicdef.h>
+#include <asm/atomic.h>
 #include "irq.h"
 
 #define PRId64 "d"
@@ -298,6 +299,7 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
 			     int vector, int level, int trig_mode)
 {
 	int result = 0;
+	int orig_irr;
 
 	switch (delivery_mode) {
 	case APIC_DM_FIXED:
@@ -306,7 +308,8 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
 		if (unlikely(!apic_enabled(apic)))
 			break;
 
-		if (apic_test_and_set_irr(vector, apic) && trig_mode) {
+		orig_irr = apic_test_and_set_irr(vector, apic);
+		if (orig_irr && trig_mode) {
 			apic_debug("level trig mode repeatedly for vector %d",
 				   vector);
 			break;
@@ -320,7 +323,7 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
 
 		kvm_vcpu_kick(apic->vcpu);
 
-		result = 1;
+		result = (orig_irr == 0);
 		break;
 
 	case APIC_DM_REMRD:
@@ -581,6 +584,7 @@ static void apic_mmio_write(struct kvm_io_device *this,
 				apic_set_reg(apic, APIC_LVTT + 0x10 * i,
 					     lvt_val | APIC_LVT_MASKED);
 			}
+			atomic_set(&apic->timer.pending, 0);
 
 		}
 		break;
@@ -622,6 +626,7 @@ static void apic_mmio_write(struct kvm_io_device *this,
 			/* Make sure the lock ordering is coherent */
 			spin_unlock_bh(&apic->lock);
 			hrtimer_cancel(&apic->timer.dev);
+			atomic_set(&apic->timer.pending, 0);
 			hrtimer_start(&apic->timer.dev,
 				      ktime_add_ns(now, apic->timer.period),
 				      HRTIMER_MODE_ABS);
@@ -804,7 +809,7 @@ static void lapic_reset(struct kvm_vcpu *vcpu)
 		apic_set_reg(apic, APIC_TMR + 0x10 * i, 0);
 	}
 	apic->timer.divide_count = 0;
-	apic->timer.pending = 0;
+	atomic_set(&apic->timer.pending, 0);
 	if (vcpu->vcpu_id == 0)
 		vcpu->apic_base |= MSR_IA32_APICBASE_BSP;
 	apic_update_ppr(apic);
@@ -845,38 +850,33 @@ EXPORT_SYMBOL_GPL(kvm_lapic_get_regs);
  * timer interface
  *----------------------------------------------------------------------
  */
+
+/* TODO: make sure __apic_timer_fn runs in current pCPU */
 static int __apic_timer_fn(struct kvm_lapic *apic)
 {
-	u32 vector;
 	int result = 0;
+	wait_queue_head_t *q = &apic->vcpu->wq;
 
-	if (unlikely(!apic_enabled(apic) ||
-		     !apic_lvt_enabled(apic, APIC_LVTT))) {
-		apic_debug("%s: time interrupt although apic is down\n",
-			   __FUNCTION__);
-		return 0;
-	}
-
-	vector = apic_lvt_vector(apic, APIC_LVTT);
-	apic->timer.last_update = apic->timer.dev.expires;
-	apic->timer.pending++;
-	__apic_accept_irq(apic, APIC_DM_FIXED, vector, 1, 0);
-
+	atomic_inc(&apic->timer.pending);
+	if (waitqueue_active(q))
+		wake_up_interruptible(q);
 	if (apic_lvtt_period(apic)) {
-		u32 offset;
-		u32 tmict = apic_get_reg(apic, APIC_TMICT);
-
-		offset = APIC_BUS_CYCLE_NS * apic->timer.divide_count * tmict;
-
 		result = 1;
 		apic->timer.dev.expires = ktime_add_ns(
 					apic->timer.dev.expires,
 					apic->timer.period);
 	}
-
 	return result;
 }
 
+static int __inject_apic_timer_irq(struct kvm_lapic *apic)
+{
+	int vector;
+
+	vector = apic_lvt_vector(apic, APIC_LVTT);
+	return __apic_accept_irq(apic, APIC_DM_FIXED, vector, 1, 0);
+}
+
 static enum hrtimer_restart apic_timer_fn(struct hrtimer *data)
 {
 	struct kvm_lapic *apic;
@@ -951,6 +951,26 @@ int kvm_apic_has_interrupt(struct kvm_vcpu *vcpu)
 	return highest_irr;
 }
 
+void kvm_inject_apic_timer_irqs(struct kvm_vcpu *vcpu)
+{
+	struct kvm_lapic *apic = vcpu->apic;
+
+	if (apic && atomic_read(&apic->timer.pending) > 0) {
+		if (__inject_apic_timer_irq(apic))
+			atomic_dec(&apic->timer.pending);
+	}
+}
+
+void kvm_apic_timer_intr_post(struct kvm_vcpu *vcpu, int vec)
+{
+	struct kvm_lapic *apic = vcpu->apic;
+
+	if (apic && apic_lvt_vector(apic, APIC_LVTT) == vec)
+		apic->timer.last_update = ktime_add_ns(
+				apic->timer.last_update,
+				apic->timer.period);
+}
+
 int kvm_get_apic_interrupt(struct kvm_vcpu *vcpu)
 {
 	int vector = kvm_apic_has_interrupt(vcpu);
@@ -980,7 +1000,7 @@ void kvm_apic_post_state_restore(struct kvm_vcpu *vcpu)
 	/* TODO: following code can be in a common API */
 	spin_lock_bh(&apic->lock);
 	hrtimer_cancel(&apic->timer.dev);
-	apic->timer.pending = 0;
+	atomic_set(&apic->timer.pending, 0);
 	val = apic_get_reg(apic, APIC_TDCR);
 	tmp = ((val & 0x3) | ((val & 0x8) >> 1)) + 1;
 	apic->timer.divide_count = 0x1 << (tmp & 0x7);
diff --git a/drivers/kvm/svm.c b/drivers/kvm/svm.c
index 139499b..b080feb 100644
--- a/drivers/kvm/svm.c
+++ b/drivers/kvm/svm.c
@@ -1331,7 +1331,9 @@ static void svm_intr_assist(struct vcpu_svm *svm)
 {
 	struct vmcb *vmcb = svm->vmcb;
 	int intr_vector = -1;
+	struct kvm_vcpu *vcpu = &svm->vcpu;
 
+	kvm_inject_pending_timer_irqs(vcpu);
 	if ((vmcb->control.exit_int_info & SVM_EVTINJ_VALID) &&
 	    ((vmcb->control.exit_int_info & SVM_EVTINJ_TYPE_MASK) == 0)) {
 		intr_vector = vmcb->control.exit_int_info &
@@ -1344,7 +1346,7 @@ static void svm_intr_assist(struct vcpu_svm *svm)
 	if (vmcb->control.int_ctl & V_IRQ_MASK)
 		return;
 
-	if (!kvm_cpu_has_interrupt(&svm->vcpu))
+	if (!kvm_cpu_has_interrupt(vcpu))
 		return;
 
 	if (!(vmcb->save.rflags & X86_EFLAGS_IF) ||
@@ -1356,8 +1358,9 @@ static void svm_intr_assist(struct vcpu_svm *svm)
 		return;
 	}
 	/* Okay, we can deliver the interrupt: grab it and update PIC state. */
-	intr_vector = kvm_cpu_get_interrupt(&svm->vcpu);
+	intr_vector = kvm_cpu_get_interrupt(vcpu);
 	svm_inject_irq(svm, intr_vector);
+	kvm_timer_intr_post(vcpu, intr_vector);
 }
 
 static void kvm_reput_irq(struct vcpu_svm *svm)
diff --git a/drivers/kvm/vmx.c b/drivers/kvm/vmx.c
index aad02c5..9fa7356 100644
--- a/drivers/kvm/vmx.c
+++ b/drivers/kvm/vmx.c
@@ -2091,7 +2091,9 @@ static void vmx_intr_assist(struct kvm_vcpu *vcpu)
 {
 	u32 idtv_info_field, intr_info_field;
 	int has_ext_irq, interrupt_window_open;
+	int vector;
 
+	kvm_inject_pending_timer_irqs(vcpu);
 	has_ext_irq = kvm_cpu_has_interrupt(vcpu);
 	intr_info_field = vmcs_read32(VM_ENTRY_INTR_INFO_FIELD);
 	idtv_info_field = vmcs_read32(IDT_VECTORING_INFO_FIELD);
@@ -2121,9 +2123,11 @@ static void vmx_intr_assist(struct kvm_vcpu *vcpu)
 	interrupt_window_open =
 		((vmcs_readl(GUEST_RFLAGS) & X86_EFLAGS_IF) &&
 		 (vmcs_read32(GUEST_INTERRUPTIBILITY_INFO) & 3) == 0);
-	if (interrupt_window_open)
-		vmx_inject_irq(vcpu, kvm_cpu_get_interrupt(vcpu));
-	else
+	if (interrupt_window_open) {
+		vector = kvm_cpu_get_interrupt(vcpu);
+		vmx_inject_irq(vcpu, vector);
+		kvm_timer_intr_post(vcpu, vector);
+	} else
 		enable_irq_window(vcpu);
 }
 

[-- Attachment #5: Type: text/plain, Size: 315 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

[-- Attachment #6: Type: text/plain, Size: 186 bytes --]

_______________________________________________
kvm-devel mailing list
kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/kvm-devel

Re: add back pending timer irqs for kernel APIC timer

[Dong, Eddie]

[-- Attachment #1: Type: text/plain, Size: 594 bytes --]

Avi:
	Any comments to this patch? Seems not in yet.
	The sequence is apic-timer-TMCCT.patch, apic-timer-last5.patch and apic-timer-pending4.patch.
thx,eddie

>-----Original Message-----
>From: kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org 
>[mailto:kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org] On Behalf Of 
>Dong, Eddie
>Sent: 2007年8月14日 11:25
>To: Avi Kivity
>Cc: kvm-devel
>Subject: Re: [kvm-devel] add back pending timer irqs for 
>kernel APIC timer
>
>Here are rebased and fixed patches, please check.
>thanks, eddie
>
>

[-- Attachment #2: apic-timer-tmcct.patch --]
[-- Type: application/octet-stream, Size: 2281 bytes --]

commit 9b1a9ad955d9ac44ee6e60b4a565edf044d3dcc7
Author: root <root@vt32-pae.(none)>
Date:   Fri Aug 10 22:38:59 2007 +0800

    APIC_TMCCT is a dynamically running count which always
    need to be recalculated at the time it is read. Setting
    intermediate value is meaningless.
    
    Signed-off-by: Yaozu (Eddie) Dong <Eddie.Dong@intel.com>

diff --git a/drivers/kvm/lapic.c b/drivers/kvm/lapic.c
index d59b69f..fc53e88 100644
--- a/drivers/kvm/lapic.c
+++ b/drivers/kvm/lapic.c
@@ -445,7 +445,7 @@ static u32 apic_get_tmcct(struct kvm_lapic *apic)
 {
 	u32 counter_passed;
 	ktime_t passed, now = apic->timer.dev.base->get_time();
-	u32 tmcct = apic_get_reg(apic, APIC_TMCCT);
+	u32 tmcct = apic_get_reg(apic, APIC_TMICT);
 
 	ASSERT(apic != NULL);
 
@@ -474,9 +474,6 @@ static u32 apic_get_tmcct(struct kvm_lapic *apic)
 			} while (tmcct <= 0);
 	}
 
-	apic->timer.last_update = now;
-	apic_set_reg(apic, APIC_TMCCT, tmcct);
-
 	return tmcct;
 }
 
@@ -633,7 +630,6 @@ static void apic_mmio_write(struct kvm_io_device *this,
 			u32 offset;
 
 			apic_set_reg(apic, APIC_TMICT, val);
-			apic_set_reg(apic, APIC_TMCCT, val);
 			apic->timer.last_update = now;
 			offset =
 			    APIC_BUS_CYCLE_NS * apic->timer.divide_count * val;
@@ -815,7 +811,6 @@ static void lapic_reset(struct kvm_vcpu *vcpu)
 	apic_set_reg(apic, APIC_ICR2, 0);
 	apic_set_reg(apic, APIC_TDCR, 0);
 	apic_set_reg(apic, APIC_TMICT, 0);
-	apic_set_reg(apic, APIC_TMCCT, 0);
 	for (i = 0; i < 8; i++) {
 		apic_set_reg(apic, APIC_IRR + 0x10 * i, 0);
 		apic_set_reg(apic, APIC_ISR + 0x10 * i, 0);
@@ -886,13 +881,10 @@ static int __apic_timer_fn(struct kvm_lapic *apic)
 		u32 offset;
 		u32 tmict = apic_get_reg(apic, APIC_TMICT);
 
-		apic_set_reg(apic, APIC_TMCCT, tmict);
 		offset = APIC_BUS_CYCLE_NS * apic->timer.divide_count * tmict;
 
 		result = 1;
 		apic->timer.dev.expires = ktime_add_ns(now, offset);
-	} else {
-		apic_set_reg(apic, APIC_TMCCT, 0);
 	}
 
 	return result;
@@ -996,7 +988,6 @@ void kvm_apic_post_state_restore(struct kvm_vcpu *vcpu)
 			     MSR_IA32_APICBASE_BASE;
 	apic_set_reg(apic, APIC_LVR, APIC_VERSION);
 	val = apic_get_reg(apic, APIC_TMICT);
-	apic_set_reg(apic, APIC_TMCCT, val);
 	apic_update_ppr(apic);
 
  	/* TODO: following code can be in a common API */

[-- Attachment #3: apic-timer-last5.patch --]
[-- Type: application/octet-stream, Size: 4220 bytes --]

        In order to calculate instant APIC counter register value,
        KVM refers host time stamp of previous APIC timer firing point
        (last_update) to get the time passed and thus calculate
        current APIC time counter register. Using NOW of
        the host hrtime firing time will have accumulated difference
        since the timer callback is executed with uncertainity delay,
        and it may introduce many drift as the time goes on.
    
        This patch fix this by adjust last_update by expected period.
    
        Signed-off-by: Yaozu (Eddie) Dong <Eddie.Dong@intel.com>
diff --git a/drivers/kvm/irq.c b/drivers/kvm/irq.c
diff --git a/drivers/kvm/irq.h b/drivers/kvm/irq.h
index 44e1fa4..ed6d20a 100644
--- a/drivers/kvm/irq.h
+++ b/drivers/kvm/irq.h
@@ -111,6 +111,7 @@ struct kvm_lapic {
 	struct kvm_io_device dev;
 	struct {
 		unsigned long pending;
+		s64 period;	/* unit: ns */
 		u32 divide_count;
 		ktime_t last_update;
 		struct hrtimer dev;
diff --git a/drivers/kvm/kvm_main.c b/drivers/kvm/kvm_main.c
diff --git a/drivers/kvm/lapic.c b/drivers/kvm/lapic.c
index 1430667..09aadcb 100644
--- a/drivers/kvm/lapic.c
+++ b/drivers/kvm/lapic.c
@@ -350,18 +350,6 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
 	return result;
 }
 
-static inline int apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
-				  int vector, int level, int trig_mode)
-{
-	int result = 0;
-
-	spin_lock_bh(&apic->lock);
-	result = __apic_accept_irq(apic, delivery_mode,
-				   vector, level, trig_mode);
-	spin_unlock_bh(&apic->lock);
-	return result;
-}
-
 struct kvm_lapic *kvm_apic_round_robin(struct kvm *kvm, u8 vector,
 				       unsigned long bitmap)
 {
@@ -625,27 +613,28 @@ static void apic_mmio_write(struct kvm_io_device *this,
 	case APIC_TMICT:
 		{
 			ktime_t now = apic->timer.dev.base->get_time();
-			u32 offset;
 
 			apic_set_reg(apic, APIC_TMICT, val);
 			apic->timer.last_update = now;
-			offset =
+			apic->timer.period =
 			    APIC_BUS_CYCLE_NS * apic->timer.divide_count * val;
 
 			/* Make sure the lock ordering is coherent */
 			spin_unlock_bh(&apic->lock);
 			hrtimer_cancel(&apic->timer.dev);
 			hrtimer_start(&apic->timer.dev,
-				      ktime_add_ns(now, offset),
+				      ktime_add_ns(now, apic->timer.period),
 				      HRTIMER_MODE_ABS);
 
 			apic_debug("%s: bus cycle is %" PRId64 "ns, now 0x%016"
 				   PRIx64 ", "
-				   "timer initial count 0x%x, offset 0x%x, "
+				   "timer initial count 0x%x, period %lldns, "
 				   "expire @ 0x%016" PRIx64 ".\n", __FUNCTION__,
 				   APIC_BUS_CYCLE_NS, ktime_to_ns(now),
-				   apic_get_reg(apic, APIC_TMICT), offset,
-				   ktime_to_ns(ktime_add_ns(now, offset)));
+				   apic_get_reg(apic, APIC_TMICT),
+				   apic->timer.period,
+				   ktime_to_ns(ktime_add_ns(now,
+						apic->timer.period)));
 		}
 		return;
 
@@ -760,7 +749,7 @@ void kvm_lapic_set_base(struct kvm_vcpu *vcpu, u64 value)
 
 	/* with FSB delivery interrupt, we can restart APIC functionality */
 	apic_debug("apic base msr is 0x%016" PRIx64 ", and base address is "
-		   "0x%lx.\n", apic->apic_base, apic->base_address);
+		   "0x%lx.\n", apic->vcpu->apic_base, apic->base_address);
 
 	spin_unlock_bh(&apic->lock);
 }
@@ -859,7 +848,6 @@ EXPORT_SYMBOL_GPL(kvm_lapic_get_regs);
 static int __apic_timer_fn(struct kvm_lapic *apic)
 {
 	u32 vector;
-	ktime_t now;
 	int result = 0;
 
 	if (unlikely(!apic_enabled(apic) ||
@@ -870,8 +858,7 @@ static int __apic_timer_fn(struct kvm_lapic *apic)
 	}
 
 	vector = apic_lvt_vector(apic, APIC_LVTT);
-	now = apic->timer.dev.base->get_time();
-	apic->timer.last_update = now;
+	apic->timer.last_update = apic->timer.dev.expires;
 	apic->timer.pending++;
 	__apic_accept_irq(apic, APIC_DM_FIXED, vector, 1, 0);
 
@@ -882,7 +869,9 @@ static int __apic_timer_fn(struct kvm_lapic *apic)
 		offset = APIC_BUS_CYCLE_NS * apic->timer.divide_count * tmict;
 
 		result = 1;
-		apic->timer.dev.expires = ktime_add_ns(now, offset);
+		apic->timer.dev.expires = ktime_add_ns(
+					apic->timer.dev.expires,
+					apic->timer.period);
 	}
 
 	return result;
diff --git a/drivers/kvm/svm.c b/drivers/kvm/svm.c
diff --git a/drivers/kvm/vmx.c b/drivers/kvm/vmx.c

[-- Attachment #4: apic-timer-pending4.patch --]
[-- Type: application/octet-stream, Size: 9108 bytes --]

commit f1c8d1076b48625469b69abd6767bf429f35120d
Author: root <root@vt32-pae.(none)>
Date:   Tue Aug 14 11:22:47 2007 +0800

        APIC timer IRQ is set every time when a certain period
        expires at host time, but the guest may be descheduled
        at that time and thus the irq be overwritten by later fire.
        This patch keep track of firing irq numbers and decrease
        only when the IRQ is injected to guest or buffered in
        APIC.
    
        Signed-off-by: Yaozu (Eddie) Dong <Eddie.Dong@intel.com>

diff --git a/drivers/kvm/irq.c b/drivers/kvm/irq.c
index e09cd65..eb72b56 100644
--- a/drivers/kvm/irq.c
+++ b/drivers/kvm/irq.c
@@ -78,3 +78,17 @@ void kvm_vcpu_kick(struct kvm_vcpu *vcpu)
 		smp_call_function_single(ipi_pcpu, vcpu_kick_intr, vcpu, 0, 0);
 }
 
+void kvm_inject_pending_timer_irqs(struct kvm_vcpu *vcpu)
+{
+	kvm_inject_apic_timer_irqs(vcpu);
+	/* TODO: PIT, RTC etc. */
+}
+EXPORT_SYMBOL_GPL(kvm_inject_pending_timer_irqs);
+
+void kvm_timer_intr_post(struct kvm_vcpu *vcpu, int vec)
+{
+	kvm_apic_timer_intr_post(vcpu, vec);
+	/* TODO: PIT, RTC etc. */
+}
+EXPORT_SYMBOL_GPL(kvm_timer_intr_post);
+
diff --git a/drivers/kvm/irq.h b/drivers/kvm/irq.h
index ed6d20a..419004e 100644
--- a/drivers/kvm/irq.h
+++ b/drivers/kvm/irq.h
@@ -110,7 +110,7 @@ struct kvm_lapic {
 	unsigned long base_address;
 	struct kvm_io_device dev;
 	struct {
-		unsigned long pending;
+		atomic_t pending;
 		s64 period;	/* unit: ns */
 		u32 divide_count;
 		ktime_t last_update;
@@ -153,5 +153,9 @@ int kvm_apic_set_irq(struct kvm_lapic *apic, u8 vec, u8 trig);
 void kvm_apic_post_state_restore(struct kvm_vcpu *vcpu);
 int kvm_ioapic_init(struct kvm *kvm);
 void kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int irq, int level);
+void kvm_apic_timer_intr_post(struct kvm_vcpu *vcpu, int vec);
+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);
 
 #endif
diff --git a/drivers/kvm/kvm_main.c b/drivers/kvm/kvm_main.c
index c59e761..bb3fd24 100644
--- a/drivers/kvm/kvm_main.c
+++ b/drivers/kvm/kvm_main.c
@@ -283,6 +283,8 @@ EXPORT_SYMBOL_GPL(kvm_vcpu_init);
 void kvm_vcpu_uninit(struct kvm_vcpu *vcpu)
 {
 	kvm_mmu_destroy(vcpu);
+	if (vcpu->apic)
+		hrtimer_cancel(&vcpu->apic->timer.dev);
 	kvm_free_apic(vcpu->apic);
 	free_page((unsigned long)vcpu->pio_data);
 	free_page((unsigned long)vcpu->run);
diff --git a/drivers/kvm/lapic.c b/drivers/kvm/lapic.c
index 09aadcb..a6fec50 100644
--- a/drivers/kvm/lapic.c
+++ b/drivers/kvm/lapic.c
@@ -30,6 +30,7 @@
 #include <asm/page.h>
 #include <asm/current.h>
 #include <asm/apicdef.h>
+#include <asm/atomic.h>
 #include "irq.h"
 
 #define PRId64 "d"
@@ -298,6 +299,7 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
 			     int vector, int level, int trig_mode)
 {
 	int result = 0;
+	int orig_irr;
 
 	switch (delivery_mode) {
 	case APIC_DM_FIXED:
@@ -306,7 +308,8 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
 		if (unlikely(!apic_enabled(apic)))
 			break;
 
-		if (apic_test_and_set_irr(vector, apic) && trig_mode) {
+		orig_irr = apic_test_and_set_irr(vector, apic);
+		if (orig_irr && trig_mode) {
 			apic_debug("level trig mode repeatedly for vector %d",
 				   vector);
 			break;
@@ -320,7 +323,7 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
 
 		kvm_vcpu_kick(apic->vcpu);
 
-		result = 1;
+		result = (orig_irr == 0);
 		break;
 
 	case APIC_DM_REMRD:
@@ -581,6 +584,7 @@ static void apic_mmio_write(struct kvm_io_device *this,
 				apic_set_reg(apic, APIC_LVTT + 0x10 * i,
 					     lvt_val | APIC_LVT_MASKED);
 			}
+			atomic_set(&apic->timer.pending, 0);
 
 		}
 		break;
@@ -622,6 +626,7 @@ static void apic_mmio_write(struct kvm_io_device *this,
 			/* Make sure the lock ordering is coherent */
 			spin_unlock_bh(&apic->lock);
 			hrtimer_cancel(&apic->timer.dev);
+			atomic_set(&apic->timer.pending, 0);
 			hrtimer_start(&apic->timer.dev,
 				      ktime_add_ns(now, apic->timer.period),
 				      HRTIMER_MODE_ABS);
@@ -804,7 +809,7 @@ static void lapic_reset(struct kvm_vcpu *vcpu)
 		apic_set_reg(apic, APIC_TMR + 0x10 * i, 0);
 	}
 	apic->timer.divide_count = 0;
-	apic->timer.pending = 0;
+	atomic_set(&apic->timer.pending, 0);
 	if (vcpu->vcpu_id == 0)
 		vcpu->apic_base |= MSR_IA32_APICBASE_BSP;
 	apic_update_ppr(apic);
@@ -845,38 +850,33 @@ EXPORT_SYMBOL_GPL(kvm_lapic_get_regs);
  * timer interface
  *----------------------------------------------------------------------
  */
+
+/* TODO: make sure __apic_timer_fn runs in current pCPU */
 static int __apic_timer_fn(struct kvm_lapic *apic)
 {
-	u32 vector;
 	int result = 0;
+	wait_queue_head_t *q = &apic->vcpu->wq;
 
-	if (unlikely(!apic_enabled(apic) ||
-		     !apic_lvt_enabled(apic, APIC_LVTT))) {
-		apic_debug("%s: time interrupt although apic is down\n",
-			   __FUNCTION__);
-		return 0;
-	}
-
-	vector = apic_lvt_vector(apic, APIC_LVTT);
-	apic->timer.last_update = apic->timer.dev.expires;
-	apic->timer.pending++;
-	__apic_accept_irq(apic, APIC_DM_FIXED, vector, 1, 0);
-
+	atomic_inc(&apic->timer.pending);
+	if (waitqueue_active(q))
+		wake_up_interruptible(q);
 	if (apic_lvtt_period(apic)) {
-		u32 offset;
-		u32 tmict = apic_get_reg(apic, APIC_TMICT);
-
-		offset = APIC_BUS_CYCLE_NS * apic->timer.divide_count * tmict;
-
 		result = 1;
 		apic->timer.dev.expires = ktime_add_ns(
 					apic->timer.dev.expires,
 					apic->timer.period);
 	}
-
 	return result;
 }
 
+static int __inject_apic_timer_irq(struct kvm_lapic *apic)
+{
+	int vector;
+
+	vector = apic_lvt_vector(apic, APIC_LVTT);
+	return __apic_accept_irq(apic, APIC_DM_FIXED, vector, 1, 0);
+}
+
 static enum hrtimer_restart apic_timer_fn(struct hrtimer *data)
 {
 	struct kvm_lapic *apic;
@@ -951,6 +951,26 @@ int kvm_apic_has_interrupt(struct kvm_vcpu *vcpu)
 	return highest_irr;
 }
 
+void kvm_inject_apic_timer_irqs(struct kvm_vcpu *vcpu)
+{
+	struct kvm_lapic *apic = vcpu->apic;
+
+	if (apic && atomic_read(&apic->timer.pending) > 0) {
+		if (__inject_apic_timer_irq(apic))
+			atomic_dec(&apic->timer.pending);
+	}
+}
+
+void kvm_apic_timer_intr_post(struct kvm_vcpu *vcpu, int vec)
+{
+	struct kvm_lapic *apic = vcpu->apic;
+
+	if (apic && apic_lvt_vector(apic, APIC_LVTT) == vec)
+		apic->timer.last_update = ktime_add_ns(
+				apic->timer.last_update,
+				apic->timer.period);
+}
+
 int kvm_get_apic_interrupt(struct kvm_vcpu *vcpu)
 {
 	int vector = kvm_apic_has_interrupt(vcpu);
@@ -980,7 +1000,7 @@ void kvm_apic_post_state_restore(struct kvm_vcpu *vcpu)
 	/* TODO: following code can be in a common API */
 	spin_lock_bh(&apic->lock);
 	hrtimer_cancel(&apic->timer.dev);
-	apic->timer.pending = 0;
+	atomic_set(&apic->timer.pending, 0);
 	val = apic_get_reg(apic, APIC_TDCR);
 	tmp = ((val & 0x3) | ((val & 0x8) >> 1)) + 1;
 	apic->timer.divide_count = 0x1 << (tmp & 0x7);
diff --git a/drivers/kvm/svm.c b/drivers/kvm/svm.c
index 139499b..b080feb 100644
--- a/drivers/kvm/svm.c
+++ b/drivers/kvm/svm.c
@@ -1331,7 +1331,9 @@ static void svm_intr_assist(struct vcpu_svm *svm)
 {
 	struct vmcb *vmcb = svm->vmcb;
 	int intr_vector = -1;
+	struct kvm_vcpu *vcpu = &svm->vcpu;
 
+	kvm_inject_pending_timer_irqs(vcpu);
 	if ((vmcb->control.exit_int_info & SVM_EVTINJ_VALID) &&
 	    ((vmcb->control.exit_int_info & SVM_EVTINJ_TYPE_MASK) == 0)) {
 		intr_vector = vmcb->control.exit_int_info &
@@ -1344,7 +1346,7 @@ static void svm_intr_assist(struct vcpu_svm *svm)
 	if (vmcb->control.int_ctl & V_IRQ_MASK)
 		return;
 
-	if (!kvm_cpu_has_interrupt(&svm->vcpu))
+	if (!kvm_cpu_has_interrupt(vcpu))
 		return;
 
 	if (!(vmcb->save.rflags & X86_EFLAGS_IF) ||
@@ -1356,8 +1358,9 @@ static void svm_intr_assist(struct vcpu_svm *svm)
 		return;
 	}
 	/* Okay, we can deliver the interrupt: grab it and update PIC state. */
-	intr_vector = kvm_cpu_get_interrupt(&svm->vcpu);
+	intr_vector = kvm_cpu_get_interrupt(vcpu);
 	svm_inject_irq(svm, intr_vector);
+	kvm_timer_intr_post(vcpu, intr_vector);
 }
 
 static void kvm_reput_irq(struct vcpu_svm *svm)
diff --git a/drivers/kvm/vmx.c b/drivers/kvm/vmx.c
index aad02c5..9fa7356 100644
--- a/drivers/kvm/vmx.c
+++ b/drivers/kvm/vmx.c
@@ -2091,7 +2091,9 @@ static void vmx_intr_assist(struct kvm_vcpu *vcpu)
 {
 	u32 idtv_info_field, intr_info_field;
 	int has_ext_irq, interrupt_window_open;
+	int vector;
 
+	kvm_inject_pending_timer_irqs(vcpu);
 	has_ext_irq = kvm_cpu_has_interrupt(vcpu);
 	intr_info_field = vmcs_read32(VM_ENTRY_INTR_INFO_FIELD);
 	idtv_info_field = vmcs_read32(IDT_VECTORING_INFO_FIELD);
@@ -2121,9 +2123,11 @@ static void vmx_intr_assist(struct kvm_vcpu *vcpu)
 	interrupt_window_open =
 		((vmcs_readl(GUEST_RFLAGS) & X86_EFLAGS_IF) &&
 		 (vmcs_read32(GUEST_INTERRUPTIBILITY_INFO) & 3) == 0);
-	if (interrupt_window_open)
-		vmx_inject_irq(vcpu, kvm_cpu_get_interrupt(vcpu));
-	else
+	if (interrupt_window_open) {
+		vector = kvm_cpu_get_interrupt(vcpu);
+		vmx_inject_irq(vcpu, vector);
+		kvm_timer_intr_post(vcpu, vector);
+	} else
 		enable_irq_window(vcpu);
 }
 

[-- Attachment #5: Type: text/plain, Size: 315 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

[-- Attachment #6: Type: text/plain, Size: 186 bytes --]

_______________________________________________
kvm-devel mailing list
kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/kvm-devel

Re: add back pending timer irqs for kernel APIC timer

[Avi Kivity]

[-- Attachment #1: Type: text/plain, Size: 295 bytes --]

Dong, Eddie wrote:
> Avi:
> 	Any comments to this patch? Seems not in yet.
> 	The sequence is apic-timer-TMCCT.patch, apic-timer-last5.patch and apic-timer-pending4.patch.
> thx,eddie
>   

I'd like to get guest smp in first...

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



[-- Attachment #2: Type: text/plain, Size: 315 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

[-- Attachment #3: Type: text/plain, Size: 186 bytes --]

_______________________________________________
kvm-devel mailing list
kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/kvm-devel

Re: add back pending timer irqs for kernel APIC timer

[Dong, Eddie]

Avi Kivity wrote:
> Dong, Eddie wrote:
>> Avi:
>> 	Any comments to this patch? Seems not in yet.
>> 	The sequence is apic-timer-TMCCT.patch,
> apic-timer-last5.patch and apic-timer-pending4.patch.
>> thx,eddie
>> 
> 
> I'd like to get guest smp in first...
> 

Avi:
	We have got SMP Linux up too, which is based on top of these
patches, can u check in these to make next series simple?
thx,eddie

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: add back pending timer irqs for kernel APIC timer

[Dong, Eddie]

[-- Attachment #1: Type: text/plain, Size: 152 bytes --]

Avi:
Here is the rebased one toward time issue fix and lock removal. The
comming SMP serial patches are based on APIC lock removal.

thanks, eddie

[-- Attachment #2: 1-tmcct.patch --]
[-- Type: application/octet-stream, Size: 2158 bytes --]

    APIC_TMCCT is a dynamically running count which always
    need to be recalculated at the time it is read. Setting
    intermediate value is meaningless.
    
    Signed-off-by: Yaozu (Eddie) Dong <Eddie.Dong@intel.com>

diff --git a/drivers/kvm/lapic.c b/drivers/kvm/lapic.c
index b2b6dec..1f6e3a6 100644
--- a/drivers/kvm/lapic.c
+++ b/drivers/kvm/lapic.c
@@ -463,7 +463,7 @@ static u32 apic_get_tmcct(struct kvm_lapic *apic)
 {
 	u32 counter_passed;
 	ktime_t passed, now = apic->timer.dev.base->get_time();
-	u32 tmcct = apic_get_reg(apic, APIC_TMCCT);
+	u32 tmcct = apic_get_reg(apic, APIC_TMICT);
 
 	ASSERT(apic != NULL);
 
@@ -492,9 +492,6 @@ static u32 apic_get_tmcct(struct kvm_lapic *apic)
 			} while (tmcct <= 0);
 	}
 
-	apic->timer.last_update = now;
-	apic_set_reg(apic, APIC_TMCCT, tmcct);
-
 	return tmcct;
 }
 
@@ -652,7 +649,6 @@ static void apic_mmio_write(struct kvm_io_device *this,
 			u32 offset;
 
 			apic_set_reg(apic, APIC_TMICT, val);
-			apic_set_reg(apic, APIC_TMCCT, val);
 			apic->timer.last_update = now;
 			offset =
 			    APIC_BUS_CYCLE_NS * apic->timer.divide_count * val;
@@ -835,7 +831,6 @@ static void lapic_reset(struct kvm_vcpu *vcpu)
 	apic_set_reg(apic, APIC_ICR2, 0);
 	apic_set_reg(apic, APIC_TDCR, 0);
 	apic_set_reg(apic, APIC_TMICT, 0);
-	apic_set_reg(apic, APIC_TMCCT, 0);
 	for (i = 0; i < 8; i++) {
 		apic_set_reg(apic, APIC_IRR + 0x10 * i, 0);
 		apic_set_reg(apic, APIC_ISR + 0x10 * i, 0);
@@ -907,13 +902,10 @@ static int __apic_timer_fn(struct kvm_lapic *apic)
 		u32 offset;
 		u32 tmict = apic_get_reg(apic, APIC_TMICT);
 
-		apic_set_reg(apic, APIC_TMCCT, tmict);
 		offset = APIC_BUS_CYCLE_NS * apic->timer.divide_count * tmict;
 
 		result = 1;
 		apic->timer.dev.expires = ktime_add_ns(now, offset);
-	} else {
-		apic_set_reg(apic, APIC_TMCCT, 0);
 	}
 
 	return result;
@@ -1020,7 +1012,6 @@ void kvm_apic_post_state_restore(struct kvm_vcpu *vcpu)
 			     MSR_IA32_APICBASE_BASE;
 	apic_set_reg(apic, APIC_LVR, APIC_VERSION);
 	val = apic_get_reg(apic, APIC_TMICT);
-	apic_set_reg(apic, APIC_TMCCT, val);
 	apic_update_ppr(apic);
 
 	/* TODO: following code can be in a common API */

[-- Attachment #3: 2-last.patch --]
[-- Type: application/octet-stream, Size: 3631 bytes --]

        In order to calculate instant APIC counter register value,
        KVM refers host time stamp of previous APIC timer firing point
        (last_update) to get the time passed and thus calculate
        current APIC time counter register. Using NOW of
        the host hrtime firing time will have accumulated difference
        since the timer callback is executed with uncertainity delay,
        and it may introduce many drift as the time goes on.
    
        This patch fix this by adjust last_update by expected period.
    
        Signed-off-by: Yaozu (Eddie) Dong <Eddie.Dong@intel.com>

diff --git a/drivers/kvm/irq.h b/drivers/kvm/irq.h
index 7f4f20e..80cefce 100644
--- a/drivers/kvm/irq.h
+++ b/drivers/kvm/irq.h
@@ -111,6 +111,7 @@ struct kvm_lapic {
 	struct kvm_io_device dev;
 	struct {
 		unsigned long pending;
+		s64 period;	/* unit: ns */
 		u32 divide_count;
 		ktime_t last_update;
 		struct hrtimer dev;
diff --git a/drivers/kvm/lapic.c b/drivers/kvm/lapic.c
index 1f6e3a6..06eec82 100644
--- a/drivers/kvm/lapic.c
+++ b/drivers/kvm/lapic.c
@@ -370,18 +370,6 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
 	return result;
 }
 
-static inline int apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
-				  int vector, int level, int trig_mode)
-{
-	int result = 0;
-
-	spin_lock_bh(&apic->lock);
-	result = __apic_accept_irq(apic, delivery_mode,
-				   vector, level, trig_mode);
-	spin_unlock_bh(&apic->lock);
-	return result;
-}
-
 struct kvm_lapic *kvm_apic_round_robin(struct kvm *kvm, u8 vector,
 				       unsigned long bitmap)
 {
@@ -646,27 +634,28 @@ static void apic_mmio_write(struct kvm_io_device *this,
 	case APIC_TMICT:
 		{
 			ktime_t now = apic->timer.dev.base->get_time();
-			u32 offset;
 
 			apic_set_reg(apic, APIC_TMICT, val);
 			apic->timer.last_update = now;
-			offset =
+			apic->timer.period =
 			    APIC_BUS_CYCLE_NS * apic->timer.divide_count * val;
 
 			/* Make sure the lock ordering is coherent */
 			spin_unlock_bh(&apic->lock);
 			hrtimer_cancel(&apic->timer.dev);
 			hrtimer_start(&apic->timer.dev,
-				      ktime_add_ns(now, offset),
+				      ktime_add_ns(now, apic->timer.period),
 				      HRTIMER_MODE_ABS);
 
 			apic_debug("%s: bus cycle is %" PRId64 "ns, now 0x%016"
 				   PRIx64 ", "
-				   "timer initial count 0x%x, offset 0x%x, "
+				   "timer initial count 0x%x, period %lldns, "
 				   "expire @ 0x%016" PRIx64 ".\n", __FUNCTION__,
 				   APIC_BUS_CYCLE_NS, ktime_to_ns(now),
-				   apic_get_reg(apic, APIC_TMICT), offset,
-				   ktime_to_ns(ktime_add_ns(now, offset)));
+				   apic_get_reg(apic, APIC_TMICT),
+				   apic->timer.period,
+				   ktime_to_ns(ktime_add_ns(now,
+						apic->timer.period)));
 		}
 		return;
 
@@ -882,7 +871,6 @@ EXPORT_SYMBOL_GPL(kvm_lapic_get_regs);
 static int __apic_timer_fn(struct kvm_lapic *apic)
 {
 	u32 vector;
-	ktime_t now;
 	int result = 0;
 
 	if (unlikely(!apic_enabled(apic) ||
@@ -893,8 +881,7 @@ static int __apic_timer_fn(struct kvm_lapic *apic)
 	}
 
 	vector = apic_lvt_vector(apic, APIC_LVTT);
-	now = apic->timer.dev.base->get_time();
-	apic->timer.last_update = now;
+	apic->timer.last_update = apic->timer.dev.expires;
 	apic->timer.pending++;
 	__apic_accept_irq(apic, APIC_DM_FIXED, vector, 1, 0);
 
@@ -905,7 +892,9 @@ static int __apic_timer_fn(struct kvm_lapic *apic)
 		offset = APIC_BUS_CYCLE_NS * apic->timer.divide_count * tmict;
 
 		result = 1;
-		apic->timer.dev.expires = ktime_add_ns(now, offset);
+		apic->timer.dev.expires = ktime_add_ns(
+					apic->timer.dev.expires,
+					apic->timer.period);
 	}
 
 	return result;

[-- Attachment #4: 3-pending.patch --]
[-- Type: application/octet-stream, Size: 9180 bytes --]

        APIC timer IRQ is set every time when a certain period
        expires at host time, but the guest may be descheduled
        at that time and thus the irq be overwritten by later fire.
        This patch keep track of firing irq numbers and decrease
        only when the IRQ is injected to guest or buffered in
        APIC.
    
        Signed-off-by: Yaozu (Eddie) Dong <Eddie.Dong@intel.com>

diff --git a/drivers/kvm/irq.c b/drivers/kvm/irq.c
index e1a39fc..ab2bc72 100644
--- a/drivers/kvm/irq.c
+++ b/drivers/kvm/irq.c
@@ -86,3 +86,17 @@ void kvm_vcpu_kick(struct kvm_vcpu *vcpu)
 		smp_call_function_single(ipi_pcpu, vcpu_kick_intr, vcpu, 0, 0);
 }
 
+void kvm_inject_pending_timer_irqs(struct kvm_vcpu *vcpu)
+{
+	kvm_inject_apic_timer_irqs(vcpu);
+	/* TODO: PIT, RTC etc. */
+}
+EXPORT_SYMBOL_GPL(kvm_inject_pending_timer_irqs);
+
+void kvm_timer_intr_post(struct kvm_vcpu *vcpu, int vec)
+{
+	kvm_apic_timer_intr_post(vcpu, vec);
+	/* TODO: PIT, RTC etc. */
+}
+EXPORT_SYMBOL_GPL(kvm_timer_intr_post);
+
diff --git a/drivers/kvm/irq.h b/drivers/kvm/irq.h
index 80cefce..b6283d2 100644
--- a/drivers/kvm/irq.h
+++ b/drivers/kvm/irq.h
@@ -110,7 +110,7 @@ struct kvm_lapic {
 	unsigned long base_address;
 	struct kvm_io_device dev;
 	struct {
-		unsigned long pending;
+		atomic_t pending;
 		s64 period;	/* unit: ns */
 		u32 divide_count;
 		ktime_t last_update;
@@ -155,5 +155,9 @@ int kvm_ioapic_init(struct kvm *kvm);
 void kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int irq, int level);
 int kvm_lapic_enabled(struct kvm_vcpu *vcpu);
 int kvm_lapic_find_highest_irr(struct kvm_vcpu *vcpu);
+void kvm_apic_timer_intr_post(struct kvm_vcpu *vcpu, int vec);
+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);
 
 #endif
diff --git a/drivers/kvm/kvm_main.c b/drivers/kvm/kvm_main.c
index b627b42..9667cfc 100644
--- a/drivers/kvm/kvm_main.c
+++ b/drivers/kvm/kvm_main.c
@@ -284,6 +284,8 @@ EXPORT_SYMBOL_GPL(kvm_vcpu_init);
 void kvm_vcpu_uninit(struct kvm_vcpu *vcpu)
 {
 	kvm_mmu_destroy(vcpu);
+	if (vcpu->apic)
+		hrtimer_cancel(&vcpu->apic->timer.dev);
 	kvm_free_apic(vcpu->apic);
 	free_page((unsigned long)vcpu->pio_data);
 	free_page((unsigned long)vcpu->run);
@@ -1404,8 +1406,6 @@ EXPORT_SYMBOL_GPL(kvm_vcpu_kernel_halt);
 
 int kvm_emulate_halt(struct kvm_vcpu *vcpu)
 {
-	static int last_halt = -1;
-
 	++vcpu->stat.halt_exits;
 	if (irqchip_in_kernel(vcpu->kvm)) {
 		kvm_vcpu_kernel_halt(vcpu);
diff --git a/drivers/kvm/lapic.c b/drivers/kvm/lapic.c
index 06eec82..751ec03 100644
--- a/drivers/kvm/lapic.c
+++ b/drivers/kvm/lapic.c
@@ -30,6 +30,7 @@
 #include <asm/page.h>
 #include <asm/current.h>
 #include <asm/apicdef.h>
+#include <asm/atomic.h>
 #include "irq.h"
 
 #define PRId64 "d"
@@ -313,6 +314,7 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
 			     int vector, int level, int trig_mode)
 {
 	int result = 0;
+	int orig_irr;
 
 	switch (delivery_mode) {
 	case APIC_DM_FIXED:
@@ -321,7 +323,8 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
 		if (unlikely(!apic_enabled(apic)))
 			break;
 
-		if (apic_test_and_set_irr(vector, apic) && trig_mode) {
+		orig_irr = apic_test_and_set_irr(vector, apic);
+		if (orig_irr && trig_mode) {
 			apic_debug("level trig mode repeatedly for vector %d",
 				   vector);
 			break;
@@ -335,7 +338,7 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
 
 		kvm_vcpu_kick(apic->vcpu);
 
-		result = 1;
+		result = (orig_irr == 0);
 		break;
 
 	case APIC_DM_REMRD:
@@ -602,6 +605,7 @@ static void apic_mmio_write(struct kvm_io_device *this,
 				apic_set_reg(apic, APIC_LVTT + 0x10 * i,
 					     lvt_val | APIC_LVT_MASKED);
 			}
+			atomic_set(&apic->timer.pending, 0);
 
 		}
 		break;
@@ -643,6 +647,7 @@ static void apic_mmio_write(struct kvm_io_device *this,
 			/* Make sure the lock ordering is coherent */
 			spin_unlock_bh(&apic->lock);
 			hrtimer_cancel(&apic->timer.dev);
+			atomic_set(&apic->timer.pending, 0);
 			hrtimer_start(&apic->timer.dev,
 				      ktime_add_ns(now, apic->timer.period),
 				      HRTIMER_MODE_ABS);
@@ -826,7 +831,7 @@ static void lapic_reset(struct kvm_vcpu *vcpu)
 		apic_set_reg(apic, APIC_TMR + 0x10 * i, 0);
 	}
 	apic->timer.divide_count = 0;
-	apic->timer.pending = 0;
+	atomic_set(&apic->timer.pending, 0);
 	if (vcpu->vcpu_id == 0)
 		vcpu->apic_base |= MSR_IA32_APICBASE_BSP;
 	apic_update_ppr(apic);
@@ -868,38 +873,33 @@ EXPORT_SYMBOL_GPL(kvm_lapic_get_regs);
  * timer interface
  *----------------------------------------------------------------------
  */
+
+/* TODO: make sure __apic_timer_fn runs in current pCPU */
 static int __apic_timer_fn(struct kvm_lapic *apic)
 {
-	u32 vector;
 	int result = 0;
+	wait_queue_head_t *q = &apic->vcpu->wq;
 
-	if (unlikely(!apic_enabled(apic) ||
-		     !apic_lvt_enabled(apic, APIC_LVTT))) {
-		apic_debug("%s: time interrupt although apic is down\n",
-			   __FUNCTION__);
-		return 0;
-	}
-
-	vector = apic_lvt_vector(apic, APIC_LVTT);
-	apic->timer.last_update = apic->timer.dev.expires;
-	apic->timer.pending++;
-	__apic_accept_irq(apic, APIC_DM_FIXED, vector, 1, 0);
-
+	atomic_inc(&apic->timer.pending);
+	if (waitqueue_active(q))
+		wake_up_interruptible(q);
 	if (apic_lvtt_period(apic)) {
-		u32 offset;
-		u32 tmict = apic_get_reg(apic, APIC_TMICT);
-
-		offset = APIC_BUS_CYCLE_NS * apic->timer.divide_count * tmict;
-
 		result = 1;
 		apic->timer.dev.expires = ktime_add_ns(
 					apic->timer.dev.expires,
 					apic->timer.period);
 	}
-
 	return result;
 }
 
+static int __inject_apic_timer_irq(struct kvm_lapic *apic)
+{
+	int vector;
+
+	vector = apic_lvt_vector(apic, APIC_LVTT);
+	return __apic_accept_irq(apic, APIC_DM_FIXED, vector, 1, 0);
+}
+
 static enum hrtimer_restart apic_timer_fn(struct hrtimer *data)
 {
 	struct kvm_lapic *apic;
@@ -977,6 +977,26 @@ int kvm_apic_has_interrupt(struct kvm_vcpu *vcpu)
 	return highest_irr;
 }
 
+void kvm_inject_apic_timer_irqs(struct kvm_vcpu *vcpu)
+{
+	struct kvm_lapic *apic = vcpu->apic;
+
+	if (apic && atomic_read(&apic->timer.pending) > 0) {
+		if (__inject_apic_timer_irq(apic))
+			atomic_dec(&apic->timer.pending);
+	}
+}
+
+void kvm_apic_timer_intr_post(struct kvm_vcpu *vcpu, int vec)
+{
+	struct kvm_lapic *apic = vcpu->apic;
+
+	if (apic && apic_lvt_vector(apic, APIC_LVTT) == vec)
+		apic->timer.last_update = ktime_add_ns(
+				apic->timer.last_update,
+				apic->timer.period);
+}
+
 int kvm_get_apic_interrupt(struct kvm_vcpu *vcpu)
 {
 	int vector = kvm_apic_has_interrupt(vcpu);
@@ -1006,7 +1026,7 @@ void kvm_apic_post_state_restore(struct kvm_vcpu *vcpu)
 	/* TODO: following code can be in a common API */
 	spin_lock_bh(&apic->lock);
 	hrtimer_cancel(&apic->timer.dev);
-	apic->timer.pending = 0;
+	atomic_set(&apic->timer.pending, 0);
 	val = apic_get_reg(apic, APIC_TDCR);
 	tmp = ((val & 0x3) | ((val & 0x8) >> 1)) + 1;
 	apic->timer.divide_count = 0x1 << (tmp & 0x7);
diff --git a/drivers/kvm/svm.c b/drivers/kvm/svm.c
index b977c70..c64fe6d 100644
--- a/drivers/kvm/svm.c
+++ b/drivers/kvm/svm.c
@@ -1331,7 +1331,9 @@ static void svm_intr_assist(struct vcpu_svm *svm)
 {
 	struct vmcb *vmcb = svm->vmcb;
 	int intr_vector = -1;
+	struct kvm_vcpu *vcpu = &svm->vcpu;
 
+	kvm_inject_pending_timer_irqs(vcpu);
 	if ((vmcb->control.exit_int_info & SVM_EVTINJ_VALID) &&
 	    ((vmcb->control.exit_int_info & SVM_EVTINJ_TYPE_MASK) == 0)) {
 		intr_vector = vmcb->control.exit_int_info &
@@ -1344,7 +1346,7 @@ static void svm_intr_assist(struct vcpu_svm *svm)
 	if (vmcb->control.int_ctl & V_IRQ_MASK)
 		return;
 
-	if (!kvm_cpu_has_interrupt(&svm->vcpu))
+	if (!kvm_cpu_has_interrupt(vcpu))
 		return;
 
 	if (!(vmcb->save.rflags & X86_EFLAGS_IF) ||
@@ -1356,8 +1358,9 @@ static void svm_intr_assist(struct vcpu_svm *svm)
 		return;
 	}
 	/* Okay, we can deliver the interrupt: grab it and update PIC state. */
-	intr_vector = kvm_cpu_get_interrupt(&svm->vcpu);
+	intr_vector = kvm_cpu_get_interrupt(vcpu);
 	svm_inject_irq(svm, intr_vector);
+	kvm_timer_intr_post(vcpu, intr_vector);
 }
 
 static void kvm_reput_irq(struct vcpu_svm *svm)
diff --git a/drivers/kvm/vmx.c b/drivers/kvm/vmx.c
index 3ca4cff..b537104 100644
--- a/drivers/kvm/vmx.c
+++ b/drivers/kvm/vmx.c
@@ -2151,8 +2151,10 @@ static void vmx_intr_assist(struct kvm_vcpu *vcpu)
 {
 	u32 idtv_info_field, intr_info_field;
 	int has_ext_irq, interrupt_window_open;
+	int vector;
 
 	update_tpr_threshold(vcpu);
+	kvm_inject_pending_timer_irqs(vcpu);
 
 	has_ext_irq = kvm_cpu_has_interrupt(vcpu);
 	intr_info_field = vmcs_read32(VM_ENTRY_INTR_INFO_FIELD);
@@ -2183,9 +2185,11 @@ static void vmx_intr_assist(struct kvm_vcpu *vcpu)
 	interrupt_window_open =
 		((vmcs_readl(GUEST_RFLAGS) & X86_EFLAGS_IF) &&
 		 (vmcs_read32(GUEST_INTERRUPTIBILITY_INFO) & 3) == 0);
-	if (interrupt_window_open)
-		vmx_inject_irq(vcpu, kvm_cpu_get_interrupt(vcpu));
-	else
+	if (interrupt_window_open) {
+		vector = kvm_cpu_get_interrupt(vcpu);
+		vmx_inject_irq(vcpu, vector);
+		kvm_timer_intr_post(vcpu, vector);
+	} else
 		enable_irq_window(vcpu);
 }
 

[-- Attachment #5: Type: text/plain, Size: 315 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

[-- Attachment #6: Type: text/plain, Size: 186 bytes --]

_______________________________________________
kvm-devel mailing list
kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/kvm-devel

Re: add back pending timer irqs for kernel APIC timer

[Avi Kivity]

Dong, Eddie wrote:
> Avi Kivity wrote:
>   
>> Dong, Eddie wrote:
>>     
>>> Avi:
>>> 	Any comments to this patch? Seems not in yet.
>>> 	The sequence is apic-timer-TMCCT.patch,
>>>       
>> apic-timer-last5.patch and apic-timer-pending4.patch.
>>     
>>> thx,eddie
>>>
>>>       
>> I'd like to get guest smp in first...
>>
>>     
>
> Avi:
> 	We have got SMP Linux up too, which is based on top of these
> patches, can u check in these to make next series simple?
> thx,eddie
>   

I have issues with the lock removal patches on which the last pending
timer patches were based.  If you like, send a version which does not
depend on apic lock removal, or we can wait until the issues are resolved.

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: add back pending timer irqs for kernel APIC timer

[Dong, Eddie]

Avi Kivity wrote:
> Dong, Eddie wrote:
>> Avi Kivity wrote:
>> 
>>> Dong, Eddie wrote:
>>> 
>>>> Avi:
>>>> 	Any comments to this patch? Seems not in yet.
>>>> 	The sequence is apic-timer-TMCCT.patch,
>>>> 
>>> apic-timer-last5.patch and apic-timer-pending4.patch.
>>> 
>>>> thx,eddie
>>>> 
>>>> 
>>> I'd like to get guest smp in first...
>>> 
>>> 
>> 
>> Avi:
>> 	We have got SMP Linux up too, which is based on top of these
>> patches, can u check in these to make next series simple? thx,eddie
>> 
> 
> I have issues with the lock removal patches on which the last pending
> timer patches were based.  If you like, send a version which does not
> depend on apic lock removal, or we can wait until the issues
> are resolved.
> 
These 3 patches are actually independent. Previously you say you want
to see SMP guest first. Now Xin got it up, and this one works fine.
(Xin is based on this since very beginning we suspect Linux AP is
blocked
by the widely used lock.) So, I think we can check in this first to
avoid rebase.
The lock removal thread can be continuously discussed.
thx,eddie

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: add back pending timer irqs for kernel APIC timer

[Avi Kivity]

Dong, Eddie wrote:
>
> These 3 patches are actually independent. Previously you say you want
> to see SMP guest first. Now Xin got it up, and this one works fine.
> (Xin is based on this since very beginning we suspect Linux AP is
> blocked
> by the widely used lock.) So, I think we can check in this first to
> avoid rebase.
> The lock removal thread can be continuously discussed.
> thx,eddie
>   


Ah, I misunderstood and thought they were rebased on top of the lock 
removal patches.  Applied & pushed.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: add back pending timer irqs for kernel APIC timer

[Avi Kivity]

Dong, Eddie wrote:
>     Add back pending irqs for apic timer to get precise guest
>     APIC timer interrupt.
>   

Can you explain the problem and the solution in more detail?


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: add back pending timer irqs for kernel APIC timer

[Dong, Eddie]

Avi Kivity wrote:
> Dong, Eddie wrote:
>>     Add back pending irqs for apic timer to get precise guest    
>> APIC timer interrupt. 
>> 
> 
> Can you explain the problem and the solution in more detail?

Today guest sleep 10 seconds only get about 5-6 seconds in host time.
A design philosiphy in various VMM is that guest wall clock should catch
up host wall clock. 

The reason (partly) is that A VM may be descheduled, while the host time
is still going.  For periodic timer like APIC timer, guest expect to see
certain amount of interrupt that stands for the time passed (host time).
In previous
APIC timer virtualization policy, we inject an IRQ to APIC when a period
of  host time is passed, but the guest may not take it if it is
descheduled.
In that way the previous irq in APIC will be overwritten by next
injection from fire of 
host hrtimer.  In that way guest get less amount of APIC timer IRQ.

This patch keep track of the pending irqs and inject them back to guest
eventually
even the guest may be descheduled. This is also what we did in Xen.
BTW, This policy will also be applied to future kernel PIT, I just do it
step by step.


Thx,eddie

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: add back pending timer irqs for kernel APIC timer

[Avi Kivity]

Dong, Eddie wrote:
> Avi Kivity wrote:
>   
>> Dong, Eddie wrote:
>>     
>>>     Add back pending irqs for apic timer to get precise guest    
>>> APIC timer interrupt. 
>>>
>>>       
>> Can you explain the problem and the solution in more detail?
>>     
>
> Today guest sleep 10 seconds only get about 5-6 seconds in host time.
> A design philosiphy in various VMM is that guest wall clock should catch
> up host wall clock. 
>
> The reason (partly) is that A VM may be descheduled, while the host time
> is still going.  For periodic timer like APIC timer, guest expect to see
> certain amount of interrupt that stands for the time passed (host time).
> In previous
> APIC timer virtualization policy, we inject an IRQ to APIC when a period
> of  host time is passed, but the guest may not take it if it is
> descheduled.
> In that way the previous irq in APIC will be overwritten by next
> injection from fire of 
> host hrtimer.  In that way guest get less amount of APIC timer IRQ.
>
> This patch keep track of the pending irqs and inject them back to guest
> eventually
> even the guest may be descheduled. This is also what we did in Xen.
> BTW, This policy will also be applied to future kernel PIT, I just do it
> step by step.
>
>   

I see.  We have something like that in userspace (called the 
time-drift-fix, or tdf).  Will look at the patch now.


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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: add back pending timer irqs for kernel APIC timer

[Gregory Haskins]

On Mon, 2007-08-13 at 11:50 +0300, Avi Kivity wrote:

> > This patch keep track of the pending irqs and inject them back to guest
> > eventually
> > even the guest may be descheduled. This is also what we did in Xen.
> > BTW, This policy will also be applied to future kernel PIT, I just do it
> > step by step.
> >
> >   
> 
> I see.  We have something like that in userspace (called the 
> time-drift-fix, or tdf).  Will look at the patch now.
> 
> 


Note that my old lapic branch did a similar "tdf" thing as well and it
worked quite nicely, so I think Eddie is on the right track with adding
this.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: add back pending timer irqs for kernel APIC timer

[Dong, Eddie]

> 
> 
> Note that my old lapic branch did a similar "tdf" thing as well and it
> worked quite nicely, so I think Eddie is on the right track with
> adding this.

I just copied the whole APIC timer stuff from your old patch at that
time as TODO. maybe I missed something :-(

Time virtualization has quit a long way to go and it is quit very
beginning now. Something in my plan:

1: Current 3 patches to fix the pending irq issues and accumulate issue.

2: stop hrtimer when the guest is descheduled to increase scalibility
and remove apic->lock.

3: Use scale + shift like Xen did to make sure no internal overflow if
we run guest contiguously for long time say 2 years.

Above 1-3 is already in Xen and we just need to port.


4: Should we use hrtimer? How efficieny it is? 


Will u be able to help together?
thx,eddie

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: add back pending timer irqs for kernel APIC timer

[Avi Kivity]

Dong, Eddie wrote:
>> Note that my old lapic branch did a similar "tdf" thing as well and it
>> worked quite nicely, so I think Eddie is on the right track with
>> adding this.
>>     
>
> I just copied the whole APIC timer stuff from your old patch at that
> time as TODO. maybe I missed something :-(
>
> Time virtualization has quit a long way to go and it is quit very
> beginning now. Something in my plan:
>
> 1: Current 3 patches to fix the pending irq issues and accumulate issue.
>
> 2: stop hrtimer when the guest is descheduled to increase scalibility
> and remove apic->lock.
>   

Why is this important?  An hrtimer is just an entry on a list, no?

Do you mean, just if the guest is preempted (not during hlt)?  Is it so 
important?  We're talking a few thousand wakeups per second.

> 3: Use scale + shift like Xen did to make sure no internal overflow if
> we run guest contiguously for long time say 2 years.
>   

hrtimers are in 64-bit nanoseconds.  That gives about 160 years.

> Above 1-3 is already in Xen and we just need to port.
>
>
> 4: Should we use hrtimer? How efficieny it is? 
>
>
>   

Sure, hrtimer is the future.

> Will u be able to help together?
> thx,eddie
>   


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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: add back pending timer irqs for kernel APIC timer

[Dong, Eddie]

Avi Kivity wrote:
>> 2: stop hrtimer when the guest is descheduled to increase
>> scalibility and remove apic->lock. 
>> 
> 
> Why is this important?  An hrtimer is just an entry on a list, no?

Does it need an IRQ to drive? If we have ten VMs, we got extra 10 IRQs.

> 
> Do you mean, just if the guest is preempted (not during hlt)?
> Is it so
> important?  We're talking a few thousand wakeups per second.

The majority of wakeup is APIC timer in normal SMP case.
This is important for deep C state power management. If guest is 1KHZ,
we may get 10KHZ IRQs if 10 VMs are running.

If it is in HLT, we need to set up an hrtimer or similar to wakeup the
HLT. 

> 
>> 3: Use scale + shift like Xen did to make sure no internal overflow
>> if we run guest contiguously for long time say 2 years.
>> 
> 
> hrtimers are in 64-bit nanoseconds.  That gives about 160 years.
> 

I assume TSC is better than hrtimer, see next bullet. When we convert
from TSC to PIT timer, a multiply is a must and we need to carefully
design
the scale and factor to not overflow if we stick in 64bits ops.

Notes here: We may not want to use 64bits * 64bits = 128bits as it is
too expansive. (?)
This is What Xen did and have some sense if we pay attention to
performance very much.

>> Above 1-3 is already in Xen and we just need to port.
>> 
>> 
>> 4: Should we use hrtimer? How efficieny it is?
>> 
>> 
>> 
> 
> Sure, hrtimer is the future.

Maybe I am wrong, but the point is that we don't care about accuracy
since
we only need an IRQ to interrupt guest at around the guest fire time. 
(A host timer IRQ triggered VM Exit and many callbacks in host which
should
exceeds 5K cycles) We can simply use TSC (much efficient IMO) and
present guest APIC 
freqeuncy = host TSC frequency. This way we don't need to do 
muliple/dividen etc. Anyway it depends on if we can find an much
efficient timer:-)
I have no idea here.


Eddie

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/

Re: add back pending timer irqs for kernel APIC timer

[Avi Kivity]

Dong, Eddie wrote:
> Avi Kivity wrote:
>   
>>> 2: stop hrtimer when the guest is descheduled to increase
>>> scalibility and remove apic->lock. 
>>>
>>>       
>> Why is this important?  An hrtimer is just an entry on a list, no?
>>     
>
> Does it need an IRQ to drive? If we have ten VMs, we got extra 10 IRQs.
>   

You're right -- if the host is overloaded, this will generate interrupts 
which cannot be handled in time.  So it makes sense to disable the 
hrtimer if we are preempted and if we are not in hlt sleep (e.g. 
involuntary preemption).

>   
>> Do you mean, just if the guest is preempted (not during hlt)?
>> Is it so
>> important?  We're talking a few thousand wakeups per second.
>>     
>
> The majority of wakeup is APIC timer in normal SMP case.
> This is important for deep C state power management. If guest is 1KHZ,
> we may get 10KHZ IRQs if 10 VMs are running.
>
> If it is in HLT, we need to set up an hrtimer or similar to wakeup the
> HLT. 
>   

So we need the hrtimer anyway.  Seems like it's only a win if the host 
is overloaded and the guests are running; an idle guest requires an hrtimer.

There is an alternative, if we have multiple vcpus set with the same 
frequency we can try to use one hrtimer per cpu for all of them.  This 
doesn't work for dyntick guests but should work if most guests are the 
same type.

>   
>>> 3: Use scale + shift like Xen did to make sure no internal overflow
>>> if we run guest contiguously for long time say 2 years.
>>>
>>>       
>> hrtimers are in 64-bit nanoseconds.  That gives about 160 years.
>>
>>     
>
> I assume TSC is better than hrtimer, see next bullet. When we convert
> from TSC to PIT timer, a multiply is a must and we need to carefully
> design
> the scale and factor to not overflow if we stick in 64bits ops.
>
> Notes here: We may not want to use 64bits * 64bits = 128bits as it is
> too expansive. (?)
> This is What Xen did and have some sense if we pay attention to
> performance very much.
>   

I think the hrtimer code deals with this, Linux has to deal with a lot 
of timers.

>   
>>> Above 1-3 is already in Xen and we just need to port.
>>>
>>>
>>> 4: Should we use hrtimer? How efficieny it is?
>>>
>>>
>>>
>>>       
>> Sure, hrtimer is the future.
>>     
>
> Maybe I am wrong, but the point is that we don't care about accuracy
> since
> we only need an IRQ to interrupt guest at around the guest fire time. 
> (A host timer IRQ triggered VM Exit and many callbacks in host which
> should
> exceeds 5K cycles) We can simply use TSC (much efficient IMO) and
> present guest APIC 
> freqeuncy = host TSC frequency. This way we don't need to do 
> muliple/dividen etc. Anyway it depends on if we can find an much
> efficient timer:-)
> I have no idea here.
>   

We need to make sure we are solving the right problem... do you have any 
traces which show that timer handling is a performance issue?

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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/