Skip to contentSkip to conversation

KVM: PIC: call ack notifiers for irqs that are dropped form irr

PATCH In review Gleb Natapov 2 participants 6 messages Jul 17, 2012 – Jul 24, 2012

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 KVM: PIC: call ack notifiers for irqs that are dropped form irr

After commit 242ec97c358256 PIT interrupts are no longer delivered after PIC reset. It happens because PIT injects interrupt only if previous one was acked, but since on PIC reset it is dropped from irr it will never be delivered and hence acknowledged. Fix that by calling ack notifier on PIC reset.

arch/x86/kvm/i8259.c +4 -0
index 81cf4fa..f09e790 100644
@@ -305,6 +305,7 @@ static void pic_ioport_write(void *opaque, u32 addr, u32 val)
 	addr &= 1;
 	if (addr == 0) {
 		if (val & 0x10) {
+			u8 edge_irr = s->irr & ~s->elcr;
 			s->init4 = val & 1;
 			s->last_irr = 0;
 			s->irr &= s->elcr;
@@ -322,6 +323,9 @@ static void pic_ioport_write(void *opaque, u32 addr, u32 val)
 			if (val & 0x08)
 				pr_pic_unimpl(
 					"level sensitive irq not supported");
+			for (irq = 0; irq < PIC_NUM_PINS/2; irq++)
+				if (edge_irr & (1 << irq))
+					pic_clear_isr(s, irq);
 		} else if (val & 0x08) {
 			if (val & 0x04)
 				s->poll = 1;

--
Gleb.

#1 lore raw
Quoted 30 lines · Gleb Natapov

After commit 242ec97c358256 PIT interrupts are no longer delivered after PIC reset. It happens because PIT injects interrupt only if previous one was acked, but since on PIC reset it is dropped from irr it will never be delivered and hence acknowledged. Fix that by calling ack notifier on PIC reset.

arch/x86/kvm/i8259.c +4 -1
index 81cf4fa..f09e790 100644
@@ -305,6 +305,7 @@ static void pic_ioport_write(void *opaque, u32 addr, u32 val)
 	addr &= 1;
 	if (addr == 0) {
 		if (val & 0x10) {
+			u8 edge_irr = s->irr & ~s->elcr;
 			s->init4 = val & 1;
 			s->last_irr = 0;
 			s->irr &= s->elcr;
@@ -322,6 +323,9 @@ static void pic_ioport_write(void *opaque, u32 addr, u32 val)
 			if (val & 0x08)
 				pr_pic_unimpl(
 					"level sensitive irq not supported");
+			for (irq = 0; irq < PIC_NUM_PINS/2; irq++)
+				if (edge_irr & (1 << irq))
+					pic_clear_isr(s, irq);
 		} else if (val & 0x08) {
 			if (val & 0x04)
 				s->poll = 1;
--
Quoted 1 line

Gleb.

Can modify kvm_pic_reset (currently unused BTW) to conform to 9ed049c3b6230b6898 ? It checks for APIC handling interrupts before acking.

#2 lore raw
Quoted 28 lines · Marcelo Tosatti
Quoted 27 lines · Gleb Natapov

After commit 242ec97c358256 PIT interrupts are no longer delivered after PIC reset. It happens because PIT injects interrupt only if previous one was acked, but since on PIC reset it is dropped from irr it will never be delivered and hence acknowledged. Fix that by calling ack notifier on PIC reset.

arch/x86/kvm/i8259.c +4 -0
index 81cf4fa..f09e790 100644
@@ -305,6 +305,7 @@ static void pic_ioport_write(void *opaque, u32 addr, u32 val)
 	addr &= 1;
 	if (addr == 0) {
 		if (val & 0x10) {
+			u8 edge_irr = s->irr & ~s->elcr;
 			s->init4 = val & 1;
 			s->last_irr = 0;
 			s->irr &= s->elcr;
@@ -322,6 +323,9 @@ static void pic_ioport_write(void *opaque, u32 addr, u32 val)
 			if (val & 0x08)
 				pr_pic_unimpl(
 					"level sensitive irq not supported");
+			for (irq = 0; irq < PIC_NUM_PINS/2; irq++)
+				if (edge_irr & (1 << irq))
+					pic_clear_isr(s, irq);
 		} else if (val & 0x08) {
Quoted 8 lines
Quoted 4 lines
arch/x86/kvm/i8259.c +0 -1
 			if (val & 0x04)
 				s->poll = 1;
--

Gleb.

Can modify kvm_pic_reset (currently unused BTW) to conform to 9ed049c3b6230b6898 ? It checks for APIC handling interrupts before acking.

Since it is not used we can make it do anything. But preferably in speared patch otherwise bug fix will be obfuscated by code move.

--
Gleb.

#3 lore raw
In reply to Gleb Natapov

Ping. Is some additional work expected from me before this is applied?

Quoted 30 lines · Gleb Natapov

After commit 242ec97c358256 PIT interrupts are no longer delivered after PIC reset. It happens because PIT injects interrupt only if previous one was acked, but since on PIC reset it is dropped from irr it will never be delivered and hence acknowledged. Fix that by calling ack notifier on PIC reset.

arch/x86/kvm/i8259.c +4 -1
index 81cf4fa..f09e790 100644
@@ -305,6 +305,7 @@ static void pic_ioport_write(void *opaque, u32 addr, u32 val)
 	addr &= 1;
 	if (addr == 0) {
 		if (val & 0x10) {
+			u8 edge_irr = s->irr & ~s->elcr;
 			s->init4 = val & 1;
 			s->last_irr = 0;
 			s->irr &= s->elcr;
@@ -322,6 +323,9 @@ static void pic_ioport_write(void *opaque, u32 addr, u32 val)
 			if (val & 0x08)
 				pr_pic_unimpl(
 					"level sensitive irq not supported");
+			for (irq = 0; irq < PIC_NUM_PINS/2; irq++)
+				if (edge_irr & (1 << irq))
+					pic_clear_isr(s, irq);
 		} else if (val & 0x08) {
 			if (val & 0x04)
 				s->poll = 1;
--
Quoted 5 lines

Gleb.
--
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

--
Gleb.

#4 lore raw
Quoted 1 line · Gleb Natapov

Ping. Is some additional work expected from me before this is applied?

Still not clear to me why its safe to skip kvm_apic_accept_pic_intr check before calling ack notifier.

Quoted 33 lines · Gleb Natapov
Quoted 32 lines

After commit 242ec97c358256 PIT interrupts are no longer delivered after PIC reset. It happens because PIT injects interrupt only if previous one was acked, but since on PIC reset it is dropped from irr it will never be delivered and hence acknowledged. Fix that by calling ack notifier on PIC reset.

arch/x86/kvm/i8259.c +4 -1
index 81cf4fa..f09e790 100644
@@ -305,6 +305,7 @@ static void pic_ioport_write(void *opaque, u32 addr, u32 val)
 	addr &= 1;
 	if (addr == 0) {
 		if (val & 0x10) {
+			u8 edge_irr = s->irr & ~s->elcr;
 			s->init4 = val & 1;
 			s->last_irr = 0;
 			s->irr &= s->elcr;
@@ -322,6 +323,9 @@ static void pic_ioport_write(void *opaque, u32 addr, u32 val)
 			if (val & 0x08)
 				pr_pic_unimpl(
 					"level sensitive irq not supported");
+			for (irq = 0; irq < PIC_NUM_PINS/2; irq++)
+				if (edge_irr & (1 << irq))
+					pic_clear_isr(s, irq);
 		} else if (val & 0x08) {
 			if (val & 0x04)
 				s->poll = 1;
--

Gleb.
--

#5 lore raw
Quoted 7 lines · Marcelo Tosatti
Quoted 1 line · Gleb Natapov

Ping. Is some additional work expected from me before this is applied?

Still not clear to me why its safe to skip kvm_apic_accept_pic_intr check before calling ack notifier.

I do not think it is actually.

Quoted 25 lines
Quoted 25 lines · Gleb Natapov
Quoted 24 lines

After commit 242ec97c358256 PIT interrupts are no longer delivered after PIC reset. It happens because PIT injects interrupt only if previous one was acked, but since on PIC reset it is dropped from irr it will never be delivered and hence acknowledged. Fix that by calling ack notifier on PIC reset.

arch/x86/kvm/i8259.c +2 -0
index 81cf4fa..f09e790 100644
@@ -305,6 +305,7 @@ static void pic_ioport_write(void *opaque, u32 addr, u32 val)
 	addr &= 1;
 	if (addr == 0) {
 		if (val & 0x10) {
+			u8 edge_irr = s->irr & ~s->elcr;
 			s->init4 = val & 1;
 			s->last_irr = 0;
 			s->irr &= s->elcr;
@@ -322,6 +323,9 @@ static void pic_ioport_write(void *opaque, u32 addr, u32 val)
 			if (val & 0x08)
 				pr_pic_unimpl(
 					"level sensitive irq not supported");
+			for (irq = 0; irq < PIC_NUM_PINS/2; irq++)
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
arch/x86/kvm/i8259.c +2 -1
+				if (edge_irr & (1 << irq))
+					pic_clear_isr(s, irq);
 		} else if (val & 0x08) {
 			if (val & 0x04)
 				s->poll = 1;
--

Gleb.
--

--
Gleb.