From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marek Vasut Date: Mon, 16 Sep 2019 10:10:20 +0200 Subject: [U-Boot] [PATCH] [RFC] usb: storage: Limit transfer size to 120 kiB In-Reply-To: References: <20190915222420.6891-1-marek.vasut+renesas@gmail.com> Message-ID: <8297cc1b-3cd4-557f-f01e-1d6f3879cb41@gmail.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 9/16/19 8:53 AM, Bin Meng wrote: > On Mon, Sep 16, 2019 at 6:24 AM Marek Vasut wrote: >> >> Due to constant influx of more and more weird and broken USB sticks, >> do as Linux does in commit 779b457f66e10de3471479373463b27fd308dc85 >> >> usb: storage: scsiglue: further describe our 240 sector limit >> >> Just so we have some sort of documentation as to why >> we limit our Mass Storage transfers to 240 sectors, >> let's update the comment to make clearer that >> devices were found that would choke with larger >> transfers. >> >> While at that, also make sure to clarify that other >> operating systems have similar, albeit different, >> limits on mass storage transfers. >> >> And reduce the maximum transfer length of USB storage to 120 kiB. >> >> --- >> common/usb_storage.c | 27 +++++++++++++++++---------- >> 1 file changed, 17 insertions(+), 10 deletions(-) >> >> diff --git a/common/usb_storage.c b/common/usb_storage.c >> index 8c889bb1a6..130eab7832 100644 >> --- a/common/usb_storage.c >> +++ b/common/usb_storage.c >> @@ -943,21 +943,28 @@ static void usb_stor_set_max_xfer_blk(struct usb_device *udev, >> int __maybe_unused ret; >> >> #if !CONFIG_IS_ENABLED(DM_USB) >> -#ifdef CONFIG_USB_EHCI_HCD >> /* >> - * The U-Boot EHCI driver can handle any transfer length as long as >> - * there is enough free heap space left, but the SCSI READ(10) and >> - * WRITE(10) commands are limited to 65535 blocks. >> + * Limit the total size of a transfer to 120 KB. >> + * >> + * Some devices are known to choke with anything larger. It seems like >> + * the problem stems from the fact that original IDE controllers had >> + * only an 8-bit register to hold the number of sectors in one transfer >> + * and even those couldn't handle a full 256 sectors. >> + * >> + * Because we want to make sure we interoperate with as many devices as >> + * possible, we will maintain a 240 sector transfer size limit for USB >> + * Mass Storage devices. >> + * >> + * Tests show that other operating have similar limits with Microsoft >> + * Windows 7 limiting transfers to 128 sectors for both USB2 and USB3 >> + * and Apple Mac OS X 10.11 limiting transfers to 256 sectors for USB2 >> + * and 2048 for USB3 devices. >> */ >> - blk = USHRT_MAX; >> -#else >> - blk = 20; >> -#endif >> + blk = 240; >> #else >> ret = usb_get_max_xfer_size(udev, (size_t *)&size); > > If we blindly set blk to 240 for both non-dm usb and dm usb here, Blindly ? > which makes usb_get_max_xfer_size() useless. Should we completely drop > the usb_get_max_xfer_size() call? No, I think we should keep it if we ever decided to start implementing quirks like US_FL_MAX_SECTORS_64 (cfr. Linux scsiglue.c). >> if (ret < 0) { >> - /* unimplemented, let's use default 20 */ >> - blk = 20; >> + blk = 240; >> } else { >> if (size > USHRT_MAX * 512) >> size = USHRT_MAX * 512; >> -- > > Regards, > Bin > -- Best regards, Marek Vasut