On Tue, 16 Mar 2010, Alan Stern wrote: > On Mon, 15 Mar 2010, Oliver Neukum wrote: > > > > Here's a patch. Oliver, does it fix your problem? > > > > It works. > > Below is an updated version, based on a comment from James Bottomley. > Can you test it using both the text and binary usbmon interfaces? Pete > has a program for the binary interface on his web page: > > http://people.redhat.com/zaitcev/linux/usbmon-5.4.tar.gz Forget about that. Here is a completely different approach, suggested by a discussion with FUJITA Tomonori. The idea is not to try accessing the data buffer while it is still mapped; instead always do the mapping and unmapping in hcd.c at a time when usbmon won't need to look at the buffer. I think this makes the overall design a lot cleaner. Pete, I removed the test for IOMMU coalescing in mon_text.c. I figured it didn't matter since we were only going to look at the first 32 bytes of the first scatterlist buffer anyway. Is this valid? Alan Stern Index: usb-2.6/include/linux/usb.h =================================================================== --- usb-2.6.orig/include/linux/usb.h +++ usb-2.6/include/linux/usb.h @@ -963,10 +963,19 @@ extern int usb_disabled(void); * needed */ #define URB_FREE_BUFFER 0x0100 /* Free transfer buffer with the URB */ +/* The following flags are used internally by usbcore and HCDs */ #define URB_DIR_IN 0x0200 /* Transfer from device to host */ #define URB_DIR_OUT 0 #define URB_DIR_MASK URB_DIR_IN +#define URB_DMA_MAP_SINGLE 0x00010000 /* Non-scatter-gather mapping */ +#define URB_DMA_MAP_PAGE 0x00020000 /* HCD-unsupported S-G */ +#define URB_DMA_MAP_SG 0x00040000 /* HCD-supported S-G */ +#define URB_MAP_LOCAL 0x00080000 /* HCD-local-memory mapping */ +#define URB_SETUP_MAP_SINGLE 0x00100000 /* Setup packet DMA mapped */ +#define URB_SETUP_MAP_LOCAL 0x00200000 /* HCD-local setup packet */ +#define URB_DMA_SG_COMBINED 0x00400000 /* S-G entries were combined */ + struct usb_iso_packet_descriptor { unsigned int offset; unsigned int length; /* expected length */ Index: usb-2.6/drivers/usb/core/hcd.c =================================================================== --- usb-2.6.orig/drivers/usb/core/hcd.c +++ usb-2.6/drivers/usb/core/hcd.c @@ -1261,6 +1261,51 @@ static void hcd_free_coherent(struct usb *dma_handle = 0; } +static void unmap_urb_for_dma(struct usb_hcd *hcd, struct urb *urb) +{ + enum dma_data_direction dir; + + if (urb->transfer_flags & URB_SETUP_MAP_SINGLE) + dma_unmap_single(hcd->self.controller, + urb->setup_dma, + sizeof(struct usb_ctrlrequest), + DMA_TO_DEVICE); + else if (urb->transfer_flags & URB_SETUP_MAP_LOCAL) + hcd_free_coherent(urb->dev->bus, + &urb->setup_dma, + (void **) &urb->setup_packet, + sizeof(struct usb_ctrlrequest), + DMA_TO_DEVICE); + + dir = usb_urb_dir_in(urb) ? DMA_FROM_DEVICE : DMA_TO_DEVICE; + if (urb->transfer_flags & URB_DMA_MAP_SG) + dma_unmap_sg(hcd->self.controller, + urb->sg->sg, + urb->num_sgs, + dir); + else if (urb->transfer_flags & URB_DMA_MAP_PAGE) + dma_unmap_page(hcd->self.controller, + urb->transfer_dma, + urb->transfer_buffer_length, + dir); + else if (urb->transfer_flags & URB_DMA_MAP_SINGLE) + dma_unmap_single(hcd->self.controller, + urb->transfer_dma, + urb->transfer_buffer_length, + dir); + else if (urb->transfer_flags & URB_MAP_LOCAL) + hcd_free_coherent(urb->dev->bus, + &urb->transfer_dma, + &urb->transfer_buffer, + urb->transfer_buffer_length, + dir); + + /* Make it safe to call this routine more than once */ + urb->transfer_flags &= ~(URB_SETUP_MAP_SINGLE | URB_SETUP_MAP_LOCAL | + URB_DMA_MAP_SG | URB_DMA_MAP_PAGE | + URB_DMA_MAP_SINGLE | URB_MAP_LOCAL); +} + static int map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flags) { @@ -1286,6 +1331,7 @@ static int map_urb_for_dma(struct usb_hc if (dma_mapping_error(hcd->self.controller, urb->setup_dma)) return -EAGAIN; + urb->transfer_flags |= URB_SETUP_MAP_SINGLE; } else if (hcd->driver->flags & HCD_LOCAL_MEM) ret = hcd_alloc_coherent( urb->dev->bus, mem_flags, @@ -1293,20 +1339,57 @@ static int map_urb_for_dma(struct usb_hc (void **)&urb->setup_packet, sizeof(struct usb_ctrlrequest), DMA_TO_DEVICE); + if (ret) + return ret; + urb->transfer_flags |= URB_SETUP_MAP_LOCAL; } dir = usb_urb_dir_in(urb) ? DMA_FROM_DEVICE : DMA_TO_DEVICE; - if (ret == 0 && urb->transfer_buffer_length != 0 + if (urb->transfer_buffer_length != 0 && !(urb->transfer_flags & URB_NO_TRANSFER_DMA_MAP)) { if (hcd->self.uses_dma) { - urb->transfer_dma = dma_map_single ( - hcd->self.controller, - urb->transfer_buffer, - urb->transfer_buffer_length, - dir); - if (dma_mapping_error(hcd->self.controller, + if (urb->num_sgs) { + int n = dma_map_sg( + hcd->self.controller, + urb->sg->sg, + urb->num_sgs, + dir); + if (n <= 0) + ret = -EAGAIN; + else + urb->transfer_flags |= URB_DMA_MAP_SG; + if (n != urb->num_sgs) { + urb->num_sgs = n; + urb->transfer_flags |= + URB_DMA_SG_COMBINED; + } + } else if (urb->sg) { + struct scatterlist *sg; + + sg = (struct scatterlist *) urb->sg; + urb->transfer_dma = dma_map_page( + hcd->self.controller, + sg_page(sg), + sg->offset, + urb->transfer_buffer_length, + dir); + if (dma_mapping_error(hcd->self.controller, urb->transfer_dma)) - return -EAGAIN; + ret = -EAGAIN; + else + urb->transfer_flags |= URB_DMA_MAP_PAGE; + } else { + urb->transfer_dma = dma_map_single( + hcd->self.controller, + urb->transfer_buffer, + urb->transfer_buffer_length, + dir); + if (dma_mapping_error(hcd->self.controller, + urb->transfer_dma)) + ret = -EAGAIN; + else + urb->transfer_flags |= URB_DMA_MAP_SINGLE; + } } else if (hcd->driver->flags & HCD_LOCAL_MEM) { ret = hcd_alloc_coherent( urb->dev->bus, mem_flags, @@ -1314,55 +1397,16 @@ static int map_urb_for_dma(struct usb_hc &urb->transfer_buffer, urb->transfer_buffer_length, dir); - - if (ret && usb_endpoint_xfer_control(&urb->ep->desc) - && !(urb->transfer_flags & URB_NO_SETUP_DMA_MAP)) - hcd_free_coherent(urb->dev->bus, - &urb->setup_dma, - (void **)&urb->setup_packet, - sizeof(struct usb_ctrlrequest), - DMA_TO_DEVICE); + if (ret == 0) + urb->transfer_flags |= URB_MAP_LOCAL; } + if (ret && (urb->transfer_flags & (URB_SETUP_MAP_SINGLE | + URB_SETUP_MAP_LOCAL))) + unmap_urb_for_dma(hcd, urb); } return ret; } -static void unmap_urb_for_dma(struct usb_hcd *hcd, struct urb *urb) -{ - enum dma_data_direction dir; - - if (is_root_hub(urb->dev)) - return; - - if (usb_endpoint_xfer_control(&urb->ep->desc) - && !(urb->transfer_flags & URB_NO_SETUP_DMA_MAP)) { - if (hcd->self.uses_dma) - dma_unmap_single(hcd->self.controller, urb->setup_dma, - sizeof(struct usb_ctrlrequest), - DMA_TO_DEVICE); - else if (hcd->driver->flags & HCD_LOCAL_MEM) - hcd_free_coherent(urb->dev->bus, &urb->setup_dma, - (void **)&urb->setup_packet, - sizeof(struct usb_ctrlrequest), - DMA_TO_DEVICE); - } - - dir = usb_urb_dir_in(urb) ? DMA_FROM_DEVICE : DMA_TO_DEVICE; - if (urb->transfer_buffer_length != 0 - && !(urb->transfer_flags & URB_NO_TRANSFER_DMA_MAP)) { - if (hcd->self.uses_dma) - dma_unmap_single(hcd->self.controller, - urb->transfer_dma, - urb->transfer_buffer_length, - dir); - else if (hcd->driver->flags & HCD_LOCAL_MEM) - hcd_free_coherent(urb->dev->bus, &urb->transfer_dma, - &urb->transfer_buffer, - urb->transfer_buffer_length, - dir); - } -} - /*-------------------------------------------------------------------------*/ /* may be called in any context with a valid urb->dev usecount Index: usb-2.6/drivers/usb/core/message.c =================================================================== --- usb-2.6.orig/drivers/usb/core/message.c +++ usb-2.6/drivers/usb/core/message.c @@ -259,9 +259,6 @@ static void sg_clean(struct usb_sg_reque kfree(io->urbs); io->urbs = NULL; } - if (io->dev->dev.dma_mask != NULL) - usb_buffer_unmap_sg(io->dev, usb_pipein(io->pipe), - io->sg, io->nents); io->dev = NULL; } @@ -364,7 +361,6 @@ int usb_sg_init(struct usb_sg_request *i { int i; int urb_flags; - int dma; int use_sg; if (!io || !dev || !sg @@ -378,21 +374,9 @@ int usb_sg_init(struct usb_sg_request *i io->pipe = pipe; io->sg = sg; io->nents = nents; - - /* not all host controllers use DMA (like the mainstream pci ones); - * they can use PIO (sl811) or be software over another transport. - */ - dma = (dev->dev.dma_mask != NULL); - if (dma) - io->entries = usb_buffer_map_sg(dev, usb_pipein(pipe), - sg, nents); - else - io->entries = nents; + io->entries = nents; /* initialize all the urbs we'll use */ - if (io->entries <= 0) - return io->entries; - if (dev->bus->sg_tablesize > 0) { io->urbs = kmalloc(sizeof *io->urbs, mem_flags); use_sg = true; @@ -404,8 +388,6 @@ int usb_sg_init(struct usb_sg_request *i goto nomem; urb_flags = 0; - if (dma) - urb_flags |= URB_NO_TRANSFER_DMA_MAP; if (usb_pipein(pipe)) urb_flags |= URB_SHORT_NOT_OK; @@ -423,12 +405,13 @@ int usb_sg_init(struct usb_sg_request *i io->urbs[0]->complete = sg_complete; io->urbs[0]->context = io; + /* A length of zero means transfer the whole sg list */ io->urbs[0]->transfer_buffer_length = length; if (length == 0) { for_each_sg(sg, sg, io->entries, i) { io->urbs[0]->transfer_buffer_length += - sg_dma_len(sg); + sg->length; } } io->urbs[0]->sg = io; @@ -454,26 +437,16 @@ int usb_sg_init(struct usb_sg_request *i io->urbs[i]->context = io; /* - * Some systems need to revert to PIO when DMA is temporarily - * unavailable. For their sakes, both transfer_buffer and - * transfer_dma are set when possible. - * - * Note that if IOMMU coalescing occurred, we cannot - * trust sg_page anymore, so check if S/G list shrunk. + * Some systems can't use DMA; they use PIO instead. + * For their sakes, transfer_buffer is set whenever + * possible. */ - if (io->nents == io->entries && !PageHighMem(sg_page(sg))) + if (!PageHighMem(sg_page(sg))) io->urbs[i]->transfer_buffer = sg_virt(sg); else io->urbs[i]->transfer_buffer = NULL; - if (dma) { - io->urbs[i]->transfer_dma = sg_dma_address(sg); - len = sg_dma_len(sg); - } else { - /* hc may use _only_ transfer_buffer */ - len = sg->length; - } - + len = sg->length; if (length) { len = min_t(unsigned, len, length); length -= len; @@ -481,6 +454,8 @@ int usb_sg_init(struct usb_sg_request *i io->entries = i + 1; } io->urbs[i]->transfer_buffer_length = len; + + io->urbs[i]->sg = (struct usb_sg_request *) sg; } io->urbs[--i]->transfer_flags &= ~URB_NO_INTERRUPT; } Index: usb-2.6/drivers/usb/core/urb.c =================================================================== --- usb-2.6.orig/drivers/usb/core/urb.c +++ usb-2.6/drivers/usb/core/urb.c @@ -333,9 +333,12 @@ int usb_submit_urb(struct urb *urb, gfp_ is_out = usb_endpoint_dir_out(&ep->desc); } - /* Cache the direction for later use */ - urb->transfer_flags = (urb->transfer_flags & ~URB_DIR_MASK) | - (is_out ? URB_DIR_OUT : URB_DIR_IN); + /* Clear the internal flags and cache the direction for later use */ + urb->transfer_flags &= ~(URB_DIR_MASK | URB_DMA_MAP_SINGLE | + URB_DMA_MAP_PAGE | URB_DMA_MAP_SG | URB_MAP_LOCAL | + URB_SETUP_MAP_SINGLE | URB_SETUP_MAP_LOCAL | + URB_DMA_SG_COMBINED); + urb->transfer_flags |= (is_out ? URB_DIR_OUT : URB_DIR_IN); if (xfertype != USB_ENDPOINT_XFER_CONTROL && dev->state < USB_STATE_CONFIGURED) Index: usb-2.6/drivers/usb/core/usb.c =================================================================== --- usb-2.6.orig/drivers/usb/core/usb.c +++ usb-2.6/drivers/usb/core/usb.c @@ -881,6 +881,7 @@ void usb_buffer_unmap(struct urb *urb) EXPORT_SYMBOL_GPL(usb_buffer_unmap); #endif /* 0 */ +#if 0 /** * usb_buffer_map_sg - create scatterlist DMA mapping(s) for an endpoint * @dev: device to which the scatterlist will be mapped @@ -924,6 +925,7 @@ int usb_buffer_map_sg(const struct usb_d is_in ? DMA_FROM_DEVICE : DMA_TO_DEVICE) ? : -ENOMEM; } EXPORT_SYMBOL_GPL(usb_buffer_map_sg); +#endif /* XXX DISABLED, no users currently. If you wish to re-enable this * XXX please determine whether the sync is to transfer ownership of @@ -960,6 +962,7 @@ void usb_buffer_dmasync_sg(const struct EXPORT_SYMBOL_GPL(usb_buffer_dmasync_sg); #endif +#if 0 /** * usb_buffer_unmap_sg - free DMA mapping(s) for a scatterlist * @dev: device to which the scatterlist will be mapped @@ -985,6 +988,7 @@ void usb_buffer_unmap_sg(const struct us is_in ? DMA_FROM_DEVICE : DMA_TO_DEVICE); } EXPORT_SYMBOL_GPL(usb_buffer_unmap_sg); +#endif /* To disable USB, kernel command line is 'nousb' not 'usbcore.nousb' */ #ifdef MODULE Index: usb-2.6/drivers/usb/host/whci/qset.c =================================================================== --- usb-2.6.orig/drivers/usb/host/whci/qset.c +++ usb-2.6/drivers/usb/host/whci/qset.c @@ -645,7 +645,7 @@ int qset_add_urb(struct whc *whc, struct wurb->urb = urb; INIT_WORK(&wurb->dequeue_work, urb_dequeue_work); - if (urb->sg) { + if (urb->num_sgs) { ret = qset_add_urb_sg(whc, qset, urb, mem_flags); if (ret == -EINVAL) { qset_free_stds(qset, urb); Index: usb-2.6/drivers/usb/host/xhci-ring.c =================================================================== --- usb-2.6.orig/drivers/usb/host/xhci-ring.c +++ usb-2.6/drivers/usb/host/xhci-ring.c @@ -1937,7 +1937,7 @@ int xhci_queue_bulk_tx(struct xhci_hcd * int running_total, trb_buff_len, ret; u64 addr; - if (urb->sg) + if (urb->num_sgs) return queue_bulk_sg_tx(xhci, mem_flags, urb, slot_id, ep_index); ep_ring = xhci->devs[slot_id]->eps[ep_index].ring; Index: usb-2.6/drivers/usb/mon/mon_bin.c =================================================================== --- usb-2.6.orig/drivers/usb/mon/mon_bin.c +++ usb-2.6/drivers/usb/mon/mon_bin.c @@ -415,7 +415,7 @@ static unsigned int mon_bin_get_data(con } else { /* If IOMMU coalescing occurred, we cannot trust sg_page */ - if (urb->sg->nents != urb->num_sgs) { + if (urb->transfer_flags & URB_DMA_SG_COMBINED) { *flag = 'D'; return length; } Index: usb-2.6/drivers/usb/mon/mon_text.c =================================================================== --- usb-2.6.orig/drivers/usb/mon/mon_text.c +++ usb-2.6/drivers/usb/mon/mon_text.c @@ -160,9 +160,7 @@ static inline char mon_text_get_data(str } else { struct scatterlist *sg = urb->sg->sg; - /* If IOMMU coalescing occurred, we cannot trust sg_page */ - if (urb->sg->nents != urb->num_sgs || - PageHighMem(sg_page(sg))) + if (PageHighMem(sg_page(sg))) return 'D'; /* For the text interface we copy only the first sg buffer */ -- 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
