Re: [PATCH] usb: storage: Fix `us->iobuf` size for BOT transmission to prevent memory overflow

36 views
Skip to first unread message

Greg KH

unread,
Mar 11, 2025, 5:48:16 AMMar 11
to Xin Dai, st...@rowland.harvard.edu, linu...@vger.kernel.org, usb-s...@lists.one-eyed-alien.net, linux-...@vger.kernel.org
On Tue, Mar 11, 2025 at 04:41:11PM +0800, Xin Dai wrote:
> When the DWC2 controller detects a packet Babble Error, where a device
> transmits more data over USB than the host controller anticipates for a
> transaction. It follows this process:
>
> 1. The interrupt handler marks the transfer result of the URB as
> `OVERFLOW` and returns it to the USB storage driver.
> 2. The USB storage driver interprets the data phase transfer result of
> the BOT (Bulk-Only Transport) as `USB_STOR_XFER_LONG`.
> 3. The USB storage driver initiates the CSW (Command Status Wrapper)
> phase of the BOT, requests an IN transaction, and retrieves the
> execution status of the corresponding CBW (Command Block Wrapper)
> command.
> 4. The USB storage driver evaluates the CSW and finds it does not meet
> expectations. It marks the entire BOT transfer result as
> `USB_STOR_XFER_ERROR` and notifies the SCSI layer that a `DID_ERROR`
> has occurred during the transfer.
> 5. The USB storage driver requests the DWC2 controller to initiate a
> port reset, notifying the device of an issue with the previous
> transmission.
> 6. The SCSI layer implements a retransmission mechanism.
>
> In step 3, the device remains unaware of the Babble Error until the
> connected port is reset. We observed that the device continues to send
> 512 bytes of data to the host (according to the BBB Transport protocol,
> it should send only 13 bytes). However, the USB storage driver
> pre-allocates a default buffer size of 64 bytes for CBW/CSW, posing a
> risk of memory overflow. To mitigate this risk, we have adjusted the
> buffer size to 512 bytes to prevent potential errors.

Where is this memory being overflowed? I see it being used in the
usb_stor_CB_transport() call, should we just be checking the buffer size
there?

Your change just bumps the buffer up, it does not actually check any
tests for when the buffer is written to, which feels like it is not the
correct fix. What's to prevent a device from sending a bigger message
to overflow it?

But again, where exactly is the overflow happening?

thanks,

greg k-h

Xin Dai

unread,
Mar 11, 2025, 8:24:42 AMMar 11
to st...@rowland.harvard.edu, linu...@vger.kernel.org, usb-s...@lists.one-eyed-alien.net, linux-...@vger.kernel.org, Xin Dai
Signed-off-by: Xin Dai <daixi...@163.com>
---
drivers/usb/storage/usb.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/storage/usb.h b/drivers/usb/storage/usb.h
index 97c6196d639b..fd8dcb21137a 100644
--- a/drivers/usb/storage/usb.h
+++ b/drivers/usb/storage/usb.h
@@ -71,7 +71,7 @@ struct us_unusual_dev {
* size we'll allocate.
*/

-#define US_IOBUF_SIZE 64 /* Size of the DMA-mapped I/O buffer */
+#define US_IOBUF_SIZE 512 /* Size of the DMA-mapped I/O buffer */
#define US_SENSE_SIZE 18 /* Size of the autosense data buffer */

typedef int (*trans_cmnd)(struct scsi_cmnd *, struct us_data*);
--
2.34.1

Alan Stern

unread,
Mar 11, 2025, 10:12:39 AMMar 11
to Xin Dai, linu...@vger.kernel.org, usb-s...@lists.one-eyed-alien.net, linux-...@vger.kernel.org
On Tue, Mar 11, 2025 at 04:41:11PM +0800, Xin Dai wrote:
There is no risk of memory overflow. The length of the transfer for the
CSW is limited to US_BULK_CS_WRAP_LEN, which is 13. And the length of a
CBW transfer is limited to US_BULK_CB_WRAP_LEN, which is 31 (or to 32
if the US_FL_BULK32 quirk flag is set). Therefore a 64-byte buffer is
more than enough.

Alan Stern

Matthew Dharm

unread,
Mar 11, 2025, 9:09:17 PMMar 11
to Alan Stern, Xin Dai, linu...@vger.kernel.org, usb-s...@lists.one-eyed-alien.net, linux-...@vger.kernel.org
On Tue, Mar 11, 2025 at 7:12 AM Alan Stern <st...@rowland.harvard.edu> wrote:
>
> On Tue, Mar 11, 2025 at 04:41:11PM +0800, Xin Dai wrote:
> > When the DWC2 controller detects a packet Babble Error, where a device
> > transmits more data over USB than the host controller anticipates for a
> > transaction. It follows this process:
> >
> There is no risk of memory overflow. The length of the transfer for the
> CSW is limited to US_BULK_CS_WRAP_LEN, which is 13. And the length of a
> CBW transfer is limited to US_BULK_CB_WRAP_LEN, which is 31 (or to 32
> if the US_FL_BULK32 quirk flag is set). Therefore a 64-byte buffer is
> more than enough.

There is no risk of memory overflow *unless* the DWC controller
doesn't respect the buffer length as given in the URB. If there is an
overflow issue here, it is an issue with the controller level.
Matt


--
Matthew Dharm
Former Maintainer, USB Mass Storage driver for Linux

daixin_tkzc

unread,
Mar 13, 2025, 8:12:38 AMMar 13
to st...@rowland.harvard.edu, Greg KH, matthew dharm, linu...@vger.kernel.org, usb-s...@lists.one-eyed-alien.net, linux-...@vger.kernel.org
Thank you for reviewing my patch.

I'm sorry I just responded individually.

Of course, when the USB device and host are transmitting normally, us->iobuf size is 64, which is enough for CBW/CSW and there will be no problem. 
Howerver, we encountered a problem in the FPGA verification environment, that is, the DWC otg controller detected a Babble Error, and we believe that the processing flow of the device driver will cause the risk of us->iobuf overflow. 

Regarding USB Babble Error, the DWC_otg_programming manual describes it as follows:

3.8.1 Handling Babble Conditions

DWC_otg handles two cases of babble: packet babble and port babble. Packet babble occurs if the device sends more data than the maximum packet size for the channel. Port babble occurs if the controller continues to receive data from the device at EOF2 (the end of frame 2, which is very close to SOF).

When DWC_otg detects a packet babble, it stops writing data into the Rx buffer and waits for the end of packet (EOP). When it detects an EOP, it flushes already-written data in the Rx buffer and generates a Babble interrupt to the application


When the usb_stor_bulk_transport interface calls usb_stor_bulk_srb for data transmission (such as the SCSI layer 120K data read request), the controller detects a Babble Error and enters the interrupt processing function dwc2_hc_babble_intr, which eventually parses the data stage transmission result of the BOT as USB_STOR_XFER_LONGand also halts the transmission channel. However, the USB device does not know that a Babble Error has occurred at this time.

In order to ensure the integrity of the BOT transfer , the usb_stor_bulk_transfer_buf interface is called for status stage transmission(as the comments say: get CSW for device status), and an IN transfer transaction is requested: HCTSIZ register value of the host channel transmission is configured to be 512 bytes, and the DMA address is 0x7000_4000 (CBW/CSW address, see the appendix document). Under normal circumstances, the device should return 13 bytes.
However, what we observed is 512 bytes are actually returned (see the appendix document). According to our analysis, when a Babble Error occurs, the device will continuously return data.

According to the mass storage driver flow, software will parse the data header 13 bytes, which is not the expected CSW format. It is considered that the complete BOT transfer result is USB_STOR_XFER_ERROR, and the SCSI layer result is returned as DID_ERROR. The usb_stor_invoke_transport interface will initiate a port reset, that is, usb_stor_port_reset, to notify the device that a problem has occurred, then device will enter the enumeration process.
thanks,
Dai Xin
Appendix document.docx

Alan Stern

unread,
Mar 13, 2025, 10:36:36 AMMar 13
to daixin_tkzc, Greg KH, matthew dharm, linu...@vger.kernel.org, usb-s...@lists.one-eyed-alien.net, linux-...@vger.kernel.org
On Thu, Mar 13, 2025 at 08:12:20PM +0800, daixin_tkzc wrote:
> Thank you for reviewing my patch.
>
>
> I'm sorry I just responded individually.
>
>
> Of course, when the USB device and host are transmitting normally, us->iobuf size is 64, which is enough for CBW/CSW and there will be no problem.
> Howerver, we encountered a problem in the FPGA verification environment, that is, the DWC otg controller detected a Babble Error, and we believe that the processing flow of the device driver will cause the risk of us->iobuf overflow.
>
>
> Regarding USB Babble Error, the DWC_otg_programming manual describes it as follows:
> |
>
> 3.8.1 Handling Babble Conditions
>
> DWC_otg handles two cases of babble: packet babble and port babble. Packet babble occurs if the device sends more data than the maximum packet size for the channel. Port babble occurs if the controller continues to receive data from the device at EOF2 (the end of frame 2, which is very close to SOF).
>
> When DWC_otg detects a packet babble, it stops writing data into the Rx buffer and waits for the end of packet (EOP). When it detects an EOP, it flushes already-written data in the Rx buffer and generates a Babble interrupt to the application
>
> |

What is your point?

Are you claiming that the DWC_otg driver doesn't handle packet babble
properly? If that is true then you need to fix the DWC_otg driver, not
change the usb-storage driver.

You have not done a good job of explaining how us->iobuf overflow could
occur.

Alan Stern

daixin_tkzc

unread,
Mar 13, 2025, 10:28:52 PMMar 13
to Alan Stern, Greg KH, matthew dharm, linu...@vger.kernel.org, usb-s...@lists.one-eyed-alien.net, linux-...@vger.kernel.org

I'm sorry for not making my point clear. 

DWC_otg driver can handle packet babble in the data phase properly. It provides interrupt function, dwc2_hc_babble_intr, It mainly does two things:

1) Delete the URB request from the endpoint linked list maintained by the USB host controller, mark the URB transfer result as OVERFLOW error, and delete the remaining URB requests in the data phase of the BOT transfer.

2) Halt the currently used channel and indicate that the reason for the channel halt is Babble Error.

When the urb complete (babble error occurs), the sg_complete function of urb(s) will notify the mass storage driver that the data phase of the BOT transfer is over. The rest is done by the mass storage driver, such us:

1) Get CSW for device status, interpret CSW, return USB_STOR_TRANSPORT_ERROR.

2) Handle Errors(here is USB_STOR_TRANSPORT_ERROR).

3) Initiate port reset.

4) Notify the SCSI layer implements a retransmission mechanism.

How us->iobuf overflow could occur?

For 1), the USB device does not know that a Babble Error has occurred at this time (DWC_otg knows what happened), It actually continuously returns 512 bytes data through DMA write to CSW address (As can be seen in the waveform in the appendix document before). The DWC_otg controller driver cannot control how much data the device returns(13 or 512 bytes). However, the USB storage driver pre-allocates a default buffer size of 64 bytes for CBW/CSW.

For 3), the device does not realize that a babble error has occurred until the port reset is successfully completed (by interface usb_stor_port_reset). Then device will enter the enumeration process. It looks like things are heading in the right direction.

For 4), as for the urb that has a babble error, SCSI will execute a retransmission mechanism.

thanks,

Dai xin




Matthew Dharm

unread,
Mar 14, 2025, 1:38:05 AMMar 14
to daixin_tkzc, Alan Stern, Greg KH, linu...@vger.kernel.org, usb-s...@lists.one-eyed-alien.net, linux-...@vger.kernel.org
On Thu, Mar 13, 2025 at 7:28 PM daixin_tkzc <daixi...@163.com> wrote:
>
> When the urb complete (babble error occurs), the sg_complete function of urb(s) will notify the mass storage driver that the data phase of the BOT transfer is over. The rest is done by the mass storage driver, such us:

You appear very focused on a specific sequence of events which causes
the babble error, but we are telling you that you are looking in the
wrong place.

If the DWC_otc driver does, in fact, handle packet babble properly,
then it will never overflow the buffer.

For example, forget the specifics of usb-storage. Consider a BULK IN
request to an arbitrary device with an URB that provides an iobuf of
only 32 bytes, but the device sends 512 bytes -- the reason the device
sends too much data is not important; this is a babble condition. The
controller and controller driver is *required* NOT to overflow the
32-byte buffer. The remaining bytes received by the host are required
to be discarded.

Thus, even when a usb-storage device gets "out of sync" (i.e. is
sending data instead of a CSW), a buffer overflow is NOT POSSIBLE if
the controller is functioning properly. If the controller writes data
beyond the end of the buffer, then that is an error of the controller
and/or controller driver software. The design of the Linux USB stack
places this requirement on the controllers.

Matt

Greg KH

unread,
Mar 14, 2025, 1:43:37 AMMar 14
to daixin_tkzc, Alan Stern, matthew dharm, linu...@vger.kernel.org, usb-s...@lists.one-eyed-alien.net, linux-...@vger.kernel.org
On Fri, Mar 14, 2025 at 10:28:41AM +0800, daixin_tkzc wrote:
> I'm sorry for not making my point clear.

<snip>

Sorry, please do not send html email to the mailing lists. It causes
your emails to be rejected.

Also please do not top-post.

thanks,

greg k-h

Greg KH

unread,
Mar 14, 2025, 1:44:58 AMMar 14
to daixin_tkzc, Alan Stern, matthew dharm, linu...@vger.kernel.org, usb-s...@lists.one-eyed-alien.net, linux-...@vger.kernel.org
On Fri, Mar 14, 2025 at 10:28:41AM +0800, daixin_tkzc wrote:
> How us->iobuf overflow could occur?
>
> For 1), the USB device does not know that a Babble Error has occurred at this time (DWC_otg knows what happened), It actually continuously returns 512 bytes data through DMA write to CSW address (As can be seen in the waveform in the appendix document before). The DWC_otg controller driver cannot control how much data the device returns(13 or 512 bytes). However, the USB storage driver pre-allocates a default buffer size of 64 bytes for CBW/CSW.

If this really is true, it is a bug in the dwc driver. Please fix it
there, otherwise you will have to modify every single USB driver in
Linux to have a larger buffer size, not just the storage one.

thanks,

greg k-h

Alan Stern

unread,
Mar 14, 2025, 10:16:52 AMMar 14
to daixin_tkzc, Greg KH, matthew dharm, linu...@vger.kernel.org, usb-s...@lists.one-eyed-alien.net, linux-...@vger.kernel.org
On Fri, Mar 14, 2025 at 10:28:41AM +0800, daixin_tkzc wrote:
> I'm sorry for not making my point clear.
>
> DWC_otg driver can handle packet babble in the data phase properly. It provides interrupt function, dwc2_hc_babble_intr, It mainly does two things:
>
> 1) Delete the URB request from the endpoint linked list maintained by the USB host controller, mark the URB transfer result as OVERFLOW error, and delete the remaining URB requests in the data phase of the BOT transfer.
>
> 2) Halt the currently used channel and indicate that the reason for the channel halt is Babble Error.
>
> When the urb complete (babble error occurs), the sg_complete function of urb(s) will notify the mass storage driver that the data phase of the BOT transfer is over. The rest is done by the mass storage driver, such us:
>
> 1) Get CSW for device status, interpret CSW, return USB_STOR_TRANSPORT_ERROR.
>
> 2) Handle Errors(here is USB_STOR_TRANSPORT_ERROR).
>
> 3) Initiate port reset.
>
> 4) Notify the SCSI layer implements a retransmission mechanism.
>
> How us->iobuf overflow could occur?
>
> For 1), the USB device does not know that a Babble Error has occurred at this time (DWC_otg knows what happened), It actually continuously returns 512 bytes data through DMA write to CSW address (As can be seen in the waveform in the appendix document before). The DWC_otg controller driver cannot control how much data the device returns(13 or 512 bytes).

But the DWC_otg controller _can_ control how much data gets written to
the URB's transfer buffer.

> However, the USB storage driver pre-allocates a default buffer size of 64 bytes for CBW/CSW.

Furthermore, the CSW URB has a transfer_length of 13. So the DWC_otg
controller will ensure that no more than 13 bytes are written to the
buffer, even if the device sends 512 bytes. Right?

Alan Stern

> For 3), the device does not realize that a babble error has occurred until the port reset is successfully completed (by interface usb_stor_port_reset). Then device will enter the enumeration process. It looks like things are heading in the right direction.
>
> For 4), as for the urb that has a babble error, SCSI will execute a retransmission mechanism.
>
> thanks,
>
> Dai xin
>
>
>
>
>
>
>
>
>
>
>
>
>
>
> At 2025-03-13 22:36:32, "Alan Stern" <st...@rowland.harvard.edu> wrote:
> >On Thu, Mar 13, 2025 at 08:12:20PM +0800, daixin_tkzc wrote:
> >> Thank you for reviewing my patch.
> >>
> >>
> >> I'm sorry I just responded individually.
> >>
> >>
> >> Of course, when the USB device and host are transmitting normally, us->iobuf size is 64, which is enough for CBW/CSW and there will be no problem.
> >> Howerver, we encountered a problem in the FPGA verification environment, that is, the DWC otg controller detected a Babble Error, and we believe that the processing flow of the device driver will cause the risk of us->iobuf overflow.
> >>
> >>
> >> Regarding USB Babble Error, the DWC_otg_programming manual describes it as follows:
> >> |
> >>
> >> 3.8.1 Handling Babble Conditions
> >>
> >> DWC_otg handles two cases of babble: packet babble and port babble. Packet babble occurs if the device sends more data than the maximum packet size for the channel. Port babble occurs if the controller continues to receive data from the device at EOF2 (the end of frame 2, which is very close to SOF).
> >>
> >> When DWC_otg detects a packet babble, it stops writing data into the Rx buffer and waits for the end of packet (EOP). When it detects an EOP, it flushes already-written data in the Rx buffer and generates a Babble interrupt to the application
> >>
> >> |
> >
> >What is your point?
> >
> >Are you claiming that the DWC_otg driver doesn't handle packet babble
> >properly? If that is true then you need to fix the DWC_otg driver, not
> >change the usb-storage driver.
> >
> >You have not done a good job of explaining how us->iobuf overflow could
> >occur.
> >
> >Alan Stern
>
> --
> You received this message because you are subscribed to the Google Groups "USB Mass Storage on Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to usb-storage...@lists.one-eyed-alien.net.
> To view this discussion visit https://groups.google.com/a/lists.one-eyed-alien.net/d/msgid/usb-storage/1681f087.2727.195927b7ccb.Coremail.daixin_tkzc%40163.com.

daixin_tkzc

unread,
Mar 15, 2025, 5:05:58 AMMar 15
to Alan Stern, Greg KH, matthew dharm, linu...@vger.kernel.org, usb-s...@lists.one-eyed-alien.net, linux-...@vger.kernel.org

At 2025-03-14 22:16:48, "Alan Stern" <st...@rowland.harvard.edu> wrote: >On Fri, Mar 14, 2025 at 10:28:41AM +0800, daixin_tkzc wrote: >> I'm sorry for not making my point clear. >> >> DWC_otg driver can handle packet babble in the data phase properly. It provides interrupt function, dwc2_hc_babble_intr, It mainly does two things: >> >> 1) Delete the URB request from the endpoint linked list maintained by the USB host controller, mark the URB transfer result as OVERFLOW error, and delete the remaining URB requests in the data phase of the BOT transfer. >> >> 2) Halt the currently used channel and indicate that the reason for the channel halt is Babble Error. >> >> When the urb complete (babble error occurs), the sg_complete function of urb(s) will notify the mass storage driver that the data phase of the BOT transfer is over. The rest is done by the mass storage driver, such us: >> >> 1) Get CSW for device status, interpret CSW, return USB_STOR_TRANSPORT_ERROR. >> >> 2) Handle Errors(here is USB_STOR_TRANSPORT_ERROR). >> >> 3) Initiate port reset. >> >> 4) Notify the SCSI layer implements a retransmission mechanism. >> >> How us->iobuf overflow could occur? >> >> For 1), the USB device does not know that a Babble Error has occurred at this time (DWC_otg knows what happened), It actually continuously returns 512 bytes data through DMA write to CSW address (As can be seen in the waveform in the appendix document before). The DWC_otg controller driver cannot control how much data the device returns(13 or 512 bytes). > >But the DWC_otg controller _can_ control how much data gets written to >the URB's transfer buffer. > >> However, the USB storage driver pre-allocates a default buffer size of 64 bytes for CBW/CSW. > >Furthermore, the CSW URB has a transfer_length of 13. So the DWC_otg >controller will ensure that no more than 13 bytes are written to the >buffer, even if the device sends 512 bytes. Right? > >Alan Stern
>
Yes, dwc_otg should indeed ensure that DMA writes memory data no more than 13 bytes (even if Rxfifo receives 512 bytes).
But in fact, the dwc_otg manual has different configuration requirements for the XferSize register before DMA transfer: For BOT's OUT transaction, the HCTSIZ register is filled with as many bytes as the software requests to send; for IN transactions, no matter how many bytes the software requests to receive, the HCTSIZ register needs to be filled with 512 alignment, that is, the software requests 31 bytes in the CBW phase, and HCTSIZ is filled with 31; the software requests 13 bytes in the CSW phase, and HCTSIZ is still filled with 512. As observed in the waveform (I am sorry to quote the attached pictures because I am not sure whether the appendix document is visible).
data packets without Babble Error:

data packets with Babble Error (The device sent an extra packet204):

CSW buffer data through DMA write(0x70004000-0x7000400c) under normal circumstances:

CSW buffer data through DMA write(0x70004000-0x700041fc) after Babble Error:

From this, it seems that it is indeed a problem with the dwc2_otg controller, which does not handle the Babble error problem well. For example, as Matthew Dharm mentioned, dwc2_otg should discard the extra bytes. Unfortunately, software does not seem to be able to do anything about it. Please allow us to explain the reason behind changing the US_IOBUF_SIZE macro. 1) The macro comment says, “But Freecom needs a 64-byte buffer, so that's the size we'll allocate”. We thought that "Freecom" had a similar problem, otherwise a 32-byte buffer size would be enough. 2) We only observed the Babble Error phenomenon on mass storage devices, and the maximum packet size supported by the USB 2.0 controller for bulk transfers is only 512.

thanks,

Dai Xin

Greg KH

unread,
Mar 15, 2025, 5:35:49 AMMar 15
to daixin_tkzc, Alan Stern, matthew dharm, linu...@vger.kernel.org, usb-s...@lists.one-eyed-alien.net, linux-...@vger.kernel.org
On Sat, Mar 15, 2025 at 05:05:32PM +0800, daixin_tkzc wrote:
>

Again, please do not send html email, it is rejected by the mailing
lists.

Matthew Dharm

unread,
Mar 15, 2025, 5:37:54 AMMar 15
to daixin_tkzc, Alan Stern, Greg KH, linu...@vger.kernel.org, usb-s...@lists.one-eyed-alien.net, linux-...@vger.kernel.org
On Sat, Mar 15, 2025 at 2:05 AM daixin_tkzc <daixi...@163.com> wrote:
> Yes, dwc_otg should indeed ensure that DMA writes memory data no more than 13 bytes (even if Rxfifo receives 512 bytes).
> But in fact, the dwc_otg manual has different configuration requirements for the XferSize register before DMA transfer:
> For BOT's OUT transaction, the HCTSIZ register is filled with as many bytes as the software requests to send; for IN transactions, no matter how many bytes the software requests to receive, the HCTSIZ register needs to be filled with 512 alignment, that is, the software requests 31 bytes in the CBW phase, and HCTSIZ is filled with 31; the software requests 13 bytes in the CSW phase, and HCTSIZ is still filled with 512.

"Alignment" is not the same thing as "size". A buffer can be 32 bytes
and aligned on a 4MByte boundary. Hardware devices often impose
alignment requirements as it simplifies the logic required to access
the buffer.

> Please allow us to explain the reason behind changing the US_IOBUF_SIZE macro.
> 1) The macro comment says, “But Freecom needs a 64-byte buffer, so that's the
> size we'll allocate”. We thought that "Freecom" had a similar problem, otherwise a 32-byte buffer size would be enough.

Your reasoning is incorrect. The "Freecom" device does NOT have a
babble problem. The Freecom device uses a vendor-specific protocol
which requires a 64-byte buffer buffer for all of its command and
status transfers. us->iobuf is used by multiple protocols for command
and status, including CB, CBI, BOT, Freecom, and others -- as such, it
needs to be the largest size required by any of those protocols.

Matt

daixin_tkzc

unread,
Mar 15, 2025, 7:20:47 AMMar 15
to Matthew Dharm, Alan Stern, Greg KH, linu...@vger.kernel.org, usb-s...@lists.one-eyed-alien.net, linux-...@vger.kernel.org

At 2025-03-15 17:37:38, "Matthew Dharm" <mdhar...@one-eyed-alien.net> wrote: >On Sat, Mar 15, 2025 at 2:05 AM daixin_tkzc <daixi...@163.com> wrote: >> Yes, dwc_otg should indeed ensure that DMA writes memory data no more than 13 bytes (even if Rxfifo receives 512 bytes). >> But in fact, the dwc_otg manual has different configuration requirements for the XferSize register before DMA transfer: >> For BOT's OUT transaction, the HCTSIZ register is filled with as many bytes as the software requests to send; for IN transactions, no matter how many bytes the software requests to receive, the HCTSIZ register needs to be filled with 512 alignment, that is, the software requests 31 bytes in the CBW phase, and HCTSIZ is filled with 31; the software requests 13 bytes in the CSW phase, and HCTSIZ is still filled with 512. > >"Alignment" is not the same thing as "size". A buffer can be 32 bytes >and aligned on a 4MByte boundary. Hardware devices often impose >alignment requirements as it simplifies the logic required to access >the buffer. > >> Please allow us to explain the reason behind changing the US_IOBUF_SIZE macro. >> 1) The macro comment says, “But Freecom needs a 64-byte buffer, so that's the >> size we'll allocate”. We thought that "Freecom" had a similar problem, otherwise a 32-byte buffer size would be enough.
>
I'm sorry you may have misunderstood me.

HCTSIZ register only reflects the transfer size for the Host Channel (between host and device). The dwc_otg manual explains it as follows:
Non-Scatter/Gather DMA Mode: Transfer Size (XferSize) For an OUT, this field is the number of data bytes the host sends during the transfer. For an IN, this field is the buffer size that the application has Reserved for the transfer. The application is expected to program this field as an integer multiple of the maximum packet size for IN transactions (periodic and non-periodic).

We only see from the waveform on the axi bus: if babble error happened, it will write 512 bytes to memory(us->iobuf) in the CSW phase. In normal, it's 13, however the HCTSIZ register is always configured 512 in the CSW phase. This may be a problem with the controller itself.


>Your reasoning is incorrect. The "Freecom" device does NOT have a >babble problem. The Freecom device uses a vendor-specific protocol >which requires a 64-byte buffer buffer for all of its command and >status transfers. us->iobuf is used by multiple protocols for command >and status, including CB, CBI, BOT, Freecom, and others -- as such, it >needs to be the largest size required by any of those protocols. >
>Matt
Thanks for your explanation, we understand why this macro is set 64.



Alan Stern

unread,
Mar 15, 2025, 2:40:45 PMMar 15
to daixin_tkzc, Matthew Dharm, Greg KH, linu...@vger.kernel.org, usb-s...@lists.one-eyed-alien.net, linux-...@vger.kernel.org
On Sat, Mar 15, 2025 at 07:20:37PM +0800, daixin_tkzc wrote:
> I'm sorry you may have misunderstood me.
>
>
> HCTSIZ register only reflects the transfer size for the Host Channel (between host and device). The dwc_otg manual explains it as follows:
> Non-Scatter/Gather DMA Mode:
> Transfer Size (XferSize)
> For an OUT, this field is the number of data bytes the host sends
> during the transfer.
> For an IN, this field is the buffer size that the application has
> Reserved for the transfer. The application is expected to program
> this field as an integer multiple of the maximum packet size for IN
> transactions (periodic and non-periodic).

In that case, the dwc_otg driver needs to use a 512-byte bounce buffer.

The driver must _guarantee_ that no more than 13 bytes will be written
to the URB's transfer_buffer if the URB's transfer_length is 13. If the
hardware cannot provide this guarantee then the driver must work around
the hardware's deficiencies. That is how the kernel's USB API is
designed.

Alan Stern
Reply all
Reply to author
Forward
0 new messages