From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:47219) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TLW0w-0004pw-2l for qemu-devel@nongnu.org; Tue, 09 Oct 2012 05:22:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TLW0o-0003Sl-16 for qemu-devel@nongnu.org; Tue, 09 Oct 2012 05:22:10 -0400 Received: from mx1.redhat.com ([209.132.183.28]:4225) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TLW0n-0003Sf-OF for qemu-devel@nongnu.org; Tue, 09 Oct 2012 05:22:01 -0400 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q999M00i014056 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Tue, 9 Oct 2012 05:22:01 -0400 Message-ID: <5073ECB6.4090408@redhat.com> Date: Tue, 09 Oct 2012 11:21:58 +0200 From: Gerd Hoffmann MIME-Version: 1.0 References: <1349682696-3046-1-git-send-email-hdegoede@redhat.com> <1349682696-3046-5-git-send-email-hdegoede@redhat.com> <5072CC04.4010300@redhat.com> <5072E838.8080104@redhat.com> In-Reply-To: <5072E838.8080104@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 04/12] usb: Add support for input pipelining List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Hans de Goede Cc: qemu-devel@nongnu.org Hi, > One possible alternative to re-combining would be to pass the > short-not-ok flag along the chain all the way down into usbfs, Yes, this is what I have in mind. > but > that won't work since the USBFS_URB_SHORT_NOT_OK flag does not > work with XHCI controllers! Oops. > So lets try to split this discussion into multiple questions: > > 1) Can we agree that re-combining input packets back into their original > transfers as done at the guest device driver level, is useful and given > the serial <-> usb converters case, even necessary to have ? Given that the USBFS_URB_SHORT_NOT_OK doesn't work there is no way around that I guess. > 2) If we agree on 1), what is the right place to do them ? > > To be able to properly combine packets into one larger transfer, an > uncombine them again later, some coordination is needed between the > entity doing the combining / uncombining and the hcd code: > > 2a) The combining entity needs to know when the hcd is done with > queuing up packets for an ep. Yes. We can add a USBDeviceClass method for that. > 2b) When uncombining it is possible that less packets are actually > used / filled with data, then were combined into one transfer. When > this happens the hcd code will stop processing the queue after the > short packet, and wait for the guest to restart it. When the guest > restarts the queue it will be modified and the un-used packets will > be gone from it. Since the hcd code keeps its own queue, when > when uncombining the hcd code needs to be told to forget about > the unused packets. Yes. We can add a new USBPortOps for that, or reuse USBPortOps->complete with USBPacket->result == USB_RET_NOT_USED. > Note that currently we have the same problem on a halt of the ep > queue already, and currently we expect the hcd to completely empty > the queue on a stall. I'm planning to change this code path over > to also let the core tell the hcd to forget about the packets it > had queued beyond the stall, as this seems something to do at the > core level rather then reproducing it in every hcd emulation > separately. The core can and should do that for packets it owns (USBPacketState == USB_PACKET_QUEUED) because they are not (yet) passed to the USBDevice. Packets owned by USBDevice (USBPacketState == USB_PACKET_ASYNC) must be handled by the USBDevice itself. > Given the necessary close interaction between the hcd code and > the combine / uncombine code + not wanting to reproduce the > same code in both host-linux.c and redirect.c I believe that > the usb core is the right place for this. I think the combining should happen just before submitting the transfer to usbfs, in usb-host. I'd like usb-host see what is really going on, and the usb core not hiding this by magically creating jumbo packets. Likewise I think usbredir should do the combining on the *server* side, not in the qemu redir code. >> Maybe add a callback to notify USBDevice >> when the host controller is done filling the queue, so USBDevices can >> process all queued packets in one go (bottom half would work too >> though). > > Changing the callback which I added for this to a bh is fine with me, > I think that both from a making clear what is going on when reading > the code, and from a having all the data hot in cache pov, the callback > is better though. Yes, callback is probably better, that is just a little minor detail though. cheers, Gerd