Ming Lei <ming.lei@xxxxxxxxxxxxx> writes:
>>> The SG entries don't come from usb-storage; they come from the block
>>> layer. As far as I know, there is no way to tell the block layer that
>>> each element in an SG list (except the last) must be a multiple of
>>> some specific size.
>>>
>>> > revert commit 3804fad45411 USBNET: ax88179_178a: enable tso if usb host supports sg dma
>>> > revert commit 247bf557273d xhci 1.0: Limit arbitrarily-aligned scatter gather.
>>> >
>>> > And we'll need to focus on getting the TD fragments supported in 3.16.
>>>
>>> So far we've gotten away with this at high speed or below, because no
>>> USB mass-storage devices have a block size smaller than 512 (at least,
>>> none that I've ever heard of). But when the maxpacket size is 1024, a
>>> request for an odd number of blocks can cause trouble.
>>
>> Ok, we can't have SuperSpeed mass storage devices broken, so it looks
>> like we'll have to revert the last patch to add scatter-gather to the
>> ASIX driver to avoid that breakage. That means Mathias is going to need
>> to revert those two commits then, since he's taking over pushing xHCI
>> driver bug fixes this kernel.
>>
>> Greg, Dave, Freddy, how do you want to handle reverting commit
>> 3804fad45411? Should that come through Dave's networking tree or Greg's
>> USB tree?
>
> Sorry, could you explain why you want to revert commit 3804fad45411
> (USBNET: ax88179_178a)?
>
> If xHCI won't plan to support arbitrary-length scatter-gather any more, that
> is fine to revert the commit forever. Otherwise, it should be better to just
> clear no_sg_constraint in xhcd, shouldn't it?
No, that's what's currently causing bugs with the storage driver.
IIUC, the bug was added by
commit 10e232c597ac ("USB: check sg buffer size in usb_submit_urb")
which introduced an additional restriction on SG URBs not present
before:
+ for_each_sg(urb->sg, sg, urb->num_sgs - 1, i)
+ if (sg->length % max)
+ return -EINVAL;
where max is usb_endpoint_maxp(&ep->desc). As has been shown by
numerous bug reports lately, the storage driver will submit SG lists
with 512 byte elements on superspeed endpoints with max == 1024.
But this bug was initially hidden before anyone noticed by
commit bcc48f1a7a0d ("USB: introduce usb_device_no_sg_constraint() helper")
commit fc76051c453b ("USB: XHCI: mark no_sg_constraint")
introducing an additional test on urb->dev->bus->no_sg_constraint and
unconditionally setting this flag for all xhci controllers. So the
buggy test was now disabled for any controller where it could possibly
fail.
And all was fine for the storage driver, until
commit 247bf557273d ("xhci 1.0: Limit arbitrarily-aligned scatter gather.")
tried to fix the ax88179_178a problems by disabling no_sg_constraint for
all xhci 1.00 controllers. This made the bug from 10e232c597ac apply to
these controllers instead, and there we are today.
This was my quick analysis of the situation. It could be inaccurate to
the point that it is completely wrong. I appreciate corrections.
And please note that I don't have access to any of the hardware
involved. That is, I have *none* of these:
- xhci 1.00 host controller
- xhci 0.96 host controller
- axis 88179/178a device
- SS usb-storage device
so do not expect me to propose any patches...
Bjørn
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
