fix DMA API abuse in various mips fbdev drivers

fix DMA API abuse in various mips fbdev drivers

[Christoph Hellwig]

Hi all,

this series fixes up three mips-specific fbdev drivers to not poke
into the return values from the DMA memory allocators, as those
aren't guranteed to be pages backed (although on mips in practice
they are).  Two of them are also fixed up to use the proper DMA API
mmap helper.

Note that the first patch is required for pending mips DMA changes,
so if they are queued up for 5.3 we'll need a stable branch that
can be pulled into the dma-mapping or mips tree.

[PATCH 1/3] au1100fb: fix DMA API abuse

[Christoph Hellwig]

Virtual addresses return from dma(m)_alloc_coherent are opaque in what
backs then, and drivers must not poke into them.  Switch the driver
to use the generic DMA API mmap helper to avoid these problems.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Acked-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
 drivers/video/fbdev/au1100fb.c | 24 ++++--------------------
 drivers/video/fbdev/au1100fb.h |  1 +
 2 files changed, 5 insertions(+), 20 deletions(-)

diff --git a/drivers/video/fbdev/au1100fb.c b/drivers/video/fbdev/au1100fb.c
index 0adf0683cf08..99941ae1f3a1 100644
--- a/drivers/video/fbdev/au1100fb.c
+++ b/drivers/video/fbdev/au1100fb.c
@@ -340,14 +340,12 @@ int au1100fb_fb_pan_display(struct fb_var_screeninfo *var, struct fb_info *fbi)
  */
 int au1100fb_fb_mmap(struct fb_info *fbi, struct vm_area_struct *vma)
 {
-	struct au1100fb_device *fbdev;
-
-	fbdev = to_au1100fb_device(fbi);
+	struct au1100fb_device *fbdev = to_au1100fb_device(fbi);
 
-	vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
 	pgprot_val(vma->vm_page_prot) |= (6 << 9); //CCA=6
 
-	return vm_iomap_memory(vma, fbdev->fb_phys, fbdev->fb_len);
+	return dma_mmap_coherent(fbdev->dev, vma, fbdev->fb_mem, fbdev->fb_phys,
+			fbdev->fb_len);
 }
 
 static struct fb_ops au1100fb_ops =
@@ -412,7 +410,6 @@ static int au1100fb_drv_probe(struct platform_device *dev)
 {
 	struct au1100fb_device *fbdev;
 	struct resource *regs_res;
-	unsigned long page;
 	struct clk *c;
 
 	/* Allocate new device private */
@@ -424,6 +421,7 @@ static int au1100fb_drv_probe(struct platform_device *dev)
 		goto failed;
 
 	platform_set_drvdata(dev, (void *)fbdev);
+	fbdev->dev = &dev->dev;
 
 	/* Allocate region for our registers and map them */
 	regs_res = platform_get_resource(dev, IORESOURCE_MEM, 0);
@@ -472,20 +470,6 @@ static int au1100fb_drv_probe(struct platform_device *dev)
 	au1100fb_fix.smem_start = fbdev->fb_phys;
 	au1100fb_fix.smem_len = fbdev->fb_len;
 
-	/*
-	 * Set page reserved so that mmap will work. This is necessary
-	 * since we'll be remapping normal memory.
-	 */
-	for (page = (unsigned long)fbdev->fb_mem;
-	     page < PAGE_ALIGN((unsigned long)fbdev->fb_mem + fbdev->fb_len);
-	     page += PAGE_SIZE) {
-#ifdef CONFIG_DMA_NONCOHERENT
-		SetPageReserved(virt_to_page(CAC_ADDR((void *)page)));
-#else
-		SetPageReserved(virt_to_page(page));
-#endif
-	}
-
 	print_dbg("Framebuffer memory map at %p", fbdev->fb_mem);
 	print_dbg("phys=0x%08x, size=%dK", fbdev->fb_phys, fbdev->fb_len / 1024);
 
diff --git a/drivers/video/fbdev/au1100fb.h b/drivers/video/fbdev/au1100fb.h
index 9af19939a9c6..e7239bceefd3 100644
--- a/drivers/video/fbdev/au1100fb.h
+++ b/drivers/video/fbdev/au1100fb.h
@@ -110,6 +110,7 @@ struct au1100fb_device {
 	dma_addr_t    		fb_phys;
 	int			panel_idx;
 	struct clk		*lcdclk;
+	struct device		*dev;
 };
 
 /********************************************************************/
-- 
2.20.1

[PATCH 2/3] au1200fb: fix DMA API abuse

[Christoph Hellwig]

Virtual addresses return from dma(m)_alloc_attrs are opaque in what
backs then, and drivers must not poke into them.  Similarly caching
modes are not supposed to be directly set by the driver.  Switch the
driver to use the generic DMA API mmap helper to avoid these problems.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/video/fbdev/au1200fb.c | 19 ++++---------------
 1 file changed, 4 insertions(+), 15 deletions(-)

diff --git a/drivers/video/fbdev/au1200fb.c b/drivers/video/fbdev/au1200fb.c
index 3872ccef4cb2..26caffb02b7e 100644
--- a/drivers/video/fbdev/au1200fb.c
+++ b/drivers/video/fbdev/au1200fb.c
@@ -147,6 +147,7 @@ struct au1200_lcd_iodata_t {
 struct au1200fb_device {
 	struct fb_info *fb_info;		/* FB driver info record */
 	struct au1200fb_platdata *pd;
+	struct device *dev;
 
 	int					plane;
 	unsigned char* 		fb_mem;		/* FrameBuffer memory map */
@@ -1232,10 +1233,8 @@ static int au1200fb_fb_mmap(struct fb_info *info, struct vm_area_struct *vma)
 {
 	struct au1200fb_device *fbdev = info->par;
 
-	vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
-	pgprot_val(vma->vm_page_prot) |= _CACHE_MASK; /* CCA=7 */
-
-	return vm_iomap_memory(vma, fbdev->fb_phys, fbdev->fb_len);
+	return dma_mmap_attrs(fbdev->dev, vma, fbdev->fb_mem, fbdev->fb_phys,
+			fbdev->fb_len, DMA_ATTR_NON_CONSISTENT);
 }
 
 static void set_global(u_int cmd, struct au1200_lcd_global_regs_t *pdata)
@@ -1647,7 +1646,6 @@ static int au1200fb_drv_probe(struct platform_device *dev)
 	struct au1200fb_device *fbdev;
 	struct au1200fb_platdata *pd;
 	struct fb_info *fbi = NULL;
-	unsigned long page;
 	int bpp, plane, ret, irq;
 
 	print_info("" DRIVER_DESC "");
@@ -1685,6 +1683,7 @@ static int au1200fb_drv_probe(struct platform_device *dev)
 		fbdev = fbi->par;
 		fbdev->fb_info = fbi;
 		fbdev->pd = pd;
+		fbdev->dev = &dev->dev;
 
 		fbdev->plane = plane;
 
@@ -1702,16 +1701,6 @@ static int au1200fb_drv_probe(struct platform_device *dev)
 			goto failed;
 		}
 
-		/*
-		 * Set page reserved so that mmap will work. This is necessary
-		 * since we'll be remapping normal memory.
-		 */
-		for (page = (unsigned long)fbdev->fb_phys;
-		     page < PAGE_ALIGN((unsigned long)fbdev->fb_phys +
-			     fbdev->fb_len);
-		     page += PAGE_SIZE) {
-			SetPageReserved(pfn_to_page(page >> PAGE_SHIFT)); /* LCD DMA is NOT coherent on Au1200 */
-		}
 		print_dbg("Framebuffer memory map at %p", fbdev->fb_mem);
 		print_dbg("phys=0x%08x, size=%dK", fbdev->fb_phys, fbdev->fb_len / 1024);
 
-- 
2.20.1

Re: [PATCH 2/3] au1200fb: fix DMA API abuse

[Manuel Lauss]

Servus Christoph,

On Thu, May 9, 2019 at 7:39 PM Christoph Hellwig <hch@lst.de> wrote:
> Virtual addresses return from dma(m)_alloc_attrs are opaque in what
> backs then, and drivers must not poke into them.  Similarly caching
> modes are not supposed to be directly set by the driver.  Switch the
> driver to use the generic DMA API mmap helper to avoid these problems.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/video/fbdev/au1200fb.c | 19 ++++---------------
>  1 file changed, 4 insertions(+), 15 deletions(-)

Runs fine on my test system.

Tested-by: Manuel Lauss <manuel.lauss@gmail.com>

Re: [PATCH 2/3] au1200fb: fix DMA API abuse

[Bartlomiej Zolnierkiewicz]

On 5/15/19 1:10 PM, Manuel Lauss wrote:
> Servus Christoph,
> 
> On Thu, May 9, 2019 at 7:39 PM Christoph Hellwig <hch@lst.de> wrote:
>> Virtual addresses return from dma(m)_alloc_attrs are opaque in what
>> backs then, and drivers must not poke into them.  Similarly caching
>> modes are not supposed to be directly set by the driver.  Switch the
>> driver to use the generic DMA API mmap helper to avoid these problems.
>>
>> Signed-off-by: Christoph Hellwig <hch@lst.de>
>> ---
>>  drivers/video/fbdev/au1200fb.c | 19 ++++---------------
>>  1 file changed, 4 insertions(+), 15 deletions(-)
> 
> Runs fine on my test system.
> 
> Tested-by: Manuel Lauss <manuel.lauss@gmail.com>

Patch queued for v5.3, thanks.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

[PATCH 3/3] jz4740_fb: fix DMA API abuse

[Christoph Hellwig]

Virtual addresses return from dma(m)_alloc_coherent are opaque in what
backs then, and drivers must not poke into them.  Switch the driver
to use the generic DMA API mmap helper to avoid these problems.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/video/fbdev/jz4740_fb.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/video/fbdev/jz4740_fb.c b/drivers/video/fbdev/jz4740_fb.c
index b57df83fdbd3..b95cdfa9e0a1 100644
--- a/drivers/video/fbdev/jz4740_fb.c
+++ b/drivers/video/fbdev/jz4740_fb.c
@@ -466,7 +466,6 @@ static int jzfb_alloc_devmem(struct jzfb *jzfb)
 {
 	int max_videosize = 0;
 	struct fb_videomode *mode = jzfb->pdata->modes;
-	void *page;
 	int i;
 
 	for (i = 0; i < jzfb->pdata->num_modes; ++mode, ++i) {
@@ -491,12 +490,6 @@ static int jzfb_alloc_devmem(struct jzfb *jzfb)
 	if (!jzfb->vidmem)
 		goto err_free_framedesc;
 
-	for (page = jzfb->vidmem;
-		 page < jzfb->vidmem + PAGE_ALIGN(jzfb->vidmem_size);
-		 page += PAGE_SIZE) {
-		SetPageReserved(virt_to_page(page));
-	}
-
 	jzfb->framedesc->next = jzfb->framedesc_phys;
 	jzfb->framedesc->addr = jzfb->vidmem_phys;
 	jzfb->framedesc->id = 0xdeafbead;
-- 
2.20.1

Re: [PATCH 3/3] jz4740_fb: fix DMA API abuse

[Bartlomiej Zolnierkiewicz]

On 5/9/19 7:38 PM, Christoph Hellwig wrote:
> Virtual addresses return from dma(m)_alloc_coherent are opaque in what
> backs then, and drivers must not poke into them.  Switch the driver
> to use the generic DMA API mmap helper to avoid these problems.
Change itself looks fine but the patch description doesn't match what
the patch is actually doing (there is no conversion to DMA API helper
because the driver is already using it). Please correct it. Thank you.

> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/video/fbdev/jz4740_fb.c | 7 -------
>  1 file changed, 7 deletions(-)
> diff --git a/drivers/video/fbdev/jz4740_fb.c b/drivers/video/fbdev/jz4740_fb.c
> index b57df83fdbd3..b95cdfa9e0a1 100644
> --- a/drivers/video/fbdev/jz4740_fb.c
> +++ b/drivers/video/fbdev/jz4740_fb.c
> @@ -466,7 +466,6 @@ static int jzfb_alloc_devmem(struct jzfb *jzfb)
>  {
>  	int max_videosize = 0;
>  	struct fb_videomode *mode = jzfb->pdata->modes;
> -	void *page;
>  	int i;
>  
>  	for (i = 0; i < jzfb->pdata->num_modes; ++mode, ++i) {
> @@ -491,12 +490,6 @@ static int jzfb_alloc_devmem(struct jzfb *jzfb)
>  	if (!jzfb->vidmem)
>  		goto err_free_framedesc;
>  
> -	for (page = jzfb->vidmem;
> -		 page < jzfb->vidmem + PAGE_ALIGN(jzfb->vidmem_size);
> -		 page += PAGE_SIZE) {
> -		SetPageReserved(virt_to_page(page));
> -	}
> -
>  	jzfb->framedesc->next = jzfb->framedesc_phys;
>  	jzfb->framedesc->addr = jzfb->vidmem_phys;
>  	jzfb->framedesc->id = 0xdeafbead;

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Re: [PATCH 3/3] jz4740_fb: fix DMA API abuse

[Bartlomiej Zolnierkiewicz]

Hi,

On 6/7/19 2:21 PM, Bartlomiej Zolnierkiewicz wrote:
> 
> On 5/9/19 7:38 PM, Christoph Hellwig wrote:
>> Virtual addresses return from dma(m)_alloc_coherent are opaque in what
>> backs then, and drivers must not poke into them.  Switch the driver
>> to use the generic DMA API mmap helper to avoid these problems.
> Change itself looks fine but the patch description doesn't match what
> the patch is actually doing (there is no conversion to DMA API helper
> because the driver is already using it). Please correct it. Thank you.

I've just removed the "Switch the driver.." sentence myself from
the patch description and applied the change, thanks!

>> Signed-off-by: Christoph Hellwig <hch@lst.de>
>> ---
>>  drivers/video/fbdev/jz4740_fb.c | 7 -------
>>  1 file changed, 7 deletions(-)
>> diff --git a/drivers/video/fbdev/jz4740_fb.c b/drivers/video/fbdev/jz4740_fb.c
>> index b57df83fdbd3..b95cdfa9e0a1 100644
>> --- a/drivers/video/fbdev/jz4740_fb.c
>> +++ b/drivers/video/fbdev/jz4740_fb.c
>> @@ -466,7 +466,6 @@ static int jzfb_alloc_devmem(struct jzfb *jzfb)
>>  {
>>  	int max_videosize = 0;
>>  	struct fb_videomode *mode = jzfb->pdata->modes;
>> -	void *page;
>>  	int i;
>>  
>>  	for (i = 0; i < jzfb->pdata->num_modes; ++mode, ++i) {
>> @@ -491,12 +490,6 @@ static int jzfb_alloc_devmem(struct jzfb *jzfb)
>>  	if (!jzfb->vidmem)
>>  		goto err_free_framedesc;
>>  
>> -	for (page = jzfb->vidmem;
>> -		 page < jzfb->vidmem + PAGE_ALIGN(jzfb->vidmem_size);
>> -		 page += PAGE_SIZE) {
>> -		SetPageReserved(virt_to_page(page));
>> -	}
>> -
>>  	jzfb->framedesc->next = jzfb->framedesc_phys;
>>  	jzfb->framedesc->addr = jzfb->vidmem_phys;
>>  	jzfb->framedesc->id = 0xdeafbead;
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Re: [PATCH 3/3] jz4740_fb: fix DMA API abuse

[Christoph Hellwig]

On Fri, Jun 21, 2019 at 01:32:16PM +0200, Bartlomiej Zolnierkiewicz wrote:
> I've just removed the "Switch the driver.." sentence myself from
> the patch description and applied the change, thanks!

Sorry for not getting back to you earlier and thanks for fixing this
up and applying the patch.