[PATCH] KVM: x86: Remove support for reporting coalesced APIC IRQs

[PATCH] KVM: x86: Remove support for reporting coalesced APIC IRQs

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

Since the arrival of posted interrupt support we can no longer guarantee
that coalesced IRQs are always reported to the IRQ source. Moreover,
accumulated APIC timer events could cause a busy loop when a VCPU should
rather be halted. The consensus is to remove coalesced tracking from the
LAPIC.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

Not sure, though, if this is going too far or not far enough /wrt
removing return values from affected functions.

 arch/x86/kvm/lapic.c |   62 ++++++++++++++++++-------------------------------
 arch/x86/kvm/lapic.h |    9 ++++---
 virt/kvm/ioapic.c    |   23 +++++++++---------
 virt/kvm/ioapic.h    |    5 ++-
 virt/kvm/irq_comm.c  |   31 +++++++++++-------------
 5 files changed, 56 insertions(+), 74 deletions(-)

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index e29883c..7cfd35c 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -405,17 +405,17 @@ int kvm_lapic_find_highest_irr(struct kvm_vcpu *vcpu)
 	return highest_irr;
 }
 
-static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
-			     int vector, int level, int trig_mode,
-			     unsigned long *dest_map);
+static void __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
+			      int vector, int level, int trig_mode,
+			      unsigned long *dest_map);
 
-int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq,
-		unsigned long *dest_map)
+void kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq,
+		      unsigned long *dest_map)
 {
 	struct kvm_lapic *apic = vcpu->arch.apic;
 
-	return __apic_accept_irq(apic, irq->delivery_mode, irq->vector,
-			irq->level, irq->trig_mode, dest_map);
+	__apic_accept_irq(apic, irq->delivery_mode, irq->vector,
+			  irq->level, irq->trig_mode, dest_map);
 }
 
 static int pv_eoi_put_user(struct kvm_vcpu *vcpu, u8 val)
@@ -597,7 +597,8 @@ int kvm_apic_match_dest(struct kvm_vcpu *vcpu, struct kvm_lapic *source,
 }
 
 bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct kvm_lapic *src,
-		struct kvm_lapic_irq *irq, int *r, unsigned long *dest_map)
+				   struct kvm_lapic_irq *irq,
+				   unsigned long *dest_map)
 {
 	struct kvm_apic_map *map;
 	unsigned long bitmap = 1;
@@ -605,10 +606,8 @@ bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct kvm_lapic *src,
 	int i;
 	bool ret = false;
 
-	*r = -1;
-
 	if (irq->shorthand == APIC_DEST_SELF) {
-		*r = kvm_apic_set_irq(src->vcpu, irq, dest_map);
+		kvm_apic_set_irq(src->vcpu, irq, dest_map);
 		return true;
 	}
 
@@ -651,9 +650,7 @@ bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct kvm_lapic *src,
 	for_each_set_bit(i, &bitmap, 16) {
 		if (!dst[i])
 			continue;
-		if (*r < 0)
-			*r = 0;
-		*r += kvm_apic_set_irq(dst[i]->vcpu, irq, dest_map);
+		kvm_apic_set_irq(dst[i]->vcpu, irq, dest_map);
 	}
 
 	ret = true;
@@ -662,15 +659,11 @@ out:
 	return ret;
 }
 
-/*
- * Add a pending IRQ into lapic.
- * Return 1 if successfully added and 0 if discarded.
- */
-static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
-			     int vector, int level, int trig_mode,
-			     unsigned long *dest_map)
+/* Set an IRQ pending in the lapic. */
+static void __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
+			      int vector, int level, int trig_mode,
+			      unsigned long *dest_map)
 {
-	int result = 0;
 	struct kvm_vcpu *vcpu = apic->vcpu;
 
 	switch (delivery_mode) {
@@ -684,13 +677,10 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
 		if (dest_map)
 			__set_bit(vcpu->vcpu_id, dest_map);
 
-		if (kvm_x86_ops->deliver_posted_interrupt) {
-			result = 1;
+		if (kvm_x86_ops->deliver_posted_interrupt)
 			kvm_x86_ops->deliver_posted_interrupt(vcpu, vector);
-		} else {
-			result = !apic_test_and_set_irr(vector, apic);
-
-			if (!result) {
+		else {
+			if (apic_test_and_set_irr(vector, apic)) {
 				if (trig_mode)
 					apic_debug("level trig mode repeatedly "
 						"for vector %d", vector);
@@ -702,7 +692,7 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
 		}
 out:
 		trace_kvm_apic_accept_irq(vcpu->vcpu_id, delivery_mode,
-				trig_mode, vector, !result);
+					  trig_mode, vector, false);
 		break;
 
 	case APIC_DM_REMRD:
@@ -714,14 +704,12 @@ out:
 		break;
 
 	case APIC_DM_NMI:
-		result = 1;
 		kvm_inject_nmi(vcpu);
 		kvm_vcpu_kick(vcpu);
 		break;
 
 	case APIC_DM_INIT:
 		if (!trig_mode || level) {
-			result = 1;
 			/* assumes that there are only KVM_APIC_INIT/SIPI */
 			apic->pending_events = (1UL << KVM_APIC_INIT);
 			/* make sure pending_events is visible before sending
@@ -738,7 +726,6 @@ out:
 	case APIC_DM_STARTUP:
 		apic_debug("SIPI to vcpu %d vector 0x%02x\n",
 			   vcpu->vcpu_id, vector);
-		result = 1;
 		apic->sipi_vector = vector;
 		/* make sure sipi_vector is visible for the receiver */
 		smp_wmb();
@@ -760,7 +747,6 @@ out:
 		       delivery_mode);
 		break;
 	}
-	return result;
 }
 
 int kvm_apic_compare_prio(struct kvm_vcpu *vcpu1, struct kvm_vcpu *vcpu2)
@@ -1470,7 +1456,7 @@ int apic_has_pending_timer(struct kvm_vcpu *vcpu)
 	return 0;
 }
 
-int kvm_apic_local_deliver(struct kvm_lapic *apic, int lvt_type)
+void kvm_apic_local_deliver(struct kvm_lapic *apic, int lvt_type)
 {
 	u32 reg = kvm_apic_get_reg(apic, lvt_type);
 	int vector, mode, trig_mode;
@@ -1479,10 +1465,8 @@ int kvm_apic_local_deliver(struct kvm_lapic *apic, int lvt_type)
 		vector = reg & APIC_VECTOR_MASK;
 		mode = reg & APIC_MODE_MASK;
 		trig_mode = reg & APIC_LVT_LEVEL_TRIGGER;
-		return __apic_accept_irq(apic, mode, vector, 1, trig_mode,
-					NULL);
+		__apic_accept_irq(apic, mode, vector, 1, trig_mode, NULL);
 	}
-	return 0;
 }
 
 void kvm_apic_nmi_wd_deliver(struct kvm_vcpu *vcpu)
@@ -1608,8 +1592,8 @@ void kvm_inject_apic_timer_irqs(struct kvm_vcpu *vcpu)
 		return;
 
 	if (atomic_read(&apic->lapic_timer.pending) > 0) {
-		if (kvm_apic_local_deliver(apic, APIC_LVTT))
-			atomic_dec(&apic->lapic_timer.pending);
+		kvm_apic_local_deliver(apic, APIC_LVTT);
+		atomic_set(&apic->lapic_timer.pending, 0);
 	}
 }
 
diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
index c730ac9..4d1b9eb 100644
--- a/arch/x86/kvm/lapic.h
+++ b/arch/x86/kvm/lapic.h
@@ -57,12 +57,13 @@ void kvm_apic_update_tmr(struct kvm_vcpu *vcpu, u32 *tmr);
 void kvm_apic_update_irr(struct kvm_vcpu *vcpu, u32 *pir);
 int kvm_apic_match_physical_addr(struct kvm_lapic *apic, u16 dest);
 int kvm_apic_match_logical_addr(struct kvm_lapic *apic, u8 mda);
-int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq,
-		unsigned long *dest_map);
-int kvm_apic_local_deliver(struct kvm_lapic *apic, int lvt_type);
+void kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq,
+		      unsigned long *dest_map);
+void kvm_apic_local_deliver(struct kvm_lapic *apic, int lvt_type);
 
 bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct kvm_lapic *src,
-		struct kvm_lapic_irq *irq, int *r, unsigned long *dest_map);
+				   struct kvm_lapic_irq *irq,
+				   unsigned long *dest_map);
 
 u64 kvm_get_apic_base(struct kvm_vcpu *vcpu);
 void kvm_set_apic_base(struct kvm_vcpu *vcpu, u64 data);
diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
index 2d68297..606e09b 100644
--- a/virt/kvm/ioapic.c
+++ b/virt/kvm/ioapic.c
@@ -50,8 +50,8 @@
 #else
 #define ioapic_debug(fmt, arg...)
 #endif
-static int ioapic_deliver(struct kvm_ioapic *vioapic, int irq,
-		bool line_status);
+static void ioapic_deliver(struct kvm_ioapic *vioapic, int irq,
+			   bool line_status);
 
 static unsigned long ioapic_read_indirect(struct kvm_ioapic *ioapic,
 					  unsigned long addr,
@@ -172,9 +172,10 @@ static int ioapic_service(struct kvm_ioapic *ioapic, unsigned int idx,
 	pent = &ioapic->redirtbl[idx];
 
 	if (!pent->fields.mask) {
-		injected = ioapic_deliver(ioapic, idx, line_status);
-		if (injected && pent->fields.trig_mode == IOAPIC_LEVEL_TRIG)
+		ioapic_deliver(ioapic, idx, line_status);
+		if (pent->fields.trig_mode == IOAPIC_LEVEL_TRIG)
 			pent->fields.remote_irr = 1;
+		injected = 1;
 	}
 
 	return injected;
@@ -282,11 +283,11 @@ static void ioapic_write_indirect(struct kvm_ioapic *ioapic, u32 val)
 	}
 }
 
-static int ioapic_deliver(struct kvm_ioapic *ioapic, int irq, bool line_status)
+static void ioapic_deliver(struct kvm_ioapic *ioapic, int irq,
+			   bool line_status)
 {
 	union kvm_ioapic_redirect_entry *entry = &ioapic->redirtbl[irq];
 	struct kvm_lapic_irq irqe;
-	int ret;
 
 	ioapic_debug("dest=%x dest_mode=%x delivery_mode=%x "
 		     "vector=%x trig_mode=%x\n",
@@ -304,13 +305,11 @@ static int ioapic_deliver(struct kvm_ioapic *ioapic, int irq, bool line_status)
 
 	if (irq == RTC_GSI && line_status) {
 		BUG_ON(ioapic->rtc_status.pending_eoi != 0);
-		ret = kvm_irq_delivery_to_apic(ioapic->kvm, NULL, &irqe,
-				ioapic->rtc_status.dest_map);
-		ioapic->rtc_status.pending_eoi = ret;
+		kvm_irq_delivery_to_apic(ioapic->kvm, NULL, &irqe,
+					 ioapic->rtc_status.dest_map);
+		ioapic->rtc_status.pending_eoi++;
 	} else
-		ret = kvm_irq_delivery_to_apic(ioapic->kvm, NULL, &irqe, NULL);
-
-	return ret;
+		kvm_irq_delivery_to_apic(ioapic->kvm, NULL, &irqe, NULL);
 }
 
 int kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int irq, int irq_source_id,
diff --git a/virt/kvm/ioapic.h b/virt/kvm/ioapic.h
index 615d8c9..639fe9c 100644
--- a/virt/kvm/ioapic.h
+++ b/virt/kvm/ioapic.h
@@ -92,8 +92,9 @@ int kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int irq, int irq_source_id,
 		       int level, bool line_status);
 void kvm_ioapic_clear_all(struct kvm_ioapic *ioapic, int irq_source_id);
 void kvm_ioapic_reset(struct kvm_ioapic *ioapic);
-int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
-		struct kvm_lapic_irq *irq, unsigned long *dest_map);
+void kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
+			      struct kvm_lapic_irq *irq,
+			      unsigned long *dest_map);
 int kvm_get_ioapic(struct kvm *kvm, struct kvm_ioapic_state *state);
 int kvm_set_ioapic(struct kvm *kvm, struct kvm_ioapic_state *state);
 void kvm_vcpu_request_scan_ioapic(struct kvm *kvm);
diff --git a/virt/kvm/irq_comm.c b/virt/kvm/irq_comm.c
index 25ab480..58d4538 100644
--- a/virt/kvm/irq_comm.c
+++ b/virt/kvm/irq_comm.c
@@ -65,10 +65,11 @@ inline static bool kvm_is_dm_lowest_prio(struct kvm_lapic_irq *irq)
 #endif
 }
 
-int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
-		struct kvm_lapic_irq *irq, unsigned long *dest_map)
+void kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
+			      struct kvm_lapic_irq *irq,
+			      unsigned long *dest_map)
 {
-	int i, r = -1;
+	int i;
 	struct kvm_vcpu *vcpu, *lowest = NULL;
 
 	if (irq->dest_mode == 0 && irq->dest_id == 0xff &&
@@ -77,8 +78,8 @@ int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
 		irq->delivery_mode = APIC_DM_FIXED;
 	}
 
-	if (kvm_irq_delivery_to_apic_fast(kvm, src, irq, &r, dest_map))
-		return r;
+	if (kvm_irq_delivery_to_apic_fast(kvm, src, irq, dest_map))
+		return;
 
 	kvm_for_each_vcpu(i, vcpu, kvm) {
 		if (!kvm_apic_present(vcpu))
@@ -88,11 +89,9 @@ int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
 					irq->dest_id, irq->dest_mode))
 			continue;
 
-		if (!kvm_is_dm_lowest_prio(irq)) {
-			if (r < 0)
-				r = 0;
-			r += kvm_apic_set_irq(vcpu, irq, dest_map);
-		} else if (kvm_lapic_enabled(vcpu)) {
+		if (!kvm_is_dm_lowest_prio(irq))
+			kvm_apic_set_irq(vcpu, irq, dest_map);
+		else if (kvm_lapic_enabled(vcpu)) {
 			if (!lowest)
 				lowest = vcpu;
 			else if (kvm_apic_compare_prio(vcpu, lowest) < 0)
@@ -101,9 +100,7 @@ int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
 	}
 
 	if (lowest)
-		r = kvm_apic_set_irq(lowest, irq, dest_map);
-
-	return r;
+		kvm_apic_set_irq(lowest, irq, dest_map);
 }
 
 static inline void kvm_set_msi_irq(struct kvm_kernel_irq_routing_entry *e,
@@ -133,7 +130,8 @@ int kvm_set_msi(struct kvm_kernel_irq_routing_entry *e,
 
 	kvm_set_msi_irq(e, &irq);
 
-	return kvm_irq_delivery_to_apic(kvm, NULL, &irq, NULL);
+	kvm_irq_delivery_to_apic(kvm, NULL, &irq, NULL);
+	return 1;
 }
 
 
@@ -141,12 +139,11 @@ static int kvm_set_msi_inatomic(struct kvm_kernel_irq_routing_entry *e,
 			 struct kvm *kvm)
 {
 	struct kvm_lapic_irq irq;
-	int r;
 
 	kvm_set_msi_irq(e, &irq);
 
-	if (kvm_irq_delivery_to_apic_fast(kvm, NULL, &irq, &r, NULL))
-		return r;
+	if (kvm_irq_delivery_to_apic_fast(kvm, NULL, &irq, NULL))
+		return 1;
 	else
 		return -EWOULDBLOCK;
 }
-- 
1.7.3.4

Re: [PATCH] KVM: x86: Remove support for reporting coalesced APIC IRQs

[Gleb Natapov]

On Sun, Apr 28, 2013 at 01:27:00PM +0200, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> Since the arrival of posted interrupt support we can no longer guarantee
> that coalesced IRQs are always reported to the IRQ source. Moreover,
> accumulated APIC timer events could cause a busy loop when a VCPU should
> rather be halted. The consensus is to remove coalesced tracking from the
> LAPIC.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
> 
> Not sure, though, if this is going too far or not far enough /wrt
> removing return values from affected functions.
> 
>  arch/x86/kvm/lapic.c |   62 ++++++++++++++++++-------------------------------
>  arch/x86/kvm/lapic.h |    9 ++++---
>  virt/kvm/ioapic.c    |   23 +++++++++---------
>  virt/kvm/ioapic.h    |    5 ++-
>  virt/kvm/irq_comm.c  |   31 +++++++++++-------------
>  5 files changed, 56 insertions(+), 74 deletions(-)
> 
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index e29883c..7cfd35c 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -405,17 +405,17 @@ int kvm_lapic_find_highest_irr(struct kvm_vcpu *vcpu)
>  	return highest_irr;
>  }
>  
> -static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
> -			     int vector, int level, int trig_mode,
> -			     unsigned long *dest_map);
> +static void __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
> +			      int vector, int level, int trig_mode,
> +			      unsigned long *dest_map);
>  
> -int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq,
> -		unsigned long *dest_map)
> +void kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq,
> +		      unsigned long *dest_map)
>  {
>  	struct kvm_lapic *apic = vcpu->arch.apic;
>  
> -	return __apic_accept_irq(apic, irq->delivery_mode, irq->vector,
> -			irq->level, irq->trig_mode, dest_map);
> +	__apic_accept_irq(apic, irq->delivery_mode, irq->vector,
> +			  irq->level, irq->trig_mode, dest_map);
>  }
>  
>  static int pv_eoi_put_user(struct kvm_vcpu *vcpu, u8 val)
> @@ -597,7 +597,8 @@ int kvm_apic_match_dest(struct kvm_vcpu *vcpu, struct kvm_lapic *source,
>  }
>  
>  bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct kvm_lapic *src,
> -		struct kvm_lapic_irq *irq, int *r, unsigned long *dest_map)
> +				   struct kvm_lapic_irq *irq,
> +				   unsigned long *dest_map)
>  {
>  	struct kvm_apic_map *map;
>  	unsigned long bitmap = 1;
> @@ -605,10 +606,8 @@ bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct kvm_lapic *src,
>  	int i;
>  	bool ret = false;
>  
> -	*r = -1;
> -
>  	if (irq->shorthand == APIC_DEST_SELF) {
> -		*r = kvm_apic_set_irq(src->vcpu, irq, dest_map);
> +		kvm_apic_set_irq(src->vcpu, irq, dest_map);
>  		return true;
>  	}
>  
> @@ -651,9 +650,7 @@ bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct kvm_lapic *src,
>  	for_each_set_bit(i, &bitmap, 16) {
>  		if (!dst[i])
>  			continue;
> -		if (*r < 0)
> -			*r = 0;
> -		*r += kvm_apic_set_irq(dst[i]->vcpu, irq, dest_map);
> +		kvm_apic_set_irq(dst[i]->vcpu, irq, dest_map);
>  	}
>  
>  	ret = true;
> @@ -662,15 +659,11 @@ out:
>  	return ret;
>  }
>  
> -/*
> - * Add a pending IRQ into lapic.
> - * Return 1 if successfully added and 0 if discarded.
> - */
> -static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
> -			     int vector, int level, int trig_mode,
> -			     unsigned long *dest_map)
> +/* Set an IRQ pending in the lapic. */
> +static void __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
> +			      int vector, int level, int trig_mode,
> +			      unsigned long *dest_map)
>  {
> -	int result = 0;
>  	struct kvm_vcpu *vcpu = apic->vcpu;
>  
>  	switch (delivery_mode) {
> @@ -684,13 +677,10 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
>  		if (dest_map)
>  			__set_bit(vcpu->vcpu_id, dest_map);
>  
> -		if (kvm_x86_ops->deliver_posted_interrupt) {
> -			result = 1;
> +		if (kvm_x86_ops->deliver_posted_interrupt)
>  			kvm_x86_ops->deliver_posted_interrupt(vcpu, vector);
> -		} else {
> -			result = !apic_test_and_set_irr(vector, apic);
> -
> -			if (!result) {
> +		else {
> +			if (apic_test_and_set_irr(vector, apic)) {
>  				if (trig_mode)
>  					apic_debug("level trig mode repeatedly "
>  						"for vector %d", vector);
> @@ -702,7 +692,7 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
>  		}
>  out:
>  		trace_kvm_apic_accept_irq(vcpu->vcpu_id, delivery_mode,
> -				trig_mode, vector, !result);
> +					  trig_mode, vector, false);
>  		break;
>  
>  	case APIC_DM_REMRD:
> @@ -714,14 +704,12 @@ out:
>  		break;
>  
>  	case APIC_DM_NMI:
> -		result = 1;
>  		kvm_inject_nmi(vcpu);
>  		kvm_vcpu_kick(vcpu);
>  		break;
>  
>  	case APIC_DM_INIT:
>  		if (!trig_mode || level) {
> -			result = 1;
>  			/* assumes that there are only KVM_APIC_INIT/SIPI */
>  			apic->pending_events = (1UL << KVM_APIC_INIT);
>  			/* make sure pending_events is visible before sending
> @@ -738,7 +726,6 @@ out:
>  	case APIC_DM_STARTUP:
>  		apic_debug("SIPI to vcpu %d vector 0x%02x\n",
>  			   vcpu->vcpu_id, vector);
> -		result = 1;
>  		apic->sipi_vector = vector;
>  		/* make sure sipi_vector is visible for the receiver */
>  		smp_wmb();
> @@ -760,7 +747,6 @@ out:
>  		       delivery_mode);
>  		break;
>  	}
> -	return result;
>  }
>  
>  int kvm_apic_compare_prio(struct kvm_vcpu *vcpu1, struct kvm_vcpu *vcpu2)
> @@ -1470,7 +1456,7 @@ int apic_has_pending_timer(struct kvm_vcpu *vcpu)
>  	return 0;
>  }
>  
> -int kvm_apic_local_deliver(struct kvm_lapic *apic, int lvt_type)
> +void kvm_apic_local_deliver(struct kvm_lapic *apic, int lvt_type)
>  {
>  	u32 reg = kvm_apic_get_reg(apic, lvt_type);
>  	int vector, mode, trig_mode;
> @@ -1479,10 +1465,8 @@ int kvm_apic_local_deliver(struct kvm_lapic *apic, int lvt_type)
>  		vector = reg & APIC_VECTOR_MASK;
>  		mode = reg & APIC_MODE_MASK;
>  		trig_mode = reg & APIC_LVT_LEVEL_TRIGGER;
> -		return __apic_accept_irq(apic, mode, vector, 1, trig_mode,
> -					NULL);
> +		__apic_accept_irq(apic, mode, vector, 1, trig_mode, NULL);
>  	}
> -	return 0;
>  }
>  
>  void kvm_apic_nmi_wd_deliver(struct kvm_vcpu *vcpu)
> @@ -1608,8 +1592,8 @@ void kvm_inject_apic_timer_irqs(struct kvm_vcpu *vcpu)
>  		return;
>  
>  	if (atomic_read(&apic->lapic_timer.pending) > 0) {
> -		if (kvm_apic_local_deliver(apic, APIC_LVTT))
> -			atomic_dec(&apic->lapic_timer.pending);
> +		kvm_apic_local_deliver(apic, APIC_LVTT);
> +		atomic_set(&apic->lapic_timer.pending, 0);
>  	}
>  }
>  
> diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
> index c730ac9..4d1b9eb 100644
> --- a/arch/x86/kvm/lapic.h
> +++ b/arch/x86/kvm/lapic.h
> @@ -57,12 +57,13 @@ void kvm_apic_update_tmr(struct kvm_vcpu *vcpu, u32 *tmr);
>  void kvm_apic_update_irr(struct kvm_vcpu *vcpu, u32 *pir);
>  int kvm_apic_match_physical_addr(struct kvm_lapic *apic, u16 dest);
>  int kvm_apic_match_logical_addr(struct kvm_lapic *apic, u8 mda);
> -int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq,
> -		unsigned long *dest_map);
> -int kvm_apic_local_deliver(struct kvm_lapic *apic, int lvt_type);
> +void kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq,
> +		      unsigned long *dest_map);
> +void kvm_apic_local_deliver(struct kvm_lapic *apic, int lvt_type);
>  
>  bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct kvm_lapic *src,
> -		struct kvm_lapic_irq *irq, int *r, unsigned long *dest_map);
> +				   struct kvm_lapic_irq *irq,
> +				   unsigned long *dest_map);
>  
>  u64 kvm_get_apic_base(struct kvm_vcpu *vcpu);
>  void kvm_set_apic_base(struct kvm_vcpu *vcpu, u64 data);
> diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
> index 2d68297..606e09b 100644
> --- a/virt/kvm/ioapic.c
> +++ b/virt/kvm/ioapic.c
> @@ -50,8 +50,8 @@
>  #else
>  #define ioapic_debug(fmt, arg...)
>  #endif
> -static int ioapic_deliver(struct kvm_ioapic *vioapic, int irq,
> -		bool line_status);
> +static void ioapic_deliver(struct kvm_ioapic *vioapic, int irq,
> +			   bool line_status);
>  
>  static unsigned long ioapic_read_indirect(struct kvm_ioapic *ioapic,
>  					  unsigned long addr,
> @@ -172,9 +172,10 @@ static int ioapic_service(struct kvm_ioapic *ioapic, unsigned int idx,
>  	pent = &ioapic->redirtbl[idx];
>  
>  	if (!pent->fields.mask) {
> -		injected = ioapic_deliver(ioapic, idx, line_status);
> -		if (injected && pent->fields.trig_mode == IOAPIC_LEVEL_TRIG)
> +		ioapic_deliver(ioapic, idx, line_status);
> +		if (pent->fields.trig_mode == IOAPIC_LEVEL_TRIG)
>  			pent->fields.remote_irr = 1;
> +		injected = 1;
>  	}
>  
>  	return injected;
> @@ -282,11 +283,11 @@ static void ioapic_write_indirect(struct kvm_ioapic *ioapic, u32 val)
>  	}
>  }
>  
> -static int ioapic_deliver(struct kvm_ioapic *ioapic, int irq, bool line_status)
> +static void ioapic_deliver(struct kvm_ioapic *ioapic, int irq,
> +			   bool line_status)
>  {
>  	union kvm_ioapic_redirect_entry *entry = &ioapic->redirtbl[irq];
>  	struct kvm_lapic_irq irqe;
> -	int ret;
>  
>  	ioapic_debug("dest=%x dest_mode=%x delivery_mode=%x "
>  		     "vector=%x trig_mode=%x\n",
> @@ -304,13 +305,11 @@ static int ioapic_deliver(struct kvm_ioapic *ioapic, int irq, bool line_status)
>  
>  	if (irq == RTC_GSI && line_status) {
>  		BUG_ON(ioapic->rtc_status.pending_eoi != 0);
> -		ret = kvm_irq_delivery_to_apic(ioapic->kvm, NULL, &irqe,
> -				ioapic->rtc_status.dest_map);
> -		ioapic->rtc_status.pending_eoi = ret;
> +		kvm_irq_delivery_to_apic(ioapic->kvm, NULL, &irqe,
> +					 ioapic->rtc_status.dest_map);
> +		ioapic->rtc_status.pending_eoi++;
kvm_irq_delivery_to_apic() can deliver to more then one vcpu, so
pending_eoi should be incremented accordingly. It means that we cannot
drop kvm_irq_delivery_to_apic() return value.


>  	} else
> -		ret = kvm_irq_delivery_to_apic(ioapic->kvm, NULL, &irqe, NULL);
> -
> -	return ret;
> +		kvm_irq_delivery_to_apic(ioapic->kvm, NULL, &irqe, NULL);
>  }
>  
>  int kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int irq, int irq_source_id,
> diff --git a/virt/kvm/ioapic.h b/virt/kvm/ioapic.h
> index 615d8c9..639fe9c 100644
> --- a/virt/kvm/ioapic.h
> +++ b/virt/kvm/ioapic.h
> @@ -92,8 +92,9 @@ int kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int irq, int irq_source_id,
>  		       int level, bool line_status);
>  void kvm_ioapic_clear_all(struct kvm_ioapic *ioapic, int irq_source_id);
>  void kvm_ioapic_reset(struct kvm_ioapic *ioapic);
> -int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
> -		struct kvm_lapic_irq *irq, unsigned long *dest_map);
> +void kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
> +			      struct kvm_lapic_irq *irq,
> +			      unsigned long *dest_map);
>  int kvm_get_ioapic(struct kvm *kvm, struct kvm_ioapic_state *state);
>  int kvm_set_ioapic(struct kvm *kvm, struct kvm_ioapic_state *state);
>  void kvm_vcpu_request_scan_ioapic(struct kvm *kvm);
> diff --git a/virt/kvm/irq_comm.c b/virt/kvm/irq_comm.c
> index 25ab480..58d4538 100644
> --- a/virt/kvm/irq_comm.c
> +++ b/virt/kvm/irq_comm.c
> @@ -65,10 +65,11 @@ inline static bool kvm_is_dm_lowest_prio(struct kvm_lapic_irq *irq)
>  #endif
>  }
>  
> -int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
> -		struct kvm_lapic_irq *irq, unsigned long *dest_map)
> +void kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
> +			      struct kvm_lapic_irq *irq,
> +			      unsigned long *dest_map)
>  {
> -	int i, r = -1;
> +	int i;
>  	struct kvm_vcpu *vcpu, *lowest = NULL;
>  
>  	if (irq->dest_mode == 0 && irq->dest_id == 0xff &&
> @@ -77,8 +78,8 @@ int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
>  		irq->delivery_mode = APIC_DM_FIXED;
>  	}
>  
> -	if (kvm_irq_delivery_to_apic_fast(kvm, src, irq, &r, dest_map))
> -		return r;
> +	if (kvm_irq_delivery_to_apic_fast(kvm, src, irq, dest_map))
> +		return;
>  
>  	kvm_for_each_vcpu(i, vcpu, kvm) {
>  		if (!kvm_apic_present(vcpu))
> @@ -88,11 +89,9 @@ int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
>  					irq->dest_id, irq->dest_mode))
>  			continue;
>  
> -		if (!kvm_is_dm_lowest_prio(irq)) {
> -			if (r < 0)
> -				r = 0;
> -			r += kvm_apic_set_irq(vcpu, irq, dest_map);
> -		} else if (kvm_lapic_enabled(vcpu)) {
> +		if (!kvm_is_dm_lowest_prio(irq))
> +			kvm_apic_set_irq(vcpu, irq, dest_map);
> +		else if (kvm_lapic_enabled(vcpu)) {
>  			if (!lowest)
>  				lowest = vcpu;
>  			else if (kvm_apic_compare_prio(vcpu, lowest) < 0)
> @@ -101,9 +100,7 @@ int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
>  	}
>  
>  	if (lowest)
> -		r = kvm_apic_set_irq(lowest, irq, dest_map);
> -
> -	return r;
> +		kvm_apic_set_irq(lowest, irq, dest_map);
>  }
>  
>  static inline void kvm_set_msi_irq(struct kvm_kernel_irq_routing_entry *e,
> @@ -133,7 +130,8 @@ int kvm_set_msi(struct kvm_kernel_irq_routing_entry *e,
>  
>  	kvm_set_msi_irq(e, &irq);
>  
> -	return kvm_irq_delivery_to_apic(kvm, NULL, &irq, NULL);
> +	kvm_irq_delivery_to_apic(kvm, NULL, &irq, NULL);
> +	return 1;
>  }
>  
>  
> @@ -141,12 +139,11 @@ static int kvm_set_msi_inatomic(struct kvm_kernel_irq_routing_entry *e,
>  			 struct kvm *kvm)
>  {
>  	struct kvm_lapic_irq irq;
> -	int r;
>  
>  	kvm_set_msi_irq(e, &irq);
>  
> -	if (kvm_irq_delivery_to_apic_fast(kvm, NULL, &irq, &r, NULL))
> -		return r;
> +	if (kvm_irq_delivery_to_apic_fast(kvm, NULL, &irq, NULL))
> +		return 1;
>  	else
>  		return -EWOULDBLOCK;
>  }
> -- 
> 1.7.3.4

--
			Gleb.

[PATCH v2] KVM: x86: Remove support for reporting coalesced APIC IRQs

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

Since the arrival of posted interrupt support we can no longer guarantee
that coalesced IRQs are always reported to the IRQ source. Moreover,
accumulated APIC timer events could cause a busy loop when a VCPU should
rather be halted. The consensus is to remove coalesced tracking from the
LAPIC.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

Changes in v2:
 - preserve return values where we need to count to how many VCPUs an
   IRQ was delivered

 arch/x86/kvm/lapic.c |   57 ++++++++++++++++++++-----------------------------
 arch/x86/kvm/lapic.h |    6 ++--
 virt/kvm/irq_comm.c  |    9 +++++--
 3 files changed, 32 insertions(+), 40 deletions(-)

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index e29883c..3bc58cd 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -405,17 +405,17 @@ int kvm_lapic_find_highest_irr(struct kvm_vcpu *vcpu)
 	return highest_irr;
 }
 
-static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
-			     int vector, int level, int trig_mode,
-			     unsigned long *dest_map);
+static void __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
+			      int vector, int level, int trig_mode,
+			      unsigned long *dest_map);
 
-int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq,
-		unsigned long *dest_map)
+void kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq,
+		      unsigned long *dest_map)
 {
 	struct kvm_lapic *apic = vcpu->arch.apic;
 
-	return __apic_accept_irq(apic, irq->delivery_mode, irq->vector,
-			irq->level, irq->trig_mode, dest_map);
+	__apic_accept_irq(apic, irq->delivery_mode, irq->vector,
+			  irq->level, irq->trig_mode, dest_map);
 }
 
 static int pv_eoi_put_user(struct kvm_vcpu *vcpu, u8 val)
@@ -608,7 +608,8 @@ bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct kvm_lapic *src,
 	*r = -1;
 
 	if (irq->shorthand == APIC_DEST_SELF) {
-		*r = kvm_apic_set_irq(src->vcpu, irq, dest_map);
+		kvm_apic_set_irq(src->vcpu, irq, dest_map);
+		*r = 1;
 		return true;
 	}
 
@@ -653,7 +654,8 @@ bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct kvm_lapic *src,
 			continue;
 		if (*r < 0)
 			*r = 0;
-		*r += kvm_apic_set_irq(dst[i]->vcpu, irq, dest_map);
+		kvm_apic_set_irq(dst[i]->vcpu, irq, dest_map);
+		*r += 1;
 	}
 
 	ret = true;
@@ -662,15 +664,11 @@ out:
 	return ret;
 }
 
-/*
- * Add a pending IRQ into lapic.
- * Return 1 if successfully added and 0 if discarded.
- */
-static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
-			     int vector, int level, int trig_mode,
-			     unsigned long *dest_map)
+/* Set an IRQ pending in the lapic. */
+static void __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
+			      int vector, int level, int trig_mode,
+			      unsigned long *dest_map)
 {
-	int result = 0;
 	struct kvm_vcpu *vcpu = apic->vcpu;
 
 	switch (delivery_mode) {
@@ -684,13 +682,10 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
 		if (dest_map)
 			__set_bit(vcpu->vcpu_id, dest_map);
 
-		if (kvm_x86_ops->deliver_posted_interrupt) {
-			result = 1;
+		if (kvm_x86_ops->deliver_posted_interrupt)
 			kvm_x86_ops->deliver_posted_interrupt(vcpu, vector);
-		} else {
-			result = !apic_test_and_set_irr(vector, apic);
-
-			if (!result) {
+		else {
+			if (apic_test_and_set_irr(vector, apic)) {
 				if (trig_mode)
 					apic_debug("level trig mode repeatedly "
 						"for vector %d", vector);
@@ -702,7 +697,7 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
 		}
 out:
 		trace_kvm_apic_accept_irq(vcpu->vcpu_id, delivery_mode,
-				trig_mode, vector, !result);
+					  trig_mode, vector, false);
 		break;
 
 	case APIC_DM_REMRD:
@@ -714,14 +709,12 @@ out:
 		break;
 
 	case APIC_DM_NMI:
-		result = 1;
 		kvm_inject_nmi(vcpu);
 		kvm_vcpu_kick(vcpu);
 		break;
 
 	case APIC_DM_INIT:
 		if (!trig_mode || level) {
-			result = 1;
 			/* assumes that there are only KVM_APIC_INIT/SIPI */
 			apic->pending_events = (1UL << KVM_APIC_INIT);
 			/* make sure pending_events is visible before sending
@@ -738,7 +731,6 @@ out:
 	case APIC_DM_STARTUP:
 		apic_debug("SIPI to vcpu %d vector 0x%02x\n",
 			   vcpu->vcpu_id, vector);
-		result = 1;
 		apic->sipi_vector = vector;
 		/* make sure sipi_vector is visible for the receiver */
 		smp_wmb();
@@ -760,7 +752,6 @@ out:
 		       delivery_mode);
 		break;
 	}
-	return result;
 }
 
 int kvm_apic_compare_prio(struct kvm_vcpu *vcpu1, struct kvm_vcpu *vcpu2)
@@ -1470,7 +1461,7 @@ int apic_has_pending_timer(struct kvm_vcpu *vcpu)
 	return 0;
 }
 
-int kvm_apic_local_deliver(struct kvm_lapic *apic, int lvt_type)
+void kvm_apic_local_deliver(struct kvm_lapic *apic, int lvt_type)
 {
 	u32 reg = kvm_apic_get_reg(apic, lvt_type);
 	int vector, mode, trig_mode;
@@ -1479,10 +1470,8 @@ int kvm_apic_local_deliver(struct kvm_lapic *apic, int lvt_type)
 		vector = reg & APIC_VECTOR_MASK;
 		mode = reg & APIC_MODE_MASK;
 		trig_mode = reg & APIC_LVT_LEVEL_TRIGGER;
-		return __apic_accept_irq(apic, mode, vector, 1, trig_mode,
-					NULL);
+		__apic_accept_irq(apic, mode, vector, 1, trig_mode, NULL);
 	}
-	return 0;
 }
 
 void kvm_apic_nmi_wd_deliver(struct kvm_vcpu *vcpu)
@@ -1608,8 +1597,8 @@ void kvm_inject_apic_timer_irqs(struct kvm_vcpu *vcpu)
 		return;
 
 	if (atomic_read(&apic->lapic_timer.pending) > 0) {
-		if (kvm_apic_local_deliver(apic, APIC_LVTT))
-			atomic_dec(&apic->lapic_timer.pending);
+		kvm_apic_local_deliver(apic, APIC_LVTT);
+		atomic_set(&apic->lapic_timer.pending, 0);
 	}
 }
 
diff --git a/arch/x86/kvm/lapic.h b/arch/x86/kvm/lapic.h
index c730ac9..61a73a0 100644
--- a/arch/x86/kvm/lapic.h
+++ b/arch/x86/kvm/lapic.h
@@ -57,9 +57,9 @@ void kvm_apic_update_tmr(struct kvm_vcpu *vcpu, u32 *tmr);
 void kvm_apic_update_irr(struct kvm_vcpu *vcpu, u32 *pir);
 int kvm_apic_match_physical_addr(struct kvm_lapic *apic, u16 dest);
 int kvm_apic_match_logical_addr(struct kvm_lapic *apic, u8 mda);
-int kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq,
-		unsigned long *dest_map);
-int kvm_apic_local_deliver(struct kvm_lapic *apic, int lvt_type);
+void kvm_apic_set_irq(struct kvm_vcpu *vcpu, struct kvm_lapic_irq *irq,
+		      unsigned long *dest_map);
+void kvm_apic_local_deliver(struct kvm_lapic *apic, int lvt_type);
 
 bool kvm_irq_delivery_to_apic_fast(struct kvm *kvm, struct kvm_lapic *src,
 		struct kvm_lapic_irq *irq, int *r, unsigned long *dest_map);
diff --git a/virt/kvm/irq_comm.c b/virt/kvm/irq_comm.c
index 25ab480..d60e1f1 100644
--- a/virt/kvm/irq_comm.c
+++ b/virt/kvm/irq_comm.c
@@ -91,7 +91,8 @@ int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
 		if (!kvm_is_dm_lowest_prio(irq)) {
 			if (r < 0)
 				r = 0;
-			r += kvm_apic_set_irq(vcpu, irq, dest_map);
+			kvm_apic_set_irq(vcpu, irq, dest_map);
+			r++;
 		} else if (kvm_lapic_enabled(vcpu)) {
 			if (!lowest)
 				lowest = vcpu;
@@ -100,8 +101,10 @@ int kvm_irq_delivery_to_apic(struct kvm *kvm, struct kvm_lapic *src,
 		}
 	}
 
-	if (lowest)
-		r = kvm_apic_set_irq(lowest, irq, dest_map);
+	if (lowest) {
+		kvm_apic_set_irq(lowest, irq, dest_map);
+		r = 1;
+	}
 
 	return r;
 }
-- 
1.7.3.4

Re: [PATCH v2] KVM: x86: Remove support for reporting coalesced APIC IRQs

[Marcelo Tosatti]

On Sun, Apr 28, 2013 at 02:00:41PM +0200, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> Since the arrival of posted interrupt support we can no longer guarantee
> that coalesced IRQs are always reported to the IRQ source. Moreover,
> accumulated APIC timer events could cause a busy loop when a VCPU should
> rather be halted. The consensus is to remove coalesced tracking from the
> LAPIC.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
> 
> Changes in v2:
>  - preserve return values where we need to count to how many VCPUs an
>    IRQ was delivered

It would be best to confirm first that no guest depends on LAPIC timer
reinjection (rather than wait for guests to break and spend time
debugging). Specially since such kind of bugs are not easy to pinpoint.
Or if there is evidence to invalidate such reasoning, please point it
out.

Honestly i don't recall: would have to check RHEL5.32 UP/SMP, RHEL5.64
UP/SMP, RHEL4.32, RHEL4.64, RHEL6. Can do that in a week or so.

Re: [PATCH v2] KVM: x86: Remove support for reporting coalesced APIC IRQs

[Marcelo Tosatti]

On Thu, May 02, 2013 at 10:14:32PM -0300, Marcelo Tosatti wrote:
> On Sun, Apr 28, 2013 at 02:00:41PM +0200, Jan Kiszka wrote:
> > From: Jan Kiszka <jan.kiszka@siemens.com>
> > 
> > Since the arrival of posted interrupt support we can no longer guarantee
> > that coalesced IRQs are always reported to the IRQ source. Moreover,
> > accumulated APIC timer events could cause a busy loop when a VCPU should
> > rather be halted. The consensus is to remove coalesced tracking from the
> > LAPIC.
> > 
> > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> > ---
> > 
> > Changes in v2:
> >  - preserve return values where we need to count to how many VCPUs an
> >    IRQ was delivered
> 
> It would be best to confirm first that no guest depends on LAPIC timer
> reinjection (rather than wait for guests to break and spend time
> debugging). Specially since such kind of bugs are not easy to pinpoint.
> Or if there is evidence to invalidate such reasoning, please point it
> out.
> 
> Honestly i don't recall: would have to check RHEL5.32 UP/SMP, RHEL5.64
> UP/SMP, RHEL4.32, RHEL4.64, RHEL6. Can do that in a week or so.

Its OK to remove LAPIC timer reinjection.

Re: [PATCH v2] KVM: x86: Remove support for reporting coalesced APIC IRQs

[Gleb Natapov]

On Mon, May 13, 2013 at 10:30:33PM -0300, Marcelo Tosatti wrote:
> On Thu, May 02, 2013 at 10:14:32PM -0300, Marcelo Tosatti wrote:
> > On Sun, Apr 28, 2013 at 02:00:41PM +0200, Jan Kiszka wrote:
> > > From: Jan Kiszka <jan.kiszka@siemens.com>
> > > 
> > > Since the arrival of posted interrupt support we can no longer guarantee
> > > that coalesced IRQs are always reported to the IRQ source. Moreover,
> > > accumulated APIC timer events could cause a busy loop when a VCPU should
> > > rather be halted. The consensus is to remove coalesced tracking from the
> > > LAPIC.
> > > 
> > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> > > ---
> > > 
> > > Changes in v2:
> > >  - preserve return values where we need to count to how many VCPUs an
> > >    IRQ was delivered
> > 
> > It would be best to confirm first that no guest depends on LAPIC timer
> > reinjection (rather than wait for guests to break and spend time
> > debugging). Specially since such kind of bugs are not easy to pinpoint.
> > Or if there is evidence to invalidate such reasoning, please point it
> > out.
> > 
> > Honestly i don't recall: would have to check RHEL5.32 UP/SMP, RHEL5.64
> > UP/SMP, RHEL4.32, RHEL4.64, RHEL6. Can do that in a week or so.
> 
> Its OK to remove LAPIC timer reinjection.
Applied. Thanks for checking!

--
			Gleb.