[Qemu-devel] [PATCH v7 09/10] i8254: prepare for migration compatibility with future fixes

[Matthew Ogilvie <mmogilvi_qemu@miniinfo.net>]

Under normal operation this patch should have no effect.  But
it adds some redundant (no-change) calls to qemu_set_irq(),
so that if a running host image is migrated from a future version
of qemu that has various fixes to the i8254 output line logic,
this version can still deliver exactly the correct number of
IRQ0 leading edges to the i8259 at as close as possible to the
correct time.

The plan is to incorporate this now, and then much later (years?)
we can implement the actual fixes to the i8254, after migration to/from
qemu versions without this transitional fix is no longer a concern.

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

Alternatives:

An alternative for mode 2 might involve tweaking
pit_get_next_transition_time() to create an extra pseudo-transition
at the same clock tick that will eventually have the "real" transition
(so it has 3 "transitions" per period).  But it may be best to keep
the migration hacks together in one place.

Or support both the old model and a new/fixed model, selectable at
runtime by command line switch.

 hw/i8254.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 78 insertions(+), 7 deletions(-)

diff --git a/hw/i8254.c b/hw/i8254.c
index 982e8e7..ffe6e27 100644
--- a/hw/i8254.c
+++ b/hw/i8254.c
@@ -35,7 +35,8 @@
 #define RW_STATE_WORD0 3
 #define RW_STATE_WORD1 4
 
-static void pit_irq_timer_update(PITChannelState *s, int64_t current_time);
+static void pit_irq_timer_update(PITChannelState *s, int64_t current_time,
+                                 bool edge);
 
 static int pit_get_count(PITChannelState *s)
 {
@@ -90,7 +91,7 @@ static void pit_set_channel_gate(PITCommonState *s, PITChannelState *sc,
         if (sc->gate < val) {
             /* restart counting on rising edge */
             sc->count_load_time = qemu_get_clock_ns(vm_clock);
-            pit_irq_timer_update(sc, sc->count_load_time);
+            pit_irq_timer_update(sc, sc->count_load_time, false);
         }
         break;
     case 2:
@@ -98,7 +99,7 @@ static void pit_set_channel_gate(PITCommonState *s, PITChannelState *sc,
         if (sc->gate < val) {
             /* restart counting on rising edge */
             sc->count_load_time = qemu_get_clock_ns(vm_clock);
-            pit_irq_timer_update(sc, sc->count_load_time);
+            pit_irq_timer_update(sc, sc->count_load_time, false);
         }
         /* XXX: disable/enable counting */
         break;
@@ -128,7 +129,7 @@ static inline void pit_load_count(PITChannelState *s, int val)
      */
 
     s->count = val;
-    pit_irq_timer_update(s, s->count_load_time);
+    pit_irq_timer_update(s, s->count_load_time, false);
 }
 
 /* if already latched, do not latch again */
@@ -262,7 +263,8 @@ static uint64_t pit_ioport_read(void *opaque, hwaddr addr,
     return ret;
 }
 
-static void pit_irq_timer_update(PITChannelState *s, int64_t current_time)
+static void pit_irq_timer_update(PITChannelState *s, int64_t current_time,
+                                 bool edge)
 {
     int64_t expire_time;
     int irq_level;
@@ -272,6 +274,75 @@ static void pit_irq_timer_update(PITChannelState *s, int64_t current_time)
     }
     expire_time = pit_get_next_transition_time(s, current_time);
     irq_level = pit_get_out(s, current_time);
+
+    /* FIXME i8254_timing_fixes: Fixing pic_get_out() timings needs
+     *  to happen in stages.  For now, ensure that exactly one leading
+     *  edge is detected in the 8259 somewhere near counter expiration,
+     *  even if migrating to/from a future version that has corrected
+     *  (different) IRQ0 transitions from pic_get_out().
+     *
+     *  Quick description of how this works for various modes when
+     *  migrating between old and new versions.  To understand this,
+     *  it can help to draw out a timing diagram showing both this/old and
+     *  future version IRQ0 levels over time.
+     *     mode 0: no change.
+     *     mode 2: The leading edge stays at the same counter value,
+     *             but the timing of the trailing edge moves
+     *             from one CLK tick after the leading edge (this/old
+     *             version) to one CLK tick before the next trailing
+     *             edge (future version).  Migration cases:
+     *       - This high IRQ0 migrated to future version: Nothing special;
+     *         always correct.
+     *       - This low IRQ0 migrated to future version: May be set low
+     *         again a second time (noop) when counter gets to 1, but
+     *         otherwise nothing special.
+     *       - Future low IRQ0 migrated to this/old version: Nothing special;
+     *         always correct.
+     *       - Future high migrated to this/old version: Do a
+     *         high-low-high sequence when the next leading edge is needed.
+     *         The sequence may simplify to noop-low-high in some cases
+     *         depending on if migrates less than a tick since it went
+     *         high (so that this/old trailing edge logic applies), but it
+     *         should work correctly in any case.
+     *     mode 3: Effectively no change: The only change involves
+     *             gate, which is hard-coded for channel 0 (IRQ0).
+     *     mode 4: IRQ0 is inverted, so the leading edge is off by 1.
+     *             Migration cases:
+     *       - This high IRQ0 migrated to future version: At end of current
+     *         CLK tick, future code will set IRQ0 high again (noop).  Only
+     *         previous tick will have been delivered.
+     *       - This low IRQ0 migrated to future version: Future code will
+     *         set it low again when counter gets to 0 (noop), then
+     *         deliver the leading edge one CLK tick after that.
+     *       - Future low IRQ0 migrated to this/old version: This code
+     *         will do a low-high-low sequence at the next CLK tick.
+     *         (Normally it is already high, and so it simplifies
+     *         to noop-high-low.)
+     *       - Future high migrated to this/old version: The force-edge
+     *         code below will cause an immediate high-low-high
+     *         sequence as soon as counter reaches 0, delivering
+     *         an edge immediately.  (Normally it is already low, and
+     *         so it simplifies to noop-low-high.)
+     *     mode 1 or 5: Gate-triggered modes are never used for
+     *                  IRQ0 because gate0 is hard-wired.  But
+     *                  both of them have inverted logic similar to
+     *                  mode 4.
+     *
+     *  Also, under normal operation (this/old version, no-migration),
+     *  the if case is a noop, because it should already have the
+     *  indicated IRQ0 level.
+     *
+     *  When pic_get_out() logic is fixed, this migration compatibility
+     *  logic will need to be changed (probably just removed).
+     */
+    if (edge) {
+        if (s->mode == 2 && irq_level) {
+            qemu_set_irq(s->irq, 0);
+        } else if (s->mode == 4 || s->mode == 5 || s->mode == 1) {
+            qemu_set_irq(s->irq, !irq_level);
+        }
+    }
+
     qemu_set_irq(s->irq, irq_level);
 #ifdef DEBUG_PIT
     printf("irq_level=%d next_delay=%f\n",
@@ -289,7 +360,7 @@ static void pit_irq_timer(void *opaque)
 {
     PITChannelState *s = opaque;
 
-    pit_irq_timer_update(s, s->next_transition_time);
+    pit_irq_timer_update(s, s->next_transition_time, true);
 }
 
 static void pit_reset(DeviceState *dev)
@@ -314,7 +385,7 @@ static void pit_irq_control(void *opaque, int n, int enable)
 
     if (enable) {
         s->irq_disabled = 0;
-        pit_irq_timer_update(s, qemu_get_clock_ns(vm_clock));
+        pit_irq_timer_update(s, qemu_get_clock_ns(vm_clock), false);
     } else {
         s->irq_disabled = 1;
         qemu_del_timer(s->irq_timer);
-- 
1.7.10.2.484.gcd07cc5