diff mbox

[1/2] KVM: PIC: call ack notifiers for irqs that are dropped form irr

Message ID 1343286046-8051-1-git-send-email-gleb@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Gleb Natapov July 26, 2012, 7 a.m. UTC
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.

Signed-off-by: Gleb Natapov <gleb@redhat.com>
---
 arch/x86/kvm/i8259.c |   17 +++++++++++++++++
 1 file changed, 17 insertions(+)

Comments

Avi Kivity July 26, 2012, 9:09 a.m. UTC | #1
On 07/26/2012 10:00 AM, Gleb Natapov wrote:
> 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.
> 
> Signed-off-by: Gleb Natapov <gleb@redhat.com>
> ---
>  arch/x86/kvm/i8259.c |   17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/arch/x86/kvm/i8259.c b/arch/x86/kvm/i8259.c
> index 81cf4fa..0147d16 100644
> --- a/arch/x86/kvm/i8259.c
> +++ b/arch/x86/kvm/i8259.c
> @@ -305,6 +305,11 @@ 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;
> +			int i;
> +			bool found;
> +			struct kvm_vcpu *vcpu;
> +
>  			s->init4 = val & 1;
>  			s->last_irr = 0;
>  			s->irr &= s->elcr;
> @@ -322,6 +327,18 @@ static void pic_ioport_write(void *opaque, u32 addr, u32 val)
>  			if (val & 0x08)
>  				pr_pic_unimpl(
>  					"level sensitive irq not supported");
> +
> +			kvm_for_each_vcpu(i, vcpu, s->pics_state->kvm)
> +				if (kvm_apic_accept_pic_intr(vcpu)) {
> +					found = true;
> +					break;
> +				}
> +
> +
> +			if (found)
> +				for (irq = 0; irq < PIC_NUM_PINS/2; irq++)
> +					if (edge_irr & (1 << irq))
> +						pic_clear_isr(s, irq);
>  		} else if (val & 0x08) {


This duplicates the code in pic_reset().  Please extract it into a
function (just the new code, since this is going into 3.6).
Gleb Natapov July 26, 2012, 9:11 a.m. UTC | #2
On Thu, Jul 26, 2012 at 12:09:36PM +0300, Avi Kivity wrote:
> On 07/26/2012 10:00 AM, Gleb Natapov wrote:
> > 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.
> > 
> > Signed-off-by: Gleb Natapov <gleb@redhat.com>
> > ---
> >  arch/x86/kvm/i8259.c |   17 +++++++++++++++++
> >  1 file changed, 17 insertions(+)
> > 
> > diff --git a/arch/x86/kvm/i8259.c b/arch/x86/kvm/i8259.c
> > index 81cf4fa..0147d16 100644
> > --- a/arch/x86/kvm/i8259.c
> > +++ b/arch/x86/kvm/i8259.c
> > @@ -305,6 +305,11 @@ 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;
> > +			int i;
> > +			bool found;
> > +			struct kvm_vcpu *vcpu;
> > +
> >  			s->init4 = val & 1;
> >  			s->last_irr = 0;
> >  			s->irr &= s->elcr;
> > @@ -322,6 +327,18 @@ static void pic_ioport_write(void *opaque, u32 addr, u32 val)
> >  			if (val & 0x08)
> >  				pr_pic_unimpl(
> >  					"level sensitive irq not supported");
> > +
> > +			kvm_for_each_vcpu(i, vcpu, s->pics_state->kvm)
> > +				if (kvm_apic_accept_pic_intr(vcpu)) {
> > +					found = true;
> > +					break;
> > +				}
> > +
> > +
> > +			if (found)
> > +				for (irq = 0; irq < PIC_NUM_PINS/2; irq++)
> > +					if (edge_irr & (1 << irq))
> > +						pic_clear_isr(s, irq);
> >  		} else if (val & 0x08) {
> 
> 
> This duplicates the code in pic_reset().  Please extract it into a
> function (just the new code, since this is going into 3.6).
> 
> 
Extract what into a function? I specifically sent follow up patch to
cleanup duplicates.

--
			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
Avi Kivity July 26, 2012, 9:18 a.m. UTC | #3
On 07/26/2012 12:11 PM, Gleb Natapov wrote:
> On Thu, Jul 26, 2012 at 12:09:36PM +0300, Avi Kivity wrote:
>> On 07/26/2012 10:00 AM, Gleb Natapov wrote:
>> > 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.
>> > 
>> > Signed-off-by: Gleb Natapov <gleb@redhat.com>
>> > ---
>> >  arch/x86/kvm/i8259.c |   17 +++++++++++++++++
>> >  1 file changed, 17 insertions(+)
>> > 
>> > diff --git a/arch/x86/kvm/i8259.c b/arch/x86/kvm/i8259.c
>> > index 81cf4fa..0147d16 100644
>> > --- a/arch/x86/kvm/i8259.c
>> > +++ b/arch/x86/kvm/i8259.c
>> > @@ -305,6 +305,11 @@ 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;
>> > +			int i;
>> > +			bool found;
>> > +			struct kvm_vcpu *vcpu;
>> > +
>> >  			s->init4 = val & 1;
>> >  			s->last_irr = 0;
>> >  			s->irr &= s->elcr;
>> > @@ -322,6 +327,18 @@ static void pic_ioport_write(void *opaque, u32 addr, u32 val)
>> >  			if (val & 0x08)
>> >  				pr_pic_unimpl(
>> >  					"level sensitive irq not supported");
>> > +
>> > +			kvm_for_each_vcpu(i, vcpu, s->pics_state->kvm)
>> > +				if (kvm_apic_accept_pic_intr(vcpu)) {
>> > +					found = true;
>> > +					break;
>> > +				}
>> > +
>> > +
>> > +			if (found)
>> > +				for (irq = 0; irq < PIC_NUM_PINS/2; irq++)
>> > +					if (edge_irr & (1 << irq))
>> > +						pic_clear_isr(s, irq);
>> >  		} else if (val & 0x08) {
>> 
>> 
>> This duplicates the code in pic_reset().  Please extract it into a
>> function (just the new code, since this is going into 3.6).
>> 
>> 
> Extract what into a function? I specifically sent follow up patch to
> cleanup duplicates.

So you did.  I didn't even look at it because I wasn't going to send
cleanup patches to 3.6.

I'll apply to master then.  Please repost the cleanup once 3.6 is updated.
diff mbox

Patch

diff --git a/arch/x86/kvm/i8259.c b/arch/x86/kvm/i8259.c
index 81cf4fa..0147d16 100644
--- a/arch/x86/kvm/i8259.c
+++ b/arch/x86/kvm/i8259.c
@@ -305,6 +305,11 @@  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;
+			int i;
+			bool found;
+			struct kvm_vcpu *vcpu;
+
 			s->init4 = val & 1;
 			s->last_irr = 0;
 			s->irr &= s->elcr;
@@ -322,6 +327,18 @@  static void pic_ioport_write(void *opaque, u32 addr, u32 val)
 			if (val & 0x08)
 				pr_pic_unimpl(
 					"level sensitive irq not supported");
+
+			kvm_for_each_vcpu(i, vcpu, s->pics_state->kvm)
+				if (kvm_apic_accept_pic_intr(vcpu)) {
+					found = true;
+					break;
+				}
+
+
+			if (found)
+				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;