[U-Boot] [PATCH v2] usb_storage: fix ehci driver max transfer size

Stefan Herbrechtsmeier stefan at herbrechtsmeier.net
Tue Jul 10 08:53:50 CEST 2012


Am 10.07.2012 04:12, schrieb Marek Vasut:
>> The commit 5dd95cf93dfffa1d19a1928990852aac9f55b9d9 'usb_storage:
>> Fix EHCI "out of buffer pointers" with CD-ROM' introduce a bug in
>> usb_storage as it wrongly assumes that every transfer can use
>> 4096 bytes per qt_buffer. This is wrong if the start address of
>> the data is not page aligned to 4096 bytes and leads to 'EHCI
>> timed out on TD' messages because of 'out of buffer pointers'
>> in ehci_td_buffer function.
> Yes, this can be simply confirmed even with USB stick by loading to unaligned
> address. It'll make the buffers overflow too.
>
>> The bug appears during load of a fragmented file and
>> read from or write to an unaligned memory address.
>>
>> Cc: Marek Vasut <marex at denx.de>
>> Signed-off-by: Stefan Herbrechtsmeier <stefan at herbrechtsmeier.net>
>>
>> ---
>> Changes for v2:
>>   - Replace fixed worst case calculation with dynamic
>>     computation based on start address of transfer
>>
>>   common/usb_storage.c |   37 ++++++++++++++++++++-----------------
>>   1 file changed, 20 insertions(+), 17 deletions(-)
>>
>> diff --git a/common/usb_storage.c b/common/usb_storage.c
>> index faad237..bdc306f 100644
>> --- a/common/usb_storage.c
>> +++ b/common/usb_storage.c
>> @@ -150,12 +150,17 @@ struct us_data {
>>   	unsigned int	irqpipe;	 	/* pipe for release_irq */
>>   	unsigned char	irqmaxp;		/* max packed for irq Pipe */
>>   	unsigned char	irqinterval;		/* Intervall for IRQ Pipe */
>> -	unsigned long	max_xfer_blk;		/* Max blocks per xfer */
>>   	ccb		*srb;			/* current srb */
>>   	trans_reset	transport_reset;	/* reset routine */
>>   	trans_cmnd	transport;		/* transport routine */
>>   };
>>
>> +/*
>> + * The U-Boot EHCI driver cannot handle more than 5 page aligned buffers
>> + * of 4096 bytes in a transfer without running itself out of qt_buffers
>> + */
>> +#define USB_MAX_XFER_BLK(start, blksz)	(((4096 * 5) - (start % 4096)) /
>> blksz) +
> Can't something in include/common.h around line 900 can't be used?
If you mean the round functions I don't need them, as I need the
leftover of 4096 and I need to divide round down the count.

> btw put braces around (start) in the macro and around (blksz) .
I will send a v3 tonight.

Regards,
     Stefan



More information about the U-Boot mailing list