Commit a51c7663 authored by Thomas Zimmermann's avatar Thomas Zimmermann
Browse files

drm/fb-helper: Consolidate CONFIG_DRM_FBDEV_LEAK_PHYS_SMEM 



Consolidate all handling of CONFIG_DRM_FBDEV_LEAK_PHYS_SMEM by
making the module parameter optional in drm_fb_helper.c.

Without the config option, modules can set smem_start in struct
fb_info for internal usage, but not export if to userspace. The
address can only be exported by enabling the option and setting
the module parameter. Also update the comment.

Signed-off-by: default avatarThomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: default avatarJavier Martinez Canillas <javierm@redhat.com>
Acked-by: default avatarZack Rusin <zackr@vmware.com>
Tested-by: default avatarSui <Jingfeng&lt;suijingfeng@loongson.cn>
Link: https://patchwork.freedesktop.org/patch/msgid/20230320150751.20399-8-tzimmermann@suse.de
parent 61b5d007

drivers/gpu/drm/drm_fb_helper.c

+8 −14
Original line number Diff line number Diff line
@@ -60,16 +60,17 @@ MODULE_PARM_DESC(drm_fbdev_overalloc, 
 * In order to keep user-space compatibility, we want in certain use-cases

 * to keep leaking the fbdev physical address to the user-space program

 * handling the fbdev buffer.

 * This is a bad habit essentially kept into closed source opengl driver

 * that should really be moved into open-source upstream projects instead

 * of using legacy physical addresses in user space to communicate with

 * other out-of-tree kernel modules.

 *

 * This is a bad habit, essentially kept to support closed-source OpenGL

 * drivers that should really be moved into open-source upstream projects

 * instead of using legacy physical addresses in user space to communicate

 * with other out-of-tree kernel modules.

 *

 * This module_param *should* be removed as soon as possible and be

 * considered as a broken and legacy behaviour from a modern fbdev device.

 */

#if IS_ENABLED(CONFIG_DRM_FBDEV_LEAK_PHYS_SMEM)

static bool drm_leak_fbdev_smem;

#if IS_ENABLED(CONFIG_DRM_FBDEV_LEAK_PHYS_SMEM)

module_param_unsafe(drm_leak_fbdev_smem, bool, 0600);

MODULE_PARM_DESC(drm_leak_fbdev_smem,

		 "Allow unsafe leaking fbdev physical smem address [default=false]");
@@ -1983,10 +1984,6 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper) 
		return ret;

	}



#if IS_ENABLED(CONFIG_DRM_FBDEV_LEAK_PHYS_SMEM)

	fb_helper->hint_leak_smem_start = drm_leak_fbdev_smem;

#endif



	/* push down into drivers */

	ret = (*fb_helper->funcs->fb_probe)(fb_helper, &sizes);

	if (ret < 0)
@@ -2185,11 +2182,8 @@ __drm_fb_helper_initial_config_and_unlock(struct drm_fb_helper *fb_helper) 


	info = fb_helper->info;

	info->var.pixclock = 0;

	/* Shamelessly allow physical address leaking to userspace */

#if IS_ENABLED(CONFIG_DRM_FBDEV_LEAK_PHYS_SMEM)

	if (!fb_helper->hint_leak_smem_start)

#endif

		/* don't leak any physical addresses to userspace */



	if (!drm_leak_fbdev_smem)

		info->flags |= FBINFO_HIDE_SMEM_START;



	/* Need to drop locks to avoid recursive deadlock in

drivers/gpu/drm/drm_fbdev_dma.c

+1 −8
Original line number Diff line number Diff line
@@ -136,15 +136,8 @@ static int drm_fbdev_dma_helper_fb_probe(struct drm_fb_helper *fb_helper, 
		info->flags |= FBINFO_READS_FAST; /* signal caching */

	info->screen_size = sizes->surface_height * fb->pitches[0];

	info->screen_buffer = map.vaddr;

	info->fix.smem_len = info->screen_size;



#if IS_ENABLED(CONFIG_DRM_FBDEV_LEAK_PHYS_SMEM)

	/*

	 * Shamelessly leak the physical address to user-space.

	 */

	if (fb_helper->hint_leak_smem_start && !info->fix.smem_start)

	info->fix.smem_start = page_to_phys(virt_to_page(info->screen_buffer));

#endif

	info->fix.smem_len = info->screen_size;



	return 0;

include/drm/drm_fb_helper.h

+0 −9
Original line number Diff line number Diff line
@@ -195,15 +195,6 @@ struct drm_fb_helper { 
	 */

	int preferred_bpp;



	/**

	 * @hint_leak_smem_start:

	 *

	 * Hint to the fbdev emulation to store the framebuffer's physical

	 * address in struct &fb_info.fix.smem_start. If the hint is unset,

	 * the smem_start field should always be cleared to zero.

	 */

	bool hint_leak_smem_start;



#ifdef CONFIG_FB_DEFERRED_IO

	/**

	 * @fbdefio: