Dear Stefan Herbrechtsmeier,

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

Ok then, I think this is just perfect than and it should definitelly hit this 
release :-)

Thank you very much, sorry for pestering you too much and adding delays. Shame 
on my maintaining skills.

> Regards,
>      Stefan

Best regards,
Marek Vasut
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to