| Message ID | 20200504201348.1183246-1-jeremy.linton@arm.com (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 2bef9aed6f0e22391c8d4570749b1acc9bc3981e |
| Headers | show |
| Series | [v2] usb: usbfs: correct kernel->user page attribute mismatch | expand |
On 20-05-04 15:13:48, Jeremy Linton wrote: > On some architectures (e.g. arm64) requests for > IO coherent memory may use non-cachable attributes if > the relevant device isn't cache coherent. If these > pages are then remapped into userspace as cacheable, > they may not be coherent with the non-cacheable mappings. > > In particular this happens with libusb, when it attempts > to create zero-copy buffers for use by rtl-sdr > (https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fosmocom%2Frtl-sdr%2F&data=02%7C01%7Cpeter.chen%40nxp.com%7Cb088a6f1a4d5462d65e808d7f067b1b2%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C637242200467457447&sdata=nPbLuHBIWqKSc0h4UGqRBv7W7wcQ0SYwq51ggLVcBVs%3D&reserved=0). On low end arm > devices with non-coherent USB ports, the application will > be unexpectedly killed, while continuing to work fine on > arm machines with coherent USB controllers. > > This bug has been discovered/reported a few times over > the last few years. In the case of rtl-sdr a compile time > option to enable/disable zero copy was implemented to > work around it. > > Rather than relaying on application specific workarounds, > dma_mmap_coherent() can be used instead of remap_pfn_range(). > The page cache/etc attributes will then be correctly set in > userspace to match the kernel mapping. > > Signed-off-by: Jeremy Linton <jeremy.linton@arm.com> > --- > v1->v2: > Update commit message and change to dma_mmap_coherent() > from dma_mmap_attr(,,,0) which are the same. > > drivers/usb/core/devio.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c > index 6833c918abce..b9db9812d6c5 100644 > --- a/drivers/usb/core/devio.c > +++ b/drivers/usb/core/devio.c > @@ -217,6 +217,7 @@ static int usbdev_mmap(struct file *file, struct vm_area_struct *vma) > { > struct usb_memory *usbm = NULL; > struct usb_dev_state *ps = file->private_data; > + struct usb_hcd *hcd = bus_to_hcd(ps->dev->bus); > size_t size = vma->vm_end - vma->vm_start; > void *mem; > unsigned long flags; > @@ -250,9 +251,7 @@ static int usbdev_mmap(struct file *file, struct vm_area_struct *vma) > usbm->vma_use_count = 1; > INIT_LIST_HEAD(&usbm->memlist); > > - if (remap_pfn_range(vma, vma->vm_start, > - virt_to_phys(usbm->mem) >> PAGE_SHIFT, > - size, vma->vm_page_prot) < 0) { > + if (dma_mmap_coherent(hcd->self.sysdev, vma, mem, dma_handle, size)) { > dec_usb_memory_use_count(usbm, &usbm->vma_use_count); > return -EAGAIN; > } > -- > 2.24.1 > Tested at NXP imx8qm (ARM64) platform, it fixes mmap issue for using libusb. I am not sure if it covers all use cases at hcd_buffer_alloc.
diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c index 6833c918abce..b9db9812d6c5 100644 --- a/drivers/usb/core/devio.c +++ b/drivers/usb/core/devio.c @@ -217,6 +217,7 @@ static int usbdev_mmap(struct file *file, struct vm_area_struct *vma) { struct usb_memory *usbm = NULL; struct usb_dev_state *ps = file->private_data; + struct usb_hcd *hcd = bus_to_hcd(ps->dev->bus); size_t size = vma->vm_end - vma->vm_start; void *mem; unsigned long flags; @@ -250,9 +251,7 @@ static int usbdev_mmap(struct file *file, struct vm_area_struct *vma) usbm->vma_use_count = 1; INIT_LIST_HEAD(&usbm->memlist); - if (remap_pfn_range(vma, vma->vm_start, - virt_to_phys(usbm->mem) >> PAGE_SHIFT, - size, vma->vm_page_prot) < 0) { + if (dma_mmap_coherent(hcd->self.sysdev, vma, mem, dma_handle, size)) { dec_usb_memory_use_count(usbm, &usbm->vma_use_count); return -EAGAIN; }
On some architectures (e.g. arm64) requests for IO coherent memory may use non-cachable attributes if the relevant device isn't cache coherent. If these pages are then remapped into userspace as cacheable, they may not be coherent with the non-cacheable mappings. In particular this happens with libusb, when it attempts to create zero-copy buffers for use by rtl-sdr (https://github.com/osmocom/rtl-sdr/). On low end arm devices with non-coherent USB ports, the application will be unexpectedly killed, while continuing to work fine on arm machines with coherent USB controllers. This bug has been discovered/reported a few times over the last few years. In the case of rtl-sdr a compile time option to enable/disable zero copy was implemented to work around it. Rather than relaying on application specific workarounds, dma_mmap_coherent() can be used instead of remap_pfn_range(). The page cache/etc attributes will then be correctly set in userspace to match the kernel mapping. Signed-off-by: Jeremy Linton <jeremy.linton@arm.com> --- v1->v2: Update commit message and change to dma_mmap_coherent() from dma_mmap_attr(,,,0) which are the same. drivers/usb/core/devio.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)