[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 <address@hidden>
---
 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