Re: [PATCH v2] KVM: x86: Remove support for reporting coalesced APIC IRQs
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.kis...@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.kis...@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. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] KVM: x86: Remove support for reporting coalesced APIC IRQs
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.kis...@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.kis...@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. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] KVM: x86: Remove support for reporting coalesced APIC IRQs
On Sun, Apr 28, 2013 at 02:00:41PM +0200, Jan Kiszka wrote: From: Jan Kiszka jan.kis...@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.kis...@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. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] KVM: x86: Remove support for reporting coalesced APIC IRQs
From: Jan Kiszka jan.kis...@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.kis...@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 */
