[PATCH] usb: devio: fix mmap() on non-coherent DMA architectures

[PATCH] usb: devio: fix mmap() on non-coherent DMA architectures

[gavinli]

From: Gavin Li <git@thegavinli.com>

On architectures that are not (or are optionally) DMA coherent,
dma_alloc_coherent() returns an address into the vmalloc space,
and calling virt_to_phys() on this address returns an unusable
physical address.

This patch replaces the raw remap_pfn_range() call with a call to
dmap_mmap_coherent(), which takes care of the differences between
coherent and non-coherent code paths.

Tested on an arm64 rk3399 board.

Signed-off-by: Gavin Li <git@thegavinli.com>
---
 drivers/usb/core/devio.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
index a02448105527..76ec9aef3eff 100644
--- a/drivers/usb/core/devio.c
+++ b/drivers/usb/core/devio.c
@@ -241,11 +241,10 @@ 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) {
+	ret = dma_mmap_coherent(ps->dev->bus->sysdev, vma, mem, dma_handle, size);
+	if (ret) {
 		dec_usb_memory_use_count(usbm, &usbm->vma_use_count);
-		return -EAGAIN;
+		return ret;
 	}
 
 	vma->vm_flags |= VM_IO;
-- 
2.22.0

Re: [PATCH] usb: devio: fix mmap() on non-coherent DMA architectures

[Greg KH]

On Thu, Aug 01, 2019 at 03:04:36PM -0700, gavinli@thegavinli.com wrote:
> From: Gavin Li <git@thegavinli.com>
> 
> On architectures that are not (or are optionally) DMA coherent,
> dma_alloc_coherent() returns an address into the vmalloc space,
> and calling virt_to_phys() on this address returns an unusable
> physical address.
> 
> This patch replaces the raw remap_pfn_range() call with a call to
> dmap_mmap_coherent(), which takes care of the differences between
> coherent and non-coherent code paths.
> 
> Tested on an arm64 rk3399 board.
> 
> Signed-off-by: Gavin Li <git@thegavinli.com>
> ---
>  drivers/usb/core/devio.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)

Should this be backported to the stable kernel trees to fix the issue on
those platforms?  If so, how far back?  What commit caused this problem
to occur?

thanks,

greg k-h

Re: [PATCH] usb: devio: fix mmap() on non-coherent DMA architectures

[Gavin Li]

usbfs mmap() looks like it was introduced for 4.6 in commit
f7d34b445abc, so it should probably be backported to 4.9 and onwards.
This issue has been present since the introduction of the feature.

One sidenote: this patch will cause the following warning on x86 due
to dmap_mmap_coherent() trying to map normal memory in as uncached:

x86/PAT: ... map pfn RAM range req uncached-minus for [mem
0x77b000000-0x77b210fff], got write-back

This warning is harmless, as x86 is DMA coherent and the memory gets
correctly mapped in as write-back. I will submit a patch to the DMA
mapping team to eliminate this warning.

Best,
Gavin

On Fri, Aug 2, 2019 at 5:14 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Thu, Aug 01, 2019 at 03:04:36PM -0700, gavinli@thegavinli.com wrote:
> > From: Gavin Li <git@thegavinli.com>
> >
> > On architectures that are not (or are optionally) DMA coherent,
> > dma_alloc_coherent() returns an address into the vmalloc space,
> > and calling virt_to_phys() on this address returns an unusable
> > physical address.
> >
> > This patch replaces the raw remap_pfn_range() call with a call to
> > dmap_mmap_coherent(), which takes care of the differences between
> > coherent and non-coherent code paths.
> >
> > Tested on an arm64 rk3399 board.
> >
> > Signed-off-by: Gavin Li <git@thegavinli.com>
> > ---
> >  drivers/usb/core/devio.c | 7 +++----
> >  1 file changed, 3 insertions(+), 4 deletions(-)
>
> Should this be backported to the stable kernel trees to fix the issue on
> those platforms?  If so, how far back?  What commit caused this problem
> to occur?
>
> thanks,
>
> greg k-h

Re: [PATCH] usb: devio: fix mmap() on non-coherent DMA architectures

[Greg KH]

On Fri, Aug 02, 2019 at 10:57:00AM -0700, Gavin Li wrote:
> usbfs mmap() looks like it was introduced for 4.6 in commit
> f7d34b445abc, so it should probably be backported to 4.9 and onwards.
> This issue has been present since the introduction of the feature.
> 
> One sidenote: this patch will cause the following warning on x86 due
> to dmap_mmap_coherent() trying to map normal memory in as uncached:
> 
> x86/PAT: ... map pfn RAM range req uncached-minus for [mem
> 0x77b000000-0x77b210fff], got write-back
> 
> This warning is harmless, as x86 is DMA coherent and the memory gets
> correctly mapped in as write-back. I will submit a patch to the DMA
> mapping team to eliminate this warning.

Let me know what the git commit id of that patch is, I will wait for it
to hit the tree before adding this one.

thanks,

greg k-h

Re: [PATCH] usb: devio: fix mmap() on non-coherent DMA architectures

[Greg KH]

On Mon, Aug 05, 2019 at 05:17:13PM +0200, Greg KH wrote:
> On Fri, Aug 02, 2019 at 10:57:00AM -0700, Gavin Li wrote:
> > usbfs mmap() looks like it was introduced for 4.6 in commit
> > f7d34b445abc, so it should probably be backported to 4.9 and onwards.
> > This issue has been present since the introduction of the feature.
> > 
> > One sidenote: this patch will cause the following warning on x86 due
> > to dmap_mmap_coherent() trying to map normal memory in as uncached:
> > 
> > x86/PAT: ... map pfn RAM range req uncached-minus for [mem
> > 0x77b000000-0x77b210fff], got write-back
> > 
> > This warning is harmless, as x86 is DMA coherent and the memory gets
> > correctly mapped in as write-back. I will submit a patch to the DMA
> > mapping team to eliminate this warning.
> 
> Let me know what the git commit id of that patch is, I will wait for it
> to hit the tree before adding this one.

Dropping this patch from my queue now, please feel free to resend when
you have refreshed it against the latest tree.

thanks,

greg k-h

Re: [PATCH] usb: devio: fix mmap() on non-coherent DMA architectures

[Gavin Li]

The purpose of this section of code is to map that memory into
userspace, and the code before this patch would incorrectly calculate
the pfn required to do so. This patch simply changes it to use the
correct function to do so rather than doing it from scratch.

On Mon, Aug 5, 2019 at 4:37 AM David Laight <David.Laight@aculab.com> wrote:
>
> From: gavinli@thegavinli.com
> > Sent: 01 August 2019 23:02
> >
> > On architectures that are not (or are optionally) DMA coherent,
> > dma_alloc_coherent() returns an address into the vmalloc space,
> > and calling virt_to_phys() on this address returns an unusable
> > physical address.
>
> So? what is the code trying to use the return value of virt_to_phys() for?
>
> The 'cpu physical address' isn't (usually) a very interesting number.
> The value you normally want is the address the hardware should use
> in order to access the memory - this isn't (in general) the same value.
> (It might be different for different devices.)
>
> ISTR that dma_alloc_coherent() returns this value to the caller.
>
>         David
>
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
>

RE: [PATCH] usb: devio: fix mmap() on non-coherent DMA architectures

[David Laight]

From: gavinli@thegavinli.com
> Sent: 01 August 2019 23:02
>
> On architectures that are not (or are optionally) DMA coherent,
> dma_alloc_coherent() returns an address into the vmalloc space,
> and calling virt_to_phys() on this address returns an unusable
> physical address.

So? what is the code trying to use the return value of virt_to_phys() for?

The 'cpu physical address' isn't (usually) a very interesting number.
The value you normally want is the address the hardware should use
in order to access the memory - this isn't (in general) the same value.
(It might be different for different devices.)

ISTR that dma_alloc_coherent() returns this value to the caller.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

[PATCH] usb: devio: fix mmap() on non-coherent DMA architectures

[gavinli]

From: Gavin Li <git@thegavinli.com>

On architectures that are not (or are optionally) DMA coherent,
dma_alloc_coherent() returns an address into the vmalloc space,
and calling virt_to_phys() on this address returns an unusable
physical address.

This patch replaces the raw remap_pfn_range() call with a call to
dmap_mmap_coherent(), which takes care of the differences between
coherent and non-coherent code paths.

Tested on an arm64 rk3399 board.

Signed-off-by: Gavin Li <git@thegavinli.com>
---
 drivers/usb/core/devio.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
index a02448105527..76ec9aef3eff 100644
--- a/drivers/usb/core/devio.c
+++ b/drivers/usb/core/devio.c
@@ -241,11 +241,10 @@ 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) {
+	ret = dma_mmap_coherent(ps->dev->bus->sysdev, vma, mem, dma_handle, size);
+	if (ret) {
 		dec_usb_memory_use_count(usbm, &usbm->vma_use_count);
-		return -EAGAIN;
+		return ret;
 	}
 
 	vma->vm_flags |= VM_IO;
-- 
2.22.0