[Qemu-devel] [PATCH 1/2] KVM: fix i8259 interrupt high to low transition logic

[Matthew Ogilvie]

Intel's definition of "edge triggered" means: "asserted with a
low-to-high transition at the time an interrupt is registered
and then kept high until the interrupt is served via one of the
EOI mechanisms or goes away unhandled."

So the only difference between edge triggered and level triggered
is in the leading edge, with no difference in the trailing edge.

This bug manifested itself when the guest was Microport UNIX
System V/386 v2.1 (ca. 1987), because it would sometimes mask
off IRQ14 in the slave IMR after it had already been asserted.
The master would still try to deliver an interrupt even though
IRQ2 had dropped again, resulting in a spurious interupt
(IRQ15) and a panicked UNIX kernel.

Signed-off-by: Matthew Ogilvie <address@hidden>
---

There has been some discussions about this on the qemu mailing
list; for example see
http://www.mail-archive.com/address@hidden/msg129071.html

This fixes the test program I wrote to demonstrate the problem.  See
http://home.comcast.net/~mmogilvi/downloads/i8259-imr-test-2012-09-02.tar.bz2
(for source code to a floppy disk boot sector) or a kvm-unit-test patch
I'm posting separately.

Unfortunately, Microport UNIX still has some some unknown interrupt
problem when run under qemu with KVM enabled, although it
runs fine under plain QEMU [no KVM] with the corresponding
QEMU patch.  The unknown interrupt bug actually triggers
much earlier than this one: this one doesn't show up until
the boot floppy accesses the hard drive after some prompts,
but the unknown bug shows up early in bootup.  Also, the specific
function in which UNIX panics is different: this
one in splint(); the unknown bug in splx().  Also, the KVM
behavior is not affected by the presense or absense of this patch.

I'll probably continue looking into the unknown problem, but I'm not
sure when I'll have any results.

Despite this unknown problem, I'm confident that this known problem
should be fixed, and if not fixed will cause problems when I figure
out the unknown problem.

 arch/x86/kvm/i8254.c |  6 +++++-
 arch/x86/kvm/i8259.c | 14 ++++++--------
 2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c
index adba28f..5cbba99 100644
--- a/arch/x86/kvm/i8254.c
+++ b/arch/x86/kvm/i8254.c
@@ -302,8 +302,12 @@ static void pit_do_work(struct kthread_work *work)
        }
        spin_unlock(&ps->inject_lock);
        if (inject) {
-               kvm_set_irq(kvm, kvm->arch.vpit->irq_source_id, 0, 1);
+               /* Clear previous interrupt, then create a rising
+                * edge to request another interupt, and leave it at
+                * level=1 until time to inject another one.
+                */
                kvm_set_irq(kvm, kvm->arch.vpit->irq_source_id, 0, 0);
+               kvm_set_irq(kvm, kvm->arch.vpit->irq_source_id, 0, 1);
 
                /*
                 * Provides NMI watchdog support via Virtual Wire mode.
diff --git a/arch/x86/kvm/i8259.c b/arch/x86/kvm/i8259.c
index e498b18..b0c2346 100644
--- a/arch/x86/kvm/i8259.c
+++ b/arch/x86/kvm/i8259.c
@@ -111,8 +111,10 @@ static inline int pic_set_irq1(struct kvm_kpic_state *s, 
int irq, int level)
                                s->irr |= mask;
                        }
                        s->last_irr |= mask;
-               } else
+               } else {
+                       s->irr &= ~mask;
                        s->last_irr &= ~mask;
+               }
 
        return (s->imr & mask) ? -1 : ret;
 }
@@ -169,14 +171,10 @@ static void pic_update_irq(struct kvm_pic *s)
 {
        int irq2, irq;
 
+       /* slave PIC notifies master PIC via IRQ2 */
        irq2 = pic_get_irq(&s->pics[1]);
-       if (irq2 >= 0) {
-               /*
-                * if irq request by slave pic, signal master PIC
-                */
-               pic_set_irq1(&s->pics[0], 2, 1);
-               pic_set_irq1(&s->pics[0], 2, 0);
-       }
+       pic_set_irq1(&s->pics[0], 2, irq2 >= 0);
+
        irq = pic_get_irq(&s->pics[0]);
        pic_irq_request(s->kvm, irq >= 0);
 }
-- 
1.7.10.2.484.gcd07cc5