On Tue, Aug 06, 2019 at 09:13:54AM -0600, shuah wrote: > On 8/6/19 6:31 AM, Suwan Kim wrote: > > There are bugs on vhci with usb 3.0 storage device. In USB, each SG > > list entry buffer should be divisible by the bulk max packet size. > > But with native SG support, this problem doesn't matter because the > > SG buffer is treated as contiguous buffer. But without native SG > > support, USB storage driver breaks SG list into several URBs and the > > error occurs because of a buffer size of URB that cannot be divided > > by the bulk max packet size. The error situation is as follows. > > > > When USB Storage driver requests 31.5 KB data and has SG list which > > has 3584 bytes buffer followed by 7 4096 bytes buffer for some > > reason. USB Storage driver splits this SG list into several URBs > > because VHCI doesn't support SG and sends them separately. So the > > first URB buffer size is 3584 bytes. When receiving data from device, > > USB 3.0 device sends data packet of 1024 bytes size because the max > > packet size of BULK pipe is 1024 bytes. So device sends 4096 bytes. > > But the first URB buffer has only 3584 bytes buffer size. So host > > controller terminates the transfer even though there is more data to > > receive. So, vhci needs to support SG transfer to prevent this error. > > > > In this patch, vhci supports SG regardless of whether the server's > > host controller supports SG or not, because stub driver splits SG > > list into several URBs if the server's host controller doesn't > > support SG. > > > > To support SG, vhci_map_urb_for_dma() sets URB_DMA_MAP_SG flag in > > urb->transfer_flags if URB has SG list and this flag will tell stub > > driver to use SG list. > > > > vhci sends each SG list entry to stub driver. Then, stub driver sees > > the total length of the buffer and allocates SG table and pages > > according to the total buffer length calling sgl_alloc(). After stub > > driver receives completed URB, it again sends each SG list entry to > > vhci. > > > > If the server's host controller doesn't support SG, stub driver > > breaks a single SG request into several URBs and submits them to > > the server's host controller. When all the split URBs are completed, > > stub driver reassembles the URBs into a single return command and > > sends it to vhci. > > > > Moreover, in the situation where vhci supports SG, but stub driver > > does not, or vice versa, usbip works normally. Because there is no > > protocol modification, there is no problem in communication between > > server and client even if the one has a kernel without SG support. > > > > In the case of vhci supports SG and stub driver doesn't, because > > vhci sends only the total length of the buffer to stub driver as > > it did before the patch applied, stub driver only needs to allocate > > the required length of buffers regardless of whether vhci supports > > SG or not. > > > > If stub driver needs to send data buffer to vhci because of IN pipe, > > stub driver also sends only total length of buffer as metadata and > > then sends real data as vhci does. Then vhci receive data from stub > > driver and store it to the corresponding buffer of SG list entry. > > > > In the case of stub driver that supports SG, buffer is allocated by > > sgl_alloc(). However, stub driver that does not support SG allocates > > buffer using only kmalloc(). Therefore, if vhci supports SG and stub > > driver doesn't, stub driver has to allocate buffer with kmalloc() as > > much as the total length of SG buffer which is quite huge when vhci > > sends SG request, so it has big overhead in buffer allocation. > > > > And for the case of stub driver supports SG and vhci doesn't, since > > the USB storage driver checks that vhci doesn't support SG and sends > > the request to stub driver by splitting the SG list into multiple > > URBs, stub driver allocates a buffer with kmalloc() as it did before > > this patch. > > > > VUDC also works well with this patch. Tests are done with two USB > > gadget created by CONFIGFS USB gadget. Both use the BULK pipe. > > > > 1. Serial gadget > > 2. Mass storage gadget > > > > * Serial gadget test > > > > Serial gadget on the host sends and receives data using cat command > > on the /dev/ttyGS<N>. The client uses minicom to communicate with > > the serial gadget. > > > > * Mass storage gadget test > > > > After connecting the gadget with vhci, use "dd" to test read and > > write operation on the client side. > > > > Read - dd if=/dev/sd<N> iflag=direct of=/dev/null bs=1G count=1 > > Write - dd if=<my file path> iflag=direct of=/dev/sd<N> bs=1G count=1 > > > > Thanks for the test results. > > Were you able to test with USB lowspeed devices? I tested USB3 super-speed device and USB2 high-speed device. But I don't have full/low speed device that uses BULK transfer. In USB spec, low-speed device doesn't support BULK transfer. Do I add test description about the USB3 super-speed and USB2 high-speed device to the commit log? Regards Suwan Kim
