From patchwork Tue Dec 18 13:17:02 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: 207102 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 B54C72C008D for ; Wed, 19 Dec 2012 00:14:56 +1100 (EST) Received: from localhost ([::1]:50678 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Tkx0Y-0005gc-RW for incoming@patchwork.ozlabs.org; Tue, 18 Dec 2012 08:14:54 -0500 Received: from eggs.gnu.org ([208.118.235.92]:53380) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Tkx0P-0005g0-Cr for qemu-devel@nongnu.org; Tue, 18 Dec 2012 08:14:47 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Tkx0N-0000hD-U2 for qemu-devel@nongnu.org; Tue, 18 Dec 2012 08:14:45 -0500 Received: from mx1.redhat.com ([209.132.183.28]:3364) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Tkx0N-0000h1-Ll for qemu-devel@nongnu.org; Tue, 18 Dec 2012 08:14:43 -0500 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id qBIDEhVE019368 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Tue, 18 Dec 2012 08:14:43 -0500 Received: from shalem.localdomain.com (vpn1-7-172.ams2.redhat.com [10.36.7.172]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id qBIDEff7028796; Tue, 18 Dec 2012 08:14:41 -0500 From: Hans de Goede To: Gerd Hoffmann Date: Tue, 18 Dec 2012 14:17:02 +0100 Message-Id: <1355836622-6845-1-git-send-email-hdegoede@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 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] ehci: Use uframe precision for interrupt threshold checking (v2) 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, the following could happen: 1) Transfer completes, raises interrupt 2) .5 ms later we check if the guest has queued up any new transfers 3) We find and execute a new transfer 4) .2 ms later the new transfer completes 5) We re-run our frame_timer to write back the completion, but less then 1 ms has passed since our last run, so frindex is not changed, so the interrupt threshold code delays the interrupt 6) 1 ms from the re-run our frame-timer runs again and finally delivers the interrupt This leads to unnecessary large delays of interrupts, this code fixes this by changing frindex to uframe precision and using that for interrupt threshold control, making the interrupt fire at step 5 for guest which have low interrupt threshold settings (like Linux). Note that the guest still sees the frindex move in steps of 8 for migration compatibility. This boosts Linux read speed of a simple cheap USB thumb drive by 6 %. Changes in v2: -Make the guest see frindex move in steps of 8 by modifying ehci_opreg_read, rather then using a shadow variable Signed-off-by: Hans de Goede --- hw/usb/hcd-ehci.c | 70 +++++++++++++++++++++++++++++++++++++------------------ 1 file changed, 47 insertions(+), 23 deletions(-) diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c index c7a9a7c..320b7e7 100644 --- a/hw/usb/hcd-ehci.c +++ b/hw/usb/hcd-ehci.c @@ -109,12 +109,13 @@ #define FRAME_TIMER_FREQ 1000 #define FRAME_TIMER_NS (1000000000 / FRAME_TIMER_FREQ) +#define UFRAME_TIMER_NS (FRAME_TIMER_NS / 8) #define NB_MAXINTRATE 8 // Max rate at which controller issues ints #define BUFF_SIZE 5*4096 // Max bytes to transfer per transaction #define MAX_QH 100 // Max allowable queue heads in a chain -#define MIN_FR_PER_TICK 3 // Min frames to process when catching up -#define PERIODIC_ACTIVE 64 +#define MIN_UFR_PER_TICK 24 /* Min frames to process when catching up */ +#define PERIODIC_ACTIVE 512 /* Micro-frames */ /* Internal periodic / asynchronous schedule state machine states */ @@ -967,7 +968,15 @@ static uint64_t ehci_opreg_read(void *ptr, hwaddr addr, EHCIState *s = ptr; uint32_t val; - val = s->opreg[addr >> 2]; + switch (addr) { + case FRINDEX: + /* Round down to mult of 8, else it can go backwards on migration */ + val = s->frindex & ~7; + break; + default: + val = s->opreg[addr >> 2]; + } + trace_usb_ehci_opreg_read(addr + s->opregbase, addr2str(addr), val); return val; } @@ -1118,7 +1127,8 @@ static void ehci_opreg_write(void *ptr, hwaddr addr, break; case FRINDEX: - val &= 0x00003ff8; /* frindex is 14bits and always a multiple of 8 */ + val &= 0x00003fff; /* frindex is 14bits */ + s->usbsts_frindex = val; break; case CONFIGFLAG: @@ -2237,16 +2247,16 @@ static void ehci_advance_periodic_state(EHCIState *ehci) } } -static void ehci_update_frindex(EHCIState *ehci, int frames) +static void ehci_update_frindex(EHCIState *ehci, int uframes) { int i; - if (!ehci_enabled(ehci)) { + if (!ehci_enabled(ehci) && ehci->pstate == EST_INACTIVE) { return; } - for (i = 0; i < frames; i++) { - ehci->frindex += 8; + for (i = 0; i < uframes; i++) { + ehci->frindex++; if (ehci->frindex == 0x00002000) { ehci_raise_irq(ehci, USBSTS_FLR); @@ -2270,33 +2280,33 @@ static void ehci_frame_timer(void *opaque) int need_timer = 0; int64_t expire_time, t_now; uint64_t ns_elapsed; - int frames, skipped_frames; + int uframes, skipped_uframes; int i; t_now = qemu_get_clock_ns(vm_clock); ns_elapsed = t_now - ehci->last_run_ns; - frames = ns_elapsed / FRAME_TIMER_NS; + uframes = ns_elapsed / UFRAME_TIMER_NS; if (ehci_periodic_enabled(ehci) || ehci->pstate != EST_INACTIVE) { need_timer++; - if (frames > ehci->maxframes) { - skipped_frames = frames - ehci->maxframes; - ehci_update_frindex(ehci, skipped_frames); - ehci->last_run_ns += FRAME_TIMER_NS * skipped_frames; - frames -= skipped_frames; - DPRINTF("WARNING - EHCI skipped %d frames\n", skipped_frames); + if (uframes > (ehci->maxframes * 8)) { + skipped_uframes = uframes - (ehci->maxframes * 8); + ehci_update_frindex(ehci, skipped_uframes); + ehci->last_run_ns += UFRAME_TIMER_NS * skipped_uframes; + uframes -= skipped_uframes; + DPRINTF("WARNING - EHCI skipped %d uframes\n", skipped_uframes); } - for (i = 0; i < frames; i++) { + for (i = 0; i < uframes; i++) { /* * If we're running behind schedule, we should not catch up * too fast, as that will make some guests unhappy: - * 1) We must process a minimum of MIN_FR_PER_TICK frames, + * 1) We must process a minimum of MIN_UFR_PER_TICK frames, * otherwise we will never catch up * 2) Process frames until the guest has requested an irq (IOC) */ - if (i >= MIN_FR_PER_TICK) { + if (i >= MIN_UFR_PER_TICK) { ehci_commit_irq(ehci); if ((ehci->usbsts & USBINTR_MASK) & ehci->usbintr) { break; @@ -2306,13 +2316,15 @@ static void ehci_frame_timer(void *opaque) ehci->periodic_sched_active--; } ehci_update_frindex(ehci, 1); - ehci_advance_periodic_state(ehci); - ehci->last_run_ns += FRAME_TIMER_NS; + if ((ehci->frindex & 7) == 0) { + ehci_advance_periodic_state(ehci); + } + ehci->last_run_ns += UFRAME_TIMER_NS; } } else { ehci->periodic_sched_active = 0; - ehci_update_frindex(ehci, frames); - ehci->last_run_ns += FRAME_TIMER_NS * frames; + ehci_update_frindex(ehci, uframes); + ehci->last_run_ns += UFRAME_TIMER_NS * uframes; } if (ehci->periodic_sched_active) { @@ -2391,6 +2403,17 @@ static USBBusOps ehci_bus_ops = { .wakeup_endpoint = ehci_wakeup_endpoint, }; +static void usb_ehci_pre_save(void *opaque) +{ + EHCIState *ehci = opaque; + uint32_t new_frindex; + + /* Round down frindex to a multiple of 8 for migration compatibility */ + new_frindex = ehci->frindex & ~7; + ehci->last_run_ns -= (ehci->frindex - new_frindex) * UFRAME_TIMER_NS; + ehci->frindex = new_frindex; +} + static int usb_ehci_post_load(void *opaque, int version_id) { EHCIState *s = opaque; @@ -2441,6 +2464,7 @@ const VMStateDescription vmstate_ehci = { .name = "ehci-core", .version_id = 2, .minimum_version_id = 1, + .pre_save = usb_ehci_pre_save, .post_load = usb_ehci_post_load, .fields = (VMStateField[]) { /* mmio registers */