diff --git a/hw/i8254.c b/hw/i8254.c index ffe6e27..edb5b7a 100644 --- a/hw/i8254.c +++ b/hw/i8254.c @@ -35,8 +35,7 @@ #define RW_STATE_WORD0 3 #define RW_STATE_WORD1 4 -static void pit_irq_timer_update(PITChannelState *s, int64_t current_time, - bool edge); +static void pit_irq_timer_update(PITChannelState *s, int64_t current_time); static int pit_get_count(PITChannelState *s) { @@ -62,12 +61,7 @@ static int pit_get_count(PITChannelState *s) counter = (s->count - d) & 0xffff; break; case 3: - /* XXX: may be incorrect for odd counts - * FIXME i8254_timing_fixes: fix this by changing it to something - * like: - * counter = (s->count - ((2 * d) % s->count)) & 0xfffe; - */ - counter = s->count - ((2 * d) % s->count); + counter = (s->count - ((2 * d) % s->count)) & 0xfffe; break; default: counter = s->count - (d % s->count); @@ -91,7 +85,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, false); + pit_irq_timer_update(sc, sc->count_load_time); } break; case 2: @@ -99,7 +93,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, false); + pit_irq_timer_update(sc, sc->count_load_time); } /* XXX: disable/enable counting */ break; @@ -113,23 +107,19 @@ static inline void pit_load_count(PITChannelState *s, int val) val = 0x10000; s->count_load_time = qemu_get_clock_ns(vm_clock); - /* FIXME i8254_timing_fixes: - * In gate-triggered one-shot modes, indirectly model some pit_get_out() - * cases by setting the load time way back until gate-triggered, - * using code such as: - * - * if (s->mode == 1 || s->mode == 5) - * s->count_load_time -= muldiv64(val+2, get_ticks_per_sec(), PIT_FREQ); - * + /* 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 Or this might be redesigned if a paused timer state is added + * 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, false); + pit_irq_timer_update(s, s->count_load_time); } /* if already latched, do not latch again */ @@ -263,8 +253,7 @@ static uint64_t pit_ioport_read(void *opaque, hwaddr addr, return ret; } -static void pit_irq_timer_update(PITChannelState *s, int64_t current_time, - bool edge) +static void pit_irq_timer_update(PITChannelState *s, int64_t current_time) { int64_t expire_time; int irq_level; @@ -274,75 +263,6 @@ 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", @@ -360,7 +280,7 @@ static void pit_irq_timer(void *opaque) { PITChannelState *s = opaque; - pit_irq_timer_update(s, s->next_transition_time, true); + pit_irq_timer_update(s, s->next_transition_time); } static void pit_reset(DeviceState *dev) @@ -385,7 +305,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), false); + pit_irq_timer_update(s, qemu_get_clock_ns(vm_clock)); } else { s->irq_disabled = 1; qemu_del_timer(s->irq_timer); diff --git a/hw/i8254_common.c b/hw/i8254_common.c index 33f352f..dc13c99 100644 --- a/hw/i8254_common.c +++ b/hw/i8254_common.c @@ -50,56 +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: - /* FIXME i8254_timing_fixes: There are various problems - * with qemu's 8254 model (especially when IRQ0's trailing - * edge occurs), but fixing them directly might cause - * problems with migration. So just comment the fixes for - * now. - * (Based on reading timing diagrams in Intel's 8254 spec - * sheet, downloadable from - * http://bochs.sourceforge.net/techspec/intel-82c54-timer.pdf.gz - * or other places.) - * FIXME i8254_timing_fixes: Both mode 0 and mode 1 should use - * out = (d >= s->count); - * So mode 0 can fall-through into a fixed mode 1. - * The difference between modes 0 and 1 is in the gate triggering. - * See also an adjustment to make to count_load_time - * when count is loaded for mode 1. - */ - out = (d < s->count); + out = (d >= s->count); break; case 2: - /* FIXME i8254_timing_fixes: This should simply be: - * out = (d % s->count) != (s->count - 1) || s->gate == 0; - * Gate is hard-wired 1 for channel 0 (IRQ0), but it can be - * adjusted in software for channel 2 (PC speaker). - */ - 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: - /* FIXME i8254_timing_fixes: This should be: - * out = (d % s->count) < ((s->count + 1) >> 1) || s->gate == 0; - * Gate is hard-wired 1 for channel 0 (IRQ0), but it can be - * adjusted in software for channel 2 (PC speaker). - */ - out = (d % s->count) < ((s->count + 1) >> 1); + out = (d % s->count) < ((s->count + 1) >> 1) || s->gate == 0; break; case 4: case 5: - /* FIXME i8254_timing_fixes: The logic is backwards, and should be: - * out = (d != s->count); - * The difference between modes 4 and 5 is in the gate triggering. - * See also an adjustment to make to count_load_time - * when count is loaded for mode 5. - */ - out = (d == s->count); + out = (d != s->count); break; } return out; @@ -125,18 +87,10 @@ int64_t pit_get_next_transition_time(PITChannelState *s, int64_t current_time) break; case 2: base = (d / s->count) * s->count; - /* FIXME i8254_timing_fixes: The if condition and else case should - * be changed to: - * if ((d - base) == (s->count - 1)) { - * next_time = base + s->count; - * } else { - * next_time = base + s->count - 1; - * } - */ - 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: diff --git a/hw/i8259.c b/hw/i8259.c index 71cc09a..9b2ec40 100644 --- a/hw/i8259.c +++ b/hw/i8259.c @@ -150,25 +150,8 @@ static void pic_set_irq(void *opaque, int irq, int level) /* Dropping level clears the interrupt regardless of edge trigger * vs level trigger. */ - s->last_irr &= ~mask; - - /* Migration compatibility hack: - * - * The i8254 timer model is wrong in a number of ways, - * including lowering IRQ0 much earlier than it should. - * - * FIXME i8254_timing_fixes: Eventually the i8254 - * should be fixed, but it isn't - * trivial to do so in a way that avoids possible problems with - * migration (lost or gained timer ticks). So for now, make the - * i8254 work the same way that it worked in qemu 1.2, and - * leave irr for IRQ0 alone in the i8259 here: - */ - if (irq == 0 && s->master) { - mask = 0; - } - s->irr &= ~mask; + s->last_irr &= ~mask; } pic_update_irq(s); }