From a1c0551b62b038b495177737828f986961184110 Mon Sep 17 00:00:00 2001 From: Guenter Roeck Date: Mon, 10 Feb 2020 14:04:06 -0800 Subject: [PATCH 1/2] usb: dwc2: Simplify DMA alignment code The code to align buffers for DMA was first introduced with commit 3bc04e28a030 ("usb: dwc2: host: Get aligned DMA in a more supported way"). It was updated with commit 56406e017a88 ("usb: dwc2: Fix DMA alignment to start at allocated boundary") because it did not really align buffers to DMA boundaries but to DWC2_USB_DMA_ALIGN. This was then optimized in commit 1e111e885238 ("usb: dwc2: Fix inefficient copy of unaligned buffers") to only copy actual data rather than the whole buffer. Commit 4a4863bf2e79 ("usb: dwc2: Fix DMA cache alignment issues") changed this further to add a padding at the end of the buffer to ensure that the old data pointer is not in the same cache line as the buffer. This last commit states "Otherwise, the stored_xfer_buffer gets corrupted for IN URBs on non-cache-coherent systems". However, such corruptions are still observed. Either case, DMA is not expected to overwrite more memory than it is supposed to do, suggesting that the commit may have been hiding a problem rather than fixing it. On top of that, DMA alignment is still not guaranteed since it only happens if the original buffer is not aligned to DWC2_USB_DMA_ALIGN, which is still a constant of 4 and not really associated with DMA alignment. Move the old data pointer back to the beginning of the new buffer, restoring most of the original commit and to simplify the code. Define DWC2_USB_DMA_ALIGN to dma_get_cache_alignment() to fix the DMA alignment for real this time. Signed-off-by: Guenter Roeck --- drivers/usb/dwc2/hcd.c | 50 +++++++++++++++++++----------------------- 1 file changed, 22 insertions(+), 28 deletions(-) diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c index b90f858af960..b5841197165a 100644 --- a/drivers/usb/dwc2/hcd.c +++ b/drivers/usb/dwc2/hcd.c @@ -2469,21 +2469,24 @@ static int dwc2_alloc_split_dma_aligned_buf(struct dwc2_hsotg *hsotg, return 0; } -#define DWC2_USB_DMA_ALIGN 4 +#define DWC2_USB_DMA_ALIGN dma_get_cache_alignment() + +struct dma_aligned_buffer { + void *kmalloc_ptr; + void *old_xfer_buffer; + u8 data[0]; +}; static void dwc2_free_dma_aligned_buffer(struct urb *urb) { - void *stored_xfer_buffer; + struct dma_aligned_buffer *temp; size_t length; if (!(urb->transfer_flags & URB_ALIGNED_TEMP_BUFFER)) return; - /* Restore urb->transfer_buffer from the end of the allocated area */ - memcpy(&stored_xfer_buffer, - PTR_ALIGN(urb->transfer_buffer + urb->transfer_buffer_length, - dma_get_cache_alignment()), - sizeof(urb->transfer_buffer)); + temp = container_of(urb->transfer_buffer, + struct dma_aligned_buffer, data); if (usb_urb_dir_in(urb)) { if (usb_pipeisoc(urb->pipe)) @@ -2491,17 +2494,17 @@ static void dwc2_free_dma_aligned_buffer(struct urb *urb) else length = urb->actual_length; - memcpy(stored_xfer_buffer, urb->transfer_buffer, length); + memcpy(temp->old_xfer_buffer, temp->data, length); } - kfree(urb->transfer_buffer); - urb->transfer_buffer = stored_xfer_buffer; + urb->transfer_buffer = temp->old_xfer_buffer; + kfree(temp->kmalloc_ptr); urb->transfer_flags &= ~URB_ALIGNED_TEMP_BUFFER; } static int dwc2_alloc_dma_aligned_buffer(struct urb *urb, gfp_t mem_flags) { - void *kmalloc_ptr; + struct dma_aligned_buffer *temp, *kmalloc_ptr; size_t kmalloc_size; if (urb->num_sgs || urb->sg || @@ -2509,31 +2512,22 @@ static int dwc2_alloc_dma_aligned_buffer(struct urb *urb, gfp_t mem_flags) !((uintptr_t)urb->transfer_buffer & (DWC2_USB_DMA_ALIGN - 1))) return 0; - /* - * Allocate a buffer with enough padding for original transfer_buffer - * pointer. This allocation is guaranteed to be aligned properly for - * DMA - */ + /* Allocate a buffer with enough padding for alignment */ kmalloc_size = urb->transfer_buffer_length + - (dma_get_cache_alignment() - 1) + - sizeof(urb->transfer_buffer); + sizeof(struct dma_aligned_buffer) + DWC2_USB_DMA_ALIGN - 1; kmalloc_ptr = kmalloc(kmalloc_size, mem_flags); if (!kmalloc_ptr) return -ENOMEM; - /* - * Position value of original urb->transfer_buffer pointer to the end - * of allocation for later referencing - */ - memcpy(PTR_ALIGN(kmalloc_ptr + urb->transfer_buffer_length, - dma_get_cache_alignment()), - &urb->transfer_buffer, sizeof(urb->transfer_buffer)); - + /* Position our struct dma_aligned_buffer such that data is aligned */ + temp = PTR_ALIGN(kmalloc_ptr + 1, DWC2_USB_DMA_ALIGN) - 1; + temp->kmalloc_ptr = kmalloc_ptr; + temp->old_xfer_buffer = urb->transfer_buffer; if (usb_urb_dir_out(urb)) - memcpy(kmalloc_ptr, urb->transfer_buffer, + memcpy(temp->data, urb->transfer_buffer, urb->transfer_buffer_length); - urb->transfer_buffer = kmalloc_ptr; + urb->transfer_buffer = temp->data; urb->transfer_flags |= URB_ALIGNED_TEMP_BUFFER; -- 2.17.1