From patchwork Sun Nov 25 21:51:44 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Matthew Ogilvie X-Patchwork-Id: 201574 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 934E62C0087 for ; Mon, 26 Nov 2012 08:53:30 +1100 (EST) Received: from localhost ([::1]:36723 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Tck8m-0006A2-On for incoming@patchwork.ozlabs.org; Sun, 25 Nov 2012 16:53:28 -0500 Received: from eggs.gnu.org ([208.118.235.92]:41118) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Tck8N-0005sh-Gb for qemu-devel@nongnu.org; Sun, 25 Nov 2012 16:53:05 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Tck8M-00012E-4t for qemu-devel@nongnu.org; Sun, 25 Nov 2012 16:53:03 -0500 Received: from qmta07.emeryville.ca.mail.comcast.net ([76.96.30.64]:33325) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Tck8L-00010f-SJ for qemu-devel@nongnu.org; Sun, 25 Nov 2012 16:53:02 -0500 Received: from omta01.emeryville.ca.mail.comcast.net ([76.96.30.11]) by qmta07.emeryville.ca.mail.comcast.net with comcast id Tx3j1k0020EPchoA7xt1Mv; Sun, 25 Nov 2012 21:53:01 +0000 Received: from mmogilvi.homeip.net ([75.70.117.91]) by omta01.emeryville.ca.mail.comcast.net with comcast id Txsx1k00E1yPlfP8Mxt0UX; Sun, 25 Nov 2012 21:53:00 +0000 Received: by mmogilvi.homeip.net (Postfix, from userid 501) id 2B2421E9601C; Sun, 25 Nov 2012 14:53:00 -0700 (MST) From: Matthew Ogilvie To: qemu-devel@nongnu.org Date: Sun, 25 Nov 2012 14:51:44 -0700 Message-Id: <1353880306-8004-9-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> X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 76.96.30.64 Cc: Jan Kiszka , Matthew Ogilvie , "Maciej W. Rozycki" , Avi Kivity Subject: [Qemu-devel] [PATCH v7 08/10] i8254: add comments about fixing timings 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 There may be risk of problems with migration if these are just fixed blindly, but at least comment what it ought to be changed to. Signed-off-by: Matthew Ogilvie --- hw/i8254.c | 31 ++++++++++++++++++++++++++++++- hw/i8254_common.c | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 1 deletion(-) diff --git a/hw/i8254.c b/hw/i8254.c index bea5f92..982e8e7 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,7 +61,11 @@ static int pit_get_count(PITChannelState *s) counter = (s->count - d) & 0xffff; break; case 3: - /* XXX: may be incorrect for odd counts */ + /* 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); break; default: @@ -98,6 +111,22 @@ 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); + + /* 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); + * + * (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 + * for pic_get_count(). + */ + 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..33f352f 100644 --- a/hw/i8254_common.c +++ b/hw/i8254_common.c @@ -53,9 +53,30 @@ int pit_get_out(PITChannelState *s, int64_t current_time) 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); 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 { @@ -63,10 +84,21 @@ int pit_get_out(PITChannelState *s, int64_t current_time) } 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); 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); break; } @@ -93,6 +125,14 @@ 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) { next_time = base + s->count; } else {