diff mbox series

[v3,1/5] fbdev: Put mmap for deferred I/O into drivers

Message ID 20220426120359.17437-2-tzimmermann@suse.de (mailing list archive)
State Handled Elsewhere
Headers show
Series fbdev: Decouple deferred I/O from struct page | expand

Commit Message

Thomas Zimmermann April 26, 2022, 12:03 p.m. UTC 
The fbdev mmap function fb_mmap() unconditionally overrides the
driver's implementation if deferred I/O has been activated. This
makes it hard to implement mmap with anything but a vmalloc()'ed
software buffer. That is specifically a problem for DRM, where
video memory is maintained by a memory manager.

Leave the mmap handling to drivers and expect them to call the
helper for deferred I/O by thmeselves.

v3:
	* fix warning if fb_mmap is missing (kernel test robot)
v2:
	* print a helpful error message if the defio setup is
	  incorrect (Javier)

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
 drivers/gpu/drm/drm_fb_helper.c        |  4 +++-
 drivers/gpu/drm/vmwgfx/vmwgfx_fb.c     |  1 +
 drivers/hid/hid-picolcd_fb.c           |  1 +
 drivers/staging/fbtft/fbtft-core.c     |  1 +
 drivers/video/fbdev/broadsheetfb.c     |  1 +
 drivers/video/fbdev/core/fb_defio.c    |  1 +
 drivers/video/fbdev/core/fbmem.c       | 21 +++++++++++----------
 drivers/video/fbdev/hecubafb.c         |  1 +
 drivers/video/fbdev/hyperv_fb.c        |  1 +
 drivers/video/fbdev/metronomefb.c      |  1 +
 drivers/video/fbdev/sh_mobile_lcdcfb.c |  6 ++++++
 drivers/video/fbdev/smscufx.c          |  3 +++
 drivers/video/fbdev/ssd1307fb.c        |  1 +
 drivers/video/fbdev/udlfb.c            |  3 +++
 drivers/video/fbdev/xen-fbfront.c      |  1 +
 15 files changed, 36 insertions(+), 11 deletions(-)

Comments

Dan Carpenter April 28, 2022, 9:43 a.m. UTC | #1 
Hi Thomas,

url:    https://github.com/intel-lab-lkp/linux/commits/Thomas-Zimmermann/fbdev-Decouple-deferred-I-O-from-struct-page/20220426-200655
base:   0e7deff6446a4ba2c75f499a0bfa80cd6a15c129
config: i386-randconfig-m021 (https://download.01.org/0day-ci/archive/20220428/202204280832.sGHcYdgQ-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.2.0-20) 11.2.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

New smatch warnings:
drivers/video/fbdev/core/fbmem.c:1389 fb_mmap() warn: inconsistent returns '&info->mm_lock'.

Old smatch warnings:
drivers/video/fbdev/core/fbmem.c:1660 do_register_framebuffer() error: buffer overflow 'registered_fb' 32 <= 32 (assuming for loop doesn't break)

vim +1389 drivers/video/fbdev/core/fbmem.c

^1da177e4c3f41 drivers/video/fbmem.c            Linus Torvalds    2005-04-16  1333  static int
^1da177e4c3f41 drivers/video/fbmem.c            Linus Torvalds    2005-04-16  1334  fb_mmap(struct file *file, struct vm_area_struct * vma)
^1da177e4c3f41 drivers/video/fbmem.c            Linus Torvalds    2005-04-16  1335  {
c47747fde931c0 drivers/video/fbmem.c            Linus Torvalds    2011-05-11  1336  	struct fb_info *info = file_fb_info(file);
fc9bbca8f650e5 drivers/video/fbmem.c            Linus Torvalds    2013-04-19  1337  	unsigned long mmio_pgoff;
^1da177e4c3f41 drivers/video/fbmem.c            Linus Torvalds    2005-04-16  1338  	unsigned long start;
^1da177e4c3f41 drivers/video/fbmem.c            Linus Torvalds    2005-04-16  1339  	u32 len;
^1da177e4c3f41 drivers/video/fbmem.c            Linus Torvalds    2005-04-16  1340  
c47747fde931c0 drivers/video/fbmem.c            Linus Torvalds    2011-05-11  1341  	if (!info)
c47747fde931c0 drivers/video/fbmem.c            Linus Torvalds    2011-05-11  1342  		return -ENODEV;
537a1bf059fa31 drivers/video/fbmem.c            Krzysztof Helt    2009-06-30  1343  	mutex_lock(&info->mm_lock);
12281c8dda5a3b drivers/video/fbdev/core/fbmem.c Jani Nikula       2019-11-29  1344  
8a9ea404b61faa drivers/video/fbdev/core/fbmem.c Thomas Zimmermann 2022-04-26  1345  	if (info->fbops->fb_mmap) {
^1da177e4c3f41 drivers/video/fbmem.c            Linus Torvalds    2005-04-16  1346  		int res;
95cf9264d5f36c drivers/video/fbdev/core/fbmem.c Tom Lendacky      2017-07-17  1347  
95cf9264d5f36c drivers/video/fbdev/core/fbmem.c Tom Lendacky      2017-07-17  1348  		/*
95cf9264d5f36c drivers/video/fbdev/core/fbmem.c Tom Lendacky      2017-07-17  1349  		 * The framebuffer needs to be accessed decrypted, be sure
95cf9264d5f36c drivers/video/fbdev/core/fbmem.c Tom Lendacky      2017-07-17  1350  		 * SME protection is removed ahead of the call
95cf9264d5f36c drivers/video/fbdev/core/fbmem.c Tom Lendacky      2017-07-17  1351  		 */
95cf9264d5f36c drivers/video/fbdev/core/fbmem.c Tom Lendacky      2017-07-17  1352  		vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
8a9ea404b61faa drivers/video/fbdev/core/fbmem.c Thomas Zimmermann 2022-04-26  1353  		res = info->fbops->fb_mmap(info, vma);
537a1bf059fa31 drivers/video/fbmem.c            Krzysztof Helt    2009-06-30  1354  		mutex_unlock(&info->mm_lock);
^1da177e4c3f41 drivers/video/fbmem.c            Linus Torvalds    2005-04-16  1355  		return res;
8a9ea404b61faa drivers/video/fbdev/core/fbmem.c Thomas Zimmermann 2022-04-26  1356  #if IS_ENABLED(CONFIG_FB_DEFERRED_IO)
8a9ea404b61faa drivers/video/fbdev/core/fbmem.c Thomas Zimmermann 2022-04-26  1357  	} else if (info->fbdefio) {
8a9ea404b61faa drivers/video/fbdev/core/fbmem.c Thomas Zimmermann 2022-04-26  1358  		/*
8a9ea404b61faa drivers/video/fbdev/core/fbmem.c Thomas Zimmermann 2022-04-26  1359  		 * FB deferred I/O wants you to handle mmap in your drivers. At a
8a9ea404b61faa drivers/video/fbdev/core/fbmem.c Thomas Zimmermann 2022-04-26  1360  		 * minimum, point struct fb_ops.fb_mmap to fb_deferred_io_mmap().
8a9ea404b61faa drivers/video/fbdev/core/fbmem.c Thomas Zimmermann 2022-04-26  1361  		 */
8a9ea404b61faa drivers/video/fbdev/core/fbmem.c Thomas Zimmermann 2022-04-26  1362  		dev_warn_once(info->dev, "fbdev mmap not set up for deferred I/O.\n");
8a9ea404b61faa drivers/video/fbdev/core/fbmem.c Thomas Zimmermann 2022-04-26  1363  		return -ENODEV;

mutex_unlock(&info->mm_lock); before the return.

8a9ea404b61faa drivers/video/fbdev/core/fbmem.c Thomas Zimmermann 2022-04-26  1364  #endif
^1da177e4c3f41 drivers/video/fbmem.c            Linus Torvalds    2005-04-16  1365  	}
^1da177e4c3f41 drivers/video/fbmem.c            Linus Torvalds    2005-04-16  1366  
fc9bbca8f650e5 drivers/video/fbmem.c            Linus Torvalds    2013-04-19  1367  	/*
fc9bbca8f650e5 drivers/video/fbmem.c            Linus Torvalds    2013-04-19  1368  	 * Ugh. This can be either the frame buffer mapping, or
fc9bbca8f650e5 drivers/video/fbmem.c            Linus Torvalds    2013-04-19  1369  	 * if pgoff points past it, the mmio mapping.
fc9bbca8f650e5 drivers/video/fbmem.c            Linus Torvalds    2013-04-19  1370  	 */
^1da177e4c3f41 drivers/video/fbmem.c            Linus Torvalds    2005-04-16  1371  	start = info->fix.smem_start;
fc9bbca8f650e5 drivers/video/fbmem.c            Linus Torvalds    2013-04-19  1372  	len = info->fix.smem_len;
fc9bbca8f650e5 drivers/video/fbmem.c            Linus Torvalds    2013-04-19  1373  	mmio_pgoff = PAGE_ALIGN((start & ~PAGE_MASK) + len) >> PAGE_SHIFT;
fc9bbca8f650e5 drivers/video/fbmem.c            Linus Torvalds    2013-04-19  1374  	if (vma->vm_pgoff >= mmio_pgoff) {
138f296e140f79 drivers/video/fbmem.c            Tomi Valkeinen    2013-04-24  1375  		if (info->var.accel_flags) {
138f296e140f79 drivers/video/fbmem.c            Tomi Valkeinen    2013-04-24  1376  			mutex_unlock(&info->mm_lock);
138f296e140f79 drivers/video/fbmem.c            Tomi Valkeinen    2013-04-24  1377  			return -EINVAL;
138f296e140f79 drivers/video/fbmem.c            Tomi Valkeinen    2013-04-24  1378  		}
138f296e140f79 drivers/video/fbmem.c            Tomi Valkeinen    2013-04-24  1379  
fc9bbca8f650e5 drivers/video/fbmem.c            Linus Torvalds    2013-04-19  1380  		vma->vm_pgoff -= mmio_pgoff;
^1da177e4c3f41 drivers/video/fbmem.c            Linus Torvalds    2005-04-16  1381  		start = info->fix.mmio_start;
fc9bbca8f650e5 drivers/video/fbmem.c            Linus Torvalds    2013-04-19  1382  		len = info->fix.mmio_len;
^1da177e4c3f41 drivers/video/fbmem.c            Linus Torvalds    2005-04-16  1383  	}
537a1bf059fa31 drivers/video/fbmem.c            Krzysztof Helt    2009-06-30  1384  	mutex_unlock(&info->mm_lock);
fc9bbca8f650e5 drivers/video/fbmem.c            Linus Torvalds    2013-04-19  1385  
c07fbfd17e614a drivers/video/fbmem.c            Daniel De Graaf   2010-08-10  1386  	vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
fc9bbca8f650e5 drivers/video/fbmem.c            Linus Torvalds    2013-04-19  1387  	fb_pgprotect(file, vma, start);
fc9bbca8f650e5 drivers/video/fbmem.c            Linus Torvalds    2013-04-19  1388  
fc9bbca8f650e5 drivers/video/fbmem.c            Linus Torvalds    2013-04-19 @1389  	return vm_iomap_memory(vma, start, len);
^1da177e4c3f41 drivers/video/fbmem.c            Linus Torvalds    2005-04-16  1390  }
diff mbox series

Patch

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index d265a73313c9..d06ce0e92d66 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -2118,7 +2118,9 @@  static int drm_fbdev_fb_mmap(struct fb_info *info, struct vm_area_struct *vma)
 {
 	struct drm_fb_helper *fb_helper = info->par;
 
-	if (fb_helper->dev->driver->gem_prime_mmap)
+	if (drm_fbdev_use_shadow_fb(fb_helper))
+		return fb_deferred_io_mmap(info, vma);
+	else if (fb_helper->dev->driver->gem_prime_mmap)
 		return fb_helper->dev->driver->gem_prime_mmap(fb_helper->buffer->gem, vma);
 	else
 		return -ENODEV;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c b/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
index adf17c740656..02a4a4fa3da3 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_fb.c
@@ -619,6 +619,7 @@  static const struct fb_ops vmw_fb_ops = {
 	.fb_imageblit = vmw_fb_imageblit,
 	.fb_pan_display = vmw_fb_pan_display,
 	.fb_blank = vmw_fb_blank,
+	.fb_mmap = fb_deferred_io_mmap,
 };
 
 int vmw_fb_init(struct vmw_private *vmw_priv)
diff --git a/drivers/hid/hid-picolcd_fb.c b/drivers/hid/hid-picolcd_fb.c
index 33c102a60992..78515e6bacf0 100644
--- a/drivers/hid/hid-picolcd_fb.c
+++ b/drivers/hid/hid-picolcd_fb.c
@@ -428,6 +428,7 @@  static const struct fb_ops picolcdfb_ops = {
 	.fb_imageblit = picolcd_fb_imageblit,
 	.fb_check_var = picolcd_fb_check_var,
 	.fb_set_par   = picolcd_set_par,
+	.fb_mmap      = fb_deferred_io_mmap,
 };
 
 
diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c
index 9c4d797e7ae4..d4e14f7c9d57 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -652,6 +652,7 @@  struct fb_info *fbtft_framebuffer_alloc(struct fbtft_display *display,
 	fbops->fb_imageblit =      fbtft_fb_imageblit;
 	fbops->fb_setcolreg =      fbtft_fb_setcolreg;
 	fbops->fb_blank     =      fbtft_fb_blank;
+	fbops->fb_mmap      =      fb_deferred_io_mmap;
 
 	fbdefio->delay =           HZ / fps;
 	fbdefio->sort_pagelist =   true;
diff --git a/drivers/video/fbdev/broadsheetfb.c b/drivers/video/fbdev/broadsheetfb.c
index b9054f658838..528bc0902338 100644
--- a/drivers/video/fbdev/broadsheetfb.c
+++ b/drivers/video/fbdev/broadsheetfb.c
@@ -1055,6 +1055,7 @@  static const struct fb_ops broadsheetfb_ops = {
 	.fb_fillrect	= broadsheetfb_fillrect,
 	.fb_copyarea	= broadsheetfb_copyarea,
 	.fb_imageblit	= broadsheetfb_imageblit,
+	.fb_mmap	= fb_deferred_io_mmap,
 };
 
 static struct fb_deferred_io broadsheetfb_defio = {
diff --git a/drivers/video/fbdev/core/fb_defio.c b/drivers/video/fbdev/core/fb_defio.c
index 6aaf6d0abf39..6924d489a289 100644
--- a/drivers/video/fbdev/core/fb_defio.c
+++ b/drivers/video/fbdev/core/fb_defio.c
@@ -181,6 +181,7 @@  int fb_deferred_io_mmap(struct fb_info *info, struct vm_area_struct *vma)
 	vma->vm_private_data = info;
 	return 0;
 }
+EXPORT_SYMBOL_GPL(fb_deferred_io_mmap);
 
 /* workqueue callback */
 static void fb_deferred_io_work(struct work_struct *work)
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 84427470367b..e6b625548927 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -1334,7 +1334,6 @@  static int
 fb_mmap(struct file *file, struct vm_area_struct * vma)
 {
 	struct fb_info *info = file_fb_info(file);
-	int (*fb_mmap_fn)(struct fb_info *info, struct vm_area_struct *vma);
 	unsigned long mmio_pgoff;
 	unsigned long start;
 	u32 len;
@@ -1343,14 +1342,7 @@  fb_mmap(struct file *file, struct vm_area_struct * vma)
 		return -ENODEV;
 	mutex_lock(&info->mm_lock);
 
-	fb_mmap_fn = info->fbops->fb_mmap;
-
-#if IS_ENABLED(CONFIG_FB_DEFERRED_IO)
-	if (info->fbdefio)
-		fb_mmap_fn = fb_deferred_io_mmap;
-#endif
-
-	if (fb_mmap_fn) {
+	if (info->fbops->fb_mmap) {
 		int res;
 
 		/*
@@ -1358,9 +1350,18 @@  fb_mmap(struct file *file, struct vm_area_struct * vma)
 		 * SME protection is removed ahead of the call
 		 */
 		vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
-		res = fb_mmap_fn(info, vma);
+		res = info->fbops->fb_mmap(info, vma);
 		mutex_unlock(&info->mm_lock);
 		return res;
+#if IS_ENABLED(CONFIG_FB_DEFERRED_IO)
+	} else if (info->fbdefio) {
+		/*
+		 * FB deferred I/O wants you to handle mmap in your drivers. At a
+		 * minimum, point struct fb_ops.fb_mmap to fb_deferred_io_mmap().
+		 */
+		dev_warn_once(info->dev, "fbdev mmap not set up for deferred I/O.\n");
+		return -ENODEV;
+#endif
 	}
 
 	/*
diff --git a/drivers/video/fbdev/hecubafb.c b/drivers/video/fbdev/hecubafb.c
index 00d77105161a..2c483e2cf9ec 100644
--- a/drivers/video/fbdev/hecubafb.c
+++ b/drivers/video/fbdev/hecubafb.c
@@ -204,6 +204,7 @@  static const struct fb_ops hecubafb_ops = {
 	.fb_fillrect	= hecubafb_fillrect,
 	.fb_copyarea	= hecubafb_copyarea,
 	.fb_imageblit	= hecubafb_imageblit,
+	.fb_mmap	= fb_deferred_io_mmap,
 };
 
 static struct fb_deferred_io hecubafb_defio = {
diff --git a/drivers/video/fbdev/hyperv_fb.c b/drivers/video/fbdev/hyperv_fb.c
index c8e0ea27caf1..d7f6abf827b9 100644
--- a/drivers/video/fbdev/hyperv_fb.c
+++ b/drivers/video/fbdev/hyperv_fb.c
@@ -909,6 +909,7 @@  static const struct fb_ops hvfb_ops = {
 	.fb_copyarea = hvfb_cfb_copyarea,
 	.fb_imageblit = hvfb_cfb_imageblit,
 	.fb_blank = hvfb_blank,
+	.fb_mmap = fb_deferred_io_mmap,
 };
 
 
diff --git a/drivers/video/fbdev/metronomefb.c b/drivers/video/fbdev/metronomefb.c
index af858dd23ea6..2541f2fe065b 100644
--- a/drivers/video/fbdev/metronomefb.c
+++ b/drivers/video/fbdev/metronomefb.c
@@ -564,6 +564,7 @@  static const struct fb_ops metronomefb_ops = {
 	.fb_fillrect	= metronomefb_fillrect,
 	.fb_copyarea	= metronomefb_copyarea,
 	.fb_imageblit	= metronomefb_imageblit,
+	.fb_mmap	= fb_deferred_io_mmap,
 };
 
 static struct fb_deferred_io metronomefb_defio = {
diff --git a/drivers/video/fbdev/sh_mobile_lcdcfb.c b/drivers/video/fbdev/sh_mobile_lcdcfb.c
index aa4ebe3192ec..1dc5079e4518 100644
--- a/drivers/video/fbdev/sh_mobile_lcdcfb.c
+++ b/drivers/video/fbdev/sh_mobile_lcdcfb.c
@@ -1483,6 +1483,9 @@  sh_mobile_lcdc_overlay_mmap(struct fb_info *info, struct vm_area_struct *vma)
 {
 	struct sh_mobile_lcdc_overlay *ovl = info->par;
 
+	if (info->fbdefio)
+		return fb_deferred_io_mmap(info, vma);
+
 	return dma_mmap_coherent(ovl->channel->lcdc->dev, vma, ovl->fb_mem,
 				 ovl->dma_handle, ovl->fb_size);
 }
@@ -1957,6 +1960,9 @@  sh_mobile_lcdc_mmap(struct fb_info *info, struct vm_area_struct *vma)
 {
 	struct sh_mobile_lcdc_chan *ch = info->par;
 
+	if (info->fbdefio)
+		return fb_deferred_io_mmap(info, vma);
+
 	return dma_mmap_coherent(ch->lcdc->dev, vma, ch->fb_mem,
 				 ch->dma_handle, ch->fb_size);
 }
diff --git a/drivers/video/fbdev/smscufx.c b/drivers/video/fbdev/smscufx.c
index 28768c272b73..9383f8d0914c 100644
--- a/drivers/video/fbdev/smscufx.c
+++ b/drivers/video/fbdev/smscufx.c
@@ -779,6 +779,9 @@  static int ufx_ops_mmap(struct fb_info *info, struct vm_area_struct *vma)
 	unsigned long offset = vma->vm_pgoff << PAGE_SHIFT;
 	unsigned long page, pos;
 
+	if (info->fbdefio)
+		return fb_deferred_io_mmap(info, vma);
+
 	if (vma->vm_pgoff > (~0UL >> PAGE_SHIFT))
 		return -EINVAL;
 	if (size > info->fix.smem_len)
diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
index c6d5df31111d..7547b4628afc 100644
--- a/drivers/video/fbdev/ssd1307fb.c
+++ b/drivers/video/fbdev/ssd1307fb.c
@@ -368,6 +368,7 @@  static const struct fb_ops ssd1307fb_ops = {
 	.fb_fillrect	= ssd1307fb_fillrect,
 	.fb_copyarea	= ssd1307fb_copyarea,
 	.fb_imageblit	= ssd1307fb_imageblit,
+	.fb_mmap	= fb_deferred_io_mmap,
 };
 
 static void ssd1307fb_deferred_io(struct fb_info *info,
diff --git a/drivers/video/fbdev/udlfb.c b/drivers/video/fbdev/udlfb.c
index b6ec0b8e2b72..9e5a33396ef9 100644
--- a/drivers/video/fbdev/udlfb.c
+++ b/drivers/video/fbdev/udlfb.c
@@ -326,6 +326,9 @@  static int dlfb_ops_mmap(struct fb_info *info, struct vm_area_struct *vma)
 	unsigned long offset = vma->vm_pgoff << PAGE_SHIFT;
 	unsigned long page, pos;
 
+	if (info->fbdefio)
+		return fb_deferred_io_mmap(info, vma);
+
 	if (vma->vm_pgoff > (~0UL >> PAGE_SHIFT))
 		return -EINVAL;
 	if (size > info->fix.smem_len)
diff --git a/drivers/video/fbdev/xen-fbfront.c b/drivers/video/fbdev/xen-fbfront.c
index 6826f986da43..6c8846eba2fb 100644
--- a/drivers/video/fbdev/xen-fbfront.c
+++ b/drivers/video/fbdev/xen-fbfront.c
@@ -338,6 +338,7 @@  static const struct fb_ops xenfb_fb_ops = {
 	.fb_imageblit	= xenfb_imageblit,
 	.fb_check_var	= xenfb_check_var,
 	.fb_set_par     = xenfb_set_par,
+	.fb_mmap	= fb_deferred_io_mmap,
 };
 
 static irqreturn_t xenfb_event_handler(int rq, void *dev_id)