Re: [Qemu-devel] [PATCH KVM v2 0/4] fix KVM i8259 IRQ trailing-edge logic

[Gleb Natapov]

On Wed, Dec 26, 2012 at 10:39:52PM -0700, Matthew Ogilvie wrote:
> Changes since version 1 (from Sep 9):
>    * Split off patch 1; this is the critical prerequisite to
>      make the i8254 work with the fixed i8259.
>    * Add patch 2, to make additional changes to the i8254
>      to make it consistent with the spec and with proposed changes
>      to qemu's i8254 model.
> 
> Background:
> 
> According to the spec, the i8259 will cancel an interrupt if
> the line goes low before it starts servicing it, even when
> it is considered edge-triggered.  This is a problem
> with an old Microport UNIX guest (ca 1987), where the
> guest masks off IRQ14 in the slave i8259, but the host's
> master i8259 model will still try to deliver an interrupt even after
> IRQ2 has been brought low, resulting in a spurious interrupt (IRQ15).
> 
> Before the i8259 can be fixed, the i8254 model needs to be fixed
> so it doesn't depend on the broken i8259.
> 
> Alternative: This could be narrowly fixed for IRQ2 only with no
> risk at all (and no need to touch the i8254), but if possible it
> seems reasonable to fix it for all lines at the same time.
> 
> But there may be some risk:
> 
> First, I think I've convinced myself that between the i8254 and i8259,
> the only substantial migration breakage should be when a
> whole series of conditions are met, and the combination
> should be rare enough not to worry about it.  See writeup
> in my qemu patch series (version 8; Dec 16).
> 
> Second, there is also the possibility that other devices are relying
> on the broken model.  I'm especially concerned with various users
> of a function called
> 
>         qemu_irq_pulse()
> 
> in the qemu source tree, which immediately lowers IRQ line after
> raising it.  If any of those calls are routed straight into
> the i8259 (as opposed to an APIC or some other chip), then it
> probably won't behave as desired.
> 
> I'll probably dig into qemu_irq_pulse() callers more carefully
> eventually, but there are lot of them, and any high-level incite
> anyone can provide into them would be helpful.
> 
$ git grep qemu_irq_pulse | wc -l
34

Files are:
hw/bonito.c
hw/dma.c
hw/grlib_apbuart.c
hw/grlib_gptimer.c
hw/hpet.c
hw/irq.h
hw/milkymist-ac97.c
hw/milkymist-minimac2.c
hw/milkymist-pfpu.c
hw/milkymist-softusb.c
hw/milkymist-sysctl.c
hw/milkymist-tmu2.c
hw/omap1.c
hw/omap_gptimer.c
hw/onenand.c
hw/spapr_events.c
hw/spapr_llan.c
hw/spapr_pci.c
hw/spapr_vio.c
hw/spapr_vty.c
hw/stellaris.c
hw/xilinx_ethlite.c

Looks like only two of those are relevant to PC platform hw/dma.c and
hw/hpet.c. In hw/dma.c it is used for internal qemu communication, not
real device. In hw/hpet.c from a quick glance it looks like _pulse is
only used when HPET in not in legacy emulation mode which means that
pulse should be directed to APIC.

> See the Dec 16 patch series I sent to the qemu mailing list for
> more information.
> http://search.gmane.org/?query=ogilvie&group=gmane.comp.emulators.qemu
> 
> Matthew Ogilvie (4):
>   KVM: fix i8254 IRQ0 to be normally high
>   KVM: additional i8254 output fixes
>   KVM: fix i8259 interrupt high to low transition logic
>   KVM: i8259: refactor pic_set_irq level logic
> 
>  arch/x86/kvm/i8254.c | 50 +++++++++++++++++++++++++++++++++++++++-----------
>  arch/x86/kvm/i8259.c | 36 ++++++++++++++----------------------
>  2 files changed, 53 insertions(+), 33 deletions(-)
> 
> -- 
> 1.7.10.2.484.gcd07cc5

--
                        Gleb.