public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Marek Vasut <marex@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH v2] usb_storage: fix ehci driver max transfer size
Date: Tue, 10 Jul 2012 04:12:23 +0200	[thread overview]
Message-ID: <201207100412.23870.marex@denx.de> (raw)
In-Reply-To: <fce6d97c0746ecbaaf63f29c112bbc5689c5750f.1341863102.git.stefan@herbrechtsmeier.net>

Dear Stefan Herbrechtsmeier,

> 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@denx.de>
> Signed-off-by: Stefan Herbrechtsmeier <stefan@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?

btw put braces around (start) in the macro and around (blksz) .

[...]

The rest is good, thanks! :-)

Best regards,
Marek Vasut

  reply	other threads:[~2012-07-10  2:12 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-06-18 18:22 [U-Boot] [PATCH] usb_storage: fix ehci driver max transfer size Stefan Herbrechtsmeier
2012-06-18 18:57 ` Marek Vasut
2012-07-03 18:10 ` Marek Vasut
2012-07-04  6:32   ` Stefan Herbrechtsmeier
2012-07-04  6:57     ` Schneider, Kolja
2012-07-04  7:59       ` Stefan Herbrechtsmeier
2012-07-07 21:58         ` Marek Vasut
2012-07-08 11:08           ` Stefan Herbrechtsmeier
2012-07-08 18:58             ` Marek Vasut
2012-07-09 19:52 ` [U-Boot] [PATCH v2] " Stefan Herbrechtsmeier
2012-07-10  2:12   ` Marek Vasut [this message]
2012-07-10  6:53     ` Stefan Herbrechtsmeier
2012-07-10  7:22       ` Marek Vasut
2012-07-10 17:59 ` Stefan Herbrechtsmeier
2012-07-10 18:02   ` Marek Vasut
2012-07-14 22:23     ` Ilya Yanok
2012-07-15  8:14       ` Marek Vasut
2012-07-15  8:41         ` Ilya Yanok
2012-07-15  9:51           ` Marek Vasut
2012-07-15 14:48             ` Ilya Yanok
2012-07-15  9:50   ` Marek Vasut
2012-07-18 12:50   ` Marek Vasut
2012-07-18 15:46     ` Stefan Herbrechtsmeier

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=201207100412.23870.marex@denx.de \
    --to=marex@denx.de \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox