KVM: PIC: enhance IPI avoidance

KVM: PIC: enhance IPI avoidance

The PIC code makes little effort to avoid kvm_vcpu_kick(), resulting in unnecessary guest exits in some conditions.

For example, if the timer interrupt is routed through the IOAPIC, IRR for IRQ 0 will get set but not cleared, since the APIC is handling the acks.

This means that everytime an interrupt < 16 is triggered, the priority logic will find IRQ0 pending and send an IPI to vcpu0 (in case IRQ0 is not masked, which is Linux's case).

Introduce a new variable isr_ack to represent the IRQ's for which the guest has been signalled / cleared the ISR. Use it to avoid more than one IPI per trigger-ack cycle, in addition to the avoidance when ISR is set in get_priority().

Signed-off-by Marcelo Tosatti <[email protected]>

Index: kvm/arch/x86/kvm/i8259.c
===================================================================

arch/x86/kvm/i8259.c +8 -2

@@ -33,6 +33,7 @@
 static void pic_clear_isr(struct kvm_kpic_state *s, int irq)
 {
 	s->isr &= ~(1 << irq);
+	s->isr_ack |= (1 << irq);
 }
 
 /*

@@ -213,6 +214,7 @@ void kvm_pic_reset(struct kvm_kpic_state
 	s->irr = 0;
 	s->imr = 0;
 	s->isr = 0;
+	s->isr_ack = 0xff;
 	s->priority_add = 0;
 	s->irq_base = 0;
 	s->read_reg_select = 0;

@@ -444,10 +446,14 @@ static void pic_irq_request(void *opaque
 {
 	struct kvm *kvm = opaque;
 	struct kvm_vcpu *vcpu = kvm->vcpus[0];
+	struct kvm_pic *s = pic_irqchip(kvm);
+	int irq = pic_get_irq(&s->pics[0]);
 
-	pic_irqchip(kvm)->output = level;
-	if (vcpu)
+	s->output = level;
+	if (vcpu && level && (s->pics[0].isr_ack & (1 << irq))) {
+		s->pics[0].isr_ack &= ~(1 << irq);
 		kvm_vcpu_kick(vcpu);
+	}
 }
 
 struct kvm_pic *kvm_create_pic(struct kvm *kvm)

Index: kvm/arch/x86/kvm/irq.h
===================================================================

arch/x86/kvm/irq.h +1 -1

@@ -42,6 +42,7 @@ struct kvm_kpic_state {
 	u8 irr;		/* interrupt request register */
 	u8 imr;		/* interrupt mask register */
 	u8 isr;		/* interrupt service register */
+	u8 isr_ack;	/* interrupt ack detection */
 	u8 priority_add;	/* highest irq priority */
 	u8 irq_base;
 	u8 read_reg_select;
--

To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

KVM: PIC: enhance IPI avoidance

The PIC code makes little effort to avoid kvm_vcpu_kick(), resulting in unnecessary guest exits in some conditions.

For example, if the timer interrupt is routed through the IOAPIC, IRR for IRQ 0 will get set but not cleared, since the APIC is handling the acks.

This means that everytime an interrupt < 16 is triggered, the priority logic will find IRQ0 pending and send an IPI to vcpu0 (in case IRQ0 is not masked, which is Linux's case).

Introduce a new variable isr_ack to represent the IRQ's for which the guest has been signalled / cleared the ISR. Use it to avoid more than one IPI per trigger-ack cycle, in addition to the avoidance when ISR is set in get_priority().

Signed-off-by Marcelo Tosatti <[email protected]>

struct kvm_pic *kvm_create_pic(struct kvm *kvm)
Index: kvm/arch/x86/kvm/irq.h
===================================================================

Quoted 7 lines

Index: kvm/arch/x86/kvm/irq.h
===================================================================

arch/x86/kvm/irq.h +0 -0

@@ -42,6 +42,7 @@ struct kvm_kpic_state {
 	u8 irr;		/* interrupt request register */
 	u8 imr;		/* interrupt mask register */

arch/x86/kvm/irq.h struct kvm_kpic_state { · line 48

Quoted 6 lines

arch/x86/kvm/irq.h +1 -0

 	u8 isr;		/* interrupt service register */
+	u8 isr_ack;	/* interrupt ack detection */
 	u8 priority_add;	/* highest irq priority */
 	u8 irq_base;
 	u8 read_reg_select;

Needs to be cleared by reset

and by register load from userspace, no?

Quoted 3 lines

and by register load from userspace, no?

Isnt that responsability of the guest?

Unacked IOAPIC interrupts are not
cleared on register load, are they?

Quoted 5 lines · Marcelo Tosatti

Quoted 3 lines

and by register load from userspace, no?

Isnt that responsability of the guest?

I'm talking about a restore to previous state scenario. In this case we want to disable any IPI avoidance in case it avoids a needed IPI.

Quoted 4 lines

Unacked IOAPIC interrupts are not
cleared on register load, are they?

Good question. I don't know if they should or shouldn't. But that's a different question. isr_ack is not guest visible, so nothing is lost from clearing it, but we can fail if we don't clear it.

True. Anything other potential problem you could think of?