From patchwork Sun Nov 25 21:51:45 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Matthew Ogilvie X-Patchwork-Id: 201585 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 2368E2C0087 for ; Mon, 26 Nov 2012 09:53:15 +1100 (EST) Received: from localhost ([::1]:39161 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Tck95-0007SF-Sz for incoming@patchwork.ozlabs.org; Sun, 25 Nov 2012 16:53:47 -0500 Received: from eggs.gnu.org ([208.118.235.92]:41153) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Tck8P-0005yC-6P for qemu-devel@nongnu.org; Sun, 25 Nov 2012 16:53:06 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Tck8N-00012T-46 for qemu-devel@nongnu.org; Sun, 25 Nov 2012 16:53:05 -0500 Received: from qmta01.emeryville.ca.mail.comcast.net ([76.96.30.16]:55591) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Tck8M-0000wx-SI for qemu-devel@nongnu.org; Sun, 25 Nov 2012 16:53:03 -0500 Received: from omta09.emeryville.ca.mail.comcast.net ([76.96.30.20]) by qmta01.emeryville.ca.mail.comcast.net with comcast id TtHp1k00C0S2fkCA1xt2UH; Sun, 25 Nov 2012 21:53:02 +0000 Received: from mmogilvi.homeip.net ([75.70.117.91]) by omta09.emeryville.ca.mail.comcast.net with comcast id Txsw1k00g1yPlfP8Vxt0EE; Sun, 25 Nov 2012 21:53:01 +0000 Received: by mmogilvi.homeip.net (Postfix, from userid 501) id D62FA1E9601B; Sun, 25 Nov 2012 14:53:00 -0700 (MST) From: Matthew Ogilvie To: qemu-devel@nongnu.org Date: Sun, 25 Nov 2012 14:51:45 -0700 Message-Id: <1353880306-8004-10-git-send-email-mmogilvi_qemu@miniinfo.net> X-Mailer: git-send-email 1.7.10.2.484.gcd07cc5 In-Reply-To: <1353880306-8004-1-git-send-email-mmogilvi_qemu@miniinfo.net> References: <1353880306-8004-1-git-send-email-mmogilvi_qemu@miniinfo.net> DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=comcast.net; s=q20121106; t=1353880382; bh=6775/iJEtwF/RIGKoRYc92e1R4adP0gVDTOvnrNgKCU=; h=Received:Received:Received:From:To:Subject:Date:Message-Id; b=LpZvAFkwVfH/ulIZxgNImRjNJIDrCDfZN6yTHmjUkc6WJMqTShdHMxB0V+85yrMld GTSZP/SQZ0Bu+/kDvjXNwS0LPZDbDIaXNMLenQDYmVUYHT04wzOkDPerIo2etwMrYr 6jdYEKjI3HjExLWGaCpJSA3VYd0f/uXOprJcJhUCMhQYw6H9qNVz85Tgk0t46k7f1p x1PUfelt/TAp5eoGY/VyVvBYR7LT0WvgNUHfYAnUz7vowVs1AmAW+OX9vtEP8zcYC6 m2c9c1LCW7iN+Le0rduCpMggCeHG3F4kC66ezuaRkjevEh6jKLVukBuHn4sYp5HDg/ gvm//V0og+Ydw== X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 76.96.30.16 Cc: Jan Kiszka , Matthew Ogilvie , "Maciej W. Rozycki" , Avi Kivity Subject: [Qemu-devel] [PATCH v7 09/10] i8254: prepare for migration compatibility with future fixes X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org 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 --- 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);