RE: [PATCH v2] USB: musb: Prevent Transmit Buffer Descriptor corruption

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

The problem needs to be fixed one way or another. It sounds like Ravi wants to move the solution upstream ("I think we need to fix the root cause by ensuring that ownership bit is cleared before the bd is reclaimed and added to the freelist." - Ravi) from where I tackled it, which makes good sense. 

Thanks,
Paul

-----Original Message-----
From: Felipe Balbi [mailto:balbi@xxxxxx] 
Sent: Friday, January 07, 2011 1:43 AM
To: B, Ravi
Cc: Paul Stuart; linux-usb@xxxxxxxxxxxxxxx; Davinci-linux-open-source@xxxxxxxxxxxxxxxxxxxx; sshtylyov@xxxxxxxxxx; Balbi, Felipe; Nori, Sekhar
Subject: Re: [PATCH v2] USB: musb: Prevent Transmit Buffer Descriptor corruption

On Mon, Jan 03, 2011 at 07:07:06PM +0530, B, Ravi wrote:
> Hi,
> > cppi_next_tx_segment is not checking for Transmit Buffer Descriptor
> > ownership before modifying parameters.
> > 
> > Transmit Buffer Descriptor ram is shared between the host processor and
> > the DMA. The "Ownership" bit is set by the host processor to give the
> > DMA ownership of the Transmit Buffer Descriptor, and the bit is cleared
> > by the DMA to return ownership to the host processor.
> > 
> > On USB Tx, when the system is heavily loaded, cppi_next_tx_segment can
> > overwrite a Transmit Buffer Descriptor that is still owned by the DMA,
> > causing DMA truncation error to fire, resulting in a channel abort. This
> > proposed fix adds a check for host processor ownership of the bd and
> > does not proceed to program it until the DMA  has ended ownership.
> [...]
> > --- a/drivers/usb/musb/cppi_dma.c	2010-12-06 20:09:04.000000000 -0800
> > +++ b/drivers/usb/musb/cppi_dma.c	2010-12-07 11:22:04.000000000 -0800
> > @@ -625,6 +625,14 @@ cppi_next_tx_segment(struct musb *musb,
> >  	 * size; for RNDIS there _is_ only that last packet.
> >  	 */
> >  	for (i = 0; i < n_bds; ) {
> > +
> > +		/* wait for DMA to release ownership of this bd */
> > +		if (unlikely(bd->hw_options & CPPI_OWN_SET)) {
> > +			do {
> > +				cpu_relax();
> > +			} while (bd->hw_options & CPPI_OWN_SET);
> > +		}
> > +
> The bd has been taken from freelist and therefore should have ownership
> Bit already cleared. I think we need to fix the root cause by ensuring
> that ownership bit is cleared before the bd is reclaimed and added to
> the freelist.

ping... do we need this patch ?

-- 
balbi
--
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


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux