On Wed, 26 Jun 2013, Ming Lei wrote:
> USB spec stats that short packet can only appear at the end
> of transfer. Because lost of HC(EHCI/UHCI/OHCI/...) can't
> build a full packet from discontinuous buffers, we introduce
> the limit in usb_submit_urb() to avoid such kind of bad sg
> coming from driver.
>
> The limit might be a bit strict:
> - platform has iommu to do sg list mapping
> - some host controllers may support to build full packet from
> discontinuous buffers.
>
> But considered that most of HCs don't support that, and driver
> need work well or keep consistent on different HCs or ARCHs, we
> have to introduce the limit.
>
> Currently, only usbtest is reported to pass such sg buffers to HC,
> and other users(mass storage, usbfs) don't have the problem.
>
> Reported-by: Konstantin Filatov <kfilatov@xxxxxxxxxxxxx>
> Reported-by: Denis V. Lunev <den@xxxxxxxxxx>
> Cc: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
> Cc: Felipe Balbi <balbi@xxxxxx>
> Signed-off-by: Ming Lei <ming.lei@xxxxxxxxxxxxx>
> ---
> V1:
> - avoid checking the last element in if(...) as Alan suggested
> - rewrite doc according to Alan's suggestion
>
> Thomas, please let us know if you think wireless USB needn't the check.
>
> drivers/usb/core/urb.c | 11 +++++++++++
> include/linux/usb.h | 4 +++-
> 2 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/core/urb.c b/drivers/usb/core/urb.c
> index 16927fa..3b95821 100644
> --- a/drivers/usb/core/urb.c
> +++ b/drivers/usb/core/urb.c
> @@ -7,6 +7,7 @@
> #include <linux/usb.h>
> #include <linux/wait.h>
> #include <linux/usb/hcd.h>
> +#include <linux/scatterlist.h>
>
> #define to_urb(d) container_of(d, struct urb, kref)
>
> @@ -413,6 +414,16 @@ int usb_submit_urb(struct urb *urb, gfp_t mem_flags)
> urb->iso_frame_desc[n].status = -EXDEV;
> urb->iso_frame_desc[n].actual_length = 0;
> }
> + } else {
> + /* check sg buffer size */
> + if (urb->num_sgs) {
> + struct scatterlist *sg;
> + int i;
> +
> + for_each_sg(urb->sg, sg, urb->num_sgs - 1, i)
> + if (sg->length % max)
> + return -EINVAL;
> + }
> }
>
> /* the I/O buffer must be mapped/unmapped, except when length=0 */
> diff --git a/include/linux/usb.h b/include/linux/usb.h
> index a232b7e..e99b2a1 100644
> --- a/include/linux/usb.h
> +++ b/include/linux/usb.h
> @@ -1258,7 +1258,9 @@ typedef void (*usb_complete_t)(struct urb *);
> * the device driver is saying that it provided this DMA address,
> * which the host controller driver should use in preference to the
> * transfer_buffer.
> - * @sg: scatter gather buffer list
> + * @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
> * @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
> --
> 1.7.9.5
>
>
Hi Ming,
If the target device is wireless, this check should not be necessary.
Both types of WUSB host controllers (HWA and WHCI) support building a
packet from a buffer that crosses SG boundaries.
Tom
--
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
