netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sarah Sharp <sarah.a.sharp@linux.intel.com>
To: David Laight <David.Laight@ACULAB.COM>
Cc: Ben Hutchings <bhutchings@solarflare.com>,
	Alan Stern <stern@rowland.harvard.edu>,
	netdev@vger.kernel.org, linux-usb@vger.kernel.org
Subject: Re: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.
Date: Wed, 20 Nov 2013 08:50:41 -0800	[thread overview]
Message-ID: <20131120165028.GC9611@xanatos> (raw)
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B7438@saturn3.aculab.com>

On Wed, Nov 20, 2013 at 09:46:08AM -0000, David Laight wrote:
> > From: Sarah Sharp
> ...
> > (Also, usb-storage aligns the block sizes to 512K, which explains why
> > we've never had an issue with TD fragments with that driver.)
> 
> What is a 'block' in that context?
> 512K sounds more like the value that very long transfers get chopped
> up into. With 4k pages that might be 128 fragments.

Sorry, I meant 512-byte boundaries.  See Alan's comment in
drivers/usb/storage/scsiglue.c:

        /* USB has unusual DMA-alignment requirements: Although the
         * starting address of each scatter-gather element doesn't matter,
         * the length of each element except the last must be divisible
         * by the Bulk maxpacket value.  There's currently no way to
         * express this by block-layer constraints, so we'll cop out
         * and simply require addresses to be aligned at 512-byte
         * boundaries.  This is okay since most block I/O involves
         * hardware sectors that are multiples of 512 bytes in length,
         * and since host controllers up through USB 2.0 have maxpacket
         * values no larger than 512.
         *
         * But it doesn't suffice for Wireless USB, where Bulk maxpacket
         * values can be as large as 2048.  To make that work properly
         * will require changes to the block layer.
         */
        blk_queue_update_dma_alignment(sdev->request_queue, (512 - 1));

> I'd have thought that the SG list would normally contain references
> to a number of memory pages - so each would be 4k (on x86) aligned.
> My suspicion is that the xhci controller will generate correct USB3
> data provided the link TRB is on a 1k boundary - so such data won't
> be a problem.

If the max burst size is less than four, and the scsi layer hands down
4k chunks, then the driver would still work without any modification for
TD fragments, since MBP would be 4k and there would never be a link TRB
in the middle of an MBP.

However, the driver could be in violation of the spec if the burst size
was greater than 4.  I suspect what would happen is the host controller
would read the TD, and do a shorter burst of 4 max-packet-sized 1k
chunks, and then end the burst early.  But I'm not a hardware engineer,
and we can't count on how they designed it.  I'm just trying to figure
out why usb-storage worked for so many years without running into this
issue.

> If a user program does a direct transfer from the block device
> (and that is done by locking down the user pages) then the buffer
> could have an arbitrary alignment.

Sure.  In that case though, limiting the sg_tablesize so that TDs fit
into one ring segment isn't going to help, because the block layer won't
use it.  I guess the transfers will just fail, until we can get a better
fix in.

Sarah Sharp

  parent reply	other threads:[~2013-11-20 16:51 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-11-07 17:20 [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst David Laight
2013-11-08  0:18 ` Sarah Sharp
2013-11-08 10:47   ` David Laight
2013-11-08 16:45   ` Alan Stern
     [not found]     ` <Pine.LNX.4.44L0.1311081119510.1162-100000-IYeN2dnnYyZXsRXLowluHWD2FQJk+8+b@public.gmane.org>
2013-11-08 17:39       ` David Laight
2013-11-08 18:12         ` Alan Stern
     [not found]           ` <Pine.LNX.4.44L0.1311081303460.1162-100000-IYeN2dnnYyZXsRXLowluHWD2FQJk+8+b@public.gmane.org>
2013-11-11 17:12             ` David Laight
     [not found]               ` <AE90C24D6B3A694183C094C60CF0A2F6026B73FE-CgBM+Bx2aUAnGFn1LkZF6NBPR1lH4CV8@public.gmane.org>
2013-11-11 20:34                 ` Alan Stern
2013-11-12  9:43                   ` David Laight
     [not found]                     ` <AE90C24D6B3A694183C094C60CF0A2F6026B73FF-CgBM+Bx2aUAnGFn1LkZF6NBPR1lH4CV8@public.gmane.org>
2013-11-12 16:08                       ` Alan Stern
     [not found]                         ` <Pine.LNX.4.44L0.1311121051090.1200-100000-IYeN2dnnYyZXsRXLowluHWD2FQJk+8+b@public.gmane.org>
2013-11-12 16:50                           ` David Laight
2013-11-12 19:00                             ` Alan Stern
2013-11-13  9:28                               ` David Laight
     [not found]                                 ` <AE90C24D6B3A694183C094C60CF0A2F6026B740B-CgBM+Bx2aUAnGFn1LkZF6NBPR1lH4CV8@public.gmane.org>
2013-11-13 16:20                                   ` Alan Stern
     [not found]                                     ` <Pine.LNX.4.44L0.1311131113090.1424-100000-IYeN2dnnYyZXsRXLowluHWD2FQJk+8+b@public.gmane.org>
2013-11-13 16:58                                       ` David Laight
     [not found]                                         ` <AE90C24D6B3A694183C094C60CF0A2F6026B741A-CgBM+Bx2aUAnGFn1LkZF6NBPR1lH4CV8@public.gmane.org>
2013-11-16  1:38                                           ` Ben Hutchings
     [not found]                                             ` <1384565904.29151.66.camel-/LGg1Z1CJKQ+9kgCwbf1HqK4ta4zdZpAajtMo4Cw6ucAvxtiuMwx3w@public.gmane.org>
2013-11-18  9:48                                               ` David Laight
2013-11-18 15:03                                                 ` Ben Hutchings
     [not found]                                                   ` <1384787003.10077.34.camel-nDn/Rdv9kqW9Jme8/bJn5UCKIB8iOfG2tUK59QYPAWc@public.gmane.org>
2013-11-18 15:41                                                     ` David Laight
2013-11-19 22:58                                                       ` Sarah Sharp
2013-11-20  3:09                                                         ` Alan Stern
2013-11-20  9:36                                                           ` David Laight
2013-11-20 15:03                                                             ` Eric Dumazet
     [not found]                                                               ` <1384959827.8604.141.camel-XN9IlZ5yJG9HTL0Zs8A6p/gx64E7kk8eUsxypvmhUTTZJqsBc5GL+g@public.gmane.org>
2013-11-20 15:54                                                                 ` David Laight
     [not found]                                                             ` <AE90C24D6B3A694183C094C60CF0A2F6026B7437-CgBM+Bx2aUAnGFn1LkZF6NBPR1lH4CV8@public.gmane.org>
2013-11-20 16:06                                                               ` Sarah Sharp
2013-11-20 17:02                                                                 ` David Laight
2013-11-20 17:16                                                                 ` David Laight
     [not found]                                                                   ` <AE90C24D6B3A694183C094C60CF0A2F6026B743D-CgBM+Bx2aUAnGFn1LkZF6NBPR1lH4CV8@public.gmane.org>
2013-11-20 18:39                                                                     ` Sarah Sharp
2013-11-20 21:11                                                                   ` Paul Zimmerman
2013-11-21  9:53                                                                     ` David Laight
2013-11-20  9:46                                                         ` David Laight
     [not found]                                                           ` <AE90C24D6B3A694183C094C60CF0A2F6026B7438-CgBM+Bx2aUAnGFn1LkZF6NBPR1lH4CV8@public.gmane.org>
2013-11-20 16:26                                                             ` Alan Stern
     [not found]                                                               ` <Pine.LNX.4.44L0.1311201121350.1826-100000-IYeN2dnnYyZXsRXLowluHWD2FQJk+8+b@public.gmane.org>
2013-11-20 17:08                                                                 ` David Laight
2013-11-20 16:50                                                           ` Sarah Sharp [this message]
     [not found]                   ` <Pine.LNX.4.44L0.1311111525480.17105-100000-pYrvlCTfrz9XsRXLowluHWD2FQJk+8+b@public.gmane.org>
2013-11-12 17:15                     ` Sarah Sharp
2013-11-12 17:33                       ` David Laight
2013-11-12 19:22                       ` Alan Stern
2013-11-13  9:45                         ` David Laight
     [not found]                           ` <AE90C24D6B3A694183C094C60CF0A2F6026B740C-CgBM+Bx2aUAnGFn1LkZF6NBPR1lH4CV8@public.gmane.org>
2013-11-13 16:27                             ` Alan Stern
2013-11-08 11:07 ` David Laight
     [not found]   ` <AE90C24D6B3A694183C094C60CF0A2F6026B73F1-CgBM+Bx2aUAnGFn1LkZF6NBPR1lH4CV8@public.gmane.org>
2013-11-12 17:15     ` Sarah Sharp
2013-11-12 17:28       ` David Laight

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20131120165028.GC9611@xanatos \
    --to=sarah.a.sharp@linux.intel.com \
    --cc=David.Laight@ACULAB.COM \
    --cc=bhutchings@solarflare.com \
    --cc=linux-usb@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=stern@rowland.harvard.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).