From patchwork Tue Apr 9 08:24:22 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans de Goede X-Patchwork-Id: 234988 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 851232C008F for ; Tue, 9 Apr 2013 18:22:24 +1000 (EST) Received: from localhost ([::1]:60752 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UPTos-0000UN-JE for incoming@patchwork.ozlabs.org; Tue, 09 Apr 2013 04:22:22 -0400 Received: from eggs.gnu.org ([208.118.235.92]:40492) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UPTnQ-00075M-5T for qemu-devel@nongnu.org; Tue, 09 Apr 2013 04:20:57 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UPTnN-00085z-Vx for qemu-devel@nongnu.org; Tue, 09 Apr 2013 04:20:51 -0400 Received: from mx1.redhat.com ([209.132.183.28]:8279) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UPTnN-00085g-0i for qemu-devel@nongnu.org; Tue, 09 Apr 2013 04:20:49 -0400 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 r398KmcP004700 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Tue, 9 Apr 2013 04:20:48 -0400 Received: from shalem.localdomain.com (vpn1-7-31.ams2.redhat.com [10.36.7.31]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r398KjTc009118; Tue, 9 Apr 2013 04:20:47 -0400 From: Hans de Goede To: Gerd Hoffmann Date: Tue, 9 Apr 2013 10:24:22 +0200 Message-Id: <1365495862-11276-2-git-send-email-hdegoede@redhat.com> In-Reply-To: <1365495862-11276-1-git-send-email-hdegoede@redhat.com> References: <1365495862-11276-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_free_packet: Discard finished packets when the queue is halted 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 With pipelining it is possible to encounter a finished packet when cleaning the queue due to a halt. This happens when a non stall error happens while talking to a real device. In this case the queue on the usb-host side will continue processing packets, and we can have completed packets waiting in the queue after an error condition packet causing a halt. There are 2 reasons to discard the completed packets at this point, rather then trying to writing them back to the guest: 1) The guest expect to be able to cancel and/or change packets after the packet with the error without doing an unlink, so writing them back may confuse the guest. 2) Since the queue does not advance when halted, the writing back of these packets will fail anyways since p->qtdaddr != q->qtdaddr, so the ehci_verify_qtd call in ehci_writeback_async_complete_packet will fail. Note that 2) means that then only functional change this patch introduces is the printing of a warning when this scenario happens. Note that discarding these packets means that the guest driver and the device will get out of sync! This is unfortunate, but should not be a problem since with a non stall error (iow an io-error) the 2 are out of sync already anyways. Still this patch adds a warning to signal this happening. Note that sofar this has only been seen with a DVB-T receiver, which gives of a MPEG-2 stream, which allows for recovering from lost packets, see: https://bugzilla.redhat.com/show_bug.cgi?id=890320 Signed-off-by: Hans de Goede --- hw/usb/hcd-ehci.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c index 5176251..0d3799d 100644 --- a/hw/usb/hcd-ehci.c +++ b/hw/usb/hcd-ehci.c @@ -586,17 +586,23 @@ static EHCIPacket *ehci_alloc_packet(EHCIQueue *q) static void ehci_free_packet(EHCIPacket *p) { - if (p->async == EHCI_ASYNC_FINISHED) { + if (p->async == EHCI_ASYNC_FINISHED && + !(p->queue->qh.token & QTD_TOKEN_HALT)) { ehci_writeback_async_complete_packet(p); return; } trace_usb_ehci_packet_action(p->queue, p, "free"); - if (p->async == EHCI_ASYNC_INITIALIZED) { - usb_packet_unmap(&p->packet, &p->sgl); - qemu_sglist_destroy(&p->sgl); - } if (p->async == EHCI_ASYNC_INFLIGHT) { usb_cancel_packet(&p->packet); + } + if (p->async == EHCI_ASYNC_FINISHED && + p->packet.status == USB_RET_SUCCESS) { + fprintf(stderr, + "EHCI: Dropping completed packet from halted %s ep %02X\n", + (p->pid == USB_TOKEN_IN) ? "in" : "out", + get_field(p->queue->qh.epchar, QH_EPCHAR_EP)); + } + if (p->async != EHCI_ASYNC_NONE) { usb_packet_unmap(&p->packet, &p->sgl); qemu_sglist_destroy(&p->sgl); }