[PATCH] drm/fbdev-dma: Only set smem_start is enable per module option

[Daniel Vetter]

On Mon, Jun 17, 2024 at 05:26:37PM +0200, Thomas Zimmermann wrote:
> Only export struct fb_info.fix.smem_start if that is required by the
> user and the memory does not come from vmalloc().
> 
> Setting struct fb_info.fix.smem_start breaks systems where DMA
> memory is backed by vmalloc address space. An example error is
> shown below.
> 
> [    3.536043] ------------[ cut here ]------------
> [    3.540716] virt_to_phys used for non-linear address: 000000007fc4f540 (0xffff800086001000)
> [    3.552628] WARNING: CPU: 4 PID: 61 at arch/arm64/mm/physaddr.c:12 __virt_to_phys+0x68/0x98
> [    3.565455] Modules linked in:
> [    3.568525] CPU: 4 PID: 61 Comm: kworker/u12:5 Not tainted 6.6.23-06226-g4986cc3e1b75-dirty #250
> [    3.577310] Hardware name: NXP i.MX95 19X19 board (DT)
> [    3.582452] Workqueue: events_unbound deferred_probe_work_func
> [    3.588291] pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [    3.595233] pc : __virt_to_phys+0x68/0x98
> [    3.599246] lr : __virt_to_phys+0x68/0x98
> [    3.603276] sp : ffff800083603990
> [    3.677939] Call trace:
> [    3.680393]  __virt_to_phys+0x68/0x98
> [    3.684067]  drm_fbdev_dma_helper_fb_probe+0x138/0x238
> [    3.689214]  __drm_fb_helper_initial_config_and_unlock+0x2b0/0x4c0
> [    3.695385]  drm_fb_helper_initial_config+0x4c/0x68
> [    3.700264]  drm_fbdev_dma_client_hotplug+0x8c/0xe0
> [    3.705161]  drm_client_register+0x60/0xb0
> [    3.709269]  drm_fbdev_dma_setup+0x94/0x148
> 
> Additionally, DMA memory is assumed to by contiguous in physical
> address space, which is not guaranteed by vmalloc().
> 
> Resolve this by checking the module flag drm_leak_fbdev_smem when
> DRM allocated the instance of struct fb_info. Fbdev-dma then only
> sets smem_start only if required (via FBINFO_HIDE_SMEM_START). Also
> guarantee that the framebuffer is not located in vmalloc address
> space.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
> Reported-by: Peng Fan (OSS) <peng.fan at oss.nxp.com>
> Closes: https://lore.kernel.org/dri-devel/20240604080328.4024838-1-peng.fan@oss.nxp.com/
> Fixes: b79fe9abd58b ("drm/fbdev-dma: Implement fbdev emulation for GEM DMA helpers")
> ---
>  drivers/gpu/drm/drm_fb_helper.c | 6 +++---
>  drivers/gpu/drm/drm_fbdev_dma.c | 5 ++++-
>  2 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index e2e19f49342e1..18565ec684517 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -517,6 +517,9 @@ struct fb_info *drm_fb_helper_alloc_info(struct drm_fb_helper *fb_helper)
>  	if (!info)
>  		return ERR_PTR(-ENOMEM);
>  
> +	if (!drm_leak_fbdev_smem)
> +		info->flags |= FBINFO_HIDE_SMEM_START;
> +
>  	ret = fb_alloc_cmap(&info->cmap, 256, 0);
>  	if (ret)
>  		goto err_release;
> @@ -1853,9 +1856,6 @@ __drm_fb_helper_initial_config_and_unlock(struct drm_fb_helper *fb_helper)
>  	info = fb_helper->info;
>  	info->var.pixclock = 0;
>  
> -	if (!drm_leak_fbdev_smem)
> -		info->flags |= FBINFO_HIDE_SMEM_START;
> -
>  	/* Need to drop locks to avoid recursive deadlock in
>  	 * register_framebuffer. This is ok because the only thing left to do is
>  	 * register the fbdev emulation instance in kernel_fb_helper_list. */
> diff --git a/drivers/gpu/drm/drm_fbdev_dma.c b/drivers/gpu/drm/drm_fbdev_dma.c
> index 97ef6300d47e0..4b0527ddeb5a7 100644
> --- a/drivers/gpu/drm/drm_fbdev_dma.c
> +++ b/drivers/gpu/drm/drm_fbdev_dma.c
> @@ -137,7 +137,10 @@ 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_start = page_to_phys(virt_to_page(info->screen_buffer));
> +	if (!(info->flags & FBINFO_HIDE_SMEM_START)) {

I guess since the WARN is conditional on the explicit opt-in with
drm_leak_fbdev_smem it should be fine.

Reviewed-by: Daniel Vetter <daniel.vetter at ffwll.ch>

> +		if (!drm_WARN_ON(dev, is_vmalloc_addr(info->screen_buffer)))
> +			info->fix.smem_start = page_to_phys(virt_to_page(info->screen_buffer));
> +	}
>  	info->fix.smem_len = info->screen_size;
>  
>  	/* deferred I/O */
> -- 
> 2.45.2
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch