Skip to content

Commit 35773da

Browse files
david-laightSarah Sharp
authored andcommitted
usb: xhci: Link TRB must not occur within a USB payload burst
Section 4.11.7.1 of rev 1.0 of the xhci specification states that a link TRB can only occur at a boundary between underlying USB frames (512 bytes for high speed devices). If this isn't done the USB frames aren't formatted correctly and, for example, the USB3 ethernet ax88179_178a card will stop sending (while still receiving) when running a netperf tcp transmit test with (say) and 8k buffer. This should be a candidate for stable, the ax88179_178a driver defaults to gso and tso enabled so it passes a lot of fragmented skb to the USB stack. Notes from Sarah: Discussion: http://marc.info/?l=linux-usb&m=138384509604981&w=2 This patch fixes a long-standing xHCI driver bug that was revealed by a change in 3.12 in the usb-net driver. Commit 638c511 "USBNET: support DMA SG" added support to use bulk endpoint scatter-gather (urb->sg). Only the USB ethernet drivers trigger this bug, because the mass storage driver sends sg list entries in page-sized chunks. This patch only fixes the issue for bulk endpoint scatter-gather. The problem will still occur for periodic endpoints, because hosts will interpret no-op transfers as a request to skip a service interval, which is not what we want. Luckily, the USB core isn't set up for scatter-gather on isochronous endpoints, and no USB drivers use scatter-gather for interrupt endpoints. Document this known limitation so that developers won't try to use urb->sg for interrupt endpoints until this issue is fixed. The more comprehensive fix would be to allow link TRBs in the middle of the endpoint ring and revert this patch, but that fix would touch too much code to be allowed in for stable. This patch should be backported to kernels as old as 3.12, that contain the commit 638c511 "USBNET: support DMA SG". Without this patch, the USB network device gets wedged, and stops sending packets. Mark Lord confirms this patch fixes the regression: http://marc.info/?l=linux-netdev&m=138487107625966&w=2 Signed-off-by: David Laight <david.laight@aculab.com> Signed-off-by: Sarah Sharp <sarah.a.sharp@linux.intel.com> Tested-by: Mark Lord <mlord@pobox.com> Cc: stable@vger.kernel.org
1 parent c24cb6c commit 35773da

File tree

2 files changed

+54
-2
lines changed

2 files changed

+54
-2
lines changed

drivers/usb/host/xhci-ring.c

Lines changed: 52 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2973,8 +2973,58 @@ static int prepare_ring(struct xhci_hcd *xhci, struct xhci_ring *ep_ring,
29732973
}
29742974

29752975
while (1) {
2976-
if (room_on_ring(xhci, ep_ring, num_trbs))
2977-
break;
2976+
if (room_on_ring(xhci, ep_ring, num_trbs)) {
2977+
union xhci_trb *trb = ep_ring->enqueue;
2978+
unsigned int usable = ep_ring->enq_seg->trbs +
2979+
TRBS_PER_SEGMENT - 1 - trb;
2980+
u32 nop_cmd;
2981+
2982+
/*
2983+
* Section 4.11.7.1 TD Fragments states that a link
2984+
* TRB must only occur at the boundary between
2985+
* data bursts (eg 512 bytes for 480M).
2986+
* While it is possible to split a large fragment
2987+
* we don't know the size yet.
2988+
* Simplest solution is to fill the trb before the
2989+
* LINK with nop commands.
2990+
*/
2991+
if (num_trbs == 1 || num_trbs <= usable || usable == 0)
2992+
break;
2993+
2994+
if (ep_ring->type != TYPE_BULK)
2995+
/*
2996+
* While isoc transfers might have a buffer that
2997+
* crosses a 64k boundary it is unlikely.
2998+
* Since we can't add NOPs without generating
2999+
* gaps in the traffic just hope it never
3000+
* happens at the end of the ring.
3001+
* This could be fixed by writing a LINK TRB
3002+
* instead of the first NOP - however the
3003+
* TRB_TYPE_LINK_LE32() calls would all need
3004+
* changing to check the ring length.
3005+
*/
3006+
break;
3007+
3008+
if (num_trbs >= TRBS_PER_SEGMENT) {
3009+
xhci_err(xhci, "Too many fragments %d, max %d\n",
3010+
num_trbs, TRBS_PER_SEGMENT - 1);
3011+
return -ENOMEM;
3012+
}
3013+
3014+
nop_cmd = cpu_to_le32(TRB_TYPE(TRB_TR_NOOP) |
3015+
ep_ring->cycle_state);
3016+
ep_ring->num_trbs_free -= usable;
3017+
do {
3018+
trb->generic.field[0] = 0;
3019+
trb->generic.field[1] = 0;
3020+
trb->generic.field[2] = 0;
3021+
trb->generic.field[3] = nop_cmd;
3022+
trb++;
3023+
} while (--usable);
3024+
ep_ring->enqueue = trb;
3025+
if (room_on_ring(xhci, ep_ring, num_trbs))
3026+
break;
3027+
}
29783028

29793029
if (ep_ring == xhci->cmd_ring) {
29803030
xhci_err(xhci, "Do not support expand command ring\n");

include/linux/usb.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1264,6 +1264,8 @@ typedef void (*usb_complete_t)(struct urb *);
12641264
* @sg: scatter gather buffer list, the buffer size of each element in
12651265
* the list (except the last) must be divisible by the endpoint's
12661266
* max packet size if no_sg_constraint isn't set in 'struct usb_bus'
1267+
* (FIXME: scatter-gather under xHCI is broken for periodic transfers.
1268+
* Do not use urb->sg for interrupt endpoints for now, only bulk.)
12671269
* @num_mapped_sgs: (internal) number of mapped sg entries
12681270
* @num_sgs: number of entries in the sg list
12691271
* @transfer_buffer_length: How big is transfer_buffer. The transfer may

0 commit comments

Comments
 (0)