Message-ID: <AE90C24D6B3A694183C094C60CF0A2F6026B7407@saturn3.aculab.com>
Date:	Tue, 12 Nov 2013 16:50:35 -0000
From:	"David Laight" <David.Laight@...LAB.COM>
To:	"Alan Stern" <stern@...land.harvard.edu>
Cc:	"Sarah Sharp" <sarah.a.sharp@...ux.intel.com>,
	<netdev@...r.kernel.org>, <linux-usb@...r.kernel.org>
Subject: RE: [PATCH] usb: xhci: Link TRB must not occur with a USB payload burst.

> > If all the fragments are larger than the MBP (assume 16k) then
> > that would be relatively easy. However that is very dependant
> > on the source of the data. It might be true for disk data, but
> > is unlikely to be true for ethernet data.
> 
> I don't quite understand your point.  Are you saying that if all the
> TRBs are very short, you might need more than 64 TRBs to reach a 16-KB
> boundary?

Since you don't really want to do all the work twice, the sensible
way is to add each input fragment to the ring one a time.
If you need to cross a link TRB and the last MBP boundary is within
the previous data TRB then you can split the previous data TRB at the
MBP boundary and continue.
If the previous TRB is short then you'd need to go back through the
earlier TRB until you found the one that contains a TRB boundary,
split it, and write a link TRB in the following slot.
If you are within the first MBP then you'd need to replace the first
TRB of the message with a link TRB.

And yes, if the data is really fragmented you might need a lot of
TRB for even 1k of data.

> > For bulk data the link TRB can be forced at a packet boundary
> > by splitting the TD up - the receiving end won't know the difference.
> 
> That won't work.  What happens if you split a TD up into two pieces and
> the first piece receives a short packet?  The host controller will
> automatically move to the start of the second piece.  That's not what
> we want.

You can split a bulk TD on a 1k boundary and the target won't know the
difference.

> > There is no necessity for taking an interrupt from every link segment.
> 
> Yes, there is.  The HCD needs to know when the dequeue pointer has
> moved beyond the end of the ring segment, so that it can start reusing
> the TRB slots in that segment.

You already know that because of the interrupts for the data packets
themselves.

> > I would change the code to use a single segment (for coding simplicity)
> > and queue bulk URB when there isn't enough ring space.
> > URB with too many fragments could either be rejected, sent in sections,
> > or partially linearised (and probably still sent in sections).
> 
> Rejecting an URB is not feasible.  Splitting it up into multiple TDs is
> not acceptable, as explained above.  Sending it in sections (i.e.,
> queueing only some of the TRBs at any time) would work, provided you
> got at least two interrupts every time the queue wrapped around (which
> suggests you might want at least two ring segments).

Rejecting badly fragmented URB is almost certainly ok. You really don't
want the hardware overhead of processing a TRB every few bytes.
This would be especially bad on iommu systems.
Before the ring expansion code was added there was an implicit
limit of (probably) 125 fragments for a URB. Exceeding this limit
wasn't the reason for adding the ring expansion code.

And, as I've pointed out, both bulk and isoc URB can be split unless
they are using too many fragments for a short piece of data.

The current code refuses to write into a TRB segment until it is
empty - I think that restriction is only there so that it can
add another segment when the space runs out.

	David





--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists