[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH v4 5/5] i8259: fix dynamically masking slave IRQs wi
From: |
Matthew Ogilvie |
Subject: |
[Qemu-devel] [PATCH v4 5/5] i8259: fix dynamically masking slave IRQs with IMR register |
Date: |
Sun, 2 Sep 2012 20:56:14 -0600 |
Although I haven't found any specs that say so, on real hardware
I have a test program that shows if you mask off the slave
interrupt (say IRQ14) in the IMR after it has already been raised,
the master (IRQ2) gets canceled (that is, IRQ2 acts like it is level
triggered). Without this patch, qemu delivers a
spurious interrupt (IRQ15) instead when running the test program.
Signed-off-by: Matthew Ogilvie <address@hidden>
---
I've written a test program (in the form of a floppy disk boot sector)
that demonstrates that qemu's emulation of IRQ2 propagation from the
slave i8259 to the master does not work correctly when the CPU has
interrupts disabled and it masks off the original interrupt (IRQ14)
in the slave's IMR register. This was based on simplifying breakage
observed when trying to run an old Microport UNIX system (ca 1987).
Earlier I speculated that maybe the ELCR bit for IRQ2 was incorrect,
but now I don't think changing the bit (from the target's
perspective) would be a good idea. See below.
You can download the source code for the test program from
http://home.comcast.net/~mmogilvi/downloads/i8259-imr-test-2012-09-02.tar.bz2
It can be compiled using a standard GNU i386 toolchain on Linux.
The heart of the test program is:
cli
# i8259:imm: mask off everything except IRQ2
movb $0xfb,%al # master (only IRQ2 clear)
outb %al,$0x21
movb $0xff,%al # slave
outb %al,$0xa1
mov $.msgCmdRead,%ax
call print
call initIrqHandlers
call scheduleIrq14
call .largeDelay # note: IRQ14 raised while this is waiting
mov $.msgUnmask,%ax
call print
movb $0x3f,%al # unmask IRQ14 and IRQ15
outb %al,$0xa1
call .largeDelay # (probably not important)
mov $.msgMask,%ax
call print
movb $0xff,%al # mask IRQ14 and IRQ15 again
outb %al,$0xa1
call .largeDelay # (probably not important)
mov $.msgSti,%ax
call print
sti
call .largeDelay # note: spurious interrupt (IRQ15) from qemu
cli
mov $.msgUnmask,%ax
call print
movb $0x3f,%al # unmask IRQ14 and IRQ15
outb %al,$0xa1
sti
call .largeDelay # we should finally see IRQ14 here?
# DONE:
cli
movw $.msgDone, %ax
call print
.Lhalt:
hlt
jmp .Lhalt
In qemu, the master treats IRQ2 as edge triggered, and delivers a
spurious interrupt if the CPU re-enables interrupts after
the slave's IMR is masked off (it also doesn't seem to deliver
the real interrupt when the IMR is unmasked, but maybe that is
because I'm not correctly acknowledging the spurious interrupt).
- Qemu output (without this patch):
elcr=0c00 cmdRead ummask mask sti irq15 unmask DONE
But on real hardware, the master seems to treat IRQ2 as level triggered,
and doesn't deliver an interrupt to the CPU until the slave unmasks IRQ14.
I've tried this on several machines, including a non-PCI 486 that
does not seem to have ELCR registers.
- Typical output (pentium/pentium pro/dual AMD Athlon/etc; slight
variations in some elcr bits depending on machine, but bit 0x04
is always clear):
elcr=0c20 cmdRead unmask mask sti unmask irq14 DONE
- 486 without elcr (just an ISA bus):
elcr=e0e0 cmdRead unmask mask sti unmask irq14 DONE
- One failure: It doesn't boot properly (no output) with a USB floppy
drive on my Intel Core I7. Guess: The test program just barely
fits in a sector, with no room for any tables (partition/etc)
that BIOS might check for if it isn't an original, native floppy
drive.
-----------------------
I've found a few descriptions of programming the i8259.
The closest thing I've found to a detailed spec is in
"iAPX 86, 88 User's Manual", dated August 1981:
http://ebookbrowse.com/1981-iapx-86-88-users-manual-pdf-d3089962
It has a significant section titled "Using the 8259A Programmable
Interrupt Controller" starting on page 438 or A-135.
But none of my sources seem to specify how master/slave cascading
interacts with the IMR (mask register) and edge trigger mode,
although it talks about those things individually.
Also, given the date it isn't surprising that it doesn't mention
the elcr registers at all.
I have not found any real specs for the ELCR registers, but I've found a
few hints:
- Two 8 bit registers: one for master (0x4d0) and one for slave (0x4d1).
- One bit per IRQ line: 0 for edge trigger, 1 for level trigger.
- Not present unless the machine has EISA or PCI buses. Plain
ISA doesn't have it.
- Might be implemented completely external to the actual i8259s.
- As seen in test program output above, ELCR bit 0x04 is clear,
indicating that IRQ2 is supposedly edge triggered, even though
actual tested behavior is level triggered.
- A google search (8259 elcr -linux -qemu) [exclude the
dominant Linux and qemu related pages] found at least one operating
system that checks several ELCR bits (including IRQ2) as part
of a probe to determine if ELCR exists. So simply setting
the 0x04 bit is probably a bad idea. For example, line 119 of:
https://bitbucket.org/npe/nix/src/35d39df17077/src/9kron2/k8/i8259.c
-----------------------
My guess is that if a master IRQ line is marked as coming from a slave
(in the ICW3 register), then the i8259 treats that line as level
triggered regardless of ELCR registers or the LTIM bit of ICW1 (in
addition to other special slave line logic). Otherwise, the slave
IMR register would seem rather useless.
Under that theory, something like the following patch would be appropriate
for qemu. This fixes both the test program, and the original Microport
UNIX problem.
Possible concerns with this patch:
- KVM still needs to be fixed. I haven't researched this at all,
beyond noticing that it has the same broken behavior running the
test program, and the high level symptoms from Microport UNIX
are slightly different with KVM.
- It might also be a good idea to add something like my test
program to qemu/tests somewhere. This would be a separate patch.
- Best icw3 configuration strategy?: Should it be stored, or
just assume it is correctly configured based on
PICCommonState::master and standard PC IRQ2 configuration?
Perhaps add sanity checks for reasonable values when the
guest is configuring the 8259s? Did I catch all the places
that need to deal with a new state variable (assuming we
decide to store it)?
- This patch is adding code to what may be a relatively common
code path (every time interrupts raised/lowered/acknowledged, masks
changed, etc). Potentially it could be optimized by adding an
"effective_elcr" state variable, that is maintined as a combination
of icw3 and elcr (and maybe add support for LTIM bit from ICW1 at the
same time), so that we don't need to have extra code in the
critical path. Does anyone think this is worth it? I'm leaning
towards "no".
==========================================
hw/i8259.c | 12 ++++++++----
hw/i8259_common.c | 2 ++
hw/i8259_internal.h | 1 +
3 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/hw/i8259.c b/hw/i8259.c
index 6587666..8e2f9f4 100644
--- a/hw/i8259.c
+++ b/hw/i8259.c
@@ -140,7 +140,7 @@ static void pic_set_irq(void *opaque, int irq, int level)
}
#endif
- if (s->elcr & mask) {
+ if ((s->elcr | s->icw3) & mask) {
/* level triggered */
if (level) {
s->irr |= mask;
@@ -174,7 +174,7 @@ static void pic_intack(PICCommonState *s, int irq)
s->isr |= (1 << irq);
}
/* We don't clear a level sensitive interrupt here */
- if (!(s->elcr & (1 << irq))) {
+ if (!((s->elcr | s->icw3) & (1 << irq))) {
s->irr &= ~(1 << irq);
}
pic_update_irq(s);
@@ -314,6 +314,10 @@ static void pic_ioport_write(void *opaque,
target_phys_addr_t addr64,
s->init_state = s->single_mode ? (s->init4 ? 3 : 0) : 2;
break;
case 2:
+ s->icw3 = val;
+ /* TODO: Enforce constraints?: Master is typically
+ * 0x04 for IRQ2 (maybe 0 is also OK). Slave must be 0.
+ */
if (s->init4) {
s->init_state = 3;
} else {
@@ -419,9 +423,9 @@ void pic_info(Monitor *mon)
for (i = 0; i < 2; i++) {
s = i == 0 ? DO_UPCAST(PICCommonState, dev.qdev, isa_pic) : slave_pic;
monitor_printf(mon, "pic%d: irr=%02x imr=%02x isr=%02x hprio=%d "
- "irq_base=%02x rr_sel=%d elcr=%02x fnm=%d\n",
+ "irq_base=%02x icw3=%02x rr_sel=%d elcr=%02x fnm=%d\n",
i, s->irr, s->imr, s->isr, s->priority_add,
- s->irq_base, s->read_reg_select, s->elcr,
+ s->irq_base, s->icw3, s->read_reg_select, s->elcr,
s->special_fully_nested_mode);
}
}
diff --git a/hw/i8259_common.c b/hw/i8259_common.c
index ab3d98b..dcde5f2 100644
--- a/hw/i8259_common.c
+++ b/hw/i8259_common.c
@@ -33,6 +33,7 @@ void pic_reset_common(PICCommonState *s)
s->isr = 0;
s->priority_add = 0;
s->irq_base = 0;
+ s->icw3 = 0;
s->read_reg_select = 0;
s->poll = 0;
s->special_mask = 0;
@@ -111,6 +112,7 @@ static const VMStateDescription vmstate_pic_common = {
VMSTATE_UINT8(isr, PICCommonState),
VMSTATE_UINT8(priority_add, PICCommonState),
VMSTATE_UINT8(irq_base, PICCommonState),
+ VMSTATE_UINT8(icw3, PICCommonState),
VMSTATE_UINT8(read_reg_select, PICCommonState),
VMSTATE_UINT8(poll, PICCommonState),
VMSTATE_UINT8(special_mask, PICCommonState),
diff --git a/hw/i8259_internal.h b/hw/i8259_internal.h
index 4137b61..de67d49 100644
--- a/hw/i8259_internal.h
+++ b/hw/i8259_internal.h
@@ -55,6 +55,7 @@ struct PICCommonState {
uint8_t isr; /* interrupt service register */
uint8_t priority_add; /* highest irq priority */
uint8_t irq_base;
+ uint8_t icw3; /* bit set if line is connected to a slave */
uint8_t read_reg_select;
uint8_t poll;
uint8_t special_mask;
--
1.7.10.2.484.gcd07cc5
- [Qemu-devel] [PATCH v4 0/5] Running Microport UNIX (ca 1987), Matthew Ogilvie, 2012/09/02
- [Qemu-devel] [PATCH v4 1/5] fix some debug printf format strings, Matthew Ogilvie, 2012/09/02
- [Qemu-devel] [PATCH v4 2/5] vl: fix -hdachs/-hda argument order parsing issues, Matthew Ogilvie, 2012/09/02
- [Qemu-devel] [PATCH v4 3/5] qemu-options.hx: mention retrace= VGA option, Matthew Ogilvie, 2012/09/02
- [Qemu-devel] [PATCH v4 4/5] vga: add some optional CGA compatibility hacks, Matthew Ogilvie, 2012/09/02
- [Qemu-devel] [PATCH v4 5/5] i8259: fix dynamically masking slave IRQs with IMR register,
Matthew Ogilvie <=
- Re: [Qemu-devel] [PATCH v4 5/5] i8259: fix dynamically masking slave IRQs with IMR register, Paolo Bonzini, 2012/09/03
- Re: [Qemu-devel] [PATCH v4 5/5] i8259: fix dynamically masking slave IRQs with IMR register, Andreas Färber, 2012/09/03
- Re: [Qemu-devel] [PATCH v4 5/5] i8259: fix dynamically masking slave IRQs with IMR register, Avi Kivity, 2012/09/03
- Re: [Qemu-devel] [PATCH v4 5/5] i8259: fix dynamically masking slave IRQs with IMR register, Juan Quintela, 2012/09/03
- Re: [Qemu-devel] [PATCH v4 5/5] i8259: fix dynamically masking slave IRQs with IMR register, Jan Kiszka, 2012/09/03
- Re: [Qemu-devel] [PATCH v4 5/5] i8259: fix dynamically masking slave IRQs with IMR register, Avi Kivity, 2012/09/03
- Re: [Qemu-devel] [PATCH v4 5/5] i8259: fix dynamically masking slave IRQs with IMR register, Jan Kiszka, 2012/09/03
- Re: [Qemu-devel] [PATCH v4 5/5] i8259: fix dynamically masking slave IRQs with IMR register, Avi Kivity, 2012/09/03
- Re: [Qemu-devel] [PATCH v4 5/5] i8259: fix dynamically masking slave IRQs with IMR register, Jan Kiszka, 2012/09/03
- Re: [Qemu-devel] [PATCH v4 5/5] i8259: fix dynamically masking slave IRQs with IMR register, Avi Kivity, 2012/09/03