xhci: Revert TD fragment hacks.
Hi Greg, Recently, we found that commit "35773dac5f86 "usb: xhci: Link TRB must not occur within a USB payload burst" causes a userspace regression. It will cause larger transfers submitted through usbfs that would have succeeded on older kernels to be rejected. Commit35773dac5fwas designed to address an issue where an ASIX USB ethernet device would get wedged when it was connected to a 1.0 xHCI host. Only this particular ethernet device was impacted, because only the ax88179_178a driver implemented scatter-gather in 3.12. The xHCI driver doesn't currently support TD fragment rules, and commit35773dac5fwas a quick hack that partially implemented one of the rules. This is the third regression this patch has caused. There's yet another quick hack to work around the issue, but I really want to support TD fragments properly, rather than hacking around it. It will take us a kernel release or two to get it implemented, since it is a big architectural change. This patchset backs out commit35773dac5f, and the two bug fix patches for it. The first patch limits arbitrarily aligned scatter-gather under xHCI 1.0 hosts. As a result of this patchset: 1. usb-storage and uas will still be able to use scatter-gather, since they submit max packet sized aligned transfers. 2. usbfs will behave exactly as before, no more userspace regressions. 3. The ax88179_178a driver works fine without scatter-gather (Mark Lord confirms this). Users of the ASIX chipset may still see occasional packet loss on 1.0 xHCI hosts, if the xHCI driver needs to split a TRB across 64-KB boundaries, and a link TRB happens to fall between those two TRBs. I expect this corner case to be infrequent. Sarah Sharp -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.14 (GNU/Linux) iQIcBAABAgAGBQJS+lcgAAoJEBMGWMLi1Gc5dt4P/1M4HtDKavU4ReJROonILiM3 eYcSVJPkYm6KeZx3YtNhzuHE4CEbMwCSmu4qrvffM87Ya8hckrS5iafciCJlwQpB AEZCEeuD/ACHF/F/BKnwMkZJCIIdjhdklqxUqUCR0tKC2BwUB6+JWrig6tHcGglV y9C1FLI910hq1UGxvF0fzK6/DpEEAIs4UzysdZ4B0u++cOI0u4cQmFLMwrY/CoqY gokzHQOALwcnbmbT8+LcjFwumGPAFsTKkbPyw+PWroiPoGlqcFQZErfSaSwojo4B ZQj3zWvYeO6plHv8rdBk1Q5sr6F+h2Mf9j6YXMLOw/i1sHn+Zryonf6/350cKi2p TjmJfOkmtQpZ+leAC2LUaTEVTNUE8FUc4tLY3KPWhR3nDLcx73UL3Lmh3monhiIz nB52e4EjAS+XN5v/Ef1FbtEUBccHiaNWUK9v44oUAur/bRl5MkN7JkDc1eqih92d 7gHtM3K9IXIzbLCOndnu8hDj/vlgw9kf7wSMC0W4+1vdRqK8PoB9GaBf4VrgWGnm toY26pG8DpRhZJdN0gVPA3YgoEcgucLB5XUHDxr3SRV3dM5zVHAz/1ZOC1phwO/Y 0CiS/5hpXd0hVp8d93RWKGtp5O1RvEKtUnFN6dW5XYJtXxgLTldxR7N340Z5DOWy /c219n1V4XIIU2DsL1Qe =0yTh -----END PGP SIGNATURE----- Merge tag 'for-usb-linus-2014-02-11' of git://git.kernel.org/pub/scm/linux/kernel/git/sarah/xhci into usb-linus Sarah writes: xhci: Revert TD fragment hacks. Hi Greg, Recently, we found that commit "35773dac5f86 "usb: xhci: Link TRB must not occur within a USB payload burst" causes a userspace regression. It will cause larger transfers submitted through usbfs that would have succeeded on older kernels to be rejected. Commit35773dac5fwas designed to address an issue where an ASIX USB ethernet device would get wedged when it was connected to a 1.0 xHCI host. Only this particular ethernet device was impacted, because only the ax88179_178a driver implemented scatter-gather in 3.12. The xHCI driver doesn't currently support TD fragment rules, and commit35773dac5fwas a quick hack that partially implemented one of the rules. This is the third regression this patch has caused. There's yet another quick hack to work around the issue, but I really want to support TD fragments properly, rather than hacking around it. It will take us a kernel release or two to get it implemented, since it is a big architectural change. This patchset backs out commit35773dac5f, and the two bug fix patches for it. The first patch limits arbitrarily aligned scatter-gather under xHCI 1.0 hosts. As a result of this patchset: 1. usb-storage and uas will still be able to use scatter-gather, since they submit max packet sized aligned transfers. 2. usbfs will behave exactly as before, no more userspace regressions. 3. The ax88179_178a driver works fine without scatter-gather (Mark Lord confirms this). Users of the ASIX chipset may still see occasional packet loss on 1.0 xHCI hosts, if the xHCI driver needs to split a TRB across 64-KB boundaries, and a link TRB happens to fall between those two TRBs. I expect this corner case to be infrequent. Sarah Sharp
This commit is contained in:
Greg Kroah-Hartman
commit
2991942f56
|
|
@ -2967,58 +2967,8 @@ static int prepare_ring(struct xhci_hcd *xhci, struct xhci_ring *ep_ring,
|
|||
}
|
||||
|
||||
while (1) {
|
||||
if (room_on_ring(xhci, ep_ring, num_trbs)) {
|
||||
union xhci_trb *trb = ep_ring->enqueue;
|
||||
unsigned int usable = ep_ring->enq_seg->trbs +
|
||||
TRBS_PER_SEGMENT - 1 - trb;
|
||||
u32 nop_cmd;
|
||||
|
||||
/*
|
||||
* Section 4.11.7.1 TD Fragments states that a link
|
||||
* TRB must only occur at the boundary between
|
||||
* data bursts (eg 512 bytes for 480M).
|
||||
* While it is possible to split a large fragment
|
||||
* we don't know the size yet.
|
||||
* Simplest solution is to fill the trb before the
|
||||
* LINK with nop commands.
|
||||
*/
|
||||
if (num_trbs == 1 || num_trbs <= usable || usable == 0)
|
||||
break;
|
||||
|
||||
if (ep_ring->type != TYPE_BULK)
|
||||
/*
|
||||
* While isoc transfers might have a buffer that
|
||||
* crosses a 64k boundary it is unlikely.
|
||||
* Since we can't add NOPs without generating
|
||||
* gaps in the traffic just hope it never
|
||||
* happens at the end of the ring.
|
||||
* This could be fixed by writing a LINK TRB
|
||||
* instead of the first NOP - however the
|
||||
* TRB_TYPE_LINK_LE32() calls would all need
|
||||
* changing to check the ring length.
|
||||
*/
|
||||
break;
|
||||
|
||||
if (num_trbs >= TRBS_PER_SEGMENT) {
|
||||
xhci_err(xhci, "Too many fragments %d, max %d\n",
|
||||
num_trbs, TRBS_PER_SEGMENT - 1);
|
||||
return -EINVAL;
|
||||
}
|
||||
|
||||
nop_cmd = cpu_to_le32(TRB_TYPE(TRB_TR_NOOP) |
|
||||
ep_ring->cycle_state);
|
||||
ep_ring->num_trbs_free -= usable;
|
||||
do {
|
||||
trb->generic.field[0] = 0;
|
||||
trb->generic.field[1] = 0;
|
||||
trb->generic.field[2] = 0;
|
||||
trb->generic.field[3] = nop_cmd;
|
||||
trb++;
|
||||
} while (--usable);
|
||||
ep_ring->enqueue = trb;
|
||||
if (room_on_ring(xhci, ep_ring, num_trbs))
|
||||
break;
|
||||
}
|
||||
if (room_on_ring(xhci, ep_ring, num_trbs))
|
||||
break;
|
||||
|
||||
if (ep_ring == xhci->cmd_ring) {
|
||||
xhci_err(xhci, "Do not support expand command ring\n");
|
||||
|
|
|
|||
|
|
@ -4730,11 +4730,8 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks)
|
|||
struct device *dev = hcd->self.controller;
|
||||
int retval;
|
||||
|
||||
/* Limit the block layer scatter-gather lists to half a segment. */
|
||||
hcd->self.sg_tablesize = TRBS_PER_SEGMENT / 2;
|
||||
|
||||
/* support to build packet from discontinuous buffers */
|
||||
hcd->self.no_sg_constraint = 1;
|
||||
/* Accept arbitrarily long scatter-gather lists */
|
||||
hcd->self.sg_tablesize = ~0;
|
||||
|
||||
/* XHCI controllers don't stop the ep queue on short packets :| */
|
||||
hcd->self.no_stop_on_short = 1;
|
||||
|
|
@ -4760,6 +4757,14 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks)
|
|||
/* xHCI private pointer was set in xhci_pci_probe for the second
|
||||
* registered roothub.
|
||||
*/
|
||||
xhci = hcd_to_xhci(hcd);
|
||||
/*
|
||||
* Support arbitrarily aligned sg-list entries on hosts without
|
||||
* TD fragment rules (which are currently unsupported).
|
||||
*/
|
||||
if (xhci->hci_version < 0x100)
|
||||
hcd->self.no_sg_constraint = 1;
|
||||
|
||||
return 0;
|
||||
}
|
||||
|
||||
|
|
@ -4788,6 +4793,9 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks)
|
|||
if (xhci->hci_version > 0x96)
|
||||
xhci->quirks |= XHCI_SPURIOUS_SUCCESS;
|
||||
|
||||
if (xhci->hci_version < 0x100)
|
||||
hcd->self.no_sg_constraint = 1;
|
||||
|
||||
/* Make sure the HC is halted. */
|
||||
retval = xhci_halt(xhci);
|
||||
if (retval)
|
||||
|
|
|
|||
|
|
@ -1268,7 +1268,7 @@ union xhci_trb {
|
|||
* since the command ring is 64-byte aligned.
|
||||
* It must also be greater than 16.
|
||||
*/
|
||||
#define TRBS_PER_SEGMENT 256
|
||||
#define TRBS_PER_SEGMENT 64
|
||||
/* Allow two commands + a link TRB, along with any reserved command TRBs */
|
||||
#define MAX_RSVD_CMD_TRBS (TRBS_PER_SEGMENT - 3)
|
||||
#define TRB_SEGMENT_SIZE (TRBS_PER_SEGMENT*16)
|
||||
|
|
|
|||
|
|
@ -1265,8 +1265,6 @@ typedef void (*usb_complete_t)(struct urb *);
|
|||
* @sg: scatter gather buffer list, the buffer size of each element in
|
||||
* the list (except the last) must be divisible by the endpoint's
|
||||
* max packet size if no_sg_constraint isn't set in 'struct usb_bus'
|
||||
* (FIXME: scatter-gather under xHCI is broken for periodic transfers.
|
||||
* Do not use urb->sg for interrupt endpoints for now, only bulk.)
|
||||
* @num_mapped_sgs: (internal) number of mapped sg entries
|
||||
* @num_sgs: number of entries in the sg list
|
||||
* @transfer_buffer_length: How big is transfer_buffer. The transfer may
|
||||
|
|
|
|||
Loading…
Reference in New Issue