From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-10.1 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8FC65C433DB for ; Thu, 28 Jan 2021 17:04:07 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 5EDC464E19 for ; Thu, 28 Jan 2021 17:04:07 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232844AbhA1REA (ORCPT ); Thu, 28 Jan 2021 12:04:00 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47502 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232854AbhA1RCW (ORCPT ); Thu, 28 Jan 2021 12:02:22 -0500 Received: from mout1.freenet.de (mout1.freenet.de [IPv6:2001:748:100:40::2:3]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 22C0BC0613ED for ; Thu, 28 Jan 2021 09:01:27 -0800 (PST) Received: from [195.4.92.124] (helo=sub5.freenet.de) by mout1.freenet.de with esmtpa (ID andihartmann@freenet.de) (port 25) (Exim 4.92 #3) id 1l5Afb-00007Y-NS; Thu, 28 Jan 2021 18:01:23 +0100 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=01019freenet.de; s=mjaymdexmjqk; h=Content-Transfer-Encoding:Content-Type: In-Reply-To:MIME-Version:Date:Message-ID:Subject:From:References:To:Sender: Reply-To:Cc:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help: List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=3mGE2yXi2Ohd4P8EXREARx+pfrrRSNGGu+pQ0tMGGz0=; b=mQzc0c4dJ8yLwKwrZ+DTqXhSV9 rBD8odu8fyXBzQHOwZaDk3ArOdGkmqJsy2YB/b1FwRqIqSQ7x+i6q8zKFQ0Xkllb9+WwqtOqxNGvM 7W2bNnwSIeOLXUNU/kLEcedsILrvEmO/uHwSNOY/2jPKpszNlh2V3BjXvQBGBJKz59S+CaTxd3KeH 2vMIZ9QvWqCx7KARtTGOxLr52vQCCdqohv/hMnAHDPUeJZ/FGHkrTFHnDR9jD/5B4mItebJ6eDPWH tWBJTjuGRWw5d0PAt46jyq8AhGZKfSDfgTcJwjTFNkDsoYUE85kyUekBSXVcWqVZCrZtmeGMwTyIv Klx+h5Ug==; Received: from p200300de573c8400505400fffe15ac42.dip0.t-ipconnect.de ([2003:de:573c:8400:5054:ff:fe15:ac42]:60120 helo=mail.maya.org) by sub5.freenet.de with esmtpsa (ID andihartmann@freenet.de) (TLSv1.2:ECDHE-RSA-AES128-GCM-SHA256:128) (port 465) (Exim 4.92 #3) id 1l5Afa-0002uv-CN; Thu, 28 Jan 2021 18:01:23 +0100 Received: internal info suppressed To: Mathias Nyman , linux-usb@vger.kernel.org References: <3b4e35d2-9508-e0aa-eaf8-32e524ad81c4@01019freenet.de> <756e7b88-1142-4758-b8f7-a8eaf510b422@01019freenet.de> <7e953b15-925a-1512-4d15-c07fc03f9059@01019freenet.de> <8de6ecb9-4a39-5742-a358-d6965feffc79@linux.intel.com> <021e1727-0e2e-0207-ace2-4382489eea1f@linux.intel.com> <9c3c9dd7-bcfb-785c-8c11-39f78a7fca01@01019freenet.de> From: Andreas Hartmann Subject: Re: USB2 / USB3 compatibility problems: xhci_hcd 0000:00:06.0: WARN Wrong bounce buffer write length: 0 != 512 Message-ID: <8d7c6b41-2706-86b3-b52f-1f55cee791d2@01019freenet.de> Date: Thu, 28 Jan 2021 18:01:19 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.6.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-Originated-At: 2003:de:573c:8400:5054:ff:fe15:ac42!60120 X-FNSign: v=2 s=575E0B8040521486F2384427B2EED68D2798FE8D2AA40D2E1B9CB8BB336AD8C5 X-Scan-TS: Thu, 28 Jan 2021 18:01:22 +0100 Precedence: bulk List-ID: X-Mailing-List: linux-usb@vger.kernel.org On 28.01.21 at 15:14 Mathias Nyman wrote: > >>> If I'm understanding it correctly, you're always creating a bounce >>> buffer though it is not necessary (at least in my case - my test patch >>> proofed, that no changes at all are necessary). Why aren't you checking >>> for URB_NO_TRANSFER_DMA_MAP at the very beginning? Or is it your purpose >>> to first basically test your new code path? That would be ok. > > Bounce buffer is only used when there is no other way of meeting the xHCI > alignment requirements. Worst case is once every 255 TRB, and for maximum 1024bytes. > Each TRB can point to 64K of data. This driver is from ~2012 - I'm not sure if they already considered USB 3.x at this time (comments to USB 2 and 1 can be found). *I* could not find any 512 Bytes related alignment code - but that doesn't say much. I tried to map the xHCI spec to the Linux urb API - I'm not sure how to map TRBs or TDs. Could a urb be understood as a TRB? > I can't go against the spec just because it doesn't cause issues in your > two usecases. The overhead of the bounce buffer usage is so small there's no > point in that kind of optimization. That's why I am a bit unsure at the moment regarding the result of the test. As I meanwhile know that it's working even w/o doing anything (by chance?), I can't say for sure that your patch is working as expected. The only thing I can say, is (see below): it doesn't hurt any more. > URB_NO_TRANSFER_DMA_MAP only indicates the data was DMA mapped before driver > submitted the URB, so usb core does not need to map it. > > I was speculating that it could be a cause why data is so oddly aligned > (urbs with data starting at less than 512 bytes from 64k boundary), and > thus not meeting the xHC boundary and aligment requirements even if data > is otherwise contiguous. > > xHC controller doesn't care who mapped the data, or if data is in a sg list > or already contiguous in urb->transfer_buffer as long as we follow > the aligment and boudary rules. > > Bug was that driver assumed the data that needed to be bounce > buffered was in a sg list. Your cased proved it could be in > urb->transfer_buffer instead. That's true for sure: the driver fills the data into transfer_buffer (it uses usb_fill_bulk_urb). >>> I tested with the notebook (in both directions) - it seems to work - I >>> didn't get any problems though I used 24 kB bulk packets. Throughput was >>> unaltered high. >>> >>> I'm doing the same test tomorrow with the other USB 3.1 controller! >> >> Successfully tested on the other USB 3.1 host. >> > > Thank you, much appreciated. > > I think I'll submit this patch as it is (with a proper commit message) > can I add reported-by: Andreas Hartmann , and > tested-by: Andreas Hartmann tags to the patch? That's OK for me. Thanks Andreas