From patchwork Fri Dec 14 13:35:36 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans de Goede X-Patchwork-Id: 206474 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 B64062C008C for ; Sat, 15 Dec 2012 00:44:24 +1100 (EST) Received: from localhost ([::1]:50118 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TjVQe-0006zW-D9 for incoming@patchwork.ozlabs.org; Fri, 14 Dec 2012 08:35:52 -0500 Received: from eggs.gnu.org ([208.118.235.92]:45253) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TjVOl-0004A1-JT for qemu-devel@nongnu.org; Fri, 14 Dec 2012 08:33:59 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TjVOg-0005JX-NJ for qemu-devel@nongnu.org; Fri, 14 Dec 2012 08:33:55 -0500 Received: from mx1.redhat.com ([209.132.183.28]:10081) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TjVOg-0005JL-F9 for qemu-devel@nongnu.org; Fri, 14 Dec 2012 08:33:50 -0500 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id qBEDXnIo009586 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Fri, 14 Dec 2012 08:33:49 -0500 Received: from shalem.localdomain.com (vpn1-6-137.ams2.redhat.com [10.36.6.137]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id qBEDXSSt028567; Fri, 14 Dec 2012 08:33:48 -0500 From: Hans de Goede To: Gerd Hoffmann Date: Fri, 14 Dec 2012 14:35:36 +0100 Message-Id: <1355492147-5023-16-git-send-email-hdegoede@redhat.com> In-Reply-To: <1355492147-5023-1-git-send-email-hdegoede@redhat.com> References: <1355492147-5023-1-git-send-email-hdegoede@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.23 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 209.132.183.28 Cc: Hans de Goede , qemu-devel@nongnu.org Subject: [Qemu-devel] [PATCH 15/26] uhci: Limit amount of frames processed in one go 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 Before this patch uhci would process an unlimited amount of frames when behind on schedule, by setting the timer to a time already past, causing the timer subsys to immediately recall the frame_timer function gain. This would cause invalid cancellations of bulk queues when the catching up processed more then 32 frames at a moment when the bulk qh was temporarily unlinked (which the Linux uhci driver does). This patch fixes this by processing maximum 16 frames in one go, and always setting the timer one ms later, making the code behave more like the ehci code. Signed-off-by: Hans de Goede --- hw/usb/hcd-uhci.c | 43 +++++++++++++++++++++++++++---------------- 1 file changed, 27 insertions(+), 16 deletions(-) diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c index 11ccdc8..2a5d4cc 100644 --- a/hw/usb/hcd-uhci.c +++ b/hw/usb/hcd-uhci.c @@ -78,6 +78,8 @@ /* Must be large enough to handle 10 frame delay for initial isoc requests */ #define QH_VALID 32 +#define MAX_FRAMES_PER_TICK (QH_VALID / 2) + #define NB_PORTS 2 enum { @@ -500,7 +502,7 @@ static void uhci_ioport_writew(void *opaque, uint32_t addr, uint32_t val) trace_usb_uhci_schedule_start(); s->expire_time = qemu_get_clock_ns(vm_clock) + (get_ticks_per_sec() / FRAME_TIMER_FREQ); - qemu_mod_timer(s->frame_timer, qemu_get_clock_ns(vm_clock)); + qemu_mod_timer(s->frame_timer, s->expire_time); s->status &= ~UHCI_STS_HCHALTED; } else if (!(val & UHCI_CMD_RS)) { s->status |= UHCI_STS_HCHALTED; @@ -1176,10 +1178,10 @@ static void uhci_bh(void *opaque) static void uhci_frame_timer(void *opaque) { UHCIState *s = opaque; + uint64_t t_now, t_last_run; + int i, frames; + const uint64_t frame_t = get_ticks_per_sec() / FRAME_TIMER_FREQ; - /* prepare the timer for the next frame */ - s->expire_time += (get_ticks_per_sec() / FRAME_TIMER_FREQ); - s->frame_bytes = 0; s->completions_only = false; qemu_bh_cancel(s->bh); @@ -1193,20 +1195,29 @@ static void uhci_frame_timer(void *opaque) return; } - /* Process the current frame */ - trace_usb_uhci_frame_start(s->frnum); - - uhci_async_validate_begin(s); - - uhci_process_frame(s); + /* We still store expire_time in our state, for migration */ + t_last_run = s->expire_time - frame_t; + t_now = qemu_get_clock_ns(vm_clock); - uhci_async_validate_end(s); + /* Process up to MAX_FRAMES_PER_TICK frames */ + frames = (t_now - t_last_run) / frame_t; + if (frames > MAX_FRAMES_PER_TICK) { + frames = MAX_FRAMES_PER_TICK; + } - /* The uhci spec says frnum reflects the frame currently being processed, - * and the guest must look at frnum - 1 on interrupt, so inc frnum now */ - s->frnum = (s->frnum + 1) & 0x7ff; + for (i = 0; i < frames; i++) { + s->frame_bytes = 0; + trace_usb_uhci_frame_start(s->frnum); + uhci_async_validate_begin(s); + uhci_process_frame(s); + uhci_async_validate_end(s); + /* The spec says frnum is the frame currently being processed, and + * the guest must look at frnum - 1 on interrupt, so inc frnum now */ + s->frnum = (s->frnum + 1) & 0x7ff; + s->expire_time += frame_t; + } - /* Complete the previous frame */ + /* Complete the previous frame(s) */ if (s->pending_int_mask) { s->status2 |= s->pending_int_mask; s->status |= UHCI_STS_USBINT; @@ -1214,7 +1225,7 @@ static void uhci_frame_timer(void *opaque) } s->pending_int_mask = 0; - qemu_mod_timer(s->frame_timer, s->expire_time); + qemu_mod_timer(s->frame_timer, t_now + frame_t); } static const MemoryRegionPortio uhci_portio[] = {