[Qemu-devel] [PATCH v8 00/10] i8254, i8259 and running Microport UNIX (ca 1987)

[Qemu-devel] [PATCH v8 00/10] i8254, i8259 and running Microport UNIX (ca 1987)

[Matthew Ogilvie]

This series makes a series of mostly-unrelated fixes to allow
running an old Microport UNIX (ca 1987) guest under qemu.

The most intrusive of the fixes is modifying how the i8259 handles
the trailing edge of an interrupt request (the trailing edge ought
to cancel interrupt, even when the line is edge-triggered).

Changes since version 7:
   * Abandon attempts to be fancy about handling cross version migration
     in the i8254 model.  Just fix it directly.  Analysis suggests
     migration issues should essentially always be minor - see comments
     for patch 5.
   * Added qtest-based test/pic-test.c (and associated infrastructure)
     to test the i8259's handling of the trailing edge of an interrupt
     request, to prevent future regressions.

I'm still wondering if maybe the non-i825[49]-related patches should be
split off from the rest of this series?  Any chance some of them (at
least the trivial ones) could just be included immediately?

I still need to refine the KVM patches I posted back in September
to match these latest changes.  Maybe I'll get to that next weekend.

------------
Potential issue with this:
------------

I'm not sure if it is a problem or not, but while figuring out
how to hookup pic-test.c, I noticed the existence of the

        qemu_irq_pulse()

function in hw/irq.h.  It is raising an IRQ line only
to immediately lower it again, which for a real i8259
(at least) effectively cancels the interrupt request before
it could ever be serviced.  It is used in the following files:

    hw/bonito.c
    hw/dma.c
    hw/grlib_apbuart.c
    hw/grlib_gptimer.c
    hw/hpet.c
    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

If any of these calls are ever routed into the i8259, then I doubt
they will behave as desired once the i8259 is fixed to handle the
trailing edge of an interrupt request as indicated in the
spec (patch 6).  (That is, the devices use over-simplified interrupt logic
that relies on the currently broken behavior of the i8259 model.)

On the other hand, maybe some or all of these devices are only used
in situations where the the IRQ line is fed into something else (like
an ioapic configured to invert the polarity of the line, or perhaps
some other non-intel interrupt chip that doesn't have this
canceling behavior), in which case they should be fine.

Can anyone shed some light on this?  I don't really know much
of anything about any of the devices in the above list (or any
other devices that might manually do something similar without using
qemu_irq_pulse()), so I'm not sure if there are real problems here
or not.

If more than a couple of these devices are wrong, then I'm tempted to
only fix the cascade interrupt (IRQ2) in the i8259 (leaving others
as-is), until such time as all these device models are improved.

-----

Matthew Ogilvie (10):
  fix some debug printf format strings
  vl: fix -hdachs/-hda argument order parsing issues
  qemu-options.hx: mention retrace= VGA option
  vga: add some optional CGA compatibility hacks
  fix i8254 output logic to match the spec
  i8259: fix so that dropping IRQ level always clears the interrupt
    request
  i8259: refactor pic_set_irq level logic
  qtest: fix qemu_irq_intercept_out()
  qtest: add set_irq_{in,out} infrastructure for testing interrupt
    controllers
  add test/pic-test.c

 hw/cirrus_vga.c   |   4 +-
 hw/i8254.c        |  24 ++++++++++-
 hw/i8254_common.c |  18 +++-----
 hw/i8259.c        |  34 +++++++--------
 hw/ide/cmd646.c   |   5 ++-
 hw/ide/via.c      |   5 ++-
 hw/irq.c          |  11 +++--
 hw/irq.h          |   2 +-
 hw/pc.h           |   4 ++
 hw/vga.c          |  37 ++++++++++++----
 qemu-options.hx   |  27 +++++++++++-
 qtest.c           |  53 ++++++++++++++++++++++-
 tests/Makefile    |   2 +
 tests/libqtest.c  |  12 ++++++
 tests/libqtest.h  |  48 +++++++++++++++++++++
 tests/pic-test.c  | 127 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 vl.c              |  62 +++++++++++++++++---------
 17 files changed, 403 insertions(+), 72 deletions(-)
 create mode 100644 tests/pic-test.c

-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH v8 01/10] fix some debug printf format strings

[Matthew Ogilvie]

These are normally ifdefed out and don't matter.  But if you enable
them, they ought to be correct.

Signed-off-by: Matthew Ogilvie <mmogilvi_qemu@miniinfo.net>
---
 hw/cirrus_vga.c | 4 ++--
 hw/i8259.c      | 3 ++-
 hw/ide/cmd646.c | 5 +++--
 hw/ide/via.c    | 5 +++--
 4 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c
index 40efa8a..84f2f16 100644
--- a/hw/cirrus_vga.c
+++ b/hw/cirrus_vga.c
@@ -2053,8 +2053,8 @@ static void cirrus_vga_mem_write(void *opaque,
 	}
     } else {
 #ifdef DEBUG_CIRRUS
-        printf("cirrus: mem_writeb " TARGET_FMT_plx " value %02x\n", addr,
-               mem_value);
+        printf("cirrus: mem_writeb " TARGET_FMT_plx " value %02" PRIx64 "\n",
+               addr, mem_value);
 #endif
     }
 }
diff --git a/hw/i8259.c b/hw/i8259.c
index af0ba4d..60c25ba 100644
--- a/hw/i8259.c
+++ b/hw/i8259.c
@@ -355,7 +355,8 @@ static uint64_t pic_ioport_read(void *opaque, hwaddr addr,
             ret = s->imr;
         }
     }
-    DPRINTF("read: addr=0x%02x val=0x%02x\n", addr, ret);
+    DPRINTF("read: addr=0x%02" TARGET_PRIxPHYS " val=0x%02x\n",
+            addr, ret);
     return ret;
 }
 
diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index 804db60..72ceaaf 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -154,7 +154,7 @@ static uint64_t bmdma_read(void *opaque, hwaddr addr,
         break;
     }
 #ifdef DEBUG_IDE
-    printf("bmdma: readb 0x%02x : 0x%02x\n", addr, val);
+    printf("bmdma: readb 0x%02" TARGET_PRIxPHYS " : 0x%02x\n", addr, val);
 #endif
     return val;
 }
@@ -170,7 +170,8 @@ static void bmdma_write(void *opaque, hwaddr addr,
     }
 
 #ifdef DEBUG_IDE
-    printf("bmdma: writeb 0x%02x : 0x%02x\n", addr, val);
+    printf("bmdma: writeb 0x%02" TARGET_PRIxPHYS " : 0x%02" PRIx64 "\n",
+           addr, val);
 #endif
     switch(addr & 3) {
     case 0:
diff --git a/hw/ide/via.c b/hw/ide/via.c
index efda173..10ba9b5 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -55,7 +55,7 @@ static uint64_t bmdma_read(void *opaque, hwaddr addr,
         break;
     }
 #ifdef DEBUG_IDE
-    printf("bmdma: readb 0x%02x : 0x%02x\n", addr, val);
+    printf("bmdma: readb 0x%02" TARGET_PRIxPHYS " : 0x%02x\n", addr, val);
 #endif
     return val;
 }
@@ -70,7 +70,8 @@ static void bmdma_write(void *opaque, hwaddr addr,
     }
 
 #ifdef DEBUG_IDE
-    printf("bmdma: writeb 0x%02x : 0x%02x\n", addr, val);
+    printf("bmdma: writeb 0x%02" TARGET_PRIxPHYS " : 0x%02" PRIx64 "\n",
+           addr, val);
 #endif
     switch (addr & 3) {
     case 0:
-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH v8 02/10] vl: fix -hdachs/-hda argument order parsing issues

[Matthew Ogilvie]

Without this patch, the -hdachs argument had to occur either
BEFORE the corresponding "-hda" option, or AFTER the plain
disk image name (if neither -hda nor -drive is used).  Otherwise
it would effectively be ignored.

Option -hdachs still has no effect on -drive, but that seems best.

Signed-off-by: Matthew Ogilvie <mmogilvi_qemu@miniinfo.net>
---
 vl.c | 39 ++++++++++++++++++---------------------
 1 file changed, 18 insertions(+), 21 deletions(-)

diff --git a/vl.c b/vl.c
index 3ebf01f..12786f0 100644
--- a/vl.c
+++ b/vl.c
@@ -2528,8 +2528,9 @@ int main(int argc, char **argv, char **envp)
     const char *kernel_filename, *kernel_cmdline;
     char boot_devices[33] = "cad"; /* default to HD->floppy->CD-ROM */
     DisplayState *ds;
-    int cyls, heads, secs, translation;
-    QemuOpts *hda_opts = NULL, *opts, *machine_opts;
+    char hdachs_params[512];  /* save -hdachs to apply to later -hda */
+    QemuOpts *hda_opts = NULL; /* save -hda to be modified by later -hdachs */
+    QemuOpts *opts, *machine_opts;
     QemuOptsList *olist;
     int optind;
     const char *optarg;
@@ -2584,8 +2585,7 @@ int main(int argc, char **argv, char **envp)
     cpu_model = NULL;
     ram_size = 0;
     snapshot = 0;
-    cyls = heads = secs = 0;
-    translation = BIOS_ATA_TRANSLATION_AUTO;
+    snprintf(hdachs_params, sizeof(hdachs_params), "%s", HD_OPTS);
 
     for (i = 0; i < MAX_NODES; i++) {
         node_mem[i] = 0;
@@ -2633,7 +2633,7 @@ int main(int argc, char **argv, char **envp)
         if (optind >= argc)
             break;
         if (argv[optind][0] != '-') {
-	    hda_opts = drive_add(IF_DEFAULT, 0, argv[optind++], HD_OPTS);
+            hda_opts = drive_add(IF_DEFAULT, 0, argv[optind++], hdachs_params);
         } else {
             const QEMUOption *popt;
 
@@ -2656,21 +2656,8 @@ int main(int argc, char **argv, char **envp)
                 cpu_model = optarg;
                 break;
             case QEMU_OPTION_hda:
-                {
-                    char buf[256];
-                    if (cyls == 0)
-                        snprintf(buf, sizeof(buf), "%s", HD_OPTS);
-                    else
-                        snprintf(buf, sizeof(buf),
-                                 "%s,cyls=%d,heads=%d,secs=%d%s",
-                                 HD_OPTS , cyls, heads, secs,
-                                 translation == BIOS_ATA_TRANSLATION_LBA ?
-                                 ",trans=lba" :
-                                 translation == BIOS_ATA_TRANSLATION_NONE ?
-                                 ",trans=none" : "");
-                    drive_add(IF_DEFAULT, 0, optarg, buf);
-                    break;
-                }
+                hda_opts = drive_add(IF_DEFAULT, 0, optarg, hdachs_params);
+                break;
             case QEMU_OPTION_hdb:
             case QEMU_OPTION_hdc:
             case QEMU_OPTION_hdd:
@@ -2704,7 +2691,10 @@ int main(int argc, char **argv, char **envp)
                 break;
             case QEMU_OPTION_hdachs:
                 {
+                    int cyls, heads, secs, translation;
                     const char *p;
+                    cyls = heads = secs = 0;
+                    translation = BIOS_ATA_TRANSLATION_AUTO;
                     p = optarg;
                     cyls = strtol(p, (char **)&p, 0);
                     if (cyls < 1 || cyls > 16383)
@@ -2736,7 +2726,14 @@ int main(int argc, char **argv, char **envp)
                         fprintf(stderr, "qemu: invalid physical CHS format\n");
                         exit(1);
                     }
-		    if (hda_opts != NULL) {
+                    snprintf(hdachs_params, sizeof(hdachs_params),
+                             "%s,cyls=%d,heads=%d,secs=%d%s",
+                             HD_OPTS , cyls, heads, secs,
+                             translation == BIOS_ATA_TRANSLATION_LBA ?
+                             ",trans=lba" :
+                             translation == BIOS_ATA_TRANSLATION_NONE ?
+                             ",trans=none" : "");
+                    if (hda_opts != NULL) {
                         char num[16];
                         snprintf(num, sizeof(num), "%d", cyls);
                         qemu_opt_set(hda_opts, "cyls", num);
-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH v8 03/10] qemu-options.hx: mention retrace= VGA option

[Matthew Ogilvie]

The feature was added in commit cb5a7aa8c32141bb Sep 2008.
My description is based on "Better VGA retrace emulation (needed
for some DOS games/demos)" from
http://www.boblycat.org/~malc/code/patches/qemu/index.html

Signed-off-by: Matthew Ogilvie <mmogilvi_qemu@miniinfo.net>
---
 qemu-options.hx | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 231cc4f..c50f737 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1007,7 +1007,7 @@ DEF("vga", HAS_ARG, QEMU_OPTION_vga,
     "-vga [std|cirrus|vmware|qxl|xenfb|none]\n"
     "                select video card type\n", QEMU_ARCH_ALL)
 STEXI
-@item -vga @var{type}
+@item -vga @var{type}[,@var{prop}=@var{value}[,...]]
 @findex -vga
 Select type of VGA card to emulate. Valid values for @var{type} are
 @table @option
@@ -1032,6 +1032,12 @@ Recommended choice when using the spice protocol.
 @item none
 Disable VGA card.
 @end table
+Valid optional properties are
+@table @option
+@item retrace=dumb|precise
+Select dumb (default) or precise VGA retrace logic, useful for some
+DOS games/demos.
+@end table
 ETEXI
 
 DEF("full-screen", 0, QEMU_OPTION_full_screen,
-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH v8 04/10] vga: add some optional CGA compatibility hacks

[Matthew Ogilvie]

This patch adds some optional compatibility hacks (default
disabled) to allow Microport UNIX to function under qemu.

I've tried to structure it to be easy to add more hacks for other
old CGA programs, if anyone ever needs them.

Microport UNIX System V/386 v 2.1 (ca 1987) tries to program
the CGA registers directly with neither the assistance of BIOS, nor
with proper handling of EGA/VGA-only registers.  Note that it didn't
work on real VGA hardware, either (although in that case, the most
obvious problems seemed to be out-of-range hsync and/or vsync
signalling, rather than the issues in this patch).

Eventually real MDA and/or CGA support might provide an alternative to
this patch, although a hybrid approach like this patch might still
be useful in marginal cases.

Signed-off-by: Matthew Ogilvie <mmogilvi_qemu@miniinfo.net>
---
 hw/pc.h         |  4 ++++
 hw/vga.c        | 37 +++++++++++++++++++++++++++++--------
 qemu-options.hx | 19 +++++++++++++++++++
 vl.c            | 23 +++++++++++++++++++++++
 4 files changed, 75 insertions(+), 8 deletions(-)

diff --git a/hw/pc.h b/hw/pc.h
index 2237e86..9b9538b 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -152,6 +152,10 @@ enum vga_retrace_method {
 
 extern enum vga_retrace_method vga_retrace_method;
 
+#define VGA_CGA_HACK_PALETTE_BLANKING  (1<<0)
+#define VGA_CGA_HACK_FONT_HEIGHT       (1<<1)
+extern int vga_cga_hacks;
+
 int isa_vga_mm_init(hwaddr vram_base,
                     hwaddr ctrl_base, int it_shift,
                     MemoryRegion *address_space);
diff --git a/hw/vga.c b/hw/vga.c
index c266161..2ddd09d 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -552,14 +552,31 @@ void vga_ioport_write(void *opaque, uint32_t addr, uint32_t val)
         printf("vga: write CR%x = 0x%02x\n", s->cr_index, val);
 #endif
         /* handle CR0-7 protection */
-        if ((s->cr[VGA_CRTC_V_SYNC_END] & VGA_CR11_LOCK_CR0_CR7) &&
-            s->cr_index <= VGA_CRTC_OVERFLOW) {
-            /* can always write bit 4 of CR7 */
-            if (s->cr_index == VGA_CRTC_OVERFLOW) {
-                s->cr[VGA_CRTC_OVERFLOW] = (s->cr[VGA_CRTC_OVERFLOW] & ~0x10) |
-                    (val & 0x10);
+        if (s->cr[VGA_CRTC_V_SYNC_END] & VGA_CR11_LOCK_CR0_CR7) {
+            if (s->cr_index <= VGA_CRTC_OVERFLOW) {
+                /* can always write bit 4 of CR7 */
+                if (s->cr_index == VGA_CRTC_OVERFLOW) {
+                    s->cr[VGA_CRTC_OVERFLOW] =
+                        (s->cr[VGA_CRTC_OVERFLOW] & ~0x10) | (val & 0x10);
+                }
+                return;
+            } else if ((vga_cga_hacks & VGA_CGA_HACK_FONT_HEIGHT) &&
+                       !(s->sr[VGA_SEQ_CLOCK_MODE] & VGA_SR01_CHAR_CLK_8DOTS)) {
+                /* extra CGA compatibility hacks (not in standard VGA) */
+                if (s->cr_index == VGA_CRTC_MAX_SCAN &&
+                    val == 7 &&
+                    (s->cr[VGA_CRTC_MAX_SCAN] & 0xf) == 0xf) {
+                    return;
+                } else if (s->cr_index == VGA_CRTC_CURSOR_START &&
+                           val == 6 &&
+                           (s->cr[VGA_CRTC_MAX_SCAN] & 0xf) == 0xf) {
+                    val = 0xd;
+                } else if (s->cr_index == VGA_CRTC_CURSOR_END &&
+                           val == 7 &&
+                           (s->cr[VGA_CRTC_MAX_SCAN] & 0xf) == 0xf) {
+                    val = 0xe;
+                }
             }
-            return;
         }
         s->cr[s->cr_index] = val;
 
@@ -1896,7 +1913,11 @@ static void vga_update_display(void *opaque)
         /* nothing to do */
     } else {
         full_update = 0;
-        if (!(s->ar_index & 0x20)) {
+        if (!(s->ar_index & 0x20) &&
+            /* extra CGA compatibility hacks (not in standard VGA) */
+            (!(vga_cga_hacks & VGA_CGA_HACK_PALETTE_BLANKING) ||
+             s->ar_index != 0 ||
+             !s->ar_flip_flop)) {
             graphic_mode = GMODE_BLANK;
         } else {
             graphic_mode = s->gr[VGA_GFX_MISC] & VGA_GR06_GRAPHICS_MODE;
diff --git a/qemu-options.hx b/qemu-options.hx
index c50f737..3f1e122 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1037,6 +1037,25 @@ Valid optional properties are
 @item retrace=dumb|precise
 Select dumb (default) or precise VGA retrace logic, useful for some
 DOS games/demos.
+@item cga_hacks=@var{hack1}[+@var{hack2}[+...]]
+Enable various extra CGA compatibility hacks for programs that are
+trying to directly set CGA modes without BIOS assistance nor
+real knowledge of EGA/VGA.  These might only work with -vga std.
+Valid hacks are
+@table @option
+@item palette_blanking
+Wait to blank the screen until palette registers seem to actually be
+modified, instead of blanking it as soon as the palette address bit (0x10)
+of the attribute address register (0x3c0) is cleared.
+@item font_height
+Ignore attempts to change the VGA font height (index 9),
+cursor start (index 10), and cursor end (index 11) of the CRTC control
+registers (0x3d5) if trying to set them to the default for CGA fonts
+instead of VGA fonts.
+@item all
+Enable all CGA hacks.  More CGA hacks may be added in future versions
+of qemu.
+@end table
 @end table
 ETEXI
 
diff --git a/vl.c b/vl.c
index 12786f0..15e9e46 100644
--- a/vl.c
+++ b/vl.c
@@ -180,6 +180,7 @@ int main(int argc, char **argv)
 static const char *data_dir;
 const char *bios_name = NULL;
 enum vga_retrace_method vga_retrace_method = VGA_RETRACE_DUMB;
+int vga_cga_hacks = 0;
 DisplayType display_type = DT_DEFAULT;
 static int display_remote;
 const char* keyboard_layout = NULL;
@@ -1881,6 +1882,28 @@ static void select_vgahw (const char *p)
             else if (strstart(opts, "precise", &nextopt))
                 vga_retrace_method = VGA_RETRACE_PRECISE;
             else goto invalid_vga;
+        } else if (strstart(opts, ",cga_hacks=", &nextopt)) {
+            opts = nextopt;
+            while (*opts) {
+                if (strstart(opts, "all", &nextopt)) {
+                    opts = nextopt;
+                    vga_cga_hacks |= ~0;
+                } else if (strstart(opts, "palette_blanking", &nextopt)) {
+                    opts = nextopt;
+                    vga_cga_hacks |= VGA_CGA_HACK_PALETTE_BLANKING;
+                } else if (strstart(opts, "font_height", &nextopt)) {
+                    opts = nextopt;
+                    vga_cga_hacks |= VGA_CGA_HACK_FONT_HEIGHT;
+                } else {
+                    break;
+                }
+
+                if (*opts == '+') {
+                    opts++;
+                } else {
+                    break;
+                }
+            }
         } else goto invalid_vga;
         opts = nextopt;
     }
-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH v8 05/10] fix i8254 output logic to match the spec

[Matthew Ogilvie]

This patch fixes i8254 output line (IRQ0) logic to match the spec.
Basically, IRQ0 line should normally be high, and only occasionally
go low to cause an edge.  This is an important prerequisite to
fixing the i8259 interrupt controller to cancel an unhandled interrupt
when the IRQ line goes low.

More details:
  * Fix high-vs-low counting logic to match the timing diagrams
    and descriptions in Intel's documentation.
  * Improve reading back the count in mode 3.
  * Handle GATE input properly with respect to the OUT line, and add
    a FIXME comment for "GATE-paused" counting and reading
    back the counter.  GATE is hard wired high for channel 0 (IRQ0), but
    it can be software controlled on channel 2 (PC speaker).

See http://bochs.sourceforge.net/techspec/intel-82c54-timer.pdf.gz
or search the net for 23124406.pdf.

Risks:

There is a risk that migrating between versions of qemu
with and without this patch will lose or gain a single timer
interrupt during the migration process.  The only case where
this is likely to be serious is probably losing a single-shot (mode 4)
interrupt, but if my understanding of how things work is good, then
that should only be possible if a whole slew of conditions are
all met:

 1. The guest is configured to run in a "tickless" mode (like
    modern Linux).
 2. The guest is for some reason still using the i8254 rather
    than something more modern like an HPET.  (The combination
    of 1 and 2 should be rare.)
 3. The migration is going from a fixed version back to the
    old version.  (Not sure how common this is, but it should
    be rarer than migrating from old to new.)
 4. There are not going to be any "timely" events/interrupts
    (keyboard, network, process sleeps, etc) that cause the guest
    to reset the PIT mode 4 one-shot counter "soon enough".

This combination should be rare enough that more complicated
solutions are not worth the effort.

Signed-off-by: Matthew Ogilvie <mmogilvi_qemu@miniinfo.net>
---
 hw/i8254.c        | 24 ++++++++++++++++++++++--
 hw/i8254_common.c | 18 ++++++------------
 2 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/hw/i8254.c b/hw/i8254.c
index bea5f92..edb5b7a 100644
--- a/hw/i8254.c
+++ b/hw/i8254.c
@@ -39,6 +39,15 @@ static void pit_irq_timer_update(PITChannelState *s, int64_t current_time);
 
 static int pit_get_count(PITChannelState *s)
 {
+    /* FIXME: Add some way to represent a paused timer and return
+     *   the paused-at counter value, to better model:
+     *     - gate-triggered modes (1 and 5),
+     *     - gate-pausable modes,
+     *     - [maybe] the "wait until next CLK pulse to load counter" logic,
+     *     - [maybe/not clear] half-loaded counter logic for mode 0, and
+     *       the "null count" status bit,
+     *     - etc.
+     */
     uint64_t d;
     int counter;
 
@@ -52,8 +61,7 @@ static int pit_get_count(PITChannelState *s)
         counter = (s->count - d) & 0xffff;
         break;
     case 3:
-        /* XXX: may be incorrect for odd counts */
-        counter = s->count - ((2 * d) % s->count);
+        counter = (s->count - ((2 * d) % s->count)) & 0xfffe;
         break;
     default:
         counter = s->count - (d % s->count);
@@ -98,6 +106,18 @@ static inline void pit_load_count(PITChannelState *s, int val)
     if (val == 0)
         val = 0x10000;
     s->count_load_time = qemu_get_clock_ns(vm_clock);
+
+    /* In gate-triggered one-shot modes, indirectly model some pit_get_out()
+     * cases by setting the load time way back until gate-triggered.
+     * (Generally only affects reading status from channel 2 speaker,
+     * due to hard-wired gates on other channels.)
+     *
+     * FIXME: This might be redesigned if a paused timer state is added
+     * for pic_get_count().
+     */
+    if (s->mode == 1 || s->mode == 5)
+        s->count_load_time -= muldiv64(val+2, get_ticks_per_sec(), PIT_FREQ);
+
     s->count = val;
     pit_irq_timer_update(s, s->count_load_time);
 }
diff --git a/hw/i8254_common.c b/hw/i8254_common.c
index a03d7cd..dc13c99 100644
--- a/hw/i8254_common.c
+++ b/hw/i8254_common.c
@@ -50,24 +50,18 @@ int pit_get_out(PITChannelState *s, int64_t current_time)
     switch (s->mode) {
     default:
     case 0:
-        out = (d >= s->count);
-        break;
     case 1:
-        out = (d < s->count);
+        out = (d >= s->count);
         break;
     case 2:
-        if ((d % s->count) == 0 && d != 0) {
-            out = 1;
-        } else {
-            out = 0;
-        }
+        out = (d % s->count) != (s->count - 1) || s->gate == 0;
         break;
     case 3:
-        out = (d % s->count) < ((s->count + 1) >> 1);
+        out = (d % s->count) < ((s->count + 1) >> 1) || s->gate == 0;
         break;
     case 4:
     case 5:
-        out = (d == s->count);
+        out = (d != s->count);
         break;
     }
     return out;
@@ -93,10 +87,10 @@ int64_t pit_get_next_transition_time(PITChannelState *s, int64_t current_time)
         break;
     case 2:
         base = (d / s->count) * s->count;
-        if ((d - base) == 0 && d != 0) {
+        if ((d - base) == s->count-1) {
             next_time = base + s->count;
         } else {
-            next_time = base + s->count + 1;
+            next_time = base + s->count - 1;
         }
         break;
     case 3:
-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH v8 06/10] i8259: fix so that dropping IRQ level always clears the interrupt request

[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 kernel.

Output from a test program:
-----------
Real hardware [Pentium 4]:
  cmdRead unmask IRR=4005 mask IRR=4001 sti unmask irq14 IRR=0001 DONE
[I also see a final IRR=0000 occasionally.  Probably just happened to
ask it while the timer (IRQ0) line is low.  It masks off most IRQ's, including
timer.]
-----------
Unpatched qemu:
  cmdRead unmask IRR=4015 mask IRR=4015 sti irq15 unmask IRR=4015 DONE
[Presumably IRQ4 (0x10 - qemu's serial device model?) had a transient
edge during initialization, but had been masked off even before I
masked it off?]
-----------
Patched qemu:
  cmdRead unmask IRR=4005 mask IRR=4001 sti unmask irq14 IRR=0001 DONE
-----------

Signed-off-by: Matthew Ogilvie <mmogilvi_qemu@miniinfo.net>
---

There is a risk that some other device models depend on the broken
behavior.  See my question about qemu_irq_pulse() in the cover
letter.

 hw/i8259.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/i8259.c b/hw/i8259.c
index 60c25ba..95587cd 100644
--- a/hw/i8259.c
+++ b/hw/i8259.c
@@ -157,6 +157,7 @@ static void pic_set_irq(void *opaque, int irq, int level)
             }
             s->last_irr |= mask;
         } else {
+            s->irr &= ~mask;
             s->last_irr &= ~mask;
         }
     }
-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH v8 07/10] i8259: refactor pic_set_irq level logic

[Matthew Ogilvie]

No change in functionality.

Clarify that the only difference between level triggered and
edge triggered interrupts is on the leading edge.

Signed-off-by: Matthew Ogilvie <mmogilvi_qemu@miniinfo.net>
---
 hw/i8259.c | 26 +++++++++-----------------
 1 file changed, 9 insertions(+), 17 deletions(-)

diff --git a/hw/i8259.c b/hw/i8259.c
index 95587cd..9b2ec40 100644
--- a/hw/i8259.c
+++ b/hw/i8259.c
@@ -140,26 +140,18 @@ static void pic_set_irq(void *opaque, int irq, int level)
     }
 #endif
 
-    if (s->elcr & mask) {
-        /* level triggered */
-        if (level) {
+    if (level) {
+        if ((s->last_irr & mask) == 0 ||  /* edge for edge triggered */
+            (s->elcr & mask)) {           /* or level triggered */
             s->irr |= mask;
-            s->last_irr |= mask;
-        } else {
-            s->irr &= ~mask;
-            s->last_irr &= ~mask;
         }
+        s->last_irr |= mask;
     } else {
-        /* edge triggered */
-        if (level) {
-            if ((s->last_irr & mask) == 0) {
-                s->irr |= mask;
-            }
-            s->last_irr |= mask;
-        } else {
-            s->irr &= ~mask;
-            s->last_irr &= ~mask;
-        }
+        /* Dropping level clears the interrupt regardless of edge trigger
+         * vs level trigger.
+         */
+        s->irr &= ~mask;
+        s->last_irr &= ~mask;
     }
     pic_update_irq(s);
 }
-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH v8 08/10] qtest: fix qemu_irq_intercept_out()

[Matthew Ogilvie]

For the 8259 (at least), we need to modify the entries in gpio_out
(which is pointing at PICCommonState::int_out) in-place rather than
change it to point to a totally different table.  The 8259 sends its
output to int_out even if gpio_out is a different table.

Signed-off-by: Matthew Ogilvie <mmogilvi_qemu@miniinfo.net>
---
 hw/irq.c | 11 ++++++++---
 hw/irq.h |  2 +-
 qtest.c  |  2 +-
 3 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/hw/irq.c b/hw/irq.c
index f4e2a78..60fa152 100644
--- a/hw/irq.c
+++ b/hw/irq.c
@@ -129,8 +129,13 @@ void qemu_irq_intercept_in(qemu_irq *gpio_in, qemu_irq_handler handler, int n)
     }
 }
 
-void qemu_irq_intercept_out(qemu_irq **gpio_out, qemu_irq_handler handler, int n)
+void qemu_irq_intercept_out(qemu_irq *gpio_out, qemu_irq_handler handler, int n)
 {
-    qemu_irq *old_irqs = *gpio_out;
-    *gpio_out = qemu_allocate_irqs(handler, old_irqs, n);
+    int i;
+    qemu_irq *old_irqs = qemu_allocate_irqs(NULL, NULL, n);
+    for (i = 0; i < n; i++) {
+        *old_irqs[i] = *gpio_out[i];
+        gpio_out[i]->handler = handler;
+        gpio_out[i]->opaque = old_irqs;
+    }
 }
diff --git a/hw/irq.h b/hw/irq.h
index 610e6b7..8dc26cf 100644
--- a/hw/irq.h
+++ b/hw/irq.h
@@ -52,6 +52,6 @@ qemu_irq *qemu_irq_proxy(qemu_irq **target, int n);
 /* For internal use in qtest.  Similar to qemu_irq_split, but operating
    on an existing vector of qemu_irq.  */
 void qemu_irq_intercept_in(qemu_irq *gpio_in, qemu_irq_handler handler, int n);
-void qemu_irq_intercept_out(qemu_irq **gpio_out, qemu_irq_handler handler, int n);
+void qemu_irq_intercept_out(qemu_irq *gpio_out, qemu_irq_handler handler, int n);
 
 #endif
diff --git a/qtest.c b/qtest.c
index fbfab4e..6965910 100644
--- a/qtest.c
+++ b/qtest.c
@@ -232,7 +232,7 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
         }
 
         if (words[0][14] == 'o') {
-            qemu_irq_intercept_out(&dev->gpio_out, qtest_irq_handler, dev->num_gpio_out);
+            qemu_irq_intercept_out(dev->gpio_out, qtest_irq_handler, dev->num_gpio_out);
         } else {
             qemu_irq_intercept_in(dev->gpio_in, qtest_irq_handler, dev->num_gpio_in);
         }
-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH v8 09/10] qtest: add set_irq_{in, out} infrastructure for testing interrupt controllers

[Matthew Ogilvie]

Signed-off-by: Matthew Ogilvie <mmogilvi_qemu@miniinfo.net>
---
 hw/i8259.c       |  6 ++++++
 qtest.c          | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/libqtest.c | 12 ++++++++++++
 tests/libqtest.h | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 117 insertions(+)

diff --git a/hw/i8259.c b/hw/i8259.c
index 9b2ec40..5f09f2f 100644
--- a/hw/i8259.c
+++ b/hw/i8259.c
@@ -453,6 +453,9 @@ qemu_irq *i8259_init(ISABus *bus, qemu_irq parent_irq)
     }
 
     isa_pic = &dev->qdev;
+    object_property_add_link(OBJECT(bus),
+                             "i8259-master", TYPE_DEVICE,
+                             (Object **)&isa_pic, NULL);
 
     dev = i8259_init_chip("isa-i8259", bus, false);
 
@@ -462,6 +465,9 @@ qemu_irq *i8259_init(ISABus *bus, qemu_irq parent_irq)
     }
 
     slave_pic = DO_UPCAST(PICCommonState, dev, dev);
+    object_property_add_link(OBJECT(bus),
+                             "i8259-slave", TYPE_DEVICE,
+                             (Object **)&slave_pic, NULL);
 
     return irq_set;
 }
diff --git a/qtest.c b/qtest.c
index 6965910..d4c2dd7 100644
--- a/qtest.c
+++ b/qtest.c
@@ -117,6 +117,21 @@ static bool qtest_opened;
  * where NUM is an IRQ number.  For the PC, interrupts can be intercepted
  * simply with "irq_intercept_in ioapic" (note that IRQ0 comes out with
  * NUM=0 even though it is remapped to GSI 2).
+ *
+ * Testing interrupt handler chips like the i8259:
+ *
+ *  > set_irq_in QOM-PATH IRQ LEVEL
+ *  < OK
+ *
+ *  > set_irq_out QOM-PATH IRQ LEVEL
+ *  < OK
+ *
+ * Forcibly set the given interrupt pin to the given level (from the
+ * consumer's point of view).
+ *
+ * FUTURE: Some abstracted mechanism to initiate delivery of an
+ *   interrupt to the CPU, returning the interrupt vector number
+ *   that would be delivered to that CPU.
  */
 
 static int hex2nib(char ch)
@@ -239,7 +254,43 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
         irq_intercept_dev = dev;
         qtest_send_prefix(chr);
         qtest_send(chr, "OK\n");
+    } else if (strcmp(words[0], "set_irq_in") == 0 ||
+               strcmp(words[0], "set_irq_out") == 0) {
+        DeviceState *dev;
+        qemu_irq *irqs;
+        int num_irqs;
+        int num;
+        int level;
+
+        g_assert(words[1] && words[2] && words[3]);
+
+        dev = DEVICE(object_resolve_path(words[1], NULL));
+        if (!dev) {
+            qtest_send_prefix(chr);
+            qtest_send(chr, "FAIL Unknown device\n");
+            return;
+        }
 
+        if (words[0][8] == 'o') {
+            irqs = dev->gpio_out;
+            num_irqs = dev->num_gpio_out;
+        } else {
+            irqs = dev->gpio_in;
+            num_irqs = dev->num_gpio_in;
+        }
+
+        num = strtol(words[2], NULL, 0);
+        if (num < 0 || num >= num_irqs) {
+            qtest_send_prefix(chr);
+            qtest_send(chr, "FAIL Bad IRQ number\n");
+            return;
+        }
+
+        level = strtol(words[3], NULL, 0);
+
+        qemu_set_irq(irqs[num], level != 0);
+        qtest_send_prefix(chr);
+        qtest_send(chr, "OK\n");
     } else if (strcmp(words[0], "outb") == 0 ||
                strcmp(words[0], "outw") == 0 ||
                strcmp(words[0], "outl") == 0) {
diff --git a/tests/libqtest.c b/tests/libqtest.c
index 71b84c1..f6160ad 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -379,6 +379,18 @@ void qtest_irq_intercept_in(QTestState *s, const char *qom_path)
     qtest_rsp(s, 0);
 }
 
+void qtest_set_irq_in(QTestState *s, const char *qom_path, int num, int level)
+{
+    qtest_sendf(s, "set_irq_in %s %d %d\n", qom_path, num, level);
+    qtest_rsp(s, 0);
+}
+
+void qtest_set_irq_out(QTestState *s, const char *qom_path, int num, int level)
+{
+    qtest_sendf(s, "set_irq_out %s %d %d\n", qom_path, num, level);
+    qtest_rsp(s, 0);
+}
+
 static void qtest_out(QTestState *s, const char *cmd, uint16_t addr, uint32_t value)
 {
     qtest_sendf(s, "%s 0x%x 0x%x\n", cmd, addr, value);
diff --git a/tests/libqtest.h b/tests/libqtest.h
index c8ade85..a045fc7 100644
--- a/tests/libqtest.h
+++ b/tests/libqtest.h
@@ -76,6 +76,30 @@ void qtest_irq_intercept_in(QTestState *s, const char *string);
 void qtest_irq_intercept_out(QTestState *s, const char *string);
 
 /**
+ * qtest_set_irq_in:
+ * @s: QTestState instance to operate on.
+ * @string: QOM path of a device
+ * @irq: IRQ number
+ * @level: level to set it to (0 or 1)
+ *
+ * Force given device/irq GPIO-in pin to the given level.
+ * Mostly useful for testing interrupt controllers, rather than other devices.
+ */
+void qtest_set_irq_in(QTestState *s, const char *string, int irq, int level);
+
+/**
+ * qtest_set_irq_out:
+ * @s: QTestState instance to operate on.
+ * @string: QOM path of a device
+ * @irq: IRQ number
+ * @level: level to set it to (0 or 1)
+ *
+ * Force given device/irq GPIO-out pin to the given level.
+ * Mostly useful for testing interrupt controllers, rather than other devices.
+ */
+void qtest_set_irq_out(QTestState *s, const char *string, int irq, int level);
+
+/**
  * qtest_outb:
  * @s: QTestState instance to operate on.
  * @addr: I/O port to write to.
@@ -250,6 +274,30 @@ void qtest_add_func(const char *str, void (*fn));
 #define irq_intercept_out(string) qtest_irq_intercept_out(global_qtest, string)
 
 /**
+ * qtest_set_irq_in:
+ * @string: QOM path of a device
+ * @irq: IRQ number
+ * @level: level to set it to (0 or 1)
+ *
+ * Force given device/irq GPIO-in pin to the given level.
+ * Mostly useful for testing interrupt controllers, rather than other devices.
+ */
+#define set_irq_in(string, irq, level) \
+        qtest_set_irq_in(global_qtest, string, irq, level)
+
+/**
+ * qtest_set_irq_in:
+ * @string: QOM path of a device
+ * @irq: IRQ number
+ * @level: level to set it to (0 or 1)
+ *
+ * Force given device/irq GPIO-in pin to the given level.
+ * Mostly useful for testing interrupt controllers, rather than other devices.
+ */
+#define set_irq_out(string, irq, level) \
+        qtest_set_irq_out(global_qtest, string, irq, level)
+
+/**
  * outb:
  * @addr: I/O port to write to.
  * @value: Value being written.
-- 
1.7.10.2.484.gcd07cc5

[Qemu-devel] [PATCH v8 10/10] add test/pic-test.c

[Matthew Ogilvie]

Microport UNIX System V/386 v2.1 (ca. 1987) does something similar
to the slave_imr() test, and doesn't run if the "canceling" behavior
of IRQ2 doesn't work.

I don't know anything that depends on the canceling behavior of
other interrupts (master_basic() or slave_basic() tests), but
it seems best to fix the issue generically if possible.

Signed-off-by: Matthew Ogilvie <mmogilvi_qemu@miniinfo.net>
---
 tests/Makefile   |   2 +
 tests/pic-test.c | 127 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 129 insertions(+)
 create mode 100644 tests/pic-test.c

diff --git a/tests/Makefile b/tests/Makefile
index b60f0fb..d21b0d5 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -25,6 +25,7 @@ check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh
 check-qtest-i386-y = tests/fdc-test$(EXESUF)
 check-qtest-i386-y += tests/hd-geo-test$(EXESUF)
 check-qtest-i386-y += tests/rtc-test$(EXESUF)
+check-qtest-i386-y += tests/pic-test$(EXESUF)
 check-qtest-x86_64-y = $(check-qtest-i386-y)
 check-qtest-sparc-y = tests/m48t59-test$(EXESUF)
 check-qtest-sparc64-y = tests/m48t59-test$(EXESUF)
@@ -75,6 +76,7 @@ tests/test-qmp-commands$(EXESUF): tests/test-qmp-commands.o tests/test-qmp-marsh
 tests/test-visitor-serialization$(EXESUF): tests/test-visitor-serialization.o $(test-qapi-obj-y)
 
 tests/rtc-test$(EXESUF): tests/rtc-test.o $(trace-obj-y)
+tests/pic-test$(EXESUF): tests/pic-test.o $(trace-obj-y)
 tests/m48t59-test$(EXESUF): tests/m48t59-test.o $(trace-obj-y)
 tests/fdc-test$(EXESUF): tests/fdc-test.o tests/libqtest.o $(trace-obj-y)
 tests/hd-geo-test$(EXESUF): tests/hd-geo-test.o tests/libqtest.o $(trace-obj-y)
diff --git a/tests/pic-test.c b/tests/pic-test.c
new file mode 100644
index 0000000..57b8bee
--- /dev/null
+++ b/tests/pic-test.c
@@ -0,0 +1,127 @@
+/*
+ * QTest testcase for the 8259 interrupt controller
+ *
+ * Copyright (c) 2012 Matthew Ogilvie
+ *
+ * Authors:
+ *  Matthew Ogilvie <mmogilvi_qemu@miniinfo.net>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+#include "libqtest.h"
+
+#include <glib.h>
+
+static void init_i8259(void)
+{
+    /* master: */
+    outb(0x20,0x13); /* ICW1: edge-triggered, multiple 8259s, ICW4 needed */
+    outb(0x21,0x08); /* ICW2: map to int08-int0f */
+    outb(0x21,0x04); /* ICW3: IR2 connected to slave */
+    outb(0x21,0x01); /* ICW4: not-SFNM, not-buffered, manual EOI, 8086 mode */
+
+    outb(0x21,0xfa); /* OCW1/IMR: mask off all except IRQ2 and IRQ0 */
+
+    /* slave: */
+    outb(0xa0,0x13); /* ICW1: edge-triggered, multiple 8259s, ICW4 needed */
+    outb(0xa1,0x70); /* ICW2: map to int70-int77 */
+    outb(0xa1,0x02); /* ICW3: connected to master IR2 */
+    outb(0xa1,0x01); /* ICW4: not-SFNM, not-buffered, manual EOI, 8086 mode */
+
+    outb(0xa1,0x3f); /* OCW1/IMR: mask off all except IRQ14 and IRQ15 */
+}
+
+static void set_irq(int num, int level)
+{
+    if(num > 8) {
+        set_irq_in("i8259-slave", num - 8, level);
+    } else {
+        set_irq_in("i8259-master", num, level);
+    }
+}
+
+static void slave_imr(void)
+{
+    init_i8259();
+
+    g_assert_cmpint(get_irq(0), ==, 0);
+    set_irq(14, 1);
+    g_assert_cmpint(get_irq(0), ==, 1);
+
+    outb(0xa1,0xff); /* OCW1/IMR: mask off all slave IRQs */
+    g_assert_cmpint(get_irq(0), ==, 0);
+
+    outb(0xa0,0x0a); /* slave OCW3: read IRR */
+    g_assert_cmpint(inb(0xa0)&0x40, ==, 0x40); /* IRR still has IRQ14 */
+    outb(0x20,0x0a); /* master OCW3: read IRR */
+    g_assert_cmpint(inb(0x20)&0x04, ==, 0x00); /* IRR does not have IRQ2 */
+
+    outb(0xa1,0x3f); /* OCW1/IMR: unmask IRQ14 and IRQ15 */
+    g_assert_cmpint(get_irq(0), ==, 1);
+
+    outb(0xa0,0x0a); /* slave OCW3: read IRR */
+    g_assert_cmpint(inb(0xa0)&0x40, ==, 0x40); /* IRR has IRQ14 */
+    outb(0x20,0x0a); /* master OCW3: read IRR */
+    g_assert_cmpint(inb(0x20)&0x04, ==, 0x04); /* IRR has IRQ2 */
+}
+
+static void master_cancel(void)
+{
+    init_i8259();
+
+    g_assert_cmpint(get_irq(0), ==, 0);
+    set_irq(0, 1);
+    g_assert_cmpint(get_irq(0), ==, 1);
+    set_irq(0, 0);
+    g_assert_cmpint(get_irq(0), ==, 0);
+}
+
+static void slave_cancel(void)
+{
+    init_i8259();
+
+    g_assert_cmpint(get_irq(0), ==, 0);
+    set_irq(14, 1);
+    g_assert_cmpint(get_irq(0), ==, 1);
+    set_irq(14, 0);
+    g_assert_cmpint(get_irq(0), ==, 0);
+}
+
+int main(int argc, char **argv)
+{
+    QTestState *s = NULL;
+    int ret;
+
+    g_test_init(&argc, &argv, NULL);
+
+    s = qtest_start(/*"-qtest-log /dev/tty "*/ "-display none");
+    qtest_irq_intercept_out(s, "i8259-master");
+
+    /* FUTURE: Before testing obscure cases, first test basic
+     *  interrupt delivery to the CPU and EOI functionality.
+     *  But that requires some kind of abstracted hook into
+     *  cpu_get_pic_interrupt() vs other architecture equivalents,
+     *  and the abstraction doesn't currently exist.
+     */
+
+    /* I know of at least one (admittedly obscure) guest that depends
+     * slave_imr() working.  (Microport UNIX System V/386 v2.1 (ca. 1987))
+     */
+    qtest_add_func("/pic/slave-imr", slave_imr);
+
+    /* I don't know any guest that depends on these, but a generic fix
+     * for slave_imr() issue also fixes these:
+     */
+    qtest_add_func("/pic/master-cancel", master_cancel);
+    qtest_add_func("/pic/slave-cancel", slave_cancel);
+
+    ret = g_test_run();
+
+    if (s) {
+        qtest_quit(s);
+    }
+
+    return ret;
+}
-- 
1.7.10.2.484.gcd07cc5