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@xxxxxxx> > Reported-by: Peng Fan (OSS) <peng.fan@xxxxxxxxxxx> > Closes: https://lore.kernel.org/dri-devel/20240604080328.4024838-1-peng.fan@xxxxxxxxxxx/ > 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@xxxxxxxx> > + 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
