diff mbox

KVM: x86: Remove support for reporting coalesced APIC IRQs

Message ID 517D0784.7040400@web.de (mailing list archive)
State New, archived
Headers show

Commit Message

Jan Kiszka April 28, 2013, 11:27 a.m. UTC
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(-)

Comments

Gleb Natapov April 28, 2013, 11:43 a.m. UTC | #1
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.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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;
 }